Somehow the following between me and Chetan slipped off list, so copying it back here to continue on list, probably my fault, apologies.
---------------- Hi, On 23 August 2017 at 13:29, Chetan Mehrotra <[email protected]> wrote: > > >> Can we make that more generic, not relying on Binary? > > Was being conservative there and hence not proposed a generic > adaptable api. But +1 for making it generic > > > adapting from something already Adaptable would work. > > Sling Resource might be a good option, since a) its adaptable, and b) its > > resolvable and already available to the code that would use this. > > So far Oak does not have a dependency on Sling API and this api should > be usable in non Sling setups also > In that case, Oak can't implement the AdapterFactory interface either or implement Adaptables. AdaptableBinary should strictly speaking extend Adaptable if it is to implement adaptTo and be used in a Sling based deployment. Since the AdapterFactory and Adaptable interface are not available, the benefits are not available to Oak. Hence, why not simply use binaryProp instanceof SignedBinary ? Oak would have to provide multiple versions of the Binary implementation, differing depending on if or not a SignedBinary was appropriate. That might be dependent on the properties of the node or one of its parents. It would certainly be dependent on if the Binary was in S3 (or equiv) or NodeStore (<16K). > > > > SingedBinary sb = resource.adaptTo(SignedBinary.class); > > At Oak level we do not have resource. What we have is JCR Binary > interface which does not extend Adaptable > ok, understood. > > > Also, As a caller I could specify 20 years, or 30s. Oak should decide > what > > the ttl is, not leave it to the caller. > > As mentioned earlier implementation can enforce an upper limit. My > take was that different usecase require different ttls. For e.g. if > the asset needs to be shared for some workflow processing then it may > require longer ttl. If other think its not useful we can drop that > If a long ttl is possible for certain use cases, then what is to stop someone using that long ttl where it is not appropriate (publishing it to public html).? The ttl should be configurable by the deployer, but not by the caller. OAK-6575 was only really thinking about requests being converted into redirects that would be used immediately. If something like workflow (or any client) needed direct access some time later, it should authenticate against Oak, be issued a fresh signed url as a redirect and access the binary using its own signed url. No client should be issued a signed url that could be used in the distant (relatively) future bypassing fresh ACL constraints saved to Oak. Best Regards Ian > > Chetan Mehrotra On 23 August 2017 at 13:22, Ian Boston <[email protected]> wrote: > Hi, > > On 23 August 2017 at 13:06, Julian Reschke <[email protected]> wrote: > >> On 2017-08-23 13:39, Chetan Mehrotra wrote: >> >>> Below is one possible sketch of the proposed api. We introduce a new >>> AdaptableBinary which allows adapting a Binary to some other form. >>> >>> API >>> === >>> >>> public interface AdaptableBinary { >>> >>> /** >>> * Adapts the binary to another type >>> * >>> * @param <AdapterType> The generic type to which this binary is >>> adapted >>> * to >>> * @param type The Class object of the target type >>> * @return The adapter target or <code>null</code> if the binary >>> cannot >>> * adapt to the requested type >>> */ >>> <AdapterType> AdapterType adaptTo(Class<AdapterType> type); >>> } >>> >> >> Can we make that more generic, not relying on Binary? > > > > adapting from something already Adaptable would work. > Sling Resource might be a good option, since a) its adaptable, and b) its > resolvable and already available to the code that would use this. > > > > >> >> >> Usage >>> ===== >>> >>> Binary binProp = node.getProperty("jcr:data").getBinary(); >>> >>> //Check if Binary is of type AdaptableBinary >>> if (binProp instanceof AdaptableBinary){ >>> AdaptableBinary adaptableBinary = (AdaptableBinary) binProp; >>> SignedBinary url = adaptableBinary.adaptTo(SignedBinary.class); >>> } >>> >>> > > This breaks the Adaptable pattern, which should not require intermediate > interfaces. > If the implementation needs instanceof we can drop the Adaptable pattern > and just use APIs. > > if (binProp instanceof SignedBinary) { > response.sendRedirect(((SingedBinary)binProp).getURL()); > } > > better would be > > SingedBinary sb = resource.adaptTo(SignedBinary.class); > if ( sb != null ) { // oak might decide for this invocation a signed > binary is not appropriate, and so return null. > response.sendRedirect(sb.getURL()); > } > > If the DS didn't have a AdapterFactory implemented, null will be returned. > Optionally implementing an interface on a call by call basis requires more > work, potentially, than adapting. > > > > >> Where SignedBinary is one of the supported adaptables. >>> >>> public interface SignedBinary { >>> >>> URL getUrl(int ttl, TimeUnit unit) >>> } >>> >> >> Use URI, not URL. >> > > Is there something so wrong with an immutable String in this case ? > > Also, As a caller I could specify 20 years, or 30s. Oak should decide what > the ttl is, not leave it to the caller. > > Best Regards > Ian > > >> ... >>> >> >> Best regards, Julian >> > >
