Project

General

Profile

bug #6687

Multiple representations of the same entity merge problem

Added by Andreas Kohlbecker over 1 year ago. Updated over 1 year ago.

Status:
Closed
Priority:
Highest
Category:
cdm-vaadin
Target version:
Start date:
06/02/2017
Due date:
% Done:

100%

Severity:
critical
Found in Version:
Tags:

Description

The eu.etaxonomy.vaadin.mvp.AbstractCdmEditorPresenter<DTO extends CdmBase, V extends ApplicationView<?>> class uses short running sessions. Modified entities must thus be merged into the new Session before they can be saved.
In some cases however the merge fails with an IllegalStateException like the one that is shown below. The below exception was thrown after while merging a BotanicalName instance whereas the Person (id:10) is at the same time the combinationAuthor and the author of the associated nomenclaturalReference :

Caused by: java.lang.IllegalStateException: Multiple representations of the same entity [eu.etaxonomy.cdm.model.agent.Person#10] are being merged. Detached: [Cheng-Long Shaw]; Managed: [Cheng-Long Shaw]
    at org.hibernate.event.internal.EntityCopyNotAllowedObserver.entityCopyDetected(EntityCopyNotAllowedObserver.java:37)
    at org.hibernate.event.internal.MergeContext.put(MergeContext.java:245)
    at org.hibernate.event.internal.DefaultMergeEventListener.entityIsDetached(DefaultMergeEventListener.java:304)
    at org.hibernate.event.internal.DefaultMergeEventListener.onMerge(DefaultMergeEventListener.java:170)
    at org.hibernate.internal.SessionImpl.fireMerge(SessionImpl.java:850)
    at org.hibernate.internal.SessionImpl.merge(SessionImpl.java:832)
    at org.hibernate.engine.spi.CascadingActions$6.cascade(CascadingActions.java:260)

This problem is a known issue of hibernate: Hibernate ORMHHH-9106 - Multiple representations of the same entity cannot be merged using cascade=merge

The chosen solution is to use the ConversationHolder to implement the session-per-conversation pattern in that way that a independent per view conversations exists for the whole lifetime of a eu.etaxonomy.vaadin.mvp.AbstractView. This covers read only view as well as popup editors implementing eu.etaxonomy.vaadin.mvp.AbstractPopupView. By this we completely avoid the merging problem since all instances are guaranteed to be loaded in the same session.


OLD REJECTED SOLUTION DESCTIPTION BELOW


Since Hibernate 4.3.6 there is a hibernate property to work around this problem:

 <prop key="hibernate.event.merge.entity_copy_observer">allow</prop>

When using hibernate.event.merge.entity_copy_observer=allow you must be aware of the risks and possible side effects, please refer to the patagraphs RISKS OF MERGING ENTITY COPIES and RECOMMENDATIONS in the above linked issue.

According to the recommendation given in the hibernate issue we should set hibernate.event.merge.entity_copy_observer=log and add the following lines to the log4j.properties, in development environments (~/.cdmLibrary/log4j.properties)

# Keep this on DEBUG for developing cdm-vaadin applications !!!
#
# logs merging of multiple representations of the same entity, please refer to
# https://dev.e-taxonomy.eu/redmine/issues/6687
# for further details
log4j.logger.org.hibernate.event.internal.EntityCopyAllowedLoggedObserver=DEBUG

Related issues

Related to Edit - bug #7047: Vaadin UI and View scope beans are not always destroyed correctly Closed 11/02/2017
Related to Edit - bug #7046: replace open session per view pattern by DTO strategy Closed 11/02/2017
Related to Edit - bug #7036: correctly release vaadin view resources when the browser windows has been closed Closed 10/25/2017

Associated revisions

Revision 754d57e3 (diff)
Added by Andreas Kohlbecker over 1 year ago

fix #6687 hibernate.event.merge.entity_copy_observer=log to avoid merge problems

Revision 2a3d1ef0 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 attempt to get rid of the hibernate.event.merge.entity_copy_observer=allow - not working yet

Revision c1c5eeb7 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 reverting hibernate.event.merge.entity_copy_observer to default for next release

Revision 262c713c (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 moving all update and delete operations into a dedicated class: CdmStore

Revision 3436c944 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 using conversational transaction for presave and save operation

Revision 1af6dc9f (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 SpecimenTypeDesignationWorkingsetEditor save operation
- implementing saveBean() method
- fixing bug in CdmStore

Revision aaf1bca4 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 fixing bug in ConversationHolder object comparison by == operator and better logging

Revision fe785c1e (diff)
Added by Andreas Kohlbecker over 1 year ago

fix #6687 per view implementation of the 'session-per-conversation' pattern

Revision 9b677cc7 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 ConversationHolder improvements for pre view based conversation in cdm-vaadin\n - improved logging for multi thread environments\n - configurable default flushMode

Revision f1e94220 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 closing ConversationHolder on view exit

Revision 7328ae26 (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 using read-only transactions in non editor view presenters

Revision fffc8abd (diff)
Added by Andreas Kohlbecker over 1 year ago

ref #6687 clearing session in parent view on refresh

History

#1 Updated by Andreas Kohlbecker over 1 year ago

  • Description updated (diff)

#2 Updated by Andreas Kohlbecker over 1 year ago

  • Description updated (diff)

#3 Updated by Andreas Kohlbecker over 1 year ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 50

#4 Updated by Andreas Kohlbecker over 1 year ago

Hi Andreas,

diese Lösung könnte ziemlich riskant sein. Wir haben dieses Problem ja regelmäßig im TaxEditor und es stellt sich eigentlich immer heraus, dass es eine Lösung gibt, die ohne diesen Workaround auskommt. Entscheidend dabei scheint mir zu sein, dass eben von Clientseite immer sichergestellt wird, dass wirklich nur exakt 1 Objekt für die gleiche id in die Session gemerged wird. Wenn das nicht der Fall ist, läuft an anderer Stelle etwas unsauber und man läuft v.a. Gefahr, 2 Objekte mit verschiedenen Zuständen in die gleiche Session zu mergen mit undefinierten Folgen.

Katja und auch Patrick haben sich mit diesem Problem schon viel auseinandergesetzt und eigentlich immer eine Lösung gefunden. Wichtig ist v.a. glaube ich, dass der Client, sofern er außerhalb einer Session arbeitet, ebenfalls dafür sorgt, das Objektidentität besteht, also das, was Hibernate innerhalb einer Session automatisch übernimmt, auch macht. Cherian hat das für den Editor so implementiert. Das sollten andere Clients m.E. schon aus eigenem Interesse auch immer versuchen.

Viele Grüße,
Andreas M.

#5 Updated by Andreas Kohlbecker over 1 year ago

  • Status changed from Resolved to In Progress

Hi Andreas,

I fully agree, this is too risky for a long term solution, but I haven't found a solution to the probelm in cdm-vaadin so far. Therefore I decided to use the workaround committed with cdm-vaadin|754d57e3

Andreas

#6 Updated by Andreas Kohlbecker over 1 year ago

  • Target version changed from Release 4.8 to Release 4.9

#7 Updated by Andreas Kohlbecker over 1 year ago

The solution that has been chosen is to use the ConversationHolder to implement the session-per-conversation pattern in that way that a independant per view conversations exists for the whole lifetime of a eu.etaxonomy.vaadin.mvp.AbstractView. This covers read only view as well as popup editors implementing eu.etaxonomy.vaadin.mvp.AbstractPopupView.

#8 Updated by Andreas Kohlbecker over 1 year ago

  • Description updated (diff)

#9 Updated by Andreas Kohlbecker over 1 year ago

  • Description updated (diff)

#10 Updated by Andreas Kohlbecker over 1 year ago

  • Status changed from In Progress to Resolved

#11 Updated by Andreas Kohlbecker over 1 year ago

  • Status changed from Resolved to Closed
  • % Done changed from 50 to 100

#12 Updated by Andreas Kohlbecker about 1 year ago

  • Related to bug #7047: Vaadin UI and View scope beans are not always destroyed correctly added

#13 Updated by Andreas Kohlbecker about 1 year ago

  • Related to bug #7046: replace open session per view pattern by DTO strategy added

#14 Updated by Andreas Kohlbecker about 1 year ago

  • Related to bug #7036: correctly release vaadin view resources when the browser windows has been closed added

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 40 MB)