From 4fa92ff4c48c3cda568fc0c054e539424bd98eeb Mon Sep 17 00:00:00 2001 From: Christopher Chianelli Date: Fri, 22 Jul 2022 17:01:54 -0400 Subject: [PATCH] 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. To accomplish this, a seperate map is used for isGetters in PropertyUtils. Using a seperate map removes the need to have a seperate loop for identifing the actual getter method. It also make the logic more clear (first try "get" getter, then try "is" getter). --- .../deployment/recording/PropertyUtils.java | 8 ++++- .../recording/BytecodeRecorderTestCase.java | 12 +++---- .../recording/TestJavaBeanWithBoolean.java | 31 +++++++++++++++++-- 3 files changed, 41 insertions(+), 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 index a512b89548d22..02d3cf4dd5153 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/recording/PropertyUtils.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/recording/PropertyUtils.java @@ -23,6 +23,7 @@ public Property[] apply(Class type) { Method[] methods = type.getMethods(); Map getters = new HashMap<>(); + Map isGetters = new HashMap<>(); Map setters = new HashMap<>(); for (Method i : methods) { if (i.getName().startsWith("get") && i.getName().length() > 3 && i.getParameterCount() == 0 @@ -37,16 +38,21 @@ public Property[] apply(Class type) { } else if (i.getName().startsWith("is") && i.getName().length() > 3 && i.getParameterCount() == 0 && (i.getReturnType() == boolean.class || i.getReturnType() == Boolean.class)) { String name = Character.toLowerCase(i.getName().charAt(2)) + i.getName().substring(3); - getters.put(name, i); + isGetters.put(name, i); } else if (i.getName().startsWith("set") && i.getName().length() > 3 && i.getParameterCount() == 1) { String name = Character.toLowerCase(i.getName().charAt(3)) + i.getName().substring(4); setters.put(name, i); } } + Set names = new HashSet<>(getters.keySet()); + names.addAll(isGetters.keySet()); names.addAll(setters.keySet()); for (String i : names) { Method get = getters.get(i); + if (get == null) { + get = isGetters.get(i); // If there is no "get" getter, use the "is" getter + } Method set = setters.get(i); if (get == null) { ret.add(new Property(i, get, set, set.getParameterTypes()[0])); 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 index 85b3cde0ca5b3..e037ef0685176 100644 --- 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 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 index dbec2c9f92361..eed909521f6a1 100644 --- 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() { @@ -64,7 +70,26 @@ public Boolean isBoxedBoolWithIsGetter() { return boxedBoolWithIsGetter; } + // this is not actually a getter (takes a parameter) + public Boolean getBoxedBoolWithIsGetter(String parameter) { + return !boxedBoolWithIsGetter; + } + 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; + } }