Don't prevent using withOnlyTheseFields (etc) when JPA Id fields are present

[xenoterracide]

I'm having a hate relationship with my own thoughts on JPA right now, but...

java.lang.AssertionError: EqualsVerifier found a problem in class com.xenoterracide.jpa.Bar.
-> Precondition: you can't use withOnlyTheseFields on a field marked @Id or @EmbeddedId.
Suppress Warning.SURROGATE_KEY and remove withOnlyTheseFields if you want to use only the @Id or @EmbeddedId fields in equals.

but I was trying to also compare with the @Version field, and that

java.lang.AssertionError: EqualsVerifier found a problem in class com.xenoterracide.jpa.Bar.
-> Significant fields: equals should not use version because Warning.SURROGATE_KEY is suppressed and it is not marked as @Id or @EmbeddedId, but it does.

@Version is just a field that increments in some way for optimistic locking, can be a number or a timestamp (of some kind)

but including this in my contract would allow me to have 2 versions of the same entity in a set. Is that a good idea? I don't actually even know what happens if I try to add a matching equals to a set... is it ignored or overwritten, probably the former.

I feel like I want to compare id, version, and find a way to expose the dirty tracking to equals. On the other hand now I'm just wondering if I'm stupid. Sigh, why's everything got to be so complicated? dear creator of java, equals/hashcode is a terrible idea. Somehow this is all my fault... wasn't this my idea from years ago? :'(

I guess my fuss here is that withOnlyTheseFields can't be used if I have something marked with an @Id, thereby taking control out of my hands

[jqno]

It's not so much Java that made things complicated. I mean, it's complicated, but other languages (like C#) have exactly the same complexity, and other languages (like Ruby) just ignore it and sweep it under the rug. It's only when you start using JPA/Hibernate when things get messy.

I'm not really sure what you're asking for here. @Id fields are special, and in EqualsVerifier, you have three options:

• use only non-id fields. In this case you're free to use withOnlyTheseFields.
• suppress SURROGATE_KEY and use only id fields. In this case it doesn't make sense to use withOnlyTheseFields, because that's already implied by the fact that you're using only the id fields.
• suppress SURROGATE_OR_BUSINESS_KEY, use any field you like, and deal with the consequences. But at least you're free to use withOnlyTheseFields.

I have to admit I haven't used @Version and I'm not sure how it affects equality in JPA entities. But from your story it seems like any behaviour around that should be folded into whatever EqualsVerifier does when SURROGATE_KEY is suppressed.

[xenoterracide]

It's only when you start using JPA/Hibernate when things get messy.

I think it's really a problem with Entities as it's anything that has a lifecycle that has a single identity but it's state changes over time and it persists beyond the current instance of the application.

I have to admit I haven't used @Version and I'm not sure how it affects equality in JPA entities.

It doesn't discuss that, but here's the docs.

https://docs.jboss.org/hibernate/orm/6.4/userguide/html_single/Hibernate_User_Guide.html#locking-optimistic-mapping

suppress SURROGATE_OR_BUSINESS_KEY, use any field you like, and deal with the consequences. But at least you're free to use withOnlyTheseFields

hmm... maybe this story could be remediated by mentioning this solution in the current errors?

maybe we should discuss how @Version should be handled in a different ticket. Context for this ticket only.

there's also another thought that had too, what about distinguishing aggregates (in a Domain Driven Design Sense), they frequently, these days, have an in memory collection of domain events to be emitted later, usually a kind of record of changes to them. Feels like a candidate. This is a transient field, and it's also not one to be serialized for jackson. Anywhays.

So to reiterate, I think to solve this issue you should emit something about SURROGATE_OR_BUSINESS_KEY in these error messages.

[xenoterracide]

I opened #935 to discuss null ids, version, and dirty checking. Trying to give a more comprehensive look.

[xenoterracide]

java.lang.AssertionError: EqualsVerifier found a problem in class com.xenoterracide.jpa.BarEntity.
-> Precondition: you can't use withOnlyTheseFields on a field marked @Id or @EmbeddedId.
Suppress Warning.SURROGATE_KEY and remove withOnlyTheseFields if you want to use only the @Id or @EmbeddedId fields in equals.

  @Test
  void abstractEntityEquality() {
    EqualsVerifier.forClass(BarEntity.class)
      .withRedefinedSuperclass()
      .withPrefabValues(AbstractIdentity.class, BarEntity.Id.create(), BarEntity.Id.create())
      .suppress(Warning.SURROGATE_OR_BUSINESS_KEY)
      .withOnlyTheseFields("id", "version", "dirty")
      .verify();
  }

[jqno]

I'm not sure if it makes sense for EqualsVerifier to look at the @Version field when Warning.SURROGATE_KEY is suppressed. I feel like it depends on the use case whether or not it should be included in a class's equals contract.

Maybe the behaviour should be that if this warning is suppressed, it's allowed but not mandatory to include it in equals. That's something I could add to EqualsVerifier. Either way, suppressing Warning.SURROGATE_KEY while also allowing withOnlyTheseFields makes no sense, because the suppressed warning already implies a withOnlyTheseFields("id").

In the mean time, would it be an idea to suppress Warning.SURROGATE_OR_BUSINESS_KEY, and write your own equals method based on the id, version and dirty fields?

[xenoterracide]

I'm not sure if it makes sense for EqualsVerifier to look at the @Version field when Warning.SURROGATE_KEY is suppressed. I feel like it depends on the use case whether or not it should be included in a class's equals contract.

It does, at least that's the conclusion I came to in my experiments in #935 although I think perhaps that ticket is better for the discussion of "what" needs to be in a persisted entity check and why. Then action items (if any) can spawn from it.

For this ticket, I'd rather focus on my not being able to get Equalsverifier to do what I say is correct, regardless of what anyone thinks is right. I don't seem to be able to get it to "do what I say".

In the mean time, would it be an idea to suppress Warning.SURROGATE_OR_BUSINESS_KEY, and write your own equals method based on the id, version and dirty fields?

That's what I tried in the comment above, this literally gives me the same error about supressing Warning.SURROGATE, I don't seem to be able to escape that.

Btw, all of this code exists in my public repo

  @Test
  void abstractEntityEquality() {
    EqualsVerifier.forClass(BarEntity.class)
      .withRedefinedSuperclass()
      .withPrefabValues(AbstractIdentity.class, BarEntity.Id.create(), BarEntity.Id.create())
      .suppress(Warning.SURROGATE_OR_BUSINESS_KEY)
      .withOnlyTheseFields("id", "version", "dirty")
      .verify();

[jqno]

Ah, I missed that you were using Warning.SURROGATE_OR_BUSINESS_KEY instead of Warning.SURROGATE_KEY in your previous comment. In my defense, it wasn't mentioned in the original ticket 😅 So I suppose my mind skipped over it in your later comment.

But I agree that in that case, withOnlyTheseFields should probably just work as normal.

[jqno]

I've just released version 3.16, which fixes this.