diff --git a/CHANGELOG.md b/CHANGELOG.md index a555ca206..7349ddd99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - When `Warning.SURROGATE_OR_BUSINESS_KEY` is suppressed, it is now possible to use `#withOnlyTheseFields`, and the fields may include both `@Id` fields and regular fields. ([Issue 934](https://github.com/jqno/equalsverifier/issues/934)) +- Improved the error message for so called 'versioned entities' (where entities with `0` or `null` ids are always unequal), so it's more discoverable what to do in that situation. ([Issue 932](https://github.com/jqno/equalsverifier/issues/932)) ## [3.15.8] - 2024-03-01 diff --git a/docs/_manual/10-jpa-entities.md b/docs/_manual/10-jpa-entities.md index f25ad1232..a0dffd486 100644 --- a/docs/_manual/10-jpa-entities.md +++ b/docs/_manual/10-jpa-entities.md @@ -39,7 +39,7 @@ public boolean equals(Object obj) { } Foo other = (Foo)obj; if (id == 0L && other.id == 0L) { - return super.equals(obj); + return false; } return id == other.id; } @@ -47,13 +47,29 @@ public boolean equals(Object obj) { You might see an error message such as this one: - Reflexivity: object does not equal an identical copy of itself: + Reflexivity: entity does not equal an identical copy of itself: Foo@123456 - If this is intentional, consider suppressing Warning.IDENTICAL_COPY + If this is intentional, consider suppressing Warning.IDENTICAL_COPYFOR_VERSIONED_ENTITY + +In that case, you can call `suppress(Warning.SURROGATE_KEY, Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`: `SURROGATE_KEY` because the identity is based on the entity's keys, and `IDENTICAL_COPY_FOR_VERSIONED_ENTITY` to allow the (small) breach in reflexivity. -In that case, you can call `suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`. +Similarly, if the entity has a business key, like so: + +{% highlight java %} +@Override +public boolean equals(Object obj) { + if (!(obj instanceof Foo)) { + return false; + } + Foo other = (Foo)obj; + if (id == 0L && other.id == 0L) { + return false; + } + return Objects.equals(someField, other.someField); +} +{% endhighlight %} -(`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.) +You will get the same error message. In this case you can simply call `suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY)`, without also suppressing `Warning.SURROGATE_KEY`. ### Materialized fields diff --git a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java index 0e06fef59..e9f221996 100644 --- a/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java +++ b/equalsverifier-core/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/ReflexivityFieldCheck.java @@ -14,6 +14,7 @@ import nl.jqno.equalsverifier.internal.reflection.ObjectAccessor; import nl.jqno.equalsverifier.internal.reflection.annotations.AnnotationCache; import nl.jqno.equalsverifier.internal.reflection.annotations.NonnullAnnotationVerifier; +import nl.jqno.equalsverifier.internal.reflection.annotations.SupportedAnnotations; import nl.jqno.equalsverifier.internal.util.Configuration; import nl.jqno.equalsverifier.internal.util.Formatter; @@ -125,13 +126,27 @@ private void checkReflexivityFor(T left, T right) { ); assertFalse(f, left.equals(right)); } else { - Formatter f = Formatter.of( - "Reflexivity: object does not equal an identical copy of itself:\n %%" + - "\nIf this is intentional, consider suppressing Warning.%%", - left, - Warning.IDENTICAL_COPY.toString() + boolean isEntity = annotationCache.hasClassAnnotation( + typeTag.getType(), + SupportedAnnotations.ENTITY ); - assertEquals(f, left, right); + if (isEntity) { + Formatter f = Formatter.of( + "Reflexivity: entity does not equal an identical copy of itself:\n %%" + + "\nIf this is intentional, consider suppressing Warning.%%.", + left, + Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString() + ); + assertEquals(f, left, right); + } else { + Formatter f = Formatter.of( + "Reflexivity: object does not equal an identical copy of itself:\n %%" + + "\nIf this is intentional, consider suppressing Warning.%%.", + left, + Warning.IDENTICAL_COPY.toString() + ); + assertEquals(f, left, right); + } } } } diff --git a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/VersionedEntityTest.java b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/VersionedEntityTest.java index 33de1be98..0140e2cf8 100644 --- a/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/VersionedEntityTest.java +++ b/equalsverifier-core/src/test/java/nl/jqno/equalsverifier/integration/extra_features/VersionedEntityTest.java @@ -3,9 +3,11 @@ import static nl.jqno.equalsverifier.internal.testhelpers.Util.defaultHashCode; import java.util.Objects; +import java.util.UUID; import nl.jqno.equalsverifier.EqualsVerifier; import nl.jqno.equalsverifier.Warning; import nl.jqno.equalsverifier.internal.testhelpers.ExpectedException; +import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Entity; import nl.jqno.equalsverifier.testhelpers.annotations.javax.persistence.Id; import org.junit.jupiter.api.Test; @@ -19,8 +21,8 @@ public void fail_whenInstanceWithAZeroIdDoesNotEqualItself() { .when(() -> EqualsVerifier.forClass(OtherwiseStatelessVersionedEntity.class).verify()) .assertFailure() .assertMessageContains( - "object does not equal an identical copy of itself", - Warning.IDENTICAL_COPY.toString() + "entity does not equal an identical copy of itself", + Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString() ); } @@ -48,11 +50,11 @@ public void succeed_whenInstanceWithAZeroIdDoesNotEqualItselfAndInstanceWithANon @Test public void fail_whenInstanceWithAZeroIdDoesNotEqualItself_givenAVersionedEntityWithState() { ExpectedException - .when(() -> EqualsVerifier.forClass(StringVersionedEntity.class).verify()) + .when(() -> EqualsVerifier.forClass(LongIdStringFieldVersionedEntity.class).verify()) .assertFailure() .assertMessageContains( - "object does not equal an identical copy of itself", - Warning.IDENTICAL_COPY.toString() + "entity does not equal an identical copy of itself", + Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString() ); } @@ -61,7 +63,7 @@ public void fail_whenInstanceWithANonzeroIdEqualsItself_givenAVersionedEntityWit ExpectedException .when(() -> EqualsVerifier - .forClass(StringVersionedEntity.class) + .forClass(LongIdStringFieldVersionedEntity.class) .suppress(Warning.IDENTICAL_COPY) .verify() ) @@ -72,7 +74,39 @@ public void fail_whenInstanceWithANonzeroIdEqualsItself_givenAVersionedEntityWit @Test public void succeed_whenInstanceWithAZeroIdDoesNotEqualItselfAndInstanceWithANonzeroIdDoes_givenAVersionedEntityWithStateAndVersionedEntityWarningIsSuppressed() { EqualsVerifier - .forClass(StringVersionedEntity.class) + .forClass(LongIdStringFieldVersionedEntity.class) + .suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY, Warning.SURROGATE_KEY) + .verify(); + } + + @Test + public void fail_whenInstanceWithANullIdDoesNotEqualItself_givenAVersionedEntityWithState() { + ExpectedException + .when(() -> EqualsVerifier.forClass(UuidIdStringFieldVersionedEntity.class).verify()) + .assertFailure() + .assertMessageContains( + "entity does not equal an identical copy of itself", + Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY.toString() + ); + } + + @Test + public void fail_whenInstanceWithANonnullIdEqualsItself_givenAVersionedEntityWithStateAndIdenticalCopyWarningIsSuppressed() { + ExpectedException + .when(() -> + EqualsVerifier + .forClass(UuidIdStringFieldVersionedEntity.class) + .suppress(Warning.IDENTICAL_COPY) + .verify() + ) + .assertFailure() + .assertMessageContains("Unnecessary suppression", Warning.IDENTICAL_COPY.toString()); + } + + @Test + public void succeed_whenInstanceWithANullIdDoesNotEqualItselfAndInstanceWithANonnullIdDoes_givenAVersionedEntityWithStateAndVersionedEntityWarningIsSuppressed() { + EqualsVerifier + .forClass(UuidIdStringFieldVersionedEntity.class) .suppress(Warning.IDENTICAL_COPY_FOR_VERSIONED_ENTITY, Warning.SURROGATE_KEY) .verify(); } @@ -143,6 +177,7 @@ public void fail_whenTheExceptionIsThrownInADifficultToReachPartOfTheSubclassOfA .assertMessageContains("catch me if you can"); } + @Entity public static final class OtherwiseStatelessVersionedEntity { @Id @@ -170,7 +205,8 @@ public int hashCode() { } } - public static final class StringVersionedEntity { + @Entity + public static final class LongIdStringFieldVersionedEntity { @Id private final long id; @@ -178,17 +214,17 @@ public static final class StringVersionedEntity { @SuppressWarnings("unused") private final String s; - public StringVersionedEntity(long id, String s) { + public LongIdStringFieldVersionedEntity(long id, String s) { this.id = id; this.s = s; } @Override public boolean equals(Object obj) { - if (!(obj instanceof StringVersionedEntity)) { + if (!(obj instanceof LongIdStringFieldVersionedEntity)) { return false; } - StringVersionedEntity other = (StringVersionedEntity) obj; + LongIdStringFieldVersionedEntity other = (LongIdStringFieldVersionedEntity) obj; if (id == 0L && other.id == 0L) { return false; } @@ -201,6 +237,39 @@ public int hashCode() { } } + @Entity + public static final class UuidIdStringFieldVersionedEntity { + + @Id + private final UUID id; + + @SuppressWarnings("unused") + private final String s; + + public UuidIdStringFieldVersionedEntity(UUID id, String s) { + this.id = id; + this.s = s; + } + + @Override + public boolean equals(Object obj) { + if (!(obj instanceof UuidIdStringFieldVersionedEntity)) { + return false; + } + UuidIdStringFieldVersionedEntity other = (UuidIdStringFieldVersionedEntity) obj; + if (id == null && other.id == null) { + return false; + } + return Objects.equals(id, other.id); + } + + @Override + public int hashCode() { + return Objects.hash(id); + } + } + + @Entity public static final class WeakStringVersionedEntity { @Id @@ -231,6 +300,7 @@ public int hashCode() { } } + @Entity public static final class NullCheckStringVersionedEntity { @Id @@ -259,6 +329,7 @@ public int hashCode() { } } + @Entity public static final class BusinessKeyStringVersionedEntity { @Id @@ -286,6 +357,7 @@ public int hashCode() { } } + @Entity private static class CanEqualVersionedEntity { private final Long id; @@ -318,6 +390,7 @@ public final int hashCode() { } } + @Entity private static class NonReflexiveCanEqualVersionedEntity extends CanEqualVersionedEntity { public NonReflexiveCanEqualVersionedEntity(Long id) {