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 <bd...@apache.org> on 2010/07/14 17:10:41 UTC
Re: Request Dispatching.
On Fri, Jan 8, 2010 at 5:31 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> ...We have been trying to implement a Batch method processor (ie a servlet that performs multiple requests batching
> the responses up into a single request/response) in Sling.
>
> Normally with servlets this would require requestDispacher.forward(wrappedRequest, wrappedResponse) but this doent
> work since Sling unwrapps all request back to a internal SlingHttpServletRequestImpl object...
I'm hitting the same problem in my work on SLING-550, created
https://issues.apache.org/jira/browse/SLING-1596 with a suggested
patch, also at http://codereview.appspot.com/1783044
My solution is not perfect, to be clean I think we'd need to make
RequestData a first-class interface instead of an implementation
class, but my patch already decreases coupling a bit.
Reviews welcome.
-Bertrand
Re: Request Dispatching.
Posted by Carsten Ziegeler <cz...@apache.org>.
Alexander Klimetschek wrote
> On Wed, Jul 14, 2010 at 18:02, Carsten Ziegeler <cz...@apache.org> wrote:
>> As already noted I'm not happy with this - this is internal stuff and we
>> should rather leave it the way it is.
>> Why not creating plain http servlet request objects and then going
>> through the sling main servlet?
>
> Interesting, how would you do that? I always thought the request
> dispatcher is meant for this.
>
This depends - if you have already a request, the dispatcher is the way
to go.
But as we are then talking about Sling you propably already have a
Sling request object.
If you don't have a request - and my understand is that we're talking
about this case here - you currently have a problem :) The Sling main
servlet is not registered as a service atm, but we could do this if this
is what we need.
Carsten
--
Carsten Ziegeler
cziegeler@apache.org
Re: Request Dispatching.
Posted by Alexander Klimetschek <ak...@day.com>.
On Wed, Jul 14, 2010 at 18:02, Carsten Ziegeler <cz...@apache.org> wrote:
> As already noted I'm not happy with this - this is internal stuff and we
> should rather leave it the way it is.
> Why not creating plain http servlet request objects and then going
> through the sling main servlet?
Interesting, how would you do that? I always thought the request
dispatcher is meant for this.
Regards,
Alex
--
Alexander Klimetschek
alexander.klimetschek@day.com
Re: Request Dispatching.
Posted by Carsten Ziegeler <cz...@apache.org>.
Ian Boston wrote
>
> On 14 Jul 2010, at 16:10, Bertrand Delacretaz wrote:
>
>> On Fri, Jan 8, 2010 at 5:31 PM, Ian Boston <ie...@tfd.co.uk> wrote:
>>> ...We have been trying to implement a Batch method processor (ie a servlet that performs multiple requests batching
>>> the responses up into a single request/response) in Sling.
>>>
>>> Normally with servlets this would require requestDispacher.forward(wrappedRequest, wrappedResponse) but this doent
>>> work since Sling unwrapps all request back to a internal SlingHttpServletRequestImpl object...
>>
>> I'm hitting the same problem in my work on SLING-550, created
>> https://issues.apache.org/jira/browse/SLING-1596 with a suggested
>> patch, also at http://codereview.appspot.com/1783044
>>
>> My solution is not perfect, to be clean I think we'd need to make
>> RequestData a first-class interface instead of an implementation
>> class, but my patch already decreases coupling a bit.
>>
>> Reviews welcome.
>
>
> I have a horrible feeling that if the unwrapped request is not a SlingHttpServletRequestImpl then other parts of the engine misbehave, which may have been the reason for enforcing that. It also allows anything to create a new Request, including scripted components. That may not be desirable.
>
As already noted I'm not happy with this - this is internal stuff and we
should rather leave it the way it is.
Why not creating plain http servlet request objects and then going
through the sling main servlet?
Carsten
--
Carsten Ziegeler
cziegeler@apache.org
Re: Request Dispatching.
Posted by Ian Boston <ie...@tfd.co.uk>.
On 15 Jul 2010, at 08:44, Bertrand Delacretaz wrote:
> On Wed, Jul 14, 2010 at 5:51 PM, Ian Boston <ie...@tfd.co.uk> wrote:
>> ...also, did you mean to use tabs, afaik the rest of the code is space based and it can make
>> life hard if the 2 are mixed ?...
>
> Sure, sorry about that, seems like my Eclipse settings got messed up.
> -Bertrand
no problem,
mine too, I upgraded to Helios and lost all the settings,
If you see a commit without a license header (or the wrong one) from me please shout at me.
Ian
Re: Request Dispatching.
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Wed, Jul 14, 2010 at 5:51 PM, Ian Boston <ie...@tfd.co.uk> wrote:
> ...also, did you mean to use tabs, afaik the rest of the code is space based and it can make
> life hard if the 2 are mixed ?...
Sure, sorry about that, seems like my Eclipse settings got messed up.
-Bertrand
Re: Request Dispatching.
Posted by Ian Boston <ie...@tfd.co.uk>.
On 14 Jul 2010, at 16:10, Bertrand Delacretaz wrote:
> On Fri, Jan 8, 2010 at 5:31 PM, Ian Boston <ie...@tfd.co.uk> wrote:
>> ...We have been trying to implement a Batch method processor (ie a servlet that performs multiple requests batching
>> the responses up into a single request/response) in Sling.
>>
>> Normally with servlets this would require requestDispacher.forward(wrappedRequest, wrappedResponse) but this doent
>> work since Sling unwrapps all request back to a internal SlingHttpServletRequestImpl object...
>
> I'm hitting the same problem in my work on SLING-550, created
> https://issues.apache.org/jira/browse/SLING-1596 with a suggested
> patch, also at http://codereview.appspot.com/1783044
>
> My solution is not perfect, to be clean I think we'd need to make
> RequestData a first-class interface instead of an implementation
> class, but my patch already decreases coupling a bit.
>
> Reviews welcome.
I have a horrible feeling that if the unwrapped request is not a SlingHttpServletRequestImpl then other parts of the engine misbehave, which may have been the reason for enforcing that. It also allows anything to create a new Request, including scripted components. That may not be desirable.
However, if thats ok, then the patch looks good to me.
I think we abandoned dynamic re-dispatch in favour of static binding to paths or explicit service calls.
also, did you mean to use tabs, afaik the rest of the code is space based and it can make life hard if the 2 are mixed ?
Ian
> -Bertrand