You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Konrad Windszus <ko...@gmx.de> on 2020/01/16 14:00:54 UTC

Re: [hackathon] externalization / resource mapping / rewriter

I would like to revive this. 

@Georg: IIRC you wanted to come up with a proposal for a externalizer API. Are you working on this?
Can we start by creating a JIRA ticket?

There has been a related ticket opened today: https://issues.apache.org/jira/browse/SLING-9005 <https://issues.apache.org/jira/browse/SLING-9005>

Konrad

> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org> wrote:
> 
> Specifically with regard to the re-writer
> 
> I'm working on a replacement for this which I'm currently calling the transformer -> https://github.com/apache/sling-whiteboard/tree/master/transformer
> 
> 1. It uses an HTML tokenizer that was added to the HTML utilities to generate a stream of  elements that can be modified and recombined. It's much faster then tagsoup or jsoup since it's not trying to build a valid document. This also means that it supports anything that you write out in html. HTML4,5+
> 
> 2. It uses services with an ordering priority and does pattern matching so that you can fine tune when the transformer is applied
> 
> 3. The specific use case I was working on is creating a CSP header with a nonce or hash attribute to lock down the javascript that's on the page. 
> 
> It's currently working but it's not finished.
> Are there other problems with the rewriter that I haven't addressed?
> 
> 
> --
> Jason
> 
> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>> 
>> - resource mapping
>>  - add a new SPI to define the mapping
>>  - add a default impl that reads the mapping from /etc/map as it is 
>> done today
>>  - possible to override with service ranking
>>  - but: there is already the ResourceMapper interface
>>    - introduced 1-2 years ago, use case was to get all existing 
>> mappings
>>    - with this it's currently possible to replace mapping completely 
>> with new logic
>>  - maybe add a support to "contribute" additional mappings via a 
>> service interface additional to this
>> 
>> - generic externalizer API
>>  - naming not fixed yet, should not named "link" as this is too 
>> specific. probably "URL".
>>  - needs a java API for using, and an SPI for hooking in special rules
>>  - then add mappings for views in HTL*, JSP, model exporter/JSON
>>    * probably starting with a sling-only HTL extension for try-out, 
>> add it later to the spec when design is validated
>>  - might be inspired by the basic features for wcm.io URL handler [1]
>> 
>> - rewriter pipeline
>>  - should be deprecated and no longer be used - especially not for 
>> link externalization via postprocessing
>>  - it may still be in use out there for more exotic use cases like PDF 
>> generation
>>  - should probably removed from sling starter
>> 
>> stefan
>> 
>> [1] https://wcm.io/handler/url/
>> 
>> 
>> 


Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Ruben,

so in general nothing would change about how to use rr.map() from a
consumer side (if that still happens via the rewriter pipeline by
e.g. "a link checker" as provided downstream, that's totally fine).

Now for people that want to replace the rewriter pipeline, we can just
extend the handling of the display context "html" in HTL - that is
used for XSS protection today already, it can as well call rr.map()
on links (rich text html fragments are parsed and stream twice
today)

-Georg


On 2020-03-30 04:17, rr@headwire.com wrote:
> Georg,
> 
> one of the things that have to be considered here as well are links in
> a href tags in text properties (eg from a rich text editor) in sling
> model rendered output and in html rendered output
> 
> Ruben
> 
> -----Original Message-----
> From: Georg Henzler <gh...@apache.org>
> Sent: Sunday, March 29, 2020 5:13 PM
> To: dev@sling.apache.org
> Subject: [DISCUSS] Resource Mapping SPI
> 
> Hi all,
> 
> so to eventually follow up on what has been discussed at last year's
> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
> found the time to draft the proposal for a Resource Mapping SPI.
> 
> Two important points before we start:
> * This is not meant to replace the Rewriting Pipelines as they exist
>    today - it just would allow some Sling setups (maybe > 70%?) to move
>    to a simpler solution avoiding the overhead of the rewriter pipeline
>    while obviously losing its flexibility, but for many that will
>    be ok (I fully agree with Bertrand [3] here)
> * Let's focus the discussion first on if we would like to go into this
>    direction instead of details (like mutable vs. immutable or naming,
>    we can have detailed discussions around it once we decide to move 
> on)
> 
> But now to the actual proposal to introduce two new interfaces to the 
> Sling API:
> 
> ResourceURI:
> 
> On Sling API level we treat links as strings (e.g. rr.map() returns a
> String). Using Strings for links has produced many bugs in the past, I
> think having an abstraction for a URI/link that is tailored for Sling
> would help a lot (obviously plain JDK URI can be used, but there is no
> support for selectors, suffix etc.).
> 
> Besides being generally a good idea IMHO, ResourceURI shall be used
> for the other interface:
> 
> ResourceMapper:
> 
> Allows to hook into the rr.resolve() and rr.map() by implementing
> exactly those two methods that work on instances of ResourceURI. The
> idea is that n OSGi service instances of ResourceMapper build a
> pipeline of mappers that rr would run through when rr.resolve() or
> rr.map() is called (in the opposite direction for map). The request is
> passed in as context to both methods but may be null as it is today,
> implementations of ResourceMapper need to be able to handle null.
> 
> The following mechanisms exist today for resource mapping in ootb
> Sling:
> 
> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
> * Mappings via /etc/map
> * Vanity Paths
> * sling:alias
> 
> Those could be broken up into four implementations of ResourceMapper
> that are configured by default.
> 
> About backwards-compatibility/breaking existing implementations: So
> this is a BIG CHANGE. However to keep it safe I would start with
> exactly one ResourceMapper active that is backed by today's
> implementation. The next step is to break it in separate resource
> mappers (as mentioned above) and just put them in a row.
> 
> The Resource Mapping SPI would bring the following benefits:
> 
> * Being able to hook into the resource mapping via SPI allows
>    for flexibility that is otherwise only possible Rewriting
>    Pipelines - while link rewriting/checking is the only
>    reason most projects use Rewriting Pipelines (with all its
>    performance overhead)
> 
> * Extending the mapping process via a well-defined API that
>    allows to write Touring-complete code is better than working
>    with Strings in the Transformer (also for many cases it can
>    be cleaner and better 'unit-testable' than /etc/maps)
> 
> * HTL there can be an option to automatically map all
>    attributes of context uri (better performance, better for
>    debugging if map() is called synchronously)
> 
> * Introducing the interface ResourceURI (with a backing helper
>    class for everybody to use) is useful in general
> 
> See branch [4] for some first code of the proposal, in particular
> ResourceMapping.java [5] and ResourceURI.java [6]
> 
> -Georg
> 
> [1] "[hackathon] externalization / resource mapping / rewriter"
> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
> [2] "Why get rid of the rewriter?"
> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
> 
> [4]
> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
> [5]
> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
> [6]
> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
> 
> 
> 
> On 2020-01-17 10:45, Georg Henzler wrote:
>> Hi Konrad,
>> 
>> I think SLING-9005 is a good idea, but that is independent.
>> 
>> My idea idea was to leave resourceResolver.map() unchanged (as there
>> is a lot of code using it out there, also it makes conceptually sense
>> as is). Rather I‘d like to provide an SPI that allows to customize the
>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>> ranked list of ResourceMapper services (default one being today’s
>> behavior). Robert also has done some work into this direction already,
>> I‘d like to extend on that.
>> 
>> I‘m currently on vacation but I have it on my list for when I‘m back.
>> 
>> -Georg
>> 
>>> 
>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>>> 
>>> I would like to revive this.
>>> 
>>> @Georg: IIRC you wanted to come up with a proposal for a externalizer
>>> API. Are you working on this?
>>> Can we start by creating a JIRA ticket?
>>> 
>>> There has been a related ticket opened today:
>>> https://issues.apache.org/jira/browse/SLING-9005
>>> <https://issues.apache.org/jira/browse/SLING-9005>
>>> 
>>> Konrad
>>> 
>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
>>>> wrote:
>>>> 
>>>> Specifically with regard to the re-writer
>>>> 
>>>> I'm working on a replacement for this which I'm currently calling
>>>> the transformer ->
>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>>> 
>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities to
>>>> generate a stream of  elements that can be modified and recombined.
>>>> It's much faster then tagsoup or jsoup since it's not trying to
>>>> build a valid document. This also means that it supports anything
>>>> that you write out in html. HTML4,5+
>>>> 
>>>> 2. It uses services with an ordering priority and does pattern
>>>> matching so that you can fine tune when the transformer is applied
>>>> 
>>>> 3. The specific use case I was working on is creating a CSP header
>>>> with a nonce or hash attribute to lock down the javascript that's on
>>>> the page.
>>>> 
>>>> It's currently working but it's not finished.
>>>> Are there other problems with the rewriter that I haven't addressed?
>>>> 
>>>> 
>>>> --
>>>> Jason
>>>> 
>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>>> 
>>>>> - resource mapping
>>>>> - add a new SPI to define the mapping
>>>>> - add a default impl that reads the mapping from /etc/map as it is
>>>>> done today
>>>>> - possible to override with service ranking
>>>>> - but: there is already the ResourceMapper interface
>>>>>   - introduced 1-2 years ago, use case was to get all existing
>>>>> mappings
>>>>>   - with this it's currently possible to replace mapping completely
>>>>> with new logic
>>>>> - maybe add a support to "contribute" additional mappings via a
>>>>> service interface additional to this
>>>>> 
>>>>> - generic externalizer API
>>>>> - naming not fixed yet, should not named "link" as this is too
>>>>> specific. probably "URL".
>>>>> - needs a java API for using, and an SPI for hooking in special
>>>>> rules
>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>>>   * probably starting with a sling-only HTL extension for try-out,
>>>>> add it later to the spec when design is validated
>>>>> - might be inspired by the basic features for wcm.io URL handler
>>>>> [1]
>>>>> 
>>>>> - rewriter pipeline
>>>>> - should be deprecated and no longer be used - especially not for
>>>>> link externalization via postprocessing
>>>>> - it may still be in use out there for more exotic use cases like
>>>>> PDF generation
>>>>> - should probably removed from sling starter
>>>>> 
>>>>> stefan
>>>>> 
>>>>> [1] https://wcm.io/handler/url/
>>>>> 
>>>>> 
>>>>> 
>>> 

RE: [DISCUSS] Resource Mapping SPI

Posted by rr...@headwire.com.
Georg, 

one of the things that have to be considered here as well are links in a href tags in text properties (eg from a rich text editor) in sling model rendered output and in html rendered output

Ruben

-----Original Message-----
From: Georg Henzler <gh...@apache.org> 
Sent: Sunday, March 29, 2020 5:13 PM
To: dev@sling.apache.org
Subject: [DISCUSS] Resource Mapping SPI

Hi all,

so to eventually follow up on what has been discussed at last year's adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now found the time to draft the proposal for a Resource Mapping SPI.

Two important points before we start:
* This is not meant to replace the Rewriting Pipelines as they exist
   today - it just would allow some Sling setups (maybe > 70%?) to move
   to a simpler solution avoiding the overhead of the rewriter pipeline
   while obviously losing its flexibility, but for many that will
   be ok (I fully agree with Bertrand [3] here)
* Let's focus the discussion first on if we would like to go into this
   direction instead of details (like mutable vs. immutable or naming,
   we can have detailed discussions around it once we decide to move on)

But now to the actual proposal to introduce two new interfaces to the Sling API:

ResourceURI:

On Sling API level we treat links as strings (e.g. rr.map() returns a String). Using Strings for links has produced many bugs in the past, I think having an abstraction for a URI/link that is tailored for Sling would help a lot (obviously plain JDK URI can be used, but there is no support for selectors, suffix etc.).

Besides being generally a good idea IMHO, ResourceURI shall be used for the other interface:

ResourceMapper:

Allows to hook into the rr.resolve() and rr.map() by implementing exactly those two methods that work on instances of ResourceURI. The idea is that n OSGi service instances of ResourceMapper build a pipeline of mappers that rr would run through when rr.resolve() or
rr.map() is called (in the opposite direction for map). The request is passed in as context to both methods but may be null as it is today, implementations of ResourceMapper need to be able to handle null.

The following mechanisms exist today for resource mapping in ootb
Sling:

* Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
* Mappings via /etc/map
* Vanity Paths
* sling:alias

Those could be broken up into four implementations of ResourceMapper that are configured by default.

About backwards-compatibility/breaking existing implementations: So this is a BIG CHANGE. However to keep it safe I would start with exactly one ResourceMapper active that is backed by today's implementation. The next step is to break it in separate resource mappers (as mentioned above) and just put them in a row.

The Resource Mapping SPI would bring the following benefits:

* Being able to hook into the resource mapping via SPI allows
   for flexibility that is otherwise only possible Rewriting
   Pipelines - while link rewriting/checking is the only
   reason most projects use Rewriting Pipelines (with all its
   performance overhead)

* Extending the mapping process via a well-defined API that
   allows to write Touring-complete code is better than working
   with Strings in the Transformer (also for many cases it can
   be cleaner and better 'unit-testable' than /etc/maps)

* HTL there can be an option to automatically map all
   attributes of context uri (better performance, better for
   debugging if map() is called synchronously)

* Introducing the interface ResourceURI (with a backing helper
   class for everybody to use) is useful in general

See branch [4] for some first code of the proposal, in particular ResourceMapping.java [5] and ResourceURI.java [6]

-Georg

[1] "[hackathon] externalization / resource mapping / rewriter" 
https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
[2] "Why get rid of the rewriter?" 
https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
[3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html

[4]
https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
[5]
https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
[6]
https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java



On 2020-01-17 10:45, Georg Henzler wrote:
> Hi Konrad,
> 
> I think SLING-9005 is a good idea, but that is independent.
> 
> My idea idea was to leave resourceResolver.map() unchanged (as there 
> is a lot of code using it out there, also it makes conceptually sense 
> as is). Rather I‘d like to provide an SPI that allows to customize the 
> way of the behavior of resourceResolver.map() - I‘m thinking of a 
> ranked list of ResourceMapper services (default one being today’s 
> behavior). Robert also has done some work into this direction already, 
> I‘d like to extend on that.
> 
> I‘m currently on vacation but I have it on my list for when I‘m back.
> 
> -Georg
> 
>> 
>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>> 
>> I would like to revive this.
>> 
>> @Georg: IIRC you wanted to come up with a proposal for a externalizer 
>> API. Are you working on this?
>> Can we start by creating a JIRA ticket?
>> 
>> There has been a related ticket opened today: 
>> https://issues.apache.org/jira/browse/SLING-9005
>> <https://issues.apache.org/jira/browse/SLING-9005>
>> 
>> Konrad
>> 
>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
>>> wrote:
>>> 
>>> Specifically with regard to the re-writer
>>> 
>>> I'm working on a replacement for this which I'm currently calling 
>>> the transformer -> 
>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>> 
>>> 1. It uses an HTML tokenizer that was added to the HTML utilities to 
>>> generate a stream of  elements that can be modified and recombined.
>>> It's much faster then tagsoup or jsoup since it's not trying to 
>>> build a valid document. This also means that it supports anything 
>>> that you write out in html. HTML4,5+
>>> 
>>> 2. It uses services with an ordering priority and does pattern 
>>> matching so that you can fine tune when the transformer is applied
>>> 
>>> 3. The specific use case I was working on is creating a CSP header 
>>> with a nonce or hash attribute to lock down the javascript that's on 
>>> the page.
>>> 
>>> It's currently working but it's not finished.
>>> Are there other problems with the rewriter that I haven't addressed?
>>> 
>>> 
>>> --
>>> Jason
>>> 
>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>> 
>>>> - resource mapping
>>>> - add a new SPI to define the mapping
>>>> - add a default impl that reads the mapping from /etc/map as it is 
>>>> done today
>>>> - possible to override with service ranking
>>>> - but: there is already the ResourceMapper interface
>>>>   - introduced 1-2 years ago, use case was to get all existing 
>>>> mappings
>>>>   - with this it's currently possible to replace mapping completely 
>>>> with new logic
>>>> - maybe add a support to "contribute" additional mappings via a 
>>>> service interface additional to this
>>>> 
>>>> - generic externalizer API
>>>> - naming not fixed yet, should not named "link" as this is too 
>>>> specific. probably "URL".
>>>> - needs a java API for using, and an SPI for hooking in special 
>>>> rules
>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>>   * probably starting with a sling-only HTL extension for try-out, 
>>>> add it later to the spec when design is validated
>>>> - might be inspired by the basic features for wcm.io URL handler 
>>>> [1]
>>>> 
>>>> - rewriter pipeline
>>>> - should be deprecated and no longer be used - especially not for 
>>>> link externalization via postprocessing
>>>> - it may still be in use out there for more exotic use cases like 
>>>> PDF generation
>>>> - should probably removed from sling starter
>>>> 
>>>> stefan
>>>> 
>>>> [1] https://wcm.io/handler/url/
>>>> 
>>>> 
>>>> 
>> 


Re: [DISCUSS] Resource Mapping SPI

Posted by Ruben Reusser <rr...@headwire.com>.
Georg, thanks for the reply - agree my use cases should be covered with that

On 8/24/2020 7:42 AM, Georg Henzler wrote:
> Hi Ruben,
>
>> One use case that probably also should be considered here is the case
>> where static pages are generated with sling. In that case the
>> rendering of the page is not tied to a request.
>
> While [1] has a request in the signature, the idea is not that this
> needs to be "a currently active" request. It can be
>
>

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Ruben,

> One use case that probably also should be considered here is the case
> where static pages are generated with sling. In that case the
> rendering of the page is not tied to a request.

While [1] has a request in the signature, the idea is not that this
needs to be "a currently active" request. It can be

1. null for default behaviour
2. the current request (if mapping should be aligned with current 
request)
3. an example request (or maybe better "reference request") that holds
the same properties (server name, headers, etc.) as a request that will
be issued for the generated URI in the future. The expectation is that
resolve() of the same ResourceToUriMapper service will take care of such
a mapped URI.

The last option (or maybe the first) covers the use case for generating
static pages.

-Georg

[1] 
https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L51

> Ruben
> 
> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Ruben Reusser <rr...@headwire.com>.
looking forward to the new resource mapping strategy!

One use case that probably also should be considered here is the case 
where static pages are generated with sling. In that case the rendering 
of the page is not tied to a request.

Ruben

On 8/23/2020 2:24 AM, Carsten Ziegeler wrote:
> Thanks for taking this up.
>
> As noted in the issue, I think we should investigate whether this is a 
> good opportunity to rethink mapping / resolving in general. In 
> hindsight, we probably shouldn't have added the mapping part to the 
> resource resolver. The resolve() method does two things: it applies 
> mappings (which is independent of the current user) and it resolves to 
> a resource (finding the resource and figuring out the selectors, 
> extensions etc.) based on the user.
>
> So how about moving the mapping part out of the resource resolver? As 
> we've seen recently other parts of Sling like auth core could benefit 
> from this as well.
>
> I haven't fully thought this through, but this is the rough idea:
>
> We create a new OSGi service, PathMapperFactory (its probably not a 
> good name but I don't want to invest time into naming at this stage). 
> It has one method:
>
>    PathMapper newPathMapper(HttpServletRequest request)
>
> where the request parameter is optional. PathMapper has the 
> resolve/map methods doing the mapping (based on the newly suggested 
> SPI). If a request has been provided to the factory method, that 
> object is used for resolving/mapping.
> I'm unsure about the exact methods of PathMapper, but it needs a 
> resolve method taking in a path (the request path) and returning the 
> resolved path (vanity path, aliases etc are applied) and a flag 
> whether a redirect should happen.
>
> Sling's main servlet (or more precise the processor) is changed: for 
> an incoming request it calls the PathMapperFactory with the request 
> and then the resolve method on PathMapper. If the result is absolut or 
> the redirect flag is set, the redirect happens.
>
> Otherwise, authentication is invoked with the resolved path - this 
> frees auth core (and all auth handlers etc) from dealing with mappings.
>
> If authentication is successful, a new resource resolver is created 
> and the PathMapper is passed to the resource resolver via the 
> attributes of the resource resolver factory. ResourceResolver gets a 
> new method "getPathMapper()" - if no PathMapper is passed to the 
> resource resolver factory, the factory creates a new one by invoking 
> newPathMapper(null) on the PathMapperFactory. In all cases, a resource 
> resolver has a PathMapper.
>
> We add a new method to Resource Resolver, similar to the resolve() 
> method but just doing the resolution part - no mapping. This method is 
> used by the main servlet instead of the existing resolve() method. We 
> deprecate resolve() and map() and change the implementation to 
> internally use the passed in PathMapper instance.
>
> This way we have a clear separation between mapping and resolving and 
> in which contexts the operations happen. With adding the PathMapper to 
> the resource resolver we make it available to all parts in the system 
> which need to call todays map() method - and as the PathMapper 
> instance knows the request, there is no need to pass it around anymore.
>
> Now, again, just a rough write down of the idea, don't get hung up on 
> the naming that can and needs to improve. Its also not directly about 
> the new SPI, but rather how we make the SPI available to the rest of 
> the system.
>
> Regards
> Carsten
>
> Am 14.08.2020 um 07:57 schrieb Georg Henzler:
>> Hi all,
>>
>> so I eventually got around to continue on this. I created JIRA
>> issue [1] and the branches [2] in API and [3] in resourceresolver
>> with a first working version.
>>
>>
>>> I definitely prefer immutable classes with a dedicated builder
>>
>> @Konrad I was critical first because the verbosity of the code,
>> but it made implementing ResourceUriMappingChain actually easier.
>> My concern about the verbosity was not that much a problem with
>> Java 8 anymore.
>>
>>> ...And I do believe that mapping is
>>> about the relationship between requests and resources ...
>> and
>>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>>> Resource to a form that can be requested.
>>
>> @Julian: So these are very good points, I think RequestMapper is
>> not ideal because in async code (e.g. Sling jobs), you might
>> have the case that you have a resource and you map it to a URI
>> (e.g. for sending an email). in rr.map() you use no
>> request and usually some manual work is needed to add the host.
>> My idea for the future would be to provide a simple
>> ExampleRequest that can be used to provide a hostname or headers
>> which will then tell the active mappers to do the right thing for
>> the async, non-request use case. I ended up calling the
>> SPI interface itself ResourceToUriMapper (which I think is
>> best spot on for what it does).
>>
>> So overall I have it all working now in the branches [2] and [3],
>> currently ResourceUriMappingChain is called from
>> ResourceResolverImpl.resolveInternal() and
>> ResourceMapperImpl.getAllMappings(). Other aspects like Sling
>> alias/vanity path handling, namespace mangling and context
>> paths are still handled there but could potentially be moved
>> to a ResourceToUriMapper in further steps. Also see [4]
>> for some examples on what ResourceToUriMapper can look like.
>>
>> Let me know what you think!
>>
>> -Georg
>>
>>
>> [1] https://issues.apache.org/jira/browse/SLING-9662
>> [2] 
>> https://github.com/apache/sling-org-apache-sling-api/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI 
>>
>> [3] 
>> https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI 
>>
>> [4] 
>> https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI/src/main/java/org/apache/sling/resourceresolver/impl/mappingchain/defaultmappers 
>>
>>
>> On 2020-04-01 10:04, Georg Henzler wrote:
>>> Hi Julian,
>>>
>>>> I'm all for disentangling the mapping implementations (/etc/map,
>>>> vanity, alias etc.) and making the mapping pluggable.
>>>
>>> great - this is really the main purpose of this thread, to sense
>>> if our community will support the move (because it'll be a bit of
>>> work and for that I'd like to have an agreement on the direction
>>> first)
>>>
>>>> Despite your suggestion to discuss naming later, Georg, I believe that
>>>> naming helps structure thoughts.
>>>
>>> yes, we shouldn't be too strict :)
>>>
>>>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>>>> Resource to a form that can be requested.
>>>
>>> or maybe UriMapper - while many use cases have a request at hand, there
>>> is always the default mapping that kicks in when null is passed as
>>> request (mostly relevant for for async processes like e.g. sling jobs)
>>>
>>>> I would rather give the Sling instance an identity (e.g.
>>>> protocol, hostname and port)
>>>
>>> This is an interesting idea! It wouldn't have to be protocol, hostname
>>> and port, just the protocol would make it a valid uri already, the
>>> internal representation could be slinguri:/path/to/resource.sel.html,
>>> ResourceUri.toString() could just return /path/to/resource.sel.html.
>>>
>>> -Georg
>>>
>>> On 2020-04-01 00:28, Julian Sedding wrote:
>>>> Hi
>>>>
>>>> I'm all for disentangling the mapping implementations (/etc/map,
>>>> vanity, alias etc.) and making the mapping pluggable.
>>>>
>>>> Over the years I have come to think that the ResourceResolver methods
>>>> "resolve" and "map" are actually in the "wrong" interface, because the
>>>> handle a different concern than the rest of the RR. Both "resolve" and
>>>> "map" are concerned about the HttpServletRequest, which is none of the
>>>> RR's business. We cannot fix this due to backwards compatibility, but
>>>> maybe we can improve the current situation.
>>>>
>>>> Despite your suggestion to discuss naming later, Georg, I believe that
>>>> naming helps structure thoughts. And I do believe that mapping is
>>>> about the relationship between requests and resources. Pretty much
>>>> everything in Sling is about Resources. Therefore, I suggest
>>>> reflecting the request aspect in the naming. This does not
>>>> fundamentally change what you are suggesting, and I fully agree on the
>>>> ordered chaining of mappers.
>>>>
>>>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>>>> Resource to a form that can be requested.
>>>> - RequestUri: an abstraction that can be serialized to the string
>>>> representation of a URI
>>>>
>>>> I'm not sure we need an external URI and a local one variant of
>>>> RequestUri. I would rather give the Sling instance an identity (e.g.
>>>> protocol, hostname and port) and use that in conjunction with the
>>>> RequestUri in order to decide whether the string representation should
>>>> be absolute or relative (i.e. if protocol, hostname and port match, it
>>>> can be relative). Or maybe that identity is per HTTP request, I'm not
>>>> sure yet.
>>>>
>>>> Just some (yet) unstructured thoughts. I hope I'm making sense.
>>>>
>>>> Regards
>>>> Julian
>>>>
>>>>
>>>> On Tue, Mar 31, 2020 at 11:50 PM Georg Henzler 
>>>> <gh...@apache.org> wrote:
>>>>>
>>>>> Hi Konrad,
>>>>>
>>>>> yes, ResourceURI at the moment would be both for internal and 
>>>>> external
>>>>> links.
>>>>>
>>>>> Immutable vs. Mutable is to be discussed (both ways have their own
>>>>> advantages)
>>>>>
>>>>> > ResourceUri resolve(ResourceURI, ...) (not useful for 
>>>>> absolute/external
>>>>> > URIs)
>>>>> > SlingUri map(SlingUri, ...) (can be useful even to map an 
>>>>> external URI)
>>>>>
>>>>> Because it is meant to be a pipeline of OSGi services in a row of 
>>>>> which
>>>>> any
>>>>> could make the decision to switch from a plain path to a full URI 
>>>>> IMHO
>>>>> we need
>>>>> to pass an interface that covers both. This approach could mean 
>>>>> (names
>>>>> are
>>>>> more descriptive than a real suggestion for now):
>>>>>
>>>>> interface ResourceUri extends ArbitraryLink {...}
>>>>> interface ResourcePath extends ArbitraryLink {...} #
>>>>> RequestPathInfo+query+fragment
>>>>>
>>>>> The ResourceMapping interface would then look symmetric again:
>>>>>
>>>>> ArbitraryLink resolve(ArbitraryLink, ...)
>>>>> ArbitraryLink map(ArbitraryLink, ...)
>>>>>
>>>>> -Georg
>>>>>
>>>>> On 2020-03-30 21:17, Konrad Windszus wrote:
>>>>> > Hi Georg,
>>>>> >
>>>>> > Is the class
>>>>> > 
>>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java 
>>>>>
>>>>> > 
>>>>> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java> 
>>>>>
>>>>> > supposed to be used for both internal as well as for external 
>>>>> links?
>>>>> > I would rather come up with two separate classes (they don't 
>>>>> have much
>>>>> > in common).
>>>>> >
>>>>> > What about a generic SlingUri class with the two specializations
>>>>> > ResourceUri and AbsoluteUri (or ExternalUri)?
>>>>> >
>>>>> > That you would lead the following signatures in
>>>>> > 
>>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java 
>>>>>
>>>>> > 
>>>>> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>: 
>>>>>
>>>>> >
>>>>>
>>>>> >
>>>>> > I definitely prefer immutable classes with a dedicated builder but
>>>>> > that is up for another discussion I guess,
>>>>> >
>>>>> > Konrad
>>>>> >
>>>>> >> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> 
>>>>> wrote:
>>>>> >>
>>>>> >> Hi all,
>>>>> >>
>>>>> >> so to eventually follow up on what has been discussed at last 
>>>>> year's
>>>>> >> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - 
>>>>> I now
>>>>> >> found the time to draft the proposal for a Resource Mapping SPI.
>>>>> >>
>>>>> >> Two important points before we start:
>>>>> >> * This is not meant to replace the Rewriting Pipelines as they 
>>>>> exist
>>>>> >>  today - it just would allow some Sling setups (maybe > 70%?) 
>>>>> to move
>>>>> >>  to a simpler solution avoiding the overhead of the rewriter 
>>>>> pipeline
>>>>> >>  while obviously losing its flexibility, but for many that will
>>>>> >>  be ok (I fully agree with Bertrand [3] here)
>>>>> >> * Let's focus the discussion first on if we would like to go 
>>>>> into this
>>>>> >>  direction instead of details (like mutable vs. immutable or 
>>>>> naming,
>>>>> >>  we can have detailed discussions around it once we decide to 
>>>>> move on)
>>>>> >>
>>>>> >> But now to the actual proposal to introduce two new interfaces 
>>>>> to the
>>>>> >> Sling API:
>>>>> >>
>>>>> >> ResourceURI:
>>>>> >>
>>>>> >> On Sling API level we treat links as strings (e.g. rr.map() 
>>>>> returns
>>>>> >> a String). Using Strings for links has produced many bugs in 
>>>>> the past,
>>>>> >> I think having an abstraction for a URI/link that is tailored for
>>>>> >> Sling would help a lot (obviously plain JDK URI can be used, but
>>>>> >> there is no support for selectors, suffix etc.).
>>>>> >>
>>>>> >> Besides being generally a good idea IMHO, ResourceURI shall be 
>>>>> used
>>>>> >> for the other interface:
>>>>> >>
>>>>> >> ResourceMapper:
>>>>> >>
>>>>> >> Allows to hook into the rr.resolve() and rr.map() by implementing
>>>>> >> exactly those two methods that work on instances of 
>>>>> ResourceURI. The
>>>>> >> idea is that n OSGi service instances of ResourceMapper build a
>>>>> >> pipeline of mappers that rr would run through when rr.resolve() or
>>>>> >> rr.map() is called (in the opposite direction for map). The 
>>>>> request
>>>>> >> is passed in as context to both methods but may be null as it is
>>>>> >> today, implementations of ResourceMapper need to be able to handle
>>>>> >> null.
>>>>> >>
>>>>> >> The following mechanisms exist today for resource mapping in ootb
>>>>> >> Sling:
>>>>> >>
>>>>> >> * Config "resource.resolver.mapping" in 
>>>>> JcrResourceResolverFactoryImpl
>>>>> >> * Mappings via /etc/map
>>>>> >> * Vanity Paths
>>>>> >> * sling:alias
>>>>> >>
>>>>> >> Those could be broken up into four implementations of 
>>>>> ResourceMapper
>>>>> >> that are configured by default.
>>>>> >>
>>>>> >> About backwards-compatibility/breaking existing 
>>>>> implementations: So
>>>>> >> this is a BIG CHANGE. However to keep it safe I would start with
>>>>> >> exactly one ResourceMapper active that is backed by today's
>>>>> >> implementation. The next step is to break it in separate resource
>>>>> >> mappers (as mentioned above) and just put them in a row.
>>>>> >>
>>>>> >> The Resource Mapping SPI would bring the following benefits:
>>>>> >>
>>>>> >> * Being able to hook into the resource mapping via SPI allows
>>>>> >>  for flexibility that is otherwise only possible Rewriting
>>>>> >>  Pipelines - while link rewriting/checking is the only
>>>>> >>  reason most projects use Rewriting Pipelines (with all its
>>>>> >>  performance overhead)
>>>>> >>
>>>>> >> * Extending the mapping process via a well-defined API that
>>>>> >>  allows to write Touring-complete code is better than working
>>>>> >>  with Strings in the Transformer (also for many cases it can
>>>>> >>  be cleaner and better 'unit-testable' than /etc/maps)
>>>>> >>
>>>>> >> * HTL there can be an option to automatically map all
>>>>> >>  attributes of context uri (better performance, better for
>>>>> >>  debugging if map() is called synchronously)
>>>>> >>
>>>>> >> * Introducing the interface ResourceURI (with a backing helper
>>>>> >>  class for everybody to use) is useful in general
>>>>> >>
>>>>> >> See branch [4] for some first code of the proposal, in
>>>>> >> particular ResourceMapping.java [5] and ResourceURI.java [6]
>>>>> >>
>>>>> >> -Georg
>>>>> >>
>>>>> >> [1] "[hackathon] externalization / resource mapping / rewriter"
>>>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
>>>>> >> [2] "Why get rid of the rewriter?"
>>>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
>>>>> >> [3] 
>>>>> https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
>>>>> >>
>>>>> >> [4]
>>>>> >> 
>>>>> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI 
>>>>>
>>>>> >> [5]
>>>>> >> 
>>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java 
>>>>>
>>>>> >> [6]
>>>>> >> 
>>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java 
>>>>>
>>>>> >>
>>>>> >>
>>>>> >>
>>>>> >> On 2020-01-17 10:45, Georg Henzler wrote:
>>>>> >>> Hi Konrad,
>>>>> >>> I think SLING-9005 is a good idea, but that is independent.
>>>>> >>> My idea idea was to leave resourceResolver.map() unchanged (as 
>>>>> there
>>>>> >>> is a lot of code using it out there, also it makes 
>>>>> conceptually sense
>>>>> >>> as is). Rather I‘d like to provide an SPI that allows to 
>>>>> customize
>>>>> >>> the
>>>>> >>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>>>>> >>> ranked list of ResourceMapper services (default one being today’s
>>>>> >>> behavior). Robert also has done some work into this direction
>>>>> >>> already,
>>>>> >>> I‘d like to extend on that.
>>>>> >>> I‘m currently on vacation but I have it on my list for when 
>>>>> I‘m back.
>>>>> >>> -Georg
>>>>> >>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> 
>>>>> wrote:
>>>>> >>>> I would like to revive this.
>>>>> >>>> @Georg: IIRC you wanted to come up with a proposal for a
>>>>> >>>> externalizer API. Are you working on this?
>>>>> >>>> Can we start by creating a JIRA ticket?
>>>>> >>>> There has been a related ticket opened today:
>>>>> >>>> https://issues.apache.org/jira/browse/SLING-9005
>>>>> >>>> <https://issues.apache.org/jira/browse/SLING-9005>
>>>>> >>>> Konrad
>>>>> >>>>> On 5. Sep 2019, at 17:54, Jason E Bailey 
>>>>> <ja...@24601.org>
>>>>> >>>>> wrote:
>>>>> >>>>> Specifically with regard to the re-writer
>>>>> >>>>> I'm working on a replacement for this which I'm currently 
>>>>> calling
>>>>> >>>>> the transformer ->
>>>>> >>>>> 
>>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>>>> >>>>> 1. It uses an HTML tokenizer that was added to the HTML 
>>>>> utilities
>>>>> >>>>> to generate a stream of  elements that can be modified and
>>>>> >>>>> recombined. It's much faster then tagsoup or jsoup since 
>>>>> it's not
>>>>> >>>>> trying to build a valid document. This also means that it 
>>>>> supports
>>>>> >>>>> anything that you write out in html. HTML4,5+
>>>>> >>>>> 2. It uses services with an ordering priority and does pattern
>>>>> >>>>> matching so that you can fine tune when the transformer is 
>>>>> applied
>>>>> >>>>> 3. The specific use case I was working on is creating a CSP 
>>>>> header
>>>>> >>>>> with a nonce or hash attribute to lock down the javascript 
>>>>> that's
>>>>> >>>>> on the page.
>>>>> >>>>> It's currently working but it's not finished.
>>>>> >>>>> Are there other problems with the rewriter that I haven't
>>>>> >>>>> addressed?
>>>>> >>>>> --
>>>>> >>>>> Jason
>>>>> >>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>>> >>>>>> - resource mapping
>>>>> >>>>>> - add a new SPI to define the mapping
>>>>> >>>>>> - add a default impl that reads the mapping from /etc/map 
>>>>> as it is
>>>>> >>>>>> done today
>>>>> >>>>>> - possible to override with service ranking
>>>>> >>>>>> - but: there is already the ResourceMapper interface
>>>>> >>>>>>  - introduced 1-2 years ago, use case was to get all existing
>>>>> >>>>>> mappings
>>>>> >>>>>>  - with this it's currently possible to replace mapping 
>>>>> completely
>>>>> >>>>>> with new logic
>>>>> >>>>>> - maybe add a support to "contribute" additional mappings 
>>>>> via a
>>>>> >>>>>> service interface additional to this
>>>>> >>>>>> - generic externalizer API
>>>>> >>>>>> - naming not fixed yet, should not named "link" as this is too
>>>>> >>>>>> specific. probably "URL".
>>>>> >>>>>> - needs a java API for using, and an SPI for hooking in 
>>>>> special
>>>>> >>>>>> rules
>>>>> >>>>>> - then add mappings for views in HTL*, JSP, model 
>>>>> exporter/JSON
>>>>> >>>>>>  * probably starting with a sling-only HTL extension for 
>>>>> try-out,
>>>>> >>>>>> add it later to the spec when design is validated
>>>>> >>>>>> - might be inspired by the basic features for wcm.io URL 
>>>>> handler
>>>>> >>>>>> [1]
>>>>> >>>>>> - rewriter pipeline
>>>>> >>>>>> - should be deprecated and no longer be used - especially 
>>>>> not for
>>>>> >>>>>> link externalization via postprocessing
>>>>> >>>>>> - it may still be in use out there for more exotic use 
>>>>> cases like
>>>>> >>>>>> PDF
>>>>> >>>>>> generation
>>>>> >>>>>> - should probably removed from sling starter
>>>>> >>>>>> stefan
>>>>> >>>>>> [1] https://wcm.io/handler/url/
>

Re: [DISCUSS] Sling URI Mapping SPI (was Resource Mapping SPI)

Posted by Georg Henzler <gh...@apache.org>.
Hi all,

so I created the PRs:
https://github.com/apache/sling-org-apache-sling-api/pull/25
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/22
https://github.com/apache/sling-org-apache-sling-engine/pull/10
https://github.com/apache/sling-org-apache-sling-auth-core/pull/6

Jenkins is failing due to not having the API SNAPSHOT from the branch,
I'll try to create/deploy a dedicated SNAPSHOT version from the API
branch to Maven Central (as I assume the PRs will be open for a bit
for testing and it would be nice to have the builds green).

-Georg


On 2020-09-30 12:09, Georg Henzler wrote:
> Hi Carsten,
> 
>> @Georg - I think it would be good if you create PRs, so we can better
>> review and comment on the changes
> 
> Thanks a lot - I'll quickly cross-test and create a PR this evening
> (I have an improvement for SlingUri as well, but that's independent)
> 
> -Georg
> 
>> 

Re: [DISCUSS] Sling URI Mapping SPI (was Resource Mapping SPI)

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

> @Georg - I think it would be good if you create PRs, so we can better
> review and comment on the changes

Thanks a lot - I'll quickly cross-test and create a PR this evening
(I have an improvement for SlingUri as well, but that's independent)

-Georg

> 
> Regards
> Carsten
> 
> Am 29.09.2020 um 11:44 schrieb Carsten Ziegeler:
>> Thanks Georg,
>> 
>> I had a quick look at the auth.core changes. They cover the basic 
>> flow, but we also need to remove the usage of the ResourceMapper (in 
>> the service listener). Unfortunately, this requires a rewrite of most 
>> of the tests, I'll try to have a look at this in the next days.
>> 
>> Regards
>> Carsten
>> 
>> Am 23.09.2020 um 11:06 schrieb Georg Henzler:
>>> Hi all,
>>> 
>>> so "SlingUri" (with now improved name) as prerequisite for
>>> the Mapping SPI has now been fixed via SLING-9745 and is on
>>> master. This alone is just a utility - no behaviour of
>>> resolve() or map() has been changed (yet).
>>> 
>>> With the changes [1] [2] [3] [4] (only on branches so far)
>>> the URI mapping is moved before authentication - this will
>>> improve the situation for auth requirements as Carsten
>>> outlined in [5]. I have tested this in both Sling Starter
>>> and the famous Sling-based CMS and it works well.
>>> 
>>> For now the updated proposal keeps out context hints (like
>>> e.g. "link invalid") for simplicity, we can can add this
>>> later (if we wanted to allow for link checking to get rid
>>> of the need for the rewriter pipeline that takes this
>>> responsibility in a Sling-based CMS)
>>> 
>>> I think it'll good to discuss this at the round table next
>>> Monday, however it would be great if people could already
>>> provide feedback to the API changes in [1] to see if that
>>> makes sense for everyone.
>>> 
>>> -Georg
>>> 
>>> [1] Sling API Changes
>>> https://github.com/apache/sling-org-apache-sling-api/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>> [2] Auth Core
>>> https://github.com/apache/sling-org-apache-sling-auth-core/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>> [3] Engine
>>> https://github.com/apache/sling-org-apache-sling-engine/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>> [4] Resource Resolver
>>> https://github.com/apache/sling-org-apache-sling-resourceresolver/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>> [5]
>>> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17182336 
>>> On 2020-09-20 21:46, Georg Henzler wrote:
>>>> Considering that URIs like 'mailto:jon@example.com', '?name=val'
>>>> or '#fragment' do not relate to a resource and thinking about it
>>>> a bit more, I would now propose to change the name of
>>>> ResourceUri/ResourceUriBuilder to SlingUri/SlingUriBuilder
>>>> (see also [1]) - objections?
>>>> 
>>>> -Georg
>>>> 
>>>> [1]
>>>> https://github.com/apache/sling-org-apache-sling-api/pull/23#discussion_r491726353 
>>>> On 2020-09-18 11:14, Georg Henzler wrote:
>>>>> Hi all,
>>>>> 
>>>>> to not have too many changes in one issue and PR to review,
>>>>> I created SLING-9745 and PR [1] for the ResourceUri and
>>>>> its builder. The unit test coverage is good (~85%) and also
>>>>> covers the tests from SlingRequestPathInfoTest [2] to
>>>>> ensure compatibility. Timely feedback would be appreciated
>>>>> as I would like to build upon this on Mon (with the next PR).
>>>>> 
>>>>> -Georg
>>>>> 
>>>>> [1]
>>>>> https://github.com/apache/sling-org-apache-sling-api/pull/23
>>>>> [2]
>>>>> https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28
>> 

Re: [DISCUSS] Sling URI Mapping SPI (was Resource Mapping SPI)

Posted by Carsten Ziegeler <cz...@apache.org>.
I updated the branch in auth.core to what I believe it should look like.

@Georg - I think it would be good if you create PRs, so we can better 
review and comment on the changes

Regards
Carsten

Am 29.09.2020 um 11:44 schrieb Carsten Ziegeler:
> Thanks Georg,
> 
> I had a quick look at the auth.core changes. They cover the basic flow, 
> but we also need to remove the usage of the ResourceMapper (in the 
> service listener). Unfortunately, this requires a rewrite of most of the 
> tests, I'll try to have a look at this in the next days.
> 
> Regards
> Carsten
> 
> Am 23.09.2020 um 11:06 schrieb Georg Henzler:
>> Hi all,
>>
>> so "SlingUri" (with now improved name) as prerequisite for
>> the Mapping SPI has now been fixed via SLING-9745 and is on
>> master. This alone is just a utility - no behaviour of
>> resolve() or map() has been changed (yet).
>>
>> With the changes [1] [2] [3] [4] (only on branches so far)
>> the URI mapping is moved before authentication - this will
>> improve the situation for auth requirements as Carsten
>> outlined in [5]. I have tested this in both Sling Starter
>> and the famous Sling-based CMS and it works well.
>>
>> For now the updated proposal keeps out context hints (like
>> e.g. "link invalid") for simplicity, we can can add this
>> later (if we wanted to allow for link checking to get rid
>> of the need for the rewriter pipeline that takes this
>> responsibility in a Sling-based CMS)
>>
>> I think it'll good to discuss this at the round table next
>> Monday, however it would be great if people could already
>> provide feedback to the API changes in [1] to see if that
>> makes sense for everyone.
>>
>> -Georg
>>
>> [1] Sling API Changes
>> https://github.com/apache/sling-org-apache-sling-api/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>
>>
>> [2] Auth Core
>> https://github.com/apache/sling-org-apache-sling-auth-core/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>
>>
>> [3] Engine
>> https://github.com/apache/sling-org-apache-sling-engine/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>
>>
>> [4] Resource Resolver
>> https://github.com/apache/sling-org-apache-sling-resourceresolver/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
>>
>>
>> [5]
>> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17182336 
>>
>>
>>
>> On 2020-09-20 21:46, Georg Henzler wrote:
>>> Considering that URIs like 'mailto:jon@example.com', '?name=val'
>>> or '#fragment' do not relate to a resource and thinking about it
>>> a bit more, I would now propose to change the name of
>>> ResourceUri/ResourceUriBuilder to SlingUri/SlingUriBuilder
>>> (see also [1]) - objections?
>>>
>>> -Georg
>>>
>>> [1]
>>> https://github.com/apache/sling-org-apache-sling-api/pull/23#discussion_r491726353 
>>>
>>>
>>> On 2020-09-18 11:14, Georg Henzler wrote:
>>>> Hi all,
>>>>
>>>> to not have too many changes in one issue and PR to review,
>>>> I created SLING-9745 and PR [1] for the ResourceUri and
>>>> its builder. The unit test coverage is good (~85%) and also
>>>> covers the tests from SlingRequestPathInfoTest [2] to
>>>> ensure compatibility. Timely feedback would be appreciated
>>>> as I would like to build upon this on Mon (with the next PR).
>>>>
>>>> -Georg
>>>>
>>>> [1]
>>>> https://github.com/apache/sling-org-apache-sling-api/pull/23
>>>> [2]
>>>> https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28 
>>>>
>>>>
> 

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Sling URI Mapping SPI (was Resource Mapping SPI)

Posted by Carsten Ziegeler <cz...@apache.org>.
Thanks Georg,

I had a quick look at the auth.core changes. They cover the basic flow, 
but we also need to remove the usage of the ResourceMapper (in the 
service listener). Unfortunately, this requires a rewrite of most of the 
tests, I'll try to have a look at this in the next days.

Regards
Carsten

Am 23.09.2020 um 11:06 schrieb Georg Henzler:
> Hi all,
> 
> so "SlingUri" (with now improved name) as prerequisite for
> the Mapping SPI has now been fixed via SLING-9745 and is on
> master. This alone is just a utility - no behaviour of
> resolve() or map() has been changed (yet).
> 
> With the changes [1] [2] [3] [4] (only on branches so far)
> the URI mapping is moved before authentication - this will
> improve the situation for auth requirements as Carsten
> outlined in [5]. I have tested this in both Sling Starter
> and the famous Sling-based CMS and it works well.
> 
> For now the updated proposal keeps out context hints (like
> e.g. "link invalid") for simplicity, we can can add this
> later (if we wanted to allow for link checking to get rid
> of the need for the rewriter pipeline that takes this
> responsibility in a Sling-based CMS)
> 
> I think it'll good to discuss this at the round table next
> Monday, however it would be great if people could already
> provide feedback to the API changes in [1] to see if that
> makes sense for everyone.
> 
> -Georg
> 
> [1] Sling API Changes
> https://github.com/apache/sling-org-apache-sling-api/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
> 
> 
> [2] Auth Core
> https://github.com/apache/sling-org-apache-sling-auth-core/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
> 
> 
> [3] Engine
> https://github.com/apache/sling-org-apache-sling-engine/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
> 
> 
> [4] Resource Resolver
> https://github.com/apache/sling-org-apache-sling-resourceresolver/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3 
> 
> 
> [5]
> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17182336 
> 
> 
> 
> On 2020-09-20 21:46, Georg Henzler wrote:
>> Considering that URIs like 'mailto:jon@example.com', '?name=val'
>> or '#fragment' do not relate to a resource and thinking about it
>> a bit more, I would now propose to change the name of
>> ResourceUri/ResourceUriBuilder to SlingUri/SlingUriBuilder
>> (see also [1]) - objections?
>>
>> -Georg
>>
>> [1]
>> https://github.com/apache/sling-org-apache-sling-api/pull/23#discussion_r491726353 
>>
>>
>> On 2020-09-18 11:14, Georg Henzler wrote:
>>> Hi all,
>>>
>>> to not have too many changes in one issue and PR to review,
>>> I created SLING-9745 and PR [1] for the ResourceUri and
>>> its builder. The unit test coverage is good (~85%) and also
>>> covers the tests from SlingRequestPathInfoTest [2] to
>>> ensure compatibility. Timely feedback would be appreciated
>>> as I would like to build upon this on Mon (with the next PR).
>>>
>>> -Georg
>>>
>>> [1]
>>> https://github.com/apache/sling-org-apache-sling-api/pull/23
>>> [2]
>>> https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28 
>>>
>>>

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Sling URI Mapping SPI (was Resource Mapping SPI)

Posted by Georg Henzler <gh...@apache.org>.
Hi all,

so "SlingUri" (with now improved name) as prerequisite for
the Mapping SPI has now been fixed via SLING-9745 and is on
master. This alone is just a utility - no behaviour of
resolve() or map() has been changed (yet).

With the changes [1] [2] [3] [4] (only on branches so far)
the URI mapping is moved before authentication - this will
improve the situation for auth requirements as Carsten
outlined in [5]. I have tested this in both Sling Starter
and the famous Sling-based CMS and it works well.

For now the updated proposal keeps out context hints (like
e.g. "link invalid") for simplicity, we can can add this
later (if we wanted to allow for link checking to get rid
of the need for the rewriter pipeline that takes this
responsibility in a Sling-based CMS)

I think it'll good to discuss this at the round table next
Monday, however it would be great if people could already
provide feedback to the API changes in [1] to see if that
makes sense for everyone.

-Georg

[1] Sling API Changes
https://github.com/apache/sling-org-apache-sling-api/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3

[2] Auth Core
https://github.com/apache/sling-org-apache-sling-auth-core/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3

[3] Engine
https://github.com/apache/sling-org-apache-sling-engine/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3

[4] Resource Resolver
https://github.com/apache/sling-org-apache-sling-resourceresolver/compare/master...feature/SLING-9662-Introduce-SlingUri-Mapping-SPI-v3

[5]
https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17182336


On 2020-09-20 21:46, Georg Henzler wrote:
> Considering that URIs like 'mailto:jon@example.com', '?name=val'
> or '#fragment' do not relate to a resource and thinking about it
> a bit more, I would now propose to change the name of
> ResourceUri/ResourceUriBuilder to SlingUri/SlingUriBuilder
> (see also [1]) - objections?
> 
> -Georg
> 
> [1]
> https://github.com/apache/sling-org-apache-sling-api/pull/23#discussion_r491726353
> 
> On 2020-09-18 11:14, Georg Henzler wrote:
>> Hi all,
>> 
>> to not have too many changes in one issue and PR to review,
>> I created SLING-9745 and PR [1] for the ResourceUri and
>> its builder. The unit test coverage is good (~85%) and also
>> covers the tests from SlingRequestPathInfoTest [2] to
>> ensure compatibility. Timely feedback would be appreciated
>> as I would like to build upon this on Mon (with the next PR).
>> 
>> -Georg
>> 
>> [1]
>> https://github.com/apache/sling-org-apache-sling-api/pull/23
>> [2]
>> https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28
>> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Considering that URIs like 'mailto:jon@example.com', '?name=val'
or '#fragment' do not relate to a resource and thinking about it
a bit more, I would now propose to change the name of
ResourceUri/ResourceUriBuilder to SlingUri/SlingUriBuilder
(see also [1]) - objections?

-Georg

[1] 
https://github.com/apache/sling-org-apache-sling-api/pull/23#discussion_r491726353

On 2020-09-18 11:14, Georg Henzler wrote:
> Hi all,
> 
> to not have too many changes in one issue and PR to review,
> I created SLING-9745 and PR [1] for the ResourceUri and
> its builder. The unit test coverage is good (~85%) and also
> covers the tests from SlingRequestPathInfoTest [2] to
> ensure compatibility. Timely feedback would be appreciated
> as I would like to build upon this on Mon (with the next PR).
> 
> -Georg
> 
> [1]
> https://github.com/apache/sling-org-apache-sling-api/pull/23
> [2]
> https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28
> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi all,

to not have too many changes in one issue and PR to review,
I created SLING-9745 and PR [1] for the ResourceUri and
its builder. The unit test coverage is good (~85%) and also
covers the tests from SlingRequestPathInfoTest [2] to
ensure compatibility. Timely feedback would be appreciated
as I would like to build upon this on Mon (with the next PR).

-Georg

[1]
https://github.com/apache/sling-org-apache-sling-api/pull/23
[2] 
https://github.com/apache/sling-org-apache-sling-engine/blob/43673bddb4a2e79ef9bba813ec86a7faa89e7649/src/test/java/org/apache/sling/engine/impl/request/SlingRequestPathInfoTest.java#L28

On 2020-09-11 00:45, Georg Henzler wrote:
> Hi Carsten,
> 
> so I followed the proposal and moved the resource mapping
> SPI "resolve hook-in point" before authentication (in
> sling engine) - see [1]. This will mean that once the
> path mappings are migrated to the SPI, the authentication
> requirements can be evaluated after the mapping.
> 
> The Sling API part is quite mature, ResourceUri has quite
> intensive test coverage now [2]. ResourceUriMapper [3] is
> the SPI interface (not called ResourceToUriMapper anymore
> because the SPI interface is actually taking ResourceUri
> as both input and output). The service calls all
> ResourceUriMappers is now called PathToUriMappingService [4]
> (was called ResourceUriMappingChain, now closer to your
> proposal PathRewriter). This service is part of the API
> now to allow to use is from the sling engine bundle.
> 
> The parts in resourceresolver and sling engine work well
> with the sling starter (Slingshot, Composum) but will need
> a bit of more testing...
> 
> -Georg
> 
> [1]
> https://github.com/apache/sling-org-apache-sling-api/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
> https://github.com/apache/sling-org-apache-sling-api/commit/c27db47a07ce7d85896e1c761f462653a7532e79
> 
> https://github.com/apache/sling-org-apache-sling-resourceresolver/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
> https://github.com/apache/sling-org-apache-sling-resourceresolver/commit/de835ae65bc4e8b5b6955ee5e679706e0e0a2adf
> 
> https://github.com/apache/sling-org-apache-sling-engine/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
> https://github.com/apache/sling-org-apache-sling-engine/commit/203f9a85a101749c070ea8c3d76ef194c8056b73
> 
> [2]
> https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/test/java/org/apache/sling/api/resource/uri/ResourceUriTest.java
> 
> [3]
> https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/main/java/org/apache/sling/spi/resource/mapping/ResourceUriMapper.java
> 
> [4]
> https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/main/java/org/apache/sling/api/resource/mapping/PathToUriMappingService.java
> 
> 
> On 2020-09-04 07:09, Carsten Ziegeler wrote:
>> Hi Georg,
>> 
>>> 
>>> One question though:
>>> 
>>>> PathRewriterFactory
>>> So this class is currently called ResourceUriMappingChain and it is
>>> *not* a factory - it is a regular OSG service that does not hold
>>> any state as member variables - did you have a particular reason in
>>> mind why it should be a factory or was this just following the
>>> pattern from the ResourceResolver?
>>> I wanted to have a factory that allows to create a PathRewriter which
>> contains a servlet request - this way such a PathRewriter can be
>> created for every incoming request as a first step and then simply be
>> passed - in my case to the ResourceResolverFactory and then the
>> ResourceResolver.
>> This way if you use the PathRewriter from the ResourceResolver it
>> already has the right context (request).
>> 
>> I thought it might be useful to manage some state (request in this
>> case) on the PathRewriter.
>> 
>> Regards
>> Carsten
>> 
>> --
>> Carsten Ziegeler
>> Adobe Research Switzerland
>> cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

so I followed the proposal and moved the resource mapping
SPI "resolve hook-in point" before authentication (in
sling engine) - see [1]. This will mean that once the
path mappings are migrated to the SPI, the authentication
requirements can be evaluated after the mapping.

The Sling API part is quite mature, ResourceUri has quite
intensive test coverage now [2]. ResourceUriMapper [3] is
the SPI interface (not called ResourceToUriMapper anymore
because the SPI interface is actually taking ResourceUri
as both input and output). The service calls all
ResourceUriMappers is now called PathToUriMappingService [4]
(was called ResourceUriMappingChain, now closer to your
proposal PathRewriter). This service is part of the API
now to allow to use is from the sling engine bundle.

The parts in resourceresolver and sling engine work well
with the sling starter (Slingshot, Composum) but will need
a bit of more testing...

-Georg

[1]
https://github.com/apache/sling-org-apache-sling-api/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
https://github.com/apache/sling-org-apache-sling-api/commit/c27db47a07ce7d85896e1c761f462653a7532e79

https://github.com/apache/sling-org-apache-sling-resourceresolver/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
https://github.com/apache/sling-org-apache-sling-resourceresolver/commit/de835ae65bc4e8b5b6955ee5e679706e0e0a2adf

https://github.com/apache/sling-org-apache-sling-engine/commits/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2
https://github.com/apache/sling-org-apache-sling-engine/commit/203f9a85a101749c070ea8c3d76ef194c8056b73

[2]
https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/test/java/org/apache/sling/api/resource/uri/ResourceUriTest.java

[3]
https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/main/java/org/apache/sling/spi/resource/mapping/ResourceUriMapper.java

[4]
https://github.com/apache/sling-org-apache-sling-api/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI-v2/src/main/java/org/apache/sling/api/resource/mapping/PathToUriMappingService.java


On 2020-09-04 07:09, Carsten Ziegeler wrote:
> Hi Georg,
> 
>> 
>> One question though:
>> 
>>> PathRewriterFactory
>> So this class is currently called ResourceUriMappingChain and it is
>> *not* a factory - it is a regular OSG service that does not hold
>> any state as member variables - did you have a particular reason in
>> mind why it should be a factory or was this just following the
>> pattern from the ResourceResolver?
>> I wanted to have a factory that allows to create a PathRewriter which
> contains a servlet request - this way such a PathRewriter can be
> created for every incoming request as a first step and then simply be
> passed - in my case to the ResourceResolverFactory and then the
> ResourceResolver.
> This way if you use the PathRewriter from the ResourceResolver it
> already has the right context (request).
> 
> I thought it might be useful to manage some state (request in this
> case) on the PathRewriter.
> 
> Regards
> Carsten
> 
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Georg,

> 
> One question though:
> 
>> PathRewriterFactory
> So this class is currently called ResourceUriMappingChain and it is
> *not* a factory - it is a regular OSG service that does not hold
> any state as member variables - did you have a particular reason in
> mind why it should be a factory or was this just following the
> pattern from the ResourceResolver?
> I wanted to have a factory that allows to create a PathRewriter which 
contains a servlet request - this way such a PathRewriter can be created 
for every incoming request as a first step and then simply be passed - 
in my case to the ResourceResolverFactory and then the ResourceResolver.
This way if you use the PathRewriter from the ResourceResolver it 
already has the right context (request).

I thought it might be useful to manage some state (request in this case) 
on the PathRewriter.

Regards
Carsten

--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

so this is very similar to what I have on [1] already. I was busy
with other things this week but I'll start now to fix the conflicts
from the merges of SLING-9620 and SLING-9623. Also I'll move the SPI
package underneath org.apache.sling.spi and I'll try to incorporate
your ideas from branch pathmappingproposal.

One question though:

> PathRewriterFactory
So this class is currently called ResourceUriMappingChain and it is
*not* a factory - it is a regular OSG service that does not hold
any state as member variables - did you have a particular reason in
mind why it should be a factory or was this just following the
pattern from the ResourceResolver?

-Georg

[1] 
https://github.com/apache/sling-org-apache-sling-api/blob/feature/Resource_Mapping_SPI/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java


On 2020-09-02 14:19, Carsten Ziegeler wrote:
> Hi,
> 
> in order to make it simpler to understand my proposal, I created an
> api prototype [1]. It's really just to demonstrate - nothing else. For
> class / method names, I just picked the first that came to my mind.
> 
> PathMapper is the SPI
> (https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/spi/resource/mapping/PathMapper.java)
> - I just a simpler String based interface - again just for
> demonstration; it doesn't mean that the current proposed Resource
> Mapping SPI is bad; but I want to focus on the overall design first.
> The services implementing the SPI are called in a chain, the
> difference to the current proposal is that they do not have access to
> a ResourceResolver via the context/parameters
> 
> PathRewriterFactory
> (https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/api/resource/mapping/PathRewriterFactory.java)
> creates PathRewriter objects - either with or without a servlet
> request.
> 
> The PathRewriter invokes the chain from above for mapping and reverse 
> mapping.
> 
> ResourceResolver gets two new methods
> (https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/api/resource/ResourceResolver.java#L198-205)
> - one to just invoke the resolving part and one to get the associated
> PathRewriter (which can be passed in via the RRF).
> 
> The map/resolve methods get deprecated, they will internally call the
> PathRewriter.
> 
> 
> Regards
> Carsten
> 
> 
> 
> [1]
> https://github.com/apache/sling-org-apache-sling-api/tree/feature/pathmappingproposal
> 
> Am 26.08.2020 um 10:10 schrieb Carsten Ziegeler:
>> Hi Georg,
>> 
>> Am 25.08.2020 um 18:53 schrieb Georg Henzler:
>>> 
>>>> We should have an API that can be used for this, whether this is
>>>> rr.map() or something new is a different question. I have no problem
>>>> with having rr.map() doing this.
>>> 
>>> I'm in favour of not introducing a new API if it's not absolutely
>>> needed. But calling out to another service from rr.map()/rr.resolve()
>>> should work fine.
>>> 
>> We need at least one new method for rr which replaces resolve() and 
>> only does resolving without mapping - as resolving would already be 
>> done pre authentication.
>> 
>>> 
>>> Ok fair enough - so your point was that the both map() and resolve()
>>> do not (or should not) produce different results based on the user
>>> context, my point was that to be future proof in the SPI, we need
>>> "access to the repository" to address all use cases. So "that 
>>> service"
>>> in the current implementation would be ResourceUriMappingChain [1],
>>> it currently has access to a rr via
>>> MappingChainContext.getResourceResolver() [2] - I think a resolver
>>> needs to be available to do lookups in the repository, but this
>>> could also be one based on a service user (ready only rights on
>>> the whole repo suffice).
>> 
>> Yes, ResourceUriMappingChain  would be such a service - but the 
>> methods would not have a RR as a argument but we would use a service 
>> user inside the implementation (which is then passed on using 
>> MappingChainContext).
>> 
>> Lets leave out the naming discussion for now, but my idea was to have 
>> a
>> ResourceUriMappingChainFactory as a service and that one creates new
>> ResourceUriMappingChain objects. An optional argument to the factory 
>> is the current request - so the request will be stored inside a 
>> ResourceUriMappingChain and there is no need to pass it to each and 
>> every call of a method of ResourceUriMappingChain.
>> 
>> 
>>> So to start with it it could work fine to
>>> hook in ResourceUriMappingChain.resolve() in the request processing
>>> before the resource resolver is created.
>> 
>> Yes, something along those lines
>> 
>>> Maybe then in the future
>>> we could provide a service property to ResourceToUriMapping providers
>>> that allows to be called with the request's user context after the
>>> resource resolver is created (so moving the default mapping chain
>>> before the authentication at least does not block the possibility
>>> to allow user based resolutions for the future).
>> 
>> I think this is where it gets dangerous :) If we allow for user based 
>> mapping later on, then clearly calling the chain before authentication 
>> is of no use. And I'm not sure if mapping per user is a good idea.
>>   > Also the user
>>> context could be generally be made available to rr.map(),
>>> I suppose there it wouldn't hurt (and it would allow to add users
>>> specific information to certain URLs, I think this can be quite
>>> useful). But to start with, would it be ok for you to leave
>>> MappingChainContext.getResourceResolver() [2] as is and provide
>>> a service user rr for repository access to the resolve operation?
>>> 
>> Yes, I'm not worried that much about the SPI api, I'm more worried 
>> about the service that is used which then calls the SPI 
>> (ResourceUriMappingChain)
>> 
>> Regards
>> Carsten
>> 
>> -- Carsten Ziegeler
>> Adobe Research Switzerland
>> cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Hi,

in order to make it simpler to understand my proposal, I created an api 
prototype [1]. It's really just to demonstrate - nothing else. For class 
/ method names, I just picked the first that came to my mind.

PathMapper is the SPI 
(https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/spi/resource/mapping/PathMapper.java) 
- I just a simpler String based interface - again just for 
demonstration; it doesn't mean that the current proposed Resource 
Mapping SPI is bad; but I want to focus on the overall design first.
The services implementing the SPI are called in a chain, the difference 
to the current proposal is that they do not have access to a 
ResourceResolver via the context/parameters

PathRewriterFactory 
(https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/api/resource/mapping/PathRewriterFactory.java) 
creates PathRewriter objects - either with or without a servlet request.

The PathRewriter invokes the chain from above for mapping and reverse 
mapping.

ResourceResolver gets two new methods 
(https://github.com/apache/sling-org-apache-sling-api/blob/feature/pathmappingproposal/src/main/java/org/apache/sling/api/resource/ResourceResolver.java#L198-205) 
- one to just invoke the resolving part and one to get the associated 
PathRewriter (which can be passed in via the RRF).

The map/resolve methods get deprecated, they will internally call the 
PathRewriter.


Regards
Carsten



[1] 
https://github.com/apache/sling-org-apache-sling-api/tree/feature/pathmappingproposal

Am 26.08.2020 um 10:10 schrieb Carsten Ziegeler:
> Hi Georg,
> 
> Am 25.08.2020 um 18:53 schrieb Georg Henzler:
>>
>>> We should have an API that can be used for this, whether this is
>>> rr.map() or something new is a different question. I have no problem
>>> with having rr.map() doing this.
>>
>> I'm in favour of not introducing a new API if it's not absolutely
>> needed. But calling out to another service from rr.map()/rr.resolve()
>> should work fine.
>>
> We need at least one new method for rr which replaces resolve() and only 
> does resolving without mapping - as resolving would already be done pre 
> authentication.
> 
>>
>> Ok fair enough - so your point was that the both map() and resolve()
>> do not (or should not) produce different results based on the user
>> context, my point was that to be future proof in the SPI, we need
>> "access to the repository" to address all use cases. So "that service"
>> in the current implementation would be ResourceUriMappingChain [1],
>> it currently has access to a rr via
>> MappingChainContext.getResourceResolver() [2] - I think a resolver
>> needs to be available to do lookups in the repository, but this
>> could also be one based on a service user (ready only rights on
>> the whole repo suffice).
> 
> Yes, ResourceUriMappingChain  would be such a service - but the methods 
> would not have a RR as a argument but we would use a service user inside 
> the implementation (which is then passed on using MappingChainContext).
> 
> Lets leave out the naming discussion for now, but my idea was to have a
> ResourceUriMappingChainFactory as a service and that one creates new
> ResourceUriMappingChain objects. An optional argument to the factory is 
> the current request - so the request will be stored inside a 
> ResourceUriMappingChain and there is no need to pass it to each and 
> every call of a method of ResourceUriMappingChain.
> 
> 
>> So to start with it it could work fine to
>> hook in ResourceUriMappingChain.resolve() in the request processing
>> before the resource resolver is created. 
> 
> Yes, something along those lines
> 
>> Maybe then in the future
>> we could provide a service property to ResourceToUriMapping providers
>> that allows to be called with the request's user context after the
>> resource resolver is created (so moving the default mapping chain
>> before the authentication at least does not block the possibility
>> to allow user based resolutions for the future). 
> 
> I think this is where it gets dangerous :) If we allow for user based 
> mapping later on, then clearly calling the chain before authentication 
> is of no use. And I'm not sure if mapping per user is a good idea.
>   > Also the user
>> context could be generally be made available to rr.map(),
>> I suppose there it wouldn't hurt (and it would allow to add users
>> specific information to certain URLs, I think this can be quite
>> useful). But to start with, would it be ok for you to leave
>> MappingChainContext.getResourceResolver() [2] as is and provide
>> a service user rr for repository access to the resolve operation?
>>
> Yes, I'm not worried that much about the SPI api, I'm more worried about 
> the service that is used which then calls the SPI (ResourceUriMappingChain)
> 
> Regards
> Carsten
> 
> -- 
> Carsten Ziegeler
> Adobe Research Switzerland
> cziegeler@apache.org

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Georg,

Am 25.08.2020 um 18:53 schrieb Georg Henzler:
> 
>> We should have an API that can be used for this, whether this is
>> rr.map() or something new is a different question. I have no problem
>> with having rr.map() doing this.
> 
> I'm in favour of not introducing a new API if it's not absolutely
> needed. But calling out to another service from rr.map()/rr.resolve()
> should work fine.
> 
We need at least one new method for rr which replaces resolve() and only 
does resolving without mapping - as resolving would already be done pre 
authentication.

> 
> Ok fair enough - so your point was that the both map() and resolve()
> do not (or should not) produce different results based on the user
> context, my point was that to be future proof in the SPI, we need
> "access to the repository" to address all use cases. So "that service"
> in the current implementation would be ResourceUriMappingChain [1],
> it currently has access to a rr via
> MappingChainContext.getResourceResolver() [2] - I think a resolver
> needs to be available to do lookups in the repository, but this
> could also be one based on a service user (ready only rights on
> the whole repo suffice).

Yes, ResourceUriMappingChain  would be such a service - but the methods 
would not have a RR as a argument but we would use a service user inside 
the implementation (which is then passed on using MappingChainContext).

Lets leave out the naming discussion for now, but my idea was to have a
ResourceUriMappingChainFactory as a service and that one creates new
ResourceUriMappingChain objects. An optional argument to the factory is 
the current request - so the request will be stored inside a 
ResourceUriMappingChain and there is no need to pass it to each and 
every call of a method of ResourceUriMappingChain.


> So to start with it it could work fine to
> hook in ResourceUriMappingChain.resolve() in the request processing
> before the resource resolver is created. 

Yes, something along those lines

> Maybe then in the future
> we could provide a service property to ResourceToUriMapping providers
> that allows to be called with the request's user context after the
> resource resolver is created (so moving the default mapping chain
> before the authentication at least does not block the possibility
> to allow user based resolutions for the future). 

I think this is where it gets dangerous :) If we allow for user based 
mapping later on, then clearly calling the chain before authentication 
is of no use. And I'm not sure if mapping per user is a good idea.
  > Also the user
> context could be generally be made available to rr.map(),
> I suppose there it wouldn't hurt (and it would allow to add users
> specific information to certain URLs, I think this can be quite
> useful). But to start with, would it be ok for you to leave
> MappingChainContext.getResourceResolver() [2] as is and provide
> a service user rr for repository access to the resolve operation?
>
Yes, I'm not worried that much about the SPI api, I'm more worried about 
the service that is used which then calls the SPI (ResourceUriMappingChain)

Regards
Carsten

--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

> now, I totally agree that we should get rid of the rewriter pipeline.
> With HTL we can do the rewriting links without reparsing the html
> output.

perfect, for me that's one of the main goals of the new SPI. After
the hackathon last year there were some voices arguing pro rewriter
pipeline with valid use cases, but I think those are relevant for
only 5% of the projects (those can still use the pipeline, but
95% should be able to do without). Btw. the link checker use case
(that potentially removes <a href> tags) will bring a little bit
of complexity, but the plan is to allow "Resource URI context hints"
as result of a the mapping chain (and a hint "invalid uri" will make
HTL remove the surrounding tag)


> We should have an API that can be used for this, whether this is
> rr.map() or something new is a different question. I have no problem
> with having rr.map() doing this.

I'm in favour of not introducing a new API if it's not absolutely
needed. But calling out to another service from rr.map()/rr.resolve()
should work fine.

> Today mapping (and therefore reverse mapping) is not based on the
> current user context - and I think we should keep it this way. Meaning
> for any user mapping and reverse mapping creates the same result
> regardless of the user. Following this logic, it doesn't make sense to
> have this functionality in RR - as the RR is bound to a user. So I
> suggest to make this a separate service which can be invoked without
> having a RR. We can change the implementation of rr.map() to call that
> service. rr.resolve can be changed to first call that service, too,
> and then resolve the resulting path.

Ok fair enough - so your point was that the both map() and resolve()
do not (or should not) produce different results based on the user
context, my point was that to be future proof in the SPI, we need
"access to the repository" to address all use cases. So "that service"
in the current implementation would be ResourceUriMappingChain [1],
it currently has access to a rr via
MappingChainContext.getResourceResolver() [2] - I think a resolver
needs to be available to do lookups in the repository, but this
could also be one based on a service user (ready only rights on
the whole repo suffice). So to start with it it could work fine to
hook in ResourceUriMappingChain.resolve() in the request processing
before the resource resolver is created. Maybe then in the future
we could provide a service property to ResourceToUriMapping providers
that allows to be called with the request's user context after the
resource resolver is created (so moving the default mapping chain
before the authentication at least does not block the possibility
to allow user based resolutions for the future). Also the user
context could be generally be made available to rr.map(),
I suppose there it wouldn't hurt (and it would allow to add users
specific information to certain URLs, I think this can be quite
useful). But to start with, would it be ok for you to leave
MappingChainContext.getResourceResolver() [2] as is and provide
a service user rr for repository access to the resolve operation?

-Georg

[1] 
https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/feature/SLING-9662-Introduce-Resource-Mapping-SPI/src/main/java/org/apache/sling/resourceresolver/impl/mappingchain/ResourceUriMappingChain.java

[2] 
https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/MappingChainContext.java#L32

> Does that make sense?
> 
> Regards
> Carsten
> 
> 
> 
> Am 24.08.2020 um 23:58 schrieb Georg Henzler:
>> Hi Carsten,
>> 
>>> And if you still need this, you can do this differently without using
>>> the resource mapping functionality.
>> 
>> This happens too often today because of the missing extensibility of
>> the resource resolver. The point of the  Resource Mapping SPI is to
>> improve the situation and to be able to fully rely on rr.map()
>> That way we can make default mapping happen in HTL and get rid
>> of the rewriter pipeline for 95% of the projects IMHO, if rr.map()
>> only does "half the job" this cannot easily be achieved
>> 
>>> ...but that comes with the penalty
>>> of doing these things twice - first in auth core and then again with
>>> the user context - and hopefully both operations provide the same
>>> result (another reason why per user context mapping is dangerous).
>> 
>> The idea was to check the authentication requirement only once in
>> auth core (for the alias/vanity path case the resolve method will
>> have to make use of the same map, but I'd expect it to not be too
>> much of a problem)
>> 
>>> Mapping is different from resolving and I think we should start
>>> treating it differently.
>> 
>> IMHO both yes and no:
>> 
>> Yes: Mapping happens at a completely different time than resolving,
>> mapping creates links to be resolved in the future (even if you
>> provide the current request to map(), it'll act as example for a
>> future request.
>> 
>> No: map/resolve "amendments" to the setup should always be made
>> symmetrically - ResourceToUriMapper with both map() and resolve()
>> ensures exactly that by forcing SPI providers to think about both
>> aspects.
>> 
>> Now for SLING-9622 it's actually the rr.resolve() path that is
>> causing the headache (and "unauthenticated map()" would not help
>> here). If you like I can give it a try with an SPI interface
>> ResourceUriAuthRequirementChecker...
>> 
>> -Georg
>> 
>> 
>> On 2020-08-24 18:32, Carsten Ziegeler wrote:
>>> Hi Georg,
>>> 
>>> at least today, the user context is not really taken into account for
>>> mapping. And I think everything stays nicer and cleaner if we don't
>>> open this up - otherwise you need to think about setting correct
>>> caching headers etc.
>>> And if you still need this, you can do this differently without using
>>> the resource mapping functionality.
>>> 
>>> The primary concern is that authentication handlers and requirements
>>> can be handled correctly and without much effort. Of course we can 
>>> add
>>> more methods to the SPI and potentially other services to allow the
>>> auth core to do the correct lookups, but that comes with the penalty
>>> of doing these things twice - first in auth core and then again with
>>> the user context - and hopefully both operations provide the same
>>> result (another reason why per user context mapping is dangerous).
>>> 
>>> Mapping is different from resolving and I think we should start
>>> treating it differently. If we can agree that mapping is independent
>>> of the user context (and today that is the case), then we can 
>>> simplify
>>> things and make them cleaner.
>>> 
>>> Regards
>>> Carsten
>>> 
>>> Am 24.08.2020 um 17:09 schrieb Georg Henzler:
>>>> Hi Carsten,
>>>> 
>>>>> good opportunity to rethink mapping / resolving in general.
>>>> 
>>>> so you are referring to SLING-9622 and more specifically to comment
>>>> [1] I suppose... I have thought it a bit through and I think there
>>>> are valid use cases for having "the user context" (=rr) available
>>>> for both resolve() and map(). But if it's really just about a
>>>> knowing early (in processor) if the incoming request requires
>>>> authentication or not, why not just add a method
>>>> requiresAuthentication(resourceUri) to SPI at [2]? Or maybe cleaner,
>>>> a second interface ResourceUriAuthenticationRequirementChecker could
>>>> be introduced (that "interested" ResourceToUriMapper could also
>>>> implement). Once vanity path and alias handling are migrated to this
>>>> new setup, the SPI method can access the readily available data
>>>> structure, so this should not have a performance impact... WDYT?
>>>> 
>>>> -Georg
>>>> 
>>>> 
>>>> [1] 
>>>> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17182336 
>>>> [2] 
>>>> https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L35 
>>>> On 2020-08-23 11:24, Carsten Ziegeler wrote:
>>>>> Thanks for taking this up.
>>>>> 
>>>>> As noted in the issue, I think we should investigate whether this 
>>>>> is a
>>>>> good opportunity to rethink mapping / resolving in general. In
>>>>> hindsight, we probably shouldn't have added the mapping part to the
>>>>> resource resolver. The resolve() method does two things: it applies
>>>>> mappings (which is independent of the current user) and it resolves 
>>>>> to
>>>>> a resource (finding the resource and figuring out the selectors,
>>>>> extensions etc.) based on the user.
>>>>> 
>>>>> So how about moving the mapping part out of the resource resolver? 
>>>>> As
>>>>> we've seen recently other parts of Sling like auth core could 
>>>>> benefit
>>>>> from this as well.
>>>>> 
>>>>> I haven't fully thought this through, but this is the rough idea:
>>>>> 
>>>>> We create a new OSGi service, PathMapperFactory (its probably not a
>>>>> good name but I don't want to invest time into naming at this 
>>>>> stage).
>>>>> It has one method:
>>>>> 
>>>>>    PathMapper newPathMapper(HttpServletRequest request)
>>>>> 
>>>>> where the request parameter is optional. PathMapper has the
>>>>> resolve/map methods doing the mapping (based on the newly suggested
>>>>> SPI). If a request has been provided to the factory method, that
>>>>> object is used for resolving/mapping.
>>>>> I'm unsure about the exact methods of PathMapper, but it needs a
>>>>> resolve method taking in a path (the request path) and returning 
>>>>> the
>>>>> resolved path (vanity path, aliases etc are applied) and a flag
>>>>> whether a redirect should happen.
>>>>> 
>>>>> Sling's main servlet (or more precise the processor) is changed: 
>>>>> for
>>>>> an incoming request it calls the PathMapperFactory with the request
>>>>> and then the resolve method on PathMapper. If the result is absolut 
>>>>> or
>>>>> the redirect flag is set, the redirect happens.
>>>>> 
>>>>> Otherwise, authentication is invoked with the resolved path - this
>>>>> frees auth core (and all auth handlers etc) from dealing with
>>>>> mappings.
>>>>> 
>>>>> If authentication is successful, a new resource resolver is created
>>>>> and the PathMapper is passed to the resource resolver via the
>>>>> attributes of the resource resolver factory. ResourceResolver gets 
>>>>> a
>>>>> new method "getPathMapper()" - if no PathMapper is passed to the
>>>>> resource resolver factory, the factory creates a new one by 
>>>>> invoking
>>>>> newPathMapper(null) on the PathMapperFactory. In all cases, a 
>>>>> resource
>>>>> resolver has a PathMapper.
>>>>> 
>>>>> We add a new method to Resource Resolver, similar to the resolve()
>>>>> method but just doing the resolution part - no mapping. This method 
>>>>> is
>>>>> used by the main servlet instead of the existing resolve() method. 
>>>>> We
>>>>> deprecate resolve() and map() and change the implementation to
>>>>> internally use the passed in PathMapper instance.
>>>>> 
>>>>> This way we have a clear separation between mapping and resolving 
>>>>> and
>>>>> in which contexts the operations happen. With adding the PathMapper 
>>>>> to
>>>>> the resource resolver we make it available to all parts in the 
>>>>> system
>>>>> which need to call todays map() method - and as the PathMapper
>>>>> instance knows the request, there is no need to pass it around
>>>>> anymore.
>>>>> 
>>>>> Now, again, just a rough write down of the idea, don't get hung up 
>>>>> on
>>>>> the naming that can and needs to improve. Its also not directly 
>>>>> about
>>>>> the new SPI, but rather how we make the SPI available to the rest 
>>>>> of
>>>>> the system.
>>>>> 
>>>>> Regards
>>>>> Carsten
>>>>> 
>>>>> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Georg,

now, I totally agree that we should get rid of the rewriter pipeline. 
With HTL we can do the rewriting links without reparsing the html output.

We should have an API that can be used for this, whether this is 
rr.map() or something new is a different question. I have no problem 
with having rr.map() doing this.

I think I might not have clearly described how I see the terms mapping 
and resolving. Let me try again, rr.resolve() does today both, it maps 
an incoming path (using vanity path, alias, etc/map) and then it 
resolves the resulting path (separating the resource path from the other 
information in that path, selector, extension). Example: 
/foo/bar.me.html is first mapped to /content/foo/bar.me.html (through 
mapping in etc/map) and then resolved to /content/foo/bar, selector me, 
extension html.

rr.map() today does reverse mapping using etc/map. Example: 
/content/foo/bar2.html is mapped to /foo/bar2.html

Your suggested SPI is an extension mechanism for mapping and reverse 
mapping (which is ok)

Today mapping (and therefore reverse mapping) is not based on the 
current user context - and I think we should keep it this way. Meaning 
for any user mapping and reverse mapping creates the same result 
regardless of the user. Following this logic, it doesn't make sense to 
have this functionality in RR - as the RR is bound to a user. So I 
suggest to make this a separate service which can be invoked without 
having a RR. We can change the implementation of rr.map() to call that 
service. rr.resolve can be changed to first call that service, too, and 
then resolve the resulting path.

Does that make sense?

Regards
Carsten



Am 24.08.2020 um 23:58 schrieb Georg Henzler:
> Hi Carsten,
> 
>> And if you still need this, you can do this differently without using
>> the resource mapping functionality.
> 
> This happens too often today because of the missing extensibility of
> the resource resolver. The point of the  Resource Mapping SPI is to
> improve the situation and to be able to fully rely on rr.map()
> That way we can make default mapping happen in HTL and get rid
> of the rewriter pipeline for 95% of the projects IMHO, if rr.map()
> only does "half the job" this cannot easily be achieved
> 
>> ...but that comes with the penalty
>> of doing these things twice - first in auth core and then again with
>> the user context - and hopefully both operations provide the same
>> result (another reason why per user context mapping is dangerous).
> 
> The idea was to check the authentication requirement only once in
> auth core (for the alias/vanity path case the resolve method will
> have to make use of the same map, but I'd expect it to not be too
> much of a problem)
> 
>> Mapping is different from resolving and I think we should start
>> treating it differently.
> 
> IMHO both yes and no:
> 
> Yes: Mapping happens at a completely different time than resolving,
> mapping creates links to be resolved in the future (even if you
> provide the current request to map(), it'll act as example for a
> future request.
> 
> No: map/resolve "amendments" to the setup should always be made
> symmetrically - ResourceToUriMapper with both map() and resolve()
> ensures exactly that by forcing SPI providers to think about both
> aspects.
> 
> Now for SLING-9622 it's actually the rr.resolve() path that is
> causing the headache (and "unauthenticated map()" would not help
> here). If you like I can give it a try with an SPI interface
> ResourceUriAuthRequirementChecker...
> 
> -Georg
> 
> 
> On 2020-08-24 18:32, Carsten Ziegeler wrote:
>> Hi Georg,
>>
>> at least today, the user context is not really taken into account for
>> mapping. And I think everything stays nicer and cleaner if we don't
>> open this up - otherwise you need to think about setting correct
>> caching headers etc.
>> And if you still need this, you can do this differently without using
>> the resource mapping functionality.
>>
>> The primary concern is that authentication handlers and requirements
>> can be handled correctly and without much effort. Of course we can add
>> more methods to the SPI and potentially other services to allow the
>> auth core to do the correct lookups, but that comes with the penalty
>> of doing these things twice - first in auth core and then again with
>> the user context - and hopefully both operations provide the same
>> result (another reason why per user context mapping is dangerous).
>>
>> Mapping is different from resolving and I think we should start
>> treating it differently. If we can agree that mapping is independent
>> of the user context (and today that is the case), then we can simplify
>> things and make them cleaner.
>>
>> Regards
>> Carsten
>>
>> Am 24.08.2020 um 17:09 schrieb Georg Henzler:
>>> Hi Carsten,
>>>
>>>> good opportunity to rethink mapping / resolving in general.
>>>
>>> so you are referring to SLING-9622 and more specifically to comment
>>> [1] I suppose... I have thought it a bit through and I think there
>>> are valid use cases for having "the user context" (=rr) available
>>> for both resolve() and map(). But if it's really just about a
>>> knowing early (in processor) if the incoming request requires
>>> authentication or not, why not just add a method
>>> requiresAuthentication(resourceUri) to SPI at [2]? Or maybe cleaner,
>>> a second interface ResourceUriAuthenticationRequirementChecker could
>>> be introduced (that "interested" ResourceToUriMapper could also
>>> implement). Once vanity path and alias handling are migrated to this
>>> new setup, the SPI method can access the readily available data
>>> structure, so this should not have a performance impact... WDYT?
>>>
>>> -Georg
>>>
>>>
>>> [1] 
>>> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17182336 
>>> [2] 
>>> https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L35 
>>> On 2020-08-23 11:24, Carsten Ziegeler wrote:
>>>> Thanks for taking this up.
>>>>
>>>> As noted in the issue, I think we should investigate whether this is a
>>>> good opportunity to rethink mapping / resolving in general. In
>>>> hindsight, we probably shouldn't have added the mapping part to the
>>>> resource resolver. The resolve() method does two things: it applies
>>>> mappings (which is independent of the current user) and it resolves to
>>>> a resource (finding the resource and figuring out the selectors,
>>>> extensions etc.) based on the user.
>>>>
>>>> So how about moving the mapping part out of the resource resolver? As
>>>> we've seen recently other parts of Sling like auth core could benefit
>>>> from this as well.
>>>>
>>>> I haven't fully thought this through, but this is the rough idea:
>>>>
>>>> We create a new OSGi service, PathMapperFactory (its probably not a
>>>> good name but I don't want to invest time into naming at this stage).
>>>> It has one method:
>>>>
>>>>    PathMapper newPathMapper(HttpServletRequest request)
>>>>
>>>> where the request parameter is optional. PathMapper has the
>>>> resolve/map methods doing the mapping (based on the newly suggested
>>>> SPI). If a request has been provided to the factory method, that
>>>> object is used for resolving/mapping.
>>>> I'm unsure about the exact methods of PathMapper, but it needs a
>>>> resolve method taking in a path (the request path) and returning the
>>>> resolved path (vanity path, aliases etc are applied) and a flag
>>>> whether a redirect should happen.
>>>>
>>>> Sling's main servlet (or more precise the processor) is changed: for
>>>> an incoming request it calls the PathMapperFactory with the request
>>>> and then the resolve method on PathMapper. If the result is absolut or
>>>> the redirect flag is set, the redirect happens.
>>>>
>>>> Otherwise, authentication is invoked with the resolved path - this
>>>> frees auth core (and all auth handlers etc) from dealing with
>>>> mappings.
>>>>
>>>> If authentication is successful, a new resource resolver is created
>>>> and the PathMapper is passed to the resource resolver via the
>>>> attributes of the resource resolver factory. ResourceResolver gets a
>>>> new method "getPathMapper()" - if no PathMapper is passed to the
>>>> resource resolver factory, the factory creates a new one by invoking
>>>> newPathMapper(null) on the PathMapperFactory. In all cases, a resource
>>>> resolver has a PathMapper.
>>>>
>>>> We add a new method to Resource Resolver, similar to the resolve()
>>>> method but just doing the resolution part - no mapping. This method is
>>>> used by the main servlet instead of the existing resolve() method. We
>>>> deprecate resolve() and map() and change the implementation to
>>>> internally use the passed in PathMapper instance.
>>>>
>>>> This way we have a clear separation between mapping and resolving and
>>>> in which contexts the operations happen. With adding the PathMapper to
>>>> the resource resolver we make it available to all parts in the system
>>>> which need to call todays map() method - and as the PathMapper
>>>> instance knows the request, there is no need to pass it around
>>>> anymore.
>>>>
>>>> Now, again, just a rough write down of the idea, don't get hung up on
>>>> the naming that can and needs to improve. Its also not directly about
>>>> the new SPI, but rather how we make the SPI available to the rest of
>>>> the system.
>>>>
>>>> Regards
>>>> Carsten
>>>>
>>>>

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

> And if you still need this, you can do this differently without using
> the resource mapping functionality.

This happens too often today because of the missing extensibility of
the resource resolver. The point of the  Resource Mapping SPI is to
improve the situation and to be able to fully rely on rr.map()
That way we can make default mapping happen in HTL and get rid
of the rewriter pipeline for 95% of the projects IMHO, if rr.map()
only does "half the job" this cannot easily be achieved

> ...but that comes with the penalty
> of doing these things twice - first in auth core and then again with
> the user context - and hopefully both operations provide the same
> result (another reason why per user context mapping is dangerous).

The idea was to check the authentication requirement only once in
auth core (for the alias/vanity path case the resolve method will
have to make use of the same map, but I'd expect it to not be too
much of a problem)

> Mapping is different from resolving and I think we should start
> treating it differently.

IMHO both yes and no:

Yes: Mapping happens at a completely different time than resolving,
mapping creates links to be resolved in the future (even if you
provide the current request to map(), it'll act as example for a
future request.

No: map/resolve "amendments" to the setup should always be made
symmetrically - ResourceToUriMapper with both map() and resolve()
ensures exactly that by forcing SPI providers to think about both
aspects.

Now for SLING-9622 it's actually the rr.resolve() path that is
causing the headache (and "unauthenticated map()" would not help
here). If you like I can give it a try with an SPI interface
ResourceUriAuthRequirementChecker...

-Georg


On 2020-08-24 18:32, Carsten Ziegeler wrote:
> Hi Georg,
> 
> at least today, the user context is not really taken into account for
> mapping. And I think everything stays nicer and cleaner if we don't
> open this up - otherwise you need to think about setting correct
> caching headers etc.
> And if you still need this, you can do this differently without using
> the resource mapping functionality.
> 
> The primary concern is that authentication handlers and requirements
> can be handled correctly and without much effort. Of course we can add
> more methods to the SPI and potentially other services to allow the
> auth core to do the correct lookups, but that comes with the penalty
> of doing these things twice - first in auth core and then again with
> the user context - and hopefully both operations provide the same
> result (another reason why per user context mapping is dangerous).
> 
> Mapping is different from resolving and I think we should start
> treating it differently. If we can agree that mapping is independent
> of the user context (and today that is the case), then we can simplify
> things and make them cleaner.
> 
> Regards
> Carsten
> 
> Am 24.08.2020 um 17:09 schrieb Georg Henzler:
>> Hi Carsten,
>> 
>>> good opportunity to rethink mapping / resolving in general.
>> 
>> so you are referring to SLING-9622 and more specifically to comment
>> [1] I suppose... I have thought it a bit through and I think there
>> are valid use cases for having "the user context" (=rr) available
>> for both resolve() and map(). But if it's really just about a
>> knowing early (in processor) if the incoming request requires
>> authentication or not, why not just add a method
>> requiresAuthentication(resourceUri) to SPI at [2]? Or maybe cleaner,
>> a second interface ResourceUriAuthenticationRequirementChecker could
>> be introduced (that "interested" ResourceToUriMapper could also
>> implement). Once vanity path and alias handling are migrated to this
>> new setup, the SPI method can access the readily available data
>> structure, so this should not have a performance impact... WDYT?
>> 
>> -Georg
>> 
>> 
>> [1] 
>> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17182336 
>> [2] 
>> https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L35 
>> On 2020-08-23 11:24, Carsten Ziegeler wrote:
>>> Thanks for taking this up.
>>> 
>>> As noted in the issue, I think we should investigate whether this is 
>>> a
>>> good opportunity to rethink mapping / resolving in general. In
>>> hindsight, we probably shouldn't have added the mapping part to the
>>> resource resolver. The resolve() method does two things: it applies
>>> mappings (which is independent of the current user) and it resolves 
>>> to
>>> a resource (finding the resource and figuring out the selectors,
>>> extensions etc.) based on the user.
>>> 
>>> So how about moving the mapping part out of the resource resolver? As
>>> we've seen recently other parts of Sling like auth core could benefit
>>> from this as well.
>>> 
>>> I haven't fully thought this through, but this is the rough idea:
>>> 
>>> We create a new OSGi service, PathMapperFactory (its probably not a
>>> good name but I don't want to invest time into naming at this stage).
>>> It has one method:
>>> 
>>>    PathMapper newPathMapper(HttpServletRequest request)
>>> 
>>> where the request parameter is optional. PathMapper has the
>>> resolve/map methods doing the mapping (based on the newly suggested
>>> SPI). If a request has been provided to the factory method, that
>>> object is used for resolving/mapping.
>>> I'm unsure about the exact methods of PathMapper, but it needs a
>>> resolve method taking in a path (the request path) and returning the
>>> resolved path (vanity path, aliases etc are applied) and a flag
>>> whether a redirect should happen.
>>> 
>>> Sling's main servlet (or more precise the processor) is changed: for
>>> an incoming request it calls the PathMapperFactory with the request
>>> and then the resolve method on PathMapper. If the result is absolut 
>>> or
>>> the redirect flag is set, the redirect happens.
>>> 
>>> Otherwise, authentication is invoked with the resolved path - this
>>> frees auth core (and all auth handlers etc) from dealing with
>>> mappings.
>>> 
>>> If authentication is successful, a new resource resolver is created
>>> and the PathMapper is passed to the resource resolver via the
>>> attributes of the resource resolver factory. ResourceResolver gets a
>>> new method "getPathMapper()" - if no PathMapper is passed to the
>>> resource resolver factory, the factory creates a new one by invoking
>>> newPathMapper(null) on the PathMapperFactory. In all cases, a 
>>> resource
>>> resolver has a PathMapper.
>>> 
>>> We add a new method to Resource Resolver, similar to the resolve()
>>> method but just doing the resolution part - no mapping. This method 
>>> is
>>> used by the main servlet instead of the existing resolve() method. We
>>> deprecate resolve() and map() and change the implementation to
>>> internally use the passed in PathMapper instance.
>>> 
>>> This way we have a clear separation between mapping and resolving and
>>> in which contexts the operations happen. With adding the PathMapper 
>>> to
>>> the resource resolver we make it available to all parts in the system
>>> which need to call todays map() method - and as the PathMapper
>>> instance knows the request, there is no need to pass it around
>>> anymore.
>>> 
>>> Now, again, just a rough write down of the idea, don't get hung up on
>>> the naming that can and needs to improve. Its also not directly about
>>> the new SPI, but rather how we make the SPI available to the rest of
>>> the system.
>>> 
>>> Regards
>>> Carsten
>>> 
>>> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Hi Georg,

at least today, the user context is not really taken into account for 
mapping. And I think everything stays nicer and cleaner if we don't open 
this up - otherwise you need to think about setting correct caching 
headers etc.
And if you still need this, you can do this differently without using 
the resource mapping functionality.

The primary concern is that authentication handlers and requirements can 
be handled correctly and without much effort. Of course we can add more 
methods to the SPI and potentially other services to allow the auth core 
to do the correct lookups, but that comes with the penalty of doing 
these things twice - first in auth core and then again with the user 
context - and hopefully both operations provide the same result (another 
reason why per user context mapping is dangerous).

Mapping is different from resolving and I think we should start treating 
it differently. If we can agree that mapping is independent of the user 
context (and today that is the case), then we can simplify things and 
make them cleaner.

Regards
Carsten

Am 24.08.2020 um 17:09 schrieb Georg Henzler:
> Hi Carsten,
> 
>> good opportunity to rethink mapping / resolving in general.
> 
> so you are referring to SLING-9622 and more specifically to comment
> [1] I suppose... I have thought it a bit through and I think there
> are valid use cases for having "the user context" (=rr) available
> for both resolve() and map(). But if it's really just about a
> knowing early (in processor) if the incoming request requires
> authentication or not, why not just add a method
> requiresAuthentication(resourceUri) to SPI at [2]? Or maybe cleaner,
> a second interface ResourceUriAuthenticationRequirementChecker could
> be introduced (that "interested" ResourceToUriMapper could also
> implement). Once vanity path and alias handling are migrated to this
> new setup, the SPI method can access the readily available data
> structure, so this should not have a performance impact... WDYT?
> 
> -Georg
> 
> 
> [1] 
> https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17182336 
> 
> [2] 
> https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L35 
> 
> 
> On 2020-08-23 11:24, Carsten Ziegeler wrote:
>> Thanks for taking this up.
>>
>> As noted in the issue, I think we should investigate whether this is a
>> good opportunity to rethink mapping / resolving in general. In
>> hindsight, we probably shouldn't have added the mapping part to the
>> resource resolver. The resolve() method does two things: it applies
>> mappings (which is independent of the current user) and it resolves to
>> a resource (finding the resource and figuring out the selectors,
>> extensions etc.) based on the user.
>>
>> So how about moving the mapping part out of the resource resolver? As
>> we've seen recently other parts of Sling like auth core could benefit
>> from this as well.
>>
>> I haven't fully thought this through, but this is the rough idea:
>>
>> We create a new OSGi service, PathMapperFactory (its probably not a
>> good name but I don't want to invest time into naming at this stage).
>> It has one method:
>>
>>    PathMapper newPathMapper(HttpServletRequest request)
>>
>> where the request parameter is optional. PathMapper has the
>> resolve/map methods doing the mapping (based on the newly suggested
>> SPI). If a request has been provided to the factory method, that
>> object is used for resolving/mapping.
>> I'm unsure about the exact methods of PathMapper, but it needs a
>> resolve method taking in a path (the request path) and returning the
>> resolved path (vanity path, aliases etc are applied) and a flag
>> whether a redirect should happen.
>>
>> Sling's main servlet (or more precise the processor) is changed: for
>> an incoming request it calls the PathMapperFactory with the request
>> and then the resolve method on PathMapper. If the result is absolut or
>> the redirect flag is set, the redirect happens.
>>
>> Otherwise, authentication is invoked with the resolved path - this
>> frees auth core (and all auth handlers etc) from dealing with
>> mappings.
>>
>> If authentication is successful, a new resource resolver is created
>> and the PathMapper is passed to the resource resolver via the
>> attributes of the resource resolver factory. ResourceResolver gets a
>> new method "getPathMapper()" - if no PathMapper is passed to the
>> resource resolver factory, the factory creates a new one by invoking
>> newPathMapper(null) on the PathMapperFactory. In all cases, a resource
>> resolver has a PathMapper.
>>
>> We add a new method to Resource Resolver, similar to the resolve()
>> method but just doing the resolution part - no mapping. This method is
>> used by the main servlet instead of the existing resolve() method. We
>> deprecate resolve() and map() and change the implementation to
>> internally use the passed in PathMapper instance.
>>
>> This way we have a clear separation between mapping and resolving and
>> in which contexts the operations happen. With adding the PathMapper to
>> the resource resolver we make it available to all parts in the system
>> which need to call todays map() method - and as the PathMapper
>> instance knows the request, there is no need to pass it around
>> anymore.
>>
>> Now, again, just a rough write down of the idea, don't get hung up on
>> the naming that can and needs to improve. Its also not directly about
>> the new SPI, but rather how we make the SPI available to the rest of
>> the system.
>>
>> Regards
>> Carsten
>>
>>

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Carsten,

> good opportunity to rethink mapping / resolving in general.

so you are referring to SLING-9622 and more specifically to comment
[1] I suppose... I have thought it a bit through and I think there
are valid use cases for having "the user context" (=rr) available
for both resolve() and map(). But if it's really just about a
knowing early (in processor) if the incoming request requires
authentication or not, why not just add a method
requiresAuthentication(resourceUri) to SPI at [2]? Or maybe cleaner,
a second interface ResourceUriAuthenticationRequirementChecker could
be introduced (that "interested" ResourceToUriMapper could also
implement). Once vanity path and alias handling are migrated to this
new setup, the SPI method can access the readily available data
structure, so this should not have a performance impact... WDYT?

-Georg


[1] 
https://issues.apache.org/jira/browse/SLING-9622?focusedCommentId=17182336&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17182336
[2] 
https://github.com/apache/sling-org-apache-sling-api/blob/3eedd71ec46e75ec93848282f7d3cb4a7ce862b5/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceToUriMapper.java#L35

On 2020-08-23 11:24, Carsten Ziegeler wrote:
> Thanks for taking this up.
> 
> As noted in the issue, I think we should investigate whether this is a
> good opportunity to rethink mapping / resolving in general. In
> hindsight, we probably shouldn't have added the mapping part to the
> resource resolver. The resolve() method does two things: it applies
> mappings (which is independent of the current user) and it resolves to
> a resource (finding the resource and figuring out the selectors,
> extensions etc.) based on the user.
> 
> So how about moving the mapping part out of the resource resolver? As
> we've seen recently other parts of Sling like auth core could benefit
> from this as well.
> 
> I haven't fully thought this through, but this is the rough idea:
> 
> We create a new OSGi service, PathMapperFactory (its probably not a
> good name but I don't want to invest time into naming at this stage).
> It has one method:
> 
>    PathMapper newPathMapper(HttpServletRequest request)
> 
> where the request parameter is optional. PathMapper has the
> resolve/map methods doing the mapping (based on the newly suggested
> SPI). If a request has been provided to the factory method, that
> object is used for resolving/mapping.
> I'm unsure about the exact methods of PathMapper, but it needs a
> resolve method taking in a path (the request path) and returning the
> resolved path (vanity path, aliases etc are applied) and a flag
> whether a redirect should happen.
> 
> Sling's main servlet (or more precise the processor) is changed: for
> an incoming request it calls the PathMapperFactory with the request
> and then the resolve method on PathMapper. If the result is absolut or
> the redirect flag is set, the redirect happens.
> 
> Otherwise, authentication is invoked with the resolved path - this
> frees auth core (and all auth handlers etc) from dealing with
> mappings.
> 
> If authentication is successful, a new resource resolver is created
> and the PathMapper is passed to the resource resolver via the
> attributes of the resource resolver factory. ResourceResolver gets a
> new method "getPathMapper()" - if no PathMapper is passed to the
> resource resolver factory, the factory creates a new one by invoking
> newPathMapper(null) on the PathMapperFactory. In all cases, a resource
> resolver has a PathMapper.
> 
> We add a new method to Resource Resolver, similar to the resolve()
> method but just doing the resolution part - no mapping. This method is
> used by the main servlet instead of the existing resolve() method. We
> deprecate resolve() and map() and change the implementation to
> internally use the passed in PathMapper instance.
> 
> This way we have a clear separation between mapping and resolving and
> in which contexts the operations happen. With adding the PathMapper to
> the resource resolver we make it available to all parts in the system
> which need to call todays map() method - and as the PathMapper
> instance knows the request, there is no need to pass it around
> anymore.
> 
> Now, again, just a rough write down of the idea, don't get hung up on
> the naming that can and needs to improve. Its also not directly about
> the new SPI, but rather how we make the SPI available to the rest of
> the system.
> 
> Regards
> Carsten
> 
> 

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Yes, backwards compatibility is important and I think if we introduce 
new methods in the ResourceResolver (deprecating the old ones), we'll 
get that easily. For now, I wouldn't introduce a different content model 
for the mapping in /etc - that could be done independently if required

Regards
Carsten

Am 24.08.2020 um 10:21 schrieb Bertrand Delacretaz:
> Hi,
> 
> On Sun, Aug 23, 2020 at 11:24 AM Carsten Ziegeler <cz...@apache.org> wrote:
> 
>> ...PathMapperFactory (its probably not a good name...
> 
> I think "rewriter" instead of "mapper" might be a good term, in the
> end it's similar to the famous mod_rewrite?
> 
>> ...So how about moving the mapping part out of the resource resolver? As
>> we've seen recently other parts of Sling like auth core could benefit
>> from this as well...
> 
> I think that makes sense but I'm not sure how we can guarantee
> backwards compatibility if we do that.
> 
> We might need to keep the existing mechanisms in parallel, but deprecate them?
> 
> The "global" configs in /etc/map might be a problem then but we could
> move that to /etc/rewrite and design the new rewriter to ignore
> /etc/map if a rewrite already happened. So that Sling users can turn
> off /etc/map after some time, once counters or logs show that it's not
> used anymore in their setup.
> 
>> ...This way we have a clear separation between mapping and resolving and
>> in which contexts the operations happen....
> 
> I like that, assuming we can cleanly take care of backwards compatibility.
> 
> -Bertrand
> 

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,

On Sun, Aug 23, 2020 at 11:24 AM Carsten Ziegeler <cz...@apache.org> wrote:

> ...PathMapperFactory (its probably not a good name...

I think "rewriter" instead of "mapper" might be a good term, in the
end it's similar to the famous mod_rewrite?

> ...So how about moving the mapping part out of the resource resolver? As
> we've seen recently other parts of Sling like auth core could benefit
> from this as well...

I think that makes sense but I'm not sure how we can guarantee
backwards compatibility if we do that.

We might need to keep the existing mechanisms in parallel, but deprecate them?

The "global" configs in /etc/map might be a problem then but we could
move that to /etc/rewrite and design the new rewriter to ignore
/etc/map if a rewrite already happened. So that Sling users can turn
off /etc/map after some time, once counters or logs show that it's not
used anymore in their setup.

> ...This way we have a clear separation between mapping and resolving and
> in which contexts the operations happen....

I like that, assuming we can cleanly take care of backwards compatibility.

-Bertrand

Re: [DISCUSS] Resource Mapping SPI

Posted by Carsten Ziegeler <cz...@apache.org>.
Thanks for taking this up.

As noted in the issue, I think we should investigate whether this is a 
good opportunity to rethink mapping / resolving in general. In 
hindsight, we probably shouldn't have added the mapping part to the 
resource resolver. The resolve() method does two things: it applies 
mappings (which is independent of the current user) and it resolves to a 
resource (finding the resource and figuring out the selectors, 
extensions etc.) based on the user.

So how about moving the mapping part out of the resource resolver? As 
we've seen recently other parts of Sling like auth core could benefit 
from this as well.

I haven't fully thought this through, but this is the rough idea:

We create a new OSGi service, PathMapperFactory (its probably not a good 
name but I don't want to invest time into naming at this stage). It has 
one method:

    PathMapper newPathMapper(HttpServletRequest request)

where the request parameter is optional. PathMapper has the resolve/map 
methods doing the mapping (based on the newly suggested SPI). If a 
request has been provided to the factory method, that object is used for 
resolving/mapping.
I'm unsure about the exact methods of PathMapper, but it needs a resolve 
method taking in a path (the request path) and returning the resolved 
path (vanity path, aliases etc are applied) and a flag whether a 
redirect should happen.

Sling's main servlet (or more precise the processor) is changed: for an 
incoming request it calls the PathMapperFactory with the request and 
then the resolve method on PathMapper. If the result is absolut or the 
redirect flag is set, the redirect happens.

Otherwise, authentication is invoked with the resolved path - this frees 
auth core (and all auth handlers etc) from dealing with mappings.

If authentication is successful, a new resource resolver is created and 
the PathMapper is passed to the resource resolver via the attributes of 
the resource resolver factory. ResourceResolver gets a new method 
"getPathMapper()" - if no PathMapper is passed to the resource resolver 
factory, the factory creates a new one by invoking newPathMapper(null) 
on the PathMapperFactory. In all cases, a resource resolver has a 
PathMapper.

We add a new method to Resource Resolver, similar to the resolve() 
method but just doing the resolution part - no mapping. This method is 
used by the main servlet instead of the existing resolve() method. We 
deprecate resolve() and map() and change the implementation to 
internally use the passed in PathMapper instance.

This way we have a clear separation between mapping and resolving and in 
which contexts the operations happen. With adding the PathMapper to the 
resource resolver we make it available to all parts in the system which 
need to call todays map() method - and as the PathMapper instance knows 
the request, there is no need to pass it around anymore.

Now, again, just a rough write down of the idea, don't get hung up on 
the naming that can and needs to improve. Its also not directly about 
the new SPI, but rather how we make the SPI available to the rest of the 
system.

Regards
Carsten

Am 14.08.2020 um 07:57 schrieb Georg Henzler:
> Hi all,
> 
> so I eventually got around to continue on this. I created JIRA
> issue [1] and the branches [2] in API and [3] in resourceresolver
> with a first working version.
> 
> 
>> I definitely prefer immutable classes with a dedicated builder
> 
> @Konrad I was critical first because the verbosity of the code,
> but it made implementing ResourceUriMappingChain actually easier.
> My concern about the verbosity was not that much a problem with
> Java 8 anymore.
> 
>> ...And I do believe that mapping is
>> about the relationship between requests and resources ...
> and
>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>> Resource to a form that can be requested.
> 
> @Julian: So these are very good points, I think RequestMapper is
> not ideal because in async code (e.g. Sling jobs), you might
> have the case that you have a resource and you map it to a URI
> (e.g. for sending an email). in rr.map() you use no
> request and usually some manual work is needed to add the host.
> My idea for the future would be to provide a simple
> ExampleRequest that can be used to provide a hostname or headers
> which will then tell the active mappers to do the right thing for
> the async, non-request use case. I ended up calling the
> SPI interface itself ResourceToUriMapper (which I think is
> best spot on for what it does).
> 
> So overall I have it all working now in the branches [2] and [3],
> currently ResourceUriMappingChain is called from
> ResourceResolverImpl.resolveInternal() and
> ResourceMapperImpl.getAllMappings(). Other aspects like Sling
> alias/vanity path handling, namespace mangling and context
> paths are still handled there but could potentially be moved
> to a ResourceToUriMapper in further steps. Also see [4]
> for some examples on what ResourceToUriMapper can look like.
> 
> Let me know what you think!
> 
> -Georg
> 
> 
> [1] https://issues.apache.org/jira/browse/SLING-9662
> [2] 
> https://github.com/apache/sling-org-apache-sling-api/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI 
> 
> [3] 
> https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI 
> 
> [4] 
> https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI/src/main/java/org/apache/sling/resourceresolver/impl/mappingchain/defaultmappers 
> 
> 
> On 2020-04-01 10:04, Georg Henzler wrote:
>> Hi Julian,
>>
>>> I'm all for disentangling the mapping implementations (/etc/map,
>>> vanity, alias etc.) and making the mapping pluggable.
>>
>> great - this is really the main purpose of this thread, to sense
>> if our community will support the move (because it'll be a bit of
>> work and for that I'd like to have an agreement on the direction
>> first)
>>
>>> Despite your suggestion to discuss naming later, Georg, I believe that
>>> naming helps structure thoughts.
>>
>> yes, we shouldn't be too strict :)
>>
>>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>>> Resource to a form that can be requested.
>>
>> or maybe UriMapper - while many use cases have a request at hand, there
>> is always the default mapping that kicks in when null is passed as
>> request (mostly relevant for for async processes like e.g. sling jobs)
>>
>>> I would rather give the Sling instance an identity (e.g.
>>> protocol, hostname and port)
>>
>> This is an interesting idea! It wouldn't have to be protocol, hostname
>> and port, just the protocol would make it a valid uri already, the
>> internal representation could be slinguri:/path/to/resource.sel.html,
>> ResourceUri.toString() could just return /path/to/resource.sel.html.
>>
>> -Georg
>>
>> On 2020-04-01 00:28, Julian Sedding wrote:
>>> Hi
>>>
>>> I'm all for disentangling the mapping implementations (/etc/map,
>>> vanity, alias etc.) and making the mapping pluggable.
>>>
>>> Over the years I have come to think that the ResourceResolver methods
>>> "resolve" and "map" are actually in the "wrong" interface, because the
>>> handle a different concern than the rest of the RR. Both "resolve" and
>>> "map" are concerned about the HttpServletRequest, which is none of the
>>> RR's business. We cannot fix this due to backwards compatibility, but
>>> maybe we can improve the current situation.
>>>
>>> Despite your suggestion to discuss naming later, Georg, I believe that
>>> naming helps structure thoughts. And I do believe that mapping is
>>> about the relationship between requests and resources. Pretty much
>>> everything in Sling is about Resources. Therefore, I suggest
>>> reflecting the request aspect in the naming. This does not
>>> fundamentally change what you are suggesting, and I fully agree on the
>>> ordered chaining of mappers.
>>>
>>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>>> Resource to a form that can be requested.
>>> - RequestUri: an abstraction that can be serialized to the string
>>> representation of a URI
>>>
>>> I'm not sure we need an external URI and a local one variant of
>>> RequestUri. I would rather give the Sling instance an identity (e.g.
>>> protocol, hostname and port) and use that in conjunction with the
>>> RequestUri in order to decide whether the string representation should
>>> be absolute or relative (i.e. if protocol, hostname and port match, it
>>> can be relative). Or maybe that identity is per HTTP request, I'm not
>>> sure yet.
>>>
>>> Just some (yet) unstructured thoughts. I hope I'm making sense.
>>>
>>> Regards
>>> Julian
>>>
>>>
>>> On Tue, Mar 31, 2020 at 11:50 PM Georg Henzler <gh...@apache.org> 
>>> wrote:
>>>>
>>>> Hi Konrad,
>>>>
>>>> yes, ResourceURI at the moment would be both for internal and external
>>>> links.
>>>>
>>>> Immutable vs. Mutable is to be discussed (both ways have their own
>>>> advantages)
>>>>
>>>> > ResourceUri resolve(ResourceURI, ...) (not useful for 
>>>> absolute/external
>>>> > URIs)
>>>> > SlingUri map(SlingUri, ...) (can be useful even to map an external 
>>>> URI)
>>>>
>>>> Because it is meant to be a pipeline of OSGi services in a row of which
>>>> any
>>>> could make the decision to switch from a plain path to a full URI IMHO
>>>> we need
>>>> to pass an interface that covers both. This approach could mean (names
>>>> are
>>>> more descriptive than a real suggestion for now):
>>>>
>>>> interface ResourceUri extends ArbitraryLink {...}
>>>> interface ResourcePath extends ArbitraryLink {...} #
>>>> RequestPathInfo+query+fragment
>>>>
>>>> The ResourceMapping interface would then look symmetric again:
>>>>
>>>> ArbitraryLink resolve(ArbitraryLink, ...)
>>>> ArbitraryLink map(ArbitraryLink, ...)
>>>>
>>>> -Georg
>>>>
>>>> On 2020-03-30 21:17, Konrad Windszus wrote:
>>>> > Hi Georg,
>>>> >
>>>> > Is the class
>>>> > 
>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java 
>>>>
>>>> > 
>>>> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java> 
>>>>
>>>> > supposed to be used for both internal as well as for external links?
>>>> > I would rather come up with two separate classes (they don't have 
>>>> much
>>>> > in common).
>>>> >
>>>> > What about a generic SlingUri class with the two specializations
>>>> > ResourceUri and AbsoluteUri (or ExternalUri)?
>>>> >
>>>> > That you would lead the following signatures in
>>>> > 
>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java 
>>>>
>>>> > 
>>>> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>: 
>>>>
>>>> >
>>>>
>>>> >
>>>> > I definitely prefer immutable classes with a dedicated builder but
>>>> > that is up for another discussion I guess,
>>>> >
>>>> > Konrad
>>>> >
>>>> >> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> 
>>>> wrote:
>>>> >>
>>>> >> Hi all,
>>>> >>
>>>> >> so to eventually follow up on what has been discussed at last year's
>>>> >> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
>>>> >> found the time to draft the proposal for a Resource Mapping SPI.
>>>> >>
>>>> >> Two important points before we start:
>>>> >> * This is not meant to replace the Rewriting Pipelines as they exist
>>>> >>  today - it just would allow some Sling setups (maybe > 70%?) to 
>>>> move
>>>> >>  to a simpler solution avoiding the overhead of the rewriter 
>>>> pipeline
>>>> >>  while obviously losing its flexibility, but for many that will
>>>> >>  be ok (I fully agree with Bertrand [3] here)
>>>> >> * Let's focus the discussion first on if we would like to go into 
>>>> this
>>>> >>  direction instead of details (like mutable vs. immutable or naming,
>>>> >>  we can have detailed discussions around it once we decide to 
>>>> move on)
>>>> >>
>>>> >> But now to the actual proposal to introduce two new interfaces to 
>>>> the
>>>> >> Sling API:
>>>> >>
>>>> >> ResourceURI:
>>>> >>
>>>> >> On Sling API level we treat links as strings (e.g. rr.map() returns
>>>> >> a String). Using Strings for links has produced many bugs in the 
>>>> past,
>>>> >> I think having an abstraction for a URI/link that is tailored for
>>>> >> Sling would help a lot (obviously plain JDK URI can be used, but
>>>> >> there is no support for selectors, suffix etc.).
>>>> >>
>>>> >> Besides being generally a good idea IMHO, ResourceURI shall be used
>>>> >> for the other interface:
>>>> >>
>>>> >> ResourceMapper:
>>>> >>
>>>> >> Allows to hook into the rr.resolve() and rr.map() by implementing
>>>> >> exactly those two methods that work on instances of ResourceURI. The
>>>> >> idea is that n OSGi service instances of ResourceMapper build a
>>>> >> pipeline of mappers that rr would run through when rr.resolve() or
>>>> >> rr.map() is called (in the opposite direction for map). The request
>>>> >> is passed in as context to both methods but may be null as it is
>>>> >> today, implementations of ResourceMapper need to be able to handle
>>>> >> null.
>>>> >>
>>>> >> The following mechanisms exist today for resource mapping in ootb
>>>> >> Sling:
>>>> >>
>>>> >> * Config "resource.resolver.mapping" in 
>>>> JcrResourceResolverFactoryImpl
>>>> >> * Mappings via /etc/map
>>>> >> * Vanity Paths
>>>> >> * sling:alias
>>>> >>
>>>> >> Those could be broken up into four implementations of ResourceMapper
>>>> >> that are configured by default.
>>>> >>
>>>> >> About backwards-compatibility/breaking existing implementations: So
>>>> >> this is a BIG CHANGE. However to keep it safe I would start with
>>>> >> exactly one ResourceMapper active that is backed by today's
>>>> >> implementation. The next step is to break it in separate resource
>>>> >> mappers (as mentioned above) and just put them in a row.
>>>> >>
>>>> >> The Resource Mapping SPI would bring the following benefits:
>>>> >>
>>>> >> * Being able to hook into the resource mapping via SPI allows
>>>> >>  for flexibility that is otherwise only possible Rewriting
>>>> >>  Pipelines - while link rewriting/checking is the only
>>>> >>  reason most projects use Rewriting Pipelines (with all its
>>>> >>  performance overhead)
>>>> >>
>>>> >> * Extending the mapping process via a well-defined API that
>>>> >>  allows to write Touring-complete code is better than working
>>>> >>  with Strings in the Transformer (also for many cases it can
>>>> >>  be cleaner and better 'unit-testable' than /etc/maps)
>>>> >>
>>>> >> * HTL there can be an option to automatically map all
>>>> >>  attributes of context uri (better performance, better for
>>>> >>  debugging if map() is called synchronously)
>>>> >>
>>>> >> * Introducing the interface ResourceURI (with a backing helper
>>>> >>  class for everybody to use) is useful in general
>>>> >>
>>>> >> See branch [4] for some first code of the proposal, in
>>>> >> particular ResourceMapping.java [5] and ResourceURI.java [6]
>>>> >>
>>>> >> -Georg
>>>> >>
>>>> >> [1] "[hackathon] externalization / resource mapping / rewriter"
>>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
>>>> >> [2] "Why get rid of the rewriter?"
>>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
>>>> >> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
>>>> >>
>>>> >> [4]
>>>> >> 
>>>> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI 
>>>>
>>>> >> [5]
>>>> >> 
>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java 
>>>>
>>>> >> [6]
>>>> >> 
>>>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java 
>>>>
>>>> >>
>>>> >>
>>>> >>
>>>> >> On 2020-01-17 10:45, Georg Henzler wrote:
>>>> >>> Hi Konrad,
>>>> >>> I think SLING-9005 is a good idea, but that is independent.
>>>> >>> My idea idea was to leave resourceResolver.map() unchanged (as 
>>>> there
>>>> >>> is a lot of code using it out there, also it makes conceptually 
>>>> sense
>>>> >>> as is). Rather I‘d like to provide an SPI that allows to customize
>>>> >>> the
>>>> >>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>>>> >>> ranked list of ResourceMapper services (default one being today’s
>>>> >>> behavior). Robert also has done some work into this direction
>>>> >>> already,
>>>> >>> I‘d like to extend on that.
>>>> >>> I‘m currently on vacation but I have it on my list for when I‘m 
>>>> back.
>>>> >>> -Georg
>>>> >>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> 
>>>> wrote:
>>>> >>>> I would like to revive this.
>>>> >>>> @Georg: IIRC you wanted to come up with a proposal for a
>>>> >>>> externalizer API. Are you working on this?
>>>> >>>> Can we start by creating a JIRA ticket?
>>>> >>>> There has been a related ticket opened today:
>>>> >>>> https://issues.apache.org/jira/browse/SLING-9005
>>>> >>>> <https://issues.apache.org/jira/browse/SLING-9005>
>>>> >>>> Konrad
>>>> >>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
>>>> >>>>> wrote:
>>>> >>>>> Specifically with regard to the re-writer
>>>> >>>>> I'm working on a replacement for this which I'm currently calling
>>>> >>>>> the transformer ->
>>>> >>>>> 
>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>>> >>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities
>>>> >>>>> to generate a stream of  elements that can be modified and
>>>> >>>>> recombined. It's much faster then tagsoup or jsoup since it's not
>>>> >>>>> trying to build a valid document. This also means that it 
>>>> supports
>>>> >>>>> anything that you write out in html. HTML4,5+
>>>> >>>>> 2. It uses services with an ordering priority and does pattern
>>>> >>>>> matching so that you can fine tune when the transformer is 
>>>> applied
>>>> >>>>> 3. The specific use case I was working on is creating a CSP 
>>>> header
>>>> >>>>> with a nonce or hash attribute to lock down the javascript that's
>>>> >>>>> on the page.
>>>> >>>>> It's currently working but it's not finished.
>>>> >>>>> Are there other problems with the rewriter that I haven't
>>>> >>>>> addressed?
>>>> >>>>> --
>>>> >>>>> Jason
>>>> >>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>> >>>>>> - resource mapping
>>>> >>>>>> - add a new SPI to define the mapping
>>>> >>>>>> - add a default impl that reads the mapping from /etc/map as 
>>>> it is
>>>> >>>>>> done today
>>>> >>>>>> - possible to override with service ranking
>>>> >>>>>> - but: there is already the ResourceMapper interface
>>>> >>>>>>  - introduced 1-2 years ago, use case was to get all existing
>>>> >>>>>> mappings
>>>> >>>>>>  - with this it's currently possible to replace mapping 
>>>> completely
>>>> >>>>>> with new logic
>>>> >>>>>> - maybe add a support to "contribute" additional mappings via a
>>>> >>>>>> service interface additional to this
>>>> >>>>>> - generic externalizer API
>>>> >>>>>> - naming not fixed yet, should not named "link" as this is too
>>>> >>>>>> specific. probably "URL".
>>>> >>>>>> - needs a java API for using, and an SPI for hooking in special
>>>> >>>>>> rules
>>>> >>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>> >>>>>>  * probably starting with a sling-only HTL extension for 
>>>> try-out,
>>>> >>>>>> add it later to the spec when design is validated
>>>> >>>>>> - might be inspired by the basic features for wcm.io URL handler
>>>> >>>>>> [1]
>>>> >>>>>> - rewriter pipeline
>>>> >>>>>> - should be deprecated and no longer be used - especially not 
>>>> for
>>>> >>>>>> link externalization via postprocessing
>>>> >>>>>> - it may still be in use out there for more exotic use cases 
>>>> like
>>>> >>>>>> PDF
>>>> >>>>>> generation
>>>> >>>>>> - should probably removed from sling starter
>>>> >>>>>> stefan
>>>> >>>>>> [1] https://wcm.io/handler/url/

-- 
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi all,

so I eventually got around to continue on this. I created JIRA
issue [1] and the branches [2] in API and [3] in resourceresolver
with a first working version.


> I definitely prefer immutable classes with a dedicated builder

@Konrad I was critical first because the verbosity of the code,
but it made implementing ResourceUriMappingChain actually easier.
My concern about the verbosity was not that much a problem with
Java 8 anymore.

> ...And I do believe that mapping is
> about the relationship between requests and resources ...
and
> - RequestMapper: it maps a "request" to a Resource OR it maps a
> Resource to a form that can be requested.

@Julian: So these are very good points, I think RequestMapper is
not ideal because in async code (e.g. Sling jobs), you might
have the case that you have a resource and you map it to a URI
(e.g. for sending an email). in rr.map() you use no
request and usually some manual work is needed to add the host.
My idea for the future would be to provide a simple
ExampleRequest that can be used to provide a hostname or headers
which will then tell the active mappers to do the right thing for
the async, non-request use case. I ended up calling the
SPI interface itself ResourceToUriMapper (which I think is
best spot on for what it does).

So overall I have it all working now in the branches [2] and [3],
currently ResourceUriMappingChain is called from
ResourceResolverImpl.resolveInternal() and
ResourceMapperImpl.getAllMappings(). Other aspects like Sling
alias/vanity path handling, namespace mangling and context
paths are still handled there but could potentially be moved
to a ResourceToUriMapper in further steps. Also see [4]
for some examples on what ResourceToUriMapper can look like.

Let me know what you think!

-Georg


[1] https://issues.apache.org/jira/browse/SLING-9662
[2] 
https://github.com/apache/sling-org-apache-sling-api/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI
[3] 
https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI
[4] 
https://github.com/apache/sling-org-apache-sling-resourceresolver/tree/feature/SLING-9662-Introduce-Resource-Mapping-SPI/src/main/java/org/apache/sling/resourceresolver/impl/mappingchain/defaultmappers

On 2020-04-01 10:04, Georg Henzler wrote:
> Hi Julian,
> 
>> I'm all for disentangling the mapping implementations (/etc/map,
>> vanity, alias etc.) and making the mapping pluggable.
> 
> great - this is really the main purpose of this thread, to sense
> if our community will support the move (because it'll be a bit of
> work and for that I'd like to have an agreement on the direction
> first)
> 
>> Despite your suggestion to discuss naming later, Georg, I believe that
>> naming helps structure thoughts.
> 
> yes, we shouldn't be too strict :)
> 
>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>> Resource to a form that can be requested.
> 
> or maybe UriMapper - while many use cases have a request at hand, there
> is always the default mapping that kicks in when null is passed as
> request (mostly relevant for for async processes like e.g. sling jobs)
> 
>> I would rather give the Sling instance an identity (e.g.
>> protocol, hostname and port)
> 
> This is an interesting idea! It wouldn't have to be protocol, hostname
> and port, just the protocol would make it a valid uri already, the
> internal representation could be slinguri:/path/to/resource.sel.html,
> ResourceUri.toString() could just return /path/to/resource.sel.html.
> 
> -Georg
> 
> On 2020-04-01 00:28, Julian Sedding wrote:
>> Hi
>> 
>> I'm all for disentangling the mapping implementations (/etc/map,
>> vanity, alias etc.) and making the mapping pluggable.
>> 
>> Over the years I have come to think that the ResourceResolver methods
>> "resolve" and "map" are actually in the "wrong" interface, because the
>> handle a different concern than the rest of the RR. Both "resolve" and
>> "map" are concerned about the HttpServletRequest, which is none of the
>> RR's business. We cannot fix this due to backwards compatibility, but
>> maybe we can improve the current situation.
>> 
>> Despite your suggestion to discuss naming later, Georg, I believe that
>> naming helps structure thoughts. And I do believe that mapping is
>> about the relationship between requests and resources. Pretty much
>> everything in Sling is about Resources. Therefore, I suggest
>> reflecting the request aspect in the naming. This does not
>> fundamentally change what you are suggesting, and I fully agree on the
>> ordered chaining of mappers.
>> 
>> - RequestMapper: it maps a "request" to a Resource OR it maps a
>> Resource to a form that can be requested.
>> - RequestUri: an abstraction that can be serialized to the string
>> representation of a URI
>> 
>> I'm not sure we need an external URI and a local one variant of
>> RequestUri. I would rather give the Sling instance an identity (e.g.
>> protocol, hostname and port) and use that in conjunction with the
>> RequestUri in order to decide whether the string representation should
>> be absolute or relative (i.e. if protocol, hostname and port match, it
>> can be relative). Or maybe that identity is per HTTP request, I'm not
>> sure yet.
>> 
>> Just some (yet) unstructured thoughts. I hope I'm making sense.
>> 
>> Regards
>> Julian
>> 
>> 
>> On Tue, Mar 31, 2020 at 11:50 PM Georg Henzler <gh...@apache.org> 
>> wrote:
>>> 
>>> Hi Konrad,
>>> 
>>> yes, ResourceURI at the moment would be both for internal and 
>>> external
>>> links.
>>> 
>>> Immutable vs. Mutable is to be discussed (both ways have their own
>>> advantages)
>>> 
>>> > ResourceUri resolve(ResourceURI, ...) (not useful for absolute/external
>>> > URIs)
>>> > SlingUri map(SlingUri, ...) (can be useful even to map an external URI)
>>> 
>>> Because it is meant to be a pipeline of OSGi services in a row of 
>>> which
>>> any
>>> could make the decision to switch from a plain path to a full URI 
>>> IMHO
>>> we need
>>> to pass an interface that covers both. This approach could mean 
>>> (names
>>> are
>>> more descriptive than a real suggestion for now):
>>> 
>>> interface ResourceUri extends ArbitraryLink {...}
>>> interface ResourcePath extends ArbitraryLink {...} #
>>> RequestPathInfo+query+fragment
>>> 
>>> The ResourceMapping interface would then look symmetric again:
>>> 
>>> ArbitraryLink resolve(ArbitraryLink, ...)
>>> ArbitraryLink map(ArbitraryLink, ...)
>>> 
>>> -Georg
>>> 
>>> On 2020-03-30 21:17, Konrad Windszus wrote:
>>> > Hi Georg,
>>> >
>>> > Is the class
>>> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
>>> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java>
>>> > supposed to be used for both internal as well as for external links?
>>> > I would rather come up with two separate classes (they don't have much
>>> > in common).
>>> >
>>> > What about a generic SlingUri class with the two specializations
>>> > ResourceUri and AbsoluteUri (or ExternalUri)?
>>> >
>>> > That you would lead the following signatures in
>>> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
>>> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>:
>>> >
>>> 
>>> >
>>> > I definitely prefer immutable classes with a dedicated builder but
>>> > that is up for another discussion I guess,
>>> >
>>> > Konrad
>>> >
>>> >> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> wrote:
>>> >>
>>> >> Hi all,
>>> >>
>>> >> so to eventually follow up on what has been discussed at last year's
>>> >> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
>>> >> found the time to draft the proposal for a Resource Mapping SPI.
>>> >>
>>> >> Two important points before we start:
>>> >> * This is not meant to replace the Rewriting Pipelines as they exist
>>> >>  today - it just would allow some Sling setups (maybe > 70%?) to move
>>> >>  to a simpler solution avoiding the overhead of the rewriter pipeline
>>> >>  while obviously losing its flexibility, but for many that will
>>> >>  be ok (I fully agree with Bertrand [3] here)
>>> >> * Let's focus the discussion first on if we would like to go into this
>>> >>  direction instead of details (like mutable vs. immutable or naming,
>>> >>  we can have detailed discussions around it once we decide to move on)
>>> >>
>>> >> But now to the actual proposal to introduce two new interfaces to the
>>> >> Sling API:
>>> >>
>>> >> ResourceURI:
>>> >>
>>> >> On Sling API level we treat links as strings (e.g. rr.map() returns
>>> >> a String). Using Strings for links has produced many bugs in the past,
>>> >> I think having an abstraction for a URI/link that is tailored for
>>> >> Sling would help a lot (obviously plain JDK URI can be used, but
>>> >> there is no support for selectors, suffix etc.).
>>> >>
>>> >> Besides being generally a good idea IMHO, ResourceURI shall be used
>>> >> for the other interface:
>>> >>
>>> >> ResourceMapper:
>>> >>
>>> >> Allows to hook into the rr.resolve() and rr.map() by implementing
>>> >> exactly those two methods that work on instances of ResourceURI. The
>>> >> idea is that n OSGi service instances of ResourceMapper build a
>>> >> pipeline of mappers that rr would run through when rr.resolve() or
>>> >> rr.map() is called (in the opposite direction for map). The request
>>> >> is passed in as context to both methods but may be null as it is
>>> >> today, implementations of ResourceMapper need to be able to handle
>>> >> null.
>>> >>
>>> >> The following mechanisms exist today for resource mapping in ootb
>>> >> Sling:
>>> >>
>>> >> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
>>> >> * Mappings via /etc/map
>>> >> * Vanity Paths
>>> >> * sling:alias
>>> >>
>>> >> Those could be broken up into four implementations of ResourceMapper
>>> >> that are configured by default.
>>> >>
>>> >> About backwards-compatibility/breaking existing implementations: So
>>> >> this is a BIG CHANGE. However to keep it safe I would start with
>>> >> exactly one ResourceMapper active that is backed by today's
>>> >> implementation. The next step is to break it in separate resource
>>> >> mappers (as mentioned above) and just put them in a row.
>>> >>
>>> >> The Resource Mapping SPI would bring the following benefits:
>>> >>
>>> >> * Being able to hook into the resource mapping via SPI allows
>>> >>  for flexibility that is otherwise only possible Rewriting
>>> >>  Pipelines - while link rewriting/checking is the only
>>> >>  reason most projects use Rewriting Pipelines (with all its
>>> >>  performance overhead)
>>> >>
>>> >> * Extending the mapping process via a well-defined API that
>>> >>  allows to write Touring-complete code is better than working
>>> >>  with Strings in the Transformer (also for many cases it can
>>> >>  be cleaner and better 'unit-testable' than /etc/maps)
>>> >>
>>> >> * HTL there can be an option to automatically map all
>>> >>  attributes of context uri (better performance, better for
>>> >>  debugging if map() is called synchronously)
>>> >>
>>> >> * Introducing the interface ResourceURI (with a backing helper
>>> >>  class for everybody to use) is useful in general
>>> >>
>>> >> See branch [4] for some first code of the proposal, in
>>> >> particular ResourceMapping.java [5] and ResourceURI.java [6]
>>> >>
>>> >> -Georg
>>> >>
>>> >> [1] "[hackathon] externalization / resource mapping / rewriter"
>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
>>> >> [2] "Why get rid of the rewriter?"
>>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
>>> >> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
>>> >>
>>> >> [4]
>>> >> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
>>> >> [5]
>>> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
>>> >> [6]
>>> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
>>> >>
>>> >>
>>> >>
>>> >> On 2020-01-17 10:45, Georg Henzler wrote:
>>> >>> Hi Konrad,
>>> >>> I think SLING-9005 is a good idea, but that is independent.
>>> >>> My idea idea was to leave resourceResolver.map() unchanged (as there
>>> >>> is a lot of code using it out there, also it makes conceptually sense
>>> >>> as is). Rather I‘d like to provide an SPI that allows to customize
>>> >>> the
>>> >>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>>> >>> ranked list of ResourceMapper services (default one being today’s
>>> >>> behavior). Robert also has done some work into this direction
>>> >>> already,
>>> >>> I‘d like to extend on that.
>>> >>> I‘m currently on vacation but I have it on my list for when I‘m back.
>>> >>> -Georg
>>> >>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>>> >>>> I would like to revive this.
>>> >>>> @Georg: IIRC you wanted to come up with a proposal for a
>>> >>>> externalizer API. Are you working on this?
>>> >>>> Can we start by creating a JIRA ticket?
>>> >>>> There has been a related ticket opened today:
>>> >>>> https://issues.apache.org/jira/browse/SLING-9005
>>> >>>> <https://issues.apache.org/jira/browse/SLING-9005>
>>> >>>> Konrad
>>> >>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
>>> >>>>> wrote:
>>> >>>>> Specifically with regard to the re-writer
>>> >>>>> I'm working on a replacement for this which I'm currently calling
>>> >>>>> the transformer ->
>>> >>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>> >>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities
>>> >>>>> to generate a stream of  elements that can be modified and
>>> >>>>> recombined. It's much faster then tagsoup or jsoup since it's not
>>> >>>>> trying to build a valid document. This also means that it supports
>>> >>>>> anything that you write out in html. HTML4,5+
>>> >>>>> 2. It uses services with an ordering priority and does pattern
>>> >>>>> matching so that you can fine tune when the transformer is applied
>>> >>>>> 3. The specific use case I was working on is creating a CSP header
>>> >>>>> with a nonce or hash attribute to lock down the javascript that's
>>> >>>>> on the page.
>>> >>>>> It's currently working but it's not finished.
>>> >>>>> Are there other problems with the rewriter that I haven't
>>> >>>>> addressed?
>>> >>>>> --
>>> >>>>> Jason
>>> >>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>> >>>>>> - resource mapping
>>> >>>>>> - add a new SPI to define the mapping
>>> >>>>>> - add a default impl that reads the mapping from /etc/map as it is
>>> >>>>>> done today
>>> >>>>>> - possible to override with service ranking
>>> >>>>>> - but: there is already the ResourceMapper interface
>>> >>>>>>  - introduced 1-2 years ago, use case was to get all existing
>>> >>>>>> mappings
>>> >>>>>>  - with this it's currently possible to replace mapping completely
>>> >>>>>> with new logic
>>> >>>>>> - maybe add a support to "contribute" additional mappings via a
>>> >>>>>> service interface additional to this
>>> >>>>>> - generic externalizer API
>>> >>>>>> - naming not fixed yet, should not named "link" as this is too
>>> >>>>>> specific. probably "URL".
>>> >>>>>> - needs a java API for using, and an SPI for hooking in special
>>> >>>>>> rules
>>> >>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>> >>>>>>  * probably starting with a sling-only HTL extension for try-out,
>>> >>>>>> add it later to the spec when design is validated
>>> >>>>>> - might be inspired by the basic features for wcm.io URL handler
>>> >>>>>> [1]
>>> >>>>>> - rewriter pipeline
>>> >>>>>> - should be deprecated and no longer be used - especially not for
>>> >>>>>> link externalization via postprocessing
>>> >>>>>> - it may still be in use out there for more exotic use cases like
>>> >>>>>> PDF
>>> >>>>>> generation
>>> >>>>>> - should probably removed from sling starter
>>> >>>>>> stefan
>>> >>>>>> [1] https://wcm.io/handler/url/

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Julian,

> I'm all for disentangling the mapping implementations (/etc/map,
> vanity, alias etc.) and making the mapping pluggable.

great - this is really the main purpose of this thread, to sense
if our community will support the move (because it'll be a bit of
work and for that I'd like to have an agreement on the direction
first)

> Despite your suggestion to discuss naming later, Georg, I believe that
> naming helps structure thoughts.

yes, we shouldn't be too strict :)

> - RequestMapper: it maps a "request" to a Resource OR it maps a
> Resource to a form that can be requested.

or maybe UriMapper - while many use cases have a request at hand, there
is always the default mapping that kicks in when null is passed as
request (mostly relevant for for async processes like e.g. sling jobs)

> I would rather give the Sling instance an identity (e.g.
> protocol, hostname and port)

This is an interesting idea! It wouldn't have to be protocol, hostname
and port, just the protocol would make it a valid uri already, the
internal representation could be slinguri:/path/to/resource.sel.html,
ResourceUri.toString() could just return /path/to/resource.sel.html.

-Georg

On 2020-04-01 00:28, Julian Sedding wrote:
> Hi
> 
> I'm all for disentangling the mapping implementations (/etc/map,
> vanity, alias etc.) and making the mapping pluggable.
> 
> Over the years I have come to think that the ResourceResolver methods
> "resolve" and "map" are actually in the "wrong" interface, because the
> handle a different concern than the rest of the RR. Both "resolve" and
> "map" are concerned about the HttpServletRequest, which is none of the
> RR's business. We cannot fix this due to backwards compatibility, but
> maybe we can improve the current situation.
> 
> Despite your suggestion to discuss naming later, Georg, I believe that
> naming helps structure thoughts. And I do believe that mapping is
> about the relationship between requests and resources. Pretty much
> everything in Sling is about Resources. Therefore, I suggest
> reflecting the request aspect in the naming. This does not
> fundamentally change what you are suggesting, and I fully agree on the
> ordered chaining of mappers.
> 
> - RequestMapper: it maps a "request" to a Resource OR it maps a
> Resource to a form that can be requested.
> - RequestUri: an abstraction that can be serialized to the string
> representation of a URI
> 
> I'm not sure we need an external URI and a local one variant of
> RequestUri. I would rather give the Sling instance an identity (e.g.
> protocol, hostname and port) and use that in conjunction with the
> RequestUri in order to decide whether the string representation should
> be absolute or relative (i.e. if protocol, hostname and port match, it
> can be relative). Or maybe that identity is per HTTP request, I'm not
> sure yet.
> 
> Just some (yet) unstructured thoughts. I hope I'm making sense.
> 
> Regards
> Julian
> 
> 
> On Tue, Mar 31, 2020 at 11:50 PM Georg Henzler <gh...@apache.org> 
> wrote:
>> 
>> Hi Konrad,
>> 
>> yes, ResourceURI at the moment would be both for internal and external
>> links.
>> 
>> Immutable vs. Mutable is to be discussed (both ways have their own
>> advantages)
>> 
>> > ResourceUri resolve(ResourceURI, ...) (not useful for absolute/external
>> > URIs)
>> > SlingUri map(SlingUri, ...) (can be useful even to map an external URI)
>> 
>> Because it is meant to be a pipeline of OSGi services in a row of 
>> which
>> any
>> could make the decision to switch from a plain path to a full URI IMHO
>> we need
>> to pass an interface that covers both. This approach could mean (names
>> are
>> more descriptive than a real suggestion for now):
>> 
>> interface ResourceUri extends ArbitraryLink {...}
>> interface ResourcePath extends ArbitraryLink {...} #
>> RequestPathInfo+query+fragment
>> 
>> The ResourceMapping interface would then look symmetric again:
>> 
>> ArbitraryLink resolve(ArbitraryLink, ...)
>> ArbitraryLink map(ArbitraryLink, ...)
>> 
>> -Georg
>> 
>> On 2020-03-30 21:17, Konrad Windszus wrote:
>> > Hi Georg,
>> >
>> > Is the class
>> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
>> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java>
>> > supposed to be used for both internal as well as for external links?
>> > I would rather come up with two separate classes (they don't have much
>> > in common).
>> >
>> > What about a generic SlingUri class with the two specializations
>> > ResourceUri and AbsoluteUri (or ExternalUri)?
>> >
>> > That you would lead the following signatures in
>> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
>> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>:
>> >
>> 
>> >
>> > I definitely prefer immutable classes with a dedicated builder but
>> > that is up for another discussion I guess,
>> >
>> > Konrad
>> >
>> >> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> wrote:
>> >>
>> >> Hi all,
>> >>
>> >> so to eventually follow up on what has been discussed at last year's
>> >> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
>> >> found the time to draft the proposal for a Resource Mapping SPI.
>> >>
>> >> Two important points before we start:
>> >> * This is not meant to replace the Rewriting Pipelines as they exist
>> >>  today - it just would allow some Sling setups (maybe > 70%?) to move
>> >>  to a simpler solution avoiding the overhead of the rewriter pipeline
>> >>  while obviously losing its flexibility, but for many that will
>> >>  be ok (I fully agree with Bertrand [3] here)
>> >> * Let's focus the discussion first on if we would like to go into this
>> >>  direction instead of details (like mutable vs. immutable or naming,
>> >>  we can have detailed discussions around it once we decide to move on)
>> >>
>> >> But now to the actual proposal to introduce two new interfaces to the
>> >> Sling API:
>> >>
>> >> ResourceURI:
>> >>
>> >> On Sling API level we treat links as strings (e.g. rr.map() returns
>> >> a String). Using Strings for links has produced many bugs in the past,
>> >> I think having an abstraction for a URI/link that is tailored for
>> >> Sling would help a lot (obviously plain JDK URI can be used, but
>> >> there is no support for selectors, suffix etc.).
>> >>
>> >> Besides being generally a good idea IMHO, ResourceURI shall be used
>> >> for the other interface:
>> >>
>> >> ResourceMapper:
>> >>
>> >> Allows to hook into the rr.resolve() and rr.map() by implementing
>> >> exactly those two methods that work on instances of ResourceURI. The
>> >> idea is that n OSGi service instances of ResourceMapper build a
>> >> pipeline of mappers that rr would run through when rr.resolve() or
>> >> rr.map() is called (in the opposite direction for map). The request
>> >> is passed in as context to both methods but may be null as it is
>> >> today, implementations of ResourceMapper need to be able to handle
>> >> null.
>> >>
>> >> The following mechanisms exist today for resource mapping in ootb
>> >> Sling:
>> >>
>> >> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
>> >> * Mappings via /etc/map
>> >> * Vanity Paths
>> >> * sling:alias
>> >>
>> >> Those could be broken up into four implementations of ResourceMapper
>> >> that are configured by default.
>> >>
>> >> About backwards-compatibility/breaking existing implementations: So
>> >> this is a BIG CHANGE. However to keep it safe I would start with
>> >> exactly one ResourceMapper active that is backed by today's
>> >> implementation. The next step is to break it in separate resource
>> >> mappers (as mentioned above) and just put them in a row.
>> >>
>> >> The Resource Mapping SPI would bring the following benefits:
>> >>
>> >> * Being able to hook into the resource mapping via SPI allows
>> >>  for flexibility that is otherwise only possible Rewriting
>> >>  Pipelines - while link rewriting/checking is the only
>> >>  reason most projects use Rewriting Pipelines (with all its
>> >>  performance overhead)
>> >>
>> >> * Extending the mapping process via a well-defined API that
>> >>  allows to write Touring-complete code is better than working
>> >>  with Strings in the Transformer (also for many cases it can
>> >>  be cleaner and better 'unit-testable' than /etc/maps)
>> >>
>> >> * HTL there can be an option to automatically map all
>> >>  attributes of context uri (better performance, better for
>> >>  debugging if map() is called synchronously)
>> >>
>> >> * Introducing the interface ResourceURI (with a backing helper
>> >>  class for everybody to use) is useful in general
>> >>
>> >> See branch [4] for some first code of the proposal, in
>> >> particular ResourceMapping.java [5] and ResourceURI.java [6]
>> >>
>> >> -Georg
>> >>
>> >> [1] "[hackathon] externalization / resource mapping / rewriter"
>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
>> >> [2] "Why get rid of the rewriter?"
>> >> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
>> >> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
>> >>
>> >> [4]
>> >> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
>> >> [5]
>> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
>> >> [6]
>> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
>> >>
>> >>
>> >>
>> >> On 2020-01-17 10:45, Georg Henzler wrote:
>> >>> Hi Konrad,
>> >>> I think SLING-9005 is a good idea, but that is independent.
>> >>> My idea idea was to leave resourceResolver.map() unchanged (as there
>> >>> is a lot of code using it out there, also it makes conceptually sense
>> >>> as is). Rather I‘d like to provide an SPI that allows to customize
>> >>> the
>> >>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>> >>> ranked list of ResourceMapper services (default one being today’s
>> >>> behavior). Robert also has done some work into this direction
>> >>> already,
>> >>> I‘d like to extend on that.
>> >>> I‘m currently on vacation but I have it on my list for when I‘m back.
>> >>> -Georg
>> >>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>> >>>> I would like to revive this.
>> >>>> @Georg: IIRC you wanted to come up with a proposal for a
>> >>>> externalizer API. Are you working on this?
>> >>>> Can we start by creating a JIRA ticket?
>> >>>> There has been a related ticket opened today:
>> >>>> https://issues.apache.org/jira/browse/SLING-9005
>> >>>> <https://issues.apache.org/jira/browse/SLING-9005>
>> >>>> Konrad
>> >>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
>> >>>>> wrote:
>> >>>>> Specifically with regard to the re-writer
>> >>>>> I'm working on a replacement for this which I'm currently calling
>> >>>>> the transformer ->
>> >>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>> >>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities
>> >>>>> to generate a stream of  elements that can be modified and
>> >>>>> recombined. It's much faster then tagsoup or jsoup since it's not
>> >>>>> trying to build a valid document. This also means that it supports
>> >>>>> anything that you write out in html. HTML4,5+
>> >>>>> 2. It uses services with an ordering priority and does pattern
>> >>>>> matching so that you can fine tune when the transformer is applied
>> >>>>> 3. The specific use case I was working on is creating a CSP header
>> >>>>> with a nonce or hash attribute to lock down the javascript that's
>> >>>>> on the page.
>> >>>>> It's currently working but it's not finished.
>> >>>>> Are there other problems with the rewriter that I haven't
>> >>>>> addressed?
>> >>>>> --
>> >>>>> Jason
>> >>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>> >>>>>> - resource mapping
>> >>>>>> - add a new SPI to define the mapping
>> >>>>>> - add a default impl that reads the mapping from /etc/map as it is
>> >>>>>> done today
>> >>>>>> - possible to override with service ranking
>> >>>>>> - but: there is already the ResourceMapper interface
>> >>>>>>  - introduced 1-2 years ago, use case was to get all existing
>> >>>>>> mappings
>> >>>>>>  - with this it's currently possible to replace mapping completely
>> >>>>>> with new logic
>> >>>>>> - maybe add a support to "contribute" additional mappings via a
>> >>>>>> service interface additional to this
>> >>>>>> - generic externalizer API
>> >>>>>> - naming not fixed yet, should not named "link" as this is too
>> >>>>>> specific. probably "URL".
>> >>>>>> - needs a java API for using, and an SPI for hooking in special
>> >>>>>> rules
>> >>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>> >>>>>>  * probably starting with a sling-only HTL extension for try-out,
>> >>>>>> add it later to the spec when design is validated
>> >>>>>> - might be inspired by the basic features for wcm.io URL handler
>> >>>>>> [1]
>> >>>>>> - rewriter pipeline
>> >>>>>> - should be deprecated and no longer be used - especially not for
>> >>>>>> link externalization via postprocessing
>> >>>>>> - it may still be in use out there for more exotic use cases like
>> >>>>>> PDF
>> >>>>>> generation
>> >>>>>> - should probably removed from sling starter
>> >>>>>> stefan
>> >>>>>> [1] https://wcm.io/handler/url/

Re: [DISCUSS] Resource Mapping SPI

Posted by Julian Sedding <js...@gmail.com>.
Hi

I'm all for disentangling the mapping implementations (/etc/map,
vanity, alias etc.) and making the mapping pluggable.

Over the years I have come to think that the ResourceResolver methods
"resolve" and "map" are actually in the "wrong" interface, because the
handle a different concern than the rest of the RR. Both "resolve" and
"map" are concerned about the HttpServletRequest, which is none of the
RR's business. We cannot fix this due to backwards compatibility, but
maybe we can improve the current situation.

Despite your suggestion to discuss naming later, Georg, I believe that
naming helps structure thoughts. And I do believe that mapping is
about the relationship between requests and resources. Pretty much
everything in Sling is about Resources. Therefore, I suggest
reflecting the request aspect in the naming. This does not
fundamentally change what you are suggesting, and I fully agree on the
ordered chaining of mappers.

- RequestMapper: it maps a "request" to a Resource OR it maps a
Resource to a form that can be requested.
- RequestUri: an abstraction that can be serialized to the string
representation of a URI

I'm not sure we need an external URI and a local one variant of
RequestUri. I would rather give the Sling instance an identity (e.g.
protocol, hostname and port) and use that in conjunction with the
RequestUri in order to decide whether the string representation should
be absolute or relative (i.e. if protocol, hostname and port match, it
can be relative). Or maybe that identity is per HTTP request, I'm not
sure yet.

Just some (yet) unstructured thoughts. I hope I'm making sense.

Regards
Julian


On Tue, Mar 31, 2020 at 11:50 PM Georg Henzler <gh...@apache.org> wrote:
>
> Hi Konrad,
>
> yes, ResourceURI at the moment would be both for internal and external
> links.
>
> Immutable vs. Mutable is to be discussed (both ways have their own
> advantages)
>
> > ResourceUri resolve(ResourceURI, ...) (not useful for absolute/external
> > URIs)
> > SlingUri map(SlingUri, ...) (can be useful even to map an external URI)
>
> Because it is meant to be a pipeline of OSGi services in a row of which
> any
> could make the decision to switch from a plain path to a full URI IMHO
> we need
> to pass an interface that covers both. This approach could mean (names
> are
> more descriptive than a real suggestion for now):
>
> interface ResourceUri extends ArbitraryLink {...}
> interface ResourcePath extends ArbitraryLink {...} #
> RequestPathInfo+query+fragment
>
> The ResourceMapping interface would then look symmetric again:
>
> ArbitraryLink resolve(ArbitraryLink, ...)
> ArbitraryLink map(ArbitraryLink, ...)
>
> -Georg
>
> On 2020-03-30 21:17, Konrad Windszus wrote:
> > Hi Georg,
> >
> > Is the class
> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java>
> > supposed to be used for both internal as well as for external links?
> > I would rather come up with two separate classes (they don't have much
> > in common).
> >
> > What about a generic SlingUri class with the two specializations
> > ResourceUri and AbsoluteUri (or ExternalUri)?
> >
> > That you would lead the following signatures in
> > https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
> > <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>:
> >
>
> >
> > I definitely prefer immutable classes with a dedicated builder but
> > that is up for another discussion I guess,
> >
> > Konrad
> >
> >> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> wrote:
> >>
> >> Hi all,
> >>
> >> so to eventually follow up on what has been discussed at last year's
> >> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
> >> found the time to draft the proposal for a Resource Mapping SPI.
> >>
> >> Two important points before we start:
> >> * This is not meant to replace the Rewriting Pipelines as they exist
> >>  today - it just would allow some Sling setups (maybe > 70%?) to move
> >>  to a simpler solution avoiding the overhead of the rewriter pipeline
> >>  while obviously losing its flexibility, but for many that will
> >>  be ok (I fully agree with Bertrand [3] here)
> >> * Let's focus the discussion first on if we would like to go into this
> >>  direction instead of details (like mutable vs. immutable or naming,
> >>  we can have detailed discussions around it once we decide to move on)
> >>
> >> But now to the actual proposal to introduce two new interfaces to the
> >> Sling API:
> >>
> >> ResourceURI:
> >>
> >> On Sling API level we treat links as strings (e.g. rr.map() returns
> >> a String). Using Strings for links has produced many bugs in the past,
> >> I think having an abstraction for a URI/link that is tailored for
> >> Sling would help a lot (obviously plain JDK URI can be used, but
> >> there is no support for selectors, suffix etc.).
> >>
> >> Besides being generally a good idea IMHO, ResourceURI shall be used
> >> for the other interface:
> >>
> >> ResourceMapper:
> >>
> >> Allows to hook into the rr.resolve() and rr.map() by implementing
> >> exactly those two methods that work on instances of ResourceURI. The
> >> idea is that n OSGi service instances of ResourceMapper build a
> >> pipeline of mappers that rr would run through when rr.resolve() or
> >> rr.map() is called (in the opposite direction for map). The request
> >> is passed in as context to both methods but may be null as it is
> >> today, implementations of ResourceMapper need to be able to handle
> >> null.
> >>
> >> The following mechanisms exist today for resource mapping in ootb
> >> Sling:
> >>
> >> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
> >> * Mappings via /etc/map
> >> * Vanity Paths
> >> * sling:alias
> >>
> >> Those could be broken up into four implementations of ResourceMapper
> >> that are configured by default.
> >>
> >> About backwards-compatibility/breaking existing implementations: So
> >> this is a BIG CHANGE. However to keep it safe I would start with
> >> exactly one ResourceMapper active that is backed by today's
> >> implementation. The next step is to break it in separate resource
> >> mappers (as mentioned above) and just put them in a row.
> >>
> >> The Resource Mapping SPI would bring the following benefits:
> >>
> >> * Being able to hook into the resource mapping via SPI allows
> >>  for flexibility that is otherwise only possible Rewriting
> >>  Pipelines - while link rewriting/checking is the only
> >>  reason most projects use Rewriting Pipelines (with all its
> >>  performance overhead)
> >>
> >> * Extending the mapping process via a well-defined API that
> >>  allows to write Touring-complete code is better than working
> >>  with Strings in the Transformer (also for many cases it can
> >>  be cleaner and better 'unit-testable' than /etc/maps)
> >>
> >> * HTL there can be an option to automatically map all
> >>  attributes of context uri (better performance, better for
> >>  debugging if map() is called synchronously)
> >>
> >> * Introducing the interface ResourceURI (with a backing helper
> >>  class for everybody to use) is useful in general
> >>
> >> See branch [4] for some first code of the proposal, in
> >> particular ResourceMapping.java [5] and ResourceURI.java [6]
> >>
> >> -Georg
> >>
> >> [1] "[hackathon] externalization / resource mapping / rewriter"
> >> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
> >> [2] "Why get rid of the rewriter?"
> >> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
> >> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
> >>
> >> [4]
> >> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
> >> [5]
> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
> >> [6]
> >> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
> >>
> >>
> >>
> >> On 2020-01-17 10:45, Georg Henzler wrote:
> >>> Hi Konrad,
> >>> I think SLING-9005 is a good idea, but that is independent.
> >>> My idea idea was to leave resourceResolver.map() unchanged (as there
> >>> is a lot of code using it out there, also it makes conceptually sense
> >>> as is). Rather I‘d like to provide an SPI that allows to customize
> >>> the
> >>> way of the behavior of resourceResolver.map() - I‘m thinking of a
> >>> ranked list of ResourceMapper services (default one being today’s
> >>> behavior). Robert also has done some work into this direction
> >>> already,
> >>> I‘d like to extend on that.
> >>> I‘m currently on vacation but I have it on my list for when I‘m back.
> >>> -Georg
> >>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
> >>>> I would like to revive this.
> >>>> @Georg: IIRC you wanted to come up with a proposal for a
> >>>> externalizer API. Are you working on this?
> >>>> Can we start by creating a JIRA ticket?
> >>>> There has been a related ticket opened today:
> >>>> https://issues.apache.org/jira/browse/SLING-9005
> >>>> <https://issues.apache.org/jira/browse/SLING-9005>
> >>>> Konrad
> >>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org>
> >>>>> wrote:
> >>>>> Specifically with regard to the re-writer
> >>>>> I'm working on a replacement for this which I'm currently calling
> >>>>> the transformer ->
> >>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
> >>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities
> >>>>> to generate a stream of  elements that can be modified and
> >>>>> recombined. It's much faster then tagsoup or jsoup since it's not
> >>>>> trying to build a valid document. This also means that it supports
> >>>>> anything that you write out in html. HTML4,5+
> >>>>> 2. It uses services with an ordering priority and does pattern
> >>>>> matching so that you can fine tune when the transformer is applied
> >>>>> 3. The specific use case I was working on is creating a CSP header
> >>>>> with a nonce or hash attribute to lock down the javascript that's
> >>>>> on the page.
> >>>>> It's currently working but it's not finished.
> >>>>> Are there other problems with the rewriter that I haven't
> >>>>> addressed?
> >>>>> --
> >>>>> Jason
> >>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
> >>>>>> - resource mapping
> >>>>>> - add a new SPI to define the mapping
> >>>>>> - add a default impl that reads the mapping from /etc/map as it is
> >>>>>> done today
> >>>>>> - possible to override with service ranking
> >>>>>> - but: there is already the ResourceMapper interface
> >>>>>>  - introduced 1-2 years ago, use case was to get all existing
> >>>>>> mappings
> >>>>>>  - with this it's currently possible to replace mapping completely
> >>>>>> with new logic
> >>>>>> - maybe add a support to "contribute" additional mappings via a
> >>>>>> service interface additional to this
> >>>>>> - generic externalizer API
> >>>>>> - naming not fixed yet, should not named "link" as this is too
> >>>>>> specific. probably "URL".
> >>>>>> - needs a java API for using, and an SPI for hooking in special
> >>>>>> rules
> >>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
> >>>>>>  * probably starting with a sling-only HTL extension for try-out,
> >>>>>> add it later to the spec when design is validated
> >>>>>> - might be inspired by the basic features for wcm.io URL handler
> >>>>>> [1]
> >>>>>> - rewriter pipeline
> >>>>>> - should be deprecated and no longer be used - especially not for
> >>>>>> link externalization via postprocessing
> >>>>>> - it may still be in use out there for more exotic use cases like
> >>>>>> PDF
> >>>>>> generation
> >>>>>> - should probably removed from sling starter
> >>>>>> stefan
> >>>>>> [1] https://wcm.io/handler/url/

Re: [DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi Konrad,

yes, ResourceURI at the moment would be both for internal and external 
links.

Immutable vs. Mutable is to be discussed (both ways have their own 
advantages)

> ResourceUri resolve(ResourceURI, ...) (not useful for absolute/external 
> URIs)
> SlingUri map(SlingUri, ...) (can be useful even to map an external URI)

Because it is meant to be a pipeline of OSGi services in a row of which 
any
could make the decision to switch from a plain path to a full URI IMHO 
we need
to pass an interface that covers both. This approach could mean (names 
are
more descriptive than a real suggestion for now):

interface ResourceUri extends ArbitraryLink {...}
interface ResourcePath extends ArbitraryLink {...} # 
RequestPathInfo+query+fragment

The ResourceMapping interface would then look symmetric again:

ArbitraryLink resolve(ArbitraryLink, ...)
ArbitraryLink map(ArbitraryLink, ...)

-Georg

On 2020-03-30 21:17, Konrad Windszus wrote:
> Hi Georg,
> 
> Is the class
> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java>
> supposed to be used for both internal as well as for external links?
> I would rather come up with two separate classes (they don't have much
> in common).
> 
> What about a generic SlingUri class with the two specializations
> ResourceUri and AbsoluteUri (or ExternalUri)?
> 
> That you would lead the following signatures in
> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
> <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>:
> 

> 
> I definitely prefer immutable classes with a dedicated builder but
> that is up for another discussion I guess,
> 
> Konrad
> 
>> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> wrote:
>> 
>> Hi all,
>> 
>> so to eventually follow up on what has been discussed at last year's
>> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
>> found the time to draft the proposal for a Resource Mapping SPI.
>> 
>> Two important points before we start:
>> * This is not meant to replace the Rewriting Pipelines as they exist
>>  today - it just would allow some Sling setups (maybe > 70%?) to move
>>  to a simpler solution avoiding the overhead of the rewriter pipeline
>>  while obviously losing its flexibility, but for many that will
>>  be ok (I fully agree with Bertrand [3] here)
>> * Let's focus the discussion first on if we would like to go into this
>>  direction instead of details (like mutable vs. immutable or naming,
>>  we can have detailed discussions around it once we decide to move on)
>> 
>> But now to the actual proposal to introduce two new interfaces to the
>> Sling API:
>> 
>> ResourceURI:
>> 
>> On Sling API level we treat links as strings (e.g. rr.map() returns
>> a String). Using Strings for links has produced many bugs in the past,
>> I think having an abstraction for a URI/link that is tailored for
>> Sling would help a lot (obviously plain JDK URI can be used, but
>> there is no support for selectors, suffix etc.).
>> 
>> Besides being generally a good idea IMHO, ResourceURI shall be used
>> for the other interface:
>> 
>> ResourceMapper:
>> 
>> Allows to hook into the rr.resolve() and rr.map() by implementing
>> exactly those two methods that work on instances of ResourceURI. The
>> idea is that n OSGi service instances of ResourceMapper build a
>> pipeline of mappers that rr would run through when rr.resolve() or
>> rr.map() is called (in the opposite direction for map). The request
>> is passed in as context to both methods but may be null as it is
>> today, implementations of ResourceMapper need to be able to handle
>> null.
>> 
>> The following mechanisms exist today for resource mapping in ootb
>> Sling:
>> 
>> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
>> * Mappings via /etc/map
>> * Vanity Paths
>> * sling:alias
>> 
>> Those could be broken up into four implementations of ResourceMapper
>> that are configured by default.
>> 
>> About backwards-compatibility/breaking existing implementations: So
>> this is a BIG CHANGE. However to keep it safe I would start with
>> exactly one ResourceMapper active that is backed by today's
>> implementation. The next step is to break it in separate resource
>> mappers (as mentioned above) and just put them in a row.
>> 
>> The Resource Mapping SPI would bring the following benefits:
>> 
>> * Being able to hook into the resource mapping via SPI allows
>>  for flexibility that is otherwise only possible Rewriting
>>  Pipelines - while link rewriting/checking is the only
>>  reason most projects use Rewriting Pipelines (with all its
>>  performance overhead)
>> 
>> * Extending the mapping process via a well-defined API that
>>  allows to write Touring-complete code is better than working
>>  with Strings in the Transformer (also for many cases it can
>>  be cleaner and better 'unit-testable' than /etc/maps)
>> 
>> * HTL there can be an option to automatically map all
>>  attributes of context uri (better performance, better for
>>  debugging if map() is called synchronously)
>> 
>> * Introducing the interface ResourceURI (with a backing helper
>>  class for everybody to use) is useful in general
>> 
>> See branch [4] for some first code of the proposal, in
>> particular ResourceMapping.java [5] and ResourceURI.java [6]
>> 
>> -Georg
>> 
>> [1] "[hackathon] externalization / resource mapping / rewriter" 
>> https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
>> [2] "Why get rid of the rewriter?" 
>> https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
>> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
>> 
>> [4] 
>> https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
>> [5] 
>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
>> [6] 
>> https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
>> 
>> 
>> 
>> On 2020-01-17 10:45, Georg Henzler wrote:
>>> Hi Konrad,
>>> I think SLING-9005 is a good idea, but that is independent.
>>> My idea idea was to leave resourceResolver.map() unchanged (as there
>>> is a lot of code using it out there, also it makes conceptually sense
>>> as is). Rather I‘d like to provide an SPI that allows to customize 
>>> the
>>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>>> ranked list of ResourceMapper services (default one being today’s
>>> behavior). Robert also has done some work into this direction 
>>> already,
>>> I‘d like to extend on that.
>>> I‘m currently on vacation but I have it on my list for when I‘m back.
>>> -Georg
>>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>>>> I would like to revive this.
>>>> @Georg: IIRC you wanted to come up with a proposal for a 
>>>> externalizer API. Are you working on this?
>>>> Can we start by creating a JIRA ticket?
>>>> There has been a related ticket opened today: 
>>>> https://issues.apache.org/jira/browse/SLING-9005 
>>>> <https://issues.apache.org/jira/browse/SLING-9005>
>>>> Konrad
>>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org> 
>>>>> wrote:
>>>>> Specifically with regard to the re-writer
>>>>> I'm working on a replacement for this which I'm currently calling 
>>>>> the transformer -> 
>>>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities 
>>>>> to generate a stream of  elements that can be modified and 
>>>>> recombined. It's much faster then tagsoup or jsoup since it's not 
>>>>> trying to build a valid document. This also means that it supports 
>>>>> anything that you write out in html. HTML4,5+
>>>>> 2. It uses services with an ordering priority and does pattern 
>>>>> matching so that you can fine tune when the transformer is applied
>>>>> 3. The specific use case I was working on is creating a CSP header 
>>>>> with a nonce or hash attribute to lock down the javascript that's 
>>>>> on the page.
>>>>> It's currently working but it's not finished.
>>>>> Are there other problems with the rewriter that I haven't 
>>>>> addressed?
>>>>> --
>>>>> Jason
>>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>>>> - resource mapping
>>>>>> - add a new SPI to define the mapping
>>>>>> - add a default impl that reads the mapping from /etc/map as it is
>>>>>> done today
>>>>>> - possible to override with service ranking
>>>>>> - but: there is already the ResourceMapper interface
>>>>>>  - introduced 1-2 years ago, use case was to get all existing
>>>>>> mappings
>>>>>>  - with this it's currently possible to replace mapping completely
>>>>>> with new logic
>>>>>> - maybe add a support to "contribute" additional mappings via a
>>>>>> service interface additional to this
>>>>>> - generic externalizer API
>>>>>> - naming not fixed yet, should not named "link" as this is too
>>>>>> specific. probably "URL".
>>>>>> - needs a java API for using, and an SPI for hooking in special 
>>>>>> rules
>>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>>>>  * probably starting with a sling-only HTL extension for try-out,
>>>>>> add it later to the spec when design is validated
>>>>>> - might be inspired by the basic features for wcm.io URL handler 
>>>>>> [1]
>>>>>> - rewriter pipeline
>>>>>> - should be deprecated and no longer be used - especially not for
>>>>>> link externalization via postprocessing
>>>>>> - it may still be in use out there for more exotic use cases like 
>>>>>> PDF
>>>>>> generation
>>>>>> - should probably removed from sling starter
>>>>>> stefan
>>>>>> [1] https://wcm.io/handler/url/

Re: [DISCUSS] Resource Mapping SPI

Posted by Konrad Windszus <ko...@gmx.de>.
Hi Georg,

Is the class https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java> supposed to be used for both internal as well as for external links?
I would rather come up with two separate classes (they don't have much in common).

What about a generic SlingUri class with the two specializations ResourceUri and AbsoluteUri (or ExternalUri)?

That you would lead the following signatures in https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java <https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java>:

ResourceUri resolve(ResourceURI, ...) (not useful for absolute/external URIs)
SlingUri map(SlingUri, ...) (can be useful even to map an external URI)

I definitely prefer immutable classes with a dedicated builder but that is up for another discussion I guess,

Konrad

> On 30. Mar 2020, at 02:13, Georg Henzler <gh...@apache.org> wrote:
> 
> Hi all,
> 
> so to eventually follow up on what has been discussed at last year's
> adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
> found the time to draft the proposal for a Resource Mapping SPI.
> 
> Two important points before we start:
> * This is not meant to replace the Rewriting Pipelines as they exist
>  today - it just would allow some Sling setups (maybe > 70%?) to move
>  to a simpler solution avoiding the overhead of the rewriter pipeline
>  while obviously losing its flexibility, but for many that will
>  be ok (I fully agree with Bertrand [3] here)
> * Let's focus the discussion first on if we would like to go into this
>  direction instead of details (like mutable vs. immutable or naming,
>  we can have detailed discussions around it once we decide to move on)
> 
> But now to the actual proposal to introduce two new interfaces to the
> Sling API:
> 
> ResourceURI:
> 
> On Sling API level we treat links as strings (e.g. rr.map() returns
> a String). Using Strings for links has produced many bugs in the past,
> I think having an abstraction for a URI/link that is tailored for
> Sling would help a lot (obviously plain JDK URI can be used, but
> there is no support for selectors, suffix etc.).
> 
> Besides being generally a good idea IMHO, ResourceURI shall be used
> for the other interface:
> 
> ResourceMapper:
> 
> Allows to hook into the rr.resolve() and rr.map() by implementing
> exactly those two methods that work on instances of ResourceURI. The
> idea is that n OSGi service instances of ResourceMapper build a
> pipeline of mappers that rr would run through when rr.resolve() or
> rr.map() is called (in the opposite direction for map). The request
> is passed in as context to both methods but may be null as it is
> today, implementations of ResourceMapper need to be able to handle
> null.
> 
> The following mechanisms exist today for resource mapping in ootb
> Sling:
> 
> * Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
> * Mappings via /etc/map
> * Vanity Paths
> * sling:alias
> 
> Those could be broken up into four implementations of ResourceMapper
> that are configured by default.
> 
> About backwards-compatibility/breaking existing implementations: So
> this is a BIG CHANGE. However to keep it safe I would start with
> exactly one ResourceMapper active that is backed by today's
> implementation. The next step is to break it in separate resource
> mappers (as mentioned above) and just put them in a row.
> 
> The Resource Mapping SPI would bring the following benefits:
> 
> * Being able to hook into the resource mapping via SPI allows
>  for flexibility that is otherwise only possible Rewriting
>  Pipelines - while link rewriting/checking is the only
>  reason most projects use Rewriting Pipelines (with all its
>  performance overhead)
> 
> * Extending the mapping process via a well-defined API that
>  allows to write Touring-complete code is better than working
>  with Strings in the Transformer (also for many cases it can
>  be cleaner and better 'unit-testable' than /etc/maps)
> 
> * HTL there can be an option to automatically map all
>  attributes of context uri (better performance, better for
>  debugging if map() is called synchronously)
> 
> * Introducing the interface ResourceURI (with a backing helper
>  class for everybody to use) is useful in general
> 
> See branch [4] for some first code of the proposal, in
> particular ResourceMapping.java [5] and ResourceURI.java [6]
> 
> -Georg
> 
> [1] "[hackathon] externalization / resource mapping / rewriter" https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
> [2] "Why get rid of the rewriter?" https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
> [3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html
> 
> [4] https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
> [5] https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
> [6] https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java
> 
> 
> 
> On 2020-01-17 10:45, Georg Henzler wrote:
>> Hi Konrad,
>> I think SLING-9005 is a good idea, but that is independent.
>> My idea idea was to leave resourceResolver.map() unchanged (as there
>> is a lot of code using it out there, also it makes conceptually sense
>> as is). Rather I‘d like to provide an SPI that allows to customize the
>> way of the behavior of resourceResolver.map() - I‘m thinking of a
>> ranked list of ResourceMapper services (default one being today’s
>> behavior). Robert also has done some work into this direction already,
>> I‘d like to extend on that.
>> I‘m currently on vacation but I have it on my list for when I‘m back.
>> -Georg
>>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>>> I would like to revive this.
>>> @Georg: IIRC you wanted to come up with a proposal for a externalizer API. Are you working on this?
>>> Can we start by creating a JIRA ticket?
>>> There has been a related ticket opened today: https://issues.apache.org/jira/browse/SLING-9005 <https://issues.apache.org/jira/browse/SLING-9005>
>>> Konrad
>>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org> wrote:
>>>> Specifically with regard to the re-writer
>>>> I'm working on a replacement for this which I'm currently calling the transformer -> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>>> 1. It uses an HTML tokenizer that was added to the HTML utilities to generate a stream of  elements that can be modified and recombined. It's much faster then tagsoup or jsoup since it's not trying to build a valid document. This also means that it supports anything that you write out in html. HTML4,5+
>>>> 2. It uses services with an ordering priority and does pattern matching so that you can fine tune when the transformer is applied
>>>> 3. The specific use case I was working on is creating a CSP header with a nonce or hash attribute to lock down the javascript that's on the page.
>>>> It's currently working but it's not finished.
>>>> Are there other problems with the rewriter that I haven't addressed?
>>>> --
>>>> Jason
>>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>>> - resource mapping
>>>>> - add a new SPI to define the mapping
>>>>> - add a default impl that reads the mapping from /etc/map as it is
>>>>> done today
>>>>> - possible to override with service ranking
>>>>> - but: there is already the ResourceMapper interface
>>>>>  - introduced 1-2 years ago, use case was to get all existing
>>>>> mappings
>>>>>  - with this it's currently possible to replace mapping completely
>>>>> with new logic
>>>>> - maybe add a support to "contribute" additional mappings via a
>>>>> service interface additional to this
>>>>> - generic externalizer API
>>>>> - naming not fixed yet, should not named "link" as this is too
>>>>> specific. probably "URL".
>>>>> - needs a java API for using, and an SPI for hooking in special rules
>>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>>>  * probably starting with a sling-only HTL extension for try-out,
>>>>> add it later to the spec when design is validated
>>>>> - might be inspired by the basic features for wcm.io URL handler [1]
>>>>> - rewriter pipeline
>>>>> - should be deprecated and no longer be used - especially not for
>>>>> link externalization via postprocessing
>>>>> - it may still be in use out there for more exotic use cases like PDF
>>>>> generation
>>>>> - should probably removed from sling starter
>>>>> stefan
>>>>> [1] https://wcm.io/handler/url/


[DISCUSS] Resource Mapping SPI

Posted by Georg Henzler <gh...@apache.org>.
Hi all,

so to eventually follow up on what has been discussed at last year's
adaptTo Hackathon and in the mailing list afterwards [1]/[2] - I now
found the time to draft the proposal for a Resource Mapping SPI.

Two important points before we start:
* This is not meant to replace the Rewriting Pipelines as they exist
   today - it just would allow some Sling setups (maybe > 70%?) to move
   to a simpler solution avoiding the overhead of the rewriter pipeline
   while obviously losing its flexibility, but for many that will
   be ok (I fully agree with Bertrand [3] here)
* Let's focus the discussion first on if we would like to go into this
   direction instead of details (like mutable vs. immutable or naming,
   we can have detailed discussions around it once we decide to move on)

But now to the actual proposal to introduce two new interfaces to the
Sling API:

ResourceURI:

On Sling API level we treat links as strings (e.g. rr.map() returns
a String). Using Strings for links has produced many bugs in the past,
I think having an abstraction for a URI/link that is tailored for
Sling would help a lot (obviously plain JDK URI can be used, but
there is no support for selectors, suffix etc.).

Besides being generally a good idea IMHO, ResourceURI shall be used
for the other interface:

ResourceMapper:

Allows to hook into the rr.resolve() and rr.map() by implementing
exactly those two methods that work on instances of ResourceURI. The
idea is that n OSGi service instances of ResourceMapper build a
pipeline of mappers that rr would run through when rr.resolve() or
rr.map() is called (in the opposite direction for map). The request
is passed in as context to both methods but may be null as it is
today, implementations of ResourceMapper need to be able to handle
null.

The following mechanisms exist today for resource mapping in ootb
Sling:

* Config "resource.resolver.mapping" in JcrResourceResolverFactoryImpl
* Mappings via /etc/map
* Vanity Paths
* sling:alias

Those could be broken up into four implementations of ResourceMapper
that are configured by default.

About backwards-compatibility/breaking existing implementations: So
this is a BIG CHANGE. However to keep it safe I would start with
exactly one ResourceMapper active that is backed by today's
implementation. The next step is to break it in separate resource
mappers (as mentioned above) and just put them in a row.

The Resource Mapping SPI would bring the following benefits:

* Being able to hook into the resource mapping via SPI allows
   for flexibility that is otherwise only possible Rewriting
   Pipelines - while link rewriting/checking is the only
   reason most projects use Rewriting Pipelines (with all its
   performance overhead)

* Extending the mapping process via a well-defined API that
   allows to write Touring-complete code is better than working
   with Strings in the Transformer (also for many cases it can
   be cleaner and better 'unit-testable' than /etc/maps)

* HTL there can be an option to automatically map all
   attributes of context uri (better performance, better for
   debugging if map() is called synchronously)

* Introducing the interface ResourceURI (with a backing helper
   class for everybody to use) is useful in general

See branch [4] for some first code of the proposal, in
particular ResourceMapping.java [5] and ResourceURI.java [6]

-Georg

[1] "[hackathon] externalization / resource mapping / rewriter" 
https://www.mail-archive.com/dev@sling.apache.org/msg87736.html
[2] "Why get rid of the rewriter?" 
https://www.mail-archive.com/dev@sling.apache.org/msg87867.html
[3] https://www.mail-archive.com/dev@sling.apache.org/msg87855.html

[4] 
https://github.com/apache/sling-org-apache-sling-api/compare/feature/Resource_Mapping_SPI
[5] 
https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/mapping/spi/ResourceMapping.java
[6] 
https://github.com/apache/sling-org-apache-sling-api/blob/7dd23418280fcef234b42dbabf6e13e43ef2d4d0/src/main/java/org/apache/sling/api/resource/uri/ResourceURI.java



On 2020-01-17 10:45, Georg Henzler wrote:
> Hi Konrad,
> 
> I think SLING-9005 is a good idea, but that is independent.
> 
> My idea idea was to leave resourceResolver.map() unchanged (as there
> is a lot of code using it out there, also it makes conceptually sense
> as is). Rather I‘d like to provide an SPI that allows to customize the
> way of the behavior of resourceResolver.map() - I‘m thinking of a
> ranked list of ResourceMapper services (default one being today’s
> behavior). Robert also has done some work into this direction already,
> I‘d like to extend on that.
> 
> I‘m currently on vacation but I have it on my list for when I‘m back.
> 
> -Georg
> 
>> 
>> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
>> 
>> I would like to revive this.
>> 
>> @Georg: IIRC you wanted to come up with a proposal for a externalizer 
>> API. Are you working on this?
>> Can we start by creating a JIRA ticket?
>> 
>> There has been a related ticket opened today: 
>> https://issues.apache.org/jira/browse/SLING-9005 
>> <https://issues.apache.org/jira/browse/SLING-9005>
>> 
>> Konrad
>> 
>>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org> 
>>> wrote:
>>> 
>>> Specifically with regard to the re-writer
>>> 
>>> I'm working on a replacement for this which I'm currently calling the 
>>> transformer -> 
>>> https://github.com/apache/sling-whiteboard/tree/master/transformer
>>> 
>>> 1. It uses an HTML tokenizer that was added to the HTML utilities to 
>>> generate a stream of  elements that can be modified and recombined. 
>>> It's much faster then tagsoup or jsoup since it's not trying to build 
>>> a valid document. This also means that it supports anything that you 
>>> write out in html. HTML4,5+
>>> 
>>> 2. It uses services with an ordering priority and does pattern 
>>> matching so that you can fine tune when the transformer is applied
>>> 
>>> 3. The specific use case I was working on is creating a CSP header 
>>> with a nonce or hash attribute to lock down the javascript that's on 
>>> the page.
>>> 
>>> It's currently working but it's not finished.
>>> Are there other problems with the rewriter that I haven't addressed?
>>> 
>>> 
>>> --
>>> Jason
>>> 
>>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>>> 
>>>> - resource mapping
>>>> - add a new SPI to define the mapping
>>>> - add a default impl that reads the mapping from /etc/map as it is
>>>> done today
>>>> - possible to override with service ranking
>>>> - but: there is already the ResourceMapper interface
>>>>   - introduced 1-2 years ago, use case was to get all existing
>>>> mappings
>>>>   - with this it's currently possible to replace mapping completely
>>>> with new logic
>>>> - maybe add a support to "contribute" additional mappings via a
>>>> service interface additional to this
>>>> 
>>>> - generic externalizer API
>>>> - naming not fixed yet, should not named "link" as this is too
>>>> specific. probably "URL".
>>>> - needs a java API for using, and an SPI for hooking in special 
>>>> rules
>>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>>   * probably starting with a sling-only HTL extension for try-out,
>>>> add it later to the spec when design is validated
>>>> - might be inspired by the basic features for wcm.io URL handler [1]
>>>> 
>>>> - rewriter pipeline
>>>> - should be deprecated and no longer be used - especially not for
>>>> link externalization via postprocessing
>>>> - it may still be in use out there for more exotic use cases like 
>>>> PDF
>>>> generation
>>>> - should probably removed from sling starter
>>>> 
>>>> stefan
>>>> 
>>>> [1] https://wcm.io/handler/url/
>>>> 
>>>> 
>>>> 
>> 

Re: [hackathon] externalization / resource mapping / rewriter

Posted by Georg Henzler <sl...@ghenzler.de>.
Hi Konrad, 

I think SLING-9005 is a good idea, but that is independent.

My idea idea was to leave resourceResolver.map() unchanged (as there is a lot of code using it out there, also it makes conceptually sense as is). Rather I‘d like to provide an SPI that allows to customize the way of the behavior of resourceResolver.map() - I‘m thinking of a ranked list of ResourceMapper services (default one being today’s behavior). Robert also has done some work into this direction already, I‘d like to extend on that.

I‘m currently on vacation but I have it on my list for when I‘m back.

-Georg 

> 
> On 16. Jan 2020, at 15:01, Konrad Windszus <ko...@gmx.de> wrote:
> 
> I would like to revive this. 
> 
> @Georg: IIRC you wanted to come up with a proposal for a externalizer API. Are you working on this?
> Can we start by creating a JIRA ticket?
> 
> There has been a related ticket opened today: https://issues.apache.org/jira/browse/SLING-9005 <https://issues.apache.org/jira/browse/SLING-9005>
> 
> Konrad
> 
>> On 5. Sep 2019, at 17:54, Jason E Bailey <ja...@24601.org> wrote:
>> 
>> Specifically with regard to the re-writer
>> 
>> I'm working on a replacement for this which I'm currently calling the transformer -> https://github.com/apache/sling-whiteboard/tree/master/transformer
>> 
>> 1. It uses an HTML tokenizer that was added to the HTML utilities to generate a stream of  elements that can be modified and recombined. It's much faster then tagsoup or jsoup since it's not trying to build a valid document. This also means that it supports anything that you write out in html. HTML4,5+
>> 
>> 2. It uses services with an ordering priority and does pattern matching so that you can fine tune when the transformer is applied
>> 
>> 3. The specific use case I was working on is creating a CSP header with a nonce or hash attribute to lock down the javascript that's on the page. 
>> 
>> It's currently working but it's not finished.
>> Are there other problems with the rewriter that I haven't addressed?
>> 
>> 
>> --
>> Jason
>> 
>>> On Thu, Sep 5, 2019, at 10:34 AM, Stefan Seifert wrote:
>>> 
>>> - resource mapping
>>> - add a new SPI to define the mapping
>>> - add a default impl that reads the mapping from /etc/map as it is 
>>> done today
>>> - possible to override with service ranking
>>> - but: there is already the ResourceMapper interface
>>>   - introduced 1-2 years ago, use case was to get all existing 
>>> mappings
>>>   - with this it's currently possible to replace mapping completely 
>>> with new logic
>>> - maybe add a support to "contribute" additional mappings via a 
>>> service interface additional to this
>>> 
>>> - generic externalizer API
>>> - naming not fixed yet, should not named "link" as this is too 
>>> specific. probably "URL".
>>> - needs a java API for using, and an SPI for hooking in special rules
>>> - then add mappings for views in HTL*, JSP, model exporter/JSON
>>>   * probably starting with a sling-only HTL extension for try-out, 
>>> add it later to the spec when design is validated
>>> - might be inspired by the basic features for wcm.io URL handler [1]
>>> 
>>> - rewriter pipeline
>>> - should be deprecated and no longer be used - especially not for 
>>> link externalization via postprocessing
>>> - it may still be in use out there for more exotic use cases like PDF 
>>> generation
>>> - should probably removed from sling starter
>>> 
>>> stefan
>>> 
>>> [1] https://wcm.io/handler/url/
>>> 
>>> 
>>> 
>