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 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/_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. 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) 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..f6f5743e3 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() { @@ -110,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/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/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..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 @@ -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<>(); @@ -54,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; } /** @@ -129,6 +155,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 +431,7 @@ private Configuration buildConfig() { redefinedSubclass, usingGetClass, warningsToSuppress, + fieldnameToGetter, factoryCache, ignoredAnnotationClassNames, actualFields, 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/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..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 @@ -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; @@ -71,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_LINKED_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..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,21 +194,21 @@ public void postProcess(Set> types, AnnotationCache annotationCache) { } }, - LAZY_FIELD( + JPA_LINKED_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/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/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()); + } + } } 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..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 @@ -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; @@ -29,7 +30,7 @@ public void gettersAreUsed() { @Test public void basicGetterNotUsed_givenEagerLoading() { EqualsVerifier - .forClass(IncorrectBasicJpaEagerFieldContainer.class) + .forClass(CorrectBasicJpaEagerFieldContainer.class) .suppress(Warning.NONFINAL_FIELDS) .verify(); } @@ -37,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(); @@ -45,42 +46,50 @@ public void basicGetterNotUsed_givenCorrespondingFieldIgnored() { @Test public void basicGetterUsed_givenAnnotationIsOnGetter() { - getterNotUsed(CorrectBasicJpaLazyGetterContainer.class, "equals"); + 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 @@ -89,6 +98,40 @@ public void lazyGettersPickedUpInSuper() { 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) { ExpectedException .when(() -> EqualsVerifier.forClass(type).suppress(Warning.NONFINAL_FIELDS).verify()) @@ -96,22 +139,29 @@ 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 { @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) @@ -171,7 +221,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJpaEagerFieldContainer { + static class CorrectBasicJpaEagerFieldContainer { @Basic private String basic; @@ -182,10 +232,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); } @@ -196,7 +246,7 @@ public int hashCode() { } @Entity - static class IncorrectBasicJpaIgnoredLazyFieldContainer { + static class CorrectBasicJpaIgnoredLazyFieldContainer { private String somethingElse; @@ -209,11 +259,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); } @@ -224,7 +274,7 @@ public int hashCode() { } @Entity - static class CorrectBasicJpaLazyGetterContainer { + static class IncorrectBasicJpaLazyGetterContainer { private String basic; @@ -235,10 +285,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); } @@ -302,7 +352,7 @@ public int hashCode() { @Entity static class IncorrectOneToOneJpaLazyFieldContainer { - @OneToOne(fetch = FetchType.LAZY) + @OneToOne private String oneToOne; public String getOneToOne() { @@ -328,7 +378,7 @@ public int hashCode() { @Entity static class IncorrectOneToManyJpaLazyFieldContainer { - @OneToMany(fetch = FetchType.LAZY) + @OneToMany private String oneToMany; public String getOneToMany() { @@ -354,7 +404,7 @@ public int hashCode() { @Entity static class IncorrectManyToOneJpaLazyFieldContainer { - @ManyToOne(fetch = FetchType.LAZY) + @ManyToOne private String manyToOne; public String getManyToOne() { @@ -380,7 +430,7 @@ public int hashCode() { @Entity static class IncorrectManyToManyJpaLazyFieldContainer { - @ManyToMany(fetch = FetchType.LAZY) + @ManyToMany private String manyToMany; public String getManyToMany() { @@ -406,7 +456,7 @@ public int hashCode() { @Entity static class IncorrectElementCollectionJpaLazyFieldContainer { - @ElementCollection(fetch = FetchType.LAZY) + @ElementCollection private String elementCollection; public String getElementCollection() { @@ -470,4 +520,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()); + } + } }