notes-android icon indicating copy to clipboard operation
notes-android copied to clipboard

Prevent editing of unsynchronized notes

Open juk0de opened this issue 7 years ago • 24 comments

I like the app, but I recently found a problem that can cause loss of content (0.9.0 on Galaxy S5, Android 6.0.1). How to reproduce:

  1. Modify an existing note using the web interface (e.g. add or remove a line)
  2. Start the app and immediately start to edit the same note (before synchronization has been finished)
  3. Close the note on the app (using the "back" button)

Result: the changes made to the note using the web interface are lost and overwritten with the local changes (or the old version, if no local changes have been made).

I propose the following solution: the app should deny editing a note before synchronization has been finished! I. e. if I tap the "edit" button and the note has not yet been synchronized, instead of opening the editor with the outdated content, synchronization should be started and the editor should only be opened after the note has been synchronized. In order to indicate activity to the user, the same animation can be shown that is already used when triggering a manual synchronization (by swiping down). In case synchronization is not possible (e. g. no connection), a warning should be shown that makes it clear that all unsynchronized changes made through the web interface will be lost when the note is edited locally. What do you think?

juk0de avatar Jan 10 '17 09:01 juk0de

Thanks for reporting this issue, this is really a problem which should be solved. The problem also occurs in another scenario:

  1. Open a note for editing using the Android app.
  2. Modify this note on another device (e.g. using the Nextcloud Notes app or using another mobile device with this app; in practice, this could be done by another person when using shared notes).
  3. Modify and save the note on the original device (from step 1).

However, in this scenario, your approach of preventing the editing is not sufficient (the mobile user will still overwrite changes done by the other device). Furthermore, it slows down the interaction with the app -- although concurrent writes of notes are typically rather rarely. In some cases this will be just some milliseconds, but in other cases, this will be several seconds up to a minute (worst case: many large notes, slow device, slow and erroneous connection). Therefore, another solution for this issue should be found.

Unfortunately, this problem is not that easy. Therefore, I'm writing some thoughts down, but please feel free to add own ideas or concerns.

One enhancement would put your note-update to the background and then try to merge the remote changes with the (hopefully few) local changes. This could be done using a diff/merge-algorithm. A challenge is, that this has to be done in realtime (the user can do more changes in the meanwhile) and merging can produce messy conflicts. Furthermore, note-update is only possible if the network connection is active and stable.

Therefore, a solution for any scenario must realize an optimistic conflict-resolution which checks for concurrent writes when the note is synchronized back to the server. However, this requires changes in the server app (we could do that) which then has to reject the change (this should not happen) or likewise implement the diff/merge-algorithm. This has not to be done in realtime but it still can produce messy conflicts.

Finally, some background info: the problem we have here, is that we would like to facilitate consistency and availability during existing network partitions. Unfortunately, this is not possible (cp. CAP theorem). Therefore, other applications which use distributed data often use the approach server wins (e.g. DAVdroid) or client wins (e.g. this app currently), which means that in case of conflicts, all changes from the other side are overwritten by the client resp. server.

korelstar avatar Jan 10 '17 14:01 korelstar

Thanks for your response and explanation!

Actually I also thought about the possibility of merging the local and remote changes, but imo that would be very hard to implement because the notes app (if I understand correctly) changes the notes directly. Imho, in order to implement a reliable merge operation, you'd need all clients (and that includes the web interface) to make local / intermediate changes first and then try to merge them to the server when closing the editor (similar to a commit in a VCS). I assume that this would need a lot of modifications in both apps.

However, the problem I described is a lot easier, because there are no concurrent changes involved and it could therefore be solved by just making sure that a note is synced before it can be modified. Even if the synchronization should take up to a minute it has to be done in order to prevent data loss! Although I can't imagine that a single note takes that long to synchronize, you could give the user the possibility to cancel synchronization if it takes too long (e. g. by displaying a corresponding button along with the sync. animation).

Finally, to your last note: I think it's a good idea to let the user choose between "server wins", "client wins" and "ask user". Other apps are doing this too and I find it very helpful.

juk0de avatar Jan 10 '17 16:01 juk0de

Does "Client wins" mean that if there are two clients which make changes at the same time, there will be an endless fighting loop? – Edit. Ah, I see, client A will overwrite the data first, then mark it synced, then client B will overwrite with its data and mark it synced, so there will be no loop.

rfc2822 avatar Jan 10 '17 20:01 rfc2822

"Client wins" means "The last one who writes, wins".

Waiting for whole sync is not acceptable for the reasons @korelstar mentioned. "User decides"-strategy would be possible but requires another server roundtrip.

Assuming network connection is available:

  1. User edits something on the client
  2. Request the edited note from server to get the last modified date (maybe another api that only returns the modified date to keep transferred data small?)
  3. Compare modified date and then provide a dialog "Keep server" / "Keep local"

Assuming no network connection is available:

  1. User edits something on the client
  2. Next time when a connection is available all notes are synchronized
  3. Provide Pop-Up when modified date is conflicting

Though this is still not an optimal solution since, as mentioned, it requires another server roundtrip which will slow down the "live edit" feature (maybe we have to throttle it down to each 3 seconds?)

stefan-niedermann avatar Jan 11 '17 04:01 stefan-niedermann

Waiting for whole sync is not acceptable for the reasons @korelstar mentioned.

That's one way to look at it. For me, it's perfectly acceptable to wait a few seconds but it's not acceptable to lose my data. And just to be clear: I'm talking only about the note that is opened for editing. Only that note needs to by synchronized asap, the others can be synchronized in the background (you can't edit multiple notes on the same device at the same time anyway).

@korelstar only mentioned that it can take long to synchronize but didn't explain why. How is synchronization currently implemented? From what you've written above it sounds like a whole note is transferred in order to check if it's up-to-date. That would indeed be very inefficient and could be optimized. Some ideas:

  • compare timestamps first

  • compare checksums if timestamp indicates a modification

  • transfer only the modified part of a note (e.g. by using rsync)

  • use compression (e.g. zlib)

  • do this for each note individually, not all at once

  • optionally synchronize periodically in the background

If you manage to make synchronization fast, you can solve the problem I described above pretty easy.

Though this is still not an optimal solution since, as mentioned, it requires another server roundtrip which will slow down the "live edit" feature (maybe we have to throttle it down to each 3 seconds?)

I think it's important to distinguish between the scenario I described above ("single user, multiple clients") and the one you talk about ("multiple users, concurrent changes"). The first scenario is about making sure that a user never works with an outdated version of a note and the second one is about resolving merge conflicts. That are totally different problems.

To me it seems that you're trying to solve the first one by solving the second one. However the second one is much harder to solve and probably much less common. Therefore it makes sense to solve scenario one first (and help the majority of users) and scenario two later. It doesn't matter if you solve scenario two by using a merge algorithm or locking or whatever, it would still be necessary to synchronize a note before editing it, so the user always has the latest version to work with.

As a first step, you could make a menu option that prevents editing of unsynchronized notes if enabled. If that would be possible, I didn't have to manually synchronize each time before editing a note and that would already make me happy ;-)

juk0de avatar Jan 11 '17 09:01 juk0de

I agree that we have to optimize (speed up) the synchronization. I've already created an issue for that (#154). Details on realizing the optimization should be discussed there.

But apart from optimizing the synchronization, I think we should still have a synchronization of all notes when the notes app is opened resp. the main activity is called. Hence, typically, the notes should be up-to-date when you are starting to edit one. Only if you are faster than the synchronization and the note was changed remotely, you'll have a problem. Furthermore, you will only lose data, if you then change the note.

Therefore, I still don't like to slow down the overall app interaction (please note, that in the next release, you jump directly into editing when opening a note, see #141; so you can't even read an unchanged note before the synchronization has finished with your approach!). We have to find a better solution for this. And yes, you are right, my approaches are rather too complex for a soon implementation.

Here are some other (easier) ideas:

  • In the list of notes, show the state of the synchronization using a colored icon: red means that we have local changes which have to be synchronized, yellow means that we've had no synchronization for "some" time and green means that everything is "up-to-date". However, we would have to decide for a specific time which makes a formerly up-to-date synchronization outdated. Btw., that is why "Prevent editing of unsynchronized notes" is not that easy: when exactly is a note unsynchronized?

  • In the notes activity (i.e., when you are editing the note), show the date/time of the last synchronization (e.g. instead of the words Editing) and update the text-editor when synchronization has finished (check if user changed the note in the meanwhile and warn him then -- this can't be very much since synchronization will then be fast :D ).

What do you think about these ideas? We don't prevent lost updates, but the user can identify situations, when it's likely to have one. Then she/he can wait in order to prevent a lost update or go on if she/he knows that there is no remote change on this note. To be completely safe (my scenario), we have to do a conflict detection on synchronization like @stefan-niedermann and I proposed.

korelstar avatar Jan 11 '17 13:01 korelstar

Thanks for sharing your ideas!

But apart from optimizing the synchronization, I think we should still have a synchronization of all notes when the notes app is opened resp. the main activity is called.

I agree. I never meant to change that. But it should also be possible to synchronize a single note, i. e. there could be a function "synchronize(note X)" that can be called if a user opens a note that has not been synchronized yet. It should than be processes with max. priority.

Only if you are faster than the synchronization and the note was changed remotely, you'll have a problem.

You're right, but nonetheless I managed to trigger this problem within the first week after purchasing the app, so it's not that hard to reproduce ;-)

Furthermore, you will only lose data, if you then change the note.

Are you sure about that? If I remember correctly, I closed the note directly after opening the edit mode, because I realized that the content is outdated, but it still was overwritten. However, I'm only 90% sure about that...

Therefore, I still don't like to slow down the overall app interaction (please note, that in the next release, you jump directly into editing when opening a note, see #141; so you can't even read an unchanged note before the synchronization has finished with your approach!)

I understand what you're saying, but it's a very subjective issue. Currently I have to manually trigger a synchronization each time I start the app and wait for it to finish, before I can open a note. If there would be a mechanism that makes sure that the note is up-to-date when I edit it (i.e. do that automatically what I now do manually), that would improve my user experience and reduce the waiting time before I can finally edit the note. If you don't like to enable this "protective mechanism" for all users, you could make it optional.

If the note is directly opened for editing or not, does not make any difference for me. I neither want to read nor edit an outdated note (except if the device is offline or the server is unreachable, that case has to be handled somehow).

In the list of notes, show the state of the synchronization using a colored icon: red means that we have local changes which have to be synchronized, yellow means that we've had no synchronization for "some" time and green means that everything is "up-to-date".

I like that idea :+1:

Btw., that is why "Prevent editing of unsynchronized notes" is not that easy: when exactly is a note unsynchronized?

That's easy for the "single user, multiple clients" scenario: it's (potentially) outdated if it has not been synchronized since the user lastly opened the app (remember that there are no other users and I think we can safely assume that a single user is not editing the same note on different clients at the same time ;-)).

In the notes activity (i.e., when you are editing the note), show the date/time of the last synchronization (e.g. instead of the words Editing) and update the text-editor when synchronization has finished (check if user changed the note in the meanwhile and warn him then -- this can't be very much since synchronization will then be fast :D ).

That would also be an improvement. But updating a note that is currently edited is not that simple. You'd still need a merging algorithm to make it work...

What do you think about these ideas? We don't prevent lost updates, but the user can identify situations, when it's likely to have one.

Like I said, they would improve the situation. But the user still would have to manually check all the "warning signs" each time (and wait) if he wants to make sure that he gets the latest version of a note. Imho that's inconvenient and worses user experience. For users that prefer data integrity over speed (like me, and I'm sure that there are others out there) there should be an option like I described above.

To be completely safe (my scenario), we have to do a conflict detection on synchronization like @stefan-niedermann and I proposed.

There's another option: locking. Many people may consider locking as "not elegant" but it solves a lot of problems :D. If a note is always locked before somebody is editing it, there would be no need for a merge mechanism. You'd only have to make sure that the note is up-to-date before it is modified. Editing a note would then mean:

  1. Lock
  2. Synchronize
  3. Edit
  4. Commit
  5. Unlock

--> Multi-user safety without the risk of merge conflicts ;-)

For collaborative editing of complex documents, locking is not an option. But for a simple app like "notes", it would be feasible imho. But of course, you'd need to distinguish between viewing and editing a note...

juk0de avatar Jan 12 '17 09:01 juk0de

Instead of going into all details, we should concentrate on the real goal: avoid concurrent writes on the same note and provide the most current state of a note for reading.

You are requesting this workflow for opening a note:

  1. List of notes is shown
  2. Click on a note
  3. Show a progress bar
  4. Start synchronization for that note
  5. Wait for synchronization has finished
  6. Show the note in it's updated state

Please correct me, if I understood something wrong.


I don't want the user to force to wait, therefore my desired workflow is:

  1. List of notes is shown
  2. Click on a note
  3. Show the local state of the note
  4. Show a progress bar (optional with notice) but allow for reading and writing the note
  5. Start synchronization for that note
  6. When synchronization has finished, hide progress bar (and notice)
  7. If note was remotely changed, show the note in it's updated state

Please note, that

  • We will speed up synchronization (#154).
  • Closing the note without doing any changes will not overwrite remote changes (if this is currently the case, this has to be fixed; please create a new issue then).

Advantages of this solution:

  • It's a typical workflow on mobile devices, that the current state is shown, and then the data is updated. Compare, e.g., with your e-mail app.
  • In many scenarios, the note was not changed remotely since the last synchronization. In these scenarios, the user can directly begin with reading or writing without waiting.
  • If the note was changed remotely, the user often remembers, that the note was changed remotely (since it was recently -- otherwise, the note should be already synchronized) and waits for the progress bar to dissapear before editing. However, the user can begin reflecting the note (although it's an old state) without waiting.

Disadvantages of this solution:

  • If you started editing the note before the synchronization finished (not very likely), a pop-up will ask which version should be used. This is acceptable, since the user was informed about the ongoing synchronization.

I've made a mockup in order to get a feeling how this could look like:

Open a note while being online: note-updating

Open a note while begin offline: note-no-network

@stefan-niedermann @jancborchardt What do you think about this solution?

korelstar avatar Jan 16 '17 10:01 korelstar

Please correct me, if I understood something wrong.

It's correct! Of course, the progress bar would only have to be shown if the note has not yet been updated (which gets more unlikely, as soon as performance is improved).

I like the workflow / mockup you proposed, because it provides valuable feedback to the user and reduces the risk of making conflicting changes. It does not actually prevent them, but I could live with that ;-)

It's a typical workflow on mobile devices, that the current state is shown, and then the data is updated. Compare, e.g., with your e-mail app.

Comparing notes to e-mails is actually not so good, because e-mails can't be modified, so they are just visible or not, but never outdated. But I think I know what you mean ;-)

If you started editing the note before the synchronization finished (not very likely), a pop-up will ask which version should be used. This is acceptable, since the user was informed about the ongoing synchronization.

So yo don't plan to merge the changes from the server? That's OK for me. It saves a lot of work while still providing an improvement to the user. Overall, I like your ideas :+1:

juk0de avatar Jan 16 '17 15:01 juk0de

@korelstar thanks for this proposal. i think its really good compromise.

Only one thing that grinds my gears: The progress bar (Mockup 1) with the notice will show up every single time i open a note, right?

I think this is way to often since in 99% there will be no conflicts. So we should work out a way to show this progress bar and the message as less as possible. Maybe show the progress bar with a delay of hmm 500ms? Because: Most server round trips will (hopefully :wink:) not need this long and a typical user will not be able to open a note and change it within 500ms.

By the way: Let me say a clear no to any automatic merge algorithm since it is way too fault-prone. (Ever had fun with subversion merging? 😆 )

stefan-niedermann avatar Jan 16 '17 21:01 stefan-niedermann

Only one thing that grinds my gears: The progress bar (Mockup 1) with the notice will show up every single time i open a note, right?

I thought it would only be shown if the note has not been synchronized since the user started the app...

juk0de avatar Jan 17 '17 15:01 juk0de

@minimeee

So yo don't plan to merge the changes from the server?

I'm still thinking, that this is an important thing. But since it's much harder to implement, we should concentrate on the first step -- for now. But if someone wants to implement it: pull requests are very welcome (I think -- I'm not the owner of this :wink: ).

@stefan-niedermann

The progress bar (Mockup 1) with the notice will show up every single time i open a note, right? I think this is way to often since in 99% there will be no conflicts.

Yes, could be annoying. I think we have to try out, how it behaves in practice. I hope that we'll never see this progress bar, since the synchronization should typically have finished when opening a note. But your idea to delay the presentation could be a good solution to improve the situtation.

korelstar avatar Jan 18 '17 15:01 korelstar

I didn’t read the whole thread because wow is it long. But the simple fact it suggests »prevent editing« is wrong. ;) People should never be prevented from editing.

Offline should not be an error state because it isn’t. Offline is just something you sometimes are, like on the train, or in a plane, or with bad connection, etc. It needs to properly be worked into the app, rather than treated like an unusual state. :))

jancborchardt avatar Jan 24 '17 13:01 jancborchardt

Since this requires some work on the server side, I wrote (one year ago) down some ideas on how to solve the concurrent issue in nextcloud/notes#56.

korelstar avatar Apr 13 '18 19:04 korelstar

Wouldn't it be easiest to simply write out an additional file when conflicted copies are detected? That's how the *Cloud desktop clients and really any sync client handles it.

It wouldn't require any additional server-side code and allow the user to manually merge contents.

haarp avatar Sep 12 '18 20:09 haarp

Wouldn't it be easiest to simply write out an additional file when conflicted copies are detected? That's how the *Cloud desktop clients and really any sync client handles it.

@haarp the issue is that this is a very basic way to handle this, and people are left to clean it up. It’s not a nice experience at all, especially when for example in a long note only one line changed.

Whenever I see something like "File conflict" I still shudder, and I’m not even so bad with computers. ;)

jancborchardt avatar Dec 04 '18 15:12 jancborchardt

True, conflicts are terrible UX. But loosing data is certainly worse, no?

What about a simple optimistic locking strategy: Store the ETag for each note and then send an if-condition with that ETag with every write request. That would mean writing to a changed note fails and only in that case would conflict copies have to be created.

tribut avatar Dec 04 '18 15:12 tribut

Nevermind, has been already mentioned in nextcloud/notes#56. Sorry for the noise.

tribut avatar Dec 04 '18 15:12 tribut

No problem, we are happy about every kind of contribution @tribut :)

I thought trying to merge automatically if possible (like git does, when there are no conflicting changes in the same line), maybe there is some diff lib and maybe this should be part of the server app which should respond a 400 on failed automatically merge.....

stefan-niedermann avatar Dec 05 '18 08:12 stefan-niedermann

@stefan-niedermann Please see nextcloud/notes#56 😃 The correct respond would be 412 and an interesting library for merging is google-diff-match-patch. Other suggestions for libraries are welcome, please add them to nextcloud/notes#56 .

korelstar avatar Dec 05 '18 10:12 korelstar

OK, 9 mentions of data loss in this thread. Nobody likes data loss and most people here seem to be okay with slower updates or at least a warning message and being able to decide. This issue is quite old and it still makes my shared shopping list a constant cause for confusion, frustration, fights, tears and BBQs without charcoal. (Yes, I'm exaggerating a bit.) 😁

So, whom do I have to pay how much to implement this at least in a wary that nothing is being silently lost?

Update: I just saw this: https://github.com/nextcloud/notes/pull/692 So a proper solution should be possible by now. Honestly, is there someone I can pay to do this?

dryo avatar Jul 16 '21 20:07 dryo

5 years later and I still hit the very same problem while testing my local setup. Is there at least a plan for how to handle this? I might be able to help with coding.

m4ksio avatar Apr 28 '22 20:04 m4ksio

Five years later I still have to share my quite limited free time over several projects I contribute to. This is a complex topic and not just a "single line commit". My experience showed that starting several big branches is not the best idea - recent fundamental architecture changes like switching to Room and LiveData would have broken each branch that was not merged. I learned to only start working on stuff when my time frame is big enough to actually finish it.

Is there at least a plan for how to handle this? I might be able to help with coding.

Good starting points are

  1. reading this whole thread, especially korelstard comments regarding diffs
  2. reading the API documentation of the Notes server app
  3. Starting in this file, which contains the actualy synchronizatioj logic

stefan-niedermann avatar Apr 28 '22 21:04 stefan-niedermann

Wouldn't it be already a big improvement if there is a clear visual indication that the notes are currently not synchronized and that loss of data might occur if you start editing the document?

markus2330 avatar Jun 18 '22 03:06 markus2330

I'd love to have a feature according to the title. Many thanks @minimeee for creating this issue: I agree. I suffered thinking doing right but actually did wrong on several occasions now. I had a pleasant UX at first: Everything opened up directly and I read and edited the note. But what I did was terribly misguided because of the outdated data that was presented. I would have loved to wait for 10 minutes if I could then work on recent data.

Why merging and conflicts can not be out of scope of this issue?

If more people prefer a smooth appearance of the app more than preventing it from showing outdated data, this feature could be optional.

cyBea avatar Dec 23 '22 20:12 cyBea