Consider protected getters in lazy JPA field checks, as well as publi…

…c ones
jqno · Jan 9, 2024 · 5feceb6 · 5feceb6
1 parent eaeebc6
commit 5feceb6
Show file tree

Hide file tree

Showing 5 changed files with 97 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Protected JPA entity getters were ignored. ([Issue 909](https://github.com/jqno/equalsverifier/issues/909))
+
 ## [3.15.5] - 2023-12-22
 
 ### Changed

diff --git a/...verifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java b/...verifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessor.java
@@ -85,7 +85,7 @@ public boolean declaresField(Field field) {
      * @return True if T has an {@code equals} method.
      */
     public boolean declaresEquals() {
-        return declaresMethod("equals", Object.class);
+        return declaresMethod(type, "equals", Object.class);
     }
 
     /**
@@ -94,21 +94,29 @@ public boolean declaresEquals() {
      * @return True if T has an {@code hashCode} method.
      */
     public boolean declaresHashCode() {
-        return declaresMethod("hashCode");
+        return declaresMethod(type, "hashCode");
     }
 
-    private boolean declaresMethod(String name, Class<?>... parameterTypes) {
-        try {
-            type.getDeclaredMethod(name, parameterTypes);
-            return true;
-        } catch (NoSuchMethodException e) {
-            return false;
+    /**
+     * Determines whether T has a method with the given name and parameters.
+     *
+     * @param name The name of the method we're looking for.
+     * @return True if T has a method with the given name and parameters.
+     */
+    public boolean hasMethod(String name) {
+        Class<?> t = type;
+        while (t != null) {
+            if (declaresMethod(t, name)) {
+                return true;
+            }
+            t = t.getSuperclass();
         }
+        return false;
     }
 
-    public boolean hasMethod(String name, Class<?>... parameterTypes) {
+    private static boolean declaresMethod(Class<?> type, String name, Class<?>... parameterTypes) {
         try {
-            type.getMethod(name, parameterTypes);
+            type.getDeclaredMethod(name, parameterTypes);
             return true;
         } catch (NoSuchMethodException e) {
             return false;

diff --git a/...rc/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java b/...rc/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java
@@ -171,6 +171,11 @@ public void getterNotUsedForGeneratedId() {
         );
     }
 
+    @Test
+    public void gettersAreUsedAndProtected() {
+        EqualsVerifier.forClass(ProtectedJakartaLazyFieldContainer.class).verify();
+    }
+
     private void getterNotUsed(Class<?> type, String method, Warning... additionalWarnings) {
         ExpectedException
             .when(() -> EqualsVerifier.forClass(type).suppress(additionalWarnings).verify())
@@ -688,4 +693,29 @@ public int hashCode() {
             return Objects.hash(getId());
         }
     }
+
+    @Entity
+    static class ProtectedJakartaLazyFieldContainer {
+
+        @Basic(fetch = FetchType.LAZY)
+        private String basic;
+
+        protected String getBasic() {
+            return basic;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof ProtectedJakartaLazyFieldContainer)) {
+                return false;
+            }
+            ProtectedJakartaLazyFieldContainer other = (ProtectedJakartaLazyFieldContainer) obj;
+            return Objects.equals(getBasic(), other.getBasic());
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(getBasic());
+        }
+    }
 }
diff --git a/...re/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java b/...re/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java
@@ -161,6 +161,11 @@ public void getterNotUsedForGeneratedId() {
         );
     }
 
+    @Test
+    public void gettersAreUsedAndProtected() {
+        EqualsVerifier.forClass(ProtectedJpaLazyFieldContainer.class).verify();
+    }
+
     private void getterNotUsed(Class<?> type, String method, Warning... additionalWarnings) {
         ExpectedException
             .when(() -> EqualsVerifier.forClass(type).suppress(additionalWarnings).verify())
@@ -675,4 +680,29 @@ public int hashCode() {
             return Objects.hash(getId());
         }
     }
+
+    @Entity
+    static class ProtectedJpaLazyFieldContainer {
+
+        @Basic(fetch = FetchType.LAZY)
+        private String basic;
+
+        protected String getBasic() {
+            return basic;
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (!(obj instanceof ProtectedJpaLazyFieldContainer)) {
+                return false;
+            }
+            ProtectedJpaLazyFieldContainer other = (ProtectedJpaLazyFieldContainer) obj;
+            return Objects.equals(getBasic(), other.getBasic());
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(getBasic());
+        }
+    }
 }
diff --git a/...fier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessorTest.java b/...fier-core/src/test/java/nl/jqno/equalsverifier/internal/reflection/ClassAccessorTest.java
@@ -106,16 +106,28 @@ public void hasMethod() {
         assertTrue(accessor.hasMethod("m"));
     }
 
+    @Test
+    public void hasProtectedMethod() {
+        ClassAccessor<?> accessor = ClassAccessor.of(MethodContainer.class, prefabValues);
+        assertTrue(accessor.hasMethod("m_protected"));
+    }
+
     @Test
     public void hasMethodInSuper() {
         ClassAccessor<?> accessor = ClassAccessor.of(ChildOfMethodContainer.class, prefabValues);
         assertTrue(accessor.hasMethod("m"));
     }
 
+    @Test
+    public void hasProtectedMethodInSuper() {
+        ClassAccessor<?> accessor = ClassAccessor.of(ChildOfMethodContainer.class, prefabValues);
+        assertTrue(accessor.hasMethod("m_protected"));
+    }
+
     @Test
     public void doesNotHaveMethod() {
         ClassAccessor<?> accessor = ClassAccessor.of(MethodContainer.class, prefabValues);
-        assertFalse(accessor.hasMethod("doesNotExist", int.class));
+        assertFalse(accessor.hasMethod("doesNotExist"));
     }
 
     @Test
@@ -358,6 +370,8 @@ static class DefaultValues {
     static class MethodContainer {
 
         public void m() {}
+
+        protected void m_protected() {}
     }
 
     static class ChildOfMethodContainer extends MethodContainer {}