Allen Gilliland wrote:
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()
This would be true if the id is generated as is the case with Roller.
For objects where id is user supplied, above algorithm can not be used.
Thats the reason some frameworks (for example JPA) does not provide
equivalent of saveOrUpdate() method.
-Mitesh
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
