jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Klimetschek (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (OAK-7570) [DirectBinaryAccess][DISCUSS] How to access HttpBlobProvider from oak-jcr
Date Tue, 10 Jul 2018 22:50:00 GMT

    [ https://issues.apache.org/jira/browse/OAK-7570?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16539320#comment-16539320
] 

Alexander Klimetschek edited comment on OAK-7570 at 7/10/18 10:49 PM:
----------------------------------------------------------------------

[~mduerig]
{quote}Adding a pair of such methods per implementation does certainly pollute these APIs
and create undesirable strong coupling.
{quote}
It's in the generic parts of Oak that only have one implementation (oak-jcr SessionImpl, oak-api
MutableRoot), and then in the NodeStores, BlobStores and DataStores that choose to provide
this implementation.

To me (with hindsight of course :) this is a feature that was "missed" by JCR, and really
should be part of JCR (if there would be a 2.x version) . Hence it finds its way into different
places of Oak...

Having said that, if my [questions|https://github.com/apache/jackrabbit-oak/pull/94#pullrequestreview-136033951]
on the whiteboard approach can get a "is ok" answer, I don't want to object to this approach
if you think it's better for maintaining Oak.
{quote}This is not what the JavaDoc of HttpBinaryProvider#initiateHttpUpload() implies though.
{quote}
What do you think is the [current javadoc|https://github.com/mattvryan/jackrabbit-oak/blob/b2afdc74a0e3f2fbc0c6debb1f3bf9205a9e4d6a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/api/binary/HttpBinaryProvider.java#L101-L106]
implying that is a problem? The javadoc is geared towards the client. It should not fully
define the implementation approach, if there can be variations in the future (say a new repository
wide "upload binary" privilege for example). Happy to improve the docs.
{quote}Oak's access control model is powerful enough to address this by adding proper permission
checks
{quote}
This is a proper permission check using Oak's existing access control mechanisms... checking
if the user can set the binary property. We are _not_ inventing anything new here.

We simply replicate something that was previously a single request (upload binary) and single
JCR session operation (set binary property + save), into two separate requests (initiate +
complete). Where the initiate step must fail already if the permission is not right.

If we deem to change the implementation in the future to make the check based on a new repository
wide privilege for example, or some other approach, it's not a problem if the client passes
the path argument here. It could just be ignored. However, if we turn it around and start
with no path argument and no check, but later want to introduce it, we would have to change
the API and force all clients to change.


was (Author: alexander.klimetschek):
[~mduerig]
bq. Adding a pair of such methods per implementation does certainly pollute these APIs and
create undesirable strong coupling.

It's in the generic parts of Oak that only have one implementation (oak-jcr SessionImpl, oak-api
MutableRoot), and then in the NodeStores, BlobStores and DataStores that choose to provide
this implementation.

To me (with hindsight of course :-) this is a feature that was "missed" by JCR, and really
should be part of JCR (if there would be a 2.x version) . Hence it finds its way into different
places of Oak...

Having said that, if my [questions|https://github.com/apache/jackrabbit-oak/pull/94#pullrequestreview-136033951]
on the whiteboard approach can get a "is ok" answer, I don't want to object to this approach
if you think it's better for maintaining Oak.

bq. This is not what the JavaDoc of HttpBinaryProvider#initiateHttpUpload() implies though.

What do you think is the [current javadoc|https://github.com/mattvryan/jackrabbit-oak/blob/b2afdc74a0e3f2fbc0c6debb1f3bf9205a9e4d6a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/api/binary/HttpBinaryProvider.java#L101-L106]
implying that is a problem? The javadoc is geared towards the client. It should not fully
define the implementation approach, if there can be variations in the future (say a new repository
wide "upload binary" privilege for example).

bq. Oak's access control model is powerful enough to address this by adding proper permission
checks

This is a proper permission check using Oak's existing access control mechanisms... checking
if the user can set the binary property. We are _not_ inventing anything new here.

We simply replicate something that was previously a single request (upload binary) and single
JCR session operation (set binary property + save), into two separate requests (initiate +
complete). Where the initiate step must fail already if the permission is not right.

If we deem to change the implementation in the future to make the check based on a new repository
wide privilege for example, or some other approach, it's not a problem if the client passes
the path argument here. It could just be ignored. However, if we turn it around and start
with no path argument and no check, but later want to introduce it, we would have to change
the API and force all clients to change.

> [DirectBinaryAccess][DISCUSS] How to access HttpBlobProvider from oak-jcr
> -------------------------------------------------------------------------
>
>                 Key: OAK-7570
>                 URL: https://issues.apache.org/jira/browse/OAK-7570
>             Project: Jackrabbit Oak
>          Issue Type: Technical task
>          Components: jcr
>            Reporter: Matt Ryan
>            Assignee: Matt Ryan
>            Priority: Major
>
> Open discussion related to OAK-7569:
> The [original pull request|https://github.com/apache/jackrabbit-oak/pull/88] proposes
changes to oak-api, oak-segment-tar, oak-store-document, oak-core, and oak-jcr as well as
oak-blob-plugins, oak-blob-cloud, and oak-blob-azure.  Would it be possible / better to keep
the changes local to the oak-blob-* bundles and avoid making changes throughout the stack?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message