Ok, let's please not put all different topics into a single mail and lets
stick to the facts.

As noted in the issues about supporting the ModifyingResourceProvider - I
reverted this and added a simple path utility method which gives full
control to the application and does no modifications. So there is zero
modification logic in the bundle.

It is not correct, that we added a merge() method which "completely misses
the point". The service has just path handling methods, everything else is
done consistently in the resource provider. And it is done exactly as has
been specified / requested in the original issue.

I think adding a merge() method which allows to create resources that are
not available via a resource resolver will create all kinds of problems.
Even worse, it allows you to create a resource at any arbitrary path like
/libs/hello/world and pass this on to other services, totally confusing the
whole system.
Such a resource is never available via the typical resource resolution - so
all the concepts of Sling like the REST api for getting resources and
manipulating them does absolutely not apply. Or even traversing based on
such a resource is not possible.

Or in short, such a service method is bypassing all of Sling's concepts.

For the real use cases mentioned so far, the resource provider approach
works perfectly - especially as this is using Sling concepts and not adding
additional logic on top. The idea with supporting the
ModifyingResourceProvider might be a little bit over the top and is now
discarded anyway.

If an application really wants to create fake resources for whatever
reason, the application is free to provide such a service. But I'm
reluctant to add anything like that to Sling.

Carsten




2014-02-28 23:56 GMT+01:00 Gilles Knobloch <gilles.knobl...@gmail.com>:

> Hi,
>
> I would also +1 for having a method that does:
>   Resource merge(String... paths);
>
> Since I'm using it, I already discovered some use cases where I'd like to
> be able to have it, like shared content between two applications, one being
> the framework on which the other is based, where you want to be able to
> merge content without having to redefine everything in the downstream
> application.
> This method would give the flexibility to merge any resources.
>
> The resource resolver should then be based on the service by calling the
> merge method with the list of paths resolved by the resource resolver.
>
> Regards,
> Gilles
>
>
> 2014-02-28 23:05 GMT+01:00 Alexander Klimetschek <aklim...@adobe.com>:
>
> > I created https://issues.apache.org/jira/browse/SLING-3423
> >
> > Cheers,
> > Alex
> >
> > On 28.02.2014, at 13:54, Alexander Klimetschek <aklim...@adobe.com>
> wrote:
> >
> > > Hi,
> > >
> > > looking at SLING-3420 [1] I noticed that the ResourceMergerService [2]
> > fails to follow the intended design, and explains some of the
> > misunderstandings around the /mnt/overlay servlet discussion and now the
> > modifying resource merger discussion.
> > >
> > > The ResourceMergerService must have this one central API, where the
> > caller = application defines the merge paths!
> > >
> > >    Resource merge(ResourceResolver resolver, String mergeRootPath,
> > String relativePath, String... mergePaths)
> > >
> > > Example values for the use case of the overlay resource provider:
> > >    resolver = based on the current request (user)
> > >    mergeRootPath = /mnt/overlay
> > >    relativePath = <dynamic part>, for example:
> > "projectX/content/ui/toolbar"
> > >    mergePaths = /apps, /libs
> > >
> > > This was in the original patch [3], sadly removed and later added back
> > but completely missing the point.
> > >
> > > Currently it is all tied to the sling search path internally, but this
> > would just be ONE of different options the application might want to
> chose
> > for it.
> > >
> > > The specific /mnt/overlay ResourcerProvider would take the sling search
> > path and expose resources based on calling the merger service with this
> as
> > the mergePaths. (Currently there is a single implementation
> > MergedResourceProviderFactory for both ResourceMergerService and the
> > ResourceProvider service).
> > >
> > > This is simply a proper separation of concerns.
> > >
> > > Another application with different merge paths would be to merge
> > resource type hierarchies: use the resource type and its super types.
> This
> > could allow one to put UI dialogs in the resource types (say at ./dialog)
> > and then be able to extend dialogs in sub resource types using the merger
> > service.
> > >
> > > Now to the writeable discussion: the ResourceMergerService must be
> > read-only. It can never make a decision where to write. Given that the
> > merge path is a list with N entries and could be anything, the
> application
> > only knows where to write. A logic such as "if it's not in /libs, save to
> > /apps" doesn't work. If there is another search path entry (and sling
> apps
> > have more), how do you know to use /apps1 or /apps2? Or if you use custom
> > merge paths such as resource type hierarchy - there is no single answer.
> > And sometimes you want to overwrite something at /libs. Only the
> > application or the app developer can know the intended target.
> > >
> > > Actually, the design was for UI definitions or other developer created
> > content only. Thus there is usually no application code that would write
> to
> > it, it would be handcrafted by a framework developer (libs) or an
> > application developer (apps or wherever necessary). Maybe an advanded UI
> > that shows you the inheritance and where things come from and can be
> > stored, and the user can pick where to write it. This might need
> additions
> > to the service, but that's not clear yet, unless someone starts to build
> > something like that.
> > >
> > > Sorry, I missed to review that earlier when the original patch was
> > submitted, but I didn't find the time and assumed it was the above way.
> But
> > this needs to be fixed :)
> > >
> > > [1] https://issues.apache.org/jira/browse/SLING-3420
> > > [2]
> >
> http://svn.apache.org/repos/asf/sling/trunk/contrib/extensions/resourcemerger/src/main/java/org/apache/sling/resourcemerger/api/ResourceMergerService.java
> > > [3]
> >
> https://github.com/gknob/sling-resourcemerger/commit/a3e1b78c87e54cd5c32a58627a4de0420229e1f9
> > >
> > > Cheers,
> > > Alex
> >
> >
>



-- 
Carsten Ziegeler
cziege...@apache.org

Reply via email to