ufront / ufront-mvc

The core MVC framework that powers ufront
MIT License
17 stars 15 forks source link

Array of typedefs as controller arugment #48

Closed kevinresol closed 6 years ago

kevinresol commented 8 years ago
// MyController.hx
@:route(POST, "/create")
public function create(args:{name:String, objects:Array<{name:String, value:String}>}) 
{
  //...
}

This can be submitted through html forms like this:

<form>
  <input name="name">
  <input name="objects[0][name]">
  <input name="objects[0][value]">
  <input name="objects[1][name]">
  <input name="objects[1][value]">
<form>

Noted from the doc this is not supported by design Is there a chance to relax this limitation? Because on nodejs the arguments can be posted as json (and parsed by bodyParser.json()), which means the structure of the arguments can be almost anything

jasononeil commented 8 years ago

A few quick thoughts, we can discuss more later:

request.post.set('name', "Ufront");
request.post.set("objects[0][name]", "Jason");
request.post.set("objects[0][value]", "jasononeil");
request.post.set("objects[1][name]", "Kevin");
request.post.set("objects[1][value]", "kevinresol");

What do you think of this approach? It means it would work for your controller arguments, but might be more confusing if you were trying to read the objects directly from the HttpRequest.

Importantly though I think we could achieve this without breaking compatibility...

kevinresol commented 8 years ago

@jasononeil That sounds very good!

kevinresol commented 8 years ago

I just had a re-think about this.

In fact I found that MultiValueMap.combine() has been somewhat problematic since the day we allowed arrays of Int/String/etc as controller function parameters.

Say I have a query string name=Kevin&name=Jason, also I have a cookie name=John, when I define a parameter name:Array<String> in controller I will eventually get ["John","Kevin","Jason"] because the controller macro getAll('name') for me from the combined map.

But the doc says that query should has higher priority and should (as I understand) overwrite the cookie entry. So maybe I will expect getting ["Kevin","Jason"] instead. And this kind of problem will get even more complicated if we want to support array of objects.

I have a loose proposal in mind but not sure if it will work well, because I am not very sure about the original rationale behind MultiValueMap and its combine() function. I propose that combine() should not really combine the maps into a single one. Instead, all the "combined" maps stays as individual maps behind the scene. When a field is accessed, the list of maps are checked one by one (higher-priority one comes first) to see if they have that field. If yes, return it and then stop. So that we will get ["Kevin","Jason"] in the example above.

While the above proposal may solve the priority issue, we still face another issue about the representation of data. I think that representing complex data using string-with-square-bracket (e.g. "objects[0][name]", "objects[1][name]",etc) is not the best idea. Not only that it is not memory-efficient, because it has a lot of text duplication. Also, it is not computation-friendly. Suppose we want to know the length of object (given it is an array), we basically need to read through all the map keys to make sure we count all the "objects[1][name]", "objects[2][name]", "objects[3][name]", etc. On the other hand, if the data is represented in an anon obj (or json), we only need to grab out the "object" key and work on that. Yes, what I want to bring out is that IMHO MultiValueMap should probably be replaced. tink_json seems to be a good candidate.

@jasononeil Seems that you are busy, but I will be really glad to hear from you.

Surely anyone interested in improving ufront are also welcome!

kevinresol commented 8 years ago

What if, besides args:{}, we introduce cookie:{}, query:{} and post:{}, to allow users specify their needs explicitly?

kevinresol commented 6 years ago

Cleaning up my old issues and I think this is no longer needed.

varadig commented 5 years ago

@kevinresol you found any solution for use typedef as argument value? I really need this for make type safe request, but the only thing is waht I can do no is, to pass json string from client side, and parse on the server side back.

kevinresol commented 5 years ago

Nope sorry. I haven't used ufront for a long time. My focus is on tink_web now.

varadig commented 5 years ago

Thanks, I will check tink_web to it is it satisfies my needs.