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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > > > > > >
