Duck typing in Oak

[Jukka Zitting <ju...@gmail.com>]

Hi,

As a part of my effort to streamline node type handling in Oak, I've
encountered a number of cases where validators and other components
explicitly check whether a node is of a given type before processing
it further. For example the indexing code checks whether children of
an oak:index node are of the oak:indexDefinition type, and the
security code verifies that only rep:accessControllable nodes can
contain ACLs.

I wonder if we really do need such type checks. They increase
implementation complexity and add extra performance overhead (though
I've been able to cut down on that quite a bit) for no major benefit.
Enforcing such type constraints would IMO be best left to the type
validator that already has much better grasp of the available type
information and is explicitly optimized for efficient type handling.

Instead most of the other components that now validate such type
information could simply rely on duck typing to figure out the meaning
of the nodes they're processing. For example all children of an
oak:index node could be interpreted as index definitions. The type
validator (with some extensions about reserved names, etc.) could
ensure the correctness of such assumptions so there would be no need
to explicitly re-check the types of such content whenever it's
accessed.

BR,

Jukka Zitting

Re: Duck typing in Oak

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Wed, Apr 24, 2013 at 3:05 PM, Bertrand Delacretaz
<bd...@apache.org> wrote:
> Duck typing looks fine to me for index definition nodes, but for ACL I
> think the node types should be strictly enforced - if only to make
> sure that a search for rep:accessControllable nodes provides a
> complete picture of the current ACLs.

It would still be enforced; by the type validator instead of the
access control validator.

BR,

Jukka Zitting

Re: Duck typing in Oak

[Bertrand Delacretaz <bd...@apache.org>]

On Wed, Apr 24, 2013 at 1:28 PM, Jukka Zitting <ju...@gmail.com> wrote:
> ...the indexing code checks whether children of
> an oak:index node are of the oak:indexDefinition type, and the
> security code verifies that only rep:accessControllable nodes can
> contain ACLs....

Duck typing looks fine to me for index definition nodes, but for ACL I
think the node types should be strictly enforced - if only to make
sure that a search for rep:accessControllable nodes provides a
complete picture of the current ACLs.

I have no opinion on which layer should enforce that however, speaking
only as a user here.

-Bertrand

Re: Duck typing in Oak

[Angela Schreiber <an...@adobe.com>]

hi jukka

On 4/25/13 11:44 AM, Jukka Zitting wrote:
> Hi,
>
> On Thu, Apr 25, 2013 at 12:34 PM, Angela Schreiber<an...@adobe.com>  wrote:
>> from a pure node type point of view it was legal to create
>> a node named "rep:policy" underneath a nt:unstructured node.
>> similarly an nt:unstructured node would allow you to create
>> a child of type "rep:ACL"...
>
> See the suggestion about reserved namespaces from my previous post.
>
> I'd guard against such cases by only allowing content in a reserved
> namespace like "rep/internal" to occur when covered by a named item
> definition in a built-in node type.

if that doesn't cause any backwards compatibility issues i am fine
with moving the validation to the node type hook. i have strong feelings
about where the validation occurs as long as it's present.

kind regards
angela


> BR,
>
> Jukka Zitting

Re: Duck typing in Oak

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Thu, Apr 25, 2013 at 12:34 PM, Angela Schreiber <an...@adobe.com> wrote:
> from a pure node type point of view it was legal to create
> a node named "rep:policy" underneath a nt:unstructured node.
> similarly an nt:unstructured node would allow you to create
> a child of type "rep:ACL"...

See the suggestion about reserved namespaces from my previous post.

I'd guard against such cases by only allowing content in a reserved
namespace like "rep/internal" to occur when covered by a named item
definition in a built-in node type.

BR,

Jukka Zitting

Re: Duck typing in Oak

[Angela Schreiber <an...@adobe.com>]

hi jukka

i don't see how you would enforce that in the nt validation
without making moving access control specific implementation
logic (which needs to be pluggable) into the nt plugin, which
IMO was not the right thing to do...

from a pure node type point of view it was legal to create
a node named "rep:policy" underneath a nt:unstructured node.
similarly an nt:unstructured node would allow you to create
a child of type "rep:ACL"... it's just the JCR specific
protection that will not allow you to create an ACE below.
with oak it would work as as we found that we can't and
don't want to enforce the protected flag associated with
nodes and properties on the oak level.

kind regards
angela


On 4/24/13 3:23 PM, Jukka Zitting wrote:
> Hi,
>
> On Wed, Apr 24, 2013 at 3:59 PM, Angela Schreiber<an...@adobe.com>  wrote:
>> as far as access control content is concerned that validation
>> is IMO required due to the fact that without the mixin type
>> on the parent the policy nodes would have a different declaring
>> node type which IMO is part of the contract defined by the
>> implementation... e.g. the repo could contain rep:policy nodes
>> that are basically invalid.
>
> I'm proposing that we move the checking of such constraints to the
> type validator where we already have all relevant type information
> readily available and where we can more easily optimize the constraint
> checking.
>
> Note that doing so would require us to extend the type validator with
> some extra rules, like that content in reserved namespaces like
> rep/internal can only occur when there's a built-in node type with a
> matching named item definition. Such a rule would for example
> guarantee that a rep:policy node can only occur when the parent is
> rep:AccessControllable. Then there would be no need for the access
> control validator to re-check that constraint and it could simply
> treat all rep:policy nodes as containers of access control
> information.
>
>> while taking a closer look at the permission evaluation i found
>> that the commit hooks and validators get informed about all
>> index updates which then get explicitly ignored by most
>> validators/hooks, except for those dealing with index update.
>
> The best way to address this is to move the index updates to be
> performed only after all the other hooks/editors have been processed.
> Unfortunately this doesn't cover all possible cases (like when an
> entire subtree is copied around together with the contained hidden
> content), so an additional piece of functionality, described below, is
> needed.
>
>> wouldn't it be possible to make it an explicit characteristic
>> of the commit hooks if they want/need to deal with hidden
>> trees and/or properties? IMO that would simplify the wast
>> majority of all hooks and significantly reduce the amount of
>> calls.
>
> Indeed. The o.a.j.o.spi.commit.VisibleEditor wrapper class that's
> explicitly designed for this purpose. The provider of an editor (which
> is what most of our commit hooks would ideally become), that doesn't
> need to care about hidden content, can use a VisibleEditor wrapper for
> the root editor it returns to prevent any hidden content from showing
> up to that editor. See the TypeEditorProvider class for an example of
> that feature in action.
>
> BR,
>
> Jukka Zitting

Re: Duck typing in Oak

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Wed, Apr 24, 2013 at 3:59 PM, Angela Schreiber <an...@adobe.com> wrote:
> as far as access control content is concerned that validation
> is IMO required due to the fact that without the mixin type
> on the parent the policy nodes would have a different declaring
> node type which IMO is part of the contract defined by the
> implementation... e.g. the repo could contain rep:policy nodes
> that are basically invalid.

I'm proposing that we move the checking of such constraints to the
type validator where we already have all relevant type information
readily available and where we can more easily optimize the constraint
checking.

Note that doing so would require us to extend the type validator with
some extra rules, like that content in reserved namespaces like
rep/internal can only occur when there's a built-in node type with a
matching named item definition. Such a rule would for example
guarantee that a rep:policy node can only occur when the parent is
rep:AccessControllable. Then there would be no need for the access
control validator to re-check that constraint and it could simply
treat all rep:policy nodes as containers of access control
information.

> while taking a closer look at the permission evaluation i found
> that the commit hooks and validators get informed about all
> index updates which then get explicitly ignored by most
> validators/hooks, except for those dealing with index update.

The best way to address this is to move the index updates to be
performed only after all the other hooks/editors have been processed.
Unfortunately this doesn't cover all possible cases (like when an
entire subtree is copied around together with the contained hidden
content), so an additional piece of functionality, described below, is
needed.

> wouldn't it be possible to make it an explicit characteristic
> of the commit hooks if they want/need to deal with hidden
> trees and/or properties? IMO that would simplify the wast
> majority of all hooks and significantly reduce the amount of
> calls.

Indeed. The o.a.j.o.spi.commit.VisibleEditor wrapper class that's
explicitly designed for this purpose. The provider of an editor (which
is what most of our commit hooks would ideally become), that doesn't
need to care about hidden content, can use a VisibleEditor wrapper for
the root editor it returns to prevent any hidden content from showing
up to that editor. See the TypeEditorProvider class for an example of
that feature in action.

BR,

Jukka Zitting

Re: Duck typing in Oak

[Angela Schreiber <an...@adobe.com>]

hi jukka

as far as access control content is concerned that validation
is IMO required due to the fact that without the mixin type
on the parent the policy nodes would have a different declaring
node type which IMO is part of the contract defined by the
implementation... e.g. the repo could contain rep:policy nodes
that are basically invalid.

but while talking about the validators and commit hooks:

while taking a closer look at the permission evaluation i found
that the commit hooks and validators get informed about all
index updates which then get explicitly ignored by most
validators/hooks, except for those dealing with index update.

wouldn't it be possible to make it an explicit characteristic
of the commit hooks if they want/need to deal with hidden
trees and/or properties? IMO that would simplify the wast
majority of all hooks and significantly reduce the amount of
calls.

what do you think?
kind regards
angela



On 4/24/13 1:28 PM, Jukka Zitting wrote:
> Hi,
>
> As a part of my effort to streamline node type handling in Oak, I've
> encountered a number of cases where validators and other components
> explicitly check whether a node is of a given type before processing
> it further. For example the indexing code checks whether children of
> an oak:index node are of the oak:indexDefinition type, and the
> security code verifies that only rep:accessControllable nodes can
> contain ACLs.
>
> I wonder if we really do need such type checks. They increase
> implementation complexity and add extra performance overhead (though
> I've been able to cut down on that quite a bit) for no major benefit.
> Enforcing such type constraints would IMO be best left to the type
> validator that already has much better grasp of the available type
> information and is explicitly optimized for efficient type handling.
>
> Instead most of the other components that now validate such type
> information could simply rely on duck typing to figure out the meaning
> of the nodes they're processing. For example all children of an
> oak:index node could be interpreted as index definitions. The type
> validator (with some extensions about reserved names, etc.) could
> ensure the correctness of such assumptions so there would be no need
> to explicitly re-check the types of such content whenever it's
> accessed.
>
> BR,
>
> Jukka Zitting