This is a quick summary of the issues around EntityRepository.save()
that seem to exist since 1.4.2.
I'll create test cases and JIRA issues as time permits, but I think
these notes may be helpful at this stage to find out whether or not
incompatibilities experienced by other users have the same root cause.
According to Javadoc [1], this is what save() does:
"Persist (new entity) or merge the given entity. The distinction on
calling either method is done based on the primary key field being null
or not. If this results in wrong behavior for a specific case, consider
using the EntityManagerDelegate which offers both persist and merge."
As far as I can see, the description accurately describes the behaviour
in 1.4.1.
The behaviour was changed in 1.4.2 in an incompatible way without
adapting the documentation. Since 1.4.2, if the primary key is not null,
DeltaSpike also runs a database query to check whether an entity with
the given key exists. If so, it calls merge(), otherwise persist().
So there are now quite a few cases when save() calls persist() where it
would have called merge() in 1.4.1.
Some use cases:
Use case 1:
// assume separate transactions.
foo = save(foo);
remove(foo);
foo = save(foo);
Result in 1.4.1:
foo is persistent. The second save() is a merge().
Result in 1.4.2:
Exception. The second save() is a persist(), since the key no longer
exists. But then Hibernate complains it cannot persist a detached entity.
Use case 2:
Assume a one-to-many association Blog -> Comment, both with
auto-generated identities of type long.
public Blog createBlog(List<Comment> comments) {
Blog = new Blog();
blog.setComments(comments);
blog = blogRepository.save(blog);
blog.getComments().add(new Comment());
}
Now someone calls createBlog(Arrays.asList(c1, c2)).
In 1.4.1 this call succeeds. In 1.4.2, there is an
UnsupportedOperationException since blog.getComments() is immutable.
In 1.4.1, the identity member is 0. 0 is not null, so save() performs a
merge(). merge() creates a new blog instance with a new mutable comments
list (at least with Hibernate (PersistentBag)).
In 1.4.2, there is no existing blog entity with identity 0, so save()
calls persist() and returns the original blog instance. Its comments
member is the result of Arrays.asList() which is immutable.
Sometimes it is useful to question the things you think you know, like
"an entity with an identity value of 0 or null cannot be persistent". I
searched the JPA 2.1 spec and did not find a word about zero identities.
In fact, Eclipselink has an option to *enable* zero identities
(eclipselink.allow-zero-id).
The exception from Hibernate is also misleading. Just because an entity
passed to persist() has a non-null and non-zero key, it does not have to
be detached, it may well be a new transient entity.
I never really liked the fact that save() blurs the distinction between
persist() and merge(), and by its very name it encourages users to
always call save() after changing an entity state which is usually
unnecessary and sometimes even wrong - so far, I've seen each and every
new user of DeltaSpike Data doing that.
At the moment, I don't see a way to specify and implement save() in a
way that is logically consistent *and* portable across JPA providers.
(I'll be happy to be proved false.)
So for the next release we should do one of the following:
1) Adapt Javadoc to the current behaviour and document the
incompatibilities.
2) Leave Javadoc unchanged and restore compatibility.
3) Specify a new expected behaviour and adapt both Javadoc and
implementation.
4) Add a big fat warning to save() Javadoc and move persist() and
merge() from EntityManagerDelegate to EntityRepository.
[1]
http://deltaspike.apache.org/javadoc/1.4.2/org/apache/deltaspike/data/api/EntityRepository.html#save-E-
Regards,
Harald
