Notes created or edited in airplane mode are deleted when online

[wobblywob]

Steps to recreate:

1. Turn on airplane mode
2. Create or edit notes in the app. The notes appear last as duplicates to the original notes
3. Turn off airplane mode
4. Refresh app. Notes and edits are lost

owncloud server version: 9.0.2
notes version: 2.0.1
android ownCloud Notes: 0.8.1

[moshpirit]

Just happened to me. It was a huge note.

Android 6.0, OwnCloud notes 0.8.1 (F-droid)

[emunicio]

It happened to me also. However I was not in airplane mode. I didn't have internet connection, and when refreshing after create the node, the note was deleted.

[stefan-niedermann]

yep. sorry guys, can confirm this is a bug that didn't appear in 0.7.2. i am very sorry for the delay, i have currently not much time to work on this project. i will fox this as soon as possible.

[stefan-niedermann]

ps: pull requests are very welcome :)

[korelstar]

Here are the changes from 0.7.2 to 0.8.1: ef331d0...0b8907c

I would say, the source for this bug could be in https://github.com/stefan-niedermann/OwnCloud-Notes/blob/0b8907c34abb170ff791f075e6cbc53e9e449114/app/src/main/java/it/niedermann/owncloud/notes/android/activity/NotesListViewActivity.java#L84 : here, db.getNoteServerSyncHelper().downloadNotes() is called without synchronizing the notes before. A solution could be to change this call into db.getNoteServerSyncHelper().synchronize(). But I haven't tested it yet.

[korelstar]

Okay, I had a more detailed look into the source code and found that this is not trivially fixed. There are many (error) cases, which are not considered in the current implementation. I suggest to revise and restructure NoteSQLiteOpenHelper and NoteServerSyncHelper in order to be more robust.

Some ideas:

• (required) The local database must have separate columns for the local primary key and the remote ID. Otherwise, it is possible that a locally created note conflicts with a complete different remotely created note, because they have the same IDs.
• Separate database and synchronisation methods, i.e. remove NoteSQLiteOpenHelper.XAndSync() methods and set correct status of Note in remaining methods. This distinction will also be realized in the frontend.
• Merge all data changing AsyncTasks in NoteServerSyncHelper (UploadEditedNotesTask, ...) into a single AsyncTask, because a single synchronization method, which uploads all changes to the server is sufficient and more resistant to errors (retry failed server calls automatically).
• (required) Make synchronisation resistant to errors during synchronisation, i.e. NEVER (!) call db.clearDatabase() in NoteServerSyncHelper.downloadNotes() (currently indirectly through DownloadNotesTask.onPostExecute()), instead merge the server data with the client data (since a previous update could have failed)
• NotesListViewActivity should call synchronize instead of downloadNotes in order to synchronize outstanding changes
• Start synchronizations only if there is a network connection (wifi or mobile network).

I could implement this, but it would take some time -- since my time is restricted, too.

[stefan-niedermann]

@korelstar thank you very much for analyzing the problem, it caused some headache to me^^. indeed, i used .synchronize() in v 0.7.2, while i tried to minimize the overhead in more recent versions by only using the atomar methods (download, etc). seemed to be a bad idea ;).
Refactroing the NotesServerSyncHelper and NoteSQLiteOpenHelper is welcome, i will review your merge request asap.

[korelstar]

I just found another important issue in this context: you use the same database column for the local primary key and the remote ID. Hence, it is possible that a locally created (and not yet synchronized) note conflicts with a complete different remotely created note, because they have the same IDs. This could also result in data loss. Therefore, I'll have to do also a database restructuring and change of the class Note. This even more makes it necessary to distinct between two classes of notes, as I've introduced in #121 (there: DBNote and Note).

[korelstar]

There are many synchronization and concurrency issues, that were not addressed yet. But I'm almost done, now.

Please have a look at #123, test my bugfixes and give feedback!