fix #26899: Prefer get getters over is getters in recorders

When both get getters and is getters exist, they can replace the original selected getter in PropertyUtils. This can creates inconsistencies, resulting in a field not being set, causing a NPE later down the line that is hard to diagnose. To remove inconsistencies, if there is both a get method and an is method, the get method is always preferred. This introduces consistent behavior for recorders.
quarkusio · Jul 22, 2022 · 399d311 · 399d311
1 parent 3f51f74
commit 399d311
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 10 deletions.
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/recording/PropertyUtils.java b/core/deployment/src/main/java/io/quarkus/deployment/recording/PropertyUtils.java
@@ -22,9 +22,32 @@ public Property[] apply(Class<?> type) {
             List<Property> ret = new ArrayList<>();
             Method[] methods = type.getMethods();
 
+            List<Method> checkedMethodList = new ArrayList<>();
+
+            for (Method method : methods) {
+                if (method.getName().startsWith("get")) {
+                    checkedMethodList.add(method);
+                } else if (method.getName().startsWith("is")) {
+                    boolean hasDuplicate = false;
+                    for (Method maybeDuplicate : methods) {
+                        // prefer get method over is method
+                        if (maybeDuplicate.getName().equals("get" + method.getName().substring(2))) {
+                            hasDuplicate = true;
+                            break;
+                        }
+                    }
+
+                    if (!hasDuplicate) {
+                        checkedMethodList.add(method);
+                    }
+                } else if (method.getName().startsWith("set")) {
+                    checkedMethodList.add(method);
+                }
+            }
+
             Map<String, Method> getters = new HashMap<>();
             Map<String, Method> setters = new HashMap<>();
-            for (Method i : methods) {
+            for (Method i : checkedMethodList) {
                 if (i.getName().startsWith("get") && i.getName().length() > 3 && i.getParameterCount() == 0
                         && i.getReturnType() != void.class) {
                     String name = Character.toLowerCase(i.getName().charAt(3)) + i.getName().substring(4);

diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java
@@ -341,19 +341,19 @@ public void testObjects() throws Exception {
     public void testJavaBeanWithBoolean() throws Exception {
         runTest(generator -> {
             TestRecorder recorder = generator.getRecordingProxy(TestRecorder.class);
-            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(true, true, true);
+            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(true, true, true, true);
             recorder.bean(newBean);
-        }, new TestJavaBeanWithBoolean(true, true, true));
+        }, new TestJavaBeanWithBoolean(true, true, true, true));
         runTest(generator -> {
             TestRecorder recorder = generator.getRecordingProxy(TestRecorder.class);
-            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(false, false, false);
+            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(false, false, false, false);
             recorder.bean(newBean);
-        }, new TestJavaBeanWithBoolean(false, false, false));
+        }, new TestJavaBeanWithBoolean(false, false, false, false));
         runTest(generator -> {
             TestRecorder recorder = generator.getRecordingProxy(TestRecorder.class);
-            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(true, null, null);
+            TestJavaBeanWithBoolean newBean = new TestJavaBeanWithBoolean(true, null, null, null);
             recorder.bean(newBean);
-        }, new TestJavaBeanWithBoolean(true, null, null));
+        }, new TestJavaBeanWithBoolean(true, null, null, null));
     }
 
     void runTest(Consumer<BytecodeRecorderImpl> generator, Object... expected) throws Exception {

diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/TestJavaBeanWithBoolean.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestJavaBeanWithBoolean.java
@@ -8,13 +8,17 @@ public class TestJavaBeanWithBoolean {
     private Boolean boxedBool;
     private Boolean boxedBoolWithIsGetter;
 
+    private Boolean boxedBoolWithIsAndGetGetters;
+
     public TestJavaBeanWithBoolean() {
     }
 
-    public TestJavaBeanWithBoolean(boolean bool, Boolean boxedBool, Boolean boxedBoolWithIsGetter) {
+    public TestJavaBeanWithBoolean(boolean bool, Boolean boxedBool, Boolean boxedBoolWithIsGetter,
+            Boolean boxedBoolWithIsAndGetGetters) {
         this.bool = bool;
         this.boxedBool = boxedBool;
         this.boxedBoolWithIsGetter = boxedBoolWithIsGetter;
+        this.boxedBoolWithIsAndGetGetters = boxedBoolWithIsAndGetGetters;
     }
 
     @Override
@@ -23,6 +27,7 @@ public String toString() {
                 "bool=" + bool +
                 ", boxedBool=" + boxedBool +
                 ", boxedBoolWithIsGetter=" + boxedBoolWithIsGetter +
+                ", boxedBoolWithIsAndGetGetters=" + boxedBoolWithIsAndGetGetters +
                 '}';
     }
 
@@ -34,12 +39,13 @@ public boolean equals(Object o) {
             return false;
         TestJavaBeanWithBoolean that = (TestJavaBeanWithBoolean) o;
         return bool == that.bool && Objects.equals(boxedBool, that.boxedBool)
-                && Objects.equals(boxedBoolWithIsGetter, that.boxedBoolWithIsGetter);
+                && Objects.equals(boxedBoolWithIsGetter, that.boxedBoolWithIsGetter)
+                && Objects.equals(boxedBoolWithIsAndGetGetters, that.boxedBoolWithIsAndGetGetters);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(bool, boxedBool, boxedBoolWithIsGetter);
+        return Objects.hash(bool, boxedBool, boxedBoolWithIsGetter, boxedBoolWithIsAndGetGetters);
     }
 
     public boolean isBool() {
@@ -67,4 +73,18 @@ public Boolean isBoxedBoolWithIsGetter() {
     public void setBoxedBoolWithIsGetter(Boolean boxedBoolWithIsGetter) {
         this.boxedBoolWithIsGetter = boxedBoolWithIsGetter;
     }
+
+    // method unwraps boxedBoolWithIsAndGetGetters to a default value if it is null
+    public boolean isBoxedBoolWithIsAndGetGetters() {
+        return (boxedBoolWithIsAndGetGetters != null) ? boxedBoolWithIsAndGetGetters : true;
+    }
+
+    // Using both the 'is' prefix and the 'get' prefix, to check the property still get set if there are two getters
+    public Boolean getBoxedBoolWithIsAndGetGetters() {
+        return boxedBoolWithIsAndGetGetters;
+    }
+
+    public void setBoxedBoolWithIsAndGetGetters(Boolean boxedBoolWithIsAndGetGetters) {
+        this.boxedBoolWithIsAndGetGetters = boxedBoolWithIsAndGetGetters;
+    }
 }