A bit more on equals().
(1) We have a default reflective implementation of equals in
PersistentObject (I think), and I believe it is uses field-level
reflection, not "bean-style" property getter introspection/reflection.
This may be dangerous..
(2) When we do override equals(Object other), we have a tendency to
reach into other.field for comparisons. I don't know if Hibernate
proxies are "smart" enough to instantiate both x and y when x.equals(y)
is invoked, so we should also use getters there to be safe.
(3) Hibernate recommends a weak "semi-unique" equals based on business
keys only, for a reason I haven't fully grasped yet. We tend to
implement traditional strong equals. This might be something to reexamine.
--a.
Dave Johnson wrote:
On Nov 3, 2005, at 4:43 AM, Anil Gangolli wrote:
No real objection here, but a comment.
Even for the case of lazy loading, the use of getters for all
internal acess may be overkill. A given object should get
instantiated before entry to any of its methods, so code within that
method should be able to directly use "proper" (non-relational)
members. I think basically what's biting us is the reaching in from
one instance to another instance's members (which we do in the
setData() / "copy constructor" methods and typically also in equals()
.).
We ran into some problems yesterday with the all getters & setters
approach and we're only going to handle the setData() and copy
constructor cases as you suggest.
I admit that being pedantic about member access eliminates any
reliance on specific Hibernate semantics around instantiation, (which
I've had a hard time finding described very completely). On the
other hand, if we had an adequate understanding of Hibernate's
precise instantiation semantics, we may feel confident in maintaining
what seems to me to be simpler code (even with lazy loading enabled).
Agreed.
- Dave
--a.
Dave Johnson wrote:
Allen and I are getting ready to commit a fix for ROL-865
http://opensource2.atlassian.com/projects/roller/browse/ROL-865
This is a fairly big fix, so extra warning is needed. There are two
parts to the fix:
1) Apply "encapsulate members" refactoring to all POJOs so that all
use getters and setters to access members, even internally. This
should ensure that Hibernate lazy-loading will work, when it is in
effect.
2) Upgrade to XDoclet 1.2.3 and add @hibernate.class lazy="false" to
all POJOs. This will revert our use of lazy-loading back to the way
it was in Roller 1.x, that is lazy-loading is off by default and
turned on explicitly for (most) collections.
- Dave
On Nov 2, 2005, at 1:34 PM, Dave Johnson wrote:
I'm going to go ahead and commit my changes to the POJOs. I added
lazy="false" to each class, but that will have no effect until I
commit XDoclet 1.2.3.
I'm holding off on committing XDoclet 1.2.3 in case somebody has a
last minute objection.
Any objections?
- Dave
On Nov 2, 2005, at 12:41 PM, Dave Johnson wrote:
On Nov 2, 2005, at 12:25 PM, Allen Gilliland wrote:
So it sounds like there are a couple options ...
1. alter our hibernate mappings to for lazy="false" by default.
this would require upgrading our xdoclet version.
2. alter our pojos to force the use of getters/setters and
prevent the use of direct accessors.
Assuming we've correctly identified the problem, #2 alone should
solve it.
personally, i think we should do both, but i would be more
adamant about doing #2. the standard pojo/bean pattern
recommends that attributes be private and only accessed through
getters and setters and i believe we should adhere to that. it
seems pretty lame that hibernate couldn't deal with direct
access, but oh well. with a bit of IDE wizardry it shouldn't be
too hard to make this happen.
of course it also makes sense that we probably don't need to use
lazy loading on *all* pojo properties, so defaulting to
lazy="false" would be a good thing too. i don't know all the
details about why that was changed in hibernate 3, maybe there is
a good reason to use lazy loading all the time? maybe this would
account for the performance improvements you noted on your site
Dave?
That's possible, I guess, but I really don't know the answer.
what does everyone else prefer to do?
I'd prefer to do both #1 and #2 right now, but could be talked
into saving #1 for later.
- Dave
-- Allen
On Wed, 2005-11-02 at 06:17, Dave Johnson wrote:
On Nov 2, 2005, at 7:44 AM, Dave Johnson wrote:
Anil wrote:
Changing the loading behavior in the mappings to be non-lazy one
should be able to avoid this behavior. If we have a lot of code
dealing with pojo members directly, we may want to do this. I
believe the laziness defaults changed between 2.x and 3.x.
The more involved fix is to upgrade to the latest XDoclet
(which may
require building it from CVS to get around that bug that bit
me), set
lazy="false" at the class level and set lazy="true" for the
individual
collections that we want to be lazy-loaded (and I think we're
already
doing that). We're using XDoclet 1.2b4 and the latest XDoclet
is 1.2.
I've implemented that more involved fix in my workspace. I
upgraded to
XDoclet 1.2.3, found a reasonable work-around for that bug I
mentioned
and set lazy="false" at the @hibernate.class level for all 28 of
our
POJOS. Roller appears to be working fine in my workspace.
There are three reasons why we might want to commit this for 2.0:
1) it solves the intermittent null problem (we need to verify this)
2) it's better to use an official release of XDoclet rather than
custom
1.2b4 build
3) it's better to stick with lazy="false" since that's what we were
doing before 2.0
Since I can't duplicate the intermittent null problem, Allen
needs to
verify that #1 is a true statement.
- Dave
