You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beehive.apache.org by Carlin Rogers <ca...@gmail.com> on 2006/01/06 07:22:12 UTC
Question about BEEHIVE-1024
Hey Rich,
Hope your work is going well!
I have a question about svn revision 356056 (
http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev ) checked in as a fix
for BEEHIVE-1024. It seems that it changed the behavior of
PageFlowRequestProcessor.processMapping() and how we handle an unknown
action. With this change, the code path for an unknown action in
processMapping() fails the new check to see if it is in the globalApp
(...globalApp.findActionConfig(path) != null). We drop to the else statement
and into a call to processUnresolvedAction() which uses the
DefaultExceptionsHandler class and eventually writes out the HTML of our
action not found error message directly to the response. I think this looks
OK. However, having the error message written to the response may not be the
desired behavior for something like a portal using a call to
PageFlowUtils.strutsLookup(). What do you think?
If we leave the fix as is, could we use the
PageFlowRequestWrapper.isScopedLookup() condition to determine if this is
from strutsLookup() or not before calling processUnresolvedAction(). I.E. do
something different for an unknown action in a strutsLookup()? Just curious.
Thanks,
Carlin
Re: Question about BEEHIVE-1024
Posted by Rich Feit <ri...@gmail.com>.
Sure, Carlin -- thanks!
Rich
Carlin Rogers wrote:
>Yes, I agree. I think I have a fix for all this
>(*BEEHIVE-1037<https://issues.apache.org/jira/browse/BEEHIVE-1037>)
>*and will check it into the beehive svn repository. I'll add a comment to
>the bug with the svn revision number so you give it a review if you want.
>Thanks again for your help.
>
>On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
>
>
>>I was thinking the latter -- replacing getModuleConfig() with
>>ensureModuleConfig() in processMapping(). What do you think?
>>
>>Carlin Rogers wrote:
>>
>>
>>
>>>Do you mean, add a call to InternalUtils.ensureModuleConfig() from the
>>>FlowController.reinitialize() method during the create() process of the
>>>FlowController where the old initModuleConfig() call used to be? Or, were
>>>you thinking that it went in the processMapping to replace the call to
>>>getModuleConfig().
>>>
>>>On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
>>>
>>>
>>>
>>>
>>>>I see -- thanks for clarifying. I *think* that we should just change to
>>>>using InternalUtils.ensureModuleConfig() at that point. It's meant to
>>>>be used in any code path where a desired module might still be
>>>>unregistered (and as so, getModuleConfig() is actually very rarely
>>>>used). I should have switched to ensureModuleConfig(), not
>>>>getModuleConfig(), in that checkin. Does that sound alright to you?
>>>>
>>>>Rich
>>>>
>>>>Carlin Rogers wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>I haven't checked the setting or value of _moduleConfig. I've just been
>>>>>comparing the old code path to the new changes while running this
>>>>>
>>>>>
>>>>>
>>>>>
>>>>convoluted
>>>>
>>>>
>>>>
>>>>
>>>>>scenario. I noticed that in the old code, that if _moduleConfig was
>>>>>
>>>>>
>>null
>>
>>
>>>>>
>>>>>
>>>>in
>>>>
>>>>
>>>>
>>>>
>>>>>initModuleConfig(), it would see if it was already an attribute of the
>>>>>context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered
>>>>>
>>>>>
>>()
>>
>>
>>>>>was called.
>>>>>
>>>>>In turn, it called...
>>>>>AutoRegisterActionServlet.registerModule(), which called...
>>>>>AutoRegisterActionServlet.initModuleConfig().
>>>>>
>>>>>This initModuleConfig() does a servletContext.setAttribute() with the
>>>>>
>>>>>
>>new
>>
>>
>>>>>module config, as does the
>>>>>
>>>>>
>>>>>
>>>>>
>>>>AutoRegisterActionServlet.ensureModuleRegistered()
>>>>
>>>>
>>>>
>>>>
>>>>>after registerModule() returns. The ModuleConfig object is returned
>>>>>
>>>>>
>>from
>>
>>
>>>>>ensureModuleRegistered() and assigned to _moduleConfig.
>>>>>
>>>>>I've just noticed that in this scenario, when I'm in processMapping()
>>>>>
>>>>>
>>>>>
>>>>>
>>>>that a
>>>>
>>>>
>>>>
>>>>
>>>>>call to InternalUtils.getModuleConfig() for the GlobalApp module config
>>>>>
>>>>>
>>>>>
>>>>>
>>>>will
>>>>
>>>>
>>>>
>>>>
>>>>>now return null.
>>>>>
>>>>>Does that make sense? Sorry if this isn't so clear.
>>>>>
>>>>>Thanks,
>>>>>Carlin
>>>>>
>>>>>On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>Hey Carlin,
>>>>>>
>>>>>>Just want to make sure I'm understanding here. Are you saying that
>>>>>>
>>>>>>
>>the
>>
>>
>>>>>>original call to initModuleConfig() (the one I removed) also
>>>>>>
>>>>>>
>>registered
>>
>>
>>>>>>the module in the ServletContext, as a side effect? It was only
>>>>>>supposed to set the reference to _moduleConfig, which is now always
>>>>>>taken care of in getModuleConfig(). Is the problem happening because
>>>>>>the module isn't registered in the ServletContext, or because
>>>>>>_moduleConfig is somehow null?
>>>>>>
>>>>>>Rich
>>>>>>
>>>>>>Carlin Rogers wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>Hey Rich,
>>>>>>>
>>>>>>>Thanks for the reply and the information on the
>>>>>>>InternalUtils.avoidDirectResponseOutput() function.
>>>>>>>
>>>>>>>I looked at the issue some more and it turns out that for the
>>>>>>>
>>>>>>>
>>scenario
>>
>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>I'm
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>investigating, there's another revision that is also impacting the
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>behavior.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>In revision 351812,
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
>>>>
>>>>
>>>>
>>>>
>>>>>>>for BEEHIVE-1017, the FlowController.reinitialize() method was
>>>>>>>
>>>>>>>
>>modified
>>
>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>so
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>that in no longer calls initModuleConfig(), which ensured that the
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>module
>>>>
>>>>
>>>>
>>>>
>>>>>>>config was registered (attribute of the context).
>>>>>>>
>>>>>>>Now, when the initial page flow of the portal scenario is opened in a
>>>>>>>portlet, the GlobalApp module config is not added to the servlet
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>context
>>>>
>>>>
>>>>
>>>>
>>>>>>>attributes. Then when the unhandled action is hit and we fall into
>>>>>>>processMapping(), the call to InternalUtils.getModuleConfig() for the
>>>>>>>GlobalApp module config will be null and we wouldn't even be able to
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>check
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>for an action that was declared as "unknown".
>>>>>>>
>>>>>>>Unfortunately, I'm still trying to understand why the call to from
>>>>>>>FlowController.reinitialize() to initModuleConfig() was removed.
>>>>>>>
>>>>>>>I've logged a JIRA issue on this and am trying to figure out what is
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>the
>>>>
>>>>
>>>>
>>>>
>>>>>>>best way to resolve the problem. See
>>>>>>>http://issues.apache.org/jira/browse/BEEHIVE-1037
>>>>>>>The bug description might be a better illustration of the scenario
>>>>>>>
>>>>>>>
>>I'm
>>
>>
>>>>>>>trying to solve.
>>>>>>>
>>>>>>>Let me know if you have some more thoughts about how best to resolve
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>this.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>Many thanks,
>>>>>>>Carlin
>>>>>>>
>>>>>>>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>Hey Carlin,
>>>>>>>>
>>>>>>>>Sorry for the delay. I agree that we should be checking for an
>>>>>>>>"unknown" action mapping in the global app module, so if you're
>>>>>>>>suggesting making that change, I agree.
>>>>>>>>
>>>>>>>>My answer to the rest is a little more involved:
>>>>>>>>
>>>>>>>> - There's already a rough mechanism for avoiding direct response
>>>>>>>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
>>>>>>>>that's called on the request, then InternalUtils.sendError() will
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>throw
>>>>
>>>>
>>>>
>>>>
>>>>>>>>an exception instead of writing to the response. This is what I
>>>>>>>>
>>>>>>>>
>>think
>>
>>
>>>>>>>>should be happening here -- we should be calling sendError().
>>>>>>>>
>>>>>>>> - I think that two things should probably change here: 1)
>>>>>>>>InternalUtils.avoidDirectResponseOutput() should be replaced with a
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>flag
>>>>
>>>>
>>>>
>>>>
>>>>>>>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set
>>>>>>>>
>>>>>>>>
>>this
>>
>>
>>>>>>>>flag off the bat. We shouldn't be writing to the response *ever*
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>during
>>>>
>>>>
>>>>
>>>>
>>>>>>>>strutsLookup().
>>>>>>>>
>>>>>>>>Let me know what you think (and if you have any questions about
>>>>>>>>
>>>>>>>>
>>this).
>>
>>
>>>>>>>>Rich
>>>>>>>>
>>>>>>>>Carlin Rogers wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>Just wanted to note that the difference in the behavior is also
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>related
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>to a
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>struts merge where the struts module config has an action defined
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>with
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>the
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>"unknown" attribute (making it like a default). I think the missing
>>>>>>>>>condition is that we check to see if the GlobalApp has the action
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>config
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>but
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>we don't check any of the action configs on the global app to see
>>>>>>>>>
>>>>>>>>>
>>if
>>
>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>they're
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>"unknown".
>>>>>>>>>
>>>>>>>>>So, If the global app includes a Struts Merge and that struts
>>>>>>>>>
>>>>>>>>>
>>module
>>
>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>config
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>includes an unknown action, we'll never hit it.
>>>>>>>>>
>>>>>>>>>Carlin
>>>>>>>>>
>>>>>>>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>>Hey Rich,
>>>>>>>>>>
>>>>>>>>>>Hope your work is going well!
>>>>>>>>>>
>>>>>>>>>>I have a question about svn revision 356056 (
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed
>>>>>>>>>>
>>>>>>>>>>
>>the
>>
>>
>>>>>>>>>>behavior of PageFlowRequestProcessor.processMapping() and how we
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>handle
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>an
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>unknown action. With this change, the code path for an unknown
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>action
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>in
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>>processMapping() fails the new check to see if it is in the
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>globalApp
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>statement
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>and into a call to processUnresolvedAction() which uses the
>>>>>>>>>>DefaultExceptionsHandler class and eventually writes out the HTML
>>>>>>>>>>
>>>>>>>>>>
>>of
>>
>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>our
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>>action not found error message directly to the response. I think
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>this
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>looks
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>OK. However, having the error message written to the response may
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>not
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>be
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>the
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>desired behavior for something like a portal using a call to
>>>>>>>>>>PageFlowUtils.strutsLookup(). What do you think?
>>>>>>>>>>
>>>>>>>>>>If we leave the fix as is, could we use the
>>>>>>>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>this
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>is
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>from strutsLookup() or not before calling
>>>>>>>>>
>>>>>>>>>
>>processUnresolvedAction().
>>
>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>I.E
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>>do something different for an unknown action in a strutsLookup()?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>Just
>>>>
>>>>
>>>>
>>>>
>>>>>>>>>>curious.
>>>>>>>>>>
>>>>>>>>>>Thanks,
>>>>>>>>>>Carlin
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
>
>
>
Re: Question about BEEHIVE-1024
Posted by Carlin Rogers <ca...@gmail.com>.
Yes, I agree. I think I have a fix for all this
(*BEEHIVE-1037<https://issues.apache.org/jira/browse/BEEHIVE-1037>)
*and will check it into the beehive svn repository. I'll add a comment to
the bug with the svn revision number so you give it a review if you want.
Thanks again for your help.
On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
>
> I was thinking the latter -- replacing getModuleConfig() with
> ensureModuleConfig() in processMapping(). What do you think?
>
> Carlin Rogers wrote:
>
> >Do you mean, add a call to InternalUtils.ensureModuleConfig() from the
> >FlowController.reinitialize() method during the create() process of the
> >FlowController where the old initModuleConfig() call used to be? Or, were
> >you thinking that it went in the processMapping to replace the call to
> >getModuleConfig().
> >
> >On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
> >
> >
> >>I see -- thanks for clarifying. I *think* that we should just change to
> >>using InternalUtils.ensureModuleConfig() at that point. It's meant to
> >>be used in any code path where a desired module might still be
> >>unregistered (and as so, getModuleConfig() is actually very rarely
> >>used). I should have switched to ensureModuleConfig(), not
> >>getModuleConfig(), in that checkin. Does that sound alright to you?
> >>
> >>Rich
> >>
> >>Carlin Rogers wrote:
> >>
> >>
> >>
> >>>I haven't checked the setting or value of _moduleConfig. I've just been
> >>>comparing the old code path to the new changes while running this
> >>>
> >>>
> >>convoluted
> >>
> >>
> >>>scenario. I noticed that in the old code, that if _moduleConfig was
> null
> >>>
> >>>
> >>in
> >>
> >>
> >>>initModuleConfig(), it would see if it was already an attribute of the
> >>>context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered
> ()
> >>>was called.
> >>>
> >>>In turn, it called...
> >>>AutoRegisterActionServlet.registerModule(), which called...
> >>>AutoRegisterActionServlet.initModuleConfig().
> >>>
> >>>This initModuleConfig() does a servletContext.setAttribute() with the
> new
> >>>module config, as does the
> >>>
> >>>
> >>AutoRegisterActionServlet.ensureModuleRegistered()
> >>
> >>
> >>>after registerModule() returns. The ModuleConfig object is returned
> from
> >>>ensureModuleRegistered() and assigned to _moduleConfig.
> >>>
> >>>I've just noticed that in this scenario, when I'm in processMapping()
> >>>
> >>>
> >>that a
> >>
> >>
> >>>call to InternalUtils.getModuleConfig() for the GlobalApp module config
> >>>
> >>>
> >>will
> >>
> >>
> >>>now return null.
> >>>
> >>>Does that make sense? Sorry if this isn't so clear.
> >>>
> >>>Thanks,
> >>>Carlin
> >>>
> >>>On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
> >>>
> >>>
> >>>
> >>>
> >>>>Hey Carlin,
> >>>>
> >>>>Just want to make sure I'm understanding here. Are you saying that
> the
> >>>>original call to initModuleConfig() (the one I removed) also
> registered
> >>>>the module in the ServletContext, as a side effect? It was only
> >>>>supposed to set the reference to _moduleConfig, which is now always
> >>>>taken care of in getModuleConfig(). Is the problem happening because
> >>>>the module isn't registered in the ServletContext, or because
> >>>>_moduleConfig is somehow null?
> >>>>
> >>>>Rich
> >>>>
> >>>>Carlin Rogers wrote:
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>Hey Rich,
> >>>>>
> >>>>>Thanks for the reply and the information on the
> >>>>>InternalUtils.avoidDirectResponseOutput() function.
> >>>>>
> >>>>>I looked at the issue some more and it turns out that for the
> scenario
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>I'm
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>investigating, there's another revision that is also impacting the
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>behavior.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>In revision 351812,
> >>>>>
> >>>>>
> >>http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
> >>
> >>
> >>>>>for BEEHIVE-1017, the FlowController.reinitialize() method was
> modified
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>so
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>that in no longer calls initModuleConfig(), which ensured that the
> >>>>>
> >>>>>
> >>module
> >>
> >>
> >>>>>config was registered (attribute of the context).
> >>>>>
> >>>>>Now, when the initial page flow of the portal scenario is opened in a
> >>>>>portlet, the GlobalApp module config is not added to the servlet
> >>>>>
> >>>>>
> >>context
> >>
> >>
> >>>>>attributes. Then when the unhandled action is hit and we fall into
> >>>>>processMapping(), the call to InternalUtils.getModuleConfig() for the
> >>>>>GlobalApp module config will be null and we wouldn't even be able to
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>check
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>for an action that was declared as "unknown".
> >>>>>
> >>>>>Unfortunately, I'm still trying to understand why the call to from
> >>>>>FlowController.reinitialize() to initModuleConfig() was removed.
> >>>>>
> >>>>>I've logged a JIRA issue on this and am trying to figure out what is
> >>>>>
> >>>>>
> >>the
> >>
> >>
> >>>>>best way to resolve the problem. See
> >>>>>http://issues.apache.org/jira/browse/BEEHIVE-1037
> >>>>>The bug description might be a better illustration of the scenario
> I'm
> >>>>>trying to solve.
> >>>>>
> >>>>>Let me know if you have some more thoughts about how best to resolve
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>this.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>Many thanks,
> >>>>>Carlin
> >>>>>
> >>>>>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>Hey Carlin,
> >>>>>>
> >>>>>>Sorry for the delay. I agree that we should be checking for an
> >>>>>>"unknown" action mapping in the global app module, so if you're
> >>>>>>suggesting making that change, I agree.
> >>>>>>
> >>>>>>My answer to the rest is a little more involved:
> >>>>>>
> >>>>>> - There's already a rough mechanism for avoiding direct response
> >>>>>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
> >>>>>>that's called on the request, then InternalUtils.sendError() will
> >>>>>>
> >>>>>>
> >>throw
> >>
> >>
> >>>>>>an exception instead of writing to the response. This is what I
> think
> >>>>>>should be happening here -- we should be calling sendError().
> >>>>>>
> >>>>>> - I think that two things should probably change here: 1)
> >>>>>>InternalUtils.avoidDirectResponseOutput() should be replaced with a
> >>>>>>
> >>>>>>
> >>flag
> >>
> >>
> >>>>>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set
> this
> >>>>>>flag off the bat. We shouldn't be writing to the response *ever*
> >>>>>>
> >>>>>>
> >>during
> >>
> >>
> >>>>>>strutsLookup().
> >>>>>>
> >>>>>>Let me know what you think (and if you have any questions about
> this).
> >>>>>>
> >>>>>>Rich
> >>>>>>
> >>>>>>Carlin Rogers wrote:
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>Just wanted to note that the difference in the behavior is also
> >>>>>>>
> >>>>>>>
> >>related
> >>
> >>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>to a
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>struts merge where the struts module config has an action defined
> >>>>>>>
> >>>>>>>
> >>with
> >>
> >>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>the
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>"unknown" attribute (making it like a default). I think the missing
> >>>>>>>condition is that we check to see if the GlobalApp has the action
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>config
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>
> >>>>>>>
> >>>>>>but
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>we don't check any of the action configs on the global app to see
> if
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>they're
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>"unknown".
> >>>>>>>
> >>>>>>>So, If the global app includes a Struts Merge and that struts
> module
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>config
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>includes an unknown action, we'll never hit it.
> >>>>>>>
> >>>>>>>Carlin
> >>>>>>>
> >>>>>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>>Hey Rich,
> >>>>>>>>
> >>>>>>>>Hope your work is going well!
> >>>>>>>>
> >>>>>>>>I have a question about svn revision 356056 (
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed
> the
> >>>>>>>>behavior of PageFlowRequestProcessor.processMapping() and how we
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>handle
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>an
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>unknown action. With this change, the code path for an unknown
> >>>>>>>>
> >>>>>>>>
> >>action
> >>
> >>
> >>>>>>>>
> >>>>>>>>
> >>>>in
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>>processMapping() fails the new check to see if it is in the
> >>>>>>>>
> >>>>>>>>
> >>globalApp
> >>
> >>
> >>>>>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>statement
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>and into a call to processUnresolvedAction() which uses the
> >>>>>>>>DefaultExceptionsHandler class and eventually writes out the HTML
> of
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>our
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>>action not found error message directly to the response. I think
> >>>>>>>>
> >>>>>>>>
> >>this
> >>
> >>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>looks
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>OK. However, having the error message written to the response may
> >>>>>>>>
> >>>>>>>>
> >>not
> >>
> >>
> >>>>>>>>
> >>>>>>>>
> >>>>be
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>the
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>desired behavior for something like a portal using a call to
> >>>>>>>>PageFlowUtils.strutsLookup(). What do you think?
> >>>>>>>>
> >>>>>>>>If we leave the fix as is, could we use the
> >>>>>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if
> >>>>>>>>
> >>>>>>>>
> >>this
> >>
> >>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>is
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>from strutsLookup() or not before calling
> processUnresolvedAction().
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>I.E
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>>
> >>>>>>>
> >>>>>>.
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>do something different for an unknown action in a strutsLookup()?
> >>>>>>>>
> >>>>>>>>
> >>Just
> >>
> >>
> >>>>>>>>curious.
> >>>>>>>>
> >>>>>>>>Thanks,
> >>>>>>>>Carlin
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
>
Re: Question about BEEHIVE-1024
Posted by Rich Feit <ri...@gmail.com>.
I was thinking the latter -- replacing getModuleConfig() with
ensureModuleConfig() in processMapping(). What do you think?
Carlin Rogers wrote:
>Do you mean, add a call to InternalUtils.ensureModuleConfig() from the
>FlowController.reinitialize() method during the create() process of the
>FlowController where the old initModuleConfig() call used to be? Or, were
>you thinking that it went in the processMapping to replace the call to
>getModuleConfig().
>
>On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
>
>
>>I see -- thanks for clarifying. I *think* that we should just change to
>>using InternalUtils.ensureModuleConfig() at that point. It's meant to
>>be used in any code path where a desired module might still be
>>unregistered (and as so, getModuleConfig() is actually very rarely
>>used). I should have switched to ensureModuleConfig(), not
>>getModuleConfig(), in that checkin. Does that sound alright to you?
>>
>>Rich
>>
>>Carlin Rogers wrote:
>>
>>
>>
>>>I haven't checked the setting or value of _moduleConfig. I've just been
>>>comparing the old code path to the new changes while running this
>>>
>>>
>>convoluted
>>
>>
>>>scenario. I noticed that in the old code, that if _moduleConfig was null
>>>
>>>
>>in
>>
>>
>>>initModuleConfig(), it would see if it was already an attribute of the
>>>context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered()
>>>was called.
>>>
>>>In turn, it called...
>>>AutoRegisterActionServlet.registerModule(), which called...
>>>AutoRegisterActionServlet.initModuleConfig().
>>>
>>>This initModuleConfig() does a servletContext.setAttribute() with the new
>>>module config, as does the
>>>
>>>
>>AutoRegisterActionServlet.ensureModuleRegistered()
>>
>>
>>>after registerModule() returns. The ModuleConfig object is returned from
>>>ensureModuleRegistered() and assigned to _moduleConfig.
>>>
>>>I've just noticed that in this scenario, when I'm in processMapping()
>>>
>>>
>>that a
>>
>>
>>>call to InternalUtils.getModuleConfig() for the GlobalApp module config
>>>
>>>
>>will
>>
>>
>>>now return null.
>>>
>>>Does that make sense? Sorry if this isn't so clear.
>>>
>>>Thanks,
>>>Carlin
>>>
>>>On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
>>>
>>>
>>>
>>>
>>>>Hey Carlin,
>>>>
>>>>Just want to make sure I'm understanding here. Are you saying that the
>>>>original call to initModuleConfig() (the one I removed) also registered
>>>>the module in the ServletContext, as a side effect? It was only
>>>>supposed to set the reference to _moduleConfig, which is now always
>>>>taken care of in getModuleConfig(). Is the problem happening because
>>>>the module isn't registered in the ServletContext, or because
>>>>_moduleConfig is somehow null?
>>>>
>>>>Rich
>>>>
>>>>Carlin Rogers wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>Hey Rich,
>>>>>
>>>>>Thanks for the reply and the information on the
>>>>>InternalUtils.avoidDirectResponseOutput() function.
>>>>>
>>>>>I looked at the issue some more and it turns out that for the scenario
>>>>>
>>>>>
>>>>>
>>>>>
>>>>I'm
>>>>
>>>>
>>>>
>>>>
>>>>>investigating, there's another revision that is also impacting the
>>>>>
>>>>>
>>>>>
>>>>>
>>>>behavior.
>>>>
>>>>
>>>>
>>>>
>>>>>In revision 351812,
>>>>>
>>>>>
>>http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
>>
>>
>>>>>for BEEHIVE-1017, the FlowController.reinitialize() method was modified
>>>>>
>>>>>
>>>>>
>>>>>
>>>>so
>>>>
>>>>
>>>>
>>>>
>>>>>that in no longer calls initModuleConfig(), which ensured that the
>>>>>
>>>>>
>>module
>>
>>
>>>>>config was registered (attribute of the context).
>>>>>
>>>>>Now, when the initial page flow of the portal scenario is opened in a
>>>>>portlet, the GlobalApp module config is not added to the servlet
>>>>>
>>>>>
>>context
>>
>>
>>>>>attributes. Then when the unhandled action is hit and we fall into
>>>>>processMapping(), the call to InternalUtils.getModuleConfig() for the
>>>>>GlobalApp module config will be null and we wouldn't even be able to
>>>>>
>>>>>
>>>>>
>>>>>
>>>>check
>>>>
>>>>
>>>>
>>>>
>>>>>for an action that was declared as "unknown".
>>>>>
>>>>>Unfortunately, I'm still trying to understand why the call to from
>>>>>FlowController.reinitialize() to initModuleConfig() was removed.
>>>>>
>>>>>I've logged a JIRA issue on this and am trying to figure out what is
>>>>>
>>>>>
>>the
>>
>>
>>>>>best way to resolve the problem. See
>>>>>http://issues.apache.org/jira/browse/BEEHIVE-1037
>>>>>The bug description might be a better illustration of the scenario I'm
>>>>>trying to solve.
>>>>>
>>>>>Let me know if you have some more thoughts about how best to resolve
>>>>>
>>>>>
>>>>>
>>>>>
>>>>this.
>>>>
>>>>
>>>>
>>>>
>>>>>Many thanks,
>>>>>Carlin
>>>>>
>>>>>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>Hey Carlin,
>>>>>>
>>>>>>Sorry for the delay. I agree that we should be checking for an
>>>>>>"unknown" action mapping in the global app module, so if you're
>>>>>>suggesting making that change, I agree.
>>>>>>
>>>>>>My answer to the rest is a little more involved:
>>>>>>
>>>>>> - There's already a rough mechanism for avoiding direct response
>>>>>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
>>>>>>that's called on the request, then InternalUtils.sendError() will
>>>>>>
>>>>>>
>>throw
>>
>>
>>>>>>an exception instead of writing to the response. This is what I think
>>>>>>should be happening here -- we should be calling sendError().
>>>>>>
>>>>>> - I think that two things should probably change here: 1)
>>>>>>InternalUtils.avoidDirectResponseOutput() should be replaced with a
>>>>>>
>>>>>>
>>flag
>>
>>
>>>>>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
>>>>>>flag off the bat. We shouldn't be writing to the response *ever*
>>>>>>
>>>>>>
>>during
>>
>>
>>>>>>strutsLookup().
>>>>>>
>>>>>>Let me know what you think (and if you have any questions about this).
>>>>>>
>>>>>>Rich
>>>>>>
>>>>>>Carlin Rogers wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>Just wanted to note that the difference in the behavior is also
>>>>>>>
>>>>>>>
>>related
>>
>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>to a
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>struts merge where the struts module config has an action defined
>>>>>>>
>>>>>>>
>>with
>>
>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>the
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>"unknown" attribute (making it like a default). I think the missing
>>>>>>>condition is that we check to see if the GlobalApp has the action
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>config
>>>>
>>>>
>>>>
>>>>
>>>>>>>
>>>>>>>
>>>>>>but
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>we don't check any of the action configs on the global app to see if
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>they're
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>"unknown".
>>>>>>>
>>>>>>>So, If the global app includes a Struts Merge and that struts module
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>config
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>includes an unknown action, we'll never hit it.
>>>>>>>
>>>>>>>Carlin
>>>>>>>
>>>>>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>Hey Rich,
>>>>>>>>
>>>>>>>>Hope your work is going well!
>>>>>>>>
>>>>>>>>I have a question about svn revision 356056 (
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
>>>>>>>>behavior of PageFlowRequestProcessor.processMapping() and how we
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>handle
>>>>
>>>>
>>>>
>>>>
>>>>>>>>
>>>>>>>>
>>>>>>an
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>unknown action. With this change, the code path for an unknown
>>>>>>>>
>>>>>>>>
>>action
>>
>>
>>>>>>>>
>>>>>>>>
>>>>in
>>>>
>>>>
>>>>
>>>>
>>>>>>>>processMapping() fails the new check to see if it is in the
>>>>>>>>
>>>>>>>>
>>globalApp
>>
>>
>>>>>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>statement
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>and into a call to processUnresolvedAction() which uses the
>>>>>>>>DefaultExceptionsHandler class and eventually writes out the HTML of
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>our
>>>>
>>>>
>>>>
>>>>
>>>>>>>>action not found error message directly to the response. I think
>>>>>>>>
>>>>>>>>
>>this
>>
>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>looks
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>OK. However, having the error message written to the response may
>>>>>>>>
>>>>>>>>
>>not
>>
>>
>>>>>>>>
>>>>>>>>
>>>>be
>>>>
>>>>
>>>>
>>>>
>>>>>>>>
>>>>>>>>
>>>>>>the
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>desired behavior for something like a portal using a call to
>>>>>>>>PageFlowUtils.strutsLookup(). What do you think?
>>>>>>>>
>>>>>>>>If we leave the fix as is, could we use the
>>>>>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if
>>>>>>>>
>>>>>>>>
>>this
>>
>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>is
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>from strutsLookup() or not before calling processUnresolvedAction().
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>I.E
>>>>
>>>>
>>>>
>>>>
>>>>>>>
>>>>>>>
>>>>>>.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>>>do something different for an unknown action in a strutsLookup()?
>>>>>>>>
>>>>>>>>
>>Just
>>
>>
>>>>>>>>curious.
>>>>>>>>
>>>>>>>>Thanks,
>>>>>>>>Carlin
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
>
>
>
Re: Question about BEEHIVE-1024
Posted by Carlin Rogers <ca...@gmail.com>.
Do you mean, add a call to InternalUtils.ensureModuleConfig() from the
FlowController.reinitialize() method during the create() process of the
FlowController where the old initModuleConfig() call used to be? Or, were
you thinking that it went in the processMapping to replace the call to
getModuleConfig().
On 1/10/06, Rich Feit <ri...@gmail.com> wrote:
>
> I see -- thanks for clarifying. I *think* that we should just change to
> using InternalUtils.ensureModuleConfig() at that point. It's meant to
> be used in any code path where a desired module might still be
> unregistered (and as so, getModuleConfig() is actually very rarely
> used). I should have switched to ensureModuleConfig(), not
> getModuleConfig(), in that checkin. Does that sound alright to you?
>
> Rich
>
> Carlin Rogers wrote:
>
> >I haven't checked the setting or value of _moduleConfig. I've just been
> >comparing the old code path to the new changes while running this
> convoluted
> >scenario. I noticed that in the old code, that if _moduleConfig was null
> in
> >initModuleConfig(), it would see if it was already an attribute of the
> >context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered()
> >was called.
> >
> >In turn, it called...
> >AutoRegisterActionServlet.registerModule(), which called...
> >AutoRegisterActionServlet.initModuleConfig().
> >
> >This initModuleConfig() does a servletContext.setAttribute() with the new
> >module config, as does the
> AutoRegisterActionServlet.ensureModuleRegistered()
> >after registerModule() returns. The ModuleConfig object is returned from
> >ensureModuleRegistered() and assigned to _moduleConfig.
> >
> >I've just noticed that in this scenario, when I'm in processMapping()
> that a
> >call to InternalUtils.getModuleConfig() for the GlobalApp module config
> will
> >now return null.
> >
> >Does that make sense? Sorry if this isn't so clear.
> >
> >Thanks,
> >Carlin
> >
> >On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
> >
> >
> >>Hey Carlin,
> >>
> >>Just want to make sure I'm understanding here. Are you saying that the
> >>original call to initModuleConfig() (the one I removed) also registered
> >>the module in the ServletContext, as a side effect? It was only
> >>supposed to set the reference to _moduleConfig, which is now always
> >>taken care of in getModuleConfig(). Is the problem happening because
> >>the module isn't registered in the ServletContext, or because
> >>_moduleConfig is somehow null?
> >>
> >>Rich
> >>
> >>Carlin Rogers wrote:
> >>
> >>
> >>
> >>>Hey Rich,
> >>>
> >>>Thanks for the reply and the information on the
> >>>InternalUtils.avoidDirectResponseOutput() function.
> >>>
> >>>I looked at the issue some more and it turns out that for the scenario
> >>>
> >>>
> >>I'm
> >>
> >>
> >>>investigating, there's another revision that is also impacting the
> >>>
> >>>
> >>behavior.
> >>
> >>
> >>>In revision 351812,
> http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
> >>>for BEEHIVE-1017, the FlowController.reinitialize() method was modified
> >>>
> >>>
> >>so
> >>
> >>
> >>>that in no longer calls initModuleConfig(), which ensured that the
> module
> >>>config was registered (attribute of the context).
> >>>
> >>>Now, when the initial page flow of the portal scenario is opened in a
> >>>portlet, the GlobalApp module config is not added to the servlet
> context
> >>>attributes. Then when the unhandled action is hit and we fall into
> >>>processMapping(), the call to InternalUtils.getModuleConfig() for the
> >>>GlobalApp module config will be null and we wouldn't even be able to
> >>>
> >>>
> >>check
> >>
> >>
> >>>for an action that was declared as "unknown".
> >>>
> >>>Unfortunately, I'm still trying to understand why the call to from
> >>>FlowController.reinitialize() to initModuleConfig() was removed.
> >>>
> >>>I've logged a JIRA issue on this and am trying to figure out what is
> the
> >>>best way to resolve the problem. See
> >>>http://issues.apache.org/jira/browse/BEEHIVE-1037
> >>>The bug description might be a better illustration of the scenario I'm
> >>>trying to solve.
> >>>
> >>>Let me know if you have some more thoughts about how best to resolve
> >>>
> >>>
> >>this.
> >>
> >>
> >>>Many thanks,
> >>>Carlin
> >>>
> >>>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
> >>>
> >>>
> >>>
> >>>
> >>>>Hey Carlin,
> >>>>
> >>>>Sorry for the delay. I agree that we should be checking for an
> >>>>"unknown" action mapping in the global app module, so if you're
> >>>>suggesting making that change, I agree.
> >>>>
> >>>>My answer to the rest is a little more involved:
> >>>>
> >>>> - There's already a rough mechanism for avoiding direct response
> >>>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
> >>>>that's called on the request, then InternalUtils.sendError() will
> throw
> >>>>an exception instead of writing to the response. This is what I think
> >>>>should be happening here -- we should be calling sendError().
> >>>>
> >>>> - I think that two things should probably change here: 1)
> >>>>InternalUtils.avoidDirectResponseOutput() should be replaced with a
> flag
> >>>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
> >>>>flag off the bat. We shouldn't be writing to the response *ever*
> during
> >>>>strutsLookup().
> >>>>
> >>>>Let me know what you think (and if you have any questions about this).
> >>>>
> >>>>Rich
> >>>>
> >>>>Carlin Rogers wrote:
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>Just wanted to note that the difference in the behavior is also
> related
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>to a
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>struts merge where the struts module config has an action defined
> with
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>the
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>"unknown" attribute (making it like a default). I think the missing
> >>>>>condition is that we check to see if the GlobalApp has the action
> >>>>>
> >>>>>
> >>config
> >>
> >>
> >>>>>
> >>>>>
> >>>>but
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>we don't check any of the action configs on the global app to see if
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>they're
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>"unknown".
> >>>>>
> >>>>>So, If the global app includes a Struts Merge and that struts module
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>config
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>includes an unknown action, we'll never hit it.
> >>>>>
> >>>>>Carlin
> >>>>>
> >>>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>>Hey Rich,
> >>>>>>
> >>>>>>Hope your work is going well!
> >>>>>>
> >>>>>>I have a question about svn revision 356056 (
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
> >>>>>>behavior of PageFlowRequestProcessor.processMapping() and how we
> >>>>>>
> >>>>>>
> >>handle
> >>
> >>
> >>>>>>
> >>>>>>
> >>>>an
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>unknown action. With this change, the code path for an unknown
> action
> >>>>>>
> >>>>>>
> >>in
> >>
> >>
> >>>>>>processMapping() fails the new check to see if it is in the
> globalApp
> >>>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>statement
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>and into a call to processUnresolvedAction() which uses the
> >>>>>>DefaultExceptionsHandler class and eventually writes out the HTML of
> >>>>>>
> >>>>>>
> >>our
> >>
> >>
> >>>>>>action not found error message directly to the response. I think
> this
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>looks
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>OK. However, having the error message written to the response may
> not
> >>>>>>
> >>>>>>
> >>be
> >>
> >>
> >>>>>>
> >>>>>>
> >>>>the
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>desired behavior for something like a portal using a call to
> >>>>>>PageFlowUtils.strutsLookup(). What do you think?
> >>>>>>
> >>>>>>If we leave the fix as is, could we use the
> >>>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if
> this
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>is
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>from strutsLookup() or not before calling processUnresolvedAction().
> >>>>>
> >>>>>
> >>I.E
> >>
> >>
> >>>>>
> >>>>>
> >>>>.
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>>>do something different for an unknown action in a strutsLookup()?
> Just
> >>>>>>curious.
> >>>>>>
> >>>>>>Thanks,
> >>>>>>Carlin
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >
> >
> >
>
Re: Question about BEEHIVE-1024
Posted by Rich Feit <ri...@gmail.com>.
I see -- thanks for clarifying. I *think* that we should just change to
using InternalUtils.ensureModuleConfig() at that point. It's meant to
be used in any code path where a desired module might still be
unregistered (and as so, getModuleConfig() is actually very rarely
used). I should have switched to ensureModuleConfig(), not
getModuleConfig(), in that checkin. Does that sound alright to you?
Rich
Carlin Rogers wrote:
>I haven't checked the setting or value of _moduleConfig. I've just been
>comparing the old code path to the new changes while running this convoluted
>scenario. I noticed that in the old code, that if _moduleConfig was null in
>initModuleConfig(), it would see if it was already an attribute of the
>context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered()
>was called.
>
>In turn, it called...
>AutoRegisterActionServlet.registerModule(), which called...
>AutoRegisterActionServlet.initModuleConfig().
>
>This initModuleConfig() does a servletContext.setAttribute() with the new
>module config, as does the AutoRegisterActionServlet.ensureModuleRegistered()
>after registerModule() returns. The ModuleConfig object is returned from
>ensureModuleRegistered() and assigned to _moduleConfig.
>
>I've just noticed that in this scenario, when I'm in processMapping() that a
>call to InternalUtils.getModuleConfig() for the GlobalApp module config will
>now return null.
>
>Does that make sense? Sorry if this isn't so clear.
>
>Thanks,
>Carlin
>
>On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
>
>
>>Hey Carlin,
>>
>>Just want to make sure I'm understanding here. Are you saying that the
>>original call to initModuleConfig() (the one I removed) also registered
>>the module in the ServletContext, as a side effect? It was only
>>supposed to set the reference to _moduleConfig, which is now always
>>taken care of in getModuleConfig(). Is the problem happening because
>>the module isn't registered in the ServletContext, or because
>>_moduleConfig is somehow null?
>>
>>Rich
>>
>>Carlin Rogers wrote:
>>
>>
>>
>>>Hey Rich,
>>>
>>>Thanks for the reply and the information on the
>>>InternalUtils.avoidDirectResponseOutput() function.
>>>
>>>I looked at the issue some more and it turns out that for the scenario
>>>
>>>
>>I'm
>>
>>
>>>investigating, there's another revision that is also impacting the
>>>
>>>
>>behavior.
>>
>>
>>>In revision 351812, http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
>>>for BEEHIVE-1017, the FlowController.reinitialize() method was modified
>>>
>>>
>>so
>>
>>
>>>that in no longer calls initModuleConfig(), which ensured that the module
>>>config was registered (attribute of the context).
>>>
>>>Now, when the initial page flow of the portal scenario is opened in a
>>>portlet, the GlobalApp module config is not added to the servlet context
>>>attributes. Then when the unhandled action is hit and we fall into
>>>processMapping(), the call to InternalUtils.getModuleConfig() for the
>>>GlobalApp module config will be null and we wouldn't even be able to
>>>
>>>
>>check
>>
>>
>>>for an action that was declared as "unknown".
>>>
>>>Unfortunately, I'm still trying to understand why the call to from
>>>FlowController.reinitialize() to initModuleConfig() was removed.
>>>
>>>I've logged a JIRA issue on this and am trying to figure out what is the
>>>best way to resolve the problem. See
>>>http://issues.apache.org/jira/browse/BEEHIVE-1037
>>>The bug description might be a better illustration of the scenario I'm
>>>trying to solve.
>>>
>>>Let me know if you have some more thoughts about how best to resolve
>>>
>>>
>>this.
>>
>>
>>>Many thanks,
>>>Carlin
>>>
>>>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
>>>
>>>
>>>
>>>
>>>>Hey Carlin,
>>>>
>>>>Sorry for the delay. I agree that we should be checking for an
>>>>"unknown" action mapping in the global app module, so if you're
>>>>suggesting making that change, I agree.
>>>>
>>>>My answer to the rest is a little more involved:
>>>>
>>>> - There's already a rough mechanism for avoiding direct response
>>>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
>>>>that's called on the request, then InternalUtils.sendError() will throw
>>>>an exception instead of writing to the response. This is what I think
>>>>should be happening here -- we should be calling sendError().
>>>>
>>>> - I think that two things should probably change here: 1)
>>>>InternalUtils.avoidDirectResponseOutput() should be replaced with a flag
>>>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
>>>>flag off the bat. We shouldn't be writing to the response *ever* during
>>>>strutsLookup().
>>>>
>>>>Let me know what you think (and if you have any questions about this).
>>>>
>>>>Rich
>>>>
>>>>Carlin Rogers wrote:
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>Just wanted to note that the difference in the behavior is also related
>>>>>
>>>>>
>>>>>
>>>>>
>>>>to a
>>>>
>>>>
>>>>
>>>>
>>>>>struts merge where the struts module config has an action defined with
>>>>>
>>>>>
>>>>>
>>>>>
>>>>the
>>>>
>>>>
>>>>
>>>>
>>>>>"unknown" attribute (making it like a default). I think the missing
>>>>>condition is that we check to see if the GlobalApp has the action
>>>>>
>>>>>
>>config
>>
>>
>>>>>
>>>>>
>>>>but
>>>>
>>>>
>>>>
>>>>
>>>>>we don't check any of the action configs on the global app to see if
>>>>>
>>>>>
>>>>>
>>>>>
>>>>they're
>>>>
>>>>
>>>>
>>>>
>>>>>"unknown".
>>>>>
>>>>>So, If the global app includes a Struts Merge and that struts module
>>>>>
>>>>>
>>>>>
>>>>>
>>>>config
>>>>
>>>>
>>>>
>>>>
>>>>>includes an unknown action, we'll never hit it.
>>>>>
>>>>>Carlin
>>>>>
>>>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>>Hey Rich,
>>>>>>
>>>>>>Hope your work is going well!
>>>>>>
>>>>>>I have a question about svn revision 356056 (
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
>>>>
>>>>
>>>>
>>>>
>>>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
>>>>>>behavior of PageFlowRequestProcessor.processMapping() and how we
>>>>>>
>>>>>>
>>handle
>>
>>
>>>>>>
>>>>>>
>>>>an
>>>>
>>>>
>>>>
>>>>
>>>>>>unknown action. With this change, the code path for an unknown action
>>>>>>
>>>>>>
>>in
>>
>>
>>>>>>processMapping() fails the new check to see if it is in the globalApp
>>>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>statement
>>>>
>>>>
>>>>
>>>>
>>>>>>and into a call to processUnresolvedAction() which uses the
>>>>>>DefaultExceptionsHandler class and eventually writes out the HTML of
>>>>>>
>>>>>>
>>our
>>
>>
>>>>>>action not found error message directly to the response. I think this
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>looks
>>>>
>>>>
>>>>
>>>>
>>>>>>OK. However, having the error message written to the response may not
>>>>>>
>>>>>>
>>be
>>
>>
>>>>>>
>>>>>>
>>>>the
>>>>
>>>>
>>>>
>>>>
>>>>>>desired behavior for something like a portal using a call to
>>>>>>PageFlowUtils.strutsLookup(). What do you think?
>>>>>>
>>>>>>If we leave the fix as is, could we use the
>>>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if this
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>is
>>>>
>>>>
>>>>
>>>>
>>>>>>from strutsLookup() or not before calling processUnresolvedAction().
>>>>>
>>>>>
>>I.E
>>
>>
>>>>>
>>>>>
>>>>.
>>>>
>>>>
>>>>
>>>>
>>>>>>do something different for an unknown action in a strutsLookup()? Just
>>>>>>curious.
>>>>>>
>>>>>>Thanks,
>>>>>>Carlin
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>>
>
>
>
Re: Question about BEEHIVE-1024
Posted by Carlin Rogers <ca...@gmail.com>.
I haven't checked the setting or value of _moduleConfig. I've just been
comparing the old code path to the new changes while running this convoluted
scenario. I noticed that in the old code, that if _moduleConfig was null in
initModuleConfig(), it would see if it was already an attribute of the
context. Then, if not, AutoRegisterActionServlet.ensureModuleRegistered()
was called.
In turn, it called...
AutoRegisterActionServlet.registerModule(), which called...
AutoRegisterActionServlet.initModuleConfig().
This initModuleConfig() does a servletContext.setAttribute() with the new
module config, as does the AutoRegisterActionServlet.ensureModuleRegistered()
after registerModule() returns. The ModuleConfig object is returned from
ensureModuleRegistered() and assigned to _moduleConfig.
I've just noticed that in this scenario, when I'm in processMapping() that a
call to InternalUtils.getModuleConfig() for the GlobalApp module config will
now return null.
Does that make sense? Sorry if this isn't so clear.
Thanks,
Carlin
On 1/9/06, Rich Feit <ri...@gmail.com> wrote:
>
> Hey Carlin,
>
> Just want to make sure I'm understanding here. Are you saying that the
> original call to initModuleConfig() (the one I removed) also registered
> the module in the ServletContext, as a side effect? It was only
> supposed to set the reference to _moduleConfig, which is now always
> taken care of in getModuleConfig(). Is the problem happening because
> the module isn't registered in the ServletContext, or because
> _moduleConfig is somehow null?
>
> Rich
>
> Carlin Rogers wrote:
>
> >Hey Rich,
> >
> >Thanks for the reply and the information on the
> >InternalUtils.avoidDirectResponseOutput() function.
> >
> >I looked at the issue some more and it turns out that for the scenario
> I'm
> >investigating, there's another revision that is also impacting the
> behavior.
> >In revision 351812, http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev,
> >for BEEHIVE-1017, the FlowController.reinitialize() method was modified
> so
> >that in no longer calls initModuleConfig(), which ensured that the module
> >config was registered (attribute of the context).
> >
> >Now, when the initial page flow of the portal scenario is opened in a
> >portlet, the GlobalApp module config is not added to the servlet context
> >attributes. Then when the unhandled action is hit and we fall into
> >processMapping(), the call to InternalUtils.getModuleConfig() for the
> >GlobalApp module config will be null and we wouldn't even be able to
> check
> >for an action that was declared as "unknown".
> >
> >Unfortunately, I'm still trying to understand why the call to from
> >FlowController.reinitialize() to initModuleConfig() was removed.
> >
> >I've logged a JIRA issue on this and am trying to figure out what is the
> >best way to resolve the problem. See
> >http://issues.apache.org/jira/browse/BEEHIVE-1037
> >The bug description might be a better illustration of the scenario I'm
> >trying to solve.
> >
> >Let me know if you have some more thoughts about how best to resolve
> this.
> >
> >Many thanks,
> >Carlin
> >
> >On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
> >
> >
> >>Hey Carlin,
> >>
> >>Sorry for the delay. I agree that we should be checking for an
> >>"unknown" action mapping in the global app module, so if you're
> >>suggesting making that change, I agree.
> >>
> >>My answer to the rest is a little more involved:
> >>
> >> - There's already a rough mechanism for avoiding direct response
> >>output. In InternalUtils, there's avoidDirectResponseOutput(). If
> >>that's called on the request, then InternalUtils.sendError() will throw
> >>an exception instead of writing to the response. This is what I think
> >>should be happening here -- we should be calling sendError().
> >>
> >> - I think that two things should probably change here: 1)
> >>InternalUtils.avoidDirectResponseOutput() should be replaced with a flag
> >>in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
> >>flag off the bat. We shouldn't be writing to the response *ever* during
> >>strutsLookup().
> >>
> >>Let me know what you think (and if you have any questions about this).
> >>
> >>Rich
> >>
> >>Carlin Rogers wrote:
> >>
> >>
> >>
> >>>Just wanted to note that the difference in the behavior is also related
> >>>
> >>>
> >>to a
> >>
> >>
> >>>struts merge where the struts module config has an action defined with
> >>>
> >>>
> >>the
> >>
> >>
> >>>"unknown" attribute (making it like a default). I think the missing
> >>>condition is that we check to see if the GlobalApp has the action
> config
> >>>
> >>>
> >>but
> >>
> >>
> >>>we don't check any of the action configs on the global app to see if
> >>>
> >>>
> >>they're
> >>
> >>
> >>>"unknown".
> >>>
> >>>So, If the global app includes a Struts Merge and that struts module
> >>>
> >>>
> >>config
> >>
> >>
> >>>includes an unknown action, we'll never hit it.
> >>>
> >>>Carlin
> >>>
> >>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
> >>>
> >>>
> >>>
> >>>
> >>>>Hey Rich,
> >>>>
> >>>>Hope your work is going well!
> >>>>
> >>>>I have a question about svn revision 356056 (
> >>>>
> >>>>
> >>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
> >>
> >>
> >>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
> >>>>behavior of PageFlowRequestProcessor.processMapping() and how we
> handle
> >>>>
> >>>>
> >>an
> >>
> >>
> >>>>unknown action. With this change, the code path for an unknown action
> in
> >>>>processMapping() fails the new check to see if it is in the globalApp
> >>>>(...globalApp.findActionConfig(path) != null). We drop to the else
> >>>>
> >>>>
> >>statement
> >>
> >>
> >>>>and into a call to processUnresolvedAction() which uses the
> >>>>DefaultExceptionsHandler class and eventually writes out the HTML of
> our
> >>>>action not found error message directly to the response. I think this
> >>>>
> >>>>
> >>looks
> >>
> >>
> >>>>OK. However, having the error message written to the response may not
> be
> >>>>
> >>>>
> >>the
> >>
> >>
> >>>>desired behavior for something like a portal using a call to
> >>>>PageFlowUtils.strutsLookup(). What do you think?
> >>>>
> >>>>If we leave the fix as is, could we use the
> >>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if this
> >>>>
> >>>>
> >>is
> >>
> >>
> >>>>from strutsLookup() or not before calling processUnresolvedAction().
> I.E
> >>>
> >>>
> >>.
> >>
> >>
> >>>>do something different for an unknown action in a strutsLookup()? Just
> >>>>curious.
> >>>>
> >>>>Thanks,
> >>>>Carlin
> >>>>
> >>>>
> >>>>
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>
> >>
> >
> >
> >
>
Re: Question about BEEHIVE-1024
Posted by Rich Feit <ri...@gmail.com>.
Hey Carlin,
Just want to make sure I'm understanding here. Are you saying that the
original call to initModuleConfig() (the one I removed) also registered
the module in the ServletContext, as a side effect? It was only
supposed to set the reference to _moduleConfig, which is now always
taken care of in getModuleConfig(). Is the problem happening because
the module isn't registered in the ServletContext, or because
_moduleConfig is somehow null?
Rich
Carlin Rogers wrote:
>Hey Rich,
>
>Thanks for the reply and the information on the
>InternalUtils.avoidDirectResponseOutput() function.
>
>I looked at the issue some more and it turns out that for the scenario I'm
>investigating, there's another revision that is also impacting the behavior.
>In revision 351812, http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev ,
>for BEEHIVE-1017, the FlowController.reinitialize() method was modified so
>that in no longer calls initModuleConfig(), which ensured that the module
>config was registered (attribute of the context).
>
>Now, when the initial page flow of the portal scenario is opened in a
>portlet, the GlobalApp module config is not added to the servlet context
>attributes. Then when the unhandled action is hit and we fall into
>processMapping(), the call to InternalUtils.getModuleConfig() for the
>GlobalApp module config will be null and we wouldn't even be able to check
>for an action that was declared as "unknown".
>
>Unfortunately, I'm still trying to understand why the call to from
>FlowController.reinitialize() to initModuleConfig() was removed.
>
>I've logged a JIRA issue on this and am trying to figure out what is the
>best way to resolve the problem. See
>http://issues.apache.org/jira/browse/BEEHIVE-1037
>The bug description might be a better illustration of the scenario I'm
>trying to solve.
>
>Let me know if you have some more thoughts about how best to resolve this.
>
>Many thanks,
>Carlin
>
>On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
>
>
>>Hey Carlin,
>>
>>Sorry for the delay. I agree that we should be checking for an
>>"unknown" action mapping in the global app module, so if you're
>>suggesting making that change, I agree.
>>
>>My answer to the rest is a little more involved:
>>
>> - There's already a rough mechanism for avoiding direct response
>>output. In InternalUtils, there's avoidDirectResponseOutput(). If
>>that's called on the request, then InternalUtils.sendError() will throw
>>an exception instead of writing to the response. This is what I think
>>should be happening here -- we should be calling sendError().
>>
>> - I think that two things should probably change here: 1)
>>InternalUtils.avoidDirectResponseOutput() should be replaced with a flag
>>in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
>>flag off the bat. We shouldn't be writing to the response *ever* during
>>strutsLookup().
>>
>>Let me know what you think (and if you have any questions about this).
>>
>>Rich
>>
>>Carlin Rogers wrote:
>>
>>
>>
>>>Just wanted to note that the difference in the behavior is also related
>>>
>>>
>>to a
>>
>>
>>>struts merge where the struts module config has an action defined with
>>>
>>>
>>the
>>
>>
>>>"unknown" attribute (making it like a default). I think the missing
>>>condition is that we check to see if the GlobalApp has the action config
>>>
>>>
>>but
>>
>>
>>>we don't check any of the action configs on the global app to see if
>>>
>>>
>>they're
>>
>>
>>>"unknown".
>>>
>>>So, If the global app includes a Struts Merge and that struts module
>>>
>>>
>>config
>>
>>
>>>includes an unknown action, we'll never hit it.
>>>
>>>Carlin
>>>
>>>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>>>
>>>
>>>
>>>
>>>>Hey Rich,
>>>>
>>>>Hope your work is going well!
>>>>
>>>>I have a question about svn revision 356056 (
>>>>
>>>>
>>http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
>>
>>
>>>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
>>>>behavior of PageFlowRequestProcessor.processMapping() and how we handle
>>>>
>>>>
>>an
>>
>>
>>>>unknown action. With this change, the code path for an unknown action in
>>>>processMapping() fails the new check to see if it is in the globalApp
>>>>(...globalApp.findActionConfig(path) != null). We drop to the else
>>>>
>>>>
>>statement
>>
>>
>>>>and into a call to processUnresolvedAction() which uses the
>>>>DefaultExceptionsHandler class and eventually writes out the HTML of our
>>>>action not found error message directly to the response. I think this
>>>>
>>>>
>>looks
>>
>>
>>>>OK. However, having the error message written to the response may not be
>>>>
>>>>
>>the
>>
>>
>>>>desired behavior for something like a portal using a call to
>>>>PageFlowUtils.strutsLookup(). What do you think?
>>>>
>>>>If we leave the fix as is, could we use the
>>>>PageFlowRequestWrapper.isScopedLookup() condition to determine if this
>>>>
>>>>
>>is
>>
>>
>>>>from strutsLookup() or not before calling processUnresolvedAction(). I.E
>>>
>>>
>>.
>>
>>
>>>>do something different for an unknown action in a strutsLookup()? Just
>>>>curious.
>>>>
>>>>Thanks,
>>>>Carlin
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>
>>
>
>
>
Re: Question about BEEHIVE-1024
Posted by Carlin Rogers <ca...@gmail.com>.
Hey Rich,
Thanks for the reply and the information on the
InternalUtils.avoidDirectResponseOutput() function.
I looked at the issue some more and it turns out that for the scenario I'm
investigating, there's another revision that is also impacting the behavior.
In revision 351812, http://svn.apache.org/viewcvs.cgi?rev=351812&view=rev ,
for BEEHIVE-1017, the FlowController.reinitialize() method was modified so
that in no longer calls initModuleConfig(), which ensured that the module
config was registered (attribute of the context).
Now, when the initial page flow of the portal scenario is opened in a
portlet, the GlobalApp module config is not added to the servlet context
attributes. Then when the unhandled action is hit and we fall into
processMapping(), the call to InternalUtils.getModuleConfig() for the
GlobalApp module config will be null and we wouldn't even be able to check
for an action that was declared as "unknown".
Unfortunately, I'm still trying to understand why the call to from
FlowController.reinitialize() to initModuleConfig() was removed.
I've logged a JIRA issue on this and am trying to figure out what is the
best way to resolve the problem. See
http://issues.apache.org/jira/browse/BEEHIVE-1037
The bug description might be a better illustration of the scenario I'm
trying to solve.
Let me know if you have some more thoughts about how best to resolve this.
Many thanks,
Carlin
On 1/9/06, Rich Feit <ri...@privatei.com> wrote:
>
> Hey Carlin,
>
> Sorry for the delay. I agree that we should be checking for an
> "unknown" action mapping in the global app module, so if you're
> suggesting making that change, I agree.
>
> My answer to the rest is a little more involved:
>
> - There's already a rough mechanism for avoiding direct response
> output. In InternalUtils, there's avoidDirectResponseOutput(). If
> that's called on the request, then InternalUtils.sendError() will throw
> an exception instead of writing to the response. This is what I think
> should be happening here -- we should be calling sendError().
>
> - I think that two things should probably change here: 1)
> InternalUtils.avoidDirectResponseOutput() should be replaced with a flag
> in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
> flag off the bat. We shouldn't be writing to the response *ever* during
> strutsLookup().
>
> Let me know what you think (and if you have any questions about this).
>
> Rich
>
> Carlin Rogers wrote:
>
> >Just wanted to note that the difference in the behavior is also related
> to a
> >struts merge where the struts module config has an action defined with
> the
> >"unknown" attribute (making it like a default). I think the missing
> >condition is that we check to see if the GlobalApp has the action config
> but
> >we don't check any of the action configs on the global app to see if
> they're
> >"unknown".
> >
> >So, If the global app includes a Struts Merge and that struts module
> config
> >includes an unknown action, we'll never hit it.
> >
> >Carlin
> >
> >On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
> >
> >
> >>Hey Rich,
> >>
> >>Hope your work is going well!
> >>
> >>I have a question about svn revision 356056 (
> http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
> >>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
> >>behavior of PageFlowRequestProcessor.processMapping() and how we handle
> an
> >>unknown action. With this change, the code path for an unknown action in
> >>processMapping() fails the new check to see if it is in the globalApp
> >>(...globalApp.findActionConfig(path) != null). We drop to the else
> statement
> >>and into a call to processUnresolvedAction() which uses the
> >>DefaultExceptionsHandler class and eventually writes out the HTML of our
> >>action not found error message directly to the response. I think this
> looks
> >>OK. However, having the error message written to the response may not be
> the
> >>desired behavior for something like a portal using a call to
> >>PageFlowUtils.strutsLookup(). What do you think?
> >>
> >>If we leave the fix as is, could we use the
> >>PageFlowRequestWrapper.isScopedLookup() condition to determine if this
> is
> >>from strutsLookup() or not before calling processUnresolvedAction(). I.E
> .
> >>do something different for an unknown action in a strutsLookup()? Just
> >>curious.
> >>
> >>Thanks,
> >>Carlin
> >>
> >>
> >>
> >
> >
> >
>
>
Re: Question about BEEHIVE-1024
Posted by Rich Feit <ri...@privatei.com>.
Hey Carlin,
Sorry for the delay. I agree that we should be checking for an
"unknown" action mapping in the global app module, so if you're
suggesting making that change, I agree.
My answer to the rest is a little more involved:
- There's already a rough mechanism for avoiding direct response
output. In InternalUtils, there's avoidDirectResponseOutput(). If
that's called on the request, then InternalUtils.sendError() will throw
an exception instead of writing to the response. This is what I think
should be happening here -- we should be calling sendError().
- I think that two things should probably change here: 1)
InternalUtils.avoidDirectResponseOutput() should be replaced with a flag
in PageFlowRequestWrapper, and 2) strutsLookup() should just set this
flag off the bat. We shouldn't be writing to the response *ever* during
strutsLookup().
Let me know what you think (and if you have any questions about this).
Rich
Carlin Rogers wrote:
>Just wanted to note that the difference in the behavior is also related to a
>struts merge where the struts module config has an action defined with the
>"unknown" attribute (making it like a default). I think the missing
>condition is that we check to see if the GlobalApp has the action config but
>we don't check any of the action configs on the global app to see if they're
>"unknown".
>
>So, If the global app includes a Struts Merge and that struts module config
>includes an unknown action, we'll never hit it.
>
>Carlin
>
>On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>
>
>>Hey Rich,
>>
>>Hope your work is going well!
>>
>>I have a question about svn revision 356056 ( http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
>>) checked in as a fix for BEEHIVE-1024. It seems that it changed the
>>behavior of PageFlowRequestProcessor.processMapping() and how we handle an
>>unknown action. With this change, the code path for an unknown action in
>>processMapping() fails the new check to see if it is in the globalApp
>>(...globalApp.findActionConfig(path) != null). We drop to the else statement
>>and into a call to processUnresolvedAction() which uses the
>>DefaultExceptionsHandler class and eventually writes out the HTML of our
>>action not found error message directly to the response. I think this looks
>>OK. However, having the error message written to the response may not be the
>>desired behavior for something like a portal using a call to
>>PageFlowUtils.strutsLookup(). What do you think?
>>
>>If we leave the fix as is, could we use the
>>PageFlowRequestWrapper.isScopedLookup() condition to determine if this is
>>from strutsLookup() or not before calling processUnresolvedAction(). I.E.
>>do something different for an unknown action in a strutsLookup()? Just
>>curious.
>>
>>Thanks,
>>Carlin
>>
>>
>>
>
>
>
Re: Question about BEEHIVE-1024
Posted by Carlin Rogers <ca...@gmail.com>.
Just wanted to note that the difference in the behavior is also related to a
struts merge where the struts module config has an action defined with the
"unknown" attribute (making it like a default). I think the missing
condition is that we check to see if the GlobalApp has the action config but
we don't check any of the action configs on the global app to see if they're
"unknown".
So, If the global app includes a Struts Merge and that struts module config
includes an unknown action, we'll never hit it.
Carlin
On 1/5/06, Carlin Rogers <ca...@gmail.com> wrote:
>
> Hey Rich,
>
> Hope your work is going well!
>
> I have a question about svn revision 356056 ( http://svn.apache.org/viewcvs.cgi?rev=356056&view=rev
> ) checked in as a fix for BEEHIVE-1024. It seems that it changed the
> behavior of PageFlowRequestProcessor.processMapping() and how we handle an
> unknown action. With this change, the code path for an unknown action in
> processMapping() fails the new check to see if it is in the globalApp
> (...globalApp.findActionConfig(path) != null). We drop to the else statement
> and into a call to processUnresolvedAction() which uses the
> DefaultExceptionsHandler class and eventually writes out the HTML of our
> action not found error message directly to the response. I think this looks
> OK. However, having the error message written to the response may not be the
> desired behavior for something like a portal using a call to
> PageFlowUtils.strutsLookup(). What do you think?
>
> If we leave the fix as is, could we use the
> PageFlowRequestWrapper.isScopedLookup() condition to determine if this is
> from strutsLookup() or not before calling processUnresolvedAction(). I.E.
> do something different for an unknown action in a strutsLookup()? Just
> curious.
>
> Thanks,
> Carlin
>