beehive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carlin Rogers <carlin.rog...@gmail.com>
Subject Re: Question about BEEHIVE-1024
Date Tue, 10 Jan 2006 18:09:07 GMT
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 <richfeit@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 <richfeit@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 <richfeit@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 <richfeit@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 <carlin.rogers@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
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message