You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "joerghoh (via GitHub)" <gi...@apache.org> on 2025/07/30 12:26:12 UTC

[PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

joerghoh opened a new pull request, #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415#discussion_r2243064839


##########
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java:
##########
@@ -1342,8 +1344,11 @@ protected Tree createValue() {
     }
 
     private boolean canReadMixinTypes(@NotNull Tree tree) {
-        return sessionContext.getAccessManager().hasPermissions(
-                tree, EMPTY_MIXIN_TYPES, Permissions.READ_PROPERTY);
+        // cache this information per SessionContext, as it is valid independent of the node

Review Comment:
   hmpf, for some reason I oversaw this ``tree`` parameter. 
   
   But on a different view, does it even make sense to disallow reading the mixin types in the context of ``Node.isNodeType()``? In that case this API call could return a different result, depending if I have read access to that property or not.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "anchela (via GitHub)" <gi...@apache.org>.
anchela commented on code in PR #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415#discussion_r2243039703


##########
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/SessionContext.java:
##########
@@ -152,6 +155,7 @@ public SessionContext(
         this.valueFactory = new ValueFactoryImpl(
                 delegate.getRoot(), namePathMapper, this.blobAccessProvider);
         this.sessionQuerySettings = sessionQuerySettings;
+        this.cacheMap = new HashMap<>();

Review Comment:
   hm.... not sure how that works now... you would need to keep it for every node you take a look at which will result for the cache to get really big. that doesn't sound good to me.
   on the other hand: storing a boolean for each node might also come with some overhead as one would have to invalidate it upon save/refresh.



##########
oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/session/NodeImpl.java:
##########
@@ -1342,8 +1344,11 @@ protected Tree createValue() {
     }
 
     private boolean canReadMixinTypes(@NotNull Tree tree) {
-        return sessionContext.getAccessManager().hasPermissions(
-                tree, EMPTY_MIXIN_TYPES, Permissions.READ_PROPERTY);
+        // cache this information per SessionContext, as it is valid independent of the node

Review Comment:
   that is actually not correct.
   whether or not the mixin type property can be read for a particular tree may vary from tree to tree.
   so caching it for the first tree you encounter is not correct.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415#issuecomment-3136567865

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=2415)
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415#issuecomment-3136078560

   # Commit-Check ✔️


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh closed pull request #2415: OAK-11827 check the permission of the mixin tree only once
URL: https://github.com/apache/jackrabbit-oak/pull/2415


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] OAK-11827 check the permission of the mixin tree only once [jackrabbit-oak]

Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on PR #2415:
URL: https://github.com/apache/jackrabbit-oak/pull/2415#issuecomment-3136942324

   closing because this approach is incorrect and won't work.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org