Bug 336811

Comment 1 Yann Tanguy 2011-02-10 08:00:19 EST

Can you attach a corrupted file example ?

Comment 2 Ansgar Radermacher 2011-02-10 09:07:05 EST

Well, the referenced model was basically empty. I suspected that the unload mechanism causes the problem, but I cannot confirm it: when unloaded, the modification date of the referenced file does not change during save (referenced models are unloaded, if they change - the lazy load mechanism ensures that the updated contents becomes available, if required).

However, I could reproduce that the referenced model file is actually re-written: manually inserted spaces in the referenced model file vanish after the save is executing in the importing model.

Comment 3 Ansgar Radermacher 2011-02-10 09:15:53 EST

Effectively, there is a simpler test: if you open the importing model and change the imported model, the changes are actually stored in the imported model. I don't know, if someone added this as "a feature" (since it was not always the case) - it is however a very bad idea as it might interfere with changes made by an editor that opens the imported model directly.

Comment 4 Ansgar Radermacher 2011-03-03 05:37:08 EST

The error has been introduced by fix of bug 317430 (https://bugs.eclipse.org/bugs/show_bug.cgi?id=317430). I guess the basic motivation of 317430 was to let users work on a set of small models and enabling them to make changes to all of these) instead of using a single large one.
However, users are typically not aware that imports are saved and might make inconsistent updates if the imported models are referenced by more than one model. Another (minor) side-effect: If the referenced model is currently open, the user gets a pop-up that his resource has been modified.

If at all, additional models should not be saved by default, i.e. there could be a configuration property for it that is "off" by default.

>> I propose to deactivate the save of additional models completely as long as there is no property that let the user control it.

The save is done in method saveModel of class AdditionalResourcesModel (in org.eclipse.papyrus.resource.additional). The additional checks do not prevent a save in case of one model importing a model from the workspace.

public void saveModel() throws IOException {
  for(Resource r : modelSet.getResources()) {
    if(modelSet.isAdditionalResource(r.getURI())) {
      EditingDomain editingDomain = modelSet.getTransactionalEditingDomain();
      if(!r.getContents().isEmpty() && editingDomain != null && !editingDomain.isReadOnly(r) && (r.getURI().isPlatform() || r.getURI().isFile())) {
	r.save(Collections.EMPTY_MAP);
      }
    }
  }
}

Comment 5 Ansgar Radermacher 2011-03-03 08:34:26 EST

I guess I know how models could be emptied: the ResourceUpdateService assumes that imported models are read-only (see comments in ResourceUpdateService.java within diagram.common). Therefore, I considered it safe to unload imported models once they change. They will be reloaded on-demand if referenced.
However, during the save of the importing model, the unloaded imported model (thus empty) was saved as well. This is prevented since some time (20/12/2010) by a modification in class AdditionalResourceModel which now saves additional models only, if their contents is not empty. When I reported the error, I had apparently still an unpatched version of the AdditionalResourceModel class in my workspace.

Thus, it is currently probably not possible to erase existing model, but still easy to create inconsistencies (accidental modification of imported models, loose modifications that are made in the editor importing a model vs. modifications made in an editor opening it directly).

Comment 6 Tristan Faure 2011-03-03 08:59:46 EST

Models imported are saved to make possible the creation of controlled packages. If you disable this mechanism you shall guarantee that it is still possible to control/uncontrol packages

Comment 7 Ansgar Radermacher 2011-03-03 09:14:18 EST

Maybe I was ignorant concerning "controlled packages": I haven't heart about it before. Are there any documents available how the support of collaborative work on models is supposed to work?

Comment 8 Tristan Faure 2011-03-03 09:46:49 EST

There is a specification document in this section,
the document talks about collaborative work and package control

Comment 9 Tristan Faure 2011-03-03 09:52:04 EST

sorry with the link
svn+ssh://dev.eclipse.org/svnroot/modeling/org.eclipse.mdt.papyrus/trunk/doc/DevelopperDocuments/Specifications

Comment 10 Ansgar Radermacher 2011-03-11 04:41:46 EST

Checked-in trivial change into 0.7.x branch: only modified additional resources are written

Comment 11 Patrick Tessier 2013-04-29 05:55:33 EDT

change the status of the bug to "accept"

Comment 12 Camille Letavernier 2013-06-10 08:29:30 EDT

Still valid in 0.10. I won't fix it now because there's a conflict when CDO is installed (Which might lead to even worse/unexpected behaviors).

I'll fix that properly for SR1

Comment 13 Toni Siljamäki 2014-03-18 05:49:20 EDT

This is a very annoying bug.

When you have multiple models open that reference each other and then
make a modification of the model and saves it you get lots of confusing
pop-up windows asking to save (or not) the referenced models.

One example is when having a hyperlink to a diagram in a different model.

Users/architects have asked:
"Are we going to get all these pop-ups for every model we modify?"

Comment 14 Camille Letavernier 2014-05-06 07:54:44 EDT

Since the new Read-only framework is available, referenced models are not saved anymore (Unless they've explicitly been made writable by the user)

I close this task