You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ba...@apache.org on 2006/05/02 07:21:10 UTC
svn commit: r398812 - in /jakarta/commons/proper/lang/trunk/src:
java/org/apache/commons/lang/enum/Enum.java
java/org/apache/commons/lang/enums/Enum.java
test/org/apache/commons/lang/enum/EnumTest.java
test/org/apache/commons/lang/enums/EnumTest.java
Author: bayard
Date: Mon May 1 22:21:00 2006
New Revision: 398812
URL: http://svn.apache.org/viewcvs?rev=398812&view=rev
Log:
Fixing the lack of ClassLoader consideration in the compareTo methods of enum.Enum and enums.Enum, along with unit tests, as mentioned on Bugzilla entry #32619 by Kathy Van Stone.
Modified:
jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java
jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java
jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enum/EnumTest.java
jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enums/EnumTest.java
Modified: jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java?rev=398812&r1=398811&r2=398812&view=diff
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java (original)
+++ jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enum/Enum.java Mon May 1 22:21:00 2006
@@ -543,18 +543,10 @@
return iName.equals(((Enum) other).iName);
} else {
// This and other are in different class loaders, we must use reflection.
- try {
- Method mth = other.getClass().getMethod("getName", null);
- String name = (String) mth.invoke(other, null);
- return iName.equals(name);
- } catch (NoSuchMethodException e) {
- // ignore - should never happen
- } catch (IllegalAccessException e) {
- // ignore - should never happen
- } catch (InvocationTargetException e) {
- // ignore - should never happen
+ if (other.getClass().getName().equals(this.getClass().getName()) == false) {
+ return false;
}
- return false;
+ return iName.equals( getNameInOtherClassLoader(other) );
}
}
@@ -573,6 +565,9 @@
* <p>The default ordering is alphabetic by name, but this
* can be overridden by subclasses.</p>
*
+ * <p>If the parameter is in a different class loader than this instance,
+ * reflection is used to compare the names.</p>
+ *
* @see java.lang.Comparable#compareTo(Object)
* @param other the other object to compare to
* @return -ve if this is less than the other object, +ve if greater
@@ -584,7 +579,27 @@
if (other == this) {
return 0;
}
+ if (other.getClass() != this.getClass()) {
+ if (other.getClass().getName().equals(this.getClass().getName())) {
+ return iName.compareTo( getNameInOtherClassLoader(other) );
+ }
+ }
return iName.compareTo(((Enum) other).iName);
+ }
+
+ private String getNameInOtherClassLoader(Object other) {
+ try {
+ Method mth = other.getClass().getMethod("getName", null);
+ String name = (String) mth.invoke(other, null);
+ return name;
+ } catch (NoSuchMethodException e) {
+ // ignore - should never happen
+ } catch (IllegalAccessException e) {
+ // ignore - should never happen
+ } catch (InvocationTargetException e) {
+ // ignore - should never happen
+ }
+ throw new IllegalStateException("This should not happen");
}
/**
Modified: jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java?rev=398812&r1=398811&r2=398812&view=diff
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java (original)
+++ jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/enums/Enum.java Mon May 1 22:21:00 2006
@@ -543,18 +543,7 @@
if (other.getClass().getName().equals(this.getClass().getName()) == false) {
return false;
}
- try {
- Method mth = other.getClass().getMethod("getName", null);
- String name = (String) mth.invoke(other, null);
- return iName.equals(name);
- } catch (NoSuchMethodException e) {
- // ignore - should never happen
- } catch (IllegalAccessException e) {
- // ignore - should never happen
- } catch (InvocationTargetException e) {
- // ignore - should never happen
- }
- return false;
+ return iName.equals( getNameInOtherClassLoader(other) );
}
}
@@ -573,6 +562,9 @@
* <p>The default ordering is alphabetic by name, but this
* can be overridden by subclasses.</p>
*
+ * <p>If the parameter is in a different class loader than this instance,
+ * reflection is used to compare the names.</p>
+ *
* @see java.lang.Comparable#compareTo(Object)
* @param other the other object to compare to
* @return -ve if this is less than the other object, +ve if greater
@@ -584,7 +576,27 @@
if (other == this) {
return 0;
}
+ if (other.getClass() != this.getClass()) {
+ if (other.getClass().getName().equals(this.getClass().getName())) {
+ return iName.compareTo( getNameInOtherClassLoader(other) );
+ }
+ }
return iName.compareTo(((Enum) other).iName);
+ }
+
+ private String getNameInOtherClassLoader(Object other) {
+ try {
+ Method mth = other.getClass().getMethod("getName", null);
+ String name = (String) mth.invoke(other, null);
+ return name;
+ } catch (NoSuchMethodException e) {
+ // ignore - should never happen
+ } catch (IllegalAccessException e) {
+ // ignore - should never happen
+ } catch (InvocationTargetException e) {
+ // ignore - should never happen
+ }
+ throw new IllegalStateException("This should not happen");
}
/**
Modified: jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enum/EnumTest.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enum/EnumTest.java?rev=398812&r1=398811&r2=398812&view=diff
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enum/EnumTest.java (original)
+++ jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enum/EnumTest.java Mon May 1 22:21:00 2006
@@ -444,12 +444,12 @@
public void testColorEnumEqualsWithDifferentClassLoaders() throws SecurityException, IllegalArgumentException,
ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException {
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.BLUE);
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.GREEN);
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.RED);
+ this.testWithDifferentClassLoaders(ColorEnum.BLUE);
+ this.testWithDifferentClassLoaders(ColorEnum.GREEN);
+ this.testWithDifferentClassLoaders(ColorEnum.RED);
}
- void testEqualsTrueWithDifferentClassLoaders(ColorEnum colorEnum) throws ClassNotFoundException, SecurityException,
+ void testWithDifferentClassLoaders(ColorEnum colorEnum) throws ClassNotFoundException, SecurityException,
NoSuchMethodException, IllegalArgumentException, IllegalAccessException, InvocationTargetException {
// Sanity checks:
assertTrue(colorEnum.equals(colorEnum));
@@ -457,6 +457,7 @@
// set up:
ClassLoader classLoader = ClassUtilsTest.newSystemClassLoader();
Object enumObjectFromOtherClassLoader = this.getColorEnum(classLoader, colorEnum.getName());
+
// the real test, part 1.
try {
ColorEnum testCase = (ColorEnum) enumObjectFromOtherClassLoader;
@@ -464,16 +465,29 @@
} catch (ClassCastException e) {
// normal.
}
+
// the real test, part 2.
assertEquals("The two objects should match even though they are from different class loaders", colorEnum,
enumObjectFromOtherClassLoader);
- // the real test, part 3.
+
+ // the real test, part 3 - testing equals(Object)
int falseCount = 0;
for (Iterator iter = ColorEnum.iterator(); iter.hasNext();) {
ColorEnum element = (ColorEnum) iter.next();
if (!colorEnum.equals(element)) {
falseCount++;
assertFalse(enumObjectFromOtherClassLoader.equals(element));
+ }
+ }
+ assertEquals(ColorEnum.getEnumList().size() - 1, falseCount);
+
+ // the real test, part 4 - testing compareTo(Object) == 0
+ falseCount = 0;
+ for (Iterator iter = ColorEnum.iterator(); iter.hasNext();) {
+ ColorEnum element = (ColorEnum) iter.next();
+ if (!colorEnum.equals(element)) {
+ falseCount++;
+ assertFalse( ((Comparable)enumObjectFromOtherClassLoader).compareTo(element) == 0);
}
}
assertEquals(ColorEnum.getEnumList().size() - 1, falseCount);
Modified: jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enums/EnumTest.java
URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enums/EnumTest.java?rev=398812&r1=398811&r2=398812&view=diff
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enums/EnumTest.java (original)
+++ jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/enums/EnumTest.java Mon May 1 22:21:00 2006
@@ -442,12 +442,12 @@
public void testColorEnumEqualsWithDifferentClassLoaders() throws SecurityException, IllegalArgumentException,
ClassNotFoundException, NoSuchMethodException, IllegalAccessException, InvocationTargetException {
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.BLUE);
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.GREEN);
- this.testEqualsTrueWithDifferentClassLoaders(ColorEnum.RED);
+ this.testWithDifferentClassLoaders(ColorEnum.BLUE);
+ this.testWithDifferentClassLoaders(ColorEnum.GREEN);
+ this.testWithDifferentClassLoaders(ColorEnum.RED);
}
- void testEqualsTrueWithDifferentClassLoaders(ColorEnum colorEnum) throws ClassNotFoundException, SecurityException,
+ void testWithDifferentClassLoaders(ColorEnum colorEnum) throws ClassNotFoundException, SecurityException,
NoSuchMethodException, IllegalArgumentException, IllegalAccessException, InvocationTargetException {
// Sanity checks:
assertTrue(colorEnum.equals(colorEnum));
@@ -455,6 +455,7 @@
// set up:
ClassLoader classLoader = ClassUtilsTest.newSystemClassLoader();
Object enumObjectFromOtherClassLoader = this.getColorEnum(classLoader, colorEnum.getName());
+
// the real test, part 1.
try {
ColorEnum testCase = (ColorEnum) enumObjectFromOtherClassLoader;
@@ -462,16 +463,29 @@
} catch (ClassCastException e) {
// normal.
}
+
// the real test, part 2.
assertEquals("The two objects should match even though they are from different class loaders", colorEnum,
enumObjectFromOtherClassLoader);
- // the real test, part 3.
+
+ // the real test, part 3 - testing equals(Object)
int falseCount = 0;
for (Iterator iter = ColorEnum.iterator(); iter.hasNext();) {
ColorEnum element = (ColorEnum) iter.next();
if (!colorEnum.equals(element)) {
falseCount++;
assertFalse(enumObjectFromOtherClassLoader.equals(element));
+ }
+ }
+ assertEquals(ColorEnum.getEnumList().size() - 1, falseCount);
+
+ // the real test, part 4 - testing compareTo(Object) == 0
+ falseCount = 0;
+ for (Iterator iter = ColorEnum.iterator(); iter.hasNext();) {
+ ColorEnum element = (ColorEnum) iter.next();
+ if (!colorEnum.equals(element)) {
+ falseCount++;
+ assertFalse( ((Comparable)enumObjectFromOtherClassLoader).compareTo(element) == 0);
}
}
assertEquals(ColorEnum.getEnumList().size() - 1, falseCount);
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org