On 24 August 2017 at 14:42, Michael Dürig <[email protected]> wrote:
> > > On 24.08.17 15:33, Chetan Mehrotra wrote: > >> Inside Oak it would have its own version of an AdapterManager, >>> AdapterFactory. the DataStore would implement an AdapterFactory and >>> register it with the AdapterManager. The OakConversionService >>> implementation would then use the AdapterManager to perform the >>> conversion. >>> If no AdapterFactory to adapt from JCR Binary to URI existed, then null >>> would be returned from the OakConversionService. >>> >>> Thats no API changes to Blob, binary or anything. No complex >>> transformation >>> through multiple layers. No instanceof required and no difference between >>> Sling and non Sling usage. >>> It does require an Oak version of the AdapterManager and AdapterFactory >>> concepts, but does not require anything to implement Adaptable. >>> >> >> Thanks for those details. Much clear now. So with this we need not add >> adaptTo to all stuff. Instead provide an OakConversionService which >> converts the Binary to provided type and then have DataStores provide >> the AdapterFactory. >> >> This would indeed avoid any new methods in existing objects and >> provide a single entry point. >> >> +1 for this approach >> > > Yay! Rough quick PoC at [1] to ilustrate how it might work. 1 https://github.com/apache/jackrabbit-oak/compare/trunk...ieb:OAK-6575?expand=1 > > > Michael > > > >> Chetan Mehrotra >> >> >> On Thu, Aug 24, 2017 at 6:16 AM, Ian Boston <[email protected]> wrote: >> >>> Hi, >>> I am probably not helping as here as there are several layers and I think >>> they are getting confused between what I am thinking and what you are >>> thinking. >>> >>> I was thinking Oak exposed a service to convert along the lines of the >>> OSCi >>> converter service or the OakConversionService suggested earlier. Both >>> Sling >>> and other uses of Oak would use it. >>> >>> Inside Oak it would have its own version of an AdapterManager, >>> AdapterFactory. the DataStore would implement an AdapterFactory and >>> register it with the AdapterManager. The OakConversionService >>> implementation would then use the AdapterManager to perform the >>> conversion. >>> If no AdapterFactory to adapt from JCR Binary to URI existed, then null >>> would be returned from the OakConversionService. >>> >>> Thats no API changes to Blob, binary or anything. No complex >>> transformation >>> through multiple layers. No instanceof required and no difference between >>> Sling and non Sling usage. >>> It does require an Oak version of the AdapterManager and AdapterFactory >>> concepts, but does not require anything to implement Adaptable. >>> >>> As I showed in the PoC, all the S3 specific implementation fits inside >>> the >>> S3DataStore which already does everything required to perform the >>> conversion. It already goes from Binary -> Blob -> ContentIdentifier -> >>> S3 >>> Key -> S3 URL by virtue of >>> ValueImpl.getBlob((Value)jcrBinary).getContentIdentifier() -> convert to >>> S3key and then signed URL. >>> >>> If it would help, I can do a patch to show how it works. >>> Best Regards >>> Ian >>> >>> On 24 August 2017 at 13:05, Chetan Mehrotra <[email protected]> >>> wrote: >>> >>> No API changes to any existing Oak APIs, >>>>> >>>> >>>> Some API needs to be exposed. Note again Oak does not depend on Sling >>>> API. Any such integration code is implemented in Sling Base module >>>> [1]. But that module would still require some API in Oak to provide >>>> such an adaptor >>>> >>>> The adaptor proposal here is for enabling layers within Oak to allow >>>> conversion of JCR Binary instance to SignedBinary. Now how this is >>>> exposed to end user depends on usage context >>>> >>>> Outside Sling >>>> ------------------ >>>> >>>> Check if binary instanceof Oak Adaptable. If yes then cast it and adapt >>>> it >>>> >>>> import org.apache.jackrabbit.oak.api.Adaptable >>>> >>>> Binary b = ... >>>> SignedBinary sb = null >>>> if (b instanceof Adaptable) { >>>> sb = ((Adaptable)b).adaptTo(SignedBinary.class); >>>> } >>>> >>> >>> >>> Within Sling >>>> ---------------- >>>> >>>> Have an AdapterManager implemented in Sling JCR Base [1] which uses >>>> above approach >>>> >>>> Chetan Mehrotra >>>> [1] https://github.com/apache/sling/tree/trunk/bundles/jcr/base >>>> >>>> >>>> On Thu, Aug 24, 2017 at 4:55 AM, Ian Boston <[email protected]> wrote: >>>> >>>>> From the javadoc in [1] >>>>> >>>>> "The adaptable object may be any non-null object and is not required to >>>>> implement the Adaptable interface." >>>>> >>>>> >>>>> On 24 August 2017 at 12:54, Ian Boston <[email protected]> wrote: >>>>> >>>>> Hi, >>>>>> That would require javax.jcr.Binary to implement Adaptable, which it >>>>>> >>>>> cant. >>>> >>>>> (OakBinary could but it doesnt need to). >>>>>> >>>>>> Using Sling AdapterFactory/AdapterManger javadoc (to be replaced with >>>>>> >>>>> Oaks >>>> >>>>> internal version of the same) >>>>>> >>>>>> What is needed is an AdapterFactory for javax.jcr.Binary to >>>>>> SignedBinary >>>>>> provided by the S3DataStore itself. >>>>>> >>>>>> Since javax.jcr.Binary cant extend Adaptable, its not possible to call >>>>>> binary.adaptTo(SignedBinary.class) without a cast, hence, >>>>>> the call is done via the AdapterManager[1] >>>>>> >>>>>> SignedBinary signedBinary = adapterManager.getAdapter(binary, >>>>>> SignedBinary.class); >>>>>> >>>>>> --- >>>>>> You could just jump to >>>>>> URI uri = adapterManager.getAdapter(binary, URI.class); >>>>>> >>>>>> No API changes to any existing Oak APIs, >>>>>> >>>>>> Best Regards >>>>>> Ian >>>>>> >>>>>> >>>>>> 1 https://sling.apache.org/apidocs/sling5/org/apache/sling/ >>>>>> api/adapter/ >>>>>> AdapterManager.html >>>>>> >>>>>> >>>>>> >>>>>> On 24 August 2017 at 12:38, Chetan Mehrotra < >>>>>> [email protected]> >>>>>> wrote: >>>>>> >>>>>> various layers involved. The bit I don't understand is how the >>>>>>>> >>>>>>> adaptable >>>> >>>>> pattern would make those go away. To me that pattern is just another >>>>>>>> >>>>>>> way to >>>>>>> >>>>>>>> implement this but it would also need to deal with all those layers. >>>>>>>> >>>>>>> >>>>>>> Yes this adapter support would need to be implement at all layers. >>>>>>> >>>>>>> So call to >>>>>>> 1. binary.adaptTo(SignedBinary.class) //binary is JCR Binary >>>>>>> 2. results in blob.adaptTo(SignedBinary.class) //blob is Oak Blob. >>>>>>> Blob interface would extend adaptable >>>>>>> 3. results in SegmentBlob delegating to BlobStoreBlob which >>>>>>> 4. delegates to BlobStore // Here just passing the BlobId >>>>>>> 5. which delegates to DataStoreBlobStore >>>>>>> 6. which delegates to S3DataStore >>>>>>> 7. which returns the SignedBinary implementation >>>>>>> >>>>>>> However adapter support would allow us to make this instance of check >>>>>>> extensible. Otherwise we would be hardcoding instance of check to >>>>>>> SignedBinary at each of the above place though those layers need not >>>>>>> be aware of SignedBinary support (its specific to S3 impl) >>>>>>> >>>>>>> Chetan Mehrotra >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>
