Menu
▾
▴
[Pojomatic-commits] SF.net SVN: pojomatic:[149] trunk/PojomaticAll/Pojomatic/src
|
From: <iro...@us...> - 2009-09-05 16:31:56
|
Revision: 149
http://pojomatic.svn.sourceforge.net/pojomatic/?rev=149&view=rev
Author: iroberts
Date: 2009-09-05 16:31:50 +0000 (Sat, 05 Sep 2009)
Log Message:
-----------
OverridableMethods now tracks which property roles each method has, and will
raise a fuss if a method which previously had equals and no hashCode has the hashCode role added. Part of fixing bug 2845939.
This still needs documentation
Modified Paths:
--------------
trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/ClassProperties.java
trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/OverridableMethods.java
trunk/PojomaticAll/Pojomatic/src/test/java/org/pojomatic/internal/OverridableMethodsTest.java
Modified: trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/ClassProperties.java
===================================================================
--- trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/ClassProperties.java 2009-09-05 16:27:05 UTC (rev 148)
+++ trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/ClassProperties.java 2009-09-05 16:31:50 UTC (rev 149)
@@ -49,28 +49,26 @@
*/
private ClassProperties(Class<?> pojoClass) throws IllegalArgumentException {
if (pojoClass.isInterface()) {
- extractClassProperties(pojoClass, makeOverridableMethods());
+ extractClassProperties(pojoClass, new OverridableMethods());
}
else {
- walkHierarchy(pojoClass, makeOverridableMethods());
+ walkHierarchy(pojoClass, new OverridableMethods());
}
verifyPropertiesNotEmpty(pojoClass);
}
-
- private void walkHierarchy(
- Class<?> clazz, Map<PropertyRole, OverridableMethods> overridableMethods) {
+
+ private void walkHierarchy(Class<?> clazz, OverridableMethods overridableMethods) {
if (clazz != Object.class) {
walkHierarchy(clazz.getSuperclass(), overridableMethods);
extractClassProperties(clazz, overridableMethods);
}
}
- private void extractClassProperties(
- Class<?> clazz, Map<PropertyRole, OverridableMethods> overridableMethods) {
+ private void extractClassProperties(Class<?> clazz, OverridableMethods overridableMethods) {
AutoProperty autoProperty = clazz.getAnnotation(AutoProperty.class);
final DefaultPojomaticPolicy classPolicy =
(autoProperty != null) ? autoProperty.policy() : null;
- final AutoDetectPolicy autoDetectPolicy =
+ final AutoDetectPolicy autoDetectPolicy =
(autoProperty != null) ? autoProperty.autoDetect() : null;
extractFields(clazz, classPolicy, autoDetectPolicy);
@@ -78,10 +76,10 @@
}
private void extractMethods(
- Class<?> clazz,
+ Class<?> clazz,
final DefaultPojomaticPolicy classPolicy,
final AutoDetectPolicy autoDetectPolicy,
- final Map<PropertyRole, OverridableMethods> overridableMethods) {
+ final OverridableMethods overridableMethods) {
for (Method method : clazz.getDeclaredMethods()) {
Property property = method.getAnnotation(Property.class);
if (isStatic(method)) {
@@ -111,10 +109,9 @@
/* add all methods that are explicitly annotated or auto-detected, and not overriding already
* added methods */
if (propertyPolicy != null || AutoDetectPolicy.METHOD == autoDetectPolicy) {
- for (PropertyRole role : PropertyFilter.getRoles(propertyPolicy, classPolicy)) {
- if(overridableMethods.get(role).checkAndMaybeAddMethod(method)) {
- properties.get(role).add(new PropertyAccessor(method, getPropertyName(property)));
- }
+ for (PropertyRole role : overridableMethods.checkAndMaybeAddRolesToMethod(
+ method, PropertyFilter.getRoles(propertyPolicy, classPolicy))) {
+ properties.get(role).add(new PropertyAccessor(method, getPropertyName(property)));
}
}
}
@@ -210,15 +207,4 @@
}
return properties;
}
-
- private Map<PropertyRole, OverridableMethods> makeOverridableMethods() {
- Map<PropertyRole, OverridableMethods> overrideableMethods =
- new EnumMap<PropertyRole, OverridableMethods>(PropertyRole.class);
- for (PropertyRole role : PropertyRole.values()) {
- overrideableMethods.put(role, new OverridableMethods());
- }
- return overrideableMethods;
-
- }
-
}
Modified: trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/OverridableMethods.java
===================================================================
--- trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/OverridableMethods.java 2009-09-05 16:27:05 UTC (rev 148)
+++ trunk/PojomaticAll/Pojomatic/src/main/java/org/pojomatic/internal/OverridableMethods.java 2009-09-05 16:31:50 UTC (rev 149)
@@ -2,38 +2,65 @@
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
-import java.util.HashSet;
-import java.util.Set;
+import java.util.*;
/**
* A mutable set of methods which can be overridden. All methods are assumed to take no arguments
* and either public, protected or package private.
*/
class OverridableMethods {
- /**
- * Check a method to see if it is not an override; if not, add it to the collection of methods
- *
- * @param method the method to check and maybe add. It is assumed the method is not private.
- * @return {@code true} if the method is not an override
- */
- boolean checkAndMaybeAddMethod(Method method) {
+
+ Set<PropertyRole> checkAndMaybeAddRolesToMethod(Method method, Set<PropertyRole> newRoles) {
+ Set<PropertyRole> existingRoles = findExistingRoles(method);
+ if (existingRoles.contains(PropertyRole.EQUALS)
+ && !existingRoles.contains(PropertyRole.HASH_CODE)
+ && newRoles.contains(PropertyRole.HASH_CODE)) {
+ throw new IllegalArgumentException(
+ "Method " + method.getDeclaringClass().getName() + "." + method.getName()
+ + " is requested to be included in hashCode computations, but already overrides a method"
+ + " which is requested for equals computations, but not hashCode computations.");
+ }
+ Set<PropertyRole> addedRoles = EnumSet.noneOf(PropertyRole.class);
+ for (PropertyRole role : newRoles) {
+ if (!existingRoles.contains(role)) {
+ addedRoles.add(role);
+ existingRoles.add(role);
+ }
+ }
+ return addedRoles;
+ }
+
+ private Set<PropertyRole> findExistingRoles(Method method) {
+ Set<PropertyRole> existingRoles;
if (isPackagePrivate(method)) {
// This can only override another package private method
- return packageMethods.add(new PackageMethod(method));
+ PackageMethod key = new PackageMethod(method);
+ existingRoles = packageMethods.get(key);
+ if (existingRoles == null) {
+ existingRoles = EnumSet.noneOf(PropertyRole.class);
+ packageMethods.put(key, existingRoles);
+ }
}
else {
// If there is a public method already declared, then this is an override. Otherwise,
// we need to track it as a public override going forward, even if it is overriding a
// superclass method which was declared package private.
- return publicOrProtectedMethods.add(method.getName())
- && !packageMethods.contains(new PackageMethod(method));
+ existingRoles = publicOrProtectedMethods.get(method.getName());
+ if (existingRoles == null) {
+ existingRoles = packageMethods.get(new PackageMethod(method));
+ }
+ if (existingRoles == null) {
+ existingRoles = EnumSet.noneOf(PropertyRole.class);
+ publicOrProtectedMethods.put(method.getName(), existingRoles);
+ }
}
+ return existingRoles;
}
/**
* A bean to track the package and name of a package-private method
*/
- private class PackageMethod {
+ private static class PackageMethod {
PackageMethod(Method method) {
name = method.getName();
pakage = method.getDeclaringClass().getPackage();
@@ -62,9 +89,12 @@
}
}
- private Set<String> publicOrProtectedMethods = new HashSet<String>();
- private Set<PackageMethod> packageMethods = new HashSet<PackageMethod>();
+ private final Map<String, Set<PropertyRole>> publicOrProtectedMethods =
+ new HashMap<String, Set<PropertyRole>>();
+ private final Map<PackageMethod, Set<PropertyRole>> packageMethods =
+ new HashMap<PackageMethod, Set<PropertyRole>>();
+
private static boolean isPackagePrivate(Method method) {
return !(Modifier.isPublic(method.getModifiers())
|| Modifier.isProtected(method.getModifiers()));
Modified: trunk/PojomaticAll/Pojomatic/src/test/java/org/pojomatic/internal/OverridableMethodsTest.java
===================================================================
--- trunk/PojomaticAll/Pojomatic/src/test/java/org/pojomatic/internal/OverridableMethodsTest.java 2009-09-05 16:27:05 UTC (rev 148)
+++ trunk/PojomaticAll/Pojomatic/src/test/java/org/pojomatic/internal/OverridableMethodsTest.java 2009-09-05 16:31:50 UTC (rev 149)
@@ -3,6 +3,7 @@
import static org.junit.Assert.*;
import java.lang.reflect.Method;
+import java.util.EnumSet;
import org.junit.Test;
import org.pojomatic.internal.a.C1;
@@ -10,8 +11,11 @@
import org.pojomatic.internal.b.C2;
import org.pojomatic.internal.b.C4;
-
public class OverridableMethodsTest {
+ private static final EnumSet<PropertyRole> EQUALS_HASH_CODE =
+ EnumSet.of(PropertyRole.EQUALS, PropertyRole.HASH_CODE);
+ private static final EnumSet<PropertyRole> EQUALS = EnumSet.of(PropertyRole.EQUALS);
+ private static final EnumSet<PropertyRole> ALL = EnumSet.allOf(PropertyRole.class);
@Test public void testPackagePrivate() throws Exception {
checkMethod("packagePrivate", true, true, false, false);
@@ -32,15 +36,86 @@
@Test public void testPublic() throws Exception {
checkMethod("publicMethod", true, false, false, false);
}
-
+
+ @Test
+ public void testEqualsThenEquals() throws Exception {
+ OverridableMethods overridableMethods = new OverridableMethods();
+ assertEquals(
+ EQUALS, overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, "publicMethod"), EQUALS));
+ assertEquals(
+ EnumSet.noneOf(PropertyRole.class), overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, "publicMethod"), EQUALS));
+ }
+
+ @Test
+ public void testEqualsThenToString() throws Exception {
+ OverridableMethods overridableMethods = new OverridableMethods();
+ assertEquals(
+ EQUALS, overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, "publicMethod"), EQUALS));
+ assertEquals(
+ EnumSet.of(PropertyRole.TO_STRING), overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, "publicMethod"), EnumSet.of(PropertyRole.TO_STRING)));
+ }
+
+ @Test
+ public void testEqualsThenToEqualsString() throws Exception {
+ OverridableMethods overridableMethods = new OverridableMethods();
+ assertEquals(
+ EQUALS, overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, "publicMethod"), EQUALS));
+ assertEquals(
+ EnumSet.of(PropertyRole.TO_STRING), overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, "publicMethod"), EnumSet.of(PropertyRole.TO_STRING, PropertyRole.EQUALS)));
+ }
+
+ @Test
+ public void testEqualsHashCodeThenEqualsHashCode() throws Exception {
+ OverridableMethods overridableMethods = new OverridableMethods();
+ assertEquals(
+ EQUALS_HASH_CODE, overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, "publicMethod"), EQUALS_HASH_CODE));
+ assertEquals(
+ EnumSet.noneOf(PropertyRole.class), overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, "publicMethod"), EQUALS_HASH_CODE));
+ }
+
+ @Test
+ public void testEqualsThenEqualsHashCode() throws Exception {
+ OverridableMethods overridableMethods = new OverridableMethods();
+ assertEquals(
+ EQUALS, overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, "publicMethod"), EQUALS));
+ try {
+ overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, "publicMethod"), EQUALS_HASH_CODE);
+ fail("Exception expected");
+ }
+ catch (IllegalArgumentException e) {
+ assertEquals(
+ "Method org.pojomatic.internal.b.C2.publicMethod is requested to be included in hashCode"
+ + " computations, but already overrides a method which is requested for"
+ + " equals computations, but not hashCode computations.", e.getMessage());
+ }
+ }
+
private void checkMethod(
- String methodName, boolean c1Add, boolean c2Add, boolean c3Add, boolean c4Add)
- throws Exception {
+ String methodName, boolean c1Add, boolean c2Add, boolean c3Add, boolean c4Add)
+ throws Exception {
OverridableMethods overridableMethods = new OverridableMethods();
- assertEquals(c1Add, overridableMethods.checkAndMaybeAddMethod(method(C1.class, methodName)));
- assertEquals(c2Add, overridableMethods.checkAndMaybeAddMethod(method(C2.class, methodName)));
- assertEquals(c3Add, overridableMethods.checkAndMaybeAddMethod(method(C3.class, methodName)));
- assertEquals(c4Add, overridableMethods.checkAndMaybeAddMethod(method(C4.class, methodName)));
+ assertEquals(
+ c1Add, !overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C1.class, methodName), ALL).isEmpty());
+ assertEquals(
+ c2Add, !overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C2.class, methodName), ALL).isEmpty());
+ assertEquals(
+ c3Add, !overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C3.class, methodName), ALL).isEmpty());
+ assertEquals(
+ c4Add, !overridableMethods.checkAndMaybeAddRolesToMethod(
+ method(C4.class, methodName), ALL).isEmpty());
}
private static Method method(Class<?> clazz, String name) throws Exception {
This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.
|
