I personally prefer the second approach. The only thing I'm not sure about is if we want to define OakConversionService with such a wildcard method. Assuming that OakConversionService will be called from code running on top of the JCR API, we could provide instead more specific conversion methods. For example,
URI toURI(javax.jcr.Binary binary); What do you think about it? Is it too restrictive? Do we need a wildcard method like currently defined in OakConversionsService? Moreover, I would leave PrivateURI out of the picture for the moment since it's not clear from the patch how this is supposed to be used. In fact, a comment in S3Backend explicitly states that is not supported at this time. Finally, I suspect that in the second patch there was too much of an aggressive rename refactoring. "types" was renamed to "customtypes" in a lot of unrelated places. I would definitely double-check that. On Tue, Sep 5, 2017 at 2:09 PM, Ian Boston <[email protected]> wrote: > Hi, > > Repeating the comment to on OAK-6575 here for further discussion. 2 new > Patches exploring both options. > > https://github.com/ieb/jackrabbit-oak/compare/trunk...ieb:OAK-6575-1?expand=1 > > This drops the OSGi AdapterManager/AdapterFactory in favour of a non OSGi > static pattern. Implementations of the AdapterFactory self register rather > than rely on OSGi doing the wiring. This is probably an IoC anti pattern, > but does avoid exposing the AdapterFactory/AdapterManager outside Oak. > > https://github.com/ieb/jackrabbit-oak/compare/trunk...ieb:OAK-6575-2?expand=1 > > This drops the AdapterManager concept completely and attempts to get from > Value to URI using mix in interfaces and instanceof. I cant be certain it > manages to do this as there is a disconnect between Blob, Blobstore and > DataStore implementations with no guarantee that a BlobStore as seen by the > Blob implementation actually implements DataStore, or the Blob that is > exposed in the JCR Value (implemented by OakValue) actually connects to the > correct DataStore of it it connects to a FileDatastore cache on local disk. > I could only wire this as far as I did with API changes. I may have broken > some of the new multi node store and multi datastore code used for 0DT in > the process. An Oak committer with global knowledge will probably be able > to do better. > > > > On 5 September 2017 at 08:19, Ian Boston <[email protected]> wrote: > >> Hi, >> >> On 5 September 2017 at 07:55, Francesco Mari <[email protected]> >> wrote: >> >>> On Mon, Sep 4, 2017 at 6:18 PM, Ian Boston <[email protected]> wrote: >>> > Do you mean: >>> > keep the OakConversionService but put all the logic to convert from a >>> > Value to a URI inside that implementation using new Oak SPI/APIs if >>> > necessary and drop the AdapterManager completely ? >>> >>> Yes. I think there is no need to provide a generic adapter-like >>> implementation to solve this use case. >>> >>> > This would mean something the datastore implementation implements which >>> > oak-core can navigate to would have to implement a mix in interface >>> with a >>> > getURI() method. I am not certain what or how without trying to do it. >>> > >>> > Would that address your concern here ? >>> >>> I think it's worth trying. Thanks for bringing the conversation forward. >>> >> >> >> I will create 2 new branches. >> 1 with no adapter manager relying on mixin interfaces and one with a non >> OSGi adapter manager plugin pattern. >> >> Thanks for the input. >> Best Regards >> Ian >> >>
