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.
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.
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.
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.
-- 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
