search

vitruv-tools / Vitruv

View-based Development and Model Consistency Framework
http://vitruv.tools
Eclipse Public License 1.0
14 stars 19 forks source link

Move state-based propagation to views #492

Closed JanWittler closed 2 years ago

JanWittler commented 2 years ago

This PR directly addresses #454 with two contributions.

  1. Replaces references to the state-based API with delta-based or view-based API references.
  2. Replaces the state-based API with a ChangeDerivingView which offers state-based change functionality.

Since core functionality of views is shared among change recording or change deriving views, a base view class BasicView was extracted to bundle all this logic. The existing ChangeRecordingView and the new ChangeDerivingView are modeled as decorators for that view. To retrieve these extended views, the View interface got extended by the withChangeRecordingTrait() and withChangeDerivingTrait(StateBasedChangeDerivationStrategy) methods. This allows to dynamically switch between the recording / deriving trait.

Since a view without any change recording or change deriving functionality is not able to identify changes and thus cannot commit them, the commitChanges() method was moved from the View interface to the new CommittableView interface. I would have preferred to use the notions of read-only (for View) and read-write (for CommittableView) but this is imprecise as model elements can be edited in any view (just with the lack of committing for View). Though, I am not really happy with the naming, so would be glad for naming suggestions.

From my understanding, the provided ChangeDerivingView is not a real state-based view as modifications to its resources are still executed by manipulating the objects in memory (which implies that we could also monitor the changes). For a real state-based view, functionality to reload resources from disk was needed (or even better a file system observer for the view's working directory). However, this is subject to future change and probably many discussions and therefore out of scope for this issue. As file reloading functionality was not supported before, this PR does not reduce functionality.

JanWittler commented 2 years ago

This PR is currently missing that the state-based strategies are moved into the views framework. Making another subpackage for strategies clutters the tools.vitruv.framework.vsum even more than it is currently. I would suggest to move the view functionality into a separate package tools.vitruv.framework.views as views are by design decoupled from the specific VSUM implementation but rather only require a ViewSource. Then, strategies could be added as a subpackage like tools.vitruv.framework.views.changederivation. @HeikoKlare @tsaglam what do you think?

HeikoKlare commented 2 years ago

Do you propose to move view functionality into a separate package or rather into a separate project? Because views are already defined in a separate package (although it is called tools.vitruv.framework.vsum.views rather than tools.vitruv.framework.views).

With the same rationale you mention, I tried to move the view functionality into a separate project in the last PR in which we introduced the views. There was some problem due to which views are not as independent of a VirtualModel as I expected them to be, but I have to admit that I forgot to document that and cannot remember anymore. Despite that, if is it possible to separate views into an independent project, I would absolutely be in favor of it.

JanWittler commented 2 years ago

I meant separate project.

I think the problem was that the extraction into an independent project resulted in cyclic references between the two projects. In particular, views require the ChangeableModelRepository, ChangePropagationAbortCause, and ChangePropagationListener interfaces. As all of these interfaces are only dependent on elements from tools.vitruv.framework.change and at least the latter two are also semantically more close to changes than to the vsum concept, I moved the 3 interfaces into that package. With that change views and vsum could be separated into two projects.

JanWittler commented 2 years ago

I moved the extraction of views as a separate project into a different PR #493 to keep this PR readable.

JanWittler commented 2 years ago

I updated the view class hierarchy to have a base BasicView which bundles core functionality and two decorators ChangeDerivingView and ChangeRecordingView as discussed in the dev call. This required some modifications to the View interface which is documented in the PR's initial comment.