Force UUID unregistration at end of transaction

bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/AtomicEChangeUuidResolver.java

@@ -73,9 +73,11 @@ public EChange<Uuid> assignIds(EChange<EObject> resolvedEChange) {
 	}
 
 	/**
-	 * Ends a transactions such that all {@link EObject}s not being contained in a
-	 * resource, which is contained in a resource set, are removed from the UUID
-	 * mapping.
+	 * Ends a transactions such that any registered {@link EObject} not being
+	 * contained in a resource throws an error.
+	 * 
+	 * @throws IllegalStateException if an uncontained element is registered in the
+	 *                               {@link UuidResolver}.
 	 */
 	public void endTransaction() {
 		uuidResolver.endTransaction();

bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolver.java

@@ -91,7 +91,7 @@ public default Uuid registerEObject(EObject eObject) throws IllegalStateExceptio
 		registerEObject(uuid, eObject);
 		return uuid;
 	}
-	
+
 	public void unregisterEObject(Uuid uuid, EObject eObject) throws IllegalStateException;
 
 	/**
@@ -101,11 +101,12 @@ public default Uuid registerEObject(EObject eObject) throws IllegalStateExceptio
 	public Resource getResource(URI uri);
 
 	/**
-	 * Ends a transactions such that all {@link EObject}s not being contained in a
-	 * resource, which is contained in a resource set, are removed from the UUID
-	 * mapping.
+	 * Ends a transactions such that any registered {@link EObject} not being
+	 * contained in a resource throws an error.
+	 * 
+	 * @throws IllegalStateException if an uncontained element is registered.
 	 */
-	public void endTransaction();
+	public void endTransaction() throws IllegalStateException;
 
 	/**
 	 * Resolves all {@link EObject}s contained in any resource of the given

bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolverImpl.java

@@ -84,7 +84,7 @@ public void registerEObject(Uuid uuid, EObject eObject) throws IllegalStateExcep
 	public void unregisterEObject(Uuid uuid, EObject eObject) throws IllegalStateException {
 		checkState(uuid != null, "uuid must not be null");
 		checkState(eObject != null, "object must not be null");
-		checkState(eObjectToUuid.get(eObject).equals(uuid), "trying to unregister element %s but is not registered for uuid %s", eObject, uuid);
+		checkState(uuid.equals(eObjectToUuid.get(eObject)), "trying to unregister element %s but is not registered for uuid %s", eObject, uuid);
 		eObjectToUuid.remove(eObject);
 	}
 
@@ -104,7 +104,12 @@ public Resource getResource(URI uri) {
 
 	@Override
 	public void endTransaction() {
-		cleanupRemovedElements();
+		var iterator = eObjectToUuid.keySet().iterator();
+		while (iterator.hasNext()) {
+			EObject object = iterator.next();
+			checkState(object.eResource() != null, "dangling object %s detected", object);
+			checkState(object.eResource().getResourceSet() == resourceSet, "object %s is contained in wrong resource set", object);
+		}
 	}
 
 	@Override
@@ -200,18 +205,6 @@ private boolean isReadOnlyUuid(Uuid uuid) {
 		return !uuid.getRawValue().startsWith(NON_READONLY_PREFIX);
 	}
 
-	private void cleanupRemovedElements() {
-		var iterator = eObjectToUuid.keySet().iterator();
-		while (iterator.hasNext()) {
-			EObject object = iterator.next();
-			//TODO: Hard constraint for dangling elements is currently disabled as Reactions produce incomplete change sequence
-//			checkState(object.eResource() != null && object.eResource().getResourceSet() != null, "dangling object %s detected", object);
-			if (object.eResource() == null || object.eResource().getResourceSet() == null) {
-				iterator.remove();
-			}
-		}
-	}
-
 	/**
 	 * Creates a mapping from UUIDs to hierarchical IDs. If a
 	 * <code>resourcesFilter</code> is provided, only {@link EObject}s contained in

bundles/tools.vitruv.change.propagation/src/tools/vitruv/change/propagation/impl/DefaultChangeRecordingModelRepository.java

@@ -45,7 +45,7 @@ public class DefaultChangeRecordingModelRepository implements PersistableChangeR
 	private final PersistableCorrespondenceModel correspondenceModel;
 	private final ChangeRecorder changeRecorder;
 	private final Path consistencyMetadataFolder;
-	private final UuidResolver uuidResolver;
+	private UuidResolver uuidResolver;
 	private final VitruviusChangeResolver<Uuid> changeResolver;
 
 	private boolean isLoading = false;
@@ -175,10 +175,10 @@ public VitruviusChange<EObject> applyChange(VitruviusChange<Uuid> change) {
 	@Override
 	public void close() throws Exception {
 		changeRecorder.close();
-		modelsResourceSet.getResources().stream().forEach((resource) -> resource.unload());
+		modelsResourceSet.getResources().forEach(Resource::unload);
 		modelsResourceSet.getResources().clear();
-		uuidResolver.endTransaction();
 		correspondenceModel.close();
+		uuidResolver = null;
 	}
 
 	@Override

bundles/tools.vitruv.testutils/src/tools/vitruv/testutils/views/ChangePublishingTestView.xtend

@@ -29,6 +29,7 @@ import tools.vitruv.testutils.TestUserInteraction
 
 import static com.google.common.base.Preconditions.checkArgument
 import static com.google.common.base.Preconditions.checkState
+import static edu.kit.ipd.sdq.commons.util.org.eclipse.emf.common.util.URIUtil.isPathmap
 import static tools.vitruv.testutils.TestModelRepositoryFactory.createTestChangeableModelRepository
 
 import static extension edu.kit.ipd.sdq.commons.util.java.lang.IterableUtil.flatMapFixed
@@ -172,8 +173,16 @@ class ChangePublishingTestView implements NonTransactionalTestView {
 	}
 
 	override disposeViewResources() {
+		resourceSet.resources.forEach [ resource |
+			if (resource.URI === null || !isPathmap(resource.URI)) {
+				resource.allContents.forEach [
+					if (uuidResolver.hasUuid(it)) {
+						uuidResolver.unregisterEObject(uuidResolver.getUuid(it), it)
+					}
+				]
+			}
+		]
 		resourceSet.resources.clear()
-		uuidResolver.endTransaction()
 	}
 
 	override <T extends Notifier> T startRecordingChanges(T notifier) {
@@ -224,6 +233,6 @@ class ChangePublishingTestView implements NonTransactionalTestView {
 		val changeableModelRepository = createTestChangeableModelRepository(modelRepository,
 			changePropagationSpecificationProvider, userInteraction)
 		return new ChangePublishingTestView(persistenceDirectory, userInteraction, UriMode.FILE_URIS,
-			changeableModelRepository, modelRepository.uuidResolver) [ modelRepository.getModelResource(it) ]
+			changeableModelRepository, modelRepository.uuidResolver)[modelRepository.getModelResource(it)]
 	}
 }

tests/tools.vitruv.change.atomic.tests/src/tools/vitruv/change/atomic/uuid/UuidResolvingTest.java

@@ -146,20 +146,16 @@ void elementDeletionDoesNotRemoveUuid() {
 	}
 
 	@Test
-	@DisplayName("cleanup resolver when saving after element removal")
-	void cleanupAfterElementRemovalRemovesUuid() {
+	@DisplayName("register dangling element")
+	void registerDanglingElement() {
 		var root = aet.Root();
-		Uuid uuid = uuidResolver.registerEObject(root);
-		uuidResolver.endTransaction();
-		assertThrows(IllegalStateException.class, () -> uuidResolver.getEObject(uuid));
-		assertThrows(IllegalStateException.class, () -> uuidResolver.getUuid(root));
-		assertFalse(uuidResolver.hasUuid(root));
-		assertFalse(uuidResolver.hasEObject(uuid));
+		uuidResolver.registerEObject(root);
+		assertThrows(IllegalStateException.class, () -> uuidResolver.endTransaction());
 	}
 
 	@Test
-	@DisplayName("cleanup resolver when saving resource after element removal")
-	void cleanupAfterElementRemovalRemovesUuidWithResource() {
+	@DisplayName("clear resource without unregistering elements")
+	void clearResourceWithoutUnregistration() {
 		var root = aet.Root();
 		URI resourceUri = URI.createFileURI(testProjectPath.resolve("root.aet").toString());
 		Resource rootResource = resourceSet.createResource(resourceUri);
@@ -170,10 +166,6 @@ void cleanupAfterElementRemovalRemovesUuidWithResource() {
 		assertEquals(root, uuidResolver.getEObject(uuid));
 
 		rootResource.getContents().clear();
-		uuidResolver.endTransaction();
-		assertThrows(IllegalStateException.class, () -> uuidResolver.getEObject(uuid));
-		assertThrows(IllegalStateException.class, () -> uuidResolver.getUuid(root));
-		assertFalse(uuidResolver.hasUuid(root));
-		assertFalse(uuidResolver.hasEObject(uuid));
+		assertThrows(IllegalStateException.class, () -> uuidResolver.endTransaction());
 	}
 }