I have a couple comments below ...
Mitesh Meswani wrote:
Dave wrote:
On 1/18/07, Mitesh Meswani <[EMAIL PROTECTED]> wrote:
I think it is not a good idea to mutate Primary Key (ID) field of
any pojo.
I agree.
Many persistence providers might choke if a pk field is mutated.
Pardon my ignorance on Struts - How are these forms used at runtime? Do
they correspond to any screen in UI?
Yes, here's how it works:
When a HTTP POST comes in to update a FooData object, Struts hands us
a FooForm object containing the data from the posted web form.
We use FooForm.id to load the corresponding FooData object from the
database, use FooForm.copyTo() to copy all updated properties to the
FooData object
Thanks. Now I understand :)
I think this not entirely true. I think the reason the old store()
method was wacky is specifically because this is not how it was happening.
What happens when a form is posted is that the form copied *all* data
into a new (non-persistent) pojo object, *including* the id value. So
the struts actions would often not actually query for a pojo and mutate
it, instead they would construct a new pojo and copy all attributes of
the pojo from the form, which is not a good way to do things.
I am suspecting that you are getting around this problem now by having
the forms do the same thing and Hibernate's saveOrUpdate() method is
being clever and doing the same thing the old store() method did. i.e.
if you try and save a pojo that is not currently managed by Hibernate
then it tries to use the id value to load that object, then update it.
Regardless of what's actually happening, the correct way (which we
*must* fix) is to fix the way the current StrutsForm -> Pojo mapping
happens and make sure that id values cannot be copied. As Mitesh said,
we should *never* allow an id on a pojo to be manually set when that
pojo is allowing the framework to manage the id. In fact, in these
cases I think the correct thing to do is to make the setId() method
private, which is actually suggested in the Hibernate In Action book
somewhere.
So what this entails is modifying the pojos to make setId() private and
fixing the way the struts actions/forms work to make sure all actions
which modify an existing pojo actually query for that pojo first, then
mutate it.
and then we call FooManager.saveFoo() to persist those
chances.
Technically, the call to saveFoo() isn't necessary with Hibernate or
JPA because FooData is a persistent/managed object, but perhaps other
persistence frameworks (iBatis?) might beed that final call to
FooManager.saveFoo().
We should distinguish between save and update. Thus in above case
instead of calling FooManager.saveFoo(), we should call
FooManager.updateFoo() which will be implemented as a noop for JPA or
Hibernate. Other frameworks can implement it as required.
I disagree. There is no real reason why we must treat saves and updates
separately. I think this is something that the framework can take care
of on it's own and it's actually handled in a very simplistic way ...
if(object.id != null)
update()
else
save()
and the framework manages what attribute of the pojo is the id through
its mapping files. so I don't think we need to do this.
-- Allen
Regards,
Mitesh
- Dave
