Extending the JpaLazyGetterFieldCheck to allow non-lazy annotated fields

[Romanow88]

There is a case with JPA / Hibernate that is not covered by #650. THis is in regard to the problem described in https://stackoverflow.com/questions/4334970/hibernate-throws-multiplebagfetchexception-cannot-simultaneously-fetch-multipl and the correct solution to that problem - issuing multiple queries fetch joins that would load data for all entities in collections of a result set (as described in "The proper solution" part of the accepted answer).

This sets up a new requirement for how the equals should be implemented for classes that are a part of collections that are fetched.

Example flow, looking at the example classes from SO answer:

• Load Posts with fetch join issued to comments.
• A list of Posts is retrieved with Comments initialized (loaded) and Tag collection created as proxies (not loaded, lazy).
• Load the same Posts but instead fetch join the Tags.
• A list of Posts in retrieved with Tags. Hibernate, however, guaranties "application-level repeatable reads" which means that the objects retrieved in step 2 get their Tags collections filled by this query.

For this solution to function, however, the Posts and Tags entities equals and hashCode needs to use getters (similarly to what is being done in #650). The catch is that here the class does not have any lazy annotations on it.

Describe the solution you'd like
A way to enable strict checks for using getters for equals / hashCode for JPA entities (or possibly just any regular class).

Describe alternatives you've considered
Creating a delegate / proxy on test time and checking equals manually after the EqualsVerifier.forClass().verify()

[jqno]

Thanks for bringing this to my attention.

I'm sorry, I don't have the time to thoroughly read the StackOverflow page right now. Do I understand correctly that this applies to basically any JPA entity? (Or at least JPA entities that have at least one @OneToMany or @manytoone relation? Or are there other conditions that I'm missing?

Also, the top answer says that Hibernate 2.0 should "handle this". Is that true? Or does it merely mean that the exception isn't thrown anymore, but the equals method should still be calling getters?

[Romanow88]

That is correct, this can apply to any JPA entity, if it's included in a collection of another entity.

The "handle this" does not fix the issues that occur when one has a collection with both proxies and actual objects. So even if Hibernate would handle this somehow internally at some point (e.g. issuing multiple sub queries with fetches automatically), this would not fix the equals/hashCode issues.

[jqno]

Ah, so it has to be included in a collection of another entity. I don't suppose there has to be a 'backlink' referring back to the other entity? In other words, would it be easy (or even possible) to say if it's actually needed to call the getters?

The reason I ask is that the alternative would be to require getters on all JPA entities, regardless of whether they "apply" for this problem, and that might break a bunch of tests for existing users. I would be ok with that if really needed, but I'd prefer to find another way.

[Romanow88]

There is no 100% way to find if an entity is included or not in a collection in another entity or not without scanning all entities that are included in a persistence unit. It can be possible if the PU is a part of a specific package and the package is scanned, but that is not an assumption that holds for everyone.

I do not think that enabling this for all JPA entities makes sense, due to the fact that, as you said, this would break peoples tests. un-necessarily. For us, for example, the current validation was working fine until we encountered this issue. There is also the point that this may apply only to Hibernate but may not apply to other persistence providers that implement the JPA spec.

Would it be possible to make this an opt-in check?

[jqno]

Thanks for the quick responses!

If it were possible to (easily) check if an entity is affected by this, it would be preferable to possibly breaking existing tests. However, correctness of equals methods is more important to me, so given that it's actually not possible to detect this issue, I'm leaning towards making this an opt-out check. However, that does mean that the change needs to be documented really well so that people who upgrade know what's going on. So I'll have to study this some more so I understand it better, and then update the documentation and the code. It will take a bit of time.

[jqno]

OK, let's see if I understand properly. Let's say we have a Post entity with a collection of Tag entities. It's possible for the same Post to exist with its collection of Tags materialized, or unmaterialized. Even if the FetchType for Tags is EAGER, they could still be unmaterialized, if the user does what Vlad Mihalcea describes in his answer. Hence, Post must call the getter getTags instead of the field tags in its equals method if it wants to include the tags. Is that correct so far?

I don't understand though why Tags would also need to do this unless it contains a "backlink" collection of Posts. Or does hibernate populate the Post's Tags collection with uninitialized Tag instances, where the fields are nulled out but will be fetched when somebody calls a method on it?

[jqno]

Based on my undestanding as stated above, I've just released version 3.15, where EqualsVerifier checks that getters are called for all fields marked with @OneToMany, @ManyToOne, etc. It doesn't matter if they have FetchType.LAZY or FetchType.EAGER. Only for fields marked with @Basic, it still matters if FetchType.LAZY is specified or not.

[Romanow88]

Hey @jqno, sorry for the late reply.

I've double checked with the devs on our side that were fixing this.

It seems that the issue was caused by our internal code while implementing the fix for the issue described above and not Hibernate itself, where a proxy was set to an eager field of an entity.

The fix works for our purposes but may confuse other users.

Sorry for the confusion and the late response again.

[jqno]

Ah, that's too bad, I might have made some different decisions if I'd known.

But still, I think it's the right call, given that it's indeed possible for EAGER fields to be null in certain situations.