Hi, Here is an updated patch. https://github.com/apache/sling/compare/trunk...ieb:OAK-6575-3-1?expand=1
Best Regards Ian On 12 September 2017 at 17:39, Carsten Ziegeler <cziege...@apache.org> wrote: > Ian Boston wrote > > Hi, > > > > On 12 September 2017 at 17:09, Carsten Ziegeler <cziege...@apache.org > > <mailto:cziege...@apache.org>> wrote: > > > > I think there are two parts to be discussed: > > a) whether to directly code this into the get servlet > > b) how to mark the resource as being handled this way > > > > I think a) makes sense, it's a central place and allows any resource > > provider to use it. However, this is also the problem I see :) For > each > > and every resource the adaption is tried, punishing basically every > get > > request handled by this. This might be neglectable, but we should > think > > about it. > > > > > > The cost of checking is the Resource is a binary resource should be > > almost identical to the adaptTo(InputStream.class) which happens > > already, If its done the same way everything will be in memory already > > so that part probably wont add anything. This part is sub almost > > certainly sub 1ms and doesn't cause and additional objects to be loaded > > by Oak. > > > > Same goes for a Resource that can be adapted to an InputStream, upto the > > point where the InputStream is created. > > The cost of signing the a URL using a shared private key is sub 1ms on > > my MBP. creating the private key object is expensive, but that is done > > in the activate inside Oak. > > > > All of this needs to be checked on realistic hardware, not a > > unrealistically fast laptop. > > Right, I agree this shouldn't pose a problem in reality and is more of a > theoretical nature. However, it doubles the adaptTo calls (first URI, > then InputStream). So this part is doubled. Again, I think it can be > neglected but we should be aware of it. > > > > > > > > > For b) I think its good to use an approach which does not require new > > API. However, any resource could somehow implement adaption to a URI. > > That might be a URI which can only be resolved by a custom URI > handler > > or it might be a URI which can't be resolved at all or it might be an > > external URI in any form. For example the full URI to access this > > resource. So if a redirect would happen to this URI, it would be an > > endless loop. > > So just adapting to a URI might not be enough to detect this case. > > > > I'm just throwing in a wild idea which I haven't thought fully > through > > yet to foster a discussion: what about we add an > > ExternalizableInputStream to our API. It extends InputStream by a > > getURI() method. > > In the get servlet we adapt to InputStream as today. If it is an > > instance of ExternalizableInputStream getURI is called, if not, the > > input stream is used (if provided). > > > > > > This would solve the potential performance problem of a) and gives a > > clear opt-in for b). > > > > > > > > Agreed. > > Sounds like the org.apache.sling.jcr.resource bundle needs to use > > URIProvider where it does the Resource -> InputStream adaptation, rather > > than a new custom bundle. > > Is that what you were thinking of ? > > Yes, right. > > > > > If so, would ExternalizableInputStream be in the Resource API or > > somewhere else ? > > I think somewhere in the resource API, not quiet sure where but we'll > find a nice spot. > > Regards > Carsten > > > > > > > Best Regards > > Ian > > > > > > > > > > > > Regards > > Carsten > > > > Ian Boston wrote > > > Hi, > > > > > > Background: > > > OAK-6575 adds support for a Oak DataStore to implement a service > > > implementing org.apache.jackrabbit.oak.api.conversion.URIProvider > > which > > > converts a JCR Value to a URI. The implementation in OAK-6575 is > > for the > > > Oak S3 DataStore to convert to a CloudFront signed url as detailed > > in [1]. > > > The URI the URIProvider emits in this case expires quickly (< > > 60s), only > > > allows GET and is only issued to a JCR session that can read the > > binary. It > > > is intended to be used as a redirect, the flow being. > > > > > > Client > Sling -> GET <binaryURL> > > > <- redirect to CF sifgned url > > > Client -> CF -> GET <CF signed URL> > > > > > > To patch[2] has been discussed at length on Oak and now is close > > to being > > > acceptable. > > > > > > To make this work in Sling requires a patch to Sling, which needs > > > discussion. The assumptions are that it should work seamlessly and > not > > > require any core component to depend on the Oak API. One patch is > > [3]. It > > > adds a new bundle that adds and AdapterFactory that adapts from > > Resource to > > > URI. It tries all URIProviders registered by Oak and if one > > provides a URI. > > > > > > Inside the Get Servlets streaming helper, if a resource is > > adaptable to a > > > URI, that URI is used as a redirect rather than streaming the > > binaries. > > > > > > I think this patch needs discussion before being committed, hence > this > > > thread. > > > > > > Best Regards > > > Ian > > > > > > > > > > > > 1 > > > > > http://docs.aws.amazon.com/AmazonCloudFront/latest/ > DeveloperGuide/PrivateContent.html > > <http://docs.aws.amazon.com/AmazonCloudFront/latest/ > DeveloperGuide/PrivateContent.html> > > > 2 > > > > > https://github.com/ieb/jackrabbit-oak/compare/trunk.. > .ieb:OAK-6575-3?expand=1 > > <https://github.com/ieb/jackrabbit-oak/compare/trunk.. > .ieb:OAK-6575-3?expand=1> > > > 3 > > https://github.com/apache/sling/compare/trunk...ieb:OAK- > 6575-3?expand=1 > > <https://github.com/apache/sling/compare/trunk...ieb:OAK- > 6575-3?expand=1> > > > > > > > > > > > > > -- > > Carsten Ziegeler > > Adobe Research Switzerland > > cziege...@apache.org <mailto:cziege...@apache.org> > > > > > > > > > -- > Carsten Ziegeler > Adobe Research Switzerland > cziege...@apache.org >
