Re: [jira] Commented: (PLUTO-598) Retrieving Portlet invoked servlet request attributes should first check PortletRequest attributes before using fallback to the web container

[Eric Dalquist <er...@doit.wisc.edu>]

I just added the SPI we talked about to trunk: 
http://svn.apache.org/viewvc?rev=1053938&view=rev

Now the HttpServletPortletRequestWrapper gets the PortletRequestContext 
from the PortletRequest it is provided with and PortletRequestContext 
has an additional Object getAttribute(String name, ServletRequest 
servletRequest) API. The default implementation of that API in pluto is 
what was happening in HttpServletPortletRequestWrapper: "return 
servletRequest.getAttribute(name)".

This shouldn't change the default Pluto behavior at all plus it allows 
portal's complete control over the attribute resolution logic if they so 
desire.

-Eric

On 11/22/10 8:32 PM, Eric Dalquist wrote:
> Sounds good. Next time I try a SNAPSHOT update of 2.1 I'll get this 
> updated and file a new Jira referencing the two you touched tonight.
>
> Thanks for the heads up on this.
> -Eric
>
> On 11/22/10 6:28 PM, Ate Douma wrote:
>> On 23/11/10 03:13, Eric Dalquist wrote:
>>> That case, where the portal wants to hide attributes, is exactly the
>>> problem. One easy example of this problem:
>>> 1. The portal uses Spring's DispatcherServlet as the servlet which
>>> handles portal requests. This class sets a bunch of request attributes
>>> that the Spring framework uses for things like localization.
>> Right, that explains it.
>> Jetspeed doesn't use an external framework for its main operation so 
>> we never encountered this problem.
>>
>>> 2. The portal dispatches to a portlet (which lives in a separate 
>>> webapp)
>>> which uses Spring's DispatcherPortlet. This class uses many of the same
>>> request attributes for many of the same features. This causes lots of
>>> problems which manifest as ClassCastExceptions when the portlet 
>>> tries to
>>> cast SpringClassA from the portal webapp to SpringClassA from the
>>> portlet webapp which fails since they are from different classloaders.
>>>
>>> The portal has to be able to hide certain request attributes from the
>>> portlet to prevent this problem.
>>>
>>>
>>> Your idea of adding "getAttribute(String name, ServletRequest
>>> currentRequest)" to the SPI seems like a good one, then the portal can
>>> handle this resolution logic and hide attributes as needed as well as
>>> having the full context available. My only concern is 2.1.x/trunk so 
>>> I'm
>>> fine with leaving the fallback logic in 2.0.x primarily since it 
>>> doesn't
>>> affect uPortal.
>> OK, I'll leave this SPI change to you then.
>> For Jetspeed we might switch to Pluto 2.1.0 somewhat later.
>> Adjusting to the new SPI then won't be a problem.
>>
>> Regards,
>>
>> Ate
>>
>>>
>>> Thanks for the email,
>>> -Eric
>>>
>>> On 11/22/10 6:05 PM, Ate Douma wrote:
>>>> On 23/11/10 02:44, Eric Dalquist (JIRA) wrote:
>>>>>
>>>>> [
>>>>> https://issues.apache.org/jira/browse/PLUTO-598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12934693#action_12934693 
>>>>>
>>>>> ]
>>>>>
>>>>> Eric Dalquist commented on PLUTO-598:
>>>>> -------------------------------------
>>>>>
>>>>> So my primary concern here is the when the portal wants to have
>>>>> complete control the request attributes a portlet sees when
>>>>> getAttribute() is called on the PortletRequest or the ServletRequest
>>>>> based on the PortletRequest. The primary issue I was trying to
>>>>> address with PLUTO-600 was having the fallback logic in the
>>>>> HttpServletPortletRequestWrapper means that there is no reasonable
>>>>> way for a portal to influence this attribute resolution logic. My
>>>>> request is that if at all possible this logic should be part of the
>>>>> default PortletRequestContext SPI implementation so that implementing
>>>>> portals can still have full control over this attribute resolution.
>>>>
>>>> Hi Eric,
>>>>
>>>> AFAIK the portal still has (and had) this level of control already.
>>>> The only exception I've now added is for these specific and servlet
>>>> container managed attributes.
>>>> But this is still configurable if you really want or need to "disable"
>>>> or customize this, although I think a portal shouldn't (cannot)
>>>> interfere with the servlet container on this level (request dispacher
>>>> state managment).
>>>>
>>>> With regard to PLUTO-600: except for these specific attributes, flow
>>>> of control is always passed on to the (portal controllable)
>>>> portletRequest.getAttribute method.
>>>>
>>>> The only possible problem I can see (just now occurred to me while
>>>> writing this) would be when the portal explicitly wants to "hide"
>>>> certain attributes, e.g. by returning a null value.
>>>> The solution I just committed then won't work as in that case the
>>>> HttpServletPortletRequestWrapper will still delegate to
>>>> getRequest().getAttribute thereafter...
>>>>
>>>> Hmm, I need to think a little bit more about it, but probably just
>>>> only delegating to portletRequest.getAttribute might be good enough,
>>>> like your original PLUTO-600 fix, certainly now that these special
>>>> container managed attributes are taken care of upfront.
>>>> The only possible caveat of that is the portletRequest cannot "see"
>>>> the current HttpServletPortletRequestWrapper its getRequest() parent,
>>>> so it might yield yet a different result!
>>>> The only proper solution for this would be adding a new
>>>> getAttribute(String name, ServletRequest currentRequest) method,
>>>> passing full control over to the portal.
>>>> But that is an API/SPI change not to be taken lightly for PLUTO 2.0.x.
>>>> For PLUTO 2.1.x however it might be OK.
>>>>
>>>> WDYT?
>>>>
>>>> Regards,
>>>>
>>>> Ate
>>>>
>>>>>
>>>>>> Retrieving Portlet invoked servlet request attributes should first
>>>>>> check PortletRequest attributes before using fallback to the web
>>>>>> container
>>>>>> ---------------------------------------------------------------------------------------------------------------------------------------------- 
>>>>>>
>>>>>>
>>>>>>
>>>>>> Key: PLUTO-598
>>>>>> URL: https://issues.apache.org/jira/browse/PLUTO-598
>>>>>> Project: Pluto
>>>>>> Issue Type: Bug
>>>>>> Components: portlet container
>>>>>> Affects Versions: 2.0.2
>>>>>> Reporter: Ate Douma
>>>>>> Assignee: Ate Douma
>>>>>> Fix For: 2.0.3, 2.1.0
>>>>>>
>>>>>>
>>>>>> In
>>>>>> o.a.pluto.container.impl.HttpServletPortletRequestWrapper.getAttribute(String) 
>>>>>>
>>>>>> a (non path related) attribute currently first is looked up from
>>>>>> getRequest().getAttribute(String) with a fallback to the
>>>>>> PortletRequest.getAttribute(String).
>>>>>> However, this is the wrong order: first the
>>>>>> PortletRequest.getAttribute(String) should be checked to ensure a
>>>>>> possibly earlier set attribute which is *only* set with
>>>>>> PortletRequest.setAttribute(String,Object) (and possibly cached
>>>>>> there) is returned.
>>>>>
>>>>
>>>
>>
>