On Thu, 2005-11-03 at 04:38, 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 think it's also important to make sure all the attributes are changed to "private" level access. I am going to make that change in all the updates I will be committing. -- Allen > > > > 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 > >>>>>> > >>>>> > >>>> > >>> > >> > >> > > >
