Description

Hi,

I'm building a CMS system upon the Zend Framework. But I'm having troubles with the Request object, which seems not to "clear" the user params. Each page exists of several page elements; each element is a new request that is added to the actionstack. If the request's user params are printed in the IndexController everything is fine, but when I print the same request in the controller for that specific action than I see also user params of the previous actions.

I would like to submit a patch for this issue; not sure the best place to discuss it, please let me know if these comments are not the appropriate place.

I think that BC should be broken here. The ActionStack::forward method should use the exact request object passed to it when it was created, not some frankenstein object of all the user params set in previous actionstack actions. If an actionstack action2 relies on parameters set in a previous actionstack action1, then action1 should add action2 to the actionstack.

What are your thoughts on breaking BC here? I have a patch for this ready if it's acceptable. I don't believe that this optional reset parameter suggested by Dennis is viable, as _forward is not a user-callable method.

Posted by Dennis Becker (radhad) on 2009-07-27T06:44:50.000+0000

A long time has passed since I add a comment to this issue.

After re-reading everything, it seems like I have not seen the "clone" statement in the description. I'm not sure if there exists a reset() method for the request-object, but I still would prefer another way.

Posted by Mark (lightflowmark) on 2009-07-27T06:54:54.000+0000

There isn't a reset() method for the request object, but this isn't the main problem. The problem is that ActionStack::forward does not call any putative reset() method; it adds parameters to the request object using setParams() (which doesn't actually set the parameters - it adds them to existing parameters). Changing this breaks BC, but as far as I can see, it's the only way to prevent unwanted parameters being set in later ActionStack actions.

What is your use-case for building up parameters through successive ActionStack actions?

Posted by Dennis Becker (radhad) on 2009-07-27T07:10:32.000+0000

It is possible, that you set a value per setParam() which will be used by more than one Action in the later use.

My main missunderstanding is the context, where you want to change that. I thought about the Action-Controller available _forward() method and as I can see, I was wrong here. The main reason why I'm still against a reset of the values is, that no one would expect that by cloning an object! For me a clone must be an exact copy, not a half copy. You never know who uses this behaviour at the moment. So I think a reset() or resetParams() method seems to be a better way instead of implementing a functionality no one would expect when using it.

Posted by Stijn Huyberechts (huyby) on 2009-07-27T07:11:01.000+0000

I'm glad to see this issue is still alive. I'm still 'patching' the ZF with my 'solution' in the original post of this issue. As I see it now, it is clear that the request object needs to have 2 separate methods to modify or add parameters:

Dennis: absolutely. No-one's suggesting that the clone does anything other than clone the request object; only that, given 2 actions in the actionstack, each with their own request object, action1 should not magically change the parameters for action2's request.

Stijn: I agree this is by far the best solution. However, this seems like a big BC break; any input from the ZF team on this?

This is a big BC break, as the parameter munging is a specific use case for which ActionStack was designed.

I think we could introduce a flag to ActionStack to indicate that the request object user parameters should be cleared, but this is really the only concession I'd like to make at this time.

Posted by Mark (lightflowmark) on 2009-07-27T09:33:31.000+0000

Fair enough.

Out of interest, what is the use case for using ActionStack to build up parameters which isn't better served by the code I outline above, where test1Action adds test2Action to the ActionStack?

Posted by Mark (lightflowmark) on 2009-07-28T02:05:35.000+0000

Patch & test done.

I've added a clearParams() method to the request class, and a setClearRequestParams() method to the ActionStack class, which controls this behaviour. It defaults to false, so bc is not broken - I think this is confusing, and that bc should be broken here.

I'd still really like to see a robust defence of the current behaviour, this solution seems sub-optimal to me.

I'd also be interested to know why the request setParams() actually adds parameters rather than setting the parameters, and why it's not implemented as Stijn suggested, which is the root cause of this confusion.

One reason that the user params are not cleared on _forward/ActionStack calls is due to how other components, such as ContextSwitch, do their work. Imagine a scenario where ContextSwitch is in play, and a JSON context is detected because of the "format" parameter found in the user params (i.e., set by the router); if you were to clear out the params, then additional actions invoked would no longer know the current context, and would be appending HTML to what should be a JSON response.

I am reviewing the patch for inclusion; as long as the behavior is controlled by a flag and that the default state of the flag is the current behavior, we should be able to include it.