8

My code was working perfectly fine for a long period, but after a few refactors I noticed I suddenly couldn't save a Group object anymore.

I was getting the dreaded Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) error. After Googling of course I found this StackOverflow question, but it didn't help me at all as I wasn't doing anything concurrently.

After going through my refactors, the only difference I found is this change.

Before it was:

    final Collection<Sample> allByBarcode = sampleService.byBarcode(groupRequest.getSamples(), currentUser);

    if (!allByBarcode.isEmpty()) {
        Group group = new Group();

        group.setName(groupRequest.getName());
        group.setSamples(allByBarcode);
        group.setType(groupRequest.getType());
        group.setOwner(currentUser);

        group = repository.save(group);

        return Optional.ofNullable(group);
    }

after refactoring (don't remember exactly why) it became:

    final Collection<Sample> allByBarcode = sampleService.byBarcode(groupRequest.getSamples(), currentUser);

    if (!allByBarcode.isEmpty()) {
        Group group = new Group();

        group.setName(groupRequest.getName());
        group.setSamples(new HashSet<>(allByBarcode));
        group.setType(groupRequest.getType());
        group.setOwner(currentUser);

        group = repository.save(group);

        return Optional.ofNullable(group);
    }

After changing it back to the original, It suddenly started working again every time, with no errors whatsoever.

Could anyone please explain what's the reason to this error as this is literally the only difference in the code that made it work again?

Update 1:

I tried another variant:

Group group = new Group();

group.setName(groupRequest.getName());
group.setSamples(new ArrayList<>(allByBarcode));
group.setType(groupRequest.getType());
group.setOwner(currentUser);

group = repository.save(group);

Note ArrayList instead of HashSet – for some reason this code works.

I've also tried LinkedList which also works.

Any ideas?

Update 2:

The stack trace is as follows: I have cut it to remove lots of Spring and Tomcat-related trace.

Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.springframework.orm.ObjectOptimisticLockingFailureException: Object of class [uk.ac.sanger.mig.aker.domain.Group] with identifier [93]: optimistic locking failed; nested exception is org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [uk.ac.sanger.mig.aker.domain.Group#93]] with root cause

org.hibernate.StaleObjectStateException: Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) : [uk.ac.sanger.mig.aker.domain.Group#93]
        at org.hibernate.persister.entity.AbstractEntityPersister.check(AbstractEntityPersister.java:2541)
        at org.hibernate.persister.entity.AbstractEntityPersister.update(AbstractEntityPersister.java:3285)
        at org.hibernate.persister.entity.AbstractEntityPersister.updateOrInsert(AbstractEntityPersister.java:3183)
        at org.hibernate.persister.entity.AbstractEntityPersister.update(AbstractEntityPersister.java:3525)
        at org.hibernate.action.internal.EntityUpdateAction.execute(EntityUpdateAction.java:159)
        at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:465)
        at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:351)
        at org.hibernate.event.internal.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:350)
        at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:56)
        at org.hibernate.internal.SessionImpl.flush(SessionImpl.java:1222)
        at org.hibernate.internal.SessionImpl.managedFlush(SessionImpl.java:425)
        at org.hibernate.engine.transaction.internal.jdbc.JdbcTransaction.beforeTransactionCommit(JdbcTransaction.java:101)
        at org.hibernate.engine.transaction.spi.AbstractTransactionImpl.commit(AbstractTransactionImpl.java:177)
        at org.hibernate.jpa.internal.TransactionImpl.commit(TransactionImpl.java:77)
        at org.springframework.orm.jpa.JpaTransactionManager.doCommit(JpaTransactionManager.java:517)
        at org.springframework.transaction.support.AbstractPlatformTransactionManager.processCommit(AbstractPlatformTransactionManager.java:757)
        at org.springframework.transaction.support.AbstractPlatformTransactionManager.commit(AbstractPlatformTransactionManager.java:726)
        at org.springframework.transaction.interceptor.TransactionAspectSupport.commitTransactionAfterReturning(TransactionAspectSupport.java:521)
        at org.springframework.transaction.interceptor.TransactionAspectSupport.invokeWithinTransaction(TransactionAspectSupport.java:291)
        at org.springframework.transaction.interceptor.TransactionInterceptor.invoke(TransactionInterceptor.java:96)
        at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
        at org.springframework.aop.framework.JdkDynamicAopProxy.invoke(JdkDynamicAopProxy.java:207)
        at com.sun.proxy.$Proxy116.createGroup(Unknown Source)
        at uk.ac.sanger.mig.aker.controllers.GroupController.store(GroupController.java:107)
Community
  • 1
  • 1
Crembo
  • 5,198
  • 3
  • 26
  • 30
  • how did you define you samples element in the Group object. is it a set or a list. – Surendra Poranki Apr 28 '15 at 17:57
  • It's a collection, but I initialise it with a HashSet. – Crembo Apr 28 '15 at 18:51
  • It would help if you posted the stack trace to see where exactly the dreaded exception is being thrown and which entity it is referring to. Have you tried making the `samples` property of type Set? – Jukka Apr 28 '15 at 20:38
  • Also consider not using the generated id in BaseEntity.equals and hashCode, http://stackoverflow.com/questions/1638723/equals-and-hashcode-in-hibernate – Jukka Apr 28 '15 at 20:52
  • It is referring to Group, I'll post the stack trace tomorrow. I'll also try what you suggested, thanks. – Crembo Apr 28 '15 at 20:53
  • @Jukka I have now added a stack trace. – Crembo Apr 29 '15 at 13:59
  • What does the createGroup method look like? – Jukka Apr 29 '15 at 15:24
  • [GroupService#createGroup](https://github.com/wtsi-mig/aker-prototype/blob/master/src/main/java/uk/ac/sanger/mig/aker/services/GroupServiceImpl.java#L44) @Jukka – Crembo Apr 29 '15 at 15:36
  • Try making the samples field of type Set and not using the (generated) id in BaseEntity.equals and hashCode. – Jukka Apr 29 '15 at 16:48
  • @Jukka that does fix the problem. I will give you the bounty if you explain why that is the case. – Crembo Apr 30 '15 at 08:44

3 Answers3

4

In your new code, you are creating a new HashSet that will contain all the Sample elements in allByBarcode collection.

And I think the problem is raised here because those elements are not persisted by the session because they are new objects added to the new HashSet() instance and they're not really saved by the session, it explains the Row was updated or deleted by another transaction (or unsaved-value mapping was incorrect) Exception however the allByBarcode elements are taken from your service so they are persisted and recognized by the session.

And I don't see what's the need of setting a new HashSet instance where the elements should be saved(persisted) by the session (which will throw an Exception because you are trying to save elements with existing identifier) while it's more efficient and more safe to set the already saved elements, so better use:

   group.setSamples(allByBarcode);
cнŝdk
  • 31,391
  • 7
  • 56
  • 78
  • Thing is, if I do `HashSet samples = new HashSet<>(allByBarcode);` and then debug to compare to the original `allByBarcode`, it points to the exactly the same objects. Does Hibernate track the actual collection as well in the session or something? – Crembo Apr 24 '15 at 09:47
  • The fact is that `allByBarcode` and the `new HashSet<>(allByBarcode)` surely has the same objects, but the difference is that the `allByBarcode` elements are **persisted** in the session because they're taken from your service so referring to the session they are objects saved on the database, but the `new HashSet<>(allByBarcode)` elements are the same elements in `allByBarcode` but in reality they are new objects which holds the `allByBarcode` elements and referring to the session they are new objects that are **not persisted** and need to be saved by the session. – cнŝdk Apr 24 '15 at 10:05
  • What's the type of `Samples` in your `Group` class, also is the `name ` the primary key of the `Group` class else what's the primary key here? – cнŝdk Apr 24 '15 at 10:49
  • [Sample](https://github.com/wtsi-mig/aker-prototype/blob/master/src/main/java/uk/ac/sanger/mig/aker/domain/Sample.java), [Group](https://github.com/wtsi-mig/aker-prototype/blob/master/src/main/java/uk/ac/sanger/mig/aker/domain/Group.java), [BaseEntity](https://github.com/wtsi-mig/aker-prototype/blob/master/src/main/java/uk/ac/sanger/mig/aker/domain/BaseEntity.java) <- links – Crembo Apr 24 '15 at 10:51
  • From all the given information all that comes to mind is that the problem is a duplicate object or primary key issue with `HashSet`, because all the other collections that you stated allow it and only `HashSet` doesn't. And using a `HashSet` and altering the objects will caus ea problem becaus the `id` which is the primary key will be also alterated. And I think this is the issue here. – cнŝdk Apr 24 '15 at 11:07
  • I don't think that's right because there are no duplicates... I select the samples myself in the UI. – Crembo Apr 28 '15 at 18:52
1

I think the problem is that you have some duplicates objects in your collection allByBarcode that disappear when you wrap your collection in a HashSet which doesn't allow duplicates. What happens after that is : your repository is trying to save the same object more than once.

0

Since you are already initializing the samples object as a HashSet, can you try doing this instead of initializing it again.

group.getSamples().addAll(allByBarcode);
serenesat
  • 4,611
  • 10
  • 37
  • 53
  • The general rule when using collections is not to refer their concrete classes , can you change the Group Entity to use List and Set, rather than ArrayList and HashSet. – Surendra Poranki May 01 '15 at 12:35