Index: ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 Subsystem: com.intellij.openapi.diff.impl.patch.BaseRevisionTextPatchEP <+>/*\n * Licensed to the Apache Software Foundation (ASF) under one or more\n * contributor license agreements. See the NOTICE file distributed with\n * this work for additional information regarding copyright ownership.\n * The ASF licenses this file to You under the Apache License, Version 2.0\n * (the \"License\"); you may not use this file except in compliance with\n * the License. You may obtain a copy of the License at\n *\n * http://www.apache.org/licenses/LICENSE-2.0\n *\n * Unless required by applicable law or agreed to in writing, software\n * distributed under the License is distributed on an \"AS IS\" BASIS,\n * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n * See the License for the specific language governing permissions and\n * limitations under the License.\n */\npackage org.apache.jackrabbit.oak.security.authorization.accesscontrol;\n\nimport java.security.Principal;\nimport javax.jcr.AccessDeniedException;\nimport javax.jcr.security.AccessControlManager;\n\nimport org.apache.jackrabbit.JcrConstants;\nimport org.apache.jackrabbit.api.security.JackrabbitAccessControlList;\nimport org.apache.jackrabbit.api.security.authorization.PrivilegeManager;\nimport org.apache.jackrabbit.oak.api.CommitFailedException;\nimport org.apache.jackrabbit.oak.api.Tree;\nimport org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore;\nimport org.apache.jackrabbit.oak.spi.commit.CommitInfo;\nimport org.apache.jackrabbit.oak.spi.commit.Validator;\nimport org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AbstractAccessControlTest;\nimport org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AccessControlConstants;\nimport org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;\nimport org.apache.jackrabbit.oak.spi.state.NodeBuilder;\nimport org.apache.jackrabbit.oak.spi.state.NodeState;\nimport org.apache.jackrabbit.oak.util.NodeUtil;\nimport org.junit.After;\nimport org.junit.Before;\nimport org.junit.Test;\n\nimport static org.hamcrest.CoreMatchers.containsString;\nimport static org.junit.Assert.assertNotNull;\nimport static org.junit.Assert.assertNull;\nimport static org.junit.Assert.assertThat;\nimport static org.junit.Assert.assertTrue;\nimport static org.junit.Assert.fail;\n\npublic class AccessControlValidatorTest extends AbstractAccessControlTest implements AccessControlConstants {\n\n private final String testName = \"testRoot\";\n private final String testPath = '/' + testName;\n private final String aceName = \"validAce\";\n\n private Principal testPrincipal;\n\n @Before\n public void before() throws Exception {\n super.before();\n\n NodeUtil rootNode = new NodeUtil(root.getTree(\"/\"), getNamePathMapper());\n rootNode.addChild(testName, JcrConstants.NT_UNSTRUCTURED);\n\n root.commit();\n\n testPrincipal = getTestPrincipal();\n }\n\n @After\n public void after() throws Exception {\n try {\n Tree testRoot = root.getTree(testPath);\n if (testRoot.exists()) {\n testRoot.remove();\n root.commit();\n }\n } finally {\n super.after();\n }\n }\n\n private NodeUtil getTestRoot() {\n return new NodeUtil(root.getTree(testPath));\n }\n\n private NodeUtil createAcl() throws AccessDeniedException {\n NodeUtil testRoot = getTestRoot();\n testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);\n\n NodeUtil acl = testRoot.addChild(REP_POLICY, NT_REP_ACL);\n NodeUtil ace = createACE(acl, aceName, NT_REP_GRANT_ACE, testPrincipal.getName(), PrivilegeConstants.JCR_READ);\n ace.addChild(REP_RESTRICTIONS, NT_REP_RESTRICTIONS);\n return acl;\n }\n\n private static NodeUtil createACE(NodeUtil acl, String aceName, String ntName, String principalName, String... privilegeNames) throws AccessDeniedException {\n NodeUtil ace = acl.addChild(aceName, ntName);\n ace.setString(REP_PRINCIPAL_NAME, principalName);\n ace.setNames(REP_PRIVILEGES, privilegeNames);\n return ace;\n }\n\n @Test\n public void testPolicyWithOutChildOrder() throws AccessDeniedException {\n NodeUtil testRoot = getTestRoot();\n testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);\n testRoot.addChild(REP_POLICY, NT_REP_ACL);\n\n try {\n root.commit();\n fail(\"Policy node with child node ordering\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"OakAccessControl0004\")); // Order of children is not stable\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n }\n }\n\n @Test\n public void testOnlyRootIsRepoAccessControllable() {\n NodeUtil testRoot = getTestRoot();\n testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_REPO_ACCESS_CONTROLLABLE);\n\n try {\n root.commit();\n fail(\"Only the root node can be made RepoAccessControllable.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot\"));\n }\n }\n\n @Test\n public void testAddInvalidRepoPolicy() throws Exception {\n NodeUtil testRoot = getTestRoot();\n testRoot.setNames(JcrConstants.JCR_MIXINTYPES, MIX_REP_ACCESS_CONTROLLABLE);\n NodeUtil policy = getTestRoot().addChild(REP_REPO_POLICY, NT_REP_ACL);\n try {\n root.commit();\n fail(\"Attempt to add repo-policy with rep:AccessControllable node.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot\"));\n } finally {\n policy.getTree().remove();\n }\n }\n\n @Test\n public void testAddPolicyWithAcContent() throws Exception {\n NodeUtil acl = createAcl();\n NodeUtil ace = acl.getChild(aceName);\n\n NodeUtil[] acContent = new NodeUtil[]{acl, ace, ace.getChild(REP_RESTRICTIONS)};\n for (NodeUtil node : acContent) {\n NodeUtil policy = node.addChild(REP_POLICY, NT_REP_ACL);\n try {\n root.commit();\n fail(\"Adding an ACL below access control content should fail\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isConstraintViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n } finally {\n policy.getTree().remove();\n }\n }\n }\n\n @Test\n public void testAddRepoPolicyWithAcContent() throws Exception {\n NodeUtil acl = createAcl();\n NodeUtil ace = acl.getChild(aceName);\n\n NodeUtil[] acContent = new NodeUtil[]{acl, ace, ace.getChild(REP_RESTRICTIONS)};\n for (NodeUtil node : acContent) {\n NodeUtil policy = node.addChild(REP_REPO_POLICY, NT_REP_ACL);\n try {\n root.commit();\n fail(\"Adding an ACL below access control content should fail\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isConstraintViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n } finally {\n policy.getTree().remove();\n }\n }\n }\n\n @Test\n public void testAddAceWithAcContent() throws Exception {\n NodeUtil acl = createAcl();\n NodeUtil ace = acl.getChild(aceName);\n\n NodeUtil[] acContent = new NodeUtil[]{ace, ace.getChild(REP_RESTRICTIONS)};\n for (NodeUtil node : acContent) {\n NodeUtil entry = node.addChild(\"invalidACE\", NT_REP_DENY_ACE);\n try {\n root.commit();\n fail(\"Adding an ACE below an ACE or restriction should fail\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isConstraintViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy/validAce\"));\n } finally {\n entry.getTree().remove();\n }\n }\n }\n\n @Test\n public void testAddRestrictionWithAcContent() throws Exception {\n NodeUtil acl = createAcl();\n NodeUtil ace = acl.getChild(aceName);\n\n NodeUtil[] acContent = new NodeUtil[]{acl, ace.getChild(REP_RESTRICTIONS)};\n for (NodeUtil node : acContent) {\n NodeUtil entry = node.addChild(\"invalidRestriction\", NT_REP_RESTRICTIONS);\n try {\n root.commit();\n fail(\"Adding an ACE below an ACE or restriction should fail\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isConstraintViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n } finally {\n entry.getTree().remove();\n }\n }\n }\n\n @Test\n public void testAddIsolatedPolicy() throws Exception {\n String[] policyNames = new String[]{\"isolatedACL\", REP_POLICY, REP_REPO_POLICY};\n NodeUtil node = getTestRoot();\n\n for (String policyName : policyNames) {\n NodeUtil policy = node.addChild(policyName, NT_REP_ACL);\n try {\n root.commit();\n fail(\"Writing an isolated ACL without the parent being rep:AccessControllable should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot\"));\n } finally {\n // revert pending changes that cannot be saved.\n policy.getTree().remove();\n }\n }\n\n }\n\n @Test\n public void testAddIsolatedAce() throws Exception {\n String[] ntNames = new String[]{NT_REP_DENY_ACE, NT_REP_GRANT_ACE};\n NodeUtil node = getTestRoot();\n\n for (String aceNtName : ntNames) {\n NodeUtil ace = createACE(node, \"isolatedACE\", aceNtName, testPrincipal.getName(), PrivilegeConstants.JCR_READ);\n try {\n root.commit();\n fail(\"Writing an isolated ACE should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/isolatedACE\"));\n } finally {\n // revert pending changes that cannot be saved.\n ace.getTree().remove();\n }\n }\n }\n\n @Test\n public void testAddIsolatedRestriction() throws Exception {\n NodeUtil node = getTestRoot();\n NodeUtil restriction = node.addChild(\"isolatedRestriction\", NT_REP_RESTRICTIONS);\n try {\n root.commit();\n fail(\"Writing an isolated Restriction should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot\"));\n } finally {\n // revert pending changes that cannot be saved.\n restriction.getTree().remove();\n }\n }\n\n @Test\n public void testInvalidPrivilege() throws Exception {\n NodeUtil acl = createAcl();\n\n String privName = \"invalidPrivilegeName\";\n createACE(acl, \"invalid\", NT_REP_GRANT_ACE, testPrincipal.getName(), privName);\n try {\n root.commit();\n fail(\"Creating an ACE with invalid privilege should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n }\n }\n\n @Test\n public void testAbstractPrivilege() throws Exception {\n PrivilegeManager pMgr = getPrivilegeManager(root);\n pMgr.registerPrivilege(\"abstractPrivilege\", true, new String[0]);\n\n NodeUtil acl = createAcl();\n createACE(acl, \"invalid\", NT_REP_GRANT_ACE, testPrincipal.getName(), \"abstractPrivilege\");\n try {\n root.commit();\n fail(\"Creating an ACE with an abstract privilege should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n }\n }\n\n @Test\n public void testInvalidRestriction() throws Exception {\n NodeUtil restriction = createAcl().getChild(aceName).getChild(REP_RESTRICTIONS);\n restriction.setString(\"invalid\", \"value\");\n try {\n root.commit();\n fail(\"Creating an unsupported restriction should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n }\n }\n\n @Test\n public void testRestrictionWithInvalidType() throws Exception {\n NodeUtil restriction = createAcl().getChild(aceName).getChild(REP_RESTRICTIONS);\n restriction.setName(REP_GLOB, \"rep:glob\");\n try {\n root.commit();\n fail(\"Creating restriction with invalid type should fail.\");\n } catch (CommitFailedException e) {\n // success\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy\"));\n }\n }\n\n @Test\n public void testDuplicateAce() throws Exception {\n AccessControlManager acMgr = getAccessControlManager(root);\n JackrabbitAccessControlList acl = org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils.getAccessControlList(acMgr, testPath);\n acl.addAccessControlEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES));\n acMgr.setPolicy(testPath, acl);\n\n // add duplicate ac-entry on OAK-API\n NodeUtil policy = new NodeUtil(root.getTree(testPath + \"/rep:policy\"));\n NodeUtil ace = policy.addChild(\"duplicateAce\", NT_REP_GRANT_ACE);\n ace.setString(REP_PRINCIPAL_NAME, testPrincipal.getName());\n ace.setNames(AccessControlConstants.REP_PRIVILEGES, PrivilegeConstants.JCR_ADD_CHILD_NODES);\n\n try {\n root.commit();\n fail(\"Creating duplicate ACE must be detected\");\n } catch (CommitFailedException e) {\n assertTrue(e.isAccessControlViolation());\n assertThat(e.getMessage(), containsString(\"/testRoot/rep:policy/duplicateAce\"));\n }\n }\n\n @Test\n public void hiddenNodeAdded() throws CommitFailedException {\n AccessControlValidatorProvider provider = new AccessControlValidatorProvider(getSecurityProvider());\n MemoryNodeStore store = new MemoryNodeStore();\n NodeState root = store.getRoot();\n NodeBuilder builder = root.builder();\n NodeBuilder test = builder.child(\"test\");\n NodeBuilder hidden = test.child(\":hidden\");\n\n Validator validator = provider.getRootValidator(\n root, builder.getNodeState(), CommitInfo.EMPTY);\n Validator childValidator = validator.childNodeAdded(\n \"test\", test.getNodeState());\n assertNotNull(childValidator);\n\n Validator hiddenValidator = childValidator.childNodeAdded(\":hidden\", hidden.getNodeState());\n assertNull(hiddenValidator);\n }\n\n @Test\n public void hiddenNodeChanged() throws CommitFailedException {\n AccessControlValidatorProvider provider = new AccessControlValidatorProvider(getSecurityProvider());\n MemoryNodeStore store = new MemoryNodeStore();\n NodeBuilder builder = store.getRoot().builder();\n builder.child(\"test\").child(\":hidden\");\n NodeState root = builder.getNodeState();\n\n NodeBuilder test = root.builder().child(\"test\");\n NodeBuilder hidden = test.child(\":hidden\");\n hidden.child(\"added\");\n\n Validator validator = provider.getRootValidator(\n root, builder.getNodeState(), CommitInfo.EMPTY);\n Validator childValidator = validator.childNodeChanged(\n \"test\", root.getChildNode(\"test\"), test.getNodeState());\n assertNotNull(childValidator);\n\n Validator hiddenValidator = childValidator.childNodeChanged(\":hidden\", root.getChildNode(\"test\").getChildNode(\":hidden\"), hidden.getNodeState());\n assertNull(hiddenValidator);\n }\n\n @Test\n public void hiddenNodeDeleted() throws CommitFailedException {\n AccessControlValidatorProvider provider = new AccessControlValidatorProvider(getSecurityProvider());\n MemoryNodeStore store = new MemoryNodeStore();\n NodeBuilder builder = store.getRoot().builder();\n builder.child(\"test\").child(\":hidden\");\n NodeState root = builder.getNodeState();\n\n builder = root.builder();\n NodeBuilder test = builder.child(\"test\");\n test.child(\":hidden\").remove();\n\n Validator validator = provider.getRootValidator(\n root, builder.getNodeState(), CommitInfo.EMPTY);\n Validator childValidator = validator.childNodeChanged(\"test\", root.getChildNode(\"test\"), test.getNodeState());\n assertNotNull(childValidator);\n\n Validator hiddenValidator = childValidator.childNodeDeleted(\n \":hidden\", root.getChildNode(\"test\").getChildNode(\":hidden\"));\n assertNull(hiddenValidator);\n }\n} =================================================================== --- ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java (revision 1783887) +++ ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java (revision ) @@ -18,14 +18,20 @@ import java.security.Principal; import javax.jcr.AccessDeniedException; +import javax.jcr.PropertyType; +import javax.jcr.Value; +import javax.jcr.ValueFactory; import javax.jcr.security.AccessControlManager; +import com.google.common.collect.ImmutableMap; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.api.security.JackrabbitAccessControlList; import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants; import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.commit.Validator; import org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AbstractAccessControlTest; @@ -39,6 +45,7 @@ import org.junit.Test; import static org.hamcrest.CoreMatchers.containsString; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; @@ -356,7 +363,7 @@ @Test public void testDuplicateAce() throws Exception { AccessControlManager acMgr = getAccessControlManager(root); - JackrabbitAccessControlList acl = org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils.getAccessControlList(acMgr, testPath); + JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath); acl.addAccessControlEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES)); acMgr.setPolicy(testPath, acl); @@ -373,6 +380,28 @@ assertTrue(e.isAccessControlViolation()); assertThat(e.getMessage(), containsString("/testRoot/rep:policy/duplicateAce")); } + } + + @Test + public void testAceDifferentByRestrictionValue() throws Exception { + ValueFactory vf = getValueFactory(root); + + AccessControlManager acMgr = getAccessControlManager(root); + JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(acMgr, testPath); + acl.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES), true, + ImmutableMap.of(), + ImmutableMap.of(AccessControlConstants.REP_NT_NAMES, new Value[] {vf.createValue(NodeTypeConstants.NT_OAK_UNSTRUCTURED, PropertyType.NAME)})); + + // add ac-entry that only differs by the value of the restriction + acl.addEntry(testPrincipal, privilegesFromNames(PrivilegeConstants.JCR_ADD_CHILD_NODES), true, + ImmutableMap.of(), + ImmutableMap.of(AccessControlConstants.REP_NT_NAMES, new Value[] {vf.createValue(NodeTypeConstants.NT_UNSTRUCTURED, PropertyType.NAME)})); + assertEquals(2, acl.getAccessControlEntries().length); + + acMgr.setPolicy(testPath, acl); + + // persisting changes must succeed; the 2 ac-entries must not be treated as equal. + root.commit(); } @Test \ No newline at end of file Index: ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 Subsystem: com.intellij.openapi.diff.impl.patch.BaseRevisionTextPatchEP <+>/*\n * Licensed to the Apache Software Foundation (ASF) under one or more\n * contributor license agreements. See the NOTICE file distributed with\n * this work for additional information regarding copyright ownership.\n * The ASF licenses this file to You under the Apache License, Version 2.0\n * (the \"License\"); you may not use this file except in compliance with\n * the License. You may obtain a copy of the License at\n *\n * http://www.apache.org/licenses/LICENSE-2.0\n *\n * Unless required by applicable law or agreed to in writing, software\n * distributed under the License is distributed on an \"AS IS\" BASIS,\n * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n * See the License for the specific language governing permissions and\n * limitations under the License.\n */\npackage org.apache.jackrabbit.oak.spi.security.authorization.restriction;\n\nimport java.util.ArrayList;\nimport java.util.List;\n\nimport javax.annotation.Nonnull;\n\nimport com.google.common.collect.ImmutableList;\n\nimport org.apache.jackrabbit.oak.api.PropertyState;\nimport org.apache.jackrabbit.oak.api.Type;\nimport org.apache.jackrabbit.oak.plugins.memory.PropertyStates;\nimport org.apache.jackrabbit.oak.spi.security.authorization.accesscontrol.AbstractAccessControlTest;\nimport org.junit.Before;\nimport org.junit.Test;\n\nimport static org.junit.Assert.assertEquals;\nimport static org.junit.Assert.assertFalse;\nimport static org.junit.Assert.assertTrue;\nimport static org.junit.Assert.fail;\n\n/**\n * Tests for {@link RestrictionImpl}\n */\npublic class RestrictionImplTest extends AbstractAccessControlTest {\n\n private String name;\n private String value = \"value\";\n private RestrictionImpl restriction;\n\n @Before\n public void before() throws Exception {\n super.before();\n\n name = \"test:defName\";\n PropertyState property = createProperty(name, value);\n restriction = new RestrictionImpl(property, true);\n }\n\n private static PropertyState createProperty(String name, String value) {\n return PropertyStates.createProperty(name, value, Type.NAME);\n }\n\n @Test\n public void testGetName() {\n assertEquals(name, restriction.getDefinition().getName());\n }\n\n @Test\n public void testGetRequiredType() {\n assertEquals(Type.NAME, restriction.getDefinition().getRequiredType());\n }\n\n @Test\n public void testIsMandatory() {\n assertTrue(restriction.getDefinition().isMandatory());\n }\n\n @Test\n public void testInvalid() {\n try {\n new RestrictionImpl(null, false);\n fail(\"Creating RestrictionDefinition with null name should fail.\");\n } catch (NullPointerException e) {\n // success\n }\n }\n\n @Test\n public void testEquals() {\n // same definition\n assertEquals(restriction, new RestrictionImpl(createProperty(name, value), true));\n }\n\n @Test\n public void testNotEqual() {\n List rs = new ArrayList();\n // - different type\n rs.add(new RestrictionImpl(PropertyStates.createProperty(name, value, Type.STRING), true));\n // - different multi-value status\n rs.add(new RestrictionImpl(PropertyStates.createProperty(name, ImmutableList.of(value), Type.NAMES), true));\n // - different name\n rs.add(new RestrictionImpl(createProperty(\"otherName\", value), true));\n // - different value\n rs.add(new RestrictionImpl(createProperty(\"name\", \"otherValue\"), true));\n // - different mandatory flag\n rs.add(new RestrictionImpl(createProperty(name, value), false));\n // - different impl\n rs.add(new Restriction() {\n @Nonnull\n @Override\n public RestrictionDefinition getDefinition() {\n return new RestrictionDefinitionImpl(name, Type.NAME, true);\n }\n\n @Nonnull\n @Override\n public PropertyState getProperty() {\n return createProperty(name, value);\n }\n });\n\n for (Restriction r : rs) {\n assertFalse(restriction.equals(r));\n }\n }\n}\n =================================================================== --- ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java (revision 1783887) +++ ../oak/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java (revision ) @@ -18,11 +18,9 @@ import java.util.ArrayList; import java.util.List; - import javax.annotation.Nonnull; import com.google.common.collect.ImmutableList; - import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Type; import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; @@ -32,6 +30,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -40,24 +39,41 @@ */ public class RestrictionImplTest extends AbstractAccessControlTest { - private String name; - private String value = "value"; + private final Type type = Type.NAME; + private final String name = "test:defName"; + private final String value = "value"; + private final boolean isMandatory = true; private RestrictionImpl restriction; @Before public void before() throws Exception { super.before(); - name = "test:defName"; - PropertyState property = createProperty(name, value); - restriction = new RestrictionImpl(property, true); + PropertyState property = createProperty(name, value, type); + restriction = new RestrictionImpl(property, isMandatory); } - private static PropertyState createProperty(String name, String value) { - return PropertyStates.createProperty(name, value, Type.NAME); + private static PropertyState createProperty(String name, String value, Type type) { + return PropertyStates.createProperty(name, value, type); } @Test + public void testGetProperty() { + assertEquals(createProperty(name, value, type), restriction.getProperty()); + } + + @Test + public void testGetDefinition() { + assertEquals(new RestrictionDefinitionImpl(name, type, isMandatory), restriction.getDefinition()); + } + + @Test + public void testRestrictionFromDefinition() { + Restriction r = new RestrictionImpl(restriction.getProperty(), restriction.getDefinition()); + assertEquals(restriction, r); + } + + @Test public void testGetName() { assertEquals(name, restriction.getDefinition().getName()); } @@ -82,42 +98,93 @@ } } - @Test + @Test public void testEquals() { // same definition - assertEquals(restriction, new RestrictionImpl(createProperty(name, value), true)); + assertEquals(restriction, new RestrictionImpl(createProperty(name, value, type), isMandatory)); } @Test + public void testEqualsSameDefinition() { + // same definition + assertEquals(restriction, new RestrictionImpl(restriction.getProperty(), restriction.getDefinition())); + } + + @Test + public void testEqualsSameObject() { + // same object + assertEquals(restriction, restriction); + } + + @Test public void testNotEqual() { - List rs = new ArrayList(); + List rs = new ArrayList(); // - different type - rs.add(new RestrictionImpl(PropertyStates.createProperty(name, value, Type.STRING), true)); + rs.add(new RestrictionImpl(PropertyStates.createProperty(name, value, Type.STRING), isMandatory)); // - different multi-value status - rs.add(new RestrictionImpl(PropertyStates.createProperty(name, ImmutableList.of(value), Type.NAMES), true)); + rs.add(new RestrictionImpl(PropertyStates.createProperty(name, ImmutableList.of(value), Type.NAMES), isMandatory)); // - different name - rs.add(new RestrictionImpl(createProperty("otherName", value), true)); + rs.add(new RestrictionImpl(createProperty("otherName", value, type), isMandatory)); // - different value - rs.add(new RestrictionImpl(createProperty("name", "otherValue"), true)); + rs.add(new RestrictionImpl(createProperty(name, "otherValue", type), isMandatory)); // - different mandatory flag - rs.add(new RestrictionImpl(createProperty(name, value), false)); + rs.add(new RestrictionImpl(createProperty(name, value, type), !isMandatory)); // - different impl rs.add(new Restriction() { @Nonnull @Override public RestrictionDefinition getDefinition() { - return new RestrictionDefinitionImpl(name, Type.NAME, true); + return new RestrictionDefinitionImpl(name, type, isMandatory); } @Nonnull @Override public PropertyState getProperty() { - return createProperty(name, value); + return createProperty(name, value, type); } }); for (Restriction r : rs) { assertFalse(restriction.equals(r)); + } + } + + @Test + public void testSameHashCode() { + // same definition + assertEquals(restriction.hashCode(), new RestrictionImpl(createProperty(name, value, type), isMandatory).hashCode()); + } + + @Test + public void testNotSameHashCode() { + List rs = new ArrayList(); + // - different type + rs.add(new RestrictionImpl(PropertyStates.createProperty(name, value, Type.STRING), isMandatory)); + // - different multi-value status + rs.add(new RestrictionImpl(PropertyStates.createProperty(name, ImmutableList.of(value), Type.NAMES), isMandatory)); + // - different name + rs.add(new RestrictionImpl(createProperty("otherName", value, type), isMandatory)); + // - different value + rs.add(new RestrictionImpl(createProperty(name, "otherValue", type), isMandatory)); + // - different mandatory flag + rs.add(new RestrictionImpl(createProperty(name, value, type), !isMandatory)); + // - different impl + rs.add(new Restriction() { + @Nonnull + @Override + public RestrictionDefinition getDefinition() { + return new RestrictionDefinitionImpl(name, type, isMandatory); + } + + @Nonnull + @Override + public PropertyState getProperty() { + return createProperty(name, value, type); + } + }); + + for (Restriction r : rs) { + assertNotEquals(restriction.hashCode(), r.hashCode()); } } } Index: ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 Subsystem: com.intellij.openapi.diff.impl.patch.BaseRevisionTextPatchEP <+>/*\n * Licensed to the Apache Software Foundation (ASF) under one or more\n * contributor license agreements. See the NOTICE file distributed with\n * this work for additional information regarding copyright ownership.\n * The ASF licenses this file to You under the Apache License, Version 2.0\n * (the \"License\"); you may not use this file except in compliance with\n * the License. You may obtain a copy of the License at\n *\n * http://www.apache.org/licenses/LICENSE-2.0\n *\n * Unless required by applicable law or agreed to in writing, software\n * distributed under the License is distributed on an \"AS IS\" BASIS,\n * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n * See the License for the specific language governing permissions and\n * limitations under the License.\n */\npackage org.apache.jackrabbit.oak.spi.security.authorization.restriction;\n\nimport javax.annotation.Nonnull;\n\nimport com.google.common.base.Objects;\nimport org.apache.jackrabbit.oak.api.PropertyState;\n\n/**\n * {@code RestrictionImpl}\n */\npublic class RestrictionImpl implements Restriction {\n\n private final RestrictionDefinition definition;\n private final PropertyState property;\n\n public RestrictionImpl(@Nonnull PropertyState property, @Nonnull RestrictionDefinition def) {\n this.definition = def;\n this.property = property;\n }\n\n public RestrictionImpl(@Nonnull PropertyState property, boolean isMandatory) {\n this.definition = new RestrictionDefinitionImpl(property.getName(), property.getType(), isMandatory);\n this.property = property;\n }\n\n //--------------------------------------------------------< Restriction >---\n @Nonnull\n @Override\n public RestrictionDefinition getDefinition() {\n return definition;\n }\n\n @Nonnull\n @Override\n public PropertyState getProperty() {\n return property;\n }\n\n //-------------------------------------------------------------< Object >---\n @Override\n public int hashCode() {\n return Objects.hashCode(definition, property);\n }\n\n @Override\n public boolean equals(Object o) {\n if (o == this) {\n return true;\n }\n if (o instanceof RestrictionImpl) {\n RestrictionImpl other = (RestrictionImpl) o;\n return definition.equals(other.definition) && property.equals(other.property);\n }\n return false;\n }\n}\n =================================================================== --- ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java (revision 1783887) +++ ../oak/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java (revision ) @@ -20,6 +20,7 @@ import com.google.common.base.Objects; import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.spi.query.PropertyValues; /** * {@code RestrictionImpl} @@ -29,6 +30,8 @@ private final RestrictionDefinition definition; private final PropertyState property; + private int hashCode = 0; + public RestrictionImpl(@Nonnull PropertyState property, @Nonnull RestrictionDefinition def) { this.definition = def; this.property = property; @@ -55,7 +58,10 @@ //-------------------------------------------------------------< Object >--- @Override public int hashCode() { - return Objects.hashCode(definition, property); + if (hashCode == 0) { + hashCode = Objects.hashCode(definition, property, PropertyValues.create(property)); + } + return hashCode; } @Override