[RT] SlingHttpServletRequest.externalize() methods

[Alexander Klimetschek <ak...@day.com>]

Hi all,

I'd like to have a method on the SlingHttpServletRequest that
externalizes URLs, ie. just as map() in the resource resolver it
converts an internal resource path into a fully valid URL for access
from the outside:

relative: /my/sub/path => /contextpath/my/sub/path
absolute: /my/sub/path => http://server.com/contextpath/my/sub/path

For URLs relative to the current request, this is already possible by
using one of the resource resolver map() methods:

slingRequest.getResourceResolver().map(slingRequest, "/my/path"))

...which is a bit clunky for placing it into JSPs.

For absolute URLs, that must include a host/port, using map() is not
enough. If no host -> path mapping is configured (eg. myserver.80/ ->
/ in /etc/map), you'd typically like to use the current request as the
default, which isn't done by any of the map() methods.

Also, if a hostname is configured and you use the map(request, path),
because that is required for adding the context path, it will _strip_
it from the map call if the current request goes to that hostname,
which is probably the normal case (this is correct, as the
map(request, path) call is aimed at relative URLs).

Therefore additional methods on the request object make sense, I think.

See https://issues.apache.org/jira/browse/SLING-1629 for a patch.

WDYT?

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: [RT] SlingHttpServletRequest.externalize() methods

[Bertrand Delacretaz <bd...@apache.org>]

On Fri, Aug 6, 2010 at 10:24 AM, Carsten Ziegeler <cz...@apache.org> wrote:
> ...there are always a lot of useful functions one needs here or
> there - adding all of them to the request interface just because it is a
> little bit less to type does not sound like the best argument to me.
> That's why I would put them in a helper class first - if they are used
> by each and everyone in every script, then this might be a candidate to
> add it to a more prominent place...

+1

-Bertrand

Re: [RT] SlingHttpServletRequest.externalize() methods

[Alexander Klimetschek <ak...@day.com>]

On Fri, Aug 6, 2010 at 12:07, Carsten Ziegeler <cz...@apache.org> wrote:
> Alexander Klimetschek  wrote
>
>> Another reason for the prominent place is that it teaches people to do
>> the right thing more easily. I have seen many cases that create links
>> without considering context path or sling's mapping.
>>
> Hmm, but if a developer is not able to call map on the resource resolver
> why should he now call a method on the request (or response) ?
> Maybe the map method is not very prominently placed, but I would rather
> do an example showing people how to do it, instead of adding all kind of
> stuff to the request interface.
> Again, maybe this stuff is very useful for everyone and it makes sense
> to add it to a more prominent place. But atm I doubt this.

Maybe you are right.

Actually I now think that the ResourceResolver would be a better
location, to be able to generate absolute URLs (ie. including host and
context path) outside of the request scope. That would require some
additional configuration in the resolver or under /etc/map. Currently
I am doing that in a custom service.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: [RT] SlingHttpServletRequest.externalize() methods

[Alexander Klimetschek <ak...@day.com>]

On Fri, Aug 6, 2010 at 14:25, Felix Meschberger <fm...@gmail.com> wrote:
> Excellent point: Have a look at Sling's
> HttpServletResponse.encodeURL(String) method [1].
>
> And yes, this is a method which has been there for a long time, but
> developers don't seem to be used to use it ;-)

I know it, but it doesn't fit at all: encodeURL() doesn't generate
absolute URLs, its main purpose is to add the session ID, which we
don't want in Sling.

My idea was to have a reliable single method for generating absolute
URLs, and for the sake of consistency and simplicity for the client,
have a similar one (or decide via a flag) for generating relative URLs
(what ResourceResolver.map(request, path) is doing).

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: [RT] SlingHttpServletRequest.externalize() methods

[Felix Meschberger <fm...@gmail.com>]

Hi,

On 06.08.2010 12:07, Carsten Ziegeler wrote:
> Alexander Klimetschek  wrote
> 
>> Another reason for the prominent place is that it teaches people to do
>> the right thing more easily. I have seen many cases that create links
>> without considering context path or sling's mapping.
>>
> Hmm, but if a developer is not able to call map on the resource resolver
> why should he now call a method on the request (or response) ?

Excellent point: Have a look at Sling's
HttpServletResponse.encodeURL(String) method [1].

And yes, this is a method which has been there for a long time, but
developers don't seem to be used to use it ;-)

So -1 to adding such a method.

Regards
Felix

[1]
http://svn.apache.org/repos/asf/sling/trunk/bundles/engine/src/main/java/org/apache/sling/engine/impl/SlingHttpServletResponseImpl.java

> Maybe the map method is not very prominently placed, but I would rather
> do an example showing people how to do it, instead of adding all kind of
> stuff to the request interface.
> Again, maybe this stuff is very useful for everyone and it makes sense
> to add it to a more prominent place. But atm I doubt this.
> 
> Carsten

Re: [RT] SlingHttpServletRequest.externalize() methods

[Carsten Ziegeler <cz...@apache.org>]

Alexander Klimetschek  wrote

> Another reason for the prominent place is that it teaches people to do
> the right thing more easily. I have seen many cases that create links
> without considering context path or sling's mapping.
> 
Hmm, but if a developer is not able to call map on the resource resolver
why should he now call a method on the request (or response) ?
Maybe the map method is not very prominently placed, but I would rather
do an example showing people how to do it, instead of adding all kind of
stuff to the request interface.
Again, maybe this stuff is very useful for everyone and it makes sense
to add it to a more prominent place. But atm I doubt this.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] SlingHttpServletRequest.externalize() methods

[Alexander Klimetschek <ak...@day.com>]

On Fri, Aug 6, 2010 at 10:24, Carsten Ziegeler <cz...@apache.org> wrote:
> Alexander Klimetschek  wrote
>> On Fri, Aug 6, 2010 at 08:10, Carsten Ziegeler <cz...@apache.org> wrote:
>>> What about a helper class (static methods) or a jsp taglib?
>>
>> I was preferring them on the request, as you then don't need to pass
>> the request and things get shorter. A jsp taglib can also only be used
>> inside a jsp.
>>
> Sure but you were talking about jsps in your proposal :)

And servlets.

> Anyway, there are always a lot of useful functions one needs here or
> there - adding all of them to the request interface just because it is a
> little bit less to type does not sound like the best argument to me.
> That's why I would put them in a helper class first - if they are used
> by each and everyone in every script, then this might be a candidate to
> add it to a more prominent place.

Another reason for the prominent place is that it teaches people to do
the right thing more easily. I have seen many cases that create links
without considering context path or sling's mapping.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: [RT] SlingHttpServletRequest.externalize() methods

[Carsten Ziegeler <cz...@apache.org>]

Alexander Klimetschek  wrote
> On Fri, Aug 6, 2010 at 08:10, Carsten Ziegeler <cz...@apache.org> wrote:
>> What about a helper class (static methods) or a jsp taglib?
> 
> I was preferring them on the request, as you then don't need to pass
> the request and things get shorter. A jsp taglib can also only be used
> inside a jsp.
> 
Sure but you were talking about jsps in your proposal :)
Anyway, there are always a lot of useful functions one needs here or
there - adding all of them to the request interface just because it is a
little bit less to type does not sound like the best argument to me.
That's why I would put them in a helper class first - if they are used
by each and everyone in every script, then this might be a candidate to
add it to a more prominent place.

Carsten
-- 
Carsten Ziegeler
cziegeler@apache.org

Re: [RT] SlingHttpServletRequest.externalize() methods

[Alexander Klimetschek <ak...@day.com>]

On Fri, Aug 6, 2010 at 08:10, Carsten Ziegeler <cz...@apache.org> wrote:
> What about a helper class (static methods) or a jsp taglib?

I was preferring them on the request, as you then don't need to pass
the request and things get shorter. A jsp taglib can also only be used
inside a jsp.

Regards,
Alex

-- 
Alexander Klimetschek
alexander.klimetschek@day.com

Re: [RT] SlingHttpServletRequest.externalize() methods

[Carsten Ziegeler <cz...@apache.org>]

What about a helper class (static methods) or a jsp taglib?

Regards
Carsten

Alexander Klimetschek  wrote
> Hi all,
> 
> I'd like to have a method on the SlingHttpServletRequest that
> externalizes URLs, ie. just as map() in the resource resolver it
> converts an internal resource path into a fully valid URL for access
> from the outside:
> 
> relative: /my/sub/path => /contextpath/my/sub/path
> absolute: /my/sub/path => http://server.com/contextpath/my/sub/path
> 
> For URLs relative to the current request, this is already possible by
> using one of the resource resolver map() methods:
> 
> slingRequest.getResourceResolver().map(slingRequest, "/my/path"))
> 
> ...which is a bit clunky for placing it into JSPs.
> 
> For absolute URLs, that must include a host/port, using map() is not
> enough. If no host -> path mapping is configured (eg. myserver.80/ ->
> / in /etc/map), you'd typically like to use the current request as the
> default, which isn't done by any of the map() methods.
> 
> Also, if a hostname is configured and you use the map(request, path),
> because that is required for adding the context path, it will _strip_
> it from the map call if the current request goes to that hostname,
> which is probably the normal case (this is correct, as the
> map(request, path) call is aimed at relative URLs).
> 
> Therefore additional methods on the request object make sense, I think.
> 
> See https://issues.apache.org/jira/browse/SLING-1629 for a patch.
> 
> WDYT?
> 
> Regards,
> Alex
> 


-- 
Carsten Ziegeler
cziegeler@apache.org