[
https://issues.apache.org/jira/browse/OAK-1147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13814026#comment-13814026
]
angela commented on OAK-1147:
-----------------------------
one more thing... which IMO makes it a bit inconsistent: #getPropertyCount
checks for 'canReadAll(Properties)' and alternatively calculates the number of
properties:
{code}
if (treePermission.canReadProperties()) {
propertyCount = state.getPropertyCount();
} else {
propertyCount = count(filter(
state.getProperties(),
new ReadablePropertyPredicate()));
}
{code}
on the other hand #getProperties() first tries to optimize for all properties
being visible and second tests if the node state itself is readable (in which
case it reads the properties)... if not it returns an empty list.
thus, one method make the accessibility of the properties depending on the
read-status of the tree itself. the other doesn't. from a JCR level point of
view it doesn't make a difference (as Node#getProperties can only be called if
you already have the Node at hand)... but on the OAK level it might result in
odd behavior as the tree can exist or not.
> SecureNodeBuilder/SecureNodeState: Consider using
> 'TreePermission#canReadProperties'
> ------------------------------------------------------------------------------------
>
> Key: OAK-1147
> URL: https://issues.apache.org/jira/browse/OAK-1147
> Project: Jackrabbit Oak
> Issue Type: Improvement
> Reporter: angela
> Assignee: angela
>
> the methods #getProperties and #getPropertyCount have a shortcut for those
> cases where all properties are accessible.
> however, the current implemention requires SecurityContext#canReadAll to
> return true in order to enable the shortcut. without knowing the very details
> of the SecureNodeState/Builder i would have expected that
> #canReadAllProperties would be sufficient.
> [~jukkaz], do you remember what was the reason for using #canReadAll here? i
> changed it for test purpose and didn't see any difference... but that may
> also imply that we don't have enough or specific tests for this.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
