You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Justin Edelson (Created) (JIRA)" <ji...@apache.org> on 2012/02/09 16:12:03 UTC

[jira] [Created] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
-------------------------------------------------------------------------------

                 Key: SLING-2411
                 URL: https://issues.apache.org/jira/browse/SLING-2411
             Project: Sling
          Issue Type: Improvement
            Reporter: Justin Edelson


The ResourceDecorator currently has two methods:
decorate(Resource)
decorate(Resource, HttpServletRequest)

The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.

However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Resolved] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Resolved) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Justin Edelson resolved SLING-2411.
-----------------------------------

    Resolution: Fixed

deprecation done in r1245269
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>          Components: API, JCR
>            Reporter: Justin Edelson
>            Assignee: Justin Edelson
>             Fix For: JCR Resource 2.1.0, API 2.2.6
>
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Justin Edelson updated SLING-2411:
----------------------------------

    Fix Version/s: API 2.2.6
                   JCR Resource 2.1.0
         Assignee: Justin Edelson

setting fix versions
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>            Assignee: Justin Edelson
>             Fix For: JCR Resource 2.1.0, API 2.2.6
>
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Felix Meschberger (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205372#comment-13205372 ] 

Felix Meschberger commented on SLING-2411:
------------------------------------------

> Yes, as I said, it was just a quick thought - but then using a thread local inside the resolver doesn't look right either 

Oh ! Sorry. I didn't fully read this issue and proposal. No, I don't like this thread local implementation proposal either.

Considering this and considering the request is not always available, I think I favor an approach already mentioned: We deprecate the method with the request argument (and might even define that null is always passed in). If a decorator then requires access to the request, it is the task of that decorate to provide the request, for example in a thread local provided by a filter.

Or Sling provides such a (component level) filter which provides service API to get the current thread's request, response, and resource. This can be implemented in a separate bundle, is not intrusive in existing code and would provide actual data for all.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Carsten Ziegeler (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205298#comment-13205298 ] 

Carsten Ziegeler commented on SLING-2411:
-----------------------------------------

Just a thought, what about passing the request into the resource resolver factory when it is created? The created resolver can then just keep the request object

Using a thread local has the potential surprise that all resource resolvers running in that thread are using the request object, for example long running admin resolvers from a used service etc. This can be wanted or unexpected. Not sure actually
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Carsten Ziegeler (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205367#comment-13205367 ] 

Carsten Ziegeler commented on SLING-2411:
-----------------------------------------

> A ResourceResolver is not bound to a request and thus using it for the factory sounds wrong. 
Yes, as I said, it was just a quick thought - but then using a thread local inside the resolver doesn't look right either
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Julian Sedding (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Julian Sedding updated SLING-2411:
----------------------------------

    Attachment: SLING-2411-jsedding.patch

I attached a patch to illustrate what I have in mind. Currently, I am not 100% satisfied with the patch, but maybe someone has a good idea, how to address the following issues.

1. The RequestScopedResourceResolverWrapper class needs to be exported from the bundle, if it should be available for use by e.g. the ResourceDecoratorTracker. Should it be in the API bundle or exported by the engine bundle (that's how it's done in the patch)? My gut feeling is that the ResourceDecorator(Tracker) does not actually belong into the jcr.resources bundle. This should become more evident when the ResourceResolver is decoupled from the JCR-ResourceProvider (see SLING-2396).

2. I'm not certain whether it can be dangerous/undesired that RequestScopedResourceResolverWrapper redirects map(String) to map(Request, String).

Eventually, if we decide to go down this route, it may make sense to allow registering ResourceResolverDecorator services, in a similar way to the current ResourceDecorator services. Curiously, I belive that the ResourceDecorator mechanism could be fairly easily implemented as a ResourceResolverDecorator.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204578#comment-13204578 ] 

Justin Edelson commented on SLING-2411:
---------------------------------------

strawman patch here: http://codereview.appspot.com/5653043

creates a ThreadLocal to store the request and then the ResourceDecoratorTracker tries to use this if necessary.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Julian Sedding (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13206405#comment-13206405 ] 

Julian Sedding commented on SLING-2411:
---------------------------------------

Please don't let the many lines of new code distract you. They are mainly generic implementations of ResourceResolverWrapper and IteratorWrapper. The "real" changes are the addition of RequestScopedResourceResolverWrapper and changes in SlingMainServlet and ResourceDecoratorTracker.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Felix Meschberger (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205328#comment-13205328 ] 

Felix Meschberger commented on SLING-2411:
------------------------------------------

A ResourceResolver is not bound to a request and thus using it for the factory sounds wrong.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205952#comment-13205952 ] 

Justin Edelson commented on SLING-2411:
---------------------------------------

Sorry for any confusion I may have caused. I had intended to send out a VOTE email to the dev list on deprecating the two-argument method following the creation of this issue, but it looks like I never hit send.

The patch is just what I said above - a strawman. IF we are not going to deprecate the method, then I think we need to do something like the strawman. Julian's proposal sounds interesting and I'm curious to see it.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Issue Comment Edited] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Julian Sedding (Issue Comment Edited) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205364#comment-13205364 ] 

Julian Sedding edited comment on SLING-2411 at 2/10/12 11:14 AM:
-----------------------------------------------------------------

> A ResourceResolver is not bound to a request and thus using it for the factory sounds wrong.
I agree with this statement. Taking this a step further, maybe it would be worthwhile to factor out the "resolve" and "map" methods into a different service (something Vidar may already be doing in his work to decouple JCR from the ResourceResolverFactory). The methods "resolve" and "map" seem to be inherently about requests: they provide the plumbing to get from the "external" address (i.e. URI) of a resource to the "internal" address (i.e. location in Sling's resource tree). The ResourceResolver javadocs suggest this as well, regarding "resolve": "This kind of method is intended to resolve request URLs to resources."

Actually, this may be a topic for the list.

While the changes are fairly minimal in the patch, I don't like the approach. I believe the ThreadLocals shouldn't be necessary. Rather, I considered wrapping the ResourceResolver, so it can piggyback the request (of course this is optional and not applicable to e.g. an administrative resource resolver). The ResourceDecoratorTracker could then check if the ResourceResolver is an instance of the respective wrapper, cast it and get the piggybacked request. The natural place for this wrapping to happen would be in the SlingMainServlet, just before the call to processRequest.

On the other hand, I'm not too keen to put more and more functionality into the SlingMainServlet. So an alternative might be to wrap the Request (e.g. using a Filter) and let the Request#getResourceResolver return a wrapped ResourceResolver as explained above. I'm not yet sure how feasible this approach is, because ideally the ResourceResolver would be wrapped before the RequestData#initResource is called.

I've started working on a patch and hope to get this finished at the weekend. I'm interested in reading your thoughts.
                
      was (Author: jsedding):
    > A ResourceResolver is not bound to a request and thus using it for the factory sounds wrong.
I agree with this statement. Taking this a step further, maybe it would be worthwhile to factor out the "resolve" and "map" methods into a different service (something Vidar may already be doing in his work to decouple JCR from the ResourceResolverFactory). The methods "resolve" and "map" seem to be inherently about requests: they provide the plumbing to get from the "external" address (i.e. URI) of a resource to the "internal" address (i.e. location in Sling's resource tree).

Actually, this may be a topic for the list.

While the changes are fairly minimal in the patch, I don't like the approach. I believe the ThreadLocals shouldn't be necessary. Rather, I considered wrapping the ResourceResolver, so it can piggyback the request (of course this is optional and not applicable to e.g. an administrative resource resolver). The ResourceDecoratorTracker could then check if the ResourceResolver is an instance of the respective wrapper, cast it and get the piggybacked request. The natural place for this wrapping to happen would be in the SlingMainServlet, just before the call to processRequest.

On the other hand, I'm not too keen to put more and more functionality into the SlingMainServlet. So an alternative might be to wrap the Request (e.g. using a Filter) and let the Request#getResourceResolver return a wrapped ResourceResolver as explained above. I'm not yet sure how feasible this approach is, because ideally the ResourceResolver would be wrapped before the RequestData#initResource is called.

I've started working on a patch and hope to get this finished at the weekend. I'm interested in reading your thoughts.
                  
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Justin Edelson updated SLING-2411:
----------------------------------

    Component/s: JCR
                 API
    
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>          Components: API, JCR
>            Reporter: Justin Edelson
>            Assignee: Justin Edelson
>             Fix For: JCR Resource 2.1.0, API 2.2.6
>
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Justin Edelson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13209855#comment-13209855 ] 

Justin Edelson commented on SLING-2411:
---------------------------------------

Vote to deprecate has passed. See thread here: http://sling.markmail.org/thread/fozcoxaylos72c2w
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Julian Sedding (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205364#comment-13205364 ] 

Julian Sedding commented on SLING-2411:
---------------------------------------

> A ResourceResolver is not bound to a request and thus using it for the factory sounds wrong.
I agree with this statement. Taking this a step further, maybe it would be worthwhile to factor out the "resolve" and "map" methods into a different service (something Vidar may already be doing in his work to decouple JCR from the ResourceResolverFactory). The methods "resolve" and "map" seem to be inherently about requests: they provide the plumbing to get from the "external" address (i.e. URI) of a resource to the "internal" address (i.e. location in Sling's resource tree).

Actually, this may be a topic for the list.

While the changes are fairly minimal in the patch, I don't like the approach. I believe the ThreadLocals shouldn't be necessary. Rather, I considered wrapping the ResourceResolver, so it can piggyback the request (of course this is optional and not applicable to e.g. an administrative resource resolver). The ResourceDecoratorTracker could then check if the ResourceResolver is an instance of the respective wrapper, cast it and get the piggybacked request. The natural place for this wrapping to happen would be in the SlingMainServlet, just before the call to processRequest.

On the other hand, I'm not too keen to put more and more functionality into the SlingMainServlet. So an alternative might be to wrap the Request (e.g. using a Filter) and let the Request#getResourceResolver return a wrapped ResourceResolver as explained above. I'm not yet sure how feasible this approach is, because ideally the ResourceResolver would be wrapped before the RequestData#initResource is called.

I've started working on a patch and hope to get this finished at the weekend. I'm interested in reading your thoughts.
                
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>            Reporter: Justin Edelson
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Closed] (SLING-2411) ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently

Posted by "Carsten Ziegeler (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-2411?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Carsten Ziegeler closed SLING-2411.
-----------------------------------

    
> ResourceDecorator(Resource, HttpServletRequest) doesn't get called consistently
> -------------------------------------------------------------------------------
>
>                 Key: SLING-2411
>                 URL: https://issues.apache.org/jira/browse/SLING-2411
>             Project: Sling
>          Issue Type: Improvement
>          Components: API, JCR
>            Reporter: Justin Edelson
>            Assignee: Justin Edelson
>             Fix For: JCR Resource 2.1.0, API 2.2.6
>
>         Attachments: SLING-2411-jsedding.patch
>
>
> The ResourceDecorator currently has two methods:
> decorate(Resource)
> decorate(Resource, HttpServletRequest)
> The JcrResourceResolver uses the latter method when resolve(HttpServletRequest,String) is invoked. In all other cases, the former method is used. This behavior is correct.
> However, the JcrResourceResolver (and any future ResourceResolver implementation for that matter) really doesn't have any control over how it is invoked. For example, at present <sling:include> and <sling:forward> call resolve(String), not resolve(HttpServletRequest,String) (see http://s.apache.org/lL5). But even if we did a code audit within Sling and fixed all the cases like this, we don't have any control over downstream applications which may or may not invoke the two-argument resolve() method.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira