From 30dbee10b2ccf3f4725ed05521a6d11c02286364 Mon Sep 17 00:00:00 2001 From: vlorenzo Date: Mon, 30 Apr 2012 15:18:39 +0000 Subject: 342163: [Usability] Papyrus merge should use the service edit of Papyrus https://bugs.eclipse.org/bugs/show_bug.cgi?id=342163 A save of my work : Corrects some errors for MoveModelElementMerger --- .../compare/merger/utils/EObjectComparator.java | 66 +++++++++++ .../compare/merger/utils/MoveWithIndexCommand.java | 30 ++++- .../compare/merger/utils/MoveWithIndexRequest.java | 13 +- .../uml/compare/merger/utils/PapyrusEFactory.java | 132 +++++++++------------ .../uml/compare/merger/utils/PositionAdapter.java | 48 ++++++++ .../merger/provider/CMoveModelElementMerger.java | 14 +-- .../provider/PapyrusMergeCommandProvider.java | 4 +- 7 files changed, 212 insertions(+), 95 deletions(-) create mode 100644 sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/EObjectComparator.java create mode 100644 sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/PositionAdapter.java diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/EObjectComparator.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/EObjectComparator.java new file mode 100644 index 00000000000..cde7cd75373 --- /dev/null +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/EObjectComparator.java @@ -0,0 +1,66 @@ +/***************************************************************************** + * Copyright (c) 2012 CEA LIST. + * + * + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + * + * Contributors: + * Vincent Lorenzo (CEA LIST) Vincent.Lorenzo@cea.fr - Initial API and implementation + * + *****************************************************************************/ +package org.eclipse.papyrus.uml.compare.merger.utils; + +import java.util.Comparator; +import java.util.Iterator; + +import org.eclipse.emf.common.notify.Adapter; +import org.eclipse.emf.ecore.EObject; + +/** + * + * This class allows to compare EObject using the PositionAdapter. + * + * + */ +public class EObjectComparator implements Comparator { + + /** + * + * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object) + * + * @param o1 + * @param o2 + * @return + */ + public int compare(final T o1, final T o2) { + if(o1 instanceof EObject && o2 instanceof EObject) { + final int position1 = getWantedPosition((EObject)o1); + final int position2 = getWantedPosition((EObject)o2); + return position1 - position2; + } + return 0; + } + + /** + * + * @param obj1 + * an EObject + * @return + * the wanted position for this object + */ + private int getWantedPosition(final EObject obj1) { + final Iterator adapters = obj1.eAdapters().iterator(); + int expectedIndex = -1; + while(expectedIndex == -1 && adapters.hasNext()) { + final Adapter adapter = adapters.next(); + if(adapter instanceof PositionAdapter) { + expectedIndex = ((PositionAdapter)adapter).getExpectedIndex(); + } + } + return expectedIndex; + } + +} diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexCommand.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexCommand.java index 4fecc11da0d..833b4b9f6be 100644 --- a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexCommand.java +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexCommand.java @@ -23,7 +23,6 @@ import org.eclipse.core.runtime.IProgressMonitor; import org.eclipse.emf.ecore.EObject; import org.eclipse.emf.ecore.EReference; import org.eclipse.emf.ecore.util.FeatureMapUtil; -import org.eclipse.emf.edit.command.MoveCommand; import org.eclipse.gmf.runtime.common.core.command.CommandResult; import org.eclipse.gmf.runtime.emf.type.core.commands.MoveElementsCommand; import org.eclipse.gmf.runtime.emf.type.core.internal.l10n.EMFTypeCoreMessages; @@ -33,17 +32,20 @@ import org.eclipse.gmf.runtime.emf.type.core.requests.MoveRequest; //TODO move this class and create it in the service edit public class MoveWithIndexCommand extends MoveElementsCommand { - public MoveWithIndexCommand(MoveRequest request) { + public MoveWithIndexCommand(final MoveRequest request) { super(request); } - protected CommandResult doExecuteWithResult(IProgressMonitor monitor, IAdaptable info) throws ExecutionException { + //TODO : use the method reorder and attachrealposition + @Override + protected CommandResult doExecuteWithResult(final IProgressMonitor monitor, final IAdaptable info) throws ExecutionException { int index = getIndex(); if(index != -1) { for(Iterator i = getElementsToMove().keySet().iterator(); i.hasNext();) { EObject element = (EObject)i.next(); EReference feature = getTargetFeature(element); - + //we attach the real position to the object + PapyrusEFactory.attachRealPositionEAdapter(element, index); if(feature != null) { if(FeatureMapUtil.isMany(getTargetContainer(), feature)) { Object value = getTargetContainer().eGet(feature); @@ -52,13 +54,23 @@ public class MoveWithIndexCommand extends MoveElementsCommand { int indexMax = listValue.size() - 1; if(indexMax < index) { //we add the element at the end of the list - ((Collection)getTargetContainer().eGet(feature)).add(element); + List values = ((List)getTargetContainer().eGet(feature)); + values.add(element); + if(shouldReorder()) { + PapyrusEFactory.reorderList(values); + } } else { ((List)value).add(index, element); + if(shouldReorder()) { + PapyrusEFactory.reorderList((List)value); + } } } else { ((Collection)getTargetContainer().eGet(feature)).add(element); + if(shouldReorder()) { + PapyrusEFactory.reorderList((List)((Collection)getTargetContainer().eGet(feature))); + } } } else { getTargetContainer().eSet(feature, element); @@ -84,4 +96,12 @@ public class MoveWithIndexCommand extends MoveElementsCommand { } return -1; } + + protected boolean shouldReorder() { + IEditCommandRequest req = getRequest(); + if(req instanceof MoveWithIndexRequest) { + return ((MoveWithIndexRequest)req).shouldReoder(); + } + return false; + } } diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexRequest.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexRequest.java index 3c613c374ed..718fc0020c0 100644 --- a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexRequest.java +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/MoveWithIndexRequest.java @@ -26,17 +26,24 @@ import org.eclipse.gmf.runtime.emf.type.core.requests.MoveRequest; */ public class MoveWithIndexRequest extends MoveRequest { - private int index; + private final int index; + + private final boolean reorder; //TODO : and the other constructor? - public MoveWithIndexRequest(TransactionalEditingDomain editingDomain, EObject targetContainer, EReference targetFeature, EObject elementToMove, final int index) { + public MoveWithIndexRequest(final TransactionalEditingDomain editingDomain, final EObject targetContainer, final EReference targetFeature, final EObject elementToMove, final int index, final boolean reorder) { super(editingDomain, targetContainer, targetFeature, elementToMove); this.index = index; + this.reorder = reorder; } public int getIndex() { - return index; + return this.index; + } + + public boolean shouldReoder() { + return this.reorder; } } diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/PapyrusEFactory.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/PapyrusEFactory.java index 263c0270401..16a6dd1591b 100644 --- a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/PapyrusEFactory.java +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/compare/merger/utils/PapyrusEFactory.java @@ -32,7 +32,6 @@ import org.eclipse.emf.transaction.TransactionalEditingDomain; import org.eclipse.osgi.util.NLS; import org.eclipse.papyrus.uml.compare.merger.Activator; import org.eclipse.papyrus.uml.merger.provider.PapyrusMergeCommandProvider; - //TODO : merge with thepapyrus table command factory? /** * @@ -175,7 +174,57 @@ public class PapyrusEFactory { + // /** + // * If we could not merge a given object at its expected position in a list, we'll attach an Adapter to it + // * in order to "remember" that "expected" position. That will allow us to reorder the list later on if + // * need be. + // * + // * @param object + // * The object on which to attach an Adapter. + // * @param expectedPosition + // * The expected position of object in its list. + // */ + // public static void attachRealPositionEAdapter(final Object object, final int expectedPosition) { + // Class myClass = null; + // try { + // myClass = Class.forName("org.eclipse.emf.compare.util.EFactory"); + // } catch (ClassNotFoundException e2) { + // // TODO Auto-generated catch block + // e2.printStackTrace(); + // } + // Class[] parameterTypes = new Class[2]; + // parameterTypes[0] = java.lang.Object.class; + // parameterTypes[1] = Integer.TYPE; + // Method m = null; + // + // try { + // m = myClass.getDeclaredMethod("attachRealPositionEAdapter", parameterTypes); + // } catch (SecurityException e1) { + // // TODO Auto-generated catch block + // e1.printStackTrace(); + // } catch (NoSuchMethodException e1) { + // // TODO Auto-generated catch block + // e1.printStackTrace(); + // } + // m.setAccessible(true); + // Object[] parameters = new Object[2]; + // parameters[0] = object; + // parameters[1] = expectedPosition; + // + // Object result = null; + // try { + // result = m.invoke(myClass, parameters); + // } catch (IllegalArgumentException e) { + // Activator.log.error(e); + // } catch (IllegalAccessException e) { + // Activator.log.error(e); + // } catch (InvocationTargetException e) { + // Activator.log.error(e); + // } + // } + /** + * Duplicate code from EFactory * If we could not merge a given object at its expected position in a list, we'll attach an Adapter to it * in order to "remember" that "expected" position. That will allow us to reorder the list later on if * need be. @@ -186,45 +235,11 @@ public class PapyrusEFactory { * The expected position of object in its list. */ public static void attachRealPositionEAdapter(final Object object, final int expectedPosition) { - Class myClass = null; - try { - myClass = Class.forName("org.eclipse.emf.compare.util.EFactory"); - } catch (ClassNotFoundException e2) { - // TODO Auto-generated catch block - e2.printStackTrace(); - } - Class[] parameterTypes = new Class[2]; - parameterTypes[0] = java.lang.Object.class; - parameterTypes[1] = Integer.TYPE; - Method m = null; - - try { - m = myClass.getDeclaredMethod("attachRealPositionEAdapter", parameterTypes); - } catch (SecurityException e1) { - // TODO Auto-generated catch block - e1.printStackTrace(); - } catch (NoSuchMethodException e1) { - // TODO Auto-generated catch block - e1.printStackTrace(); - } - m.setAccessible(true); - Object[] parameters = new Object[2]; - parameters[0] = object; - parameters[1] = expectedPosition; - - Object result = null; - try { - result = m.invoke(myClass, parameters); - } catch (IllegalArgumentException e) { - Activator.log.error(e); - } catch (IllegalAccessException e) { - Activator.log.error(e); - } catch (InvocationTargetException e) { - Activator.log.error(e); + if(object instanceof EObject) { + ((EObject)object).eAdapters().add(new PositionAdapter(expectedPosition)); } } - /** * Reorders the given list if it contains EObjects associated with a PositionAdapter which are not located * at their expected positions. @@ -235,45 +250,16 @@ public class PapyrusEFactory { * type of the list's elements. */ public static void reorderList(final List list) { - Class myClass = null; - try { - myClass = Class.forName("org.eclipse.emf.compare.util.EFactory"); - } catch (ClassNotFoundException e2) { - // TODO Auto-generated catch block - e2.printStackTrace(); + List newList = new ArrayList(list); + Collections.sort(newList, new EObjectComparator()); + for(int i=0;iLaurent Goubet + */ +public class PositionAdapter extends AdapterImpl { + + /** The index at which we expect to find this object. */ + private final int expectedIndex; + + /** + * Creates our adapter. + * + * @param index + * The index at which we expect to find this object. + */ + public PositionAdapter(final int index) { + this.expectedIndex = index; + } + + /** + * Returns the index at which we expect to find this object. + * + * @return The index at which we expect to find this object. + */ + public int getExpectedIndex() { + return expectedIndex; + } +} diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/CMoveModelElementMerger.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/CMoveModelElementMerger.java index a3454ca68b4..f0bceedd276 100644 --- a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/CMoveModelElementMerger.java +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/CMoveModelElementMerger.java @@ -118,15 +118,10 @@ public class CMoveModelElementMerger extends MoveModelElementMerger implements I final String elementID = getXMIID(leftElement); if(rightRefValue instanceof List) { - cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveWithIndexCommand(domain, leftTarget, leftTarget, ref, leftElement, index)); + cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveWithIndexCommand(domain, leftTarget, leftTarget, ref, leftElement, index, true)); } else { cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveCommand(domain, leftTarget, leftTarget, ref, leftElement)); } - //TODO : verify this merge action : I replace remove and eAdd by a move : it seems coorect, but it should be more tested - // EcoreUtil.remove(leftElement); - // EFactory.eAdd(leftTarget, ref.getName(), leftElement, index, true); - // // Sets anew the element's ID - // setXMIID(leftElement, elementID); cmd.append(PapyrusMergeCommandProvider.INSTANCE.getSetXMIIDCommand(domain, leftElement, elementID)); } else { @@ -152,16 +147,11 @@ public class CMoveModelElementMerger extends MoveModelElementMerger implements I index = refLeftValueList.indexOf(theDiff.getLeftElement()); } if(leftRefValue instanceof List) { - cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveWithIndexCommand(domain, rightTarget, rightTarget, ref, rightElement, index)); + cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveWithIndexCommand(domain, rightTarget, rightTarget, ref, rightElement, index, true)); } else { cmd.append(PapyrusMergeCommandProvider.INSTANCE.getMoveCommand(domain, rightTarget, rightTarget, ref, rightElement)); } final String elementID = getXMIID(rightElement); - //TODO : verify this merge action : I replace remove and eAdd by a move : it seems coorect, but it should be more tested - // EcoreUtil.remove(rightElement); - // EFactory.eAdd(rightTarget, ref.getName(), rightElement, index, true); - // setXMIID(rightElement, elementID); - cmd.append(PapyrusMergeCommandProvider.INSTANCE.getSetXMIIDCommand(domain, rightElement, elementID)); } else { // shouldn't be here diff --git a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/PapyrusMergeCommandProvider.java b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/PapyrusMergeCommandProvider.java index 4f726dbb340..036c79f5bcf 100644 --- a/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/PapyrusMergeCommandProvider.java +++ b/sandbox/UMLCompareMergerExample/org.eclipse.papyrus.uml.compare.merger/src/org/eclipse/papyrus/uml/merger/provider/PapyrusMergeCommandProvider.java @@ -105,8 +105,8 @@ public class PapyrusMergeCommandProvider { } //TODO elementToEdit and targetContainer are the same - public Command getMoveWithIndexCommand(final TransactionalEditingDomain domain, final EObject elementToEdit, final EObject targetContainer, final EReference targetFeature, final EObject elementToMove, final int index) { - final IEditCommandRequest request = new MoveWithIndexRequest(domain, targetContainer, targetFeature, elementToMove, index); + public Command getMoveWithIndexCommand(final TransactionalEditingDomain domain, final EObject elementToEdit, final EObject targetContainer, final EReference targetFeature, final EObject elementToMove, final int index, final boolean reorder) { + final IEditCommandRequest request = new MoveWithIndexRequest(domain, targetContainer, targetFeature, elementToMove, index, reorder); return getCommand(elementToEdit, request); } -- cgit v1.2.3