From 55515c07789d8683f965e4bf7f0ec8b3d2702e18 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Mon, 3 Feb 2020 21:51:24 -0500 Subject: [PATCH] WW-5043 Enum comparison failure fix: - Improve OgnlOps.compareWithConversion() to handle special case of enumerated types (Enum) with element bodies. - Add additional equality and inequality unit tests for enumerated types. --- src/java/ognl/OgnlOps.java | 4 + .../ArithmeticAndLogicalOperatorsTest.java | 74 ++++++++++++++++++- 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/src/java/ognl/OgnlOps.java b/src/java/ognl/OgnlOps.java index 40621e25..7e990df7 100644 --- a/src/java/ognl/OgnlOps.java +++ b/src/java/ognl/OgnlOps.java @@ -89,6 +89,10 @@ public static int compareWithConversion(Object v1, Object v2) if ((v1 instanceof Comparable) && v1.getClass().isAssignableFrom(v2.getClass())) { result = ((Comparable) v1).compareTo(v2); break; + } else if ((v1 instanceof Enum && v2 instanceof Enum) && + (v1.getClass() == v2.getClass() || ((Enum) v1).getDeclaringClass() == ((Enum) v2).getDeclaringClass())) { + result = ((Enum) v1).compareTo(v2); + break; } else { throw new IllegalArgumentException("invalid comparison: " + v1.getClass().getName() + " and " + v2.getClass().getName()); diff --git a/src/test/java/org/ognl/test/ArithmeticAndLogicalOperatorsTest.java b/src/test/java/org/ognl/test/ArithmeticAndLogicalOperatorsTest.java index 359be78c..3546cdf7 100644 --- a/src/test/java/org/ognl/test/ArithmeticAndLogicalOperatorsTest.java +++ b/src/test/java/org/ognl/test/ArithmeticAndLogicalOperatorsTest.java @@ -37,6 +37,12 @@ public class ArithmeticAndLogicalOperatorsTest extends OgnlTestCase { + public enum EnumNoBody { ENUM1, ENUM2; }; // Basic enumeration + public enum EnumEmptyBody { ENUM1{}, ENUM2{}; }; // Enumeration whose elements have (empty) bodies + public enum EnumBasicBody { ENUM1{ public final Integer value() { return Integer.valueOf(10);} }, + ENUM2{ public final Integer value() { return Integer.valueOf(20);} }; }; // Enumeration whose elements have bodies + protected static final String FULLY_QUALIFIED_CLASSNAME = ArithmeticAndLogicalOperatorsTest.class.getName(); + private static Object[][] TESTS = { // Double-valued arithmetic expressions { "-1d", new Double(-1) }, @@ -166,7 +172,73 @@ public class ArithmeticAndLogicalOperatorsTest extends OgnlTestCase { "#y == 1", Boolean.TRUE }, { "#y == \"1\"", Boolean.TRUE }, { "#y + \"1\"", "11" }, - { "\"1\" + #y", "11" } + { "\"1\" + #y", "11" }, + + // Enumerated type equality and inequality comparisons (with and without element bodies, reversing order for completeness). + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", Boolean.FALSE }, + + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", Boolean.FALSE }, + + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", Boolean.FALSE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", Boolean.TRUE }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", Boolean.FALSE }, + + // As per JDK JavaDocs it is only possible to compare Enum elements of the same type. Attempting to compare different types + // will normally result in ClassCastExceptions. However, OGNL should avoid that and produce an IllegalArgumentException instead. + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumNoBody@ENUM2", IllegalArgumentException.class }, + + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM2", IllegalArgumentException.class }, + + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM1", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 == @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class }, + { "@" + FULLY_QUALIFIED_CLASSNAME + "$EnumBasicBody@ENUM1 != @" + FULLY_QUALIFIED_CLASSNAME + "$EnumEmptyBody@ENUM2", IllegalArgumentException.class } }; /*