Troublesome dependencies introduce with OAK-1296

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

hi jukka

while looking at the oak-core i just realized that you introduced a
dependency
to o.a.j.oak.core.TreeTypeProviderImpl in the permission evaluation code
base
located in o.a.j.oak.security.permission

that modification happened with revision 1552379 while addressing OAK-1296.

IMO there should IMO be no dependency to the core package from our
plugins. while i
see that we cannot achieve this currently due to usage of the
ImmutableTree all over
the place but that's IMO an annoyance and we should not introduce more
dependencies 
to implementations of the oak api.

if we can't get rid of the TreeTypeProvider, we should consider moving the
type provider elsewhere (some utility package) or make it accessible on
the API.

wdyt?

kind regards
angela

Re: Troublesome dependencies introduce with OAK-1296

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

hi jukka

>Agreed. That's actually one of the drivers behind revision 1552379
>where I got rid of the JCR/Oak API dependencies of
>ReadOnlyNodeTypeManager. Ideally the Editors and Validators should be
>able to operate entirely on the NodeState level, though I think
>getting there will be difficult as ImmutableTrees are used in so many
>places.

yes... see also OAK-1381 for a similar discussion.
>
>> if we can't get rid of the TreeTypeProvider, we should consider moving
>>the
>> type provider elsewhere (some utility package) or make it accessible on
>> the API.
>
>In this particular case the reason for the TreeTypeProviderImpl
>dependency is that it's needed by ImmutableTree.getType(), which in
>turn is needed by CompiledPermissionImpl. I would actually try to get
>rid of ImmutableTree.getType() entirely, and instead track the type
>information in TreePermission based on the sequence of
>getChildPermission() calls. This way TreeTypeProvider would no longer
>be needed by ImmutableTree, and could easily be moved elsewhere.

yes... that might be better. i will give it a try.

regards
angela

Re: Troublesome dependencies introduce with OAK-1296

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

Hi,

On Mon, Feb 3, 2014 at 9:22 AM, Angela Schreiber <an...@adobe.com> wrote:
> while looking at the oak-core i just realized that you introduced a
> dependency to o.a.j.oak.core.TreeTypeProviderImpl in the permission
> evaluation code base located in o.a.j.oak.security.permission

The dependency was there already, see PermissionValidatorProvider from
before my commit. I merely shuffled things around a bit, which made it
show up in (MoveAware)PermissionValidator.

> IMO there should IMO be no dependency to the core package from our
> plugins. while i see that we cannot achieve this currently due to usage of the
> ImmutableTree all over the place but that's IMO an annoyance and we
> should not introduce more dependencies to implementations of the
> oak api.

Agreed. That's actually one of the drivers behind revision 1552379
where I got rid of the JCR/Oak API dependencies of
ReadOnlyNodeTypeManager. Ideally the Editors and Validators should be
able to operate entirely on the NodeState level, though I think
getting there will be difficult as ImmutableTrees are used in so many
places.

> if we can't get rid of the TreeTypeProvider, we should consider moving the
> type provider elsewhere (some utility package) or make it accessible on
> the API.

In this particular case the reason for the TreeTypeProviderImpl
dependency is that it's needed by ImmutableTree.getType(), which in
turn is needed by CompiledPermissionImpl. I would actually try to get
rid of ImmutableTree.getType() entirely, and instead track the type
information in TreePermission based on the sequence of
getChildPermission() calls. This way TreeTypeProvider would no longer
be needed by ImmutableTree, and could easily be moved elsewhere.

BR,

Jukka Zitting