grails / grails-core

The Grails Web Application Framework
http://grails.org
Apache License 2.0
2.79k stars 952 forks source link

Grails 3.1 RC1: Forward(params) isn't sending changes made to params variables #9525

Closed orubel closed 8 years ago

orubel commented 8 years ago

A recent fix for custom URI's went in for RequestForwarder and when calling at the handlerinterceptor via forward(params), any changes made to params are lost upon forward.

Previously, a forward could pass modified params in 2.x.

It looks as if injected params are being lost somehow?

What is suggested workaround?

Grails application running at http://localhost:8080 in environment: development
##### BATCHINTERCEPTOR (BEFORE)
#### [ParamsService : initParams ] ####
initial batch setting...
apiBatch:null  // <-- sent POST vars are null; have not been set yet
reset batch params...  // <-- if variable is NULL, will rest
after:[controller:section, action:create, entryPoint:b0.1, method:POST, encoding:null, contentType:application/json, uri:/b0.1/section/create, format:JSON, apiBatch:[[sectionName:Forp], [sectionName:Dorp], [sectionName:Morp]], sectionName:Gorp]
##### BATCHFILTER (AFTER)
forwarding....
##### BATCHINTERCEPTOR (BEFORE)
#### [ParamsService : initParams ] ####
initial batch setting...
apiBatch:null <-- forwarded params with injection and is now NULL????
reset batch params... <-- resetting using existing request
after:[controller:section, action:create, entryPoint:b0.1, method:POST, encoding:null, contentType:application/json, uri:/b0.1/section/create, format:JSON, apiBatch:[[sectionName:Forp], [sectionName:Dorp], [sectionName:Morp]], sectionName:Gorp]
controller : section
#### [ApiRequestService : handleApiRequest ] ####
***** [section/create] *****
[controller:section, action:create, entryPoint:b0.1, method:POST, encoding:null, contentType:application/json, uri:/b0.1/section/create, format:JSON, apiBatch:[[sectionName:Forp], [sectionName:Dorp], [sectionName:Morp]], sectionName:Gorp, apiObject:1]
jeffscottbrown commented 8 years ago

What is suggested workaround?

@orubel We can take a look and see what might be going on but in general you probably shouldn't be mutating the servlet request parameters anyway. I would create a new Map and put it in the state you want and then use that.

orubel commented 8 years ago

Oh sorry. Documentation says we can add/remove from params; not modifying request though.

I may be dumb but can you describe what you are talking about with map in state? I don't understand.

jeffscottbrown commented 8 years ago

I may be dumb but can you describe what you are talking about with map in state? I don't understand.

I mean create a Map, populate it with whatever you want to be in it, and then use that rather than mutating the Map of parameters that is associated with the servlet request.

orubel commented 8 years ago

Well I need something that will live from forward to forward, redirect to redirect, exists across all controllers, services, etc. Thats params.

Should I just use a map for forwards and add to params everytime? That could be a quick fix for the short term.

jeffscottbrown commented 8 years ago

Is there something like params that I can use?

Yes. A Map that you create and populate with whatever values you like.

I would like for this discussion and series of questions to take place on the mailing list, not the issue tracker. Please ask questions there.

Thank you very much for your cooperation.

ctoestreich commented 8 years ago

@orubel Things that live across requests and scopes should be appropriately scoped into a session store perhaps. The best way to deal with this, as @jeffbrown mentioned above, without dirtying a global session would be to shuttle these params around on every request. This would just take more time, code and energy.

Also I think what Jeff might be impling above is to not directly mutate the params object but to pass something like forward([uri: params.uri, var1: params.var1]) which would be creating a new map from the params map. Or in general using the map as a lookup reference not a realtime property bucket. The way you did it in code before by just handing in the params to the forward method is fine as that method doesn't mutate the map at all and the params is, under the covers, just a map already.

orubel commented 8 years ago

I didn't send the forward variables in a map like that so I'll give it a shot.

The usage currently is :

forward(params)

This doesn't give one the ability to implement like that. I initially tried to send in that manner and even as

forward(params:params)

but both of those failed. This was why I was puzzled as to why forward is having these issues. Would love to forward variables with a custom uri but right now I am not seeing how.

Maybe if I play around a bit more. I may just not be seeing it. I'll keep looking.

jeffscottbrown commented 8 years ago

See https://github.com/jeffbrown/forwardparams

orubel commented 8 years ago

@ctoestreich Ok in looking at LingGenerator, 'vars' isn't an accepted variable but params is and it fails in the map...

This works...

forward([uri: params.uri])

but these fail...

forward([uri: params.uri, var1: params.var1])
forward([uri: params.uri, params: params])
orubel commented 8 years ago

@jeffbrown I may be wrong but RequestForwarder uses LinkGenerator and as a result, it should be able to implement the following using 'link' (according to its comment)

/**
 * Generates a link to a controller, action or URI for the given named parameters.
 *
 * Possible named parameters include:
 *
 * <ul>
 *    <li>resource - If linking to a REST resource, the name of the resource or resource path to link to. Either 'resource' or 'controller' should be specified, but not both</li>
 *    <li>controller - The name of the controller to use in the link, if not specified the current controller will be linked</li>
 *    <li>action -  The name of the action to use in the link, if not specified the default action will be linked</li>
 *    <li>uri -  relative URI</li>
 *    <li>url -  A map containing the action,controller,id etc.</li>
 *    <li>base -  Sets the prefix to be added to the link target address, typically an absolute server URL. This overrides the behaviour of the absolute property, if both are specified.</li>
 *    <li>absolute -  If set to "true" will prefix the link target address with the value of the grails.serverURL property from Config, or http://localhost:&lt;port&gt; if no value in Config and not running in production.</li>
 *    <li>contextPath - The context path to link to, defaults to the servlet context path</li>
 *    <li>id -  The id to use in the link</li>
 *    <li>fragment -  The link fragment (often called anchor tag) to use</li>
 *    <li>params -  A map containing URL query parameters</li>
 *    <li>mapping -  The named URL mapping to use to rewrite the link</li>
 *    <li>event -  Webflow _eventId parameter</li>
 * </ul>
 * @param params The named parameters
 * @return The generator link
 */
jeffscottbrown commented 8 years ago

@orubel I don't understand what you are saying and not sure how the link generator relates to the original problem reported here, which I cannot reproduce. I am going to close this issue pending clarity. I encourage you to post a question to the mailing list describing what it is that you want to accomplish and let that discussion take place there.

Thanks so much for the feedback and I hope we can get you whatever it is that you are looking for. Please pursue that discussion on the mailing list. Thank you very much for your cooperation.

orubel commented 8 years ago

@jeffbrown thanks a million. Just took a look at your code and now I'm more confused than ever. Shouldn't this work then?

forward(uri: params.uri, params: params)