Allen Gilliland wrote: > > > Elias Torres wrote: >> Anil, >> >> You definitely bring up some good thoughts on the issue. Thanks. More >> comments inline. >> >> Anil Gangolli wrote: >>> I think the form beans issue is separate and probably trickier to deal >>> with. You might be able to get by with a check in the >>> HibernateWeblogManagerImpl.updateTagCount() if the object is associated >>> with the current Session. I'm not sure. >> >> I extract from this a nugget: check if the object is associated with the >> current session. > > I would be careful here. In general I think this may not be the right > approach to take because we really shouldn't have to care if the tags in > the set are persisted vs. transient, hibernate should take care of that > for us.
Well.. we care about it especially if you look at the HibernatePersistenceStrategy code (store). I'm not sure how hibernate takes care of it for us. For example, if I create my own object for which I already have stored properties in the db but I don't specify an id, how would hibernate take care of it (not double storing it) in the case I call session.save()? > > I think that adding in code to do this kind of check could get very > complicated and potentially error prone. > >> >>> >>> I had some comments on the original interchange between you and >>> Allen. I think this is a pretty typical architectural issue: We need >>> Y called >>> when X.something() happens, but architecturally X really shouldn't have >>> a direct code dependency on/knowledge of Y. >> >> +1 >> >>> One of the things I've noted in working with Roller is the lack of a >>> defined lifecycle and event callbacks for the app's key business >>> objects: entry, blog/website, user, etc. >> >> Most definitely. WordPress for example has a very rich extensible >> mechanism because of their lifecycle/event/hooks/etc. >> >>> Defining key events (basic CRUD and other object-specific events of >>> interest -- rendering flow perhaps) and providing some listener >>> interfaces and registration mechanism for those events would probably >>> clean up this architecture and make a lot more pluggable fun possible. >> >> Right. > > I would agree with all of that, but IMO there are many, many things in > the codebase which need cleanup much more than that. In my mind this is > a nice to have, but not truly a necessity. > I think it's truly a necessity to have a proper lifecycle and therefore a rich extensible plugin framework. It would save us from writing everything into the core webapp as it's the case for WordPress. However, I would say instead that it's not the right time for such a rework and like you said there are many many things in the codebase needing clean up. > >> >>> In my dreams, a radical rewrite of Roller involves this sort of modular >>> definition around a Spring core. Of course, we're a good distance away >>> and not necessarily headed there. Roller 4 maybe? >> >> I'm flexible on the specific framework (for my lack of experience on >> most) but Struts is definitely not the easiest/hottest/flexible/popular >> in my opinion. > > I can see that you like to rip on struts, but because I'm evil I would > say that I don't think struts has any bearing on this situation. It's > not fair to blame struts when the problem is that we are doing some > bizarre (and IMO inappropriate) things with the way its implemented. Agree that my ripping on struts should be independent from my current issue. But I'd like to note that I was ripped on for using Struts on the #hibernate channel and not using Springs. Struts is not up to date with the current hibernate techniques and we are working around some of these issues. > > >> >>> In this specific case, you might be able to do something simple along >>> these lines for tags, e.g.: define interface TagEventListener and class >>> TagEvent and have the interested managers register with the class ? >> >> I'm not sure how this solves the problem. My problem is that I can't >> distinguish from the two different addTag() calls. Unless I use your >> suggestion of checking the session. > > I agree, lets not go down that path until we actually have a unified > plan for doing it for all the necessary places. > > >> >>> Or you could defer this type of work for later and code it directly; I >>> agree with Allen that you'ld probably not be setting a precedent by >>> doing so. >> >> I'd like more specific instructions on what do you mean by coding it >> directly please. > > Well, you could override the copyTo() and copyFrom() methods in the > WeblogEntryFormEx class and only copy the over the data that needs to be > copied. e.g. removing the calls to getTags() and setTags() > automatically embedded in those methods when they are auto-generated. Thanks for the suggestion but I'm not sure it solves the problem. We already override copyTo and copyFrom which is where we do extra logic for non-simple types. Therefore moving them from the auto-generated code to the overridden code doesn't solve my problem because I can't distinguish between copyTo/copyFrom in the action lifecycle and in the dummy method that creates a transient entry. I'm sure I can find a workaround (like adding getRemovedTags()/getAddedTags()) but I'm hoping for a suggestion that has the least chance of introducing bugs in the codebase or elegant. In general I'm getting that putting code that modifies the db inside the POJO might be just the wrong approach since Hibernate does support transient and detached objects. Therefore we should try to do things in the WeblogManager.save(). Any objections of going that route? -Elias > > -- Allen > > >> >> -Elias >> >>> --a. >>> >>> >>> >>> ----- Original Message ----- From: "Elias Torres" <[EMAIL PROTECTED]> >>> To: <[email protected]> >>> Sent: Monday, October 02, 2006 2:42 PM >>> Subject: Re: How to maintain tag aggregate table? >>> >>> >>>> I started trying option #1 and added a method updateTagCount(String >>>> name, WebsiteData website, int amount) to WeblogManager with respective >>>> calls in WeblogEntryData.addTag() and WeblogEntryData.removeTag() but >>>> ran into a wall. >>>> >>>> The problem is in the .copyTo, .copyFrom methods which are doing copies >>>> on detached objects (like in the WeblogEntryPageModel) so I would >>>> end up >>>> double counting some tags. >>>> >>>> /** returns a dummied-up weblog entry object */ >>>> public WeblogEntryData getWeblogEntry() throws RollerException >>>> { >>>> if (weblogEntry == null) >>>> { >>>> weblogEntry = new WeblogEntryData(); >>>> weblogEntry.setWebsite(getWebsite()); >>>> form.copyTo(weblogEntry, >>>> getRequest().getLocale(), >>>> getRequest().getParameterMap()); >>>> weblogEntry.setWebsite(weblogEntry.getWebsite()); >>>> } >>>> return weblogEntry; >>>> } >>>> >>>> NOTE: I don't think we need that last call to setWebsite: >>>> weblogEntry.setWebsite(weblogEntry.getWebsite()); >>>> >>>> Now I understand the comment in HibernatePersistenceStragegy on the >>>> subject: >>>> >>>> /* >>>> technically we shouldn't have any reason to support the saving >>>> of detached objects, so at some point we should re-evaluate this. >>>> >>>> objects should be re-attached before being saved again. it would >>>> be more appropriate to reject these kinds of saves because they are >>>> not really safe. >>>> >>>> NOTE: this may be coming from the way we use formbeans on the UI. >>>> we very commonly repopulate all data in a pojo (including id) from >>>> form data rather than properly loading the object from a Session >>>> then modifying its properties. >>>> */ >>>> >>>> Any suggestions on how to differentiate between a copyTo/copyFrom to a >>>> detached object so I don't recount? or should I go with the external >>>> approach of keeping track of tags added/tags removed. It seems to me >>>> that either way we have an issue with these detached objects. >>>> >>>> -Elias >>>> >>>> Allen Gilliland wrote: >>>>> >>>>> Elias Torres wrote: >>>>>> Everyone, >>>>>> >>>>>> I'm changing my code that updated the aggregated tags table from >>>>>> using a >>>>>> task to using code during the life of the request i.e. as we save >>>>>> each >>>>>> entry. >>>>>> >>>>>> My question is as follows: >>>>>> >>>>>> In WeblogEntryData we have addTag() and removeTag() meaning I have >>>>>> two >>>>>> options: >>>>>> >>>>>> - I can call WeblogManager.updateTagStat(Tag) on each >>>>>> addTag/removeTag() >>>>>> - I can perform the tag stats updates inside >>>>>> WeblogManager.save(Entry) >>>>>> >>>>>> I don't like the first as much but the second one requires me I keep >>>>>> track of added/removed tags so I can access that information at save >>>>>> time. ie. entry.getAddedTags(), entry.getRemovedTags(). >>>>> I would just go for the first option. It's nice if we don't have to >>>>> refer to managers from the pojos, but to not do that would make things >>>>> harder on ourselves, so just do it. I am pretty sure we are already >>>>> doing that anyways. >>>>> >>>>> -- Allen >>>>> >>>>> >>>>>> What do you think? >>>>>> >>>>>> -Elias >>> >
