From 959e8644105519e25a8ca5d549e8b9dbdf9024d9 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Wed, 26 Apr 2023 16:40:05 +0600 Subject: [PATCH 1/4] 1228-Remove_duplicate_remote_ids issue: https://gitlab.e.foundation/e/os/backlog/-/issues/1228 --- .../owncloud/notes/main/MainActivity.java | 10 +- .../owncloud/notes/main/MainViewModel.java | 3 +- .../notes/persistence/NotesRepository.java | 122 +++++++++++++++++- .../persistence/NotesServerSyncTask.java | 2 + .../notes/persistence/OldNoteRetriever.java | 4 +- .../notes/persistence/dao/NoteDao.java | 7 +- app/src/main/res/values/strings.xml | 2 + 7 files changed, 135 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java index c59cabb9a..9843e4cab 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/main/MainActivity.java @@ -4,28 +4,20 @@ import static android.os.Build.VERSION.SDK_INT; import static android.os.Build.VERSION_CODES.O; import static android.view.View.GONE; import static android.view.View.VISIBLE; -import static it.niedermann.owncloud.notes.NotesApplication.isDarkThemeActive; import static it.niedermann.owncloud.notes.NotesApplication.isGridViewEnabled; -import static it.niedermann.owncloud.notes.branding.BrandingUtil.getSecondaryForegroundColorDependingOnTheme; import static it.niedermann.owncloud.notes.shared.model.ENavigationCategoryType.DEFAULT_CATEGORY; import static it.niedermann.owncloud.notes.shared.model.ENavigationCategoryType.FAVORITES; import static it.niedermann.owncloud.notes.shared.model.ENavigationCategoryType.RECENT; import static it.niedermann.owncloud.notes.shared.model.ENavigationCategoryType.UNCATEGORIZED; -import static it.niedermann.owncloud.notes.shared.util.NotesColorUtil.contrastRatioIsSufficient; import static it.niedermann.owncloud.notes.shared.util.SSOUtil.askForNewAccount; import android.accounts.NetworkErrorException; import android.animation.AnimatorInflater; import android.app.SearchManager; import android.content.Intent; -import android.graphics.Color; -import android.graphics.PorterDuff; import android.net.Uri; import android.os.Bundle; import android.text.TextUtils; - -import it.niedermann.owncloud.notes.importaccount.ImportMurenaAccountViewModel; -import trikita.log.Log; import android.view.View; import androidx.annotation.NonNull; @@ -80,6 +72,7 @@ import it.niedermann.owncloud.notes.edit.category.CategoryViewModel; import it.niedermann.owncloud.notes.exception.ExceptionDialogFragment; import it.niedermann.owncloud.notes.exception.IntendedOfflineException; import it.niedermann.owncloud.notes.importaccount.ImportAccountActivity; +import it.niedermann.owncloud.notes.importaccount.ImportMurenaAccountViewModel; import it.niedermann.owncloud.notes.main.items.ItemAdapter; import it.niedermann.owncloud.notes.main.items.grid.GridItemDecoration; import it.niedermann.owncloud.notes.main.items.list.NotesListViewItemTouchHelper; @@ -101,6 +94,7 @@ import it.niedermann.owncloud.notes.shared.model.NoteClickListener; import it.niedermann.owncloud.notes.shared.util.CustomAppGlideModule; import it.niedermann.owncloud.notes.shared.util.NoteUtil; import it.niedermann.owncloud.notes.shared.util.ShareUtil; +import trikita.log.Log; public class MainActivity extends LockedActivity implements NoteClickListener, AccountPickerListener, AccountSwitcherListener, CategoryDialogFragment.CategoryDialogListener { diff --git a/app/src/main/java/it/niedermann/owncloud/notes/main/MainViewModel.java b/app/src/main/java/it/niedermann/owncloud/notes/main/MainViewModel.java index 0f1d3631b..95639fabd 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/main/MainViewModel.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/main/MainViewModel.java @@ -20,7 +20,6 @@ import android.accounts.NetworkErrorException; import android.app.Application; import android.content.Context; import android.text.TextUtils; -import trikita.log.Log; import android.util.Pair; import androidx.annotation.MainThread; @@ -66,6 +65,7 @@ import it.niedermann.owncloud.notes.shared.model.IResponseCallback; import it.niedermann.owncloud.notes.shared.model.ImportStatus; import it.niedermann.owncloud.notes.shared.model.Item; import it.niedermann.owncloud.notes.shared.model.NavigationCategory; +import trikita.log.Log; public class MainViewModel extends AndroidViewModel { @@ -667,5 +667,4 @@ public class MainViewModel extends AndroidViewModel { public void migrateOldNotes(@NonNull Account account) { repo.migrateOldNotesIfPossible(account.getId()); } - } \ No newline at end of file diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java index 06f78aef0..30065b48a 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java @@ -4,6 +4,7 @@ import static android.os.Build.VERSION.SDK_INT; import static android.os.Build.VERSION_CODES.O; import static androidx.lifecycle.Transformations.distinctUntilChanged; import static androidx.lifecycle.Transformations.map; +import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toMap; import static it.niedermann.owncloud.notes.edit.EditNoteActivity.ACTION_SHORTCUT; import static it.niedermann.owncloud.notes.shared.util.NoteUtil.generateNoteExcerpt; @@ -43,12 +44,15 @@ import com.nextcloud.android.sso.model.SingleSignOnAccount; import java.util.ArrayList; import java.util.Calendar; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.stream.Collectors; import it.niedermann.android.sharedpreferences.SharedPreferenceIntLiveData; import it.niedermann.owncloud.notes.BuildConfig; @@ -81,6 +85,7 @@ import retrofit2.Call; public class NotesRepository { private static final String PREF_KEY_MIGRATION_DONE = "old_note_migration_done"; + private static final String PREF_KEY_REMOTE_CONFLICT_RESOLVED = "remote_conflict_resolved"; private static final String TAG = NotesRepository.class.getSimpleName(); @@ -465,6 +470,15 @@ public class NotesRepository { .collect(toMap(Note::getRemoteId, Note::getId)); } + @NonNull + @WorkerThread + public Map> getNoteMapByRemoteId(long accountId) { + return db.getNoteDao() + .getActiveNoteList(accountId) + .stream() + .collect(groupingBy(Note::getRemoteId)); + } + @AnyThread public void toggleFavoriteAndSync(Account account, long noteId) { executor.submit(() -> { @@ -1005,7 +1019,7 @@ public class NotesRepository { } private boolean isNoteAlreadyExistsInNewTable(@NonNull OldNote oldNote) { - return db.getNoteDao().countByTitleAndContent(oldNote.getTitle(), oldNote.getContent()) > 0; + return db.getNoteDao().countByTitleAndContent(oldNote.getTitle(), oldNote.getContent(), oldNote.getRemoteId()) > 0; } private void addNewNote(long accountId, @NonNull OldNote oldNote) { @@ -1026,4 +1040,110 @@ public class NotesRepository { .putBoolean(PREF_KEY_MIGRATION_DONE, true) .apply(); } + + private boolean isRemoteIdConflictResolved() { + final var sharedPreferences = PreferenceManager.getDefaultSharedPreferences(context.getApplicationContext()); + return sharedPreferences.getBoolean(PREF_KEY_REMOTE_CONFLICT_RESOLVED, false); + } + + private void setRemoteIdConflictResolved() { + final var sharedPreferences = PreferenceManager.getDefaultSharedPreferences(context.getApplicationContext()); + sharedPreferences.edit() + .putBoolean(PREF_KEY_REMOTE_CONFLICT_RESOLVED, true) + .apply(); + } + + @WorkerThread + public void removeDuplicateRemoteIds() { + if (isRemoteIdConflictResolved()) { + return; + } + + List accounts = getAccounts(); + + for (Account account : accounts) { + if (account == null) { + continue; + } + + removeDuplicateNotesForAccount(account); + } + + setRemoteIdConflictResolved(); + } + + @WorkerThread + private void removeDuplicateNotesForAccount(@NonNull Account account) { + Map> noteMap = getNoteMapByRemoteId(account.getId()); + + for (Map.Entry> mapEntry : noteMap.entrySet()) { + if (notHaveDuplicateNote(mapEntry)) { + continue; + } + + removeConflictedNotes(mapEntry.getValue()); + } + } + + private boolean notHaveDuplicateNote(@NonNull Map.Entry> mapEntry) { + return mapEntry.getValue().size() <= 1; + } + + /* + * Preserve the first note & remove the duplicates. + * If notes have different content texts, concat them in the preserve note, so user will not loss any content. + */ + @WorkerThread + private void removeConflictedNotes(@NonNull List noteList) { + Note preservedNote = noteList.get(0); + + Set contentSet = new HashSet<>(); + contentSet.add(preservedNote.getContent()); + + int contentCounter = 1; + + for (int i = 1; i < noteList.size(); i++) { + Note note = noteList.get(i); + + db.getNoteDao().deleteByNoteId(note.getId(), note.getStatus()); + + if (contentSet.contains(note.getContent())) { + continue; + } + + contentSet.add(note.getContent()); + String content = getPreviousContentForConflict(contentCounter, preservedNote); + contentCounter++; + content += getConflictedContent(contentCounter, note); + preservedNote.setContent(content); + } + + updateNoteForConflict(contentCounter - 1, preservedNote); + } + + @NonNull + private String getConflictedContent(int position, @NonNull Note note) { + String modified = note.getModified().getTime().toString(); + return context.getString(R.string.conflicted_note_content_title, position, modified) + + note.getContent(); + } + + @NonNull + private String getPreviousContentForConflict(int position, @NonNull Note note) { + if (position > 1) { + return note.getContent(); + } + + return context.getString(R.string.conflict_note_detected) + + getConflictedContent(position, note); + } + + @WorkerThread + private void updateNoteForConflict(int conflictFound, @NonNull Note note) { + if (conflictFound > 0) { + note.setStatus(DBStatus.LOCAL_EDITED); + note.setModified(Calendar.getInstance()); + db.getNoteDao().updateNote(note); + } + } } diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java index b46c39f40..c53f17e2c 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesServerSyncTask.java @@ -205,6 +205,8 @@ abstract class NotesServerSyncTask extends Thread { private boolean pullRemoteChanges() { Log.d(TAG, "pullRemoteChanges() for account " + localAccount.getAccountName()); try { + repo.removeDuplicateRemoteIds(); + final var idMap = repo.getIdMap(localAccount.getId()); // FIXME re-reading the localAccount is only a workaround for a not-up-to-date eTag in localAccount. diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/OldNoteRetriever.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/OldNoteRetriever.java index 2e2e6a7d2..75d1c82e2 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/OldNoteRetriever.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/OldNoteRetriever.java @@ -70,8 +70,8 @@ public final class OldNoteRetriever { private static boolean isOldNoteTableExists(@NonNull SupportSQLiteDatabase supportSQLiteDatabase) { final Cursor tableExistCursor = supportSQLiteDatabase.query("SELECT name FROM sqlite_master WHERE type='table' AND name='" + OLD_NOTES_TABLE_NAME + "'"); final boolean tableExists = tableExistCursor.moveToNext(); - tableExistCursor.close(); - return tableExists; + tableExistCursor.close(); + return tableExists; } @NonNull diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/NoteDao.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/NoteDao.java index ce1475ccf..245d34e9d 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/NoteDao.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/dao/NoteDao.java @@ -138,6 +138,9 @@ public interface NoteDao { @Query("SELECT id, remoteId, 0 as accountId, '' as title, 0 as favorite, '' as excerpt, 0 as modified, '' as eTag, 0 as status, '' as category, '' as content, 0 as scrollY FROM NOTE WHERE accountId = :accountId AND status != 'LOCAL_DELETED' AND remoteId IS NOT NULL") List getRemoteIdAndId(long accountId); + @Query("SELECT * FROM NOTE WHERE accountId = :accountId AND status != 'LOCAL_DELETED' AND remoteId IS NOT NULL") + List getActiveNoteList(long accountId); + /** * Get a single {@link Note} by {@link Note#remoteId} (aka. Nextcloud file id) * @@ -194,6 +197,6 @@ public interface NoteDao { @Query("SELECT COUNT(*) FROM NOTE WHERE STATUS != '' AND accountId = :accountId") Long countUnsynchronizedNotes(long accountId); - @Query("SELECT COUNT(*) FROM NOTE WHERE title = :title AND content = :content") - Long countByTitleAndContent(String title, String content); + @Query("SELECT COUNT(*) FROM NOTE WHERE (title = :title AND content = :content) OR remoteId = :remoteId") + Long countByTitleAndContent(String title, String content, long remoteId); } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 0ae7db841..29be49b2c 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -367,4 +367,6 @@ Privacy policy Terms of service Authors + # We faced a conflict between your notes. Please choose the proper one. + \n\n***Note %1$d (Last modified: %2$s):***\n -- GitLab From 5c9cd277a1fa1cb436271b3ef17b49ade8d3ff0e Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 1 May 2023 16:54:11 +0600 Subject: [PATCH 2/4] refactor removeConflictNotes method --- .../owncloud/notes/persistence/NotesRepository.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java index 30065b48a..ae950ecea 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java @@ -1112,15 +1112,19 @@ public class NotesRepository { } contentSet.add(note.getContent()); - String content = getPreviousContentForConflict(contentCounter, preservedNote); - contentCounter++; - content += getConflictedContent(contentCounter, note); - preservedNote.setContent(content); + contentCounter = updatePreservedNoteContent(contentCounter, preservedNote, note); } updateNoteForConflict(contentCounter - 1, preservedNote); } + private int updatePreservedNoteContent(int position, @NonNull Note preservedNote, @NonNull Note note) { + String content = getPreviousContentForConflict(position, preservedNote); + content += getConflictedContent(++position, note); + preservedNote.setContent(content); + return position; + } + @NonNull private String getConflictedContent(int position, @NonNull Note note) { String modified = note.getModified().getTime().toString(); -- GitLab From e330e96fe5ad9d4bd673562bdc37d9d8cd804d05 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 May 2023 11:00:35 +0600 Subject: [PATCH 3/4] rename conflictFound param name to noOfConflict for better understanding --- .../owncloud/notes/persistence/NotesRepository.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java index ae950ecea..64e14ff2d 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java @@ -1143,8 +1143,8 @@ public class NotesRepository { } @WorkerThread - private void updateNoteForConflict(int conflictFound, @NonNull Note note) { - if (conflictFound > 0) { + private void updateNoteForConflict(int noOfConflicts, @NonNull Note note) { + if (noOfConflicts > 0) { note.setStatus(DBStatus.LOCAL_EDITED); note.setModified(Calendar.getInstance()); db.getNoteDao().updateNote(note); -- GitLab From 6f736668382f568b7f11e9669ccb1982669ac9a3 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 2 May 2023 12:11:56 +0600 Subject: [PATCH 4/4] refactor codes --- .../owncloud/notes/persistence/NotesRepository.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java index 64e14ff2d..edd75950e 100644 --- a/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java +++ b/app/src/main/java/it/niedermann/owncloud/notes/persistence/NotesRepository.java @@ -52,7 +52,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.stream.Collectors; import it.niedermann.android.sharedpreferences.SharedPreferenceIntLiveData; import it.niedermann.owncloud.notes.BuildConfig; @@ -1077,7 +1076,7 @@ public class NotesRepository { Map> noteMap = getNoteMapByRemoteId(account.getId()); for (Map.Entry> mapEntry : noteMap.entrySet()) { - if (notHaveDuplicateNote(mapEntry)) { + if (doNotHaveDuplicateNote(mapEntry)) { continue; } @@ -1085,7 +1084,7 @@ public class NotesRepository { } } - private boolean notHaveDuplicateNote(@NonNull Map.Entry> mapEntry) { + private boolean doNotHaveDuplicateNote(@NonNull Map.Entry> mapEntry) { return mapEntry.getValue().size() <= 1; } @@ -1143,8 +1142,8 @@ public class NotesRepository { } @WorkerThread - private void updateNoteForConflict(int noOfConflicts, @NonNull Note note) { - if (noOfConflicts > 0) { + private void updateNoteForConflict(int numberOfConflicts, @NonNull Note note) { + if (numberOfConflicts > 0) { note.setStatus(DBStatus.LOCAL_EDITED); note.setModified(Calendar.getInstance()); db.getNoteDao().updateNote(note); -- GitLab