diff --git a/CHANGELOG.md b/CHANGELOG.md index 696b68d5e..c8def7cd1 100644 --- 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] +### Changed + +- Check that getters are used for all generated JPA id fields. ([Issue 892](https://github.com/jqno/equalsverifier/issues/892)) + ## [3.15.4] - 2023-11-29 ### Added diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java index 9a5577d05..8aaeccdbe 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/JpaLazyGetterFieldCheck.java @@ -132,7 +132,7 @@ private void assertEntity( ) { assertTrue( Formatter.of( - "JPA Entity: direct reference to field %% used in %% instead of getter %%.", + "JPA Entity: direct reference to field %% used in %% instead of getter %%().", fieldName, method, getterName diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java index 439365077..b405437d4 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java @@ -207,11 +207,13 @@ public void postProcess(Set> types, AnnotationCache annotationCache) { "javax.persistence.ManyToOne", "javax.persistence.ManyToMany", "javax.persistence.ElementCollection", + "javax.persistence.GeneratedValue", "jakarta.persistence.OneToOne", "jakarta.persistence.OneToMany", "jakarta.persistence.ManyToOne", "jakarta.persistence.ManyToMany", - "jakarta.persistence.ElementCollection" + "jakarta.persistence.ElementCollection", + "jakarta.persistence.GeneratedValue" ), /** diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java index 230cac9ca..e0eeb43a3 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JakartaLazyEntityTest.java @@ -1,13 +1,6 @@ package nl.jqno.equalsverifier.integration.extra_features; -import jakarta.persistence.Basic; -import jakarta.persistence.ElementCollection; -import jakarta.persistence.Entity; -import jakarta.persistence.FetchType; -import jakarta.persistence.ManyToMany; -import jakarta.persistence.ManyToOne; -import jakarta.persistence.OneToMany; -import jakarta.persistence.OneToOne; +import jakarta.persistence.*; import java.util.Arrays; import java.util.Objects; import nl.jqno.equalsverifier.EqualsVerifier; @@ -53,7 +46,7 @@ public void basicGetterNotUsed_givenWarningSuppressed() { } @Test - public void basicGetterUsed_givenAnnotationIsOnGetter() { + public void basicGetterNotUsed_givenAnnotationIsOnGetter() { getterNotUsed(IncorrectBasicJakartaLazyGetterContainer.class, "equals"); getterNotUsed_warningSuppressed(IncorrectBasicJakartaLazyGetterContainer.class); } @@ -148,17 +141,48 @@ public void differentCodingStyle_multiple() { .verify(); } - private void getterNotUsed(Class type, String method) { + @Test + public void getterUsedForGeneratedId() { + EqualsVerifier + .forClass(CorrectGeneratedJpaIdContainer.class) + .suppress(Warning.SURROGATE_KEY) + .verify(); + EqualsVerifier + .forClass(CorrectGeneratedJpaIdContainer.class) + .suppress(Warning.SURROGATE_OR_BUSINESS_KEY) + .verify(); + } + + @Test + public void getterNotUsedForGeneratedId() { + getterNotUsed(IncorrectGeneratedJpaIdContainer.class, "equals", Warning.SURROGATE_KEY); + getterNotUsed_warningSuppressed( + IncorrectGeneratedJpaIdContainer.class, + Warning.SURROGATE_KEY + ); + getterNotUsed( + IncorrectGeneratedJpaIdContainer.class, + "equals", + Warning.SURROGATE_OR_BUSINESS_KEY + ); + getterNotUsed_warningSuppressed( + IncorrectGeneratedJpaIdContainer.class, + Warning.SURROGATE_OR_BUSINESS_KEY + ); + } + + private void getterNotUsed(Class type, String method, Warning... additionalWarnings) { ExpectedException - .when(() -> EqualsVerifier.forClass(type).suppress(Warning.NONFINAL_FIELDS).verify()) + .when(() -> EqualsVerifier.forClass(type).suppress(additionalWarnings).verify()) .assertFailure() .assertMessageContains("JPA Entity", method, "direct reference"); } - private void getterNotUsed_warningSuppressed(Class type) { + private void getterNotUsed_warningSuppressed(Class type, Warning... additionalWarnings) { EqualsVerifier .forClass(type) - .suppress(Warning.JPA_GETTER, Warning.NONFINAL_FIELDS) + .suppress(Warning.JPA_GETTER) + .suppress(additionalWarnings) .verify(); } @@ -612,4 +636,56 @@ public int hashCode() { return Objects.hash(getOneToMany(), getManyToOne()); } } + + @Entity + static class CorrectGeneratedJpaIdContainer { + + @Id + @GeneratedValue + private String id; + + public String getId() { + return id; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CorrectGeneratedJpaIdContainer)) { + return false; + } + CorrectGeneratedJpaIdContainer other = (CorrectGeneratedJpaIdContainer) obj; + return Objects.equals(getId(), other.getId()); + } + + @Override + public int hashCode() { + return Objects.hash(getId()); + } + } + + @Entity + static class IncorrectGeneratedJpaIdContainer { + + @Id + @GeneratedValue + private String id; + + public String getId() { + return id; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof IncorrectGeneratedJpaIdContainer)) { + return false; + } + IncorrectGeneratedJpaIdContainer other = (IncorrectGeneratedJpaIdContainer) obj; + return Objects.equals(id, other.id); + } + + @Override + public int hashCode() { + return Objects.hash(getId()); + } + } } diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java index 273046afc..cf47b5c93 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaLazyEntityTest.java @@ -5,14 +5,7 @@ import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Basic; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.ElementCollection; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Entity; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.FetchType; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.ManyToMany; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.ManyToOne; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.OneToMany; -import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.OneToOne; +import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.*; import org.junit.jupiter.api.Test; // CHECKSTYLE OFF: HiddenField @@ -46,7 +39,7 @@ public void basicGetterNotUsed_givenWarningSuppressed() { } @Test - public void basicGetterUsed_givenAnnotationIsOnGetter() { + public void basicGetterNotUsed_givenAnnotationIsOnGetter() { getterNotUsed(IncorrectBasicJpaLazyGetterContainer.class, "equals"); getterNotUsed_warningSuppressed(IncorrectBasicJpaLazyGetterContainer.class); } @@ -138,15 +131,49 @@ public void differentCodingStyle_multiple() { .verify(); } - private void getterNotUsed(Class type, String method) { + @Test + public void getterUsedForGeneratedId() { + EqualsVerifier + .forClass(CorrectGeneratedJpaIdContainer.class) + .suppress(Warning.SURROGATE_KEY) + .verify(); + EqualsVerifier + .forClass(CorrectGeneratedJpaIdContainer.class) + .suppress(Warning.SURROGATE_OR_BUSINESS_KEY) + .verify(); + } + + @Test + public void getterNotUsedForGeneratedId() { + getterNotUsed(IncorrectGeneratedJpaIdContainer.class, "equals", Warning.SURROGATE_KEY); + getterNotUsed_warningSuppressed( + IncorrectGeneratedJpaIdContainer.class, + Warning.SURROGATE_KEY + ); + getterNotUsed( + IncorrectGeneratedJpaIdContainer.class, + "equals", + Warning.SURROGATE_OR_BUSINESS_KEY + ); + getterNotUsed_warningSuppressed( + IncorrectGeneratedJpaIdContainer.class, + Warning.SURROGATE_OR_BUSINESS_KEY + ); + } + + private void getterNotUsed(Class type, String method, Warning... additionalWarnings) { ExpectedException - .when(() -> EqualsVerifier.forClass(type).verify()) + .when(() -> EqualsVerifier.forClass(type).suppress(additionalWarnings).verify()) .assertFailure() .assertMessageContains("JPA Entity", method, "direct reference"); } - private void getterNotUsed_warningSuppressed(Class type) { - EqualsVerifier.forClass(type).suppress(Warning.JPA_GETTER).verify(); + private void getterNotUsed_warningSuppressed(Class type, Warning... additionalWarnings) { + EqualsVerifier + .forClass(type) + .suppress(Warning.JPA_GETTER) + .suppress(additionalWarnings) + .verify(); } @Entity @@ -596,4 +623,56 @@ public int hashCode() { return Objects.hash(getOneToMany(), getManyToOne()); } } + + @Entity + static class CorrectGeneratedJpaIdContainer { + + @Id + @GeneratedValue + private String id; + + public String getId() { + return id; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof CorrectGeneratedJpaIdContainer)) { + return false; + } + CorrectGeneratedJpaIdContainer other = (CorrectGeneratedJpaIdContainer) obj; + return Objects.equals(getId(), other.getId()); + } + + @Override + public int hashCode() { + return Objects.hash(getId()); + } + } + + @Entity + static class IncorrectGeneratedJpaIdContainer { + + @Id + @GeneratedValue + private String id; + + public String getId() { + return id; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof IncorrectGeneratedJpaIdContainer)) { + return false; + } + IncorrectGeneratedJpaIdContainer other = (IncorrectGeneratedJpaIdContainer) obj; + return Objects.equals(id, other.id); + } + + @Override + public int hashCode() { + return Objects.hash(getId()); + } + } } diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/annotations/javax/persistence/GeneratedValue.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/annotations/javax/persistence/GeneratedValue.java new file mode 100644 index 000000000..ae6fa4d7d --- /dev/null +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/testhelpers/annotations/javax/persistence/GeneratedValue.java @@ -0,0 +1,11 @@ +package nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target({ ElementType.METHOD, ElementType.FIELD }) +@Retention(RetentionPolicy.RUNTIME) +public @interface GeneratedValue { +}