Re: svn commit: r1722496 - /jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/xml/ImporterImpl.java

[Chetan Mehrotra <ch...@gmail.com>]

On Fri, Jan 1, 2016 at 7:56 PM, <db...@apache.org> wrote:

>              }
> -        } else if (getDefinition(parent).isProtected()) {
> -            if (pnImporter != null) {
> -                pnImporter.end(parent);
> -                // and reset the pnImporter field waiting for the next
> protected
> -                // parent -> selecting again from available importers
> -                pnImporter = null;
> -            }
> +        } else if ((pnImporter != null) &&
> getDefinition(parent).isProtected()) {
> +            pnImporter.end(parent);
> +            // and reset the pnImporter field waiting for the next
> protected
> +            // parent -> selecting again from available importers
> +            pnImporter = null;
>          }
>

Above change is causing couple of test failures in CUG

===
Failed tests:
testNestedCug(org.apache.jackrabbit.oak.spi.security.authorization.cug.impl.CugImportIgnoreTest)

testNestedCug(org.apache.jackrabbit.oak.spi.security.authorization.cug.impl.CugImportAbortTest)

testNestedCug(org.apache.jackrabbit.oak.spi.security.authorization.cug.impl.CugImportBesteffortTest)
===

It happens because `getDefinition(parent).isProtected()` has a side of
effect of triggering an exception. With above code change that call is not
made if 'pnImporter' is null and thus causes a change in behaviour. So
better to revert that change.

Chetan Mehrotra