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
cziege...@apache.org
