diff --git a/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java b/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java index 0d38ddb98b..90219f02e4 100644 --- a/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/objectidentity/ObjectIdentityImpl.java @@ -17,7 +17,6 @@ package org.springframework.security.acls.objectidentity; import java.io.Serializable; import java.lang.reflect.Method; -import org.springframework.security.acls.jdbc.LookupStrategy; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -92,35 +91,38 @@ public class ObjectIdentityImpl implements ObjectIdentity { //~ Methods ======================================================================================================== /** - * Important so caching operates properly.

Considers an object of the same class equal if it has the same - * classname and id properties.

- * + * Important so caching operates properly. + *

+ * Considers an object of the same class equal if it has the same classname and + * id properties. *

- * Note that this class uses string equality for the identifier field, which ensures it better supports - * differences between {@link LookupStrategy} requirements and the domain object represented by this - * ObjectIdentityImpl. - *

+ * Numeric identities (Integer and Long values) are considered equal if they are numerically equal. Other + * serializable types are evaluated using a simple equality. * * @param arg0 object to compare * * @return true if the presented object matches this object */ public boolean equals(Object arg0) { - if (arg0 == null) { - return false; - } - - if (!(arg0 instanceof ObjectIdentityImpl)) { + if (arg0 == null || !(arg0 instanceof ObjectIdentityImpl)) { return false; } ObjectIdentityImpl other = (ObjectIdentityImpl) arg0; - if (this.getIdentifier().toString().equals(other.getIdentifier().toString()) && this.getJavaType().equals(other.getJavaType())) { - return true; + if (identifier instanceof Number && other.identifier instanceof Number) { + // Integers and Longs with same value should be considered equal + if (((Number)identifier).longValue() != ((Number)other.identifier).longValue()) { + return false; + } + } else { + // Use plain equality for other serializable types + if (!identifier.equals(other.identifier)) { + return false; + } } - return false; + return javaType.equals(other.javaType); } public Serializable getIdentifier() { @@ -145,7 +147,7 @@ public class ObjectIdentityImpl implements ObjectIdentity { } public String toString() { - StringBuffer sb = new StringBuffer(); + StringBuilder sb = new StringBuilder(); sb.append(this.getClass().getName()).append("["); sb.append("Java Type: ").append(this.javaType.getName()); sb.append("; Identifier: ").append(this.identifier).append("]"); diff --git a/acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityTests.java b/acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityImplTests.java similarity index 67% rename from acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityTests.java rename to acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityImplTests.java index 90e0a33f69..3244f1d73a 100644 --- a/acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityTests.java +++ b/acl/src/test/java/org/springframework/security/acls/objectidentity/ObjectIdentityImplTests.java @@ -9,10 +9,10 @@ import org.junit.Test; * * @author Andrei Stefan */ -public class ObjectIdentityTests { +public class ObjectIdentityImplTests { private static final String DOMAIN_CLASS = - "org.springframework.security.acls.objectidentity.ObjectIdentityTests$MockIdDomainObject"; + "org.springframework.security.acls.objectidentity.ObjectIdentityImplTests$MockIdDomainObject"; //~ Methods ======================================================================================================== @@ -27,7 +27,7 @@ public class ObjectIdentityTests { // Check String-Serializable constructor required field try { - new ObjectIdentityImpl("", new Long(1)); + new ObjectIdentityImpl("", Long.valueOf(1)); fail("It should have thrown IllegalArgumentException"); } catch (IllegalArgumentException expected) { } @@ -42,7 +42,7 @@ public class ObjectIdentityTests { // The correct way of using String-Serializable constructor try { - new ObjectIdentityImpl(DOMAIN_CLASS, new Long(1)); + new ObjectIdentityImpl(DOMAIN_CLASS, Long.valueOf(1)); } catch (IllegalArgumentException notExpected) { fail("It shouldn't have thrown IllegalArgumentException"); @@ -54,10 +54,16 @@ public class ObjectIdentityTests { fail("It should have thrown IllegalArgumentException"); } catch (IllegalArgumentException expected) { - } } + @Test + public void gettersReturnExpectedValues() throws Exception { + ObjectIdentity obj = new ObjectIdentityImpl(DOMAIN_CLASS, Long.valueOf(1)); + assertEquals(Long.valueOf(1), obj.getIdentifier()); + assertEquals(MockIdDomainObject.class, obj.getJavaType()); + } + @Test public void testGetIdMethodConstraints() throws Exception { // Check the getId() method is present @@ -99,53 +105,56 @@ public class ObjectIdentityTests { @Test(expected=IllegalStateException.class) public void testConstructorInvalidClassParameter() throws Exception { - new ObjectIdentityImpl("not.a.Class", new Long(1)); + new ObjectIdentityImpl("not.a.Class", Long.valueOf(1)); } @Test public void testEquals() throws Exception { - ObjectIdentity obj = new ObjectIdentityImpl(DOMAIN_CLASS, new Long(1)); + ObjectIdentity obj = new ObjectIdentityImpl(DOMAIN_CLASS, Long.valueOf(1)); MockIdDomainObject mockObj = new MockIdDomainObject(); - mockObj.setId(new Long(1)); + mockObj.setId(Long.valueOf(1)); String string = "SOME_STRING"; assertNotSame(obj, string); assertFalse(obj.equals(null)); assertFalse(obj.equals("DIFFERENT_OBJECT_TYPE")); - assertFalse(obj.equals(new ObjectIdentityImpl(DOMAIN_CLASS,new Long(2)))); + assertFalse(obj.equals(new ObjectIdentityImpl(DOMAIN_CLASS, Long.valueOf(2)))); assertFalse(obj.equals(new ObjectIdentityImpl( - "org.springframework.security.acls.objectidentity.ObjectIdentityTests$MockOtherIdDomainObject", - new Long(1)))); - assertEquals(new ObjectIdentityImpl(DOMAIN_CLASS,new Long(1)), obj); + "org.springframework.security.acls.objectidentity.ObjectIdentityImplTests$MockOtherIdDomainObject", + Long.valueOf(1)))); + assertEquals(new ObjectIdentityImpl(DOMAIN_CLASS,Long.valueOf(1)), obj); assertEquals(obj, new ObjectIdentityImpl(mockObj)); } @Test - public void testHashCode() throws Exception { - ObjectIdentity obj = new ObjectIdentityImpl(DOMAIN_CLASS, new Long(1)); - assertEquals(new ObjectIdentityImpl(DOMAIN_CLASS, new Long(1)).hashCode(), obj.hashCode()); - assertTrue(new ObjectIdentityImpl( - "org.springframework.security.acls.objectidentity.ObjectIdentityTests$MockOtherIdDomainObject", - new Long(1)).hashCode() != obj.hashCode()); - assertTrue(new ObjectIdentityImpl(DOMAIN_CLASS, new Long(2)).hashCode() != obj.hashCode()); + public void hashcodeIsDifferentForDifferentJavaTypes() throws Exception { + ObjectIdentity obj = new ObjectIdentityImpl(Object.class, Long.valueOf(1)); + ObjectIdentity obj2 = new ObjectIdentityImpl(String.class, Long.valueOf(1)); + assertFalse(obj.hashCode() == obj2.hashCode()); + } + + @Test + public void longAndIntegerIdsWithSameValueAreEqualAndHaveSameHashcode() { + ObjectIdentity obj = new ObjectIdentityImpl(Object.class, new Long(5)); + ObjectIdentity obj2 = new ObjectIdentityImpl(Object.class, new Integer(5)); + + assertEquals(obj, obj2); + assertEquals(obj.hashCode(), obj2.hashCode()); } -/* public void testHashCodeDifferentSerializableTypes() throws Exception { - ObjectIdentity obj = new ObjectIdentityImpl( - DOMAIN_CLASS, new Long(1)); - assertEquals(new ObjectIdentityImpl( - DOMAIN_CLASS, "1") - .hashCode(), obj.hashCode()); - assertEquals(new ObjectIdentityImpl( - DOMAIN_CLASS, - new Integer(1)).hashCode(), obj.hashCode()); - }*/ + @Test + public void equalStringIdsAreEqualAndHaveSameHashcode() throws Exception { + ObjectIdentity obj = new ObjectIdentityImpl(Object.class, "1000"); + ObjectIdentity obj2 = new ObjectIdentityImpl(Object.class, "1000"); + assertEquals(obj, obj2); + assertEquals(obj.hashCode(), obj2.hashCode()); + } @Test - public void testGetters() throws Exception { - ObjectIdentity obj = new ObjectIdentityImpl(DOMAIN_CLASS, new Long(1)); - assertEquals(new Long(1), obj.getIdentifier()); - assertEquals(MockIdDomainObject.class, obj.getJavaType()); + public void stringAndNumericIdsAreNotEqual() throws Exception { + ObjectIdentity obj = new ObjectIdentityImpl(Object.class, "1000"); + ObjectIdentity obj2 = new ObjectIdentityImpl(Object.class, Long.valueOf(1000)); + assertFalse(obj.equals(obj2)); } //~ Inner Classes ==================================================================================================