[
https://issues.apache.org/jira/browse/OAK-5355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16010138#comment-16010138
]
Michael Dürig commented on OAK-5355:
------------------------------------
Apart from above issue the patch looks good to me.
{{MutableRootTest.testCommit()}} shows that this is indeed a bug and needs to
be fixed. This is the type of bug I suspected already in OAK-5296 but was not
able to come up with a test for.
I'm not sure whether {{SecureNodeBuilderTest.testBaseChanged2}} (and similar)
are good tests though. This actually depends on the intended semantics of the
permission provider (i.e. at which point exactly the permissions are evaluated
and changes in them are seen). This tests seem to imply that such changes
should be visible every time they happen. But then caching of the tree
permissions is not possible as the following modification to the test
demonstrates:
{code}
===================================================================
---
oak-core/src/test/java/org/apache/jackrabbit/oak/core/SecureNodeBuilderTest.java
(date 1494834751000)
+++
oak-core/src/test/java/org/apache/jackrabbit/oak/core/SecureNodeBuilderTest.java
(revision )
@@ -181,6 +181,8 @@
try {
permissionProvider.denyAll = true;
assertFalse(secureNodeBuilder.exists());
+ permissionProvider.denyAll = false;
+ assertTrue(secureNodeBuilder.exists());
assertFalse(secureNodeBuilder.hasChildNode(NAME_ACCESSIBLE));
assertFalse(secureNodeBuilder.hasProperty("prop"));
assertFalse(secureNodeBuilder.isNew());
{code}
> Too eager refreshing of tree permissions in SecureNodeBuilder
> -------------------------------------------------------------
>
> Key: OAK-5355
> URL: https://issues.apache.org/jira/browse/OAK-5355
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Components: core
> Reporter: Michael Dürig
> Assignee: angela
> Labels: technical_debt
> Fix For: 1.7.0, 1.8
>
> Attachments: OAK-5355.patch
>
>
> {{SecureNodeBuilder.baseChanged()}} calls
> {{SecureNodeBuilder.getTreePermission()}} even though the tree permission
> would be calculated lazily as needed anyway. Re-calculating the tree
> permissions at this point bears the risk of accessing stale data from the
> underlying not yet fully refreshed root (when being called e.g. from
> {{MutableRoot.refresh()}}.
> I would thus argue for removing the call to
> {{SecureNodeBuilder.getTreePermission()}} from
> {{SecureNodeBuilder.baseChanged()}}.
> See also OAK-5296 for an in-depth analysis.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
