From cf9055b6b8e6dd19825ac4dbefc0e1ea2d01e57b Mon Sep 17 00:00:00 2001 From: Michael Leuschel <leuschel@cs.uni-duesseldorf.de> Date: Fri, 2 Nov 2018 12:46:07 +0100 Subject: [PATCH] fix EMF merge problem in Rodin 3.4 when saving Text from bodevix: I have looked at this problem and it seems to be fixed even if the fix is not very clean. I have modified PersistenceHelper to call directly the EventBMerger and to catch potential exceptions when applying a diff: this update avoids exceptions on Save. Finally, no exceptions seem to occur thanks to the EventBMerger update. In EventBMerger, I have used the AttributeChangeMerger and ReferenceChangeMerger of the library. I have also tried to merge the location information. Line information seems correct but sub-expressions are not well marked. I think it would need to find the index of the change in a multi-valued attribute. It seems the isMergedFor method could simply return true. I have kept a part of the previous code. It was supposed to avoid erasing information not managed by camille, but it seems there is nothing to do for that. Another problem I have tried to fix is that saving does not always launch workspace update. Adding resource.setModified(true); resource.eSetDeliver(true); in PersistenceHelper seems to reduce the occurrences of the problem. --- .../eventb/texttools/PersistenceHelper.java | 32 +++++++- .../texttools/diffmerge/EventBMerger.java | 75 +++++++++++++------ 2 files changed, 79 insertions(+), 28 deletions(-) mode change 100644 => 100755 org.eventb.texttools/src/org/eventb/texttools/PersistenceHelper.java diff --git a/org.eventb.texttools/src/org/eventb/texttools/PersistenceHelper.java b/org.eventb.texttools/src/org/eventb/texttools/PersistenceHelper.java old mode 100644 new mode 100755 index 0a6eee0..9423664 --- a/org.eventb.texttools/src/org/eventb/texttools/PersistenceHelper.java +++ b/org.eventb.texttools/src/org/eventb/texttools/PersistenceHelper.java @@ -23,9 +23,11 @@ import org.eclipse.emf.common.util.URI; import org.eclipse.emf.compare.Comparison; import org.eclipse.emf.compare.Diff; import org.eclipse.emf.compare.DifferenceKind; +import org.eclipse.emf.compare.DifferenceState; import org.eclipse.emf.compare.EMFCompare; import org.eclipse.emf.compare.diff.DefaultDiffEngine; import org.eclipse.emf.compare.diff.IDiffEngine; +import org.eclipse.emf.compare.internal.spec.ReferenceChangeSpec; import org.eclipse.emf.compare.match.DefaultComparisonFactory; import org.eclipse.emf.compare.match.DefaultEqualityHelperFactory; import org.eclipse.emf.compare.match.IComparisonFactory; @@ -34,9 +36,11 @@ import org.eclipse.emf.compare.match.eobject.IEObjectMatcher; import org.eclipse.emf.compare.match.impl.MatchEngineFactoryImpl; import org.eclipse.emf.compare.match.impl.MatchEngineFactoryRegistryImpl; import org.eclipse.emf.compare.merge.BatchMerger; +import org.eclipse.emf.compare.merge.ConflictMerger; import org.eclipse.emf.compare.merge.IMerger; import org.eclipse.emf.compare.merge.IMerger.Registry; import org.eclipse.emf.compare.merge.IMerger.RegistryImpl; +import org.eclipse.emf.compare.merge.PseudoConflictMerger; import org.eclipse.emf.compare.scope.DefaultComparisonScope; import org.eclipse.emf.compare.scope.IComparisonScope; import org.eclipse.emf.ecore.EClass; @@ -50,6 +54,7 @@ import org.eventb.emf.core.EventBNamedCommentedComponentElement; import org.eventb.texttools.diffmerge.EventBDiffProcessor; import org.eventb.texttools.diffmerge.EventBEObjectMatcher; import org.eventb.texttools.diffmerge.EventBMerger; +import org.eventb.texttools.diffmerge.EventBMerger; import org.eventb.texttools.prettyprint.PrettyPrinter; import de.be4.eventb.core.parser.BException; @@ -79,7 +84,11 @@ public class PersistenceHelper { final boolean overwrite, final IProgressMonitor monitor) throws CoreException { try { + //System.out.println("SAVING"); resource.save(Collections.EMPTY_MAP); + //resource.unload(); + //resource.load(Collections.EMPTY_MAP); + //System.out.println("SAVING DONE"); /* * Try to set timestamp to the same as in the annotation. Setting on @@ -108,6 +117,7 @@ public class PersistenceHelper { public static void addTextAnnotation(final Resource resource, final String textRepresentation, final long timeStamp) throws CoreException { +//System.out.println("TEXT1:"+textRepresentation); final EventBNamedCommentedComponentElement component = getComponent(resource); if (component != null) { addTextAnnotation(component, textRepresentation, timeStamp); @@ -123,6 +133,7 @@ public class PersistenceHelper { final EMap<String, Attribute> attributes = element.getAttributes(); // update text representation +//System.out.println("TEXT2:"+textRepresentation); Attribute textAttribute = attributes .get(TextToolsPlugin.TYPE_TEXTREPRESENTATION.getId()); if (textAttribute == null) { @@ -160,6 +171,15 @@ public class PersistenceHelper { } + private static void applyDiff(EventBNamedCommentedComponentElement root,IMerger evbMerger,Diff d) { + try { + if (d.getState() != DifferenceState.MERGED) + evbMerger.copyRightToLeft(d,null); + } catch(Exception e) { + System.out.println("SKIP:"+d); + } + } + private static void mergeComponents( final EventBNamedCommentedComponentElement oldVersion, final EventBNamedCommentedComponentElement newVersion, @@ -209,13 +229,15 @@ public class PersistenceHelper { IMerger evbMerger = new EventBMerger(); evbMerger.setRanking(100); registry.add(evbMerger); - BatchMerger bm = new BatchMerger(registry); - + //BatchMerger bm = new BatchMerger(registry); differences = filter(differences); - bm.copyAllRightToLeft(differences, null); - + //bm.copyAllRightToLeft(differences, null); + + for (Diff d: differences) applyDiff(oldVersion,evbMerger,d); + long time2 = System.currentTimeMillis(); + if (DEBUG) { System.out.println("new ModelMerge: " + (time1 - time0)); System.out.println("merge.applyChanges: " + (time2 - time1)); @@ -247,6 +269,8 @@ public class PersistenceHelper { long start = System.currentTimeMillis(); mergeComponents(component, newVersion, monitor); long end = System.currentTimeMillis(); + resource.setModified(true); + resource.eSetDeliver(true); if (DEBUG) { System.out .println("Time to merge components: " + (end - start)); diff --git a/org.eventb.texttools/src/org/eventb/texttools/diffmerge/EventBMerger.java b/org.eventb.texttools/src/org/eventb/texttools/diffmerge/EventBMerger.java index 7a016d5..39774f8 100644 --- a/org.eventb.texttools/src/org/eventb/texttools/diffmerge/EventBMerger.java +++ b/org.eventb.texttools/src/org/eventb/texttools/diffmerge/EventBMerger.java @@ -1,18 +1,23 @@ package org.eventb.texttools.diffmerge; +import org.eclipse.emf.compare.AttributeChange; import org.eclipse.emf.compare.Diff; import org.eclipse.emf.compare.ReferenceChange; import org.eclipse.emf.compare.merge.AbstractMerger; +import org.eclipse.emf.compare.merge.AttributeChangeMerger; +import org.eclipse.emf.compare.merge.ReferenceChangeMerger; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.EReference; -import org.eventb.emf.core.context.Context; -import org.eventb.emf.core.machine.Event; -import org.eventb.emf.core.machine.Machine; +import org.eventb.emf.core.EventBObject; +import org.eventb.texttools.TextPositionUtil; public class EventBMerger extends AbstractMerger { + private AttributeChangeMerger am = new AttributeChangeMerger(); + private ReferenceChangeMerger rm = new ReferenceChangeMerger(); + @Override public boolean isMergerFor(Diff target) { - // System.out.println(target); + //FIXME: could return true in all cases??? if (target instanceof ReferenceChange) { ReferenceChange rtarget = (ReferenceChange) target; EReference reference = rtarget.getReference(); @@ -21,11 +26,20 @@ public class EventBMerger extends AbstractMerger { || reference.getName().equals("sees")) { return true; } - } - return false; // currently disabled + //FIXME: find a way to avoid naming all core MM attributes + //but test seems useless ... => always returns true? + if (reference.getName().contains("extension")) { + System.out.println("REFERENCE:"+reference.getName()); + return false; + } + return true; + } else if (target instanceof AttributeChange) + return true; + System.out.println("TARGET:"+target); + return false; } - @Override + //@Override protected void accept(final Diff diff, boolean rightToLeft) { // Maybe we do not need this method for Camille? throw new UnsupportedOperationException(); @@ -38,27 +52,40 @@ public class EventBMerger extends AbstractMerger { EObject left = diff.getMatch().getLeft(); EObject right = diff.getMatch().getRight(); - if (left instanceof Context) { - Context leftC = (Context) left; - Context rightC = (Context) right; - leftC.getExtends().clear(); - leftC.getExtends().addAll(rightC.getExtends()); + + if (left == null) { + super.reject(diff, rightToLeft); + return; } +//TODO: find insertion index to get more precise annotations when multiplicity is > 1 + if (left instanceof EventBObject) + TextPositionUtil.annotatePosition((EventBObject)left, TextPositionUtil.getTextRange((EventBObject)right)); - if (left instanceof Machine) { - Machine leftC = (Machine) left; - Machine rightC = (Machine) right; - leftC.getRefines().clear(); - leftC.getRefines().addAll(rightC.getRefines()); - leftC.getSees().clear(); - leftC.getSees().addAll(rightC.getSees()); + if (rm.isMergerFor(diff)) { + ReferenceChange d = (ReferenceChange) diff; + //System.out.println("REF CHANGE:"+d.getReference()); + //System.out.println("REF VALUE:"+ d.getValue()); + rm.copyRightToLeft(diff,null); + Object l = left.eGet(d.getReference()); + Object r = right.eGet(d.getReference()); + if (l instanceof EventBObject && r instanceof EventBObject) + TextPositionUtil.annotatePosition((EventBObject)l, TextPositionUtil.getTextRange((EventBObject)r)); + return; } - if (left instanceof Event) { - Event leftC = (Event) left; - Event rightC = (Event) right; - leftC.getRefines().clear(); - leftC.getRefines().addAll(rightC.getRefines()); + if (am.isMergerFor(diff)) { + AttributeChange d = (AttributeChange) diff; + //System.out.println("ATTR CHANGE:"+d.getAttribute()); + //System.out.println("ATTR VALUE:"+ d.getValue()); + am.copyRightToLeft(diff,null); + Object l = left.eGet(d.getAttribute()); + Object r = right.eGet(d.getAttribute()); + if (l instanceof EventBObject && r instanceof EventBObject) + TextPositionUtil.annotatePosition((EventBObject)l, TextPositionUtil.getTextRange((EventBObject)r)); + return; } + + System.out.println("DIFF:"+diff.getClass()+" LEFT:"+left+ " - RIGHT:"+right); + } } -- GitLab