search

protegeproject / protege-client

Provides client functionality for the Protege Desktop application to connect to an OWL Ontology Server.
16 stars 9 forks source link

Client should not allow commiting changes that were other clients' changes #34

Closed rsgoncalves closed 8 years ago

rsgoncalves commented 8 years ago

This is a puzzling one. My setup had 2 clients connected. I committed a change on client 1. On client 2 I did server > update, and the server > commit menu item was enabled right after that. I checked for uncommitted changes (server > show uncommitted changes), and there were none to report. So out of curiosity I pressed commit, entered a message, and got a success confirmation that the commit was turned into revision X. Looking at the revision log diff what happened was that client 2 committed the exact same change(s) as client 1. So there seems to be some synchronization issue, but I can't quite put my finger on it yet.

bdionne commented 8 years ago

Indeed they shouldn't. This sounds very familiar and I've seen something like it before. We use the Protege HistoryManager now to determine uncommitted changes, rather than walking the ontology and computing them all the time. This requires a little finesse when doing updates from the server in the case that the client already has some edits going. It works like git stash and git stash apply. First the history is set aside, the changes from the server are applied to the ontology, the history is reset (as those changes create new history) and then the original client changes are put back.

What you describe sounds like something amiss still in this code

Show uncommitted changes also uses the HistoryManager to basically look them up

bdionne commented 8 years ago

We should discuss this one at our weekly today, or at least the three of us should. It seems like this logic should be all consolidated into the client and not in the UpdateActions and EditTab. We also need to decide what to do about the protege history-search branch

bdionne commented 8 years ago

Ok, I'm able to reproduce this, and fix it

The history manager changes I introduced needed to fire events in order for the menus to correctly update themselves.