I like the proposal and definitely agree with the idea of deprecating Resource.getResourceSuperType(), it seems confusing and I've never seen it actually used.
Just to confirm, regarding the flat list comment, you just mean that the list of components can be cached in a list structure, not that the hierarchy has to be flat, correct? Also, would it make sense to designate other structures like this for other parts of Sling applications? E.g. a dedicated folder for OSGi Configurations, installed bundles and application content? That would similarly reduce the amount of resources services would have to traverse and monitor for updates and would allow for much easier management of permissions. On Fri, Mar 18, 2016 at 5:18 AM, Oliver Lietz <apa...@oliverlietz.de> wrote: > On Friday 18 March 2016 10:03:17 Carsten Ziegeler wrote: > > Oliver Lietz wrote > > > > > On Friday 18 March 2016 08:08:55 Carsten Ziegeler wrote: > > >> Hi, > > > > > > Hi Carsten, > > > > > >> in the past years we added a lot of things here and there which over > > >> time resulted in cruft and sometimes hard to understand / debug > > >> problems. In order to have an even better framework which is easier to > > >> manage but also scales better for the things to come, we should get > rid > > >> of this additional weigth. > > >> > > >> One area which I think needs attention is the resource type handling. > > >> We started years back with the two methods on a resource, getting a > > >> resource type and optionally getting a resource super type. > > >> A little bit later this was extended with what we call today the > > >> resource type hierarchy: for a resource type a/b, a resource can be > > >> created in /libs/a/b with a resource super type property. This can be > > >> overlayed in /apps. (I'm assuming the default search paths here to > keep > > >> it simple). > > >> > > >> While this sounds nice and and simple, it has several drawbacks (in no > > >> particular order): > > >> - it's hard to find out all the super types of a resource type > > >> - the resource resolver which reads the type hierarchy needs read > access > > >> to the complete tree of /libs and /apps - just to find a few > properties > > >> here and there. > > >> - two resources with the same resource type, might have different > > >> resource super types or even a complete different hierarchy. > > >> - there is no guarantee that the resource super type is static, which > > >> means two calls to find the super type for a resource type could > result > > >> in different results, e.g. if the resource itself decides to return a > > >> different value, or if a resource decorator is used which does so. > > >> - for traversing the hierarchy we currently create an additional admin > > >> resource resolver per resource resolver - which is a huge overhead. > > >> > > >> Especially the last three points make improving the script resolution > > >> hard - if not impossible. Today the cache is using the resource type > and > > >> resource super type of the resource as a key. However, given the above > > >> information it is easy to create a scenario where the cache returns a > > >> wrong script. > > >> > > >> Taking all these things into account, I think we should somehow start > > >> fresh with handling the resource type hierarchy and come to a better > > >> solution. In this first step, I think we should not care about > > >> compatibility. Once we have a great solution, we'll find a way on how > to > > >> achieve this. > > >> > > >> My rough initial proposal would be: > > >> a) forget about Resource#getResourceSuperType. This method is ignored > > >> from now on (and deprecated) > > >> b) the resource hierarchy is not spread across /libs, /apps anymore > > >> c) we create a dedicated resource tree at /foo (I'm using /foo as a > > >> placeholder as I don't want to start an endless discussion now on what > > >> this path actually really is). We then have /foo/apps and /foo/libs. > > >> Underneath each of these resources, there is a flat list of resources. > > >> Each resource is associated with a resource type (through the name of > > >> the resource) and has two properties: resource type and resource super > > >> type. d) we create a service which has three methods > > >> > > >> // get the super type > > >> String getResourceSuperType(ResourceResolver resolver, String > > >> resourceType); // get an ordered list of all super types > > >> List<String> getResourceSuperTypes(ResourceResolver resolver, String > > >> resourceType) > > >> // check if resourceType is checkResourceType (either directly or one > of > > >> the parents) > > >> boolean isResourceType(ResourceResolver resolver, String resourceType, > > >> String checkResourceType) > > >> > > >> We pass in the resource resolver to this service in order to pass in > the > > >> search paths and the current user rights. This should allow us to > > >> continue with this service, even if we add tenant support at a later > > >> time or do some other checks. > > >> > > >> With this approach we have > > >> a) a clear resource type hierarchy which is stable. A resource of > type A > > >> has always the same resource super types. (unless the hierarchy is > > >> changed of course) > > >> b) the service reading the hierarchy only needs read access in a > single > > >> place > > >> c) it's easy to find out the hierarchy by looking into the resource > tree > > >> d) the hierarchy can easily be cached in memory > > >> > > >> WDYT? > > > > > > I don't get c): "flat list of resources" and "find out the hierarchy by > > > looking into the resource tree". Can you give an example? > > > > Sure, assume you have a resource of type a/b - currently finding out all > > super types requires you too look at /apps/a/b, see if there is a > resource > > super type property, do the same for /libs. Then use the found value and > > repeat > > the process until there is no such resource with a resource super type > > property. > > This can take a lot of time (sure you can write a script to do it) > > > > Now with a flat list at /foo/libs you'll find a /foo/libs/a:b resource > > (we have to find a mapping from path to name, but that should be > > doable), can look at the property - stay in the same place and do the > > next look up. > > It gets a little bit complicated as you have to look at /foo/apps and > > /foo/libs - but these are just two places compared to N. > > "(we have to find a mapping from path to name, but that should be > doable)" – that was the missing piece, thanks. > > O. > > > Regards > > Carsten > >
