You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Bertrand Delacretaz (JIRA)" <ji...@apache.org> on 2010/07/14 17:00:51 UTC

[jira] Created: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

Reduce coupling between RequestData and SlingHttpServletRequestImpl
-------------------------------------------------------------------

                 Key: SLING-1596
                 URL: https://issues.apache.org/jira/browse/SLING-1596
             Project: Sling
          Issue Type: Improvement
          Components: Engine
    Affects Versions: Engine 2.0.6
            Reporter: Bertrand Delacretaz
            Assignee: Bertrand Delacretaz
            Priority: Minor


As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.

I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.

I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

Posted by "Bertrand Delacretaz (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1596?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12888390#action_12888390 ] 

Bertrand Delacretaz commented on SLING-1596:
--------------------------------------------

> I think we should not do this...

How do you suggest fixing this then?

Currently Sling cannot process a request that is not a SlingHttpServletRequestImpl, even if its is a SlingHttpServletRequest. That's much uglier than exposing an implementation object as an attribute, IMHO.



> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Carsten Ziegeler commented on SLING-1596:
-----------------------------------------

I think we should not do this - request data is internal stuff - like the request impl is and we should not expose this through a request attribute or any other means

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1596?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12888395#action_12888395 ] 

Alexander Klimetschek commented on SLING-1596:
----------------------------------------------

> Usually request wrappers path through request attributes, but what if not? Do we really want to disallow this?

Most likely the wrapper would be written by someone who wants to make it work with Sling, so I think it is no problem to require the wrapper to do that.

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Carsten Ziegeler commented on SLING-1596:
-----------------------------------------

Usually request wrappers path through request attributes, but what if not? Do we really want to disallow this?

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Carsten Ziegeler commented on SLING-1596:
-----------------------------------------

Ok, maybe I don't see the real problem behind this....why not creating http servlet request/response and then pass this through the sling main servlet?

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Felix Meschberger commented on SLING-1596:
------------------------------------------

Some comments to the patch:

  - You should check the actual type of the request attribute (in the getRequestData method) instead of simply casting. If some rogue
      script would set that request attribute, this would cause improperly handled issues
  - Theoretically with SLING-500/SLING-1603 the RequestData request attribute may be set multiple times. Thus setting it must be
      be accompanied with storing any previous value and resetting the previous value at the end of the request

Finally, we should clearly see, where the RequestData.unwrap methods are used and fix that use (the unwrap methods are internal to the bundle, so this search should be complete).

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1596?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12888394#action_12888394 ] 

Alexander Klimetschek commented on SLING-1596:
----------------------------------------------

> Currently Sling cannot process a request that is not a SlingHttpServletRequestImpl, even if its is a SlingHttpServletRequest. That's much uglier than exposing an implementation object as an attribute, IMHO. 

Agreed. Also, placing implementation objects into request attributes is a very common case, eg. for passing through context information. It doesn't affect servlet code (if the name is unique, which is the case by using the fully qualified class name). I don't know of any servlet that iterates over request attributes and fails if there is some unexpected object inside...

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

Posted by "Alexander Klimetschek (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-1596?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12888381#action_12888381 ] 

Alexander Klimetschek commented on SLING-1596:
----------------------------------------------

Makes sense, had that issue once as well. Storing the RequestData as a request attribute makes the code a lot simpler. I think you no longer need the unwrap() methods at all, since a wrapper can be expected to pass through getAttribute calls that don't match locally (and I guess the wrappers in Sling only pass that call through anyway).

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Issue Comment Edited: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Carsten Ziegeler edited comment on SLING-1596 at 7/14/10 11:56 AM:
-------------------------------------------------------------------

Usually request wrappers pass through request attributes, but what if not? Do we really want to disallow this?

      was (Author: cziegeler):
    Usually request wrappers path through request attributes, but what if not? Do we really want to disallow this?
  
> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (SLING-1596) Reduce coupling between RequestData and SlingHttpServletRequestImpl

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

Bertrand Delacretaz updated SLING-1596:
---------------------------------------

    Attachment: SLING-1596.patch

> Reduce coupling between RequestData and SlingHttpServletRequestImpl
> -------------------------------------------------------------------
>
>                 Key: SLING-1596
>                 URL: https://issues.apache.org/jira/browse/SLING-1596
>             Project: Sling
>          Issue Type: Improvement
>          Components: Engine
>    Affects Versions: Engine 2.0.6
>            Reporter: Bertrand Delacretaz
>            Assignee: Bertrand Delacretaz
>            Priority: Minor
>         Attachments: SLING-1596.patch
>
>
> As discussed in http://markmail.org/thread/ldayz27ehldyvzr4 the tight coupling between RequestData and SlingHttpServletRequestImpl makes it impossible to use request classes that just implement SlingHttpServletRequest.
> I need this for example for SLING-550, where servlets run outside of the container's request/response cycle.
> I'll attach a patch that reduces coupling by grabbing the RequestData from a request attribute instead of relying on the SlingHttpServletRequestImpl class to provide it. All tests including integration pass with this patch.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.