Improve the test for potentially ambiguous bean methods

jakartaee · May 22, 2023 · 5addb99 · 5addb99
1 parent 64198e9
commit 5addb99
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/api/src/main/java/jakarta/el/BeanSupportStandalone.java b/api/src/main/java/jakarta/el/BeanSupportStandalone.java
@@ -18,6 +18,7 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -31,6 +32,15 @@
  */
 class BeanSupportStandalone extends BeanSupport {
 
+    /*
+     * The full JavaBeans implementation has a much more detailed definition of method order that applies to an entire
+     * class. When ordering write methods for a single property, a much simpler comparator can be used because it is
+     * known that the method names are the same, the return parameters are both void and the methods only have a single
+     * parameter.
+     */
+    private static final Comparator<Method> WRITE_METHOD_COMPARATOR =
+            Comparator.comparing(m -> m.getParameterTypes()[0].getName());
+
     @Override
     BeanProperties getBeanProperties(Class<?> type) {
         return new BeanPropertiesStandalone(type);
@@ -132,6 +142,9 @@ Method getWriteMethod() {
                 if (readMethod != null) {
                     type = readMethod.getReturnType();
                 } else {
+                    if (writeMethods.size() > 1) {
+                        writeMethods.sort(WRITE_METHOD_COMPARATOR);
+                    }
                     type = writeMethods.get(0).getParameterTypes()[0];
                 }
                 for (Method candidate : writeMethods) {

diff --git a/api/src/test/java/jakarta/el/TestBeanSupport.java b/api/src/test/java/jakarta/el/TestBeanSupport.java
@@ -84,15 +84,17 @@ public void testMismatchBean(boolean useStandalone) {
         doTest(useStandalone, MismatchBean.class, "value", TypeA.class, TypeA.class, null);
     }
 
-    /*
-     * The first setter found "wins".
-     */
     @ParameterizedTest
     @MethodSource("data")
-    public void testAmbiguousBean(boolean useStandalone) {
-        doTest(useStandalone, AmbiguousBean.class, "value", TypeA.class, null, TypeA.class);
+    public void testAmbiguousBean01(boolean useStandalone) {
+        doTest(useStandalone, AmbiguousBean01.class, "value", TypeA.class, null, TypeA.class);
     }
 
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testAmbiguousBean02(boolean useStandalone) {
+        doTest(useStandalone, AmbiguousBean02.class, "value", TypeA.class, null, TypeA.class);
+    }
 
     private void doTest(boolean useStandalone, Class<?> clazz, String propertyName, Class<?> type, Class<?> typeGet,
             Class<?> typeSet) {
@@ -235,7 +237,7 @@ public void setValue(@SuppressWarnings("unused") String value) {
     }
 
 
-    public static class AmbiguousBean {
+    public static class AmbiguousBean01 {
         public void setValue(@SuppressWarnings("unused") TypeA value) {
         }
 
@@ -244,6 +246,15 @@ public void setValue(@SuppressWarnings("unused") String value) {
     }
 
 
+    public static class AmbiguousBean02 {
+        public void setValue(@SuppressWarnings("unused") String value) {
+        }
+
+        public void setValue(@SuppressWarnings("unused") TypeA value) {
+        }
+    }
+
+
     public static class TypeA {
     }