Java in-memory model may not match underlying files

[JanWittler]

When propagating changes to the Java domain, the in-memory model might differ from the underlying files after finishing the propagation. This happens when a change indirectly affects objects which are not part of the actual VitruviusChange.
This is a critical issue, as our test logic uses the in-memory model instead of the underlying files to validate tests thus this issue cannot be discovered by our current test setup.
It seems that the bug is related to determining which files need to be re-written, as the file gets correctly updated when, in addition to the indirect change, a direct change to the affected object is recorded.

Example

To illustrate this problem, let's consider the UML -> Java case and assume there exist 2 classes in both UML and Java, A and B, and A has an attribute of type B. If now B is renamed to C in UML and changes are propagated, the in-memory model of Java correctly reflects this changes (attribute is of type C). However, in the generated files, A is not updated and still refers to B.
If in addition to the renaming of B some direct change to A is applied (like a rename / new attribute / new operation / ...) the file is correctly updated.

Example test case

Uses the context of UmlToJavaAttributeTest

@Test
def testRenameAttributeTypeReference() {
  typeClass.name = "Renamed"
  propagate

  val jAttr = getCorrespondingAttribute(uAttr)
  val jTypeClass = getCorrespondingClass(typeClass)
  assertJavaElementHasTypeRef(jAttr, createNamespaceReferenceFromClassifier(jTypeClass))
  assertAttributeEquals(uAttr, jAttr)
}

[JanWittler]

A test case which is already in use and which suffers from the same problem. In this specific case the Java files are not moved to the correct directory.

https://github.com/vitruv-tools/Vitruv-Applications-ComponentBasedSystems/blob/241372f1b6c8c81fb99fe01c5abd48324a1ac17e/tests/tools.vitruv.applications.umljava.tests/src/tools/vitruv/applications/umljava/tests/java2uml/JavaToUmlPackageTest.xtend#L72-L80

[HeikoKlare]

That's interesting. The essential problem is that the persistence of references uses information that may change without affecting the in-memory representation of a reference.
I do not see a generic way of identifying these kinds of references. We might say that the resources of elements referencing a changed object should be saved as well. But, first, this only solves the Java issue, but there may be other metamodels where even this is not sufficient. And, second, using the cross referencer to identify these references it quite costly.
Defining such references manually would require the possibility to adapt domain-specific logic for identifying modified resources.

So I see three options:

1. Let the domain define the logic for determining resources to be saved upon model changes. This would result in almost every (currently known) domain to only save the model containing the changed element. And in case of Java, renaming a classifier would lead to saving all models referencing that classifier.
2. The same as 1. but with the difference that we may only declare that all models of that domain are saved (which avoids the cross references for the cost of saving all models, but at least reduced to the domain).
3. We simply save all models and do not track for which of them this may be necessary.

Currently, either solution will work without noticable drawbacks. I am not in favor of querying cross references on every change. On the other hand, having large projects with a bunch of Java files may result in long saving times.

Finally, it is not even necessary to always save the resources in the VirtualModel. It is at least not necessary to perform this synchronously after change propagation, because the user of a VirtualModel should not access the saved resources anyway, but access them by deriving a view, which can even be done without having the models saved.

[JanWittler]

I would be in favour of your options 1 resp. 2. The only reason for option 3 from my perspective is reduced complexity but adding one more method to the VitruvDomain like affectedResourcesForChangedObjects(Set<EObject> changedObjects) does not seem to hard. Furthermore, I think for now we can just go with option 2, i.e. just provide a default implementation that returns all resources of the domain, as the performance of Vitruvius is right now not the most important aspect and we can quite easily later adapt the Java domain to use their own logic.

Finally, it is not even necessary to always save the resources in the VirtualModel. It is at least not necessary to perform this synchronously after change propagation, because the user of a VirtualModel should not access the saved resources anyway, but access them by deriving a view, which can even be done without having the models saved.

My first idea - which fits good with this comment - was to perform saving after a complete change propagation cycle (i.e. VirtualModel.propagate finished) is done. As you said, we don't really care when resources are saved and it often happens during one cycle that one resource is changed plenty of times. Saving the resource each time would be a huge overhead. However, I don't know how resource saving works right now and if this was a breaking change to some assumptions we are taking.