jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jukka Zitting <jukka.zitt...@gmail.com>
Subject Re: svn commit: r1465664 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
Date Wed, 10 Apr 2013 06:26:31 GMT
Hi,

On Tue, Apr 9, 2013 at 5:49 PM, Angela Schreiber <anchela@adobe.com> wrote:
>> On Tue, Apr 9, 2013 at 12:45 PM, Angela Schreiber<anchela@adobe.com>
>> How about we change the order of the checks, so that the readStatus is
>> pre-loaded only when child.getChildNodeCount() == 0? That way we could
>> avoid unnecessarily evaluating permissions at ancestor nodes, but
>> still avoid the cost of the SecureNodeState wrapper for most leaf
>> nodes.
>
> sure, we can try that... in general i would prefer to get a better
> picture on the overall impact first before starting to optimize
> things and make the code complicated.

The way I see it, the ability to avoid the SecureNodeState wrapper in
cases like these is an essential feature of this design. Otherwise we
could just as well keep the access checks in TreeImpl and avoid the
extra layer of indirection. Thus I'd really want to include this
aspect in the SecureNodeState code right from the beginning.

Here's my proposed change to wrapChildNodeEntry.apply():

    SecureNodeState secureChild =
            new SecureNodeState(SecureNodeState.this, name, child);
    if (child.getChildNodeCount() == 0
            && secureChild.getReadStatus().includes(ALLOW_ALL)) {
        return input;
    } else {
        return new MemoryChildNodeEntry(name, secureChild);
    }

> anyway. the point here is that i would prefer to get it to work
> first and optimize once we know that it really works.

Based on my above point of view, I wouldn't consider SecureNodeState
to be working as designed if at least some getChildNode() calls didn't
return the raw underlying NodeState instance. :-)

BR,

Jukka Zitting

Mime
View raw message