diff --git a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/AtomicEChangeUuidResolver.java b/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/AtomicEChangeUuidResolver.java index ddba71cf..552bd827 100644 --- a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/AtomicEChangeUuidResolver.java +++ b/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/AtomicEChangeUuidResolver.java @@ -73,9 +73,11 @@ public EChange assignIds(EChange 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(); diff --git a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolver.java b/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolver.java index 8186dc9b..97dbbcad 100644 --- a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolver.java +++ b/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 diff --git a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolverImpl.java b/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolverImpl.java index b2206316..7b95c315 100644 --- a/bundles/tools.vitruv.change.atomic/src/tools/vitruv/change/atomic/uuid/UuidResolverImpl.java +++ b/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 * resourcesFilter is provided, only {@link EObject}s contained in diff --git a/bundles/tools.vitruv.change.propagation/src/tools/vitruv/change/propagation/impl/DefaultChangeRecordingModelRepository.java b/bundles/tools.vitruv.change.propagation/src/tools/vitruv/change/propagation/impl/DefaultChangeRecordingModelRepository.java index 2dbb94a9..9258611d 100644 --- a/bundles/tools.vitruv.change.propagation/src/tools/vitruv/change/propagation/impl/DefaultChangeRecordingModelRepository.java +++ b/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 changeResolver; private boolean isLoading = false; @@ -175,10 +175,10 @@ public VitruviusChange applyChange(VitruviusChange 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 diff --git a/bundles/tools.vitruv.testutils/src/tools/vitruv/testutils/views/ChangePublishingTestView.xtend b/bundles/tools.vitruv.testutils/src/tools/vitruv/testutils/views/ChangePublishingTestView.xtend index 9c3640b4..30909afb 100644 --- a/bundles/tools.vitruv.testutils/src/tools/vitruv/testutils/views/ChangePublishingTestView.xtend +++ b/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 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)] } } diff --git a/tests/tools.vitruv.change.atomic.tests/src/tools/vitruv/change/atomic/uuid/UuidResolvingTest.java b/tests/tools.vitruv.change.atomic.tests/src/tools/vitruv/change/atomic/uuid/UuidResolvingTest.java index 44fd981d..86389268 100644 --- a/tests/tools.vitruv.change.atomic.tests/src/tools/vitruv/change/atomic/uuid/UuidResolvingTest.java +++ b/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()); } }