Hi Stefan, thanks for the feedback, more inline... > > ResourceProvider.java > --------------------- > > 1. we should rename the package to something like > o.a.sling.spi.resource.provider to distinguish clearly from the API itself > (same may apply to other classes not related to the resource provider, e.g. > ResourceDecorator)
Sounds fine to me. > > 2. property provider.required: does this replace the property for required > resource providers in ResourceResolverFactoryActivator? what happens if the > required resource provider is not deployed/started at all, in this case the > resource resolver starts anyway? No, this does not replace it - the current ResourceProvider interface has the same property. If this flag is set to true and the provider is available, authenticate will be called on the provider, and only if this succeeds for all marked providers, the login is considered successful. If it fails on one of them, the login is failed. (The prop on ResourceResolverFactoryActivator means that these providers need to be registered in order to have resource resolver). > > 3. maybe we should add methods for copy/move resources as well? the > underlying respositories have good support for it (JCR, oak). > I just stumbled on some code lines in org.apache.sling.discovery.impl which > only falls back to the JCR API to move a resource and > otherwise would be independent of JCR. this has to be added in the ResourceResolver API as well then. this could be optional so that > not all resource providers have to implement it. Ok, this would mean the following I think: 1. move/copy on the resource resolver 2. optional move/copy on a provider 3. if the resource resolver impl detects that a move is within a provider, it calls the move/copy on the provider 4. if the move/copy is across providers, the resource resolver impl does this brute force. > > 4. is it good to have only an abstract class? this makes mocking more > difficult. I would prefer to have an interface plus an abstract class. > if one decides not to use the abstract class he has the risk to get problems on additions, but the interface is clean. Having both will lead to the problem that once we add stuff to the interface, a provider might not work (resolve) anymore. You'll find the approach with the abstract class in some OSGi specs and it seems to work pretty well. I'm not an expert, but I think mocking of abstract classes is not a problem. > > 5. for what are the getAttribute/getAttributeNames properties? do they return > OSGi properties? makes this sense? or what else is returned for what purpose? This is already part of the current provider interface. Providers can provide attributes which are available through the respective resource resolver calls. > > 6. for what use is the adaptTo method with the special signature including > the ResolveContext? who should adapt ResourceProvider SPI instances? The resource resolver impl will call this, when adaptTo is called on the resource resolver. So, e.g. the jcr provider can implement adaptTo(Session.class) through this. > > > ResolveContext.java > ------------------- > > 7. add support for a "getParentResourceResolver" for providers that overlay > other ones like superimposing content provider, as discussed in the list Yepp. > > 8. for what is the <T> used which is returned as getProviderState? That's the object returned by the authenticate method on the provider. > > 9. getParameters() - shouldn't this be a <String,Object> map like most maps > in sling? Maybe this is badly named :) This is the parameter support for resolving resources which has been added some months ago. And that map is a string to string map. Regards Carsten > > > stefan > > > >> -----Original Message----- >> From: Carsten Ziegeler [mailto:[email protected]] >> Sent: Monday, May 18, 2015 8:17 AM >> To: Sling Developers >> Subject: [RT] New Resource Provider API >> >> The resource provider API has grown a lot over time and when we started >> with it we didn't really think about potential extensions of the api. >> Today, each time we add a new feature, we come up with a new marker >> interface. There is also the distinction between a resource provider >> (singleton/stateless) and the factory (creating stateful providers). >> Although the api is not intended to be used by the average resource api >> user (it's an extension), we put it in the same package. And there are >> more minor things. >> >> Therefore I think it's time to start a new API that is more future proof >> and solves the known problems. I've created a draft prototype at [1]. >> >> During the performance analysis by Joel he found out that getParent >> calls to a resource a pretty expensive as in the end these are string >> based. Therefore, e.g. the JCR implementation can't simply call >> getParent on a node and wrap it in a resource. Therefore I think we >> should add a getParent(Resource) method to the resource resolver and >> have a better way to handle this in a resource provider. >> >> Instead of having a resource provider and a resource provider factory, >> we define a single ResourceProvider which is a singleton. If this >> provider needs authentication and/or needs to keep state per user, the >> PROPERTY_AUTHENTICATE needs to be set to true and in this case the >> authenticate method is called. This one returns a data object which is >> passed in to each and every method. If auth is not required, the method >> is not called and null is passed in as the data object. >> For authentication, providers do not support login administrative >> anymore, just users and service users. >> >> A provider is mounted at a single root - no more support for mounting it >> at different path at the same time; and a provider always owns the root. >> So if a provider does not return a resource for a given path, no other >> provider is asked. This allows for improved implementations and resource >> resolving. If we decided that we need this for compatibility we can >> solve it differently. >> >> Instead of using marker interface, we define the ResourceProvider as an >> abstract class. This allows us to add new methods without breaking >> existing providers. >> >> Each method gets a ResolveContext, containing the resource resolver, >> the previously mentioned state data object and other things, e.g. the >> parameter support recently added to the resource resolving. In the >> future we can pass in additional data without breaking the interface. >> >> Apart from that the resource provider is similar to the aggregation of >> the already existing marker interfaces. There are two exceptions, >> observation and query which I'll handle in different emails. >> >> [1] >> https://svn.apache.org/repos/asf/sling/whiteboard/cziegeler/api- >> v3/src/main/java/org/apache/sling/api/resource/provider/ >> >> Regards >> Carsten >> -- >> Carsten Ziegeler >> Adobe Research Switzerland >> [email protected] -- Carsten Ziegeler Adobe Research Switzerland [email protected]
