[jira] [Comment Edited] (OAK-928) Read access is enforced on NEW items

["Jukka Zitting (JIRA)" <ji...@apache.org>]

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

Jukka Zitting edited comment on OAK-928 at 11/26/13 8:11 PM:
-------------------------------------------------------------

Revision 1545803  takes care of the {{testShadowInvisibleProperty}} test case by adding per-property {{isNew}}/{{isReplaced}} methods.

Note that the test case originally made the assumption that the lack of read access also implies lack of write access. However, since the {{PermissionValidator}} does not enforce this, I adjusted the test case in revision to highlight the interesting "write-only" behavior. See revision 1545807.

[~anchela], can you take a look at the test case update in r1545807 and check whether the logic is sound? It seems to me that such "write-only" semantics would be useful in some cases (for example Unix systems have long used such setups for various purposes) and that's the way {{PermissionValidator}} currently works. But the original {{ShadowInvisibleContentTest}} setup (it expected commit() to fail when read access is denied) implied otherwise so I'm not sure if that's the intended design.


was (Author: jukkaz):
Revision  takes care of the {{testShadowInvisibleProperty}} test case by adding per-property {{isNew}}/{{isReplaced}} methods.

Note that the test case originally made the assumption that the lack of read access also implies lack of write access. However, since the {{PermissionValidator}} does not enforce this, I adjusted the test case in revision to highlight the interesting "write-only" behavior. See revision 1545807.

[~anchela], can you take a look at the test case update in r1545807 and check whether the logic is sound? It seems to me that such "write-only" semantics would be useful in some cases (for example Unix systems have long used such setups for various purposes) and that's the way {{PermissionValidator}} currently works. But the original {{ShadowInvisibleContentTest}} setup (it expected commit() to fail when read access is denied) implied otherwise so I'm not sure if that's the intended design.

> Read access is enforced on NEW items
> ------------------------------------
>
>                 Key: OAK-928
>                 URL: https://issues.apache.org/jira/browse/OAK-928
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: core
>            Reporter: angela
>            Assignee: Jukka Zitting
>             Fix For: 0.13
>
>
> as explained in OAK-923 we may currently run into the situation where a new Tree is being added because it does not exist but the resulting child does not exist due to restricted access. the same is true for new properties.
> IMHO this breaks backwards compatibility with jackrabbit core.
> i would therefore suggest to
> - extend MutableTree#exists by checking for the tree being new
>   line 390: 
>   {noformat}            
>   - return nodeBuilder.exists();
>   + return nodeBuilder.isNew() || nodeBuilder.exists();
>   {noformat}
> - similar behavior for #hasProperty #getProperty, #getProperties and #getPropertyCount (no patch yet).
> however, i would like to get a broader consensus on this kind of modifications as this
> might have side effects in other places.



--
This message was sent by Atlassian JIRA
(v6.1#6144)