Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

more JPA @EmbbeddedId wrongly detected as surrogate #231

Closed
xenoterracide opened this issue Jan 26, 2019 · 8 comments
Closed

more JPA @EmbbeddedId wrongly detected as surrogate #231

xenoterracide opened this issue Jan 26, 2019 · 8 comments

Comments

@xenoterracide
Copy link

xenoterracide commented Jan 26, 2019

version 3.1.4

internal class Packet100hzTest {
    @Test
    fun equalsAndHashcode() {
        EqualsVerifier.forClass(Packet100hz::class.java)
            .suppress(Warning.SURROGATE_KEY)
            .withOnlyTheseFields("rfid" )
            .verify()
    }
}

java.lang.IllegalStateException: Precondition: you can't use withOnlyTheseFields when Warning.SURROGATE_KEY is suppressed.

my Id is an @EmbeddedId

an example app https://www.callicoder.com/hibernate-spring-boot-jpa-composite-primary-key-example/

making this has made me think, maybe @Id shouldn't always be a surrogate, it's only with @GeneratedValue according to. @EmbeddableId doesn't extend @Id though so why is it being detected as a surrogate.

https://www.objectdb.com/java/jpa/entity/id

not sure if we should reopen #225

@jqno
Copy link
Owner

jqno commented Jan 26, 2019

Hi Caleb,

With a 3 week old baby in the house, I don't have a lot of spare time to work on EqualsVerifier... can you please give me something that I can copy-paste directly into my IDE and run without compiler issues, like the issue template asks?

What do your class and equals method look like?

@xenoterracide
Copy link
Author

xenoterracide commented Jan 26, 2019

kotlin generates equals/hashcode methods https://kotlinlang.org/docs/reference/data-classes.html

@Embeddable
data class RFID(
    @NotBlank @Column(nullable = false) val rfid: String,
    @NotNull @Column(name = "timestamp") val timestamp: LocalDateTime
) : Serializable
@Entity
@Table(name = "packet_100hz_secure")
data class Packet100hz(
    @EmbeddedId val rfid: RFID
)

also congratulations

@jqno
Copy link
Owner

jqno commented Jan 27, 2019

Thanks.

When you suppress SURROGATE_KEY, a withOnlyTheseFields on whatever fields that have an @Id or @EmbeddedId annotation is implied, so you don't have to add it yourself. Also, it wouldn't make sense with a surrogate key to add other fields to the list of withOnlyTheseFields, so that's why it's not allowed to combine the two.

Based on the discussion in #228 I decided to have EqualsVerifier handle @Id and @EmbeddedId in the same way.

@xenoterracide
Copy link
Author

the suppress works, the error given first mentions @Id only and should probably mention @EmbeddedId

So this is the first time ever I've needed a compound key, or natural key with JPA. What I learned is if my @id doesn't have a @GeneratedValue it might not be a surrogate (e.g. if I had a @Id val rfid: String), an Id marker of some sort is required (hibernate fails to load unless all entities have one). If they can both be natural? then maybe this logic should be on the @GeneratedValue... I'm not sure which is why I'm wondering if we should swing back into #225 (of which the original spawning issue of mine presumed we'd not be banning surrogates, and this scenario never occurred to me)

@jqno
Copy link
Owner

jqno commented Jan 29, 2019

It's very likely I didn't update all of the error messages when I added support for @EmbeddedId, so I'll fix that in the next release.

Broadly, EqualsVerifier recognises two situations:

  • @(Embedded)Id is treated as a business/natural key when the id field IS NOT included in equals.
  • @(Embedded)Id is treated as a surrogate key when the id field IS included in equals, and no other fields are included in equals.

Since your class only contains an id and nothing else, and this id is included in equals (because that's how Kotlin generates it), for EqualsVerifier, your id is a surrogate key and you should mark it as such.

I agree with you that it's really a business/natural key, because RFID is a thing that exists in the world, but EqualsVerifier sadly can't smell what meaning a type has to humans.

I prefer not to drag @GeneratedValue into this, because that adds a lot of complexity that I don't have a firm grasp on (yet). For example, a UUID could be a @GeneratedValue, but it doesn't have to be.

I prefer to open new issues instead of reverting to #255, so let's just stay here :).

@xenoterracide
Copy link
Author

There's a third one I've never used mentioned in the objectdb documentation (linked above), @IdClass seems to serve much the same purpose as Embedded except the fields are duplicated in the entity.

@jqno
Copy link
Owner

jqno commented Jan 29, 2019

Looks like @IdClass goes on the class, not on the fields: https://docs.oracle.com/javaee/7/api/javax/persistence/IdClass.html
So @Id is still necessary.

@jqno
Copy link
Owner

jqno commented Feb 18, 2019

I've released version 3.1.5, where the error messages should be better.

@jqno jqno closed this as completed Feb 18, 2019
akhalikov pushed a commit to akhalikov/equalsverifier that referenced this issue Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants