On 6 Jan 2010, at 13:01, Felix Meschberger wrote:

> Hi Ian,
> 
> On 06.01.2010 11:57, Ian Boston wrote:
>> I agree that we should use the ServletRequestListener support, but I 
>> continue to wonder if there isnt a need for a request lifecycle to be 
>> exposed by the sling main servlet which would avoid having to a) create a 
>> stack of ServletFilters b) bind the SlingMainServlet to other apis' c) bind 
>> to the ServletRequestListener which is as you note not always supported ?
> 
> We might want to add request lifecycle events to the SlingMainServlet as
> far as the service method is concerned. This would work perfectly fine
> for requests handled through the SlingMainServlet (which is the majority
> of the requests handled by a Sling instance).
> 
> Specifically the WebDAV requests to the /dav subtree would not benefit
> from this (unless we add the same lifecycle support there ...)
> 
> Still: this would not help in the authentication case because
> authentication is handled outside of the SlingMainServlet before the
> service method is even called. In fact if authentication fails, the
> SlingMainServlet.service method is not called at all.


good point.

> 
>> 
>> At present we have ServletFilters for a XA Transaction manager and Request 
>> Scope Caching, the stack is good in some senses since we can perform try 
>> finally but we rely heavily on the programmer knowing exactly what is safe 
>> to do.
> 
> How about adding a SlingRequestListener interface, which mimicks the
> ServletRequestListener interface:
> 
>   public interface SlingRequestListener {
>        void requestDestroyed(SlingRequestEvent sre);
>        void requestInitialized(SlingRequestEvent sre);
>   }
> 
>   public class SlingRequestEvent {
>        SlingHttpServletRequest getSlingRequest();
>   }
> 
> The methods would be called at the same time, the RequestLogger methods
> are called (lines 252 and 363 in trunk SlingMainServlet).
> 
> WDYT ?


yes that makes perfect sense, the calls obviously wrapped in try catch to 
prevent anything doing bad things on destroy, ie the event is only for 
notification purposes and not for flow control etc

Ian

> 
> Regards
> Felix
> 
>> 
>> Ian
>> 
>> On 6 Jan 2010, at 10:37, Felix Meschberger (JIRA) wrote:
>> 
>>> 
>>>   [ 
>>> https://issues.apache.org/jira/browse/SLING-1270?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12797052#action_12797052
>>>  ] 
>>> 
>>> Felix Meschberger commented on SLING-1270:
>>> ------------------------------------------
>>> 
>>> Committed temporary session logout in the SlingMainServlet in Rev. 896372.
>>> 
>>>> Replace Session.logout from SlingMainServlet
>>>> --------------------------------------------
>>>> 
>>>>               Key: SLING-1270
>>>>               URL: https://issues.apache.org/jira/browse/SLING-1270
>>>>           Project: Sling
>>>>        Issue Type: Task
>>>>        Components: Engine
>>>>  Affects Versions: Engine 2.0.6
>>>>          Reporter: Felix Meschberger
>>>>          Assignee: Felix Meschberger
>>>>           Fix For: Engine 2.1.0
>>>> 
>>>> 
>>>> The new Commons Auth bundle from SLING-966 registers a 
>>>> ServletRequestListener to be informed when the request has terminated and 
>>>> the session may be logged out. Currently, the Http Service implementation 
>>>> does not support such listeners and the session may not be logged out at 
>>>> all.
>>>> As a workaround the Commons Auth bundle implements a Java VM finalize() 
>>>> method to try to ensure logging the session out.
>>>> As a further workaround the SlingMainServlet should - in a finally clause 
>>>> - logout the session of the request's resource resolver.
>>>> The SlingMainServlet configuration should be removed as soon as we can 
>>>> reasonably be sure of ServletRequestListener support.
>>> 
>>> -- 
>>> This message is automatically generated by JIRA.
>>> -
>>> You can reply to this email to add a comment to the issue online.
>>> 
>> 
>> 

Reply via email to