Hibernate proxies cause problems with equals method's direct property access.

[lombokissues]

Migrated from Google Code (issue 110)

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 18:19 UTC

What steps will reproduce the problem?

1. Use @ EqualsAndHashCode on a Hibernate POJO.
2. Generate a query that returns a proxy object but doesn't initialize it.
3. Call equals() method on the POJO and pass in the uninitialized hibernate
   proxy.

What is the expected output? What do you see instead?
Since no methods are actually called on the proxy object, it remains
uninitialized. All references in the uninitialized proxy are null.

What version of the product are you using? On what operating system?
Lombok 0.9.2. Hibernate 3.3.2 GA with JavaAssist bytecode generator.

Please provide any additional information below.
The simplest (and possibly most convenient) solution would be to add a flag
to @ EqualsAndHashCode like:

boolean useAccessors() = false;

If true, Lombok would use generated (if applicable) or user-specified
accessors when accessing properties in equals() (and hashCode(), though I
only mention it because this single annotation generates both). Code like
(attached) java.txt would end up generating (attached) lombok.txt.

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 18:19 UTC

🔗 java.txt

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 18:19 UTC

🔗 lombok.txt

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 18:20 UTC

Forgot to put "useAccessors=true" in the @ EqualsAndHashCode annotation, but you get
the idea. Thanks!

[lombokissues]

👤 reinierz   🕗 Mar 22, 2010 at 19:41 UTC

Actually, equality (and thus, hashCodes) for JPA/Hibernate proxies is a hairy issue. For example, hitting each field
via getX(), and then calling .equals() on each field (which we do; objects are equal if all fields are equal to each
other, which requires another equals call per non-primitive field), which would in turn hit all its fields with getX()
and call equals on all of them, can and usually will result in 1 equals call querying half your entire database.

We've met this need by allowing you write @ EqualsAndHashCode(of="foo"), where foo should be some simple
suitable field such as unid.

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 20:05 UTC

The example I posted actually satisfies the criteria you specified (write
@ EqualsAndHashCode(of="foo") where foo should be a simple field) but could STILL
trigger the issue with the uninitialized proxy, if this proxy is an argument to
equals(). Thus there is absolutely no safe way to use @ EqualsAndHashCode with
Hibernate POJOs. EVER. Even with properties of type int, char, etc. (they don't get
initialized either until a non-PK accessor or other method is called). Unless you
disable bytecode-assisted optimization in Hibernate (which I don't think is possible
anymore), or take absolute care that all your objects are initialized if you think
they're going to end up in a collection on which you call contains(), or some other
method.

The hashCode portion of the annotation doesn't suffer from the uninitialized proxy
problem, because hashCode() is an overridable method invoked on its members and will
automatically trigger proxy initialization before any private properties are directly
read.

Now my opinion is if I'm taking time enough to use "of={ ... }", then
I'm knowledgeable enough about the underlying database relationships, and thus I
should be responsible for worrying about database hits. If I end up with more than
just a few database hits while calling equals(), I need a smack in the head for my
design or laziness. Lombok shouldn't be responsible for this.

Even without Hibernate, can I make a valid case for a property that needs to be
mucked about with in the accessors? e.g. (attached) MyClass.java.

Note there's no "myProp" property, but it looks like there is to other callers since
getProp and setProp are there. Now this is sounding more like a JavaBeans kind of
problem set, but where I found the lombok annotations so convenient in this case I
thought I'd ask. At the very least, is it possible to have a standalone @ HashCode
annotation, since it doesn't trigger the same issue with Hibernate proxy objects?
Then I can just write my equals() method and be done with that part of it.

[lombokissues]

👤 silvaran   🕗 Mar 22, 2010 at 20:05 UTC

🔗 MyClass.java

[lombokissues]

👤 reinierz   🕗 Mar 23, 2010 at 06:46 UTC

Hibernate POJOs are always annotated with @ Entity, no?

It would be a rather sizable injection of 'magic', but how about this:

We'll switch to calling the get methods if there's an @ Entity annotation?

The alternative is that we simply always use getX() instead of X; the performance impact is negligible as the
JVM inlines such calls if the object is involved in an oft-run piece of code.

[lombokissues]

👤 silvaran   🕗 Mar 23, 2010 at 16:54 UTC

I like your note about the performance impact. I think you'd have a better
understanding of any other side effects that might occur while using accessors
instead of direct property accesses. Essentially you'd be accepting the JavaBeans
way of referring to properties (so that you can have "virtual" properties that don't
actually exist as a concrete private property).

@ Entity is only used when annotations are used to map Hibernate POJOs. XML mapping
files are still supported, and provide the same mapping abilities (though
non-standardized at least as far as JSRs are concerned), and don't necessitate adding
a whole slew of JARs to your project for annotation mapping support.

Or we could rally Orasun to add true property support a la C﹟. :)

[lombokissues]

👤 reinierz   🕗 Mar 24, 2010 at 23:14 UTC

Oh, yeah, Orasun is going to get right on that lickety split :)

So, the concensus is for Roel and I to sit down and figure out if there's any big (negative) impact to rerouting the
equals/hashCode calls to use the getters instead of direct field access, yeah?

Can anyone help us along and point out some issues with going the getter route? Any performance issues
preferably stated in the form of a profiler report. In my experience, reasoning about JVM performance never
works very well.

[lombokissues]

👤 mwanji   🕗 Mar 25, 2010 at 15:36 UTC

In a hand-written equals(), I use getters, too, which seems to be the recommended way
of avoiding problems with proxies. Any performance issue is going to come from
possibly triggering DB queries.

And you'd have to exclude getId() to be able to compare a saved object to an unsaved
one. Though it's a best-practice, I never find myself mixing saved and transient
instances in collections or directly comparing them. So I'm not sure what the default
behaviour should be. If you do compare IDs and at least one is set, then that might
be sufficient.

[lombokissues]

👤 silvaran   🕗 Mar 25, 2010 at 15:51 UTC

I use getters as well, but usually only for the argument (e.g. 'return
this.employeeName.equals( o.getEmployeeName() )), but that's getting a little
pedantic. Essentially you're guaranteed that "this" is properly initialized, because
you're in a non-static method.

In any case, I make sure the object graph traversed by the equals() method only goes
so deep. Occasionally I'll use the primary key for the equals contract, because
there's no clearly-defined business key enforced by the database, or because it would
take 4-5 traversals (effectively e.g.
this.getParent().getOtherParent().getAnotherParent().getName(), which could cause 3
database fetches) to actually return a result.

[lombokissues]

👤 mwanji   🕗 Mar 25, 2010 at 16:04 UTC

"I use getters as well, but usually only for the argument"

I guess that works because the proxies implement an equals() that delegates to the
wrapped object?

"this.getParent().getOtherParent().getAnotherParent().getName()"

Does that really happen? I was thinking more along the lines of
this.getOtherEntity().equals(that.getOtherEntity()), which might trigger any number
of queries. Should @ ManyToOne and @ OneToMany relationships be eliminated by default?
(I'm not even sure Lombok can resolve that kind of thing)

[lombokissues]

👤 silvaran   🕗 Mar 25, 2010 at 16:26 UTC

this.getParent().getOtherParent().getAnotherParent().getName()

That was just an example. In reality it would be exactly like your example
(this.getOtherEntity().equals( o.getOtherEntity() ). I have a database design now
where the natural id is a combination of two foreign keys. Those two foreign tables'
natural ids are each a single string column. But for the first entity:

equals( Object o ) {
return this.getFirstEntity().equals( o.getFirstEntity() )
&& this.getSecondEntity().equals( o.getSecondEntity() )
}

So each of those two calls to equals() would cause two separate database hits (4
total) if all 4 properties are uninitialized proxies. The first two database hits
occur for each equals() call, and the second two would only occur if those equals()
methods themselves use getter calls.

That's why I'm a little confused about this stuff, especially as to how responsible
Lombok should be for this, because I don't really think I take advantage of natural
keys. I never call equals(), hashCode() or even compare() on a transient (no-PK)
entity, so what's the problem with using the IDs (which are always initialized) in
@ EqualsAndHashCode? Opinions?

[lombokissues]

👤 silvaran   🕗 Mar 25, 2010 at 16:28 UTC

This issue report is turning into a Hibernate issue report, but I just wanted to add
one more thing--maybe as a temporary workaround, we could simply declare, in the
mapping file, that any foreign key entities involved in the natural id be fetched
eagerly, so they'll never be uninitialized proxies.

[lombokissues]

👤 silvaran   🕗 Apr 01, 2010 at 20:40 UTC

Proxies don't initialize the primary key either. Unless lombok's equals()
implementation will use the argument's accessors, @ EqualsAndHashCode can never be
used with Hibernate POJOs.

[lombokissues]

👤 reinierz   🕗 Jul 19, 2010 at 15:09 UTC

Sounds like just using the getters (if available) is the right option.

[lombokissues]

👤 reinierz   🕗 Jul 22, 2010 at 12:21 UTC

Completed in time for release 0.9.3 (Burrowing Whale) in a series of commits, ending in ceda2e5

[lombokissues]

End of migration