Skip to content

Commit

Permalink
jqno#231: improves error messages regarding @EmbeddedId
Browse files Browse the repository at this point in the history
  • Loading branch information
jqno committed Feb 18, 2019
1 parent d953bbc commit 58cfd57
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))


<a name="3.x"/>
Expand Down
15 changes: 8 additions & 7 deletions src/main/java/nl/jqno/equalsverifier/Warning.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
* <p>
* 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.
* <p>
* 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,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand All @@ -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.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ private static void validateClassAnnotations(
private static void validateFieldAnnotations(Class<?> type, AnnotationCache cache, Set<String> 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.")
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 58cfd57

Please sign in to comment.