Skip to content

Commit

Permalink
Support backend undo
Browse files Browse the repository at this point in the history
Can be tested by importing an apkg from the deck list, then choosing
the undo option.

- If a backend undo entry is present, it takes priority over the old
undo system to ensure data integrity. AD's reviewer saves config vars and
decks as part of the review process, and it was triggering undo entries
for those changes, which prevent the v2 scheduler review from being undone.
The calls have been updated to clear the backend undo, so that the review
takes precedence. The desktop code has the same problem, and it will be
avoidable once updating to the v3 scheduler.
- Introduce a helper to await a job in unit tests, to avoid race
conditions
  • Loading branch information
dae committed Jul 11, 2022
1 parent 924bca4 commit 590d44c
Show file tree
Hide file tree
Showing 15 changed files with 291 additions and 30 deletions.
51 changes: 40 additions & 11 deletions AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import androidx.annotation.StringRes
import androidx.annotation.VisibleForTesting
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.webkit.WebViewAssetLoader
import anki.collection.OpChanges
import com.afollestad.materialdialogs.MaterialDialog
import com.drakeet.drawer.FullDraggableContainer
import com.google.android.material.snackbar.Snackbar
Expand Down Expand Up @@ -98,6 +99,8 @@ import com.ichi2.utils.HashUtil.HashSetInit
import com.ichi2.utils.KotlinCleanup
import com.ichi2.utils.MaxExecFunction
import com.ichi2.utils.WebViewDebugging.initializeDebugging
import kotlinx.coroutines.Job
import net.ankiweb.rsdroid.BackendFactory
import net.ankiweb.rsdroid.RustCleanup
import timber.log.Timber
import java.io.*
Expand All @@ -120,7 +123,8 @@ abstract class AbstractFlashcardViewer :
TagsDialogListener,
WhiteboardMultiTouchMethods,
AutomaticallyAnswered,
OnPageFinishedCallback {
OnPageFinishedCallback,
ChangeManager.Subscriber {
private var mTtsInitialized = false
private var mReplayOnTtsInit = false
private var mAnkiDroidJsAPI: AnkiDroidJsAPI? = null
Expand Down Expand Up @@ -277,6 +281,10 @@ abstract class AbstractFlashcardViewer :
displayCardAnswer()
}

init {
ChangeManager.subscribe(this)
}

// Event handler for eases (answer buttons)
inner class SelectEaseHandler : View.OnClickListener, OnTouchListener {
private var mPrevCard: Card? = null
Expand Down Expand Up @@ -826,21 +834,33 @@ abstract class AbstractFlashcardViewer :
}
}

open fun undo() {
open fun undo(): Job? {
if (isUndoAvailable) {
val res = resources
val undoName = col.undoName(res)
Undo().runWithHandler(
answerCardHandler(false)
.alsoExecuteAfter {
showThemedToast(
this@AbstractFlashcardViewer,
res.getString(R.string.undo_succeeded, undoName),
true
)
fun legacyUndo() {
Undo().runWithHandler(
answerCardHandler(false)
.alsoExecuteAfter {
showThemedToast(
this@AbstractFlashcardViewer,
res.getString(R.string.undo_succeeded, undoName),
true
)
}
)
}
if (BackendFactory.defaultLegacySchema) {
legacyUndo()
} else {
return launchCatchingCollectionTask { col ->
if (!backendUndoAndShowPopup(col)) {
legacyUndo()
}
)
}
}
}
return null
}

private fun finishNoStorageAvailable() {
Expand Down Expand Up @@ -2559,6 +2579,15 @@ abstract class AbstractFlashcardViewer :
return AnkiDroidJsAPI(this)
}

override fun opExecuted(changes: OpChanges, handler: Any?) {
if ((changes.studyQueues || changes.noteText || changes.card) && handler !== this) {
// executing this only for the refresh side effects; there may be a better way
Undo().runWithHandler(
answerCardHandler(false)
)
}
}

companion object {
/**
* Result codes that are returned when this activity finishes.
Expand Down
47 changes: 47 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/***************************************************************************************
* Copyright (c) 2022 Ankitects Pty Ltd <https://apps.ankiweb.net> *
* *
* This program is free software; you can redistribute it and/or modify it under *
* the terms of the GNU General Public License as published by the Free Software *
* Foundation; either version 3 of the License, or (at your option) any later *
* version. *
* *
* This program is distributed in the hope that it will be useful, but WITHOUT ANY *
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A *
* PARTICULAR PURPOSE. See the GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License along with *
* this program. If not, see <http://www.gnu.org/licenses/>. *
****************************************************************************************/

package com.ichi2.anki

import com.ichi2.anki.UIUtils.showSimpleSnackbar
import com.ichi2.libanki.CollectionV16
import com.ichi2.libanki.undoNew
import com.ichi2.libanki.undoableOp
import net.ankiweb.rsdroid.BackendException

suspend fun AnkiActivity.backendUndoAndShowPopup(col: CollectionV16): Boolean {
return try {
val changes = runInBackgroundWithProgress() {
undoableOp {
col.undoNew()
}
}
showSimpleSnackbar(
this,
col.tr.undoActionUndone(changes.operation),
false
)
true
} catch (exc: BackendException) {
// FIXME: -Backend module should export this as a separate Exception
if (exc.localizedMessage == "UndoEmpty") {
// backend undo queue empty
false
} else {
throw exc
}
}
}
36 changes: 34 additions & 2 deletions AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import androidx.activity.result.contract.ActivityResultContracts.StartActivityFo
import androidx.annotation.CheckResult
import androidx.annotation.VisibleForTesting
import androidx.appcompat.widget.SearchView
import anki.collection.OpChanges
import com.afollestad.materialdialogs.list.SingleChoiceListener
import com.google.android.material.snackbar.Snackbar
import com.ichi2.anim.ActivityTransitionAnimation
Expand Down Expand Up @@ -96,6 +97,7 @@ import com.ichi2.utils.HashUtil.HashMapInit
import com.ichi2.utils.Permissions.hasStorageAccessPermission
import com.ichi2.utils.TagsUtil.getUpdatedTags
import com.ichi2.widget.WidgetStatus.update
import net.ankiweb.rsdroid.BackendFactory
import net.ankiweb.rsdroid.RustCleanup
import timber.log.Timber
import java.lang.Exception
Expand All @@ -112,7 +114,12 @@ import kotlin.math.min
@Suppress("LeakingThis") // The class is only 'open' due to testing
@KotlinCleanup("scan through this class and add attributes - not started")
@KotlinCleanup("Add TextUtils.isNotNullOrEmpty accepting nulls and use it. Remove TextUtils import")
open class CardBrowser : NavigationDrawerActivity(), SubtitleListener, DeckSelectionListener, TagsDialogListener {
open class CardBrowser :
NavigationDrawerActivity(),
SubtitleListener,
DeckSelectionListener,
TagsDialogListener,
ChangeManager.Subscriber {
@KotlinCleanup("using ?. and let keyword would be good here")
override fun onDeckSelected(deck: SelectableDeck?) {
if (deck == null) {
Expand Down Expand Up @@ -255,6 +262,10 @@ open class CardBrowser : NavigationDrawerActivity(), SubtitleListener, DeckSelec
private var mUnmountReceiver: BroadcastReceiver? = null
private val orderSingleChoiceDialogListener: SingleChoiceListener = { _, index: Int, _ -> changeCardOrder(index) }

init {
ChangeManager.subscribe(this)
}

@VisibleForTesting(otherwise = VisibleForTesting.PROTECTED)
fun changeCardOrder(which: Int) {
if (which != mOrder) {
Expand Down Expand Up @@ -1255,7 +1266,15 @@ open class CardBrowser : NavigationDrawerActivity(), SubtitleListener, DeckSelec
@VisibleForTesting
fun onUndo() {
if (col.undoAvailable()) {
Undo().runWithHandler(mUndoHandler)
if (BackendFactory.defaultLegacySchema) {
Undo().runWithHandler(mUndoHandler)
} else {
launchCatchingCollectionTask { col ->
if (!backendUndoAndShowPopup(col)) {
Undo().runWithHandler(mUndoHandler)
}
}
}
}
}

Expand Down Expand Up @@ -2612,6 +2631,19 @@ open class CardBrowser : NavigationDrawerActivity(), SubtitleListener, DeckSelec
searchCards()
}

override fun opExecuted(changes: OpChanges, handler: Any?) {
if ((
changes.browserSidebar ||
changes.browserTable ||
changes.noteText ||
changes.card
) && handler !== this
) {
// executing this only for the refresh side effects; there may be a better way
Undo().runWithHandler(mUndoHandler)
}
}

companion object {
@JvmField
var sCardBrowserCard: Card? = null
Expand Down
20 changes: 19 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ suspend fun <T> Backend.withProgress(

/**
* Run the provided operation in the background, showing a progress
* window.
* window. Progress info is polled from the backend.
*/
suspend fun <T> AnkiActivity.runInBackgroundWithProgress(
backend: Backend,
Expand All @@ -122,6 +122,24 @@ suspend fun <T> AnkiActivity.runInBackgroundWithProgress(
}
}

/**
* Run the provided operation in the background, showing a progress
* window with the provided message.
*/
suspend fun <T> AnkiActivity.runInBackgroundWithProgress(
message: String = "",
op: suspend () -> T
): T = withProgressDialog(
context = this@runInBackgroundWithProgress,
onCancel = null
) { dialog ->
@Suppress("Deprecation") // ProgressDialog deprecation
dialog.setMessage(message)
runInBackground {
op()
}
}

private suspend fun <T> withProgressDialog(
context: AnkiActivity,
onCancel: (() -> Unit)?,
Expand Down
20 changes: 16 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ open class DeckPicker :
MediaCheckDialogListener,
OnRequestPermissionsResultCallback,
CustomStudyListener,
ChangeManager.ChangeSubscriber {
ChangeManager.Subscriber {
// Short animation duration from system
private var mShortAnimDuration = 0
private var mBackButtonPressedToExit = false
Expand Down Expand Up @@ -1222,9 +1222,20 @@ open class DeckPicker :

private fun undo() {
Timber.i("undo()")
val undoReviewString = resources.getString(R.string.undo_action_review)
val isReview = undoReviewString == col.undoName(resources)
Undo().runWithHandler(undoTaskListener(isReview))
fun legacyUndo() {
val undoReviewString = resources.getString(R.string.undo_action_review)
val isReview = undoReviewString == col.undoName(resources)
Undo().runWithHandler(undoTaskListener(isReview))
}
if (BackendFactory.defaultLegacySchema) {
legacyUndo()
} else {
launchCatchingCollectionTask { col ->
if (!backendUndoAndShowPopup(col)) {
legacyUndo()
}
}
}
}

// Show dialogs to deal with database loading issues etc
Expand Down Expand Up @@ -2623,6 +2634,7 @@ open class DeckPicker :

override fun opExecuted(changes: OpChanges, handler: Any?) {
if (changes.studyQueues && handler !== this) {
invalidateOptionsMenu()
updateDeckList()
}
}
Expand Down
73 changes: 73 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/libanki/BackendUndo.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/***************************************************************************************
* Copyright (c) 2022 Ankitects Pty Ltd <https://apps.ankiweb.net> *
* *
* This program is free software; you can redistribute it and/or modify it under *
* the terms of the GNU General Public License as published by the Free Software *
* Foundation; either version 3 of the License, or (at your option) any later *
* version. *
* *
* This program is distributed in the hope that it will be useful, but WITHOUT ANY *
* WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A *
* PARTICULAR PURPOSE. See the GNU General Public License for more details. *
* *
* You should have received a copy of the GNU General Public License along with *
* this program. If not, see <http://www.gnu.org/licenses/>. *
****************************************************************************************/

package com.ichi2.libanki

import anki.collection.OpChangesAfterUndo
import net.ankiweb.rsdroid.RustCleanup
import anki.collection.UndoStatus as UndoStatusProto

/**
* If undo/redo available, a localized string describing the action will be set.
*/
data class UndoStatus(
val undo: String?,
val redo: String?,
// not currently used
val lastStep: Int
) {
companion object {
fun from(proto: UndoStatusProto): UndoStatus {
return UndoStatus(
undo = proto.undo.ifEmpty { null },
redo = proto.redo.ifEmpty { null },
lastStep = proto.lastStep
)
}
}
}

/**
* Undo the last backend operation.
*
* Should be called via collection.op(), which will notify
* [ChangeManager.Subscriber] of the changes.
*
* Will throw if no undo operation is possible (due to legacy code
* directly mutating the database).
*/
@RustCleanup("Once fully migrated, and v2 scheduler dropped, rename to undo()")
fun CollectionV16.undoNew(): OpChangesAfterUndo {
val changes = backend.undo()
// clear legacy undo log
clearUndo()
return changes
}

/** Redoes the previously-undone operation. See the docs for
[CollectionV16.undoOperation]
*/
fun CollectionV16.redo(): OpChangesAfterUndo {
val changes = backend.redo()
// clear legacy undo log
clearUndo()
return changes
}

/** See [UndoStatus] */
fun CollectionV16.undoStatus(): UndoStatus {
return UndoStatus.from(backend.getUndoStatus())
}
8 changes: 4 additions & 4 deletions AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1440,17 +1440,17 @@ open class Collection(
} else null
}

fun undoName(res: Resources?): String {
open fun undoName(res: Resources): String {
val type = undoType()
return type?.name(res!!) ?: ""
return type?.name(res) ?: ""
}

fun undoAvailable(): Boolean {
open fun undoAvailable(): Boolean {
Timber.d("undoAvailable() undo size: %s", undo.size)
return !undo.isEmpty()
}

fun undo(): Card? {
open fun undo(): Card? {
val lastUndo: UndoAction = undo.removeLast()
Timber.d("undo() of type %s", lastUndo.javaClass)
return lastUndo.undo(this)
Expand Down
Loading

0 comments on commit 590d44c

Please sign in to comment.