Should HibernateSaveOperation be saveOrUpdate?

[brittm]

I've noticed (accidentally of course, -ahem-) that if you attempt to call jbpmContext.save(processInstance) on a pi that was loaded outside of the current session, Hibernate will save a new pi with a new Id--however it will reference the same rootToken, etc. as the original pi, and all kinds of unexpected behavior quietly ensues.

Of course, loading a fresh copy of the pi immediately before you do anything with it avoids the problem; however, a lot of serious data integrity hangs on remembering to do that every time. (for instance, when a pi being viewed in a UI is injected into an action component, the programmer must remember to load up a fresh copy before working on it.)

Since failing to re-load a detached pi can result in what seems to be a broken jBPM data model, jBPM should probably be doing something itself to avoid the problem.

I think there are possibly two ways to prevent this from happening:

1) Simply changing the HibernateSaveOperation.save(processInstance) method to saveOrUpdate() seems to perform cleanly.
2) Do a check on the pi, and throw an error if it doesn't belong to the current hibernate session.

I did a forum/jira search on saveOrUpdate and didn't come up with anything recognizable, so unless someone shows that I'm just missing something here, I'll create a jira issue for this.

[kukeltje]

I'd go for 1 if there are no side effects.

[tom.baeyens]

i'm very scared of side effects. i recall having changed that once or twice over time and that the diffs were very subtle.

[brittm]

I've heard down the line that Gavin and company somewhat regret introducing a difference between 'save' and 'saveOrUpdate', and that given an opportunity to do it again they would not bother with only 'save'ing. In fact, its my understanding that JPA itself only supports a 'saveOrUpdate' concept. But I'm simply repeating what I've heard rather than what I know myself.

Perhaps someone could ping Gavin and he could comment on any nuances between the two that might affect us.