commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bay...@apache.org
Subject 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
Date Tue, 02 May 2006 05:21:10 GMT
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


Mime
View raw message