On Fri, Jan 1, 2016 at 7:56 PM, <dbros...@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