Add indirection in View API to allow view type to transform view changes

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/CommittableView.java

@@ -1,9 +1,5 @@
 package tools.vitruv.framework.views;
 
-import java.util.List;
-
-import tools.vitruv.change.composite.description.PropagatedChange;
-
 /**
  * A {@link View} that allows to commit its changes back to the underlying {@link ChangeableViewSource}.
  */
@@ -20,12 +16,11 @@ public interface CommittableView extends View {
 	 * commit, consider using {@link #commitChangesAndUpdate()} instead to perform
 	 * an update of the view afterwards.
 	 * 
-	 * @return the changes resulting from propagating the recorded changes
 	 * @throws IllegalStateException if called on a closed view
 	 * @see #isClosed()
 	 * @see #commitChangesAndUpdate()
 	 */
-	List<PropagatedChange> commitChanges();
+	void commitChanges();
 
 	/**
 	 * Convenience method for subsequent execution of {@link #commitChanges()} and
@@ -34,15 +29,13 @@ public interface CommittableView extends View {
 	 * elements from the {@link ChangeableViewSource} afterwards to reflect
 	 * potential further changes made during commit.
 	 * 
-	 * @return the changes resulting from propagating the recorded changes
 	 * @throws IllegalStateException if called on a closed view
 	 * @see #commitChanges()
 	 * @see #update()
 	 * @see #isClosed()
 	 */
-	default List<PropagatedChange> commitChangesAndUpdate() {
-		List<PropagatedChange> changes = commitChanges();
+	default void commitChangesAndUpdate() {
+		commitChanges();
 		update();
-		return changes;
 	}
 }

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/changederivation/DefaultStateBasedChangeResolutionStrategy.xtend

@@ -93,7 +93,7 @@ class DefaultStateBasedChangeResolutionStrategy implements StateBasedChangeResol
             changeRecorder.beginRecording
             changeRecorder.addToRecording(resource)
             function.apply()
-            return changeRecorder.endRecording
+            return changeRecorder.endRecording.unresolve
         }
     }
 

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/changederivation/StateBasedChangeResolutionStrategy.java

@@ -0,0 +1,53 @@
+package tools.vitruv.framework.views.changederivation;
+
+import org.eclipse.emf.ecore.resource.Resource;
+
+import tools.vitruv.change.composite.description.VitruviusChange;
+
+/**
+ * Strategy for resolving state-based changes to individual change sequences.
+ * This strategy is used by the domains when there are no change sequences
+ * available and changes need to be propagated based on the difference between
+ * the old and new state.
+ */
+public interface StateBasedChangeResolutionStrategy {
+
+	/**
+	 * Resolves the state-based delta of two resources and returns the correlating
+	 * change sequences. Changes must use hierarchical IDs and are returned
+	 * unresolved. The {@code oldState} remains unmodified from calling this method.
+	 * 
+	 * @param newState is the new state of the resource, must not be
+	 *                 <code>null</code> and must not contain proxies.
+	 * @param oldState is the current or old state of the resource, must not be
+	 *                 <code>null</code> and must not contain proxies.
+	 * @return a unresolved {@link VitruviusChange} that contains the individual
+	 *         change sequence.
+	 */
+	VitruviusChange getChangeSequenceBetween(Resource newState, Resource oldState);
+
+	/**
+	 * Resolves the state-based delta for creating the given resource and returns
+	 * the correlating change sequences. Changes must use hierarchical IDs and are
+	 * returned unresolved.
+	 * 
+	 * @param newState is the new state of the resource, must not be
+	 *                 <code>null</code> and must not contain proxies.
+	 * @return a unresolved {@link VitruviusChange} that contains the individual
+	 *         change sequence.
+	 */
+	VitruviusChange getChangeSequenceForCreated(Resource newState);
+
+	/**
+	 * Resolves the state-based delta for deleting the given resource and returns
+	 * the correlating change sequences. Changes must use hierarchical IDs and are
+	 * returned unresolved. The {@code oldState} remains unmodified from calling
+	 * this method.
+	 * 
+	 * @param oldState is the new state of the resource, must not be
+	 *                 <code>null</code> and must not contain proxies.
+	 * @return a unresolved {@link VitruviusChange} that contains the individual
+	 *         change sequence.
+	 */
+	VitruviusChange getChangeSequenceForDeleted(Resource oldState);
+}

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/impl/ChangeDerivingView.xtend

@@ -8,6 +8,7 @@ import org.eclipse.emf.ecore.resource.ResourceSet
 import org.eclipse.emf.ecore.resource.impl.ResourceSetImpl
 import org.eclipse.xtend.lib.annotations.Delegate
 import tools.vitruv.change.composite.description.VitruviusChange
+import tools.vitruv.change.composite.description.VitruviusChangeFactory
 import tools.vitruv.framework.views.CommittableView
 import tools.vitruv.framework.views.View
 import tools.vitruv.framework.views.changederivation.StateBasedChangeResolutionStrategy
@@ -55,17 +56,16 @@ class ChangeDerivingView implements ModifiableView, CommittableView {
 
     override commitChanges() {
         view.checkNotClosed()
-        val propagatedChanges = new ArrayList()
+        val changes = new ArrayList()
         val allResources = new HashSet(originalStateResourceMapping.keySet)
         allResources.addAll(view.viewResourceSet.resources) // consider newly added resources
         for (changedResource : allResources.filter[!URI.isPathmap]) {
             val change = generateChange(changedResource, originalStateResourceMapping.get(changedResource))
-            if (change.containsConcreteChange) {
-                propagatedChanges += viewSource.propagateChange(change)
-            }
+            changes += change
         }
+        val change = VitruviusChangeFactory.instance.createCompositeChange(changes)
+        view.viewType.commitViewChanges(this, change)
         view.viewChanged = false
-        return propagatedChanges
     }
 
     override close() throws Exception {

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/impl/ChangeRecordingView.xtend

@@ -40,10 +40,9 @@ class ChangeRecordingView implements ModifiableView, CommittableView {
     override commitChanges() {
         view.checkNotClosed()
         changeRecorder.endRecording()
-        val propagatedChanges = viewSource.propagateChange(changeRecorder.change)
+        view.viewType.commitViewChanges(this, changeRecorder.change)
         view.viewChanged = false
         changeRecorder.beginRecording()
-        return propagatedChanges
     }
 
     override close() throws Exception {

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/impl/IdentityMappingViewType.xtend

@@ -1,12 +1,14 @@
 package tools.vitruv.framework.views.impl
 
+import tools.vitruv.change.composite.description.VitruviusChange
 import tools.vitruv.framework.views.ChangeableViewSource
 import tools.vitruv.framework.views.View
 import tools.vitruv.framework.views.ViewSource
 import tools.vitruv.framework.views.selectors.DirectViewElementSelector
 import tools.vitruv.framework.views.util.ResourceCopier
 
 import static com.google.common.base.Preconditions.checkArgument
+import static com.google.common.base.Preconditions.checkNotNull
 
 import static extension tools.vitruv.framework.views.util.ResourceCopier.requiresFullCopy
 
@@ -46,4 +48,11 @@ class IdentityMappingViewType extends AbstractViewType<DirectViewElementSelector
 			ResourceCopier.copyViewSourceResources(resourcesWithSelectedElements, viewResourceSet) [selection.isViewObjectSelected(it)]
 		]
 	}
+
+	override commitViewChanges(ModifiableView view, VitruviusChange viewChange) {
+		checkNotNull(view, "view must not be null")
+		checkNotNull(viewChange, "view change must not be null");
+		view.viewSource.propagateChange(viewChange)
+	}
+
 }

bundles/tools.vitruv.framework.views/src/tools/vitruv/framework/views/impl/ViewCreatingViewType.java

@@ -1,7 +1,9 @@
 package tools.vitruv.framework.views.impl;
 
-import tools.vitruv.framework.views.ViewType;
+import tools.vitruv.change.composite.description.VitruviusChange;
+import tools.vitruv.framework.views.ChangeableViewSource;
 import tools.vitruv.framework.views.ViewSelector;
+import tools.vitruv.framework.views.ViewType;
 
 /**
  * A specific view type that is able to create and update views. This is not its
@@ -27,4 +29,17 @@ public interface ViewCreatingViewType<S extends ViewSelector> extends ViewType<S
 	 * @param view is the view to be updated.
 	 */
 	void updateView(ModifiableView view);
+
+	/**
+	 * Commits the changes made to the view and its containing elements to the
+	 * underlying {@link ChangeableViewSource}. Since view elements do not
+	 * necessarily correspond to elements of the underlying view source, the view
+	 * type is responsible for transforming the given {@link VitruviusChange} such
+	 * that the underlying view source can process it. The given changes must use
+	 * hierarchical IDs and may be unresolved.
+	 * 
+	 * @param view       is the modified view.
+	 * @param viewChange are the changes performed to the view.
+	 */
+	void commitViewChanges(ModifiableView view, VitruviusChange viewChange);
 }

tests/tools.vitruv.framework.views.tests/src/tools/vitruv/framework/views/changederivation/BasicStateChangePropagationTest.xtend

@@ -163,7 +163,8 @@ class BasicStateChangePropagationTest extends StateChangePropagationTest {
 
         val validationResourceSet = new ResourceSetImpl().withGlobalFactories()
         val oldState = validationResourceSet.getResource(testUri, true)
-        val changes = strategyToTest.getChangeSequenceBetween(-modelResource, oldState)
+        val unresolvedChanges = strategyToTest.getChangeSequenceBetween(-modelResource, oldState)
+        val changes = unresolvedChanges.resolveAndApply(validationResourceSet)
         switch (strategyToTest.useIdentifiers) {
             case ONLY,
             case WHEN_AVAILABLE: {
@@ -251,7 +252,8 @@ class BasicStateChangePropagationTest extends StateChangePropagationTest {
 
         val validationResourceSet = new ResourceSetImpl().withGlobalFactories()
         val oldState = validationResourceSet.getResource(testUri, true)
-        val changes = strategyToTest.getChangeSequenceBetween(-modelResource, oldState)
+        val unresolvedChanges = strategyToTest.getChangeSequenceBetween(-modelResource, oldState)
+        val changes = unresolvedChanges.resolveAndApply(validationResourceSet)
         switch (strategyToTest.useIdentifiers) {
             case ONLY,
             case WHEN_AVAILABLE: {

tests/tools.vitruv.framework.views.tests/src/tools/vitruv/framework/views/changederivation/StateChangePropagationTest.xtend

@@ -97,8 +97,9 @@ abstract class StateChangePropagationTest {
 	protected def compareChanges(Resource model, Resource checkpoint, StateBasedChangeResolutionStrategy strategyToTest) {
 		model.save(null)
 		val deltaBasedChange = resourceSet.endRecording
-		val stateBasedChange = strategyToTest.getChangeSequenceBetween(model, checkpoint)
-		assertNotNull(stateBasedChange)
+		val unresolvedStateBasedChange = strategyToTest.getChangeSequenceBetween(model, checkpoint)
+		assertNotNull(unresolvedStateBasedChange)
+		val stateBasedChange = unresolvedStateBasedChange.resolveAndApply(checkpoint.resourceSet)
 		val message = getTextualRepresentation(stateBasedChange, deltaBasedChange)
 		val stateBasedChangedObjects = stateBasedChange.affectedAndReferencedEObjects
 		val deltaBasedChangedObjects = deltaBasedChange.affectedAndReferencedEObjects

tests/tools.vitruv.framework.views.tests/src/tools/vitruv/framework/views/impl/ChangeDerivingViewTest.java

@@ -6,7 +6,9 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static tools.vitruv.testutils.matchers.ModelMatchers.equalsDeeply;
@@ -27,12 +29,12 @@
 
 import allElementTypes.NonRoot;
 import allElementTypes.Root;
-import tools.vitruv.change.composite.description.VitruviusChange;
 import tools.vitruv.change.atomic.eobject.EobjectPackage;
 import tools.vitruv.change.atomic.feature.attribute.ReplaceSingleValuedEAttribute;
 import tools.vitruv.change.atomic.root.InsertRootEObject;
 import tools.vitruv.change.atomic.root.RootFactory;
 import tools.vitruv.change.atomic.root.RootPackage;
+import tools.vitruv.change.composite.description.VitruviusChange;
 import tools.vitruv.framework.views.ChangeableViewSource;
 import tools.vitruv.framework.views.ModifiableViewSelection;
 import tools.vitruv.framework.views.changederivation.DefaultStateBasedChangeResolutionStrategy;
@@ -203,18 +205,21 @@ public void commitChanges() throws Exception {
 				String testResourceUriString = "test://test.aet";
 				view.registerRoot(root, URI.createURI(testResourceUriString));
 				assertThat(view.getRootObjects(), hasItem(root));
-				ArgumentCaptor<VitruviusChange> argument = ArgumentCaptor.forClass(VitruviusChange.class);
+				ArgumentCaptor<ChangeDerivingView> viewArgument = ArgumentCaptor.forClass(ChangeDerivingView.class);
+				ArgumentCaptor<VitruviusChange> changeArgument = ArgumentCaptor.forClass(VitruviusChange.class);
 				view.commitChanges();
-				verify(mockChangeableViewSource).propagateChange(argument.capture());
+				verify(mockViewType).commitViewChanges(viewArgument.capture(), changeArgument.capture());
+				assertThat(viewArgument.getValue(), is(view));
+				VitruviusChange change = changeArgument.getValue().resolveAndApply(root.eResource().getResourceSet());
 				InsertRootEObject<EObject> expectedChange = RootFactory.eINSTANCE.createInsertRootEObject();
 				expectedChange.setNewValue(root);
 				expectedChange.setUri(testResourceUriString);
-				assertThat(argument.getValue().getEChanges().size(), is(3)); // Create, Insert, ReplaceId
-				assertThat(argument.getValue().getEChanges().get(1),
+				assertThat(change.getEChanges().size(), is(3)); // Create, Insert, ReplaceId
+				assertThat(change.getEChanges().get(1),
 						equalsDeeply(expectedChange,
 								ignoringFeatures(EobjectPackage.eINSTANCE.getEObjectAddedEChange_NewValueID(),
 										RootPackage.eINSTANCE.getRootEChange_Resource())));
-				assertThat(argument.getValue().getEChanges().get(2), instanceOf(ReplaceSingleValuedEAttribute.class));
+				assertThat(change.getEChanges().get(2), instanceOf(ReplaceSingleValuedEAttribute.class));
 			}
 		}
 	}
@@ -275,12 +280,15 @@ public void commitChanges() throws Exception {
 				String movedResourceUriString = "test://test2.aet";
 				view.registerRoot(root, URI.createURI("test://test.aet"));
 				view.commitChanges();
-				reset(mockChangeableViewSource);
+				reset(mockChangeableViewSource, mockViewType);
 				view.moveRoot(root, URI.createURI(movedResourceUriString));
 				assertThat(view.getRootObjects(), hasItem(root));
+				ArgumentCaptor<ChangeDerivingView> viewArgument = ArgumentCaptor.forClass(ChangeDerivingView.class);
+				ArgumentCaptor<VitruviusChange> changeArgument = ArgumentCaptor.forClass(VitruviusChange.class);
 				view.commitChangesAndUpdate();
-				ArgumentCaptor<VitruviusChange> argument = ArgumentCaptor.forClass(VitruviusChange.class);
-				verify(mockChangeableViewSource).propagateChange(argument.capture());
+				verify(mockViewType).commitViewChanges(viewArgument.capture(), changeArgument.capture());
+				assertThat(viewArgument.getValue(), is(view));
+				assertTrue(changeArgument.getValue().containsConcreteChange(), "change must contain some concrete change");
 				assertThat(view.getRootObjects().size(), is(1));
 				Root updatedRoot = (Root)view.getRootObjects().iterator().next();
 				assertThat(updatedRoot.eResource().getURI(), is(URI.createURI(movedResourceUriString)));
@@ -300,8 +308,8 @@ public void prepareViewWithRootElement() {
 					mockViewSelection), new DefaultStateBasedChangeResolutionStrategy());
 			root = aet.Root();
 			view.registerRoot(root, URI.createURI("test://test.aet"));
-			view.commitChanges();
-			reset(mockChangeableViewSource);
+			view.commitChangesAndUpdate();
+			reset(mockChangeableViewSource, mockViewType);
 		}
 
 		@AfterEach
@@ -315,9 +323,12 @@ public void once() {
 			NonRoot nonRoot = aet.NonRoot();
 			nonRoot.setId("nonRoot");
 			root.setSingleValuedContainmentEReference(nonRoot);
+			ArgumentCaptor<ChangeDerivingView> viewArgument = ArgumentCaptor.forClass(ChangeDerivingView.class);
+			ArgumentCaptor<VitruviusChange> changeArgument = ArgumentCaptor.forClass(VitruviusChange.class);
 			view.commitChangesAndUpdate();
-			ArgumentCaptor<VitruviusChange> argument = ArgumentCaptor.forClass(VitruviusChange.class);
-			verify(mockChangeableViewSource).propagateChange(argument.capture());
+			verify(mockViewType).commitViewChanges(viewArgument.capture(), changeArgument.capture());
+			assertThat(viewArgument.getValue(), is(view));
+			assertTrue(changeArgument.getValue().containsConcreteChange(), "change must contain some concrete change");
 			assertThat(view.getRootObjects().size(), is(1));
 			Root root = (Root)view.getRootObjects().iterator().next();
 			assertThat(root.eContents().size(), is(1));
@@ -331,13 +342,16 @@ public void twice() {
 			firstNonRoot.setId("first");
 			root.setSingleValuedContainmentEReference(firstNonRoot);
 			view.commitChanges();
-			reset(mockChangeableViewSource);
+			reset(mockChangeableViewSource, mockViewType);
 			NonRoot secondNonRoot = aet.NonRoot();
 			secondNonRoot.setId("second");
 			root.setSingleValuedContainmentEReference(secondNonRoot);
+			ArgumentCaptor<ChangeDerivingView> viewArgument = ArgumentCaptor.forClass(ChangeDerivingView.class);
+			ArgumentCaptor<VitruviusChange> changeArgument = ArgumentCaptor.forClass(VitruviusChange.class);
 			view.commitChangesAndUpdate();
-			ArgumentCaptor<VitruviusChange> argument = ArgumentCaptor.forClass(VitruviusChange.class);
-			verify(mockChangeableViewSource).propagateChange(argument.capture());
+			verify(mockViewType).commitViewChanges(viewArgument.capture(), changeArgument.capture());
+			assertThat(viewArgument.getValue(), is(view));
+			assertTrue(changeArgument.getValue().containsConcreteChange(), "change must contain some concrete change");
 			assertThat(view.getRootObjects().size(), is(1));
 			Root root = (Root)view.getRootObjects().iterator().next();
 			assertThat(root.eContents().size(), is(1));
@@ -347,9 +361,12 @@ public void twice() {
 		@Test
 		@DisplayName("without changes")
 		public void withoutChanges() {
+			ArgumentCaptor<ChangeDerivingView> viewArgument = ArgumentCaptor.forClass(ChangeDerivingView.class);
+			ArgumentCaptor<VitruviusChange> changeArgument = ArgumentCaptor.forClass(VitruviusChange.class);
 			view.commitChangesAndUpdate();
-			ArgumentCaptor<VitruviusChange> argument = ArgumentCaptor.forClass(VitruviusChange.class);
-			verify(mockChangeableViewSource).propagateChange(argument.capture());
+			verify(mockViewType).commitViewChanges(viewArgument.capture(), changeArgument.capture());
+			assertThat(viewArgument.getValue(), is(view));
+			assertFalse(changeArgument.getValue().containsConcreteChange(), "change must be empty");
 			assertThat(view.getRootObjects().size(), is(1));
 		}
 	}