Merge with conflict leads to "theirs" work to be lost (erased by "mine")

[jbsarrodie]

Logging this for my own investigation

I very recently had two colleagues who faced a big issue with Archi 4.6 and coArchi 0.5.3.

In both cases, the publish their changes while their last publish had been done 1 or 2 months before. A one can imagine, they had some conflicts to solve. In both case they choose "theirs" to be sure to keep everything done by the rest of the team.

The result is a commit history not showing the usual merge commit labeled "Merge branch 'master' of ...", but instead it shows their lastest "real" commit (the one with a manually entered commit message) as a merge commit.

The impact is that in fact despite having chosen "theirs" in teh conflict solver, the model now contains their own (old) version and not a merge of their version and the current one on the git server.

I don't understand how this could happen, so I'm investigating....

[jbsarrodie]

I can now reproduce the bug:

Create a new model with only one view inside one sub-folder, and another folder. Then publish it (using this one https://collaboration.archimatetool.com/jbsarrodie/Issue_130):

From now on, we have two users (A and B) who work on this same version of the model)

User A updates moves the sub-folder "Some Folder" into "Folder 2", and create new views (but doesn't change the existing one) and publish it:

User B does a lot of changes to the original view, and add a new folder and a new view. Then publish:

A conflict is raised:

This happens because there have been so many changes to the view that git is not able to see this as a change on the same file (updated and moved), so git thinks it is a new view and thus thinks User A deleted the original one. Under the hood, I think git has no issue tracking the folder.xml file, so it is moved without any issue. Leading (if I choose "Mine") after the merge to two "Some Folder" folders, one with the original version of the view and folder.xml, the other with the new version of the view and no folder.xml.

Let's continue (keep "Mine"). As predicted, we have an error message about the missing folder.xml:

Let's click on "OK".

Everything seems normal and model is still opened in Archi. If we look at the change history, it clearly shows that the merge did not occured:

But now, let's try to publish again (that's what most user would do in this case). I'm asked to commit but there's no reasons as nothing changed. Let's enter a commit message and continue...

Publication is now done:

We could think everything is back to normal, but it is not. This last commit created is in fact a merge commit:

This has a nasty side effect of publishing the local version (not the one resulting of the previous, failed, merge), and thus erasing other people's work (if you look at previous screenshot, you'll see that "Folder 3" -created by User A- is missing).

[jbsarrodie]

I can reproduce this bug also with coArchi 0.6.0. The only slight difference is that after the error message about folder.xml, the model is closed. But other than that, impact is the same at the end: User A loses all his/her work.

[jbsarrodie]

@Phillipus I've found the origin of this bug: RefreshModelAction was not reseting the repository to local state in this very specific case: we have had a conflict, conflict has been solved, merge is ok from a git point of view, but resulting model is corrupted and cannot be loaded by GraficoModelImporter.

I've commited a fix in branch issue130. Can you have a look and tell me if you think there is any side effect please?

[Phillipus]

Your change calls loader.loadModel() in both cases in the if/else clause. Would it make sense to only call it once (as in the existing code) but surround that with a try/catch block and throw the ex exception?

[jbsarrodie]

Would it make sense to only call it once (as in the existing code) but surround that with a try/catch block and throw the ex exception?

The issue if that I can't call resetToLocal() outside the inner block

[Phillipus]

The issue if that I can't call resetToLocal() outside the inner block

OK, I see that.

And should it re-throw the exception? Or quietly absorb it and return?

[jbsarrodie]

And should it re-throw the exception? Or quietly absorb it and return?

We have to notify the user that something wrong happened, so forwarding the exception seems fine.