[Fix] Fixed issue with deleteTraces() not removing trace links

The previous implementation of GenericMetaModelAdapter.deleteTraces()
compared the object references of the traces to delete with the traces
in the trace model. This meant that in many situations, traces that
should have been deleted were not removed. Now, EcoreUtils.equals() is
used instead to compare the trace links structurally. In addition,
TestCreateTraceOperation has been extended and now creates two links, of
which one operation is undone. The test then checks for the absence of
the one whose creation was undone and the presence of the other.

Change-Id: Idf6553a08cda4ef46c2c32d2c98bf999f1a72ab2

2 files changed, 59 insertions, 29 deletions

diff --git a/bundles/org.eclipse.capra.generic.tracemodel/src/org/eclipse/capra/generic/tracemodel/GenericMetaModelAdapter.java b/bundles/org.eclipse.capra.generic.tracemodel/src/org/eclipse/capra/generic/tracemodel/GenericMetaModelAdapter.java
index 43d9b73d..e4ebdd5e 100644
--- a/bundles/org.eclipse.capra.generic.tracemodel/src/org/eclipse/capra/generic/tracemodel/GenericMetaModelAdapter.java
+++ b/bundles/org.eclipse.capra.generic.tracemodel/src/org/eclipse/capra/generic/tracemodel/GenericMetaModelAdapter.java
@@ -13,7 +13,6 @@
  *******************************************************************************/
 package org.eclipse.capra.generic.tracemodel;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -24,12 +23,8 @@ import org.eclipse.capra.core.adapters.TraceMetaModelAdapter;
 import org.eclipse.capra.core.adapters.TracePersistenceAdapter;
 import org.eclipse.capra.core.helpers.ArtifactHelper;
 import org.eclipse.capra.core.helpers.ExtensionPointHelper;
-import org.eclipse.emf.common.util.EList;
-import org.eclipse.emf.common.util.URI;
 import org.eclipse.emf.ecore.EClass;
 import org.eclipse.emf.ecore.EObject;
-import org.eclipse.emf.ecore.resource.Resource;
-import org.eclipse.emf.ecore.resource.ResourceSet;
 import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl;
 import org.eclipse.emf.ecore.util.EcoreUtil;
 import org.slf4j.Logger;
@@ -39,7 +34,7 @@ import org.slf4j.LoggerFactory;
  * Provides generic functionality to deal with traceability meta models.
  */
 public class GenericMetaModelAdapter extends AbstractMetaModelAdapter implements TraceMetaModelAdapter {
-	
+
 	private static final Logger LOG = LoggerFactory.getLogger(GenericMetaModelAdapter.class);
 
 	private static final int DEFAULT_INITIAL_TRANSITIVITY_DEPTH = 1;
@@ -129,8 +124,8 @@ public class GenericMetaModelAdapter extends AbstractMetaModelAdapter implements
 		List<Connection> connections = new ArrayList<>();
 		List<RelatedTo> traces = root.getTraces();
 
-		if (selectedRelationshipTypes.size() == 0 || selectedRelationshipTypes
-				.contains(TracemodelPackage.eINSTANCE.getRelatedTo().getName())) {
+		if (selectedRelationshipTypes.size() == 0
+				|| selectedRelationshipTypes.contains(TracemodelPackage.eINSTANCE.getRelatedTo().getName())) {
 			if (element instanceof RelatedTo) {
 				RelatedTo trace = (RelatedTo) element;
 				connections.add(new Connection(element, trace.getItem(), trace));
@@ -169,7 +164,8 @@ public class GenericMetaModelAdapter extends AbstractMetaModelAdapter implements
 	@Override
 	public List<Connection> getTransitivelyConnectedElements(EObject element, EObject traceModel, int maximumDepth) {
 		List<Object> accumulator = new ArrayList<>();
-		return getTransitivelyConnectedElements(element, traceModel, accumulator, DEFAULT_INITIAL_TRANSITIVITY_DEPTH, maximumDepth);
+		return getTransitivelyConnectedElements(element, traceModel, accumulator, DEFAULT_INITIAL_TRANSITIVITY_DEPTH,
+				maximumDepth);
 	}
 
 	@Override
@@ -189,25 +185,22 @@ public class GenericMetaModelAdapter extends AbstractMetaModelAdapter implements
 
 	@Override
 	public void deleteTrace(List<Connection> toDelete, EObject traceModel) {
+		List<Object> toRemove = new ArrayList<>();
 		if (traceModel instanceof GenericTraceModel) {
 			GenericTraceModel tModel = (GenericTraceModel) traceModel;
-			EList<RelatedTo> links = tModel.getTraces();
-			ResourceSet resourceSet = new ResourceSetImpl();
 			for (Connection c : toDelete) {
-				links.remove(c.getTlink());
+				for (RelatedTo trace : tModel.getTraces()) {
+					if (EcoreUtil.equals(trace, c.getTlink())) {
+						toRemove.add(trace);
+					}
+				}
 			}
-			GenericTraceModel newTraceModel = TracemodelFactory.eINSTANCE.createGenericTraceModel();
-			newTraceModel.getTraces().addAll(links);
-			URI traceModelURI = EcoreUtil.getURI(traceModel);
-			Resource resourceForTraces = resourceSet.createResource(traceModelURI);
-			resourceForTraces.getContents().add(newTraceModel);
-	
-			try {
-				resourceForTraces.save(null);
-                          	// TODO: Think of a way to let the developer handle such sitations (e.g., via an Exception)
-			} catch (IOException e) {
-				LOG.error("Trace model could not be saved.", e);
+			for (Object trace : toRemove) {
+				tModel.getTraces().remove(trace);
 			}
+			
+			TracePersistenceAdapter persistenceAdapter = ExtensionPointHelper.getTracePersistenceAdapter().get();
+			persistenceAdapter.saveTracesAndArtifacts(tModel, persistenceAdapter.getArtifactWrappers(new ResourceSetImpl()));
 		}
 	}
 
@@ -215,8 +208,8 @@ public class GenericMetaModelAdapter extends AbstractMetaModelAdapter implements
 	public List<Connection> getTransitivelyConnectedElements(EObject element, EObject traceModel,
 			List<String> selectedRelationshipTypes, int maximumDepth) {
 		List<Object> accumulator = new ArrayList<>();
-		return getTransitivelyConnectedElements(element, traceModel, accumulator, selectedRelationshipTypes, DEFAULT_INITIAL_TRANSITIVITY_DEPTH,
-				maximumDepth);
+		return getTransitivelyConnectedElements(element, traceModel, accumulator, selectedRelationshipTypes,
+				DEFAULT_INITIAL_TRANSITIVITY_DEPTH, maximumDepth);
 	}
 
 	private List<Connection> getTransitivelyConnectedElements(EObject element, EObject traceModel,
diff --git a/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestCreateTraceOperation.java b/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestCreateTraceOperation.java
index 9d1800fe..bbfc3aa4 100644
--- a/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestCreateTraceOperation.java
+++ b/tests/org.eclipse.capra.testsuite/src/org/eclipse/capra/testsuite/TestCreateTraceOperation.java
@@ -28,6 +28,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 
@@ -58,6 +59,7 @@ public class TestCreateTraceOperation {
 
 	private static final String CLASS_A_NAME = "A";
 	private static final String CLASS_B_NAME = "B";
+	private static final String CLASS_C_NAME = "C";
 
 	private static final String MODEL_A_FILENAME = "modelA.ecore";
 	private static final String MODEL_B_FILENAME = "modelB.ecore";
@@ -86,6 +88,7 @@ public class TestCreateTraceOperation {
 
 		EPackage b = createEcoreModel(MODEL_B_NAME);
 		createEClassInEPackage(b, CLASS_B_NAME);
+		createEClassInEPackage(b, CLASS_C_NAME);
 		save(testProject, b);
 
 		// Load them, choose two elements
@@ -98,6 +101,7 @@ public class TestCreateTraceOperation {
 		EPackage _b = load(testProject, MODEL_B_FILENAME, rs);
 		assertEquals(_b.getName(), MODEL_B_NAME);
 		EClass _B = (EClass) _b.getEClassifier(CLASS_B_NAME);
+		EClass _C = (EClass) _b.getEClassifier(CLASS_C_NAME);
 
 		// Add them to the selection view
 		SelectionView.getOpenedView().dropToSelection(_A);
@@ -131,19 +135,52 @@ public class TestCreateTraceOperation {
 		ArtifactHelper artifactHelper = new ArtifactHelper(artifactModel);
 		List<EObject> selection = artifactHelper.createWrappers(SelectionView.getOpenedView().getSelection());
 
-		// Check that the trace exists
+		// Check that the trace between A and B exists
+		traceHelper = new TraceHelper(persistenceAdapter.getTraceModel(new ResourceSetImpl()));
+		selection = artifactHelper.createWrappers(SelectionView.getOpenedView().getSelection());
+		assertTrue(traceHelper.traceExists(selection, traceType));
+
+		// Remove both elements from SelectionView
+		SelectionView.getOpenedView().clearSelection();
+
+		// Create link between A and C
+		// Add them to the selection view
+		SelectionView.getOpenedView().dropToSelection(_A);
+		SelectionView.getOpenedView().dropToSelection(_C);
+
+		CreateTraceOperation createTraceOperation2 = new CreateTraceOperation("Create trace link",
+				SelectionView.getOpenedView().getSelection());
+		createTraceOperation2.setChooseTraceType((traceTypes, sel) -> {
+			if (traceTypes.contains(traceType)) {
+				return Optional.of(traceType);
+			} else {
+				return Optional.empty();
+			}
+		});
+		try {
+			assertEquals(operationHistory.execute(createTraceOperation2, null, adapter), Status.OK_STATUS);
+		} catch (ExecutionException e) {
+			fail("Could not create trace: ExecutionException in operation");
+		}
+
+		// Check that the trace between A and C exists
+		traceHelper = new TraceHelper(persistenceAdapter.getTraceModel(new ResourceSetImpl()));
 		selection = artifactHelper.createWrappers(SelectionView.getOpenedView().getSelection());
 		assertTrue(traceHelper.traceExists(selection, traceType));
 
+		// Undo the second trace creation
 		try {
-			assertEquals(operationHistory.undoOperation(createTraceOperation, null, adapter), Status.OK_STATUS);
+			assertEquals(operationHistory.undoOperation(createTraceOperation2, null, adapter), Status.OK_STATUS);
 		} catch (ExecutionException e) {
 			fail("Could not undo trace creation: ExecutionException in operation");
 		}
 
-		// Check that the trace does not exist
+		// Check that the trace between A and C does not exist
 		traceHelper = new TraceHelper(persistenceAdapter.getTraceModel(new ResourceSetImpl()));
-		selection = artifactHelper.createWrappers(SelectionView.getOpenedView().getSelection());
 		assertFalse(traceHelper.traceExists(selection, traceType));
+
+		// Check that the trace between A and B is still around
+		selection = artifactHelper.createWrappers(Arrays.asList(_A, _B));
+		assertTrue(traceHelper.traceExists(selection, traceType));
 	}
 }