Skip to content

Commit

Permalink
Merge pull request orgzly-revived#248 from amberin/handle-remote-dele…
Browse files Browse the repository at this point in the history
…tions

Handle remote deletions more gracefully
  • Loading branch information
amberin authored Jun 19, 2024
2 parents 6f29477 + 1da0da0 commit f53bf57
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,7 @@ public void testSortByPriorityDesc() {
"* Note D\n");
scenario = ActivityScenario.launch(MainActivity.class);

SystemClock.sleep(200);
onView(allOf(withText("notebook"), isDisplayed())).perform(click());
searchForTextCloseKeyboard(".o.p");
onView(withId(R.id.fragment_query_search_view_flipper)).check(matches(isDisplayed()));
Expand Down Expand Up @@ -779,6 +780,7 @@ public void testDeSelectRemovedNoteInSearch() {
public void testNoNotesFoundMessageIsDisplayedInSearch() {
scenario = ActivityScenario.launch(MainActivity.class);
searchForTextCloseKeyboard("Note");
SystemClock.sleep(200);
onView(withText(R.string.no_notes_found_after_search)).check(matches(isDisplayed()));
}

Expand Down
66 changes: 66 additions & 0 deletions app/src/androidTest/java/com/orgzly/android/repos/SyncTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,28 @@ public void testOnlyBookWithLink() {
assertEquals(BookSyncStatus.ONLY_BOOK_WITH_LINK.toString(), book.getBook().getSyncStatus());
}

@Test
public void testOnlyBookWithoutLinkAndOneRepo() {
testUtils.setupRepo(RepoType.MOCK, "mock://repo-a");
testUtils.setupBook("book-1", "Content");
testUtils.sync();

BookView book = dataRepository.getBooks().get(0);
assertEquals(BookSyncStatus.ONLY_BOOK_WITHOUT_LINK_AND_ONE_REPO.toString(), book.getBook().getSyncStatus());
}

@Test
public void testOnlyBookWithoutLinkAndMultipleRepos() {
testUtils.setupRepo(RepoType.MOCK, "mock://repo-a");
testUtils.setupRepo(RepoType.MOCK, "mock://repo-b");
testUtils.setupBook("book-1", "Content");
testUtils.sync();

BookView book = dataRepository.getBooks().get(0);
assertEquals(BookSyncStatus.ONLY_BOOK_WITHOUT_LINK_AND_MULTIPLE_REPOS.toString(),
book.getBook().getSyncStatus());
}

@Test
public void testMultipleRooks() {
Repo repoA = testUtils.setupRepo(RepoType.MOCK, "mock://repo-a");
Expand Down Expand Up @@ -467,4 +489,48 @@ public void testRenameSyncedBookWithDifferentLink() throws IOException {
assertEquals("mock://repo-b", book.getLinkRepo().getUrl());
assertEquals("mock://repo-a/Booky.org", book.getSyncedTo().getUri().toString());
}

/**
* We remove the local book's' syncedTo attribute and repository link when its remote file
* has been deleted, to make it easier to ascertain the book's state during subsequent sync
* attempts.
*/
@Test
public void testRemoteFileDeletion() throws IOException {
BookView book;
Repo repo = testUtils.setupRepo(RepoType.MOCK, "mock://repo-a");
testUtils.setupRook(repo, "mock://repo-a/book.org", "", "1abcdef", 1400067156);
testUtils.sync();
book = dataRepository.getBooks().get(0);
assertNotNull(book.getLinkRepo());
assertNotNull(book.getSyncedTo());
dbRepoBookRepository.deleteBook(Uri.parse("mock://repo-a/book.org"));
testUtils.sync();
book = dataRepository.getBooks().get(0);
assertEquals(BookSyncStatus.ROOK_NO_LONGER_EXISTS.toString(), book.getBook().getSyncStatus());
assertNull(book.getLinkRepo());
assertNull(book.getSyncedTo());
}

/**
* The "remote file has been deleted" error status is only shown once, and then the book's
* repo link is removed. Subsequent syncing of the same book should result in a more general
* message, indicating that the user may sync the book again by explicitly setting a repo link.
*/
@Test
public void testBookStatusAfterMultipleSyncsFollowingRemoteFileDeletion() throws IOException {
BookView book;
Repo repo = testUtils.setupRepo(RepoType.MOCK, "mock://repo-a");
testUtils.setupRook(repo, "mock://repo-a/book.org", "", "1abcdef", 1400067156);
testUtils.sync();
book = dataRepository.getBooks().get(0);
assertEquals(BookSyncStatus.DUMMY_WITHOUT_LINK_AND_ONE_ROOK.toString(), book.getBook().getSyncStatus());

dbRepoBookRepository.deleteBook(Uri.parse("mock://repo-a/book.org"));
testUtils.sync();
testUtils.sync();
book = dataRepository.getBooks().get(0);
assertNull(book.getLinkRepo());
assertEquals(BookSyncStatus.BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK.toString(), book.getBook().getSyncStatus());
}
}
4 changes: 4 additions & 0 deletions app/src/main/java/com/orgzly/android/data/DataRepository.kt
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,10 @@ class DataRepository @Inject constructor(
db.bookSync().upsert(bookId, versionedRookId)
}

fun removeBookSyncedTo(bookId: Long) {
db.bookSync().delete(bookId)
}

private fun updateBookIsModified(bookId: Long, isModified: Boolean, time: Long = System.currentTimeMillis()) {
updateBookIsModified(setOf(bookId), isModified, time)
}
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/java/com/orgzly/android/db/dao/BookSyncDao.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ interface BookSyncDao : BaseDao<BookSync> {
@Query("SELECT * FROM book_syncs WHERE book_id = :bookId")
fun get(bookId: Long): BookSync?

@Query("DELETE FROM book_syncs WHERE book_id = :bookId")
fun delete(bookId: Long)

@Transaction
fun upsert(bookId: Long, versionedRookId: Long) {
val sync = get(bookId)
Expand Down
10 changes: 10 additions & 0 deletions app/src/main/java/com/orgzly/android/prefs/AppPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,16 @@ public static void remindersForEventsEnabled(Context context, boolean value) {
getDefaultSharedPreferences(context).edit().putBoolean(key, value).apply();
}

public static boolean anyNotificationsEnabled(Context context) {
return (
showSyncNotifications(context) ||
newNoteNotification(context) ||
remindersForScheduledEnabled(context) ||
remindersForDeadlineEnabled(context) ||
remindersForEventsEnabled(context)
);
}

public static boolean remindersSound(Context context) {
return getDefaultSharedPreferences(context).getBoolean(
context.getResources().getString(R.string.pref_key_reminders_sound),
Expand Down
26 changes: 19 additions & 7 deletions app/src/main/java/com/orgzly/android/sync/BookNamesake.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.content.Context;

import com.orgzly.android.BookName;
import com.orgzly.android.db.entity.BookAction;
import com.orgzly.android.db.entity.BookView;
import com.orgzly.android.db.entity.Repo;
import com.orgzly.android.repos.VersionedRook;
Expand Down Expand Up @@ -116,7 +117,6 @@ public String toString() {
* - Book has a last-synced-with remote book
* - Remote book exists
*/
/* TODO: Case: Remote book deleted? */
public void updateStatus(int reposCount) {
/* Sanity check. Group's name must come from somewhere - local or remote books. */
if (book == null && versionedRooks.isEmpty()) {
Expand All @@ -142,17 +142,29 @@ public void updateStatus(int reposCount) {

} else {
if (book.hasLink()) { /* Only local book with a link. */
status = BookSyncStatus.ONLY_BOOK_WITH_LINK;

if (book.hasSync()) {
// Book was previously synced with a remote book which no longer exists.
status = BookSyncStatus.ROOK_NO_LONGER_EXISTS;
} else {
// Book is linked to a repo, but not yet synced.
status = BookSyncStatus.ONLY_BOOK_WITH_LINK;
}
} else { /* Only local book without link. */
if (reposCount > 1) {
status = BookSyncStatus.ONLY_BOOK_WITHOUT_LINK_AND_MULTIPLE_REPOS;
} else {
status = BookSyncStatus.ONLY_BOOK_WITHOUT_LINK_AND_ONE_REPO;
} // TODO: What about no repos?
} else { // Only one repository configured
BookAction lastAction = book.getBook().getLastAction();
if (lastAction != null && lastAction.getType() == BookAction.Type.ERROR) {
// Book is an error state.
status = BookSyncStatus.BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK;
} else {
// Book is not in an error state (automatic linking may be
// attempted).
status = BookSyncStatus.ONLY_BOOK_WITHOUT_LINK_AND_ONE_REPO;
}
}
}
}

return;
}

Expand Down
14 changes: 12 additions & 2 deletions app/src/main/java/com/orgzly/android/sync/BookSyncStatus.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ enum class BookSyncStatus {
BOOK_WITH_LINK_AND_ROOK_EXISTS_BUT_LINK_POINTING_TO_DIFFERENT_ROOK,
ONLY_DUMMY,
ROOK_AND_VROOK_HAVE_DIFFERENT_REPOS,
BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK,

/* Conflict. */
CONFLICT_BOTH_BOOK_AND_ROOK_MODIFIED,
Expand All @@ -29,7 +30,10 @@ enum class BookSyncStatus {
/* Book can be saved. */
ONLY_BOOK_WITHOUT_LINK_AND_ONE_REPO,
BOOK_WITH_LINK_LOCAL_MODIFIED,
ONLY_BOOK_WITH_LINK;
ONLY_BOOK_WITH_LINK,

/* Previously synced remote book no longer exists. */
ROOK_NO_LONGER_EXISTS;

// TODO: Extract string resources
@JvmOverloads
Expand All @@ -49,7 +53,7 @@ enum class BookSyncStatus {
return context.getString(R.string.sync_status_no_book_multiple_rooks)

ONLY_BOOK_WITHOUT_LINK_AND_MULTIPLE_REPOS ->
return "Notebook has no link and multiple repositories exist"
return context.getString(R.string.sync_status_no_link_and_multiple_repos)

BOOK_WITH_LINK_AND_ROOK_EXISTS_BUT_LINK_POINTING_TO_DIFFERENT_ROOK ->
return "Notebook has link and remote notebook with the same name exists, but link is pointing to a different remote notebook which does not exist"
Expand All @@ -60,6 +64,9 @@ enum class BookSyncStatus {
ROOK_AND_VROOK_HAVE_DIFFERENT_REPOS ->
return "Linked and synced notebooks have different repositories"

BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK ->
return context.getString(R.string.sync_status_no_link_and_previous_error)

CONFLICT_BOTH_BOOK_AND_ROOK_MODIFIED ->
return "Both local and remote notebook have been modified"

Expand All @@ -75,6 +82,9 @@ enum class BookSyncStatus {
ONLY_BOOK_WITHOUT_LINK_AND_ONE_REPO, BOOK_WITH_LINK_LOCAL_MODIFIED, ONLY_BOOK_WITH_LINK ->
return context.getString(R.string.sync_status_saved, "$arg")

ROOK_NO_LONGER_EXISTS ->
return context.getString(R.string.sync_status_rook_no_longer_exists)

else ->
throw IllegalArgumentException("Unknown sync status " + this)
}
Expand Down
23 changes: 15 additions & 8 deletions app/src/main/java/com/orgzly/android/sync/SyncUtils.kt
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ object SyncUtils {
for (repo in repoList) {
if (repo is GitRepo && repo.isUnchanged) {
for (book in dataRepository.getBooks()) {
if (book.hasLink()) {
if (book.linkRepo!!.url == repo.uri.toString()) {
result.add(book.syncedTo!!)
}
if (book.hasLink() && book.linkRepo!!.url == repo.uri.toString() && book.hasSync()) {
result.add(book.syncedTo!!)
}
}
if (result.isNotEmpty()) {
Expand Down Expand Up @@ -114,6 +112,8 @@ object SyncUtils {
BookSyncStatus.NO_CHANGE ->
bookAction = BookAction.forNow(BookAction.Type.INFO, namesake.status.msg())

/* Error states */

BookSyncStatus.BOOK_WITHOUT_LINK_AND_ONE_OR_MORE_ROOKS_EXIST,
BookSyncStatus.DUMMY_WITHOUT_LINK_AND_MULTIPLE_ROOKS,
BookSyncStatus.NO_BOOK_MULTIPLE_ROOKS,
Expand All @@ -123,9 +123,18 @@ object SyncUtils {
BookSyncStatus.CONFLICT_BOOK_WITH_LINK_AND_ROOK_BUT_NEVER_SYNCED_BEFORE,
BookSyncStatus.CONFLICT_LAST_SYNCED_ROOK_AND_LATEST_ROOK_ARE_DIFFERENT,
BookSyncStatus.ROOK_AND_VROOK_HAVE_DIFFERENT_REPOS,
BookSyncStatus.ONLY_DUMMY ->
BookSyncStatus.ONLY_DUMMY,
BookSyncStatus.BOOK_WITH_PREVIOUS_ERROR_AND_NO_LINK ->
bookAction = BookAction.forNow(BookAction.Type.ERROR, namesake.status.msg())

BookSyncStatus.ROOK_NO_LONGER_EXISTS -> {
/* Remove repository link and "synced to" information. User must set a repo link if
* they want to keep the book and sync it. */
dataRepository.setLink(namesake.book.book.id, null)
dataRepository.removeBookSyncedTo(namesake.book.book.id)
bookAction = BookAction.forNow(BookAction.Type.ERROR, namesake.status.msg())
}

/* Load remote book. */

BookSyncStatus.NO_BOOK_ONE_ROOK, BookSyncStatus.DUMMY_WITHOUT_LINK_AND_ONE_ROOK -> {
Expand Down Expand Up @@ -169,8 +178,6 @@ object SyncUtils {
}
}

// if (BuildConfig.LOG_DEBUG) LogUtils.d(TAG, "Syncing $namesake: $bookAction")

return bookAction
}

Expand Down Expand Up @@ -214,4 +221,4 @@ object SyncUtils {
}
return noNewMergeConflicts
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.content.IntentFilter;
import android.content.res.Configuration;
import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.view.MenuItem;
Expand Down Expand Up @@ -71,6 +72,7 @@
import com.orgzly.android.usecase.UseCase;
import com.orgzly.android.usecase.UseCaseResult;
import com.orgzly.android.usecase.UseCaseWorker;
import com.orgzly.android.util.AppPermissions;
import com.orgzly.android.util.LogUtils;
import com.orgzly.org.datetime.OrgDateTime;

Expand Down Expand Up @@ -143,6 +145,13 @@ protected void onCreate(Bundle savedInstanceState) {

setupDisplay(savedInstanceState);

if (AppPreferences.anyNotificationsEnabled(this)) {
if (Build.VERSION.SDK_INT >= 33) {
// Ensure we have the POST_NOTIFICATIONS permission
AppPermissions.isGrantedOrRequest(this, AppPermissions.Usage.POST_NOTIFICATIONS);
}
}

if (AppPreferences.newNoteNotification(this)) {
Notifications.showOngoingNotification(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class GitRepoActivity : CommonActivity(), GitPreferences {
// Using SSH transport requires notification permission (for server key verification)
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
AppPermissions.isGrantedOrRequest(
App.getCurrentActivity(),
this,
AppPermissions.Usage.POST_NOTIFICATIONS
)
}
Expand Down
5 changes: 4 additions & 1 deletion app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
<string name="ssh_keygen_preference_title">SSH key generation</string>
<string name="ssh_keygen_preference_summary">Generate key pair for Git repo sync</string>
<string name="ssh_show_public_key_preference_title">View generated SSH public key</string>
<string name="message_book_has_no_link">No link set</string>
<string name="message_book_has_no_link">Notebook has no repository link</string>
<string name="message_note_created">Note created</string>
<string name="message_failed_creating_note">Failed to create note</string>
<string name="message_failed_updating_note">Failed to update note</string>
Expand Down Expand Up @@ -772,9 +772,12 @@

<!-- Sync status -->
<string name="sync_status_no_change">No change</string>
<string name="sync_status_no_link_and_multiple_repos">Notebook has no link and multiple repositories exist</string>
<string name="sync_status_book_without_link_and_one_or_more_rooks_exist">Notebook has no link and one or more remote notebooks with the same name exist</string>
<string name="sync_status_dummy_without_link_and_multiple_rooks">Notebook has no link and multiple remote notebooks with the same name exist</string>
<string name="sync_status_no_book_multiple_rooks">No notebook and multiple remote notebooks with the same name exist</string>
<string name="sync_status_rook_no_longer_exists">Remote file no longer exists; repository link removed. Set a link if you want to sync to a repository.</string>
<string name="sync_status_no_link_and_previous_error">Notebook has no repository link</string>
<string name="sync_status_loaded">Loaded from %s</string>
<string name="sync_status_saved">Saved to %s</string>
</resources>

0 comments on commit f53bf57

Please sign in to comment.