From 2fad0ea976b3c0a294ae3c61b6ea7749dbf2e394 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 29 Jun 2023 13:48:54 +0200 Subject: [PATCH 1/8] Allow different field and getter method naming standards --- .../ConfiguredEqualsVerifier.java | 19 +++++-- .../equalsverifier/api/EqualsVerifierApi.java | 14 ++++++ .../api/MultipleTypeEqualsVerifierApi.java | 10 ++++ .../api/SingleTypeEqualsVerifierApi.java | 39 +++++++++++++-- .../fieldchecks/JpaLazyGetterFieldCheck.java | 6 ++- .../internal/util/Configuration.java | 22 ++++++++- .../extra_features/JpaLazyEntityTest.java | 49 +++++++++++++++++++ 7 files changed, 150 insertions(+), 9 deletions(-) diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java index f1e5a6b6e..edddea870 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java @@ -3,6 +3,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.List; +import java.util.function.Function; import nl.jqno.equalsverifier.Func.Func1; import nl.jqno.equalsverifier.Func.Func2; import nl.jqno.equalsverifier.api.EqualsVerifierApi; @@ -20,21 +21,24 @@ public final class ConfiguredEqualsVerifier implements EqualsVerifierApi { private final EnumSet warningsToSuppress; private final FactoryCache factoryCache; private boolean usingGetClass; + private Function fieldnameToGetter; /** Constructor. */ public ConfiguredEqualsVerifier() { - this(EnumSet.noneOf(Warning.class), new FactoryCache(), false); + this(EnumSet.noneOf(Warning.class), new FactoryCache(), false, null); } /** Private constructor. For internal use only. */ private ConfiguredEqualsVerifier( EnumSet warningsToSuppress, FactoryCache factoryCache, - boolean usingGetClass + boolean usingGetClass, + Function fieldnameToGetter ) { this.warningsToSuppress = warningsToSuppress; this.factoryCache = factoryCache; this.usingGetClass = usingGetClass; + this.fieldnameToGetter = fieldnameToGetter; } /** @@ -46,7 +50,8 @@ public ConfiguredEqualsVerifier copy() { return new ConfiguredEqualsVerifier( EnumSet.copyOf(warningsToSuppress), new FactoryCache().merge(factoryCache), - usingGetClass + usingGetClass, + fieldnameToGetter ); } @@ -91,6 +96,14 @@ public ConfiguredEqualsVerifier usingGetClass() { return this; } + @Override + public ConfiguredEqualsVerifier withFieldnameToGetterConverter( + Function converter + ) { + this.fieldnameToGetter = converter; + return this; + } + /** {@inheritDoc} */ @Override public ConfiguredEqualsVerifier withResetCaches() { diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/EqualsVerifierApi.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/EqualsVerifierApi.java index 70a753954..fbbc5e1e4 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/EqualsVerifierApi.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/EqualsVerifierApi.java @@ -1,5 +1,6 @@ package nl.jqno.equalsverifier.api; +import java.util.function.Function; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Func.Func1; import nl.jqno.equalsverifier.Func.Func2; @@ -70,6 +71,19 @@ public interface EqualsVerifierApi { */ EqualsVerifierApi usingGetClass(); + /** + * Determines how a getter name can be derived from a field name. + * + * The default behavior is to uppercase the field's first letter and prepend 'get'. For + * instance, a field name 'employee' would correspond to getter name 'getEmployee'. + * + * This method can be used if your project has a different naming convention. + * + * @param converter A function that converts from field name to getter name. + * @return {@code this}, for easy method chaining. + */ + EqualsVerifierApi withFieldnameToGetterConverter(Function converter); + /** * Signals that all internal caches need to be reset. This is useful when the test framework * uses multiple ClassLoaders to run tests, causing {@link java.lang.Class} instances diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/MultipleTypeEqualsVerifierApi.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/MultipleTypeEqualsVerifierApi.java index a4a88a74b..b178689e7 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/MultipleTypeEqualsVerifierApi.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/MultipleTypeEqualsVerifierApi.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.List; +import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; import nl.jqno.equalsverifier.ConfiguredEqualsVerifier; @@ -70,6 +71,15 @@ public MultipleTypeEqualsVerifierApi usingGetClass() { return this; } + /** {@inheritDoc} */ + @Override + public MultipleTypeEqualsVerifierApi withFieldnameToGetterConverter( + Function converter + ) { + ev.withFieldnameToGetterConverter(converter); + return this; + } + /** {@inheritDoc} */ @Override public MultipleTypeEqualsVerifierApi withResetCaches() { diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java index 881a0c32c..14fbb3c4e 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java @@ -1,16 +1,38 @@ package nl.jqno.equalsverifier.api; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.function.Function; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.EqualsVerifierReport; import nl.jqno.equalsverifier.Func.Func1; import nl.jqno.equalsverifier.Func.Func2; import nl.jqno.equalsverifier.Warning; -import nl.jqno.equalsverifier.internal.checkers.*; +import nl.jqno.equalsverifier.internal.checkers.AbstractDelegationChecker; +import nl.jqno.equalsverifier.internal.checkers.CachedHashCodeChecker; +import nl.jqno.equalsverifier.internal.checkers.Checker; +import nl.jqno.equalsverifier.internal.checkers.ExamplesChecker; +import nl.jqno.equalsverifier.internal.checkers.FieldsChecker; +import nl.jqno.equalsverifier.internal.checkers.HierarchyChecker; +import nl.jqno.equalsverifier.internal.checkers.MapEntryHashCodeRequirementChecker; +import nl.jqno.equalsverifier.internal.checkers.NullChecker; +import nl.jqno.equalsverifier.internal.checkers.RecordChecker; +import nl.jqno.equalsverifier.internal.checkers.SignatureChecker; import nl.jqno.equalsverifier.internal.exceptions.MessagingException; import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache; -import nl.jqno.equalsverifier.internal.util.*; +import nl.jqno.equalsverifier.internal.util.CachedHashCodeInitializer; +import nl.jqno.equalsverifier.internal.util.Configuration; +import nl.jqno.equalsverifier.internal.util.ErrorMessage; +import nl.jqno.equalsverifier.internal.util.FieldNameExtractor; import nl.jqno.equalsverifier.internal.util.Formatter; +import nl.jqno.equalsverifier.internal.util.ObjenesisWrapper; +import nl.jqno.equalsverifier.internal.util.PrefabValuesApi; +import nl.jqno.equalsverifier.internal.util.Validations; /** * Helps to construct an {@link EqualsVerifier} test with a fluent API. @@ -29,6 +51,7 @@ public class SingleTypeEqualsVerifierApi implements EqualsVerifierApi { private FactoryCache factoryCache = new FactoryCache(); private CachedHashCodeInitializer cachedHashCodeInitializer = CachedHashCodeInitializer.passthrough(); + private Function fieldnameToGetter = null; private Set allExcludedFields = new HashSet<>(); private Set allIncludedFields = new HashSet<>(); private Set nonnullFields = new HashSet<>(); @@ -129,6 +152,15 @@ public SingleTypeEqualsVerifierApi usingGetClass() { return this; } + /** {@inheritDoc} */ + @Override + public SingleTypeEqualsVerifierApi withFieldnameToGetterConverter( + Function converter + ) { + this.fieldnameToGetter = converter; + return this; + } + /** * Signals that all given fields are not relevant for the {@code equals} contract. {@code * EqualsVerifier} will not fail if one of these fields does not affect the outcome of {@code @@ -396,6 +428,7 @@ private Configuration buildConfig() { redefinedSubclass, usingGetClass, warningsToSuppress, + fieldnameToGetter, factoryCache, ignoredAnnotationClassNames, actualFields, 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 f97da7c4a..d4ad11fd4 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 @@ -5,6 +5,7 @@ import static nl.jqno.equalsverifier.internal.util.Assert.assertTrue; import java.util.Set; +import java.util.function.Function; import nl.jqno.equalsverifier.internal.exceptions.EqualsVerifierInternalBugException; import nl.jqno.equalsverifier.internal.prefabvalues.PrefabValues; import nl.jqno.equalsverifier.internal.prefabvalues.TypeTag; @@ -24,6 +25,7 @@ public class JpaLazyGetterFieldCheck implements FieldCheck { private final Set ignoredFields; private final PrefabValues prefabValues; private final AnnotationCache annotationCache; + private final Function fieldnameToGetter; public JpaLazyGetterFieldCheck(Configuration config) { this.type = config.getType(); @@ -31,6 +33,7 @@ public JpaLazyGetterFieldCheck(Configuration config) { this.ignoredFields = config.getIgnoredFields(); this.prefabValues = config.getPrefabValues(); this.annotationCache = config.getAnnotationCache(); + this.fieldnameToGetter = config.getFieldnameToGetter(); } @Override @@ -40,8 +43,7 @@ public void execute( FieldAccessor fieldAccessor ) { String fieldName = fieldAccessor.getFieldName(); - String getterName = - "get" + Character.toUpperCase(fieldName.charAt(0)) + fieldName.substring(1); + String getterName = fieldnameToGetter.apply(fieldName); if (ignoredFields.contains(fieldName) || !fieldIsLazy(fieldAccessor)) { return; diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java index fc1ea2e0a..2924db52c 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/util/Configuration.java @@ -1,8 +1,13 @@ package nl.jqno.equalsverifier.internal.util; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Function; import java.util.stream.Collectors; import nl.jqno.equalsverifier.Warning; import nl.jqno.equalsverifier.internal.prefabvalues.FactoryCache; @@ -17,6 +22,9 @@ public final class Configuration { + private static final Function DEFAULT_FIELDNAME_TO_GETTER_CONVERTER = fn -> + "get" + Character.toUpperCase(fn.charAt(0)) + fn.substring(1); + private final Class type; private final Set nonnullFields; private final CachedHashCodeInitializer cachedHashCodeInitializer; @@ -24,6 +32,7 @@ public final class Configuration { private final Class redefinedSubclass; private final boolean usingGetClass; private final EnumSet warningsToSuppress; + private final Function fieldnameToGetter; private final TypeTag typeTag; private final PrefabValues prefabValues; @@ -48,6 +57,7 @@ private Configuration( Class redefinedSubclass, boolean usingGetClass, EnumSet warningsToSuppress, + Function fieldnameToGetter, List equalExamples, List unequalExamples ) { @@ -63,6 +73,7 @@ private Configuration( this.redefinedSubclass = redefinedSubclass; this.usingGetClass = usingGetClass; this.warningsToSuppress = warningsToSuppress; + this.fieldnameToGetter = fieldnameToGetter; this.equalExamples = equalExamples; this.unequalExamples = unequalExamples; } @@ -77,6 +88,7 @@ public static Configuration build( Class redefinedSubclass, boolean usingGetClass, EnumSet warningsToSuppress, + Function fieldnameToGetter, FactoryCache factoryCache, Set ignoredAnnotationClassNames, Set actualFields, @@ -96,6 +108,9 @@ public static Configuration build( includedFields, actualFields ); + Function converter = fieldnameToGetter != null + ? fieldnameToGetter + : DEFAULT_FIELDNAME_TO_GETTER_CONVERTER; List unequals = ensureUnequalExamples(typeTag, classAccessor, unequalExamples); return new Configuration<>( @@ -111,6 +126,7 @@ public static Configuration build( redefinedSubclass, usingGetClass, warningsToSuppress, + converter, equalExamples, unequals ); @@ -233,6 +249,10 @@ public EnumSet getWarningsToSuppress() { return EnumSet.copyOf(warningsToSuppress); } + public Function getFieldnameToGetter() { + return fieldnameToGetter; + } + public List getEqualExamples() { return Collections.unmodifiableList(equalExamples); } 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 e373062cf..68054660e 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 @@ -89,6 +89,17 @@ public void lazyGettersPickedUpInSuper() { EqualsVerifier.forClass(ChildOfLazyGetterContainer.class).usingGetClass().verify(); } + @Test + public void differentCodingStyle() { + EqualsVerifier + .forClass(DifferentCodingStyleContainer.class) + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .verify(); + } + private void getterNotUsed(Class type, String method) { ExpectedException .when(() -> EqualsVerifier.forClass(type).suppress(Warning.NONFINAL_FIELDS).verify()) @@ -470,4 +481,42 @@ public boolean equals(Object obj) { return super.equals(obj); } } + + @Entity + static class DifferentCodingStyleContainer { + + // CHECKSTYLE OFF: MemberName + @OneToMany(fetch = FetchType.LAZY) + private String m_oneToMany; + + @ManyToOne(fetch = FetchType.LAZY) + private String m_manyToOne; + + // CHECKSTYLE ON: MemberName + + public String getOneToMany() { + return m_oneToMany; + } + + public String getManyToOne() { + return m_manyToOne; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof DifferentCodingStyleContainer)) { + return false; + } + DifferentCodingStyleContainer other = (DifferentCodingStyleContainer) obj; + return ( + Objects.equals(getOneToMany(), other.getOneToMany()) && + Objects.equals(getManyToOne(), other.getManyToOne()) + ); + } + + @Override + public int hashCode() { + return Objects.hash(getOneToMany(), getManyToOne()); + } + } } From 80b03608e4e8c8aa9d92935107953ba12894ad90 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Fri, 30 Jun 2023 10:32:23 +0200 Subject: [PATCH 2/8] Propagates fieldnameToGetter via ConfiguredEqualsVerifier --- .../ConfiguredEqualsVerifier.java | 3 ++- .../api/SingleTypeEqualsVerifierApi.java | 5 +++- .../extra_features/JpaLazyEntityTest.java | 26 ++++++++++++++++++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java index edddea870..f6f5743e3 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/ConfiguredEqualsVerifier.java @@ -123,7 +123,8 @@ public SingleTypeEqualsVerifierApi forClass(Class type) { type, EnumSet.copyOf(warningsToSuppress), factoryCache, - usingGetClass + usingGetClass, + fieldnameToGetter ); } diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java index 14fbb3c4e..bb64d54c1 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/api/SingleTypeEqualsVerifierApi.java @@ -77,17 +77,20 @@ public SingleTypeEqualsVerifierApi(Class type) { * @param factoryCache Factories that can be used to create values. * @param usingGetClass Whether {@code getClass} is used in the implementation of the {@code * equals} method, instead of an {@code instanceof} check. + * @param converter A function that converts from field name to getter name. */ public SingleTypeEqualsVerifierApi( Class type, EnumSet warningsToSuppress, FactoryCache factoryCache, - boolean usingGetClass + boolean usingGetClass, + Function converter ) { this(type); this.warningsToSuppress = EnumSet.copyOf(warningsToSuppress); this.factoryCache = this.factoryCache.merge(factoryCache); this.usingGetClass = usingGetClass; + this.fieldnameToGetter = converter; } /** 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 68054660e..4d442fa1e 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 @@ -1,5 +1,6 @@ package nl.jqno.equalsverifier.integration.extra_features; +import java.util.Arrays; import java.util.Objects; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; @@ -90,7 +91,7 @@ public void lazyGettersPickedUpInSuper() { } @Test - public void differentCodingStyle() { + public void differentCodingStyle_single() { EqualsVerifier .forClass(DifferentCodingStyleContainer.class) .suppress(Warning.NONFINAL_FIELDS) @@ -100,6 +101,29 @@ public void differentCodingStyle() { .verify(); } + @Test + public void differentCodingStyle_configured() { + EqualsVerifier + .configure() + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .forClass(DifferentCodingStyleContainer.class) + .verify(); + } + + @Test + public void differentCodingStyle_multiple() { + EqualsVerifier + .forClasses(Arrays.asList(DifferentCodingStyleContainer.class)) + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .verify(); + } + private void getterNotUsed(Class type, String method) { ExpectedException .when(() -> EqualsVerifier.forClass(type).suppress(Warning.NONFINAL_FIELDS).verify()) From 41c219c7a7365f15ed42cf80402816cd94bad7aa Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 6 Jul 2023 12:21:10 +0200 Subject: [PATCH 3/8] Requires getters for JPA collection fields even if they're not lazy --- .../fieldchecks/JpaLazyGetterFieldCheck.java | 15 +++++-- .../annotations/SupportedAnnotations.java | 8 ++-- .../extra_features/JpaLazyEntityTest.java | 44 +++++++++---------- 3 files changed, 37 insertions(+), 30 deletions(-) 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 d4ad11fd4..0ae094cad 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 @@ -73,10 +73,17 @@ public void execute( } private boolean fieldIsLazy(FieldAccessor fieldAccessor) { - return annotationCache.hasFieldAnnotation( - type, - fieldAccessor.getFieldName(), - SupportedAnnotations.LAZY_FIELD + return ( + annotationCache.hasFieldAnnotation( + type, + fieldAccessor.getFieldName(), + SupportedAnnotations.JPA_COLLECTION_FIELD + ) || + annotationCache.hasFieldAnnotation( + type, + fieldAccessor.getFieldName(), + SupportedAnnotations.JPA_LAZY_FIELD + ) ); } 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 dcf8a02b7..8add3903d 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 @@ -194,21 +194,21 @@ public void postProcess(Set> types, AnnotationCache annotationCache) { } }, - LAZY_FIELD( + JPA_COLLECTION_FIELD( true, - "javax.persistence.Basic", "javax.persistence.OneToOne", "javax.persistence.OneToMany", "javax.persistence.ManyToOne", "javax.persistence.ManyToMany", "javax.persistence.ElementCollection", - "jakarta.persistence.Basic", "jakarta.persistence.OneToOne", "jakarta.persistence.OneToMany", "jakarta.persistence.ManyToOne", "jakarta.persistence.ManyToMany", "jakarta.persistence.ElementCollection" - ) { + ), + + JPA_LAZY_FIELD(true, "javax.persistence.Basic", "jakarta.persistence.Basic") { @Override public boolean validate( AnnotationProperties properties, 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 4d442fa1e..a27451e51 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 @@ -30,7 +30,7 @@ public void gettersAreUsed() { @Test public void basicGetterNotUsed_givenEagerLoading() { EqualsVerifier - .forClass(IncorrectBasicJpaEagerFieldContainer.class) + .forClass(CorrectBasicJpaEagerFieldContainer.class) .suppress(Warning.NONFINAL_FIELDS) .verify(); } @@ -38,7 +38,7 @@ public void basicGetterNotUsed_givenEagerLoading() { @Test public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { EqualsVerifier - .forClass(IncorrectBasicJpaIgnoredLazyFieldContainer.class) + .forClass(CorrectBasicJpaIgnoredLazyFieldContainer.class) .withIgnoredFields("basic") .suppress(Warning.NONFINAL_FIELDS) .verify(); @@ -46,7 +46,7 @@ public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { @Test public void basicGetterUsed_givenAnnotationIsOnGetter() { - getterNotUsed(CorrectBasicJpaLazyGetterContainer.class, "equals"); + getterNotUsed(IncorrectBasicJpaLazyGetterContainer.class, "equals"); } @Test @@ -137,16 +137,16 @@ static class CorrectJpaLazyFieldContainer { @Basic(fetch = FetchType.LAZY) private String basic; - @OneToOne(fetch = FetchType.LAZY) + @OneToOne private String oneToOne; - @OneToMany(fetch = FetchType.LAZY) + @OneToMany private String oneToMany; - @ManyToOne(fetch = FetchType.LAZY) + @ManyToOne private String manyToOne; - @ManyToMany(fetch = FetchType.LAZY) + @ManyToMany private String manyToMany; @ElementCollection(fetch = FetchType.LAZY) @@ -206,7 +206,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJpaEagerFieldContainer { + static class CorrectBasicJpaEagerFieldContainer { @Basic private String basic; @@ -217,10 +217,10 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof IncorrectBasicJpaEagerFieldContainer)) { + if (!(obj instanceof CorrectBasicJpaEagerFieldContainer)) { return false; } - IncorrectBasicJpaEagerFieldContainer other = (IncorrectBasicJpaEagerFieldContainer) obj; + CorrectBasicJpaEagerFieldContainer other = (CorrectBasicJpaEagerFieldContainer) obj; return Objects.equals(basic, other.basic); } @@ -231,7 +231,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJpaIgnoredLazyFieldContainer { + static class CorrectBasicJpaIgnoredLazyFieldContainer { private String somethingElse; @@ -244,11 +244,11 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof IncorrectBasicJpaIgnoredLazyFieldContainer)) { + if (!(obj instanceof CorrectBasicJpaIgnoredLazyFieldContainer)) { return false; } - IncorrectBasicJpaIgnoredLazyFieldContainer other = - (IncorrectBasicJpaIgnoredLazyFieldContainer) obj; + CorrectBasicJpaIgnoredLazyFieldContainer other = + (CorrectBasicJpaIgnoredLazyFieldContainer) obj; return Objects.equals(somethingElse, other.somethingElse); } @@ -259,7 +259,7 @@ public int hashCode() { } @Entity - static class CorrectBasicJpaLazyGetterContainer { + static class IncorrectBasicJpaLazyGetterContainer { private String basic; @@ -270,10 +270,10 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof CorrectBasicJpaLazyGetterContainer)) { + if (!(obj instanceof IncorrectBasicJpaLazyGetterContainer)) { return false; } - CorrectBasicJpaLazyGetterContainer other = (CorrectBasicJpaLazyGetterContainer) obj; + IncorrectBasicJpaLazyGetterContainer other = (IncorrectBasicJpaLazyGetterContainer) obj; return Objects.equals(basic, other.basic); } @@ -337,7 +337,7 @@ public int hashCode() { @Entity static class IncorrectOneToOneJpaLazyFieldContainer { - @OneToOne(fetch = FetchType.LAZY) + @OneToOne private String oneToOne; public String getOneToOne() { @@ -363,7 +363,7 @@ public int hashCode() { @Entity static class IncorrectOneToManyJpaLazyFieldContainer { - @OneToMany(fetch = FetchType.LAZY) + @OneToMany private String oneToMany; public String getOneToMany() { @@ -389,7 +389,7 @@ public int hashCode() { @Entity static class IncorrectManyToOneJpaLazyFieldContainer { - @ManyToOne(fetch = FetchType.LAZY) + @ManyToOne private String manyToOne; public String getManyToOne() { @@ -415,7 +415,7 @@ public int hashCode() { @Entity static class IncorrectManyToManyJpaLazyFieldContainer { - @ManyToMany(fetch = FetchType.LAZY) + @ManyToMany private String manyToMany; public String getManyToMany() { @@ -441,7 +441,7 @@ public int hashCode() { @Entity static class IncorrectElementCollectionJpaLazyFieldContainer { - @ElementCollection(fetch = FetchType.LAZY) + @ElementCollection private String elementCollection; public String getElementCollection() { From 0fdbd43b1cd5233212ca71a529c9926eceebf183 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 6 Jul 2023 12:43:24 +0200 Subject: [PATCH 4/8] Adds Warning.JPA_GETTER --- .../main/java/nl/jqno/equalsverifier/Warning.java | 13 +++++++++++++ .../internal/checkers/FieldsChecker.java | 5 ++++- .../extra_features/JpaLazyEntityTest.java | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/Warning.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/Warning.java index 5b3939839..ab67cf27a 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/Warning.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/Warning.java @@ -165,6 +165,19 @@ public enum Warning { */ SURROGATE_OR_BUSINESS_KEY, + /** + * Disables the check that collection fields in JPA, or @Basic fields marked with + * FetchType.LAZY, should be accessed through their getter methods in {@code equals} and + * {@code hashCode} methods. + * + *

Normally, it is necessary to go through the getter for these fields, because their + * content may not be materialized in some instances. Calling the getter will materialize them, + * but referencing the field directly will not. This can lead to situations where the + * {@code equals} method of objects that should be equal to each other returns false, because + * one instance has the content materialized and the other does not. + */ + JPA_GETTER, + /** * Disables the check that transient fields not be part of the {@code equals} contract. * diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/FieldsChecker.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/FieldsChecker.java index e83cd8fc5..ab958f5fe 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/FieldsChecker.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/FieldsChecker.java @@ -95,7 +95,10 @@ public void check() { } AnnotationCache cache = config.getAnnotationCache(); - if (cache.hasClassAnnotation(config.getType(), SupportedAnnotations.ENTITY)) { + if ( + cache.hasClassAnnotation(config.getType(), SupportedAnnotations.ENTITY) && + !config.getWarningsToSuppress().contains(Warning.JPA_GETTER) + ) { inspector.check(jpaLazyGetterFieldCheck); } } 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 a27451e51..c88b8def3 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 @@ -47,41 +47,49 @@ public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { @Test public void basicGetterUsed_givenAnnotationIsOnGetter() { getterNotUsed(IncorrectBasicJpaLazyGetterContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectBasicJpaLazyGetterContainer.class); } @Test public void basicGetterNotUsedInHashCode() { getterNotUsed(IncorrectBasicJpaLazyFieldContainerHashCode.class, "hashCode"); + getterNotUsed_warningSuppressed(IncorrectBasicJpaLazyFieldContainerHashCode.class); } @Test public void basicGetterNotUsed() { getterNotUsed(IncorrectBasicJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectBasicJpaLazyFieldContainer.class); } @Test public void oneToOneGetterNotUsed() { getterNotUsed(IncorrectOneToOneJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectOneToOneJpaLazyFieldContainer.class); } @Test public void oneToManyGetterNotUsed() { getterNotUsed(IncorrectOneToManyJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectOneToManyJpaLazyFieldContainer.class); } @Test public void manyToOneGetterNotUsed() { getterNotUsed(IncorrectManyToOneJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectManyToOneJpaLazyFieldContainer.class); } @Test public void manyToManyGetterNotUsed() { getterNotUsed(IncorrectManyToManyJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectManyToManyJpaLazyFieldContainer.class); } @Test public void elementCollectionGetterNotUsed() { getterNotUsed(IncorrectElementCollectionJpaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectElementCollectionJpaLazyFieldContainer.class); } @Test @@ -131,6 +139,13 @@ private void getterNotUsed(Class type, String method) { .assertMessageContains("JPA Entity", method, "direct reference"); } + private void getterNotUsed_warningSuppressed(Class type) { + EqualsVerifier + .forClass(type) + .suppress(Warning.JPA_GETTER, Warning.NONFINAL_FIELDS) + .verify(); + } + @Entity static class CorrectJpaLazyFieldContainer { From 053eefd7b487c88acaad4fbd231d13f20ff46b0b Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 6 Jul 2023 12:52:52 +0200 Subject: [PATCH 5/8] Adds tests for Jakarta lazy fields --- .../fieldchecks/JpaLazyGetterFieldCheck.java | 2 +- .../annotations/SupportedAnnotations.java | 2 +- .../extra_features/JakartaLazyEntityTest.java | 166 ++++++++++++++++-- 3 files changed, 153 insertions(+), 17 deletions(-) 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 0ae094cad..5f86da9ec 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 @@ -77,7 +77,7 @@ private boolean fieldIsLazy(FieldAccessor fieldAccessor) { annotationCache.hasFieldAnnotation( type, fieldAccessor.getFieldName(), - SupportedAnnotations.JPA_COLLECTION_FIELD + SupportedAnnotations.JPA_LINKED_FIELD ) || annotationCache.hasFieldAnnotation( type, 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 8add3903d..5240cbd00 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 @@ -194,7 +194,7 @@ public void postProcess(Set> types, AnnotationCache annotationCache) { } }, - JPA_COLLECTION_FIELD( + JPA_LINKED_FIELD( true, "javax.persistence.OneToOne", "javax.persistence.OneToMany", 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 d2d746ab2..bf22a1507 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 @@ -8,6 +8,7 @@ import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToMany; import jakarta.persistence.OneToOne; +import java.util.Arrays; import java.util.Objects; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; @@ -29,7 +30,7 @@ public void gettersAreUsed() { @Test public void basicGetterNotUsed_givenEagerLoading() { EqualsVerifier - .forClass(IncorrectBasicJakartaEagerFieldContainer.class) + .forClass(CorrectBasicJakartaEagerFieldContainer.class) .suppress(Warning.NONFINAL_FIELDS) .verify(); } @@ -37,7 +38,7 @@ public void basicGetterNotUsed_givenEagerLoading() { @Test public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { EqualsVerifier - .forClass(IncorrectBasicJakartaIgnoredLazyFieldContainer.class) + .forClass(CorrectBasicJakartaIgnoredLazyFieldContainer.class) .withIgnoredFields("basic") .suppress(Warning.NONFINAL_FIELDS) .verify(); @@ -45,42 +46,90 @@ public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { @Test public void basicGetterUsed_givenAnnotationIsOnGetter() { - getterNotUsed(CorrectBasicJakartaLazyGetterContainer.class, "equals"); + getterNotUsed(IncorrectBasicJakartaLazyGetterContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectBasicJakartaLazyGetterContainer.class); } @Test public void basicGetterNotUsedInHashCode() { getterNotUsed(IncorrectBasicJakartaLazyFieldContainerHashCode.class, "hashCode"); + getterNotUsed_warningSuppressed(IncorrectBasicJakartaLazyFieldContainerHashCode.class); } @Test public void basicGetterNotUsed() { getterNotUsed(IncorrectBasicJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectBasicJakartaLazyFieldContainer.class); } @Test public void oneToOneGetterNotUsed() { getterNotUsed(IncorrectOneToOneJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectOneToOneJakartaLazyFieldContainer.class); } @Test public void oneToManyGetterNotUsed() { getterNotUsed(IncorrectOneToManyJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectOneToManyJakartaLazyFieldContainer.class); } @Test public void manyToOneGetterNotUsed() { getterNotUsed(IncorrectManyToOneJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectManyToOneJakartaLazyFieldContainer.class); } @Test public void manyToManyGetterNotUsed() { getterNotUsed(IncorrectManyToManyJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectManyToManyJakartaLazyFieldContainer.class); } @Test public void elementCollectionGetterNotUsed() { getterNotUsed(IncorrectElementCollectionJakartaLazyFieldContainer.class, "equals"); + getterNotUsed_warningSuppressed(IncorrectElementCollectionJakartaLazyFieldContainer.class); + } + + @Test + public void lazyGettersPickedUpInSuper() { + EqualsVerifier.forClass(LazyGetterContainer.class).usingGetClass().verify(); + EqualsVerifier.forClass(ChildOfLazyGetterContainer.class).usingGetClass().verify(); + } + + @Test + public void differentCodingStyle_single() { + EqualsVerifier + .forClass(DifferentCodingStyleContainer.class) + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .verify(); + } + + @Test + public void differentCodingStyle_configured() { + EqualsVerifier + .configure() + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .forClass(DifferentCodingStyleContainer.class) + .verify(); + } + + @Test + public void differentCodingStyle_multiple() { + EqualsVerifier + .forClasses(Arrays.asList(DifferentCodingStyleContainer.class)) + .suppress(Warning.NONFINAL_FIELDS) + .withFieldnameToGetterConverter(fn -> + "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .verify(); } private void getterNotUsed(Class type, String method) { @@ -90,6 +139,13 @@ private void getterNotUsed(Class type, String method) { .assertMessageContains("JPA Entity", method, "direct reference"); } + private void getterNotUsed_warningSuppressed(Class type) { + EqualsVerifier + .forClass(type) + .suppress(Warning.JPA_GETTER, Warning.NONFINAL_FIELDS) + .verify(); + } + @Entity static class CorrectJakartaLazyFieldContainer { @@ -165,7 +221,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJakartaEagerFieldContainer { + static class CorrectBasicJakartaEagerFieldContainer { @Basic private String basic; @@ -176,11 +232,11 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof IncorrectBasicJakartaEagerFieldContainer)) { + if (!(obj instanceof CorrectBasicJakartaEagerFieldContainer)) { return false; } - IncorrectBasicJakartaEagerFieldContainer other = - (IncorrectBasicJakartaEagerFieldContainer) obj; + CorrectBasicJakartaEagerFieldContainer other = + (CorrectBasicJakartaEagerFieldContainer) obj; return Objects.equals(basic, other.basic); } @@ -191,7 +247,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJakartaIgnoredLazyFieldContainer { + static class CorrectBasicJakartaIgnoredLazyFieldContainer { private String somethingElse; @@ -204,11 +260,11 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof IncorrectBasicJakartaIgnoredLazyFieldContainer)) { + if (!(obj instanceof CorrectBasicJakartaIgnoredLazyFieldContainer)) { return false; } - IncorrectBasicJakartaIgnoredLazyFieldContainer other = - (IncorrectBasicJakartaIgnoredLazyFieldContainer) obj; + CorrectBasicJakartaIgnoredLazyFieldContainer other = + (CorrectBasicJakartaIgnoredLazyFieldContainer) obj; return Objects.equals(somethingElse, other.somethingElse); } @@ -219,7 +275,7 @@ public int hashCode() { } @Entity - static class CorrectBasicJakartaLazyGetterContainer { + static class IncorrectBasicJakartaLazyGetterContainer { private String basic; @@ -230,11 +286,11 @@ public String getBasic() { @Override public boolean equals(Object obj) { - if (!(obj instanceof CorrectBasicJakartaLazyGetterContainer)) { + if (!(obj instanceof IncorrectBasicJakartaLazyGetterContainer)) { return false; } - CorrectBasicJakartaLazyGetterContainer other = - (CorrectBasicJakartaLazyGetterContainer) obj; + IncorrectBasicJakartaLazyGetterContainer other = + (IncorrectBasicJakartaLazyGetterContainer) obj; return Objects.equals(basic, other.basic); } @@ -425,4 +481,84 @@ public int hashCode() { return Objects.hash(getElementCollection()); } } + + @Entity + static class LazyGetterContainer { + + @Basic(fetch = FetchType.LAZY) + private String s; + + public String getS() { + return s; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + return Objects.equals(getS(), ((LazyGetterContainer) obj).getS()); + } + + @Override + public int hashCode() { + return Objects.hash(getS()); + } + } + + @Entity + static class ChildOfLazyGetterContainer extends LazyGetterContainer { + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + return super.equals(obj); + } + } + + @Entity + static class DifferentCodingStyleContainer { + + // CHECKSTYLE OFF: MemberName + @OneToMany(fetch = FetchType.LAZY) + private String m_oneToMany; + + @ManyToOne(fetch = FetchType.LAZY) + private String m_manyToOne; + + // CHECKSTYLE ON: MemberName + + public String getOneToMany() { + return m_oneToMany; + } + + public String getManyToOne() { + return m_manyToOne; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof DifferentCodingStyleContainer)) { + return false; + } + DifferentCodingStyleContainer other = (DifferentCodingStyleContainer) obj; + return ( + Objects.equals(getOneToMany(), other.getOneToMany()) && + Objects.equals(getManyToOne(), other.getManyToOne()) + ); + } + + @Override + public int hashCode() { + return Objects.hash(getOneToMany(), getManyToOne()); + } + } } From 2f1f85da72e54297af883f9ceccd1cfd97ca3c4a Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 6 Jul 2023 13:05:53 +0200 Subject: [PATCH 6/8] Adds section on materialized fields in documentation --- docs/_manual/09-jpa-entities.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/docs/_manual/09-jpa-entities.md b/docs/_manual/09-jpa-entities.md index 450a49b06..7a2951511 100644 --- a/docs/_manual/09-jpa-entities.md +++ b/docs/_manual/09-jpa-entities.md @@ -54,6 +54,29 @@ In that case, you can call `suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY (`Warning.IDENTICAL_COPY`, which the error message suggests, is not appropriate in this case because that is meant for classes which have no state at all.) +### Materialized fields +Some fields have a mapping annotation that links them with data from a different database table or entity. These annotations include `@OneToMany`, `@ManyToOne` and `@ManyToMany`. In certain situations, you can have an instance where these fields are not materialized yet. In other words, they're not fetched from the database, and their content is undefined. Most often, this happens when they have `fetchType = FetchType.LAZY`, but even with `FetchType.EAGER`, it can happen that they are not yet materialized. This also applies to fields with `@Basic(fetchType = FetchType.LAZY)`. JPA will materialize this data on demand. For example, when the getter for such a field is called, JPA is triggered and queries the data. However, this trigger does not happen when the field is referenced directly. + +Therefore, when these fields are used in `equals` and `hashCode`, it's important to call their getter method instead of referencing the field directly. Otherwise, the data may not be materialized, and it's possible that calling `equals` on two equal objects returns `false`, because one instance doesn't have the content yet while the other does. + +EqualsVerifier checks for these fields that their getter is used. If they're referenced directly, EqualsVerifier will fail. Note that this can be disabled by suppressing `Warning.JPA_GETTER`. + +By default, EqualsVerifier assumes that the JavaBeans conventions are used to determine the name of the getter. For example, if a field is called `employee`, it assumes that the getter is called `getEmployee()`. If your project uses a different convention, you can use `#withFieldnameToGetterConverter()` to override that behavior. + +For example, if in your project, a field must have a prefix, like so: `m_employee`, but the getter is still `getEmployee()`, you might call EqualsVerifier like this: + +{% highlight java %} +EqualsVerifier + .forClass(Foo.class) + .withFieldnameToGetterConverter( + fn -> "get" + Character.toUpperCase(fn.charAt(2)) + fn.substring(3) + ) + .verify(); +{% endhighlight %} + +This will chop off the `m_` prefix, uppercase the first letter, and prepend the word `get`. + + ### Transient fields Since fields marked with the `@Transient` annotation are not persisted, they should generally not participate in `equals` and `hashCode` either. Therefore, EqualsVerifier will implicitly call [`withIgnoredFields`](/equalsverifier/manual/ignoring-fields) for these fields. From 22ed6f5949e1383071d6e24d1e8579fbcaed30c4 Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Thu, 6 Jul 2023 13:28:36 +0200 Subject: [PATCH 7/8] Adds error message explained for JPA Getter error --- .../jpa-direct-reference-instead-of-getter.md | 36 +++++++++++++++++++ docs/_pages/errormessages.md | 1 + 2 files changed, 37 insertions(+) create mode 100644 docs/_errormessages/jpa-direct-reference-instead-of-getter.md diff --git a/docs/_errormessages/jpa-direct-reference-instead-of-getter.md b/docs/_errormessages/jpa-direct-reference-instead-of-getter.md new file mode 100644 index 000000000..7d586837d --- /dev/null +++ b/docs/_errormessages/jpa-direct-reference-instead-of-getter.md @@ -0,0 +1,36 @@ +--- +title: "JPA Entity: direct reference to field ... used in equals instead of getter" +--- +For fields with a mapping annotation like `@OneToMany`, `@ManyToOne` or `@ManyToMany`, you might run into this error: + + JPA Entity: direct reference to field employee used in equals + instead of getter getEmployee. + +This error occurs when the field is used directly in `equals` or `hashCode`: + +{% highlight java %} +@ManyToOne +private Employee employee; + +public boolean equals(Object other) { + // ... + return Objects.equals(employee, that.employee); +} +{% endhighlight %} + +This is problematic, because the field `employee` might not be materialized yet. In other words, JPA may not have queried the `employee` yet and the reference could still be null. This might lead to incorrect results when calling `equals` or `hashCode`. JPA will materialize the field when the getter is called, but not when the field is referenced directly. Therefore, the field should always be referenced through its getter, in `equals` and `hashCode`: + +{% highlight java %} +public boolean equals(Object other) { + // ... + return Objects.equals(getEmployee(), that.getEmployee()); +} + +public int hashCode() { + return Objects.hash(getEmployee()); +} +{% endhighlight %} + +If you have a reason, you can disable this check by suppressing `Warning.JPA_GETTER`. Also, EqualsVerifier assumes you use the JavaBeans convention to name your fields and getters. If you use a different convention, you can use `#withFieldnameToGetterConverter()` to override that. + +See the [manual page about JPA entities](/equalsverifier/manual/jpa-entities), specifically the section on Materialized fields, for more details. diff --git a/docs/_pages/errormessages.md b/docs/_pages/errormessages.md index fb24e32cb..e417e9802 100644 --- a/docs/_pages/errormessages.md +++ b/docs/_pages/errormessages.md @@ -24,6 +24,7 @@ This is not a complete list. I'll add to it as needed, so if you need help with * [Coverage is not 100%](/equalsverifier/errormessages/coverage-is-not-100-percent) * [Double: equals doesn't use Double.compare for field foo](/equalsverifier/errormessages/double-equals-doesnt-use-doublecompare-for-field-foo) * [Float: equals doesn't use Float.compare for field foo](/equalsverifier/errormessages/float-equals-doesnt-use-floatcompare-for-field-foo) +* [JPA Entity: direct reference to field ... used instead of getter](/equalsverifier/errormessages/jpa-direct-reference-instead-of-getter) * [Mutability: equals depends on mutable field](/equalsverifier/errormessages/mutability-equals-depends-on-mutable-field) * [NoClassDefFoundError](/equalsverifier/errormessages/noclassdeffounderror) * [Non-nullity: equals/hashCode/toString throws NullPointerException](/equalsverifier/errormessages/non-nullity-equals-hashcode-tostring-throws-nullpointerexception) From 2f3f538e766b40d1edc925ffabfb6f1e6854ad7e Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Fri, 7 Jul 2023 11:59:32 +0200 Subject: [PATCH 8/8] Updates CHANGELOG for new behaviour around JPA entities --- CHANGELOG.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9368a4494..4c19aa39c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Check that getters are used for all mapped fields in JPA entities, not just the ones with `FetchType.LAZY`. ([Issue 830](https://github.com/jqno/equalsverifier/issues/830))
Note that this is a **breaking change** for JPA entity tests. This can be disabled by suppressing `Warning.JPA_GETTER`. See the [manual page about JPA entities](/equalsverifier/manual/jpa-entities), specifically the section on Materialized fields, for more details. + +### Added + +- `#withFieldnameToGetterConverter()` to override the derivation of getter names from field names when testing JPA entities. ([Issue 829](https://github.com/jqno/equalsverifier/issues/829)) +- `Warning.JPA_GETTER` to suppress the check that getters should be used instead of direct field references in JPA entities. + + ## [3.14.3] - 2023-06-23 ### Fixed