You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2017/02/24 17:15:15 UTC
svn commit: r1784304 - in /jackrabbit/oak/branches/1.6: ./
oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/
oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/
oak-core/src/test...
Author: angela
Date: Fri Feb 24 17:15:15 2017
New Revision: 1784304
URL: http://svn.apache.org/viewvc?rev=1784304&view=rev
Log:
OAK-5784 : hashCode of RestrictionImpl doesn't include value (merge rev.1784162)
Modified:
jackrabbit/oak/branches/1.6/ (props changed)
jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java
jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java
Propchange: jackrabbit/oak/branches/1.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Feb 24 17:15:15 2017
@@ -1,3 +1,3 @@
/jackrabbit/oak/branches/1.0:1665962
-/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782770,1782945,1782973,1782990,1783061,1783066,1783089,1783104-1783105,1783619,1783731,1783733,1783742,1783855,1784023,1784130,1784251
+/jackrabbit/oak/trunk:1781068,1781075,1781248,1781386,1781846,1781907,1782000,1782029,1782196,1782447,1782770,1782945,1782973,1782990,1783061,1783066,1783089,1783104-1783105,1783619,1783731,1783733,1783742,1783855,1784023,1784130,1784162,1784251
/jackrabbit/trunk:1345480
Modified: jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java?rev=1784304&r1=1784303&r2=1784304&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImpl.java Fri Feb 24 17:15:15 2017
@@ -20,6 +20,7 @@ import javax.annotation.Nonnull;
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 @@ public class RestrictionImpl implements
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 @@ public class RestrictionImpl implements
//-------------------------------------------------------------< Object >---
@Override
public int hashCode() {
- return Objects.hashCode(definition, property);
+ if (hashCode == 0) {
+ hashCode = Objects.hashCode(definition, property, PropertyValues.create(property));
+ }
+ return hashCode;
}
@Override
Modified: jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java?rev=1784304&r1=1784303&r2=1784304&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorTest.java Fri Feb 24 17:15:15 2017
@@ -18,14 +18,20 @@ package org.apache.jackrabbit.oak.securi
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.Before;
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 @@ public class AccessControlValidatorTest
@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);
@@ -376,6 +383,28 @@ public class AccessControlValidatorTest
}
@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.<String, Value>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.<String, Value>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
public void hiddenNodeAdded() throws CommitFailedException {
AccessControlValidatorProvider provider = new AccessControlValidatorProvider(getSecurityProvider());
MemoryNodeStore store = new MemoryNodeStore();
Modified: jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java?rev=1784304&r1=1784303&r2=1784304&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java (original)
+++ jackrabbit/oak/branches/1.6/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/security/authorization/restriction/RestrictionImplTest.java Fri Feb 24 17:15:15 2017
@@ -18,11 +18,9 @@ package org.apache.jackrabbit.oak.spi.se
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 org.junit.Test;
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,21 +39,38 @@ import static org.junit.Assert.fail;
*/
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, 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());
}
- private static PropertyState createProperty(String name, String value) {
- return PropertyStates.createProperty(name, value, Type.NAME);
+ @Test
+ public void testRestrictionFromDefinition() {
+ Restriction r = new RestrictionImpl(restriction.getProperty(), restriction.getDefinition());
+ assertEquals(restriction, r);
}
@Test
@@ -82,37 +98,49 @@ public class RestrictionImplTest extends
}
}
- @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<Restriction> rs = new ArrayList<Restriction>();
+ List<Restriction> 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);
}
});
@@ -120,4 +148,43 @@ public class RestrictionImplTest extends
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<Restriction> rs = new ArrayList<Restriction>();
+ // - 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());
+ }
+ }
}