Skip to content

Commit

Permalink
Fix for Issue vitruv-tools#71
Browse files Browse the repository at this point in the history 
createDeleteChanges (formerly createDeleteChange) creates, for each
deleted object that has a container, a

1. RemoveReferenceChange, if this containment relationship is
many(=multi)-valued,
2. ReplaceSingleReferenceChange, if the containment only allows one
contained object at at time.
  • Loading branch information
@bjthehun
bjthehun committed Oct 31, 2024
1 parent 6f38451 commit e0fb652
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 46 deletions.
39 changes: 19 additions & 20 deletions ....vitruv.change.composite/src/tools/vitruv/change/composite/recording/ChangeRecorder.xtend
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import static extension tools.vitruv.change.atomic.EChangeUtil.*
* that all objects that have been removed from their containment reference without being added to a new containment
* reference while changes were being recorded have been deleted, resulting in an appropriate delete change.
* The recorder considers resources being loaded as existing and does thus not produce changes for it.
*
*
* Does not record changes of the <code>xmi:id</code> tag in an
* {@link org.eclipse.emf.ecore.xmi.XMLResource XMLResource} if it is not stored in the element
* but directly in the <code>Resource</code>.
Expand Down Expand Up @@ -75,7 +75,7 @@ class ChangeRecorder implements AutoCloseable {
// it can be potentially a reference to a third party model, for which no create shall be instantiated
create = create && (addedObject.eResource === null || affectedObject === null ||
addedObject.eResource == affectedObject.eResource)
if (create) existingObjects += addedObject
if(create) existingObjects += addedObject
return create;
}

Expand All @@ -88,12 +88,11 @@ class ChangeRecorder implements AutoCloseable {
def void addToRecording(Notifier notifier) {
checkNotDisposed()
checkNotNull(notifier, "notifier")
checkArgument(notifier.isInOurResourceSet,
"cannot record changes in a different resource set!")
checkArgument(notifier.isInOurResourceSet, "cannot record changes in a different resource set!")

if (rootObjects += notifier) {
notifier.recursively [
if (it instanceof EObject) existingObjects += it
if(it instanceof EObject) existingObjects += it
addAdapter()
]
}
Expand Down Expand Up @@ -183,7 +182,7 @@ class ChangeRecorder implements AutoCloseable {
if (allElementsToDelete.values.exists[it.contains(element)]) {
return
}
var elementsToDelete = element.eAllContents.toList.reverse //delete from inner to outer
var elementsToDelete = element.eAllContents.toList.reverse // delete from inner to outer
elementsToDelete.forEach [ child |
if (allElementsToDelete.containsKey(child)) {
allElementsToDelete.remove(child)
Expand All @@ -192,8 +191,8 @@ class ChangeRecorder implements AutoCloseable {
elementsToDelete.add(element)
allElementsToDelete.put(element, elementsToDelete)
]
changes += allElementsToDelete.values.flatMap [ elementsToDelete |
elementsToDelete.map [ converter.createDeleteChange(it) ]
changes += allElementsToDelete.values.flatMap [ elementsToDelete |
elementsToDelete.flatMap[converter.createDeleteChanges(it)]
].toList
}
return changes
Expand Down Expand Up @@ -315,18 +314,18 @@ class ChangeRecorder implements AutoCloseable {
converter.convert(new NotificationInfo(notification))
}
if (!changes.isEmpty) {
// Register any added object as existing, even if we are not recording
changes.forEach [
switch it {
EObjectAddedEChange<EObject>: {
existingObjects += newValue
switch it {
UpdateReferenceEChange<EObject>: existingObjects += affectedElement
}
}
}
]
}
// Register any added object as existing, even if we are not recording
changes.forEach [
switch it {
EObjectAddedEChange<EObject>: {
existingObjects += newValue
switch it {
UpdateReferenceEChange<EObject>: existingObjects += affectedElement
}
}
}
]
}
return changes
}

Expand Down
90 changes: 67 additions & 23 deletions ...omposite/src/tools/vitruv/change/composite/recording/NotificationToEChangeConverter.xtend
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import tools.vitruv.change.atomic.feature.reference.UpdateReferenceEChange

import static extension edu.kit.ipd.sdq.commons.util.java.lang.IterableUtil.*
import static extension tools.vitruv.change.composite.recording.EChangeCreationUtil.*
import java.util.LinkedList

/**
* Converts an EMF notification to an {@link EChange}.
Expand All @@ -23,20 +24,46 @@ import static extension tools.vitruv.change.composite.recording.EChangeCreationU
@FinalFieldsConstructor
package final class NotificationToEChangeConverter {
extension val TypeInferringAtomicEChangeFactory changeFactory = TypeInferringAtomicEChangeFactory.instance

val (EObject, EObject)=>boolean isCreateChange

def EChange<EObject> createDeleteChange(EObject eObject) {
return createDeleteEObjectChange(eObject)

/**
* Creates the required changes for the deletion of eObject:
* <ul>
* <li>If eObject is contained by another object o2, we first create a RemoveReferenceChange
* or ReplaceSingleReferenceChange, dependent on whether the containment relation of o2 may
* also contain other elements, or not. </li>
* <li>In any case, we also create a DeleteEObjectChange.</li>
* </ul>
*/
def List<EChange<EObject>> createDeleteChanges(EObject eObject) {
var changes = new LinkedList<EChange<EObject>>

val containingFeature = eObject.eContainmentFeature
if (containingFeature !== null) {
var container = eObject.eContainer

if (containingFeature.isMany) {
var indexOfEObject = containingFeature.eContents.indexOf(eObject)
changes.add(createRemoveReferenceChange(container, containingFeature, eObject, indexOfEObject))
}
else {
changes.add(createReplaceSingleReferenceChange(container, eObject.eContainmentFeature, eObject, null))
}
}

changes.add(createDeleteEObjectChange(eObject))
changes
}

private def String convertExceptionMessage(EventType eventType, String notificationType) {
String.format("Event type {} for {} Notifications unexpected.")
}

final String ATTRIBUTE_TYPE = "Attribute";
final String REFERENCE_TYPE = "Reference";
final String RESOURCE_CONTENTS_TYPE = "Resource Contents"

/**
* Converts the given notification to a list of {@link EChange}s.
* @param n the notification to convert
Expand All @@ -57,8 +84,10 @@ package final class NotificationToEChangeConverter {
case REMOVE: handleRemoveAttribute(notification)
case REMOVE_MANY: handleMultiRemoveAttribute(notification)
case MOVE: handleMoveAttribute(notification)
case RESOLVE: throw new IllegalArgumentException(convertExceptionMessage(EventType.RESOLVE, ATTRIBUTE_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(convertExceptionMessage(EventType.REMOVING_ADAPTER, ATTRIBUTE_TYPE))
case RESOLVE: throw new IllegalArgumentException(
convertExceptionMessage(EventType.RESOLVE, ATTRIBUTE_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(
convertExceptionMessage(EventType.REMOVING_ADAPTER, ATTRIBUTE_TYPE))
default: throw new IllegalArgumentException("Unexpected event type " + eventType)
}
case notification.isReferenceNotification:
Expand All @@ -70,8 +99,10 @@ package final class NotificationToEChangeConverter {
case REMOVE: handleRemoveReference(notification)
case REMOVE_MANY: handleMultiRemoveReference(notification)
case MOVE: handleMoveReference(notification)
case RESOLVE: throw new IllegalArgumentException(convertExceptionMessage(EventType.RESOLVE, REFERENCE_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(convertExceptionMessage(EventType.REMOVING_ADAPTER, REFERENCE_TYPE))
case RESOLVE: throw new IllegalArgumentException(
convertExceptionMessage(EventType.RESOLVE, REFERENCE_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(
convertExceptionMessage(EventType.REMOVING_ADAPTER, REFERENCE_TYPE))
default: throw new IllegalArgumentException("Unexpected event type " + eventType)
}
case notifier instanceof Resource:
Expand All @@ -82,17 +113,23 @@ package final class NotificationToEChangeConverter {
case ADD_MANY: handleMultiInsertRootChange(notification)
case REMOVE: handleRemoveRootChange(notification)
case REMOVE_MANY: handleMultiRemoveRootChange(notification)
case SET: throw new IllegalArgumentException(convertExceptionMessage(EventType.SET, RESOURCE_CONTENTS_TYPE))
case UNSET: throw new IllegalArgumentException(convertExceptionMessage(EventType.UNSET, RESOURCE_CONTENTS_TYPE))
case MOVE: throw new IllegalArgumentException(convertExceptionMessage(EventType.MOVE, RESOURCE_CONTENTS_TYPE))
case RESOLVE: throw new IllegalArgumentException(convertExceptionMessage(EventType.RESOLVE, RESOURCE_CONTENTS_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(convertExceptionMessage(EventType.REMOVING_ADAPTER, RESOURCE_CONTENTS_TYPE))
case SET: throw new IllegalArgumentException(
convertExceptionMessage(EventType.SET, RESOURCE_CONTENTS_TYPE))
case UNSET: throw new IllegalArgumentException(
convertExceptionMessage(EventType.UNSET, RESOURCE_CONTENTS_TYPE))
case MOVE: throw new IllegalArgumentException(
convertExceptionMessage(EventType.MOVE, RESOURCE_CONTENTS_TYPE))
case RESOLVE: throw new IllegalArgumentException(
convertExceptionMessage(EventType.RESOLVE, RESOURCE_CONTENTS_TYPE))
case REMOVING_ADAPTER: throw new IllegalArgumentException(
convertExceptionMessage(EventType.REMOVING_ADAPTER, RESOURCE_CONTENTS_TYPE))
default: throw new IllegalArgumentException("Unexpected event type " + eventType)
}
case Resource.RESOURCE__URI:
switch (eventTypeEnum) {
case SET: handleSetUriChange(notification)
default: throw new IllegalArgumentException("Unexpected event type " + eventType + " for Resource URI Notification.")
default: throw new IllegalArgumentException("Unexpected event type " + eventType +
" for Resource URI Notification.")
}
default:
emptyList()
Expand All @@ -101,14 +138,14 @@ package final class NotificationToEChangeConverter {
emptyList()
}
}

private def Iterable<? extends EChange<EObject>> handleMoveAttribute(extension NotificationInfo notification) {
#[
createRemoveAttributeChange(notifierModelElement, attribute, oldValue as Integer, newValue),
createInsertAttributeChange(notifierModelElement, attribute, position, newValue)
]
}

private def Iterable<? extends EChange<EObject>> handleMoveReference(extension NotificationInfo notification) {
#[
createRemoveReferenceChange(notifierModelElement, reference, newModelElementValue, oldValue as Integer),
Expand Down Expand Up @@ -229,7 +266,8 @@ package final class NotificationToEChangeConverter {
surroundWithCreateAndFeatureChangesIfNecessary()
}

private def Iterable<? extends EChange<EObject>> handleMultiInsertReference(extension NotificationInfo notification) {
private def Iterable<? extends EChange<EObject>> handleMultiInsertReference(
extension NotificationInfo notification) {
(newValue as List<EObject>).flatMapFixedIndexed [ index, value |
createInsertReferenceChange(notifierModelElement, reference, value, initialIndex + index).
surroundWithCreateAndFeatureChangesIfNecessary()
Expand Down Expand Up @@ -272,12 +310,17 @@ package final class NotificationToEChangeConverter {
]
}

def private Iterable<? extends EChange<EObject>> allAdditiveChangesForChangeRelevantFeatures(EObjectAddedEChange<EObject> change, EObject eObject) {
def private Iterable<? extends EChange<EObject>> allAdditiveChangesForChangeRelevantFeatures(
EObjectAddedEChange<EObject> change, EObject eObject) {
change.newValue.walkChangeRelevantFeatures(
[object, attribute|createAdditiveEChangeForAttribute(object, attribute)],
[object, reference|if (reference.isContainment) createAdditiveEChangeForReferencedObject(object, reference, [referencedObject | isCreateChange.apply(object, referencedObject)])]
[ object, reference |
if(reference.isContainment) createAdditiveEChangeForReferencedObject(object, reference, [ referencedObject |
isCreateChange.apply(object, referencedObject)
])
]
) + change.newValue.walkChangeRelevantFeatures(null) [ object, reference |
if (!reference.isContainment) createAdditiveEChangeForReferencedObject(object, reference, [false])
if(!reference.isContainment) createAdditiveEChangeForReferencedObject(object, reference, [false])
]
}

Expand Down Expand Up @@ -317,7 +360,8 @@ package final class NotificationToEChangeConverter {
emptyList()
}

def private <T extends EChange<EObject>> addUnsetChangeIfNecessary(Iterable<T> changes, NotificationInfo notification) {
def private <T extends EChange<EObject>> addUnsetChangeIfNecessary(Iterable<T> changes,
NotificationInfo notification) {
return if (notification.wasUnset)
changes +
List.of(createUnsetFeatureChange(notification.notifierModelElement, notification.structuralFeature))
Expand Down
6 changes: 4 additions & 2 deletions ...ange.composite.tests/src/tools/vitruv/change/composite/recording/ChangeRecorderTest.xtend
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,9 @@ class ChangeRecorderTest {
resource.contents.clear()
]

assertThat(changeRecorder.change, hasEChanges(RemoveRootEObject, DeleteEObject, DeleteEObject, DeleteEObject, DeleteEObject))
assertThat(changeRecorder.change, hasEChanges(
RemoveRootEObject, RemoveEReference, DeleteEObject, RemoveEReference, DeleteEObject, ReplaceSingleValuedEReference, DeleteEObject, DeleteEObject
))
}

@DisplayName("adds an object set as containment to the recording")
Expand Down Expand Up @@ -370,7 +372,7 @@ class ChangeRecorderTest {
record [
resource.delete(emptyMap)
]
assertThat(changeRecorder.change, hasEChanges(RemoveRootEObject, DeleteEObject, DeleteEObject))
assertThat(changeRecorder.change, hasEChanges(RemoveRootEObject, ReplaceSingleValuedEReference, DeleteEObject, DeleteEObject))
}

@ParameterizedTest(name="while isRecording={0}")
Expand Down
3 changes: 2 additions & 1 deletion .../src/tools/vitruv/change/composite/reference/ChangeDescription2RemoveEReferenceTest.xtend
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,11 @@ class ChangeDescription2RemoveEReferenceTest extends ChangeDescription2ChangeTra
]

// assert
result.assertChangeCount(5)
result.assertChangeCount(6)
.assertReplaceSingleValuedEReference(uniquePersistedRoot, ROOT__RECURSIVE_ROOT, containedRoot, null, true, false, false)
.assertReplaceSingleValuedEReference(innerContainedRoot, ROOT__SINGLE_VALUED_CONTAINMENT_EREFERENCE, nonRoot, null, true, false, false)
.assertReplaceSingleValuedEReference(uniquePersistedRoot, ROOT__SINGLE_VALUED_CONTAINMENT_EREFERENCE, null, nonRoot, true, false, false)
.assertReplaceSingleValuedEReference(containedRoot, ROOT__RECURSIVE_ROOT, innerContainedRoot, null, true, false, false)
.assertDeleteEObjectAndContainedElements(containedRoot)
.assertEmpty
}
Expand Down

0 comments on commit e0fb652

Please sign in to comment.