beehive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rich Feit <richf...@gmail.com>
Subject Re: Question about BEEHIVE-1024
Date Tue, 10 Jan 2006 13:57:26 GMT
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
View raw message