Mailing-List: contact dev-help@beehive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Beehive Developers" <dev@beehive.apache.org>
Received-SPF: pass (asf.osuosl.org: domain of carlin.rogers@gmail.com
 designates 64.233.162.196 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references;
        b=PXDUF/74XhRAoL2aS1sFbh7aCbChGL5fGWA8a10DotT2a4VcQf6peyDDFauqDqhyQ9pHt5utKEU/cWgg+qDOg/CqKLWmj4ZLbx4javRt+Wq3cMt00KomdlSuPi55w3f0uDt4VL1vCAPvl/tuUDuxqXoSFOXEZ6XbQZP1FruPEa8=
Message-ID: <ba6911de0601101009i383765acj23c5baf638a528b0@mail.gmail.com>
Date: Tue, 10 Jan 2006 11:09:07 -0700
From: Carlin Rogers <carlin.rogers@gmail.com>
To: Beehive Developers <dev@beehive.apache.org>
Subject: Re: Question about BEEHIVE-1024
In-Reply-To: <43C3DADC.1010708@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="----=_Part_17304_24503283.1136916547487"
References: <ba6911de0601052222w24a5fb54t6e50a6441c6e4ebe@mail.gmail.com>
	 <ba6911de0601061701o22062b3dha0e57c2f2c728b4f@mail.gmail.com>
	 <43C2105A.20309@privatei.com>
	 <ba6911de0601091741x59d47089w911f4737d16bc7ef@mail.gmail.com>
	 <43C31770.40508@gmail.com>
	 <ba6911de0601092102m77e5852aw17ae011f46f15220@mail.gmail.com>
	 <43C3BD46.5020608@gmail.com>
	 <ba6911de0601100707m757071b6pf44c9058ad1ea6a2@mail.gmail.com>
	 <43C3DADC.1010708@gmail.com>

------=_Part_17304_24503283.1136916547487
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline

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, wer=
e
> >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 t=
o
> >>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 bee=
n
> >>>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.ensureModuleRegistere=
d
> ()
> >>>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 confi=
g
> >>>
> >>>
> >>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=3D351812&view=3Drev,
> >>
> >>
> >>>>>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 th=
e
> >>>>>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 missin=
g
> >>>>>>>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=3D356056&view=3Drev
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>>>) 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) !=3D null). We drop to the e=
lse
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>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
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
>

------=_Part_17304_24503283.1136916547487--