From 58cfd57b77bf02b23b1d0f29ad69a82702a196dd Mon Sep 17 00:00:00 2001 From: Jan Ouwens Date: Mon, 18 Feb 2019 16:23:24 +0100 Subject: [PATCH] #231: improves error messages regarding @EmbeddedId --- CHANGELOG.md | 2 ++ src/main/java/nl/jqno/equalsverifier/Warning.java | 15 ++++++++------- .../fieldchecks/SignificantFieldCheck.java | 6 +++--- .../annotations/SupportedAnnotations.java | 6 +++--- .../equalsverifier/internal/util/Validations.java | 4 ++-- .../integration/extra_features/JpaIdTest.java | 10 +++++++++- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe06254fc..67fc3c0b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed +- Improved error messages regarding JPA's `@EmbeddedId` annotation. ([Issue 231](https://github.com/jqno/equalsverifier/issues/231)) diff --git a/src/main/java/nl/jqno/equalsverifier/Warning.java b/src/main/java/nl/jqno/equalsverifier/Warning.java index af9b9cc5b..b25de2ee7 100644 --- a/src/main/java/nl/jqno/equalsverifier/Warning.java +++ b/src/main/java/nl/jqno/equalsverifier/Warning.java @@ -163,13 +163,14 @@ public enum Warning { STRICT_INHERITANCE, /** - * Disables the check that fields marked with the @Id annotation in JPA - * entities may not be used in the {@code equals} contract. - *

- * When this warning is suppressed, the fields marked with @Id will become - * the entity's surrogate key. Only these fields can now be part of the - * {@code equals} contract; all other fields may no longer be used in - * {@code equals}. + * Disables the check that fields marked with the @Id or @EmbeddedId + * annotations in JPA entities may not be used in the {@code equals} + * contract. + *

+ * When this warning is suppressed, the fields marked with @Id + * or @EmbeddedId will become the entity's surrogate key. Only these fields + * can now be part of the {@code equals} contract; all other fields may no + * longer be used in {@code equals}. */ SURROGATE_KEY, diff --git a/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java b/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java index c89091a32..ad92b1454 100644 --- a/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java +++ b/src/main/java/nl/jqno/equalsverifier/internal/checkers/fieldchecks/SignificantFieldCheck.java @@ -122,7 +122,7 @@ private void assertFieldShouldHaveBeenUsed(String fieldName, boolean equalsChang } else if (anotherFieldIsMarkedAsId) { message = "Significant fields: equals does not use %%, or it is stateless.\n" + - "Suppress Warning.SURROGATE_KEY if you want to use only the @Id field(s)."; + "Suppress Warning.SURROGATE_KEY if you want to use only the @Id or @EmbeddedId field(s)."; } else { message = "Significant fields: equals does not use %%, or it is stateless."; @@ -137,11 +137,11 @@ private void assertFieldShouldNotBeUsed(String fieldName, boolean equalsChanged, final String message; if (thisFieldIsMarkedAsId) { message = "Significant fields: %% is marked @Id or @EmbeddedId so equals should not use it, but it does.\n" + - "Suppress Warning.SURROGATE_KEY if you want to use only the @Id field(s)."; + "Suppress Warning.SURROGATE_KEY if you want to use only the @Id or @EmbeddedId field(s)."; } else if (anotherFieldIsMarkedAsId) { message = "Significant fields: equals should not use %% because Warning.SURROGATE_KEY is suppressed" + - " and it is not marked as @Id, but it does."; + " and it is not marked as @Id or @EmbeddedId, but it does."; } else { message = "Significant fields: equals should not use %%, but it does."; diff --git a/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java b/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java index ac36c0ce0..3ad9a5311 100644 --- a/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java +++ b/src/main/java/nl/jqno/equalsverifier/internal/reflection/annotations/SupportedAnnotations.java @@ -138,9 +138,9 @@ public boolean validate(AnnotationProperties properties, AnnotationCache annotat TRANSIENT(true, "javax.persistence.Transient"), /** - * Fields in JPA Entities that are marked @Id are usually part of the - * entity's surrogate key. EqualsVerifier will therefore assume that it - * must not be used in the equals/hashCode contract, unless + * Fields in JPA Entities that are marked @Id or @EmbeddedId are usually + * part of the entity's surrogate key. EqualsVerifier will therefore assume + * that it must not be used in the equals/hashCode contract, unless * {@link Warning#SURROGATE_KEY} is suppressed. */ ID(false, "javax.persistence.Id", "javax.persistence.EmbeddedId") { diff --git a/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java b/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java index d9ce69479..50c5ad593 100644 --- a/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java +++ b/src/main/java/nl/jqno/equalsverifier/internal/util/Validations.java @@ -102,8 +102,8 @@ private static void validateClassAnnotations( private static void validateFieldAnnotations(Class type, AnnotationCache cache, Set includedFields) { FieldIterable.of(type).forEach(f -> validate(includedFields.contains(f.getName()) && cache.hasFieldAnnotation(type, f.getName(), SupportedAnnotations.ID), - "you can't use withOnlyTheseFields on a field marked @Id.\n" + - "Suppress Warning.SURROGATE_KEY if you want to use only the @Id fields in equals.") + "you can't use withOnlyTheseFields on a field marked @Id or @EmbeddedId.\n" + + "Suppress Warning.SURROGATE_KEY if you want to use only the @Id or @EmbeddedId fields in equals.") ); } diff --git a/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java b/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java index 7e7f942bb..29122f0bc 100644 --- a/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java +++ b/src/test/java/nl/jqno/equalsverifier/integration/extra_features/JpaIdTest.java @@ -97,6 +97,14 @@ public void fail_whenIdFieldIsNotUsed_givenIdIsAnnotatedWithEmbeddedIdAndSurroga .verify(); } + @Test + public void fail_whenEmbeddedIdFieldIsTheOnlyFieldUsed() { + expectFailure("Precondition: you can't use withOnlyTheseFields on a field marked @Id or @EmbeddedId.", "Suppress Warning.SURROGATE_KEY if"); + EqualsVerifier.forClass(JpaEmbeddedIdBusinessKeyPerson.class) + .withOnlyTheseFields("id") + .verify(); + } + @Test public void succeed_whenOnlySocialSecurityIsUsed_givenSocialSecurityIsAnnotatedWithNaturalId() { EqualsVerifier.forClass(NaturalIdBusinessKeyPerson.class) @@ -158,7 +166,7 @@ public void succeed_whenMethodsAreAnnotatedInsteadOfFields() { @Test public void fail_whenIdFieldIsTheOnlyFieldUsed() { - expectFailure("Precondition: you can't use withOnlyTheseFields on a field marked @Id.", "Suppress Warning.SURROGATE_KEY if"); + expectFailure("Precondition: you can't use withOnlyTheseFields on a field marked @Id or @EmbeddedId.", "Suppress Warning.SURROGATE_KEY if"); EqualsVerifier.forClass(JpaIdBusinessKeyPerson.class) .withOnlyTheseFields("id") .verify();