From b91392f67b9fbb265751aac7c87a7d546f3de557 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 29 Jun 2022 09:48:47 +1000 Subject: [PATCH 01/15] Fix slow browser load in new backend https://github.com/ankidroid/Anki-Android/pull/11644#issuecomment-1169380381 Closes #11733 --- .../main/java/com/ichi2/libanki/CollectionV16.kt | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index b627754326bb..dfb40f4e212c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -16,6 +16,7 @@ package com.ichi2.libanki import android.content.Context +import com.ichi2.async.CancelListener.Companion.isCancelled import com.ichi2.async.CollectionTask import com.ichi2.libanki.backend.* import com.ichi2.libanki.backend.model.toProtoBuf @@ -93,11 +94,15 @@ class CollectionV16( return TemplateManager.TemplateRenderContext.from_existing_card(c, browser).render() } + @RustCleanup("drop the PartialSearch handling in the browse screen, which slows things down") override fun findCards( search: String, order: SortOrder, task: CollectionTask.PartialSearch? ): List { + if (task?.isCancelled() == true) { + return listOf() + } val adjustedOrder = if (order is SortOrder.UseCollectionOrdering) { @Suppress("DEPRECATION") SortOrder.BuiltinSortKind( @@ -112,8 +117,12 @@ class CollectionV16( } catch (e: BackendInvalidInputException) { throw InvalidSearchException(e) } - - task?.doProgress(cardIdsList) + for (id in cardIdsList) { + if (task?.isCancelled() == true) { + break + } + task?.doProgress(listOf(id)) + } return cardIdsList } From 7ed90ff96797c89e080700e3e4dd1cfa423e8719 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 29 Jun 2022 10:05:42 +1000 Subject: [PATCH 02/15] Fix DB check in new backend https://github.com/ankidroid/Anki-Android/pull/11644#issuecomment-1169380381 Closes #11735 --- .../main/java/com/ichi2/anki/DatabaseCheck.kt | 46 +++++++++++++++++++ .../main/java/com/ichi2/anki/DeckPicker.kt | 6 ++- .../java/com/ichi2/libanki/CollectionV16.kt | 6 +++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt new file mode 100644 index 000000000000..b5b0ba31e022 --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt @@ -0,0 +1,46 @@ +/*************************************************************************************** + * Copyright (c) 2022 Ankitects Pty Ltd * + * * + * 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 . * + ****************************************************************************************/ + +package com.ichi2.anki + +import timber.log.Timber + +fun DeckPicker.handleDatabaseCheck() { + val col = CollectionHelper.getInstance().getCol(baseContext).newBackend + val deckPicker = this + catchingLifecycleScope(this) { + val problems = runInBackgroundWithProgress(col, { + if (it.hasDatabaseCheck()) { + // TODO: show progress in GUI + it.databaseCheck.run { + if (stageTotal > 0) { + Timber.i("$stage: $stageCurrent/$stageTotal") + } else { + Timber.i("$stage") + } + } + } + }) { + col.fixIntegrity() + } + val message = if (problems.isNotEmpty()) { + problems.joinToString("\n") + } else { + col.tr.databaseCheckRebuilt() + } + deckPicker.showSimpleMessageDialog(message) + } +} diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 3d7b7edcff9d..e808ef56b91c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -1370,7 +1370,11 @@ open class DeckPicker : private fun performIntegrityCheck() { Timber.i("performIntegrityCheck()") - TaskManager.launchCollectionTask(CheckDatabase(), CheckDatabaseListener()) + if (BackendFactory.defaultLegacySchema) { + TaskManager.launchCollectionTask(CheckDatabase(), CheckDatabaseListener()) + } else { + handleDatabaseCheck() + } } private fun mediaCheckListener(): MediaCheckListener { diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index dfb40f4e212c..0d73fef29275 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -130,4 +130,10 @@ class CollectionV16( fun i18nResourcesRaw(input: ByteArray): ByteArray { return backend.i18nResourcesRaw(input = input) } + + /** Fixes and optimizes the database. If any errors are encountered, a list of + * problems is returned. Throws if DB is unreadable. */ + fun fixIntegrity(): List { + return backend.checkDatabase() + } } From d7a5acbc990d218b17f3744c877416ecbdbea509 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 21 Jul 2022 16:10:49 +1000 Subject: [PATCH 03/15] Fix full sync not being forced in new backend Closes #11732 --- AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt | 2 +- .../src/main/java/com/ichi2/libanki/CollectionV16.kt | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index f5619dee0cca..b0f9ce5ce94a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -479,7 +479,7 @@ open class Collection( * is used so that the type does not states that an exception is * thrown when in fact it is never thrown. */ - fun modSchemaNoCheck() { + open fun modSchemaNoCheck() { scm = TimeManager.time.intTimeMS() setMod() } diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index 0d73fef29275..7921e5c32302 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -21,6 +21,7 @@ import com.ichi2.async.CollectionTask import com.ichi2.libanki.backend.* import com.ichi2.libanki.backend.model.toProtoBuf import com.ichi2.libanki.exception.InvalidSearchException +import com.ichi2.libanki.utils.TimeManager import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendInvalidInputException @@ -136,4 +137,12 @@ class CollectionV16( fun fixIntegrity(): List { return backend.checkDatabase() } + + override fun modSchemaNoCheck() { + db.execute( + "update col set scm=?, mod=?", + TimeManager.time.intTimeMS(), + TimeManager.time.intTimeMS() + ) + } } From 9ca9f0c4aeb57a26f6bffa4b4ca3608e55a69a1b Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 30 Jun 2022 13:55:46 +1000 Subject: [PATCH 04/15] CollectionV16.load() doesn't need to read from col table scm/ls are fetched on demand now, since backend methods may mutate them. --- .../main/java/com/ichi2/libanki/Collection.kt | 11 ++++++----- .../java/com/ichi2/libanki/CollectionV16.kt | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index b0f9ce5ce94a..0cb193b7ad0d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -135,7 +135,7 @@ open class Collection( "move accessor methods here, maybe reconsider return type." + "See variable: conf" ) - private var _config: ConfigManager? = null + protected var _config: ConfigManager? = null @KotlinCleanup("see if we can inline a function inside init {} and make this `val`") lateinit var sched: AbstractSched @@ -147,7 +147,8 @@ open class Collection( // BEGIN: SQL table columns open var crt: Long = 0 open var mod: Long = 0 - var scm: Long = 0 + open var scm: Long = 0 + @RustCleanup("remove") var dirty: Boolean = false private var mUsn = 0 private var mLs: Long = 0 @@ -278,7 +279,7 @@ open class Collection( * DB-related *************************************************************** ******************************** */ @KotlinCleanup("Cleanup: make cursor a val + move cursor and cursor.close() to the try block") - fun load() { + open fun load() { var cursor: Cursor? = null var deckConf: String? try { @@ -504,12 +505,12 @@ open class Collection( } /** True if schema changed since last sync. */ - fun schemaChanged(): Boolean { + open fun schemaChanged(): Boolean { return scm > mLs } @KotlinCleanup("maybe change to getter") - fun usn(): Int { + open fun usn(): Int { return if (server) { mUsn } else { diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index 7921e5c32302..169b967dac9e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -60,6 +60,11 @@ class CollectionV16( override val newMedia: BackendMedia get() = this.media as BackendMedia + override fun load() { + _config = initConf(null) + decks = initDecks(null) + } + override fun flush(mod: Long) { // no-op } @@ -70,6 +75,20 @@ class CollectionV16( override var crt: Long = 0 get() = db.queryLongScalar("select crt from col") + override var scm: Long = 0 + get() = db.queryLongScalar("select scm from col") + + var lastSync: Long = 0 + get() = db.queryLongScalar("select ls from col") + + override fun usn(): Int { + return -1 + } + + override fun schemaChanged(): Boolean { + return scm > lastSync + } + /** col.conf is now unused, handled by [ConfigV16] which has a separate table */ override fun flushConf(): Boolean = false From 7e2a9646daa4290b076b4c9014aedba893e85105 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 29 Jun 2022 12:51:00 +1000 Subject: [PATCH 05/15] Remove TagsBackend/RustTagsBackend indirection Follow-up of https://github.com/ankidroid/Anki-Android/commit/b02515b0f6a403a4e9f278d21620a1b07d0b1232 --- .../main/java/com/ichi2/libanki/Collection.kt | 2 + .../java/com/ichi2/libanki/CollectionV16.kt | 5 +- .../main/java/com/ichi2/libanki/TagManager.kt | 19 +----- .../main/java/com/ichi2/libanki/TagsV16.kt | 68 +++++-------------- .../ichi2/libanki/backend/RustTagsBackend.kt | 37 ---------- .../com/ichi2/libanki/backend/TagsBackend.kt | 25 ------- .../test/java/com/ichi2/libanki/FinderTest.kt | 2 +- 7 files changed, 26 insertions(+), 132 deletions(-) delete mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustTagsBackend.kt delete mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/backend/TagsBackend.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index 0cb193b7ad0d..5122d549dcc8 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -101,6 +101,8 @@ open class Collection( get() = throw Exception("invalid call to newBackend on old backend") open val newMedia: BackendMedia get() = throw Exception("invalid call to newMedia on old backend") + open val newTags: TagsV16 + get() = throw Exception("invalid call to newTags on old backend") @VisibleForTesting(otherwise = VisibleForTesting.NONE) fun debugEnsureNoOpenPointers() { diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index 169b967dac9e..d82d5dae691f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -35,7 +35,7 @@ class CollectionV16( ) : Collection(context, path, server, log, backend) { override fun initTags(): TagManager { - return TagsV16(this, RustTagsBackend(backend)) + return TagsV16(this) } override fun initDecks(deckConf: String?): DeckManager { @@ -60,6 +60,9 @@ class CollectionV16( override val newMedia: BackendMedia get() = this.media as BackendMedia + override val newTags: TagsV16 + get() = this.tags as TagsV16 + override fun load() { _config = initConf(null) decks = initDecks(null) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/TagManager.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/TagManager.kt index bcc78feff89c..8c90641241cd 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/TagManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/TagManager.kt @@ -74,24 +74,9 @@ abstract class TagManager { * *********************************************************** */ - /** - * FIXME: This method must be fixed before it is used. See note below. - * Add/remove tags in bulk. TAGS is space-separated. - * - * @param ids The cards to tag. - * @param tags List of tags to add/remove. They are space-separated. - */ - fun bulkAdd(ids: List, tags: String) = bulkAdd(ids, tags, true) - /** - * FIXME: This method must be fixed before it is used. Its behaviour is currently incorrect. - * This method is currently unused in AnkiDroid so it will not cause any errors in its current state. - * - * @param ids The cards to tag. - * @param tags List of tags to add/remove. They are space-separated. - * @param add True/False to add/remove. - */ + /* Legacy signature, currently only used by unit tests. New code in TagsV16 + takes two args. */ abstract fun bulkAdd(ids: List, tags: String, add: Boolean = true) - fun bulkRem(ids: List, tags: String) = bulkAdd(ids, tags, false) /* * String-based utilities diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/TagsV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/TagsV16.kt index 50dcbd0db360..b978fb92356d 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/TagsV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/TagsV16.kt @@ -24,12 +24,13 @@ package com.ichi2.libanki +import anki.collection.OpChangesWithCount import com.ichi2.libanki.Utils.ids2str -import com.ichi2.libanki.backend.TagsBackend import com.ichi2.libanki.backend.model.TagUsnTuple import com.ichi2.libanki.utils.join import com.ichi2.libanki.utils.list import com.ichi2.libanki.utils.set +import net.ankiweb.rsdroid.RustCleanup import java.util.regex.Pattern /** @@ -38,10 +39,10 @@ import java.util.regex.Pattern * tracked, so unused tags can only be removed from the list with a DB check. * This module manages the tag cache and tags for notes. */ -class TagsV16(val col: Collection, private val backend: TagsBackend) : TagManager() { +class TagsV16(val col: CollectionV16) : TagManager() { /** all tags */ - override fun all(): List = backend.all_tags() + override fun all(): List = col.backend.allTags() /** List of (tag, usn) */ override fun allItems(): List { @@ -58,44 +59,15 @@ class TagsV16(val col: Collection, private val backend: TagsBackend) : TagManage usn: Int?, clear_first: Boolean ) { - - val preserve_usn: Boolean - val usn_: Int - if (usn == null) { - preserve_usn = false - usn_ = 0 - } else { - usn_ = usn - preserve_usn = true - } - backend.register_tags( - tags = " ".join(tags), preserve_usn = preserve_usn, usn = usn_, clear_first = clear_first - ) + TODO("no longer in backend") } /** Add any missing tags from notes to the tags list. */ override fun registerNotes(nids: kotlin.collections.Collection?) { - - // when called without an argument, the old list is cleared first. - val clear: Boolean - val lim: String - if (nids != null) { - lim = " where id in " + ids2str(nids) - clear = false - } else { - lim = "" - clear = true - } - register( - set( - split( - " ".join(col.db.queryStringList("select distinct tags from notes" + lim)) - ) - ), - clear_first = clear, - ) + TODO("no longer in backend") } + @RustCleanup("remove after migrating to backend custom study code") override fun byDeck(did: Long, children: Boolean): List { val basequery = "select n.tags from cards c, notes n WHERE c.nid = n.id" val query: String @@ -119,31 +91,25 @@ class TagsV16(val col: Collection, private val backend: TagsBackend) : TagManage ############################################################# */ - /** Add space-separate tags to provided notes, - * @return changed count. */ - fun bulk_add(nids: List, tags: String): Int { - return backend.add_note_tags(nids = nids, tags = tags) + /** Add space-separate tags to provided notes. */ + fun bulkAdd(noteIds: List, tags: String): OpChangesWithCount { + return col.backend.addNoteTags(noteIds = noteIds, tags = tags) } - /** Replace space-separated tags, returning changed count. - * Tags replaced with an empty string will be removed. - * @return changed count. - */ + /* Remove space-separated tags from provided notes. */ fun bulkRemove( - nids: List, + noteIds: List, tags: String, - ): Int { - return backend.remove_note_tags( - nids = nids, tags = tags + ): OpChangesWithCount { + return col.backend.removeNoteTags( + noteIds = noteIds, tags = tags ) } - // legacy routines - - /** Add tags in bulk. TAGS is space-separated. */ + /* Legacy signature, used by unit tests. */ override fun bulkAdd(ids: List, tags: String, add: Boolean) { if (add) { - bulk_add(ids, tags) + bulkAdd(ids, tags) } else { bulkRemove(ids, tags) } diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustTagsBackend.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustTagsBackend.kt deleted file mode 100644 index 886c539ec20d..000000000000 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustTagsBackend.kt +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (c) 2021 David Allison - * - * 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 . - */ - -package com.ichi2.libanki.backend - -import net.ankiweb.rsdroid.Backend - -class RustTagsBackend(val backend: Backend) : TagsBackend { - override fun all_tags(): List { - return backend.allTags() - } - - override fun register_tags(tags: String, preserve_usn: Boolean, usn: Int, clear_first: Boolean) { - TODO("no longer in backend") - } - - override fun remove_note_tags(nids: List, tags: String): Int { - return backend.removeNoteTags(nids, tags).count - } - - override fun add_note_tags(nids: List, tags: String): Int { - return backend.addNoteTags(nids, tags).count - } -} diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/TagsBackend.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/TagsBackend.kt deleted file mode 100644 index c42f4a7a958b..000000000000 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/TagsBackend.kt +++ /dev/null @@ -1,25 +0,0 @@ -/* - * Copyright (c) 2021 David Allison - * - * 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 . - */ - -package com.ichi2.libanki.backend - -interface TagsBackend { - fun all_tags(): List - fun register_tags(tags: String, preserve_usn: Boolean, usn: Int, clear_first: Boolean) - fun remove_note_tags(nids: List, tags: String): Int - /** @return changed count. */ - fun add_note_tags(nids: List, tags: String): Int -} diff --git a/AnkiDroid/src/test/java/com/ichi2/libanki/FinderTest.kt b/AnkiDroid/src/test/java/com/ichi2/libanki/FinderTest.kt index bb02b908132e..203364b606b7 100644 --- a/AnkiDroid/src/test/java/com/ichi2/libanki/FinderTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/libanki/FinderTest.kt @@ -165,7 +165,7 @@ class FinderTest : RobolectricTest() { col.tags.bulkAdd(col.db.queryLongList("select id from notes"), "foo bar") assertEquals(5, col.findCards("tag:foo").size) assertEquals(5, col.findCards("tag:bar").size) - col.tags.bulkRem(col.db.queryLongList("select id from notes"), "foo") + col.tags.bulkAdd(col.db.queryLongList("select id from notes"), "foo", add = false) assertEquals(0, col.findCards("tag:foo").size) assertEquals(5, col.findCards("tag:bar").size) // text searches From 3d8dbc4df145a0aa6b36bfcd8b9d8efde8613cb0 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 29 Jun 2022 15:56:31 +1000 Subject: [PATCH 06/15] Remove ModelsBackend/ModelsBackendImpl indirection Follow-up of https://github.com/ankidroid/Anki-Android/commit/b02515b0f6a403a4e9f278d21620a1b07d0b1232 + Convert Optional references to native Kotlin nullable types + Remove some methods that have been replaced with a new API --- .../main/java/com/ichi2/libanki/Collection.kt | 5 + .../java/com/ichi2/libanki/CollectionV16.kt | 5 +- .../main/java/com/ichi2/libanki/ModelsV16.kt | 224 ++++++------------ .../ichi2/libanki/backend/ModelsBackend.kt | 103 -------- .../java/com/ichi2/libanki/ModelTest.java | 4 + 5 files changed, 81 insertions(+), 260 deletions(-) delete mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/backend/ModelsBackend.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index 5122d549dcc8..a58d7dd39121 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -99,11 +99,16 @@ open class Collection( open val newBackend: CollectionV16 get() = throw Exception("invalid call to newBackend on old backend") + open val newMedia: BackendMedia get() = throw Exception("invalid call to newMedia on old backend") + open val newTags: TagsV16 get() = throw Exception("invalid call to newTags on old backend") + open val newModels: ModelsV16 + get() = throw Exception("invalid call to newModels on old backend") + @VisibleForTesting(otherwise = VisibleForTesting.NONE) fun debugEnsureNoOpenPointers() { val result = backend.getActiveSequenceNumbers() diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index d82d5dae691f..be9f15091e38 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -43,7 +43,7 @@ class CollectionV16( } override fun initModels(): ModelManager { - return ModelsV16(this, backend) + return ModelsV16(this) } override fun initConf(conf: String?): ConfigManager { @@ -63,6 +63,9 @@ class CollectionV16( override val newTags: TagsV16 get() = this.tags as TagsV16 + override val newModels: ModelsV16 + get() = this.models as ModelsV16 + override fun load() { _config = initConf(null) decks = initDecks(null) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt index 19000f52d6ea..14f54a3df79e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt @@ -25,23 +25,21 @@ package com.ichi2.libanki +import anki.notetypes.StockNotetype import com.ichi2.anki.R -import com.ichi2.anki.exception.ConfirmModSchemaException -import com.ichi2.libanki.Consts.MODEL_CLOZE import com.ichi2.libanki.Utils.* -import com.ichi2.libanki.backend.ModelsBackend -import com.ichi2.libanki.backend.ModelsBackendImpl -import com.ichi2.libanki.backend.NoteTypeNameID -import com.ichi2.libanki.backend.NoteTypeNameIDUseCount +import com.ichi2.libanki.backend.* +import com.ichi2.libanki.backend.BackendUtils.to_json_bytes import com.ichi2.libanki.utils.* import com.ichi2.utils.JSONArray import com.ichi2.utils.JSONObject -import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendNotFoundException import timber.log.Timber -import java.util.* -import kotlin.collections.HashMap + +class NoteTypeNameID(val name: String, val id: ntid) +class NoteTypeNameIDUseCount(val id: Long, val name: String, val useCount: UInt) +class BackendNote(val fields: MutableList) private typealias int = Long // # types @@ -102,14 +100,13 @@ var NoteType.type: Int put("type", value) } -class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { +class ModelsV16(col: CollectionV16) : ModelManager(col) { /* # Saving/loading registry ############################################################# */ private var _cache: Dict = Dict() - private val modelsBackend: ModelsBackend = ModelsBackendImpl(backend) init { _cache = Dict() @@ -157,8 +154,8 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { _cache.remove(ntid) } - private fun _get_cached(ntid: int): Optional { - return _cache.getOptional(ntid) + private fun _get_cached(ntid: int): NoteType? { + return _cache.get(ntid) } fun _clear_cache() { @@ -171,11 +168,15 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { */ fun all_names_and_ids(): Sequence { - return modelsBackend.get_notetype_names() + return col.backend.getNotetypeNames().map { + NoteTypeNameID(it.name, it.id) + }.asSequence() } fun all_use_counts(): Sequence { - return modelsBackend.get_notetype_names_and_counts() + return col.backend.getNotetypeNamesAndCounts().map { + NoteTypeNameIDUseCount(it.id, it.name, it.useCount.toUInt()) + }.asSequence() } /* legacy */ @@ -218,11 +219,11 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { ############################################################# */ - fun id_for_name(name: str): Optional { - try { - return modelsBackend.get_notetype_id_by_name(name) + fun id_for_name(name: str): Long? { + return try { + col.backend.getNotetypeIdByName(name) } catch (e: BackendNotFoundException) { - return Optional.empty() + null } } @@ -237,15 +238,19 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { return null } var nt = _get_cached(id) - if (!nt.isPresent) { + if (nt == null) { try { - nt = Optional.of(modelsBackend.get_notetype_legacy(id)) - _update_cache(nt.get()) + nt = NoteType( + BackendUtils.from_json_bytes( + col.backend.getNotetypeLegacy(id) + ) + ) + _update_cache(nt) } catch (e: BackendNotFoundException) { return null } } - return nt.orElse(null) + return nt } /** Get all models */ @@ -256,26 +261,30 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { /** Get model with NAME. */ override fun byName(name: str): NoteType? { val id = id_for_name(name) - if (id.isPresent) { - return get(id.get()) - } else { - return null - } + return id?.let { get(it) } } @RustCleanup("When we're kotlin only, rename to 'new', name existed due to Java compat") override fun newModel(name: str): NoteType = new(name) - /** Create a new model, and return it. */ + /** Create a new non-cloze model, and return it. */ fun new(name: str): NoteType { // caller should call save() after modifying - val nt = modelsBackend.get_stock_notetype_legacy() + val nt = newBasicNotetype() nt.flds = JSONArray() nt.tmpls = JSONArray() nt.name = name return nt } + private fun newBasicNotetype(): NoteType { + return NoteType( + BackendUtils.from_json_bytes( + col.backend.getStockNotetypeLegacy(StockNotetype.Kind.BASIC) + ) + ) + } + /** Delete model, and all its cards/notes. */ override fun rem(m: NoteType) { remove(m.id) @@ -284,29 +293,27 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { fun remove_all_notetypes() { for (nt in all_names_and_ids()) { _remove_from_cache(nt.id) - modelsBackend.remove_notetype(nt.id) + col.backend.removeNotetype(nt.id) } } /** Modifies schema. */ fun remove(id: int) { _remove_from_cache(id) - modelsBackend.remove_notetype(id) + col.backend.removeNotetype(id) } override fun add(m: NoteType) { save(m) } - @RustCleanup("Python uses .time()") fun ensureNameUnique(m: NoteType) { - val existing_id = id_for_name(m.name) - if (existing_id.isPresent && existing_id.get() != m.id) { - /* - >>> pp(anki.utils.checksum(str(time.time()))[:5]) = '07a29' - >>> pp(anki.utils.checksum(str(time.time()))) = '07a2939b5546263476ba9c7eca7489fa95af4a18' - */ - m.name += "-" + checksum(TimeManager.time.intTimeMS().toString()).substring(0, 5) + val existingId = id_for_name(m.name) + existingId?.let { + if (it != m.id) { + // Python uses a float time, but it doesn't really matter, the goal is just a random id. + m.name += "-" + checksum(TimeManager.time.intTimeMS().toString()).substring(0, 5) + } } } @@ -314,7 +321,11 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { override fun update(m: NoteType, preserve_usn_and_mtime: Boolean) { _remove_from_cache(m.id) ensureNameUnique(m) - m.id = modelsBackend.add_or_update_notetype(model = m, preserve_usn_and_mtime = preserve_usn_and_mtime) + m.id = col.backend.addOrUpdateNotetype( + json = to_json_bytes(m), + preserveUsnAndMtime = preserve_usn_and_mtime, + skipChecks = preserve_usn_and_mtime + ) setCurrent(m) _mutate_after_write(m) } @@ -346,7 +357,11 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { @RustCleanup("not in libAnki any more - may not be needed") override fun tmplUseCount(m: NoteType, ord: Int): Int { - return col.db.queryScalar("select count() from cards, notes where cards.nid = notes.id and notes.mid = ? and cards.ord = ?", m.id, ord) + return col.db.queryScalar( + "select count() from cards, notes where cards.nid = notes.id and notes.mid = ? and cards.ord = ?", + m.id, + ord + ) } /* @@ -370,8 +385,7 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { /** Mapping of field name : (ord, field). */ fun fieldMap(m: NoteType): Map> { - return m.flds.jsonObjectIterable().map { - f -> + return m.flds.jsonObjectIterable().map { f -> Pair(f.getString("name"), Pair(f.getLong("ord"), f)) }.toMap() } @@ -391,7 +405,7 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { @RustCleanup("Check JSONObject.NULL") fun new_field(name: str): Field { - val nt = modelsBackend.get_stock_notetype_legacy() + val nt = newBasicNotetype() val field = nt.flds.getJSONObject(0) field.put("name", name) field.put("ord", JSONObject.NULL) @@ -483,7 +497,7 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { @RustCleanup("Check JSONObject.NULL") fun new_template(name: str): Template { - val nt = modelsBackend.get_stock_notetype_legacy() + val nt = newBasicNotetype() val template = nt.tmpls.getJSONObject(0) template["name"] = name template["qfmt"] = "" @@ -534,20 +548,6 @@ class ModelsV16(col: Collection, backend: Backend) : ModelManager(col) { save(m) } - override fun change(m: NoteType, nid: Long, newModel: NoteType, fmap: Map?, cmap: Map?) { - change(m, listOf(nid), newModel, Optional.ofNullable(fmap), Optional.ofNullable(cmap)) - } - - fun template_use_count(ntid: int, ord: int): int { - return col.db.queryLongScalar( - """ -select count() from cards, notes where cards.nid = notes.id -and notes.mid = ? and cards.ord = ?""", - ntid, - ord, - ) - } - /* # Model changing ########################################################################## @@ -555,103 +555,14 @@ and notes.mid = ? and cards.ord = ?""", # - newModel should be self if model is not changing */ - @Throws(ConfirmModSchemaException::class) - fun change( + override fun change( m: NoteType, - nids: List, + nid: Long, newModel: NoteType, - fmap: Optional>, - cmap: Optional>, + fmap: Map?, + cmap: Map? ) { - col.modSchema() - assert(newModel.id == m.id || (fmap.isPresent && cmap.isPresent)) - if (fmap.isPresent) { - _changeNotes(nids, newModel, fmap.get()) - } - if (cmap.isPresent) { - _changeCards(nids, m, newModel, cmap.get()) - } - modelsBackend.after_note_updates(nids, mark_modified = true) - } - - private fun _changeNotes(nids: List, newModel: NoteType, map: Map) { - val d = mutableListOf>() - - val cursor = col.db.query("select id, flds from notes where id in " + ids2str(nids)) - cursor.use { - while (cursor.moveToNext()) { - val nid = cursor.getLong(0) - val fldsString = cursor.getString(1) - - var flds = splitFields(fldsString) - // Kotlin: we can't expand a list via index, so use a HashMap - val newflds = HashMap() - for ((old, new) in list(map.entries)) { - if (new == null) { - continue - } - newflds[new] = flds[old] - } - flds = Array(flds.size) { "" } - newflds.forEach { - (i, fld) -> - flds[i] = fld - } - val fldsAsString = joinFields(flds) - d.append( - arrayOf( - fldsAsString, - newModel.id, - TimeManager.time.intTime(), - col.usn(), - nid, - ) - ) - } - } - col.db.executeMany("update notes set flds=?,mid=?,mod=?,usn=? where id = ?", d) - } - - private fun _changeCards( - nids: List, - oldModel: NoteType, - newModel: NoteType, - map: Map, - ) { - val d = mutableListOf>() - val deleted = mutableListOf() - val c = col.db.query( - "select id, ord from cards where nid in " + ids2str(nids) - ) - c.use { - while (c.moveToNext()) { - val cid = c.getLong(0) - val ord = c.getInt(1) - // if the src model is a cloze, we ignore the map, as the gui - // doesn't currently support mapping them - var new: Int? - if (oldModel.type == MODEL_CLOZE) { - new = ord - if (newModel.type != MODEL_CLOZE) { - // if we're mapping to a regular note, we need to check if - // the destination ord is valid - if (len(newModel.tmpls) <= ord) { - new = null - } - } - } else { - // mapping from a regular note, so the map should be valid - new = map[ord] - } - if (new != null) { - d.append(arrayOf(new, col.usn(), TimeManager.time.intTime(), cid)) - } else { - deleted.append(cid) - } - } - } - col.db.executeMany("update cards set ord=?,usn=?,mod=? where id=?", d) - modelsBackend.remove_cards_and_orphaned_notes(deleted) + TODO("backend provides new API") } /* @@ -682,8 +593,9 @@ and notes.mid = ? and cards.ord = ?""", flds: str, allowEmpty: bool = true ): kotlin.collections.Collection { - print("_availClozeOrds() is deprecated; use note.cloze_numbers_in_fields()") - return modelsBackend.cloze_numbers_in_note(listOf(flds)) + TODO("should no longer be needed") +// print("_availClozeOrds() is deprecated; use note.cloze_numbers_in_fields()") +// return modelsBackend.cloze_numbers_in_note(listOf(flds)) } /* diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/ModelsBackend.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/ModelsBackend.kt deleted file mode 100644 index 170cfb737471..000000000000 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/ModelsBackend.kt +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright (c) 2021 David Allison - * - * 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 . - */ - -@file:Suppress("FunctionName") - -package com.ichi2.libanki.backend - -import android.content.res.Resources -import com.ichi2.libanki.NoteType -import com.ichi2.libanki.backend.BackendUtils.from_json_bytes -import com.ichi2.libanki.backend.BackendUtils.to_json_bytes -import net.ankiweb.rsdroid.Backend -import net.ankiweb.rsdroid.RustCleanup -import java.util.* - -private typealias ntid = Long - -class NoteTypeNameID(val name: String, val id: ntid) -class NoteTypeNameIDUseCount(val id: Long, val name: String, val useCount: UInt) -class BackendNote(val fields: MutableList) - -interface ModelsBackend { - fun get_notetype_names(): Sequence - fun get_notetype_names_and_counts(): Sequence - fun get_notetype_legacy(id: Long): NoteType - fun get_notetype_id_by_name(name: String): Optional - fun get_stock_notetype_legacy(): NoteType - fun cloze_numbers_in_note(flds: List): List - fun remove_notetype(id: ntid) - fun add_or_update_notetype(model: NoteType, preserve_usn_and_mtime: Boolean): Long - @RustCleanup("This should be in col") - fun after_note_updates(nids: List, mark_modified: Boolean, generate_cards: Boolean = true) - @RustCleanup("This should be in col") - /** "You probably want .remove_notes_by_card() instead." */ - fun remove_cards_and_orphaned_notes(card_ids: List) -} - -@Suppress("unused") -class ModelsBackendImpl(private val backend: Backend) : ModelsBackend { - override fun get_notetype_names(): Sequence { - return backend.getNotetypeNames().map { - NoteTypeNameID(it.name, it.id) - }.asSequence() - } - - override fun get_notetype_names_and_counts(): Sequence { - return backend.getNotetypeNamesAndCounts().map { - NoteTypeNameIDUseCount(it.id, it.name, it.useCount.toUInt()) - }.asSequence() - } - - override fun get_notetype_legacy(id: Long): NoteType { - return NoteType(from_json_bytes(backend.getNotetypeLegacy(id))) - } - - override fun get_notetype_id_by_name(name: String): Optional { - return try { - Optional.of(backend.getNotetypeIdByName(name)) - } catch (ex: Resources.NotFoundException) { - Optional.empty() - } - } - - override fun get_stock_notetype_legacy(): NoteType { - val fromJsonBytes = from_json_bytes(backend.getStockNotetypeLegacy(anki.notetypes.StockNotetype.Kind.BASIC)) - return NoteType(fromJsonBytes) - } - - override fun cloze_numbers_in_note(flds: List): List { - val note = anki.notes.Note.newBuilder().addAllFields(flds).build() - return backend.clozeNumbersInNote(note) - } - - override fun remove_notetype(id: ntid) { - backend.removeNotetype(id) - } - - override fun add_or_update_notetype(model: NoteType, preserve_usn_and_mtime: Boolean): ntid { - val toJsonBytes = to_json_bytes(model) - return backend.addOrUpdateNotetype(toJsonBytes, preserve_usn_and_mtime, preserve_usn_and_mtime) - } - - override fun after_note_updates(nids: List, mark_modified: Boolean, generate_cards: Boolean) { - backend.afterNoteUpdates(nids, mark_modified, generate_cards) - } - - override fun remove_cards_and_orphaned_notes(card_ids: List) { - backend.removeCards(card_ids) - } -} diff --git a/AnkiDroid/src/test/java/com/ichi2/libanki/ModelTest.java b/AnkiDroid/src/test/java/com/ichi2/libanki/ModelTest.java index f2095c2fa06f..d882cc6648b8 100644 --- a/AnkiDroid/src/test/java/com/ichi2/libanki/ModelTest.java +++ b/AnkiDroid/src/test/java/com/ichi2/libanki/ModelTest.java @@ -427,6 +427,10 @@ public void test_chained_mods() throws ConfirmModSchemaException { @Test public void test_modelChange() throws ConfirmModSchemaException { + if (!BackendFactory.getDefaultLegacySchema()) { + // backend provides different API with TypeScript frontend + return; + } Collection col = getCol(); Model cloze = col.getModels().byName("Cloze"); // enable second template and add a note From dc64215e450e5a838bdc3cffeb2ae0714e6d18fb Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 29 Jun 2022 16:49:57 +1000 Subject: [PATCH 07/15] Remove DecksBackend/RustDroidDeckBackend indirection Follow-up of https://github.com/ankidroid/Anki-Android/commit/b02515b0f6a403a4e9f278d21620a1b07d0b1232 + Convert Optional references to native Kotlin nullable types --- .../main/java/com/ichi2/libanki/Collection.kt | 3 + .../java/com/ichi2/libanki/CollectionV16.kt | 5 +- .../main/java/com/ichi2/libanki/DecksV16.kt | 248 ++++++++++-------- .../com/ichi2/libanki/backend/DecksBackend.kt | 184 ------------- 4 files changed, 152 insertions(+), 288 deletions(-) delete mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/backend/DecksBackend.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index a58d7dd39121..6c5fde5f5c4b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -109,6 +109,9 @@ open class Collection( open val newModels: ModelsV16 get() = throw Exception("invalid call to newModels on old backend") + open val newDecks: DecksV16 + get() = throw Exception("invalid call to newDecks on old backend") + @VisibleForTesting(otherwise = VisibleForTesting.NONE) fun debugEnsureNoOpenPointers() { val result = backend.getActiveSequenceNumbers() diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index be9f15091e38..161018589563 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -39,7 +39,7 @@ class CollectionV16( } override fun initDecks(deckConf: String?): DeckManager { - return DecksV16(this, RustDroidDeckBackend(backend)) + return DecksV16(this) } override fun initModels(): ModelManager { @@ -66,6 +66,9 @@ class CollectionV16( override val newModels: ModelsV16 get() = this.models as ModelsV16 + override val newDecks: DecksV16 + get() = this.decks as DecksV16 + override fun load() { _config = initConf(null) decks = initDecks(null) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt index f7d5c7882cfe..bea31e111624 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt @@ -24,18 +24,25 @@ */ @file:Suppress( - "RedundantIf", "LiftReturnOrAssignment", "MemberVisibilityCanBePrivate", "FunctionName", "ConvertToStringTemplate", "LocalVariableName", - "NonPublicNonStaticFieldName", "ConstantFieldName" + "RedundantIf", + "LiftReturnOrAssignment", + "MemberVisibilityCanBePrivate", + "FunctionName", + "ConvertToStringTemplate", + "LocalVariableName", + "NonPublicNonStaticFieldName", + "ConstantFieldName" ) package com.ichi2.libanki +import anki.collection.OpChangesWithCount +import anki.collection.OpChangesWithId +import com.google.protobuf.ByteString import com.ichi2.libanki.Decks.ACTIVE_DECKS import com.ichi2.libanki.Decks.CURRENT_DECK import com.ichi2.libanki.Utils.ids2str -import com.ichi2.libanki.backend.DeckNameId -import com.ichi2.libanki.backend.DeckTreeNode -import com.ichi2.libanki.backend.DecksBackend +import com.ichi2.libanki.backend.BackendUtils import com.ichi2.libanki.backend.exception.DeckRenameException import com.ichi2.libanki.utils.* import com.ichi2.utils.CollectionUtils @@ -43,9 +50,25 @@ import com.ichi2.utils.JSONArray import com.ichi2.utils.JSONObject import java8.util.Optional import net.ankiweb.rsdroid.RustCleanup +import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException +import net.ankiweb.rsdroid.exceptions.BackendNotFoundException import timber.log.Timber import java.util.* +data class DeckNameId(val name: String, val id: did) + +data class DeckTreeNode( + val deck_id: Long, + val name: String, + val children: List, + val level: UInt, + val collapsed: Boolean, + val review_count: UInt, + val learn_count: UInt, + val new_count: UInt, + val filtered: Boolean +) + // legacy code may pass this in as the type argument to .id() const val defaultDeck = 0 const val defaultDynamicDeck = 1 @@ -178,7 +201,8 @@ private typealias childMapNode = Dict * Afterwards, we can finish up the implementations, run our tests, and use this with a V16 * collection, using decks as a separate table */ -class DecksV16(private val col: Collection, private val decksBackend: DecksBackend) : DeckManager() { +class DecksV16(private val col: CollectionV16) : + DeckManager() { /* Registry save/load */ @@ -215,7 +239,10 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke } @RustCleanup("unused in V16") - override fun load(@Suppress("UNUSED_PARAMETER") decks: String, @Suppress("UNUSED_PARAMETER") dconf: String) { + override fun load( + @Suppress("UNUSED_PARAMETER") decks: String, + @Suppress("UNUSED_PARAMETER") dconf: String + ) { } // legacy @@ -248,14 +275,26 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke val deck = this.new_deck_legacy(type != 0) deck.name = name - deck.id = decksBackend.addDeckLegacy(deck) + addDeckLegacy(deck) return Optional.of(deck.id) } + fun addDeckLegacy(deck: DeckV16): OpChangesWithId { + val changes = col.backend.addDeckLegacy( + json = BackendUtils.to_json_bytes(deck.getJsonObject()) + ) + deck.id = changes.id + return changes + } + /** Remove the deck. If cardsToo, delete any cards inside. */ override fun rem(did: did, cardsToo: bool, childrenToo: bool) { assert(cardsToo && childrenToo) - decksBackend.remove_deck(did) + col.backend.removeDecks(listOf(did)) + } + + fun removeDecks(deckIds: Iterable): OpChangesWithCount { + return col.backend.removeDecks(dids = deckIds) } @Suppress("deprecation") @@ -268,42 +307,63 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke skip_empty_default: bool = false, include_filtered: bool = true ): ImmutableList { - - return decksBackend.all_names_and_ids( - skip_empty_default = skip_empty_default, - include_filtered = include_filtered - ) + return col.backend.getDeckNames(skip_empty_default, include_filtered).map { + entry -> + DeckNameId(entry.name, entry.id) + } } override fun id_for_name(name: str): did? { - return decksBackend.id_for_name(name).orElse(null) + try { + return col.backend.getDeckIdByName(name) + } catch (ex: BackendNotFoundException) { + return null + } } fun get_legacy(did: did): Deck? { - return decksBackend.get_deck_legacy(did).map { x -> Deck(x.getJsonObject()) }.orElse(null) + return get_deck_legacy(did)?.let { x -> Deck(x.getJsonObject()) } + } + + private fun get_deck_legacy(did: did): DeckV16? { + try { + val jsonObject = BackendUtils.from_json_bytes(col.backend.getDeckLegacy(did)) + val ret = if (Decks.isDynamic(Deck(jsonObject))) { + DeckV16.FilteredDeck(jsonObject) + } else { + DeckV16.NonFilteredDeck(jsonObject) + } + return ret + } catch (ex: BackendNotFoundException) { + return null + } } fun get_all_legacy(): ImmutableList { - return decksBackend.all_decks_legacy() + return BackendUtils.from_json_bytes(col.backend.getAllDecksLegacy()) + .objectIterable { obj -> DeckV16.Generic(obj) } + .toList() + } + + private fun JSONObject.objectIterable(f: (JSONObject) -> T) = sequence { + keys().forEach { k -> yield(f(getJSONObject(k))) } } + fun new_deck_legacy(filtered: bool): DeckV16 { - val deck = decksBackend.new_deck_legacy(filtered) - if (filtered) { + val deck = BackendUtils.from_json_bytes(col.backend.newDeckLegacy(filtered)) + return if (filtered) { // until migrating to the dedicated method for creating filtered decks, // we need to ensure the default config matches legacy expectations - val json = deck.getJsonObject() - val terms = json.getJSONArray("terms").getJSONArray(0) + val terms = deck.getJSONArray("terms").getJSONArray(0) terms.put(0, "") terms.put(2, 0) - json.put("terms", JSONArray(listOf(terms))) - json.put("browserCollapsed", false) - json.put("collapsed", false) + deck.put("terms", JSONArray(listOf(terms))) + deck.put("browserCollapsed", false) + deck.put("collapsed", false) + DeckV16.FilteredDeck(deck) + } else { + DeckV16.NonFilteredDeck(deck) } - return deck - } - - fun deck_tree(): DeckTreeNode { - return decksBackend.deck_tree(now = 0L) } /** All decks. Expensive; prefer all_names_and_ids() */ @@ -320,8 +380,7 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke fun allNames(dyn: bool = true, force_default: bool = true): MutableList { return this.all_names_and_ids( skip_empty_default = !force_default, include_filtered = dyn - ).map { - x -> + ).map { x -> x.name }.toMutableList() } @@ -374,7 +433,15 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke /** Add or update an existing deck. Used for syncing and merging. */ fun update(g: DeckV16, preserve_usn: bool = true) { - g.id = decksBackend.add_or_update_deck_legacy(g, preserve_usn) + g.id = try { + col.backend.addOrUpdateDeckLegacy(g.to_json_bytes(), preserve_usn) + } catch (ex: BackendDeckIsFilteredException) { + throw DeckRenameException.filteredAncestor(g.name, "") + } + } + + private fun DeckV16.to_json_bytes(): ByteString { + return BackendUtils.toByteString(this.getJsonObject()) } /** Rename deck prefix to NAME if not exists. Updates children. */ @@ -383,58 +450,14 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke this.update(g, preserve_usn = false) } - /* Commented in the Java - also buggy in the Kotlin - Drag/drop - - fun renameForDragAndDrop(draggedDeckDid: int, ontoDeckDid: Optional) { - var draggedDeck = this.get(draggedDeckDid) - var draggedDeckName = draggedDeck.name - var ontoDeckName = this.get(ontoDeckDid).name - - if (ontoDeckDid is None || ontoDeckDid == "") { - if (len(path(draggedDeckName)) > 1) { - this.rename(draggedDeck, basename(draggedDeckName)) - } - } else if (this._canDragAndDrop(draggedDeckName, ontoDeckName)) { - draggedDeck = this.get(draggedDeckDid) - draggedDeckName = draggedDeck.name - ontoDeckName = this.get(ontoDeckDid).name - assert(ontoDeckName.strip()) - this.rename( - draggedDeck, ontoDeckName + "::" + basename(draggedDeckName) - ) - } - } - - fun _canDragAndDrop(draggedDeckName: str, ontoDeckName: str) : bool { - if ( - draggedDeckName == ontoDeckName - || this._isParent(ontoDeckName, draggedDeckName) - || this._isAncestor(draggedDeckName, ontoDeckName) - ) { - return false - } else { - return true - } - } - - fun _isParent(parentDeckName: str, childDeckName: str) : bool { - // incorrect - return path(childDeckName) == path(parentDeckName).add(basename(childDeckName)) - } - - fun _isAncestor(ancestorDeckName: str, descendantDeckName: str) : bool { - val ancestorPath = path(ancestorDeckName) - // incorrect - return ancestorPath == path(descendantDeckName).take(len(ancestorPath)) - } - */ - /* Deck configurations */ /** A list of all deck config. */ fun all_config(): ImmutableList { - return decksBackend.all_config() + return BackendUtils.jsonToArray(col.backend.allDeckConfigLegacy()) + .jsonObjectIterable() + .map { obj -> DeckConfigV16.Config(obj) } + .toList() } @RustCleanup("Return v16 config - we return a typed object here") @@ -444,25 +467,30 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke val deckValue = deck.get() if (deckValue.hasKey("conf")) { val dcid = deckValue.conf // TODO: may be a string - var conf = this.get_config(dcid) - if (conf.isEmpty) { - // fall back on default - conf = this.get_config(1) - } - val knownConf = conf.get() - knownConf.dyn = false - return DeckConfig(knownConf.config, knownConf.source) + val conf = get_config(dcid) + conf.dyn = false + return DeckConfig(conf.config, conf.source) } // dynamic decks have embedded conf return DeckConfig(deck.get().getJsonObject(), DeckConfig.Source.DECK_EMBEDDED) } - fun get_config(conf_id: dcid): Optional { - return decksBackend.get_config(conf_id) + /* Backend will return default config if provided id doesn't exist. */ + fun get_config(conf_id: dcid): DeckConfigV16 { + val jsonObject = BackendUtils.from_json_bytes(col.backend.getDeckConfigLegacy(conf_id)) + val config = DeckConfigV16.Config(jsonObject) + return config } fun update_config(conf: DeckConfigV16, preserve_usn: bool = false) { - conf.id = decksBackend.update_config(conf, preserve_usn) + if (preserve_usn) { + TODO("no longer supported; need to switch to new sync code") + } + conf.id = col.backend.addOrUpdateDeckConfigLegacy(conf.to_json_bytes()) + } + + private fun DeckConfigV16.to_json_bytes(): ByteString { + return BackendUtils.toByteString(this.config) } fun add_config( @@ -474,13 +502,18 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke conf = clone_from.get().deepClone() conf.id = 0L } else { - conf = decksBackend.new_deck_config_legacy() + conf = newDeckConfigLegacy() } conf.name = name this.update_config(conf) return conf } + private fun newDeckConfigLegacy(): DeckConfigV16 { + val jsonObject = BackendUtils.from_json_bytes(col.backend.newDeckConfigLegacy()) + return DeckConfigV16.Config(jsonObject) + } + fun add_config_returning_id( name: str, clone_from: Optional = Optional.empty() @@ -499,7 +532,7 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke this.save(g) } } - decksBackend.remove_deck_config(id) + col.backend.removeDeckConfig(dcid = id) } override fun setConf(grp: Deck, id: Long) { @@ -531,7 +564,7 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke fun restoreToDefault(conf: DeckConfigV16) { val oldOrder = conf.getJSONObject("new").getInt("order") - val new = decksBackend.new_deck_config_legacy() + val new = newDeckConfigLegacy() new.id = conf.id new.name = conf.name this.update_config(new) @@ -542,16 +575,23 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke } // legacy - override fun allConf() = all_config().map { x -> DeckConfig(x.config, x.source) }.toMutableList() - override fun getConf(confId: dcid): DeckConfig? = get_config(confId).map { x -> DeckConfig(x.config, x.source) }.orElse(null) + override fun allConf() = + all_config().map { x -> DeckConfig(x.config, x.source) }.toMutableList() + + /* Reverts to default if provided id missing */ + override fun getConf(confId: dcid): DeckConfig = + get_config(confId).let { x -> DeckConfig(x.config, x.source) } override fun confId(name: String, cloneFrom: String): Long { - val config: Optional = Optional.of(DeckConfigV16.Config(JSONObject(cloneFrom))) + val config: Optional = + Optional.of(DeckConfigV16.Config(JSONObject(cloneFrom))) return add_config_returning_id(name, config) } override fun updateConf(g: DeckConfig) = updateConf(DeckConfigV16.from(g), preserve_usn = false) - fun updateConf(conf: DeckConfigV16, preserve_usn: bool = false) = update_config(conf, preserve_usn) + fun updateConf(conf: DeckConfigV16, preserve_usn: bool = false) = + update_config(conf, preserve_usn) + override fun remConf(id: dcid) = remove_config(id) fun confId(name: str, clone_from: Optional = Optional.empty()) = add_config_returning_id(name, clone_from) @@ -646,7 +686,10 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke companion object { @JvmStatic - fun find_deck_in_tree(node: anki.decks.DeckTreeNode, deck_id: did): Optional { + fun find_deck_in_tree( + node: anki.decks.DeckTreeNode, + deck_id: did + ): Optional { if (node.deckId == deck_id) { return Optional.of(node) } @@ -713,11 +756,9 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke fun child_ids(parent_name: str): Iterable { val prefix = parent_name + "::" - return all_names_and_ids().filter { - x -> + return all_names_and_ids().filter { x -> x.name.startsWith(prefix) - }.map { - d -> + }.map { d -> d.id }.toMutableList() } @@ -737,6 +778,7 @@ class DecksV16(private val col: Collection, private val decksBackend: DecksBacke gather(child as childMapNode, arr) } } + val arr = mutableListOf() gather(childMap[did] as childMapNode, arr) return arr diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DecksBackend.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DecksBackend.kt deleted file mode 100644 index 8237084a0e7a..000000000000 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DecksBackend.kt +++ /dev/null @@ -1,184 +0,0 @@ -/* - * Copyright (c) 2021 David Allison - * - * 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 . - */ - -@file:Suppress("NonPublicNonStaticFieldName", "FunctionName") - -package com.ichi2.libanki.backend - -import com.google.protobuf.ByteString -import com.ichi2.libanki.* -import com.ichi2.libanki.backend.BackendUtils.from_json_bytes -import com.ichi2.libanki.backend.BackendUtils.jsonToArray -import com.ichi2.libanki.backend.BackendUtils.toByteString -import com.ichi2.libanki.backend.BackendUtils.to_json_bytes -import com.ichi2.libanki.backend.exception.DeckRenameException -import com.ichi2.utils.JSONObject -import java8.util.Optional -import net.ankiweb.rsdroid.Backend -import net.ankiweb.rsdroid.database.NotImplementedException -import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException -import net.ankiweb.rsdroid.exceptions.BackendNotFoundException - -private typealias did = Long -private typealias dcid = Long - -data class DeckNameId(val name: String, val id: did) - -data class DeckTreeNode( - val deck_id: Long, - val name: String, - val children: List, - val level: UInt, - val collapsed: Boolean, - val review_count: UInt, - val learn_count: UInt, - val new_count: UInt, - val filtered: Boolean -) - -/** Anti-corruption layer, removing the dependency on protobuf types from libAnki code */ -interface DecksBackend { - fun get_config(conf_id: dcid): Optional - fun update_config(conf: DeckConfigV16, preserve_usn: Boolean): dcid - fun new_deck_config_legacy(): DeckConfigV16 - fun all_config(): List - fun add_or_update_deck_legacy(deck: DeckV16, preserve_usn: Boolean): did - fun id_for_name(name: String): Optional - fun get_deck_legacy(did: did): Optional - fun all_decks_legacy(): List - fun new_deck_legacy(filtered: Boolean): DeckV16 - /** A sorted sequence of deck names and IDs. */ - fun all_names_and_ids(skip_empty_default: Boolean, include_filtered: Boolean): List - fun deck_tree(now: Long): DeckTreeNode - fun remove_deck_config(id: dcid) - fun remove_deck(did: did) - fun addDeckLegacy(deck: DeckV16): Long -} - -/** WIP: Backend implementation for usage in Decks.kt */ -class RustDroidDeckBackend(private val backend: Backend) : DecksBackend { - - override fun get_config(conf_id: dcid): Optional { - return try { - val jsonObject = from_json_bytes(backend.getDeckConfigLegacy(conf_id)) - val config = DeckConfigV16.Config(jsonObject) - Optional.of(config) - } catch (ex: BackendNotFoundException) { - Optional.empty() - } - } - - override fun update_config(conf: DeckConfigV16, preserve_usn: Boolean): dcid { - if (preserve_usn) { - TODO("no longer supported; need to switch to new sync code") - } - return backend.addOrUpdateDeckConfigLegacy(conf.to_json_bytes()) - } - - override fun new_deck_config_legacy(): DeckConfigV16 { - val jsonObject = from_json_bytes(backend.newDeckConfigLegacy()) - return DeckConfigV16.Config(jsonObject) - } - - override fun all_config(): MutableList { - return jsonToArray(backend.allDeckConfigLegacy()) - .jsonObjectIterable() - .map { obj -> DeckConfigV16.Config(obj) } - .toMutableList() - } - - @Throws(DeckRenameException::class) - override fun add_or_update_deck_legacy(deck: DeckV16, preserve_usn: Boolean): did { - try { - return backend.addOrUpdateDeckLegacy(deck.to_json_bytes(), preserve_usn) - } catch (ex: BackendDeckIsFilteredException) { - throw DeckRenameException.filteredAncestor(deck.name, "") - } - } - - override fun id_for_name(name: String): Optional { - try { - return Optional.of(backend.getDeckIdByName(name)) - } catch (ex: BackendNotFoundException) { - return Optional.empty() - } - } - - override fun get_deck_legacy(did: did): Optional { - try { - val jsonObject = from_json_bytes(backend.getDeckLegacy(did)) - val ret = if (Decks.isDynamic(Deck(jsonObject))) { - DeckV16.FilteredDeck(jsonObject) - } else { - DeckV16.NonFilteredDeck(jsonObject) - } - return Optional.of(ret) - } catch (ex: BackendNotFoundException) { - return Optional.empty() - } - } - - override fun new_deck_legacy(filtered: Boolean): DeckV16 { - val deck = from_json_bytes(backend.newDeckLegacy(filtered)) - return if (filtered) { - DeckV16.FilteredDeck(deck) - } else { - DeckV16.NonFilteredDeck(deck) - } - } - - override fun all_decks_legacy(): MutableList { - return from_json_bytes(backend.getAllDecksLegacy()) - .objectIterable { obj -> DeckV16.Generic(obj) } - .toMutableList() - } - - override fun all_names_and_ids(skip_empty_default: Boolean, include_filtered: Boolean): List { - return backend.getDeckNames(skip_empty_default, include_filtered).map { - entry -> - DeckNameId(entry.name, entry.id) - } - } - - override fun deck_tree(now: Long): DeckTreeNode { - backend.deckTree(now) - throw NotImplementedException() - } - - override fun remove_deck_config(id: dcid) { - backend.removeDeckConfig(id) - } - - override fun remove_deck(did: did) { - backend.removeDecks(listOf(did)) - } - - private fun DeckV16.to_json_bytes(): ByteString { - return toByteString(this.getJsonObject()) - } - - private fun DeckConfigV16.to_json_bytes(): ByteString { - return toByteString(this.config) - } - - private fun JSONObject.objectIterable(f: (JSONObject) -> T) = sequence { - keys().forEach { k -> yield(f(getJSONObject(k))) } - } - - override fun addDeckLegacy(deck: DeckV16): Long { - return backend.addDeckLegacy(to_json_bytes(deck.getJsonObject())).id - } -} From 81d3f6dbcf3b30de51f7a04564b245ca1fb145b8 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 6 Jul 2022 17:07:52 +1000 Subject: [PATCH 08/15] Refactor coroutine handling - Progress is now shown in the GUI - Cancellation of tasks is supported - Removes some boilerplate and improves ergonomics - runInBackgroundWithProgress is now scoped to AnkiActivity. - When errors encountered, they are shown until user clicks OK. - Hooks up new backend to import->colpkg option Cancellation will not work correctly until the backend is updated, as the cancellation routines need to be excluded from the backend mutex - until updating, attempting to cancel will block the UI until the operation completes. --- .../java/com/ichi2/anki/BackendBackups.kt | 30 ++-- .../java/com/ichi2/anki/BackendImporting.kt | 46 ++--- .../java/com/ichi2/anki/CoroutineHelpers.kt | 163 +++++++++++++----- .../main/java/com/ichi2/anki/DatabaseCheck.kt | 32 ++-- .../main/java/com/ichi2/anki/DeckPicker.kt | 13 +- .../src/main/java/com/ichi2/anki/Sync.kt | 144 ++++++++++------ .../java/com/ichi2/libanki/ChangeManager.kt | 8 + 7 files changed, 278 insertions(+), 158 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendBackups.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendBackups.kt index 5f91e5142b60..b258159cbed0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/BackendBackups.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendBackups.kt @@ -22,11 +22,9 @@ import com.ichi2.libanki.CollectionV16 import com.ichi2.libanki.awaitBackupCompletion import com.ichi2.libanki.createBackup import kotlinx.coroutines.* -import timber.log.Timber fun DeckPicker.performBackupInBackground() { - val col = CollectionHelper.getInstance().getCol(baseContext).newBackend - catchingLifecycleScope(this) { + launchCatchingCollectionTask { col -> // Wait a second to allow the deck list to finish loading first, or it // will hang until the first stage of the backup completes. delay(1000) @@ -35,21 +33,21 @@ fun DeckPicker.performBackupInBackground() { } fun DeckPicker.importColpkg(colpkgPath: String) { - val deckPicker = this - catchingLifecycleScope(this) { - runInBackground { - val helper = CollectionHelper.getInstance() - val backend = helper.getOrCreateBackend(baseContext) - backend.withProgress({ - if (it.hasImporting()) { - // TODO: show progress in GUI - Timber.i("%s", it.importing) + launchCatchingTask { + val helper = CollectionHelper.getInstance() + val backend = helper.getOrCreateBackend(baseContext) + runInBackgroundWithProgress( + backend, + extractProgress = { + if (progress.hasImporting()) { + text = progress.importing } - }) { - helper.importColpkg(baseContext, colpkgPath) - } + }, + ) { + helper.importColpkg(baseContext, colpkgPath) } - deckPicker.updateDeckList() + invalidateOptionsMenu() + updateDeckList() } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendImporting.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendImporting.kt index a361e0cd880e..06d78fc0795a 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/BackendImporting.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendImporting.kt @@ -21,22 +21,26 @@ package com.ichi2.anki import anki.import_export.ImportResponse import com.ichi2.libanki.exportAnkiPackage import com.ichi2.libanki.importAnkiPackage +import com.ichi2.libanki.undoableOp import net.ankiweb.rsdroid.Translations -import timber.log.Timber -fun DeckPicker.importApkg(apkgPath: String) { - val deckPicker = this - val col = CollectionHelper.getInstance().getCol(deckPicker.baseContext).newBackend - catchingLifecycleScope(this) { - val report = col.opWithProgress({ - if (it.hasImporting()) { - // TODO: show progress in GUI - Timber.i("%s", it.importing) +fun DeckPicker.importApkgs(apkgPaths: List) { + launchCatchingCollectionTask { col -> + for (apkgPath in apkgPaths) { + val report = runInBackgroundWithProgress( + col.backend, + extractProgress = { + if (progress.hasImporting()) { + text = progress.importing + } + }, + ) { + undoableOp { + col.importAnkiPackage(apkgPath) + } } - }) { - importAnkiPackage(apkgPath) + showSimpleMessageDialog(summarizeReport(col.tr, report)) } - showSimpleMessageDialog(summarizeReport(col.tr, report)) } } @@ -65,15 +69,15 @@ fun DeckPicker.exportApkg( withMedia: Boolean, deckId: Long? ) { - val deckPicker = this - val col = CollectionHelper.getInstance().getCol(deckPicker.baseContext).newBackend - catchingLifecycleScope(this) { - runInBackgroundWithProgress(col, { - if (it.hasExporting()) { - // TODO: show progress in GUI - Timber.i("%s", it.exporting) - } - }) { + launchCatchingCollectionTask { col -> + runInBackgroundWithProgress( + col.backend, + extractProgress = { + if (progress.hasExporting()) { + text = progress.exporting + } + }, + ) { col.exportAnkiPackage(apkgPath, withScheduling, withMedia, deckId) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index 3164137fc758..c9923f0b7fb4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -1,5 +1,5 @@ /*************************************************************************************** - * Copyright (c) 2022 Ankitects Pty Ltd * + * Copyright (c) 2022 Ankitects Pty Ltd * * * * 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 * @@ -16,59 +16,78 @@ package com.ichi2.anki -import android.app.Activity -import androidx.lifecycle.LifecycleOwner +import android.content.Context +import androidx.appcompat.app.AlertDialog import androidx.lifecycle.coroutineScope import anki.collection.Progress import com.ichi2.anki.UIUtils.showSimpleSnackbar -import com.ichi2.libanki.ChangeManager import com.ichi2.libanki.CollectionV16 +import com.ichi2.themes.StyledProgressDialog import kotlinx.coroutines.* import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.BackendException +import net.ankiweb.rsdroid.exceptions.BackendInterruptedException +import timber.log.Timber /** * Launch a job that catches any uncaught errors and reports them to the user. * Errors from the backend contain localized text that is often suitable to show to the user as-is. * Other errors should ideally be handled in the block. */ -// TODO: require user confirmation before message disappears -fun LifecycleOwner.catchingLifecycleScope(activity: Activity, block: suspend CoroutineScope.() -> Unit): Job { +fun AnkiActivity.launchCatchingTask( + block: suspend CoroutineScope.() -> Unit +): Job { return lifecycle.coroutineScope.launch { try { block() + } catch (exc: BackendInterruptedException) { + Timber.e("caught: %s", exc) + showSimpleSnackbar(this@launchCatchingTask, exc.localizedMessage, false) } catch (exc: BackendException) { - showSimpleSnackbar(activity, exc.localizedMessage, false) + Timber.e("caught: %s", exc) + showError(this@launchCatchingTask, exc.localizedMessage!!) } catch (exc: Exception) { - // TODO: localize - showSimpleSnackbar(activity, "An error occurred: {exc}", false) + Timber.e("caught: %s", exc) + showError(this@launchCatchingTask, exc.toString()) } } } -suspend fun runInBackground(block: suspend CoroutineScope.() -> T): T { - return withContext(Dispatchers.IO) { - block() +private fun showError(context: Context, msg: String) { + AlertDialog.Builder(context) + .setTitle(R.string.vague_error) + .setMessage(msg) + .setPositiveButton(R.string.dialog_ok) { _, _ -> } + .show() +} + +/** Launch a catching task that requires a collection with the new schema enabled. */ +fun AnkiActivity.launchCatchingCollectionTask(block: suspend CoroutineScope.(col: CollectionV16) -> Unit): Job { + val col = CollectionHelper.getInstance().getCol(baseContext).newBackend + return launchCatchingTask { + block(col) } } -/** - * Run an operation and notify change subscribers. - * * See the docs in ChangeManager.kt - * */ -suspend fun CollectionV16.op(handler: Any? = null, block: suspend CollectionV16.() -> T): T { - return runInBackground { +/** Run a blocking call in a background thread pool. */ +suspend fun runInBackground(block: suspend CoroutineScope.() -> T): T { + return withContext(Dispatchers.IO) { block() - }.also { - ChangeManager.notifySubscribers(it, handler) } } -suspend fun Backend.withProgress(onProgress: (Progress) -> Unit, block: suspend CoroutineScope.() -> T): T { - val backend = this +/** In most cases, you'll want [AnkiActivity.runInBackgroundWithProgress] + * instead. This lower-level routine can be used to integrate your own + * progress UI. + */ +suspend fun Backend.withProgress( + extractProgress: ProgressContext.() -> Unit, + updateUi: ProgressContext.() -> Unit, + block: suspend CoroutineScope.() -> T, +): T { return coroutineScope { val monitor = launch { - monitorProgress(backend, onProgress) + monitorProgress(this@withProgress, extractProgress, updateUi) } try { block() @@ -78,43 +97,93 @@ suspend fun Backend.withProgress(onProgress: (Progress) -> Unit, block: susp } } -suspend fun runInBackgroundWithProgress( - col: CollectionV16, - onProgress: (Progress) -> Unit, - op: suspend (CollectionV16) -> T -): T = coroutineScope { - col.backend.withProgress(onProgress) { - runInBackground { op(col) } +/** + * Run the provided operation in the background, showing a progress + * window. + */ +suspend fun AnkiActivity.runInBackgroundWithProgress( + backend: Backend, + extractProgress: ProgressContext.() -> Unit, + onCancel: ((Backend) -> Unit)? = { it.setWantsAbort() }, + op: suspend () -> T +): T = withProgressDialog( + context = this@runInBackgroundWithProgress, + onCancel = if (onCancel != null) { + fun() { onCancel(backend) } + } else { + null + } +) { dialog -> + backend.withProgress( + extractProgress = extractProgress, + updateUi = { updateDialog(dialog) } + ) { + runInBackground { op() } } } -/** - * Run an operation and notify change subscribers, and capture backend progress. - * - * See the docs in ChangeManager.kt - * */ -suspend fun CollectionV16.opWithProgress( - onProgress: (Progress) -> Unit, - handler: Any? = null, - op: suspend CollectionV16.() -> T, -): T = coroutineScope { - backend.withProgress(onProgress) { - this@opWithProgress.op(handler) { - op() - } +private suspend fun withProgressDialog( + context: AnkiActivity, + onCancel: (() -> Unit)?, + @Suppress("Deprecation") // ProgressDialog deprecation + op: suspend (android.app.ProgressDialog) -> T +): T { + val dialog = StyledProgressDialog.show( + context, null, + null, onCancel != null + ) + onCancel?.let { + dialog.setOnCancelListener { it() } + } + return try { + op(dialog) + } finally { + dialog.dismiss() } } /** * Poll the backend for progress info every 100ms until cancelled by caller. + * Calls extractProgress() to gather progress info and write it into + * [ProgressContext]. Calls updateUi() to update the UI with the extracted + * progress. */ -private suspend fun monitorProgress(backend: Backend, op: (Progress) -> Unit) { +private suspend fun monitorProgress( + backend: Backend, + extractProgress: ProgressContext.() -> Unit, + updateUi: ProgressContext.() -> Unit, +) { + var state = ProgressContext(Progress.getDefaultInstance()) while (true) { - val progress = backend.latestProgress() + state.progress = backend.latestProgress() + state.extractProgress() // on main thread, so op can update UI withContext(Dispatchers.Main) { - op(progress) + state.updateUi() } delay(100) } } + +/** Holds the current backend progress, and text/amount properties + * that can be written to in order to update the UI. + */ +data class ProgressContext( + var progress: Progress, + var text: String = "", + /** If set, shows progress bar with a of b complete. */ + var amount: Pair? = null, +) + +@Suppress("Deprecation") // ProgressDialog deprecation +private fun ProgressContext.updateDialog(dialog: android.app.ProgressDialog) { + // ideally this would show a progress bar, but MaterialDialog does not support + // setting progress after starting with indeterminate progress, so we just use + // this for now + // this code has since been updated to ProgressDialog, and the above not rechecked + val progressText = amount?.let { + " ${it.first}/${it.second}" + } ?: "" + @Suppress("Deprecation") // ProgressDialog deprecation + dialog.setMessage(text + progressText) +} diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt index b5b0ba31e022..440842f2e202 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DatabaseCheck.kt @@ -16,24 +16,24 @@ package com.ichi2.anki -import timber.log.Timber - fun DeckPicker.handleDatabaseCheck() { - val col = CollectionHelper.getInstance().getCol(baseContext).newBackend - val deckPicker = this - catchingLifecycleScope(this) { - val problems = runInBackgroundWithProgress(col, { - if (it.hasDatabaseCheck()) { - // TODO: show progress in GUI - it.databaseCheck.run { - if (stageTotal > 0) { - Timber.i("$stage: $stageCurrent/$stageTotal") - } else { - Timber.i("$stage") + launchCatchingCollectionTask { col -> + val problems = runInBackgroundWithProgress( + col.backend, + extractProgress = { + if (progress.hasDatabaseCheck()) { + progress.databaseCheck.let { + text = it.stage + if (it.stageTotal > 0) { + amount = Pair(it.stageCurrent, it.stageTotal) + } else { + amount = null + } } } - } - }) { + }, + onCancel = null, + ) { col.fixIntegrity() } val message = if (problems.isNotEmpty()) { @@ -41,6 +41,6 @@ fun DeckPicker.handleDatabaseCheck() { } else { col.tr.databaseCheckRebuilt() } - deckPicker.showSimpleMessageDialog(message) + showSimpleMessageDialog(message) } } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index e808ef56b91c..a24238e8113f 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -1846,16 +1846,19 @@ open class DeckPicker : if (BackendFactory.defaultLegacySchema) { TaskManager.launchCollectionTask(ImportAdd(importPath), mImportAddListener) } else { - for (file in importPath) { - importApkg(file) - } + importApkgs(importPath) } } // Callback to import a file -- replacing the existing collection @NeedsTest("Test 2 successful files & test 1 failure & 1 successful file") override fun importReplace(importPath: List) { - TaskManager.launchCollectionTask(ImportReplace(importPath), importReplaceListener()) + if (BackendFactory.defaultLegacySchema) { + TaskManager.launchCollectionTask(ImportReplace(importPath), importReplaceListener()) + } else { + // multiple colpkg files is nonsensical + importColpkg(importPath[0]) + } } /** @@ -2050,7 +2053,7 @@ open class DeckPicker : context.mDueTree = result.map { x -> x.unsafeCastToType(AbstractDeckTreeNode::class.java) } context.renderPage() // Update the mini statistics bar as well - deckPicker?.catchingLifecycleScope(deckPicker) { + deckPicker?.launchCatchingTask { AnkiStatsTaskHandler.createReviewSummaryStatistics(context.col, context.mReviewSummaryTextView) } Timber.d("Startup - Deck List UI Completed") diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt index 1e8366c14454..6254682b95d4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt @@ -25,9 +25,11 @@ package com.ichi2.anki import android.content.Context +import androidx.appcompat.app.AlertDialog import androidx.core.content.edit import anki.sync.SyncAuth import anki.sync.SyncCollectionResponse +import anki.sync.syncAuth import com.ichi2.anim.ActivityTransitionAnimation import com.ichi2.anki.dialogs.SyncErrorDialog import com.ichi2.anki.web.HostNumFactory @@ -35,6 +37,7 @@ import com.ichi2.async.Connection import com.ichi2.libanki.CollectionV16 import com.ichi2.libanki.createBackup import com.ichi2.libanki.sync.* +import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.exceptions.BackendSyncException import timber.log.Timber @@ -43,35 +46,31 @@ fun DeckPicker.handleNewSync( hostNum: Int, conflict: Connection.ConflictResolution? ) { - val auth = SyncAuth.newBuilder().apply { + val auth = syncAuth { this.hkey = hkey this.hostNumber = hostNum - }.build() - - val col = CollectionHelper.getInstance().getCol(baseContext).newBackend + } val deckPicker = this - - catchingLifecycleScope(this) { + launchCatchingCollectionTask { col -> try { when (conflict) { - Connection.ConflictResolution.FULL_DOWNLOAD -> handleDownload(col, auth, deckPicker) - Connection.ConflictResolution.FULL_UPLOAD -> handleUpload(col, auth, deckPicker) - null -> handleNormalSync(baseContext, col, auth, deckPicker) + Connection.ConflictResolution.FULL_DOWNLOAD -> handleDownload(deckPicker, col, auth) + Connection.ConflictResolution.FULL_UPLOAD -> handleUpload(deckPicker, col, auth) + null -> handleNormalSync(deckPicker, col, auth) } } catch (exc: BackendSyncException.BackendSyncAuthFailedException) { // auth failed; log out updateLogin(baseContext, "", "") throw exc } - deckPicker.refreshState() + refreshState() } } fun MyAccount.handleNewLogin(username: String, password: String) { - val col = CollectionHelper.getInstance().getCol(baseContext).newBackend - catchingLifecycleScope(this) { + launchCatchingCollectionTask { col -> val auth = try { - runInBackgroundWithProgress(col, { }) { + runInBackgroundWithProgress(col.backend, {}, onCancel = ::cancelSync) { col.syncLogin(username, password) } } catch (exc: BackendSyncException.BackendSyncAuthFailedException) { @@ -92,22 +91,30 @@ private fun updateLogin(context: Context, username: String, hkey: String?) { } } +private fun cancelSync(backend: Backend) { + backend.setWantsAbort() + backend.abortSync() +} + private suspend fun handleNormalSync( - context: Context, + deckPicker: DeckPicker, col: CollectionV16, - auth: SyncAuth, - deckPicker: DeckPicker + auth: SyncAuth ) { - val output = runInBackgroundWithProgress(col, { - if (it.hasNormalSync()) { - it.normalSync.run { updateProgress("$added $removed") } - } - }) { + val output = deckPicker.runInBackgroundWithProgress( + col.backend, + extractProgress = { + if (progress.hasNormalSync()) { + text = progress.normalSync.run { "$added\n$removed" } + } + }, + onCancel = ::cancelSync + ) { col.syncCollection(auth) } // Save current host number - HostNumFactory.getInstance(context).setHostNum(output.hostNumber) + HostNumFactory.getInstance(deckPicker).setHostNum(output.hostNumber) when (output.required) { SyncCollectionResponse.ChangesRequired.NO_CHANGES -> { @@ -115,17 +122,17 @@ private suspend fun handleNormalSync( deckPicker.showSyncLogMessage(R.string.sync_database_acknowledge, output.serverMessage) // kick off media sync - future implementations may want to run this in the // background instead - handleMediaSync(col, auth) + handleMediaSync(deckPicker, col, auth) } SyncCollectionResponse.ChangesRequired.FULL_DOWNLOAD -> { - handleDownload(col, auth, deckPicker) - handleMediaSync(col, auth) + handleDownload(deckPicker, col, auth) + handleMediaSync(deckPicker, col, auth) } SyncCollectionResponse.ChangesRequired.FULL_UPLOAD -> { - handleUpload(col, auth, deckPicker) - handleMediaSync(col, auth) + handleUpload(deckPicker, col, auth) + handleMediaSync(deckPicker, col, auth) } SyncCollectionResponse.ChangesRequired.FULL_SYNC -> { @@ -140,16 +147,25 @@ private suspend fun handleNormalSync( } } +private fun fullDownloadProgress(title: String): ProgressContext.() -> Unit { + return { + if (progress.hasFullSync()) { + text = title + amount = progress.fullSync.run { Pair(transferred, total) } + } + } +} + private suspend fun handleDownload( + deckPicker: DeckPicker, col: CollectionV16, - auth: SyncAuth, - deckPicker: DeckPicker + auth: SyncAuth ) { - runInBackgroundWithProgress(col, { - if (it.hasFullSync()) { - it.fullSync.run { updateProgress("downloaded $transferred/$total") } - } - }) { + deckPicker.runInBackgroundWithProgress( + col.backend, + extractProgress = fullDownloadProgress(col.tr.syncDownloadingFromAnkiweb()), + onCancel = ::cancelSync + ) { col.createBackup( BackupManager.getBackupDirectoryFromCollection(col.path), force = true, @@ -168,15 +184,15 @@ private suspend fun handleDownload( } private suspend fun handleUpload( + deckPicker: DeckPicker, col: CollectionV16, - auth: SyncAuth, - deckPicker: DeckPicker + auth: SyncAuth ) { - runInBackgroundWithProgress(col, { - if (it.hasFullSync()) { - it.fullSync.run { updateProgress("uploaded $transferred/$total") } - } - }) { + deckPicker.runInBackgroundWithProgress( + col.backend, + extractProgress = fullDownloadProgress(col.tr.syncUploadingToAnkiweb()), + onCancel = ::cancelSync + ) { col.close(save = true, downgrade = false, forFullSync = true) try { col.fullUpload(auth) @@ -184,26 +200,48 @@ private suspend fun handleUpload( col.reopen(afterFullSync = true) } } - Timber.i("Full Upload Completed") deckPicker.showSyncLogMessage(R.string.sync_log_uploading_message, "") } -@Suppress("UNUSED_PARAMETER", "UNREACHABLE_CODE") +// TODO: this needs a dedicated UI for media syncing, and needs to expose +// a way to interrupt the sync + +private fun cancelMediaSync(backend: Backend) { + backend.setWantsAbort() + backend.abortMediaSync() +} + private suspend fun handleMediaSync( + deckPicker: DeckPicker, col: CollectionV16, auth: SyncAuth ) { - runInBackgroundWithProgress(col, { - if (it.hasMediaSync()) { - it.mediaSync.run { updateProgress("media: $added $removed $checked") } + // TODO: show this in a way that is clear it can be continued in background, + // but also warn user that media files will not be available until it completes. + // TODO: provide a way for users to abort later, and see it's still going + val dialog = AlertDialog.Builder(deckPicker) + .setTitle(col.tr.syncMediaLogTitle()) + .setMessage("") + .setPositiveButton("Background") { _, _ -> } + .show() + try { + col.backend.withProgress( + extractProgress = { + if (progress.hasMediaSync()) { + text = + progress.mediaSync.run { "\n$added\n$removed\n$checked" } + } + }, + updateUi = { + dialog.setMessage(text) + }, + ) { + runInBackground { + col.syncMedia(auth) + } } - }) { - col.syncMedia(auth) + } finally { + dialog.dismiss() } } - -// FIXME: display/update a popup progress window instead of logging -private fun updateProgress(text: String) { - Timber.i("progress: $text") -} diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt index 3998b9319edd..ac70013e400e 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt @@ -79,3 +79,11 @@ object ChangeManager { notifySubscribers(opChanges, initiator) } } + +/** Wrap a routine that returns OpChanges* or similar undo info with this + * to notify change subscribers of the changes. */ +fun undoableOp(handler: Any? = null, block: () -> T): T { + return block().also { + ChangeManager.notifySubscribers(it, handler) + } +} From d1a886fbce5a2a0838c085a0c810d48832ff02a4 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 21 Jul 2022 16:11:22 +1000 Subject: [PATCH 09/15] Make sure change notifications fire on the main thread --- .../src/main/java/com/ichi2/anki/DeckPicker.kt | 2 +- .../java/com/ichi2/libanki/ChangeManager.kt | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index a24238e8113f..0e082000fb25 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -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 diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt index ac70013e400e..71f6ef98f7a0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/ChangeManager.kt @@ -34,10 +34,12 @@ import anki.collection.OpChangesAfterUndo import anki.collection.OpChangesWithCount import anki.collection.OpChangesWithId import anki.import_export.ImportResponse +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import java.lang.ref.WeakReference object ChangeManager { - interface ChangeSubscriber { + interface Subscriber { /** * Called after a backend method invoked via col.op() or col.opWithProgress() * has modified the collection. Subscriber should inspect the changes, and update @@ -46,14 +48,14 @@ object ChangeManager { fun opExecuted(changes: OpChanges, handler: Any?) } - private val subscribers = mutableListOf>() + private val subscribers = mutableListOf>() - fun subscribe(subscriber: ChangeSubscriber) { + fun subscribe(subscriber: Subscriber) { subscribers.add(WeakReference(subscriber)) } private fun notifySubscribers(changes: OpChanges, handler: Any?) { - val expired = mutableListOf>() + val expired = mutableListOf>() for (subscriber in subscribers) { val ref = subscriber.get() if (ref == null) { @@ -67,7 +69,7 @@ object ChangeManager { } } - fun notifySubscribers(changes: T, initiator: Any?) { + internal fun notifySubscribers(changes: T, initiator: Any?) { val opChanges = when (changes) { is OpChanges -> changes is OpChangesWithCount -> changes.changes @@ -82,8 +84,10 @@ object ChangeManager { /** Wrap a routine that returns OpChanges* or similar undo info with this * to notify change subscribers of the changes. */ -fun undoableOp(handler: Any? = null, block: () -> T): T { +suspend fun undoableOp(handler: Any? = null, block: () -> T): T { return block().also { - ChangeManager.notifySubscribers(it, handler) + withContext(Dispatchers.Main) { + ChangeManager.notifySubscribers(it, handler) + } } } From 4cf1698141f3b148566a55ab9440329e510cb586 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 6 Jul 2022 15:45:22 +1000 Subject: [PATCH 10/15] Support backend undo 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 --- .../com/ichi2/anki/AbstractFlashcardViewer.kt | 51 ++++++++++--- .../main/java/com/ichi2/anki/BackendUndo.kt | 47 ++++++++++++ .../main/java/com/ichi2/anki/CardBrowser.kt | 36 ++++++++- .../java/com/ichi2/anki/CoroutineHelpers.kt | 20 ++++- .../main/java/com/ichi2/anki/DeckPicker.kt | 18 ++++- .../java/com/ichi2/libanki/BackendUndo.kt | 73 +++++++++++++++++++ .../main/java/com/ichi2/libanki/Collection.kt | 8 +- .../java/com/ichi2/libanki/CollectionV16.kt | 20 ++++- .../main/java/com/ichi2/libanki/DecksV16.kt | 13 +++- .../main/java/com/ichi2/libanki/ModelsV16.kt | 7 +- .../libanki/backend/RustConfigBackend.kt | 2 +- .../ichi2/anki/ReviewerKeyboardInputTest.kt | 4 +- .../com/ichi2/anki/ReviewerNoParamTest.kt | 3 +- .../test/java/com/ichi2/anki/ReviewerTest.kt | 3 +- .../java/com/ichi2/anki/RobolectricTest.kt | 14 ++++ 15 files changed, 290 insertions(+), 29 deletions(-) create mode 100644 AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt create mode 100644 AnkiDroid/src/main/java/com/ichi2/libanki/BackendUndo.kt diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt index bedc17e33e71..000f74828b30 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.kt @@ -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 @@ -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.* @@ -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 @@ -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 @@ -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() { @@ -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. diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt new file mode 100644 index 000000000000..1698db1edca5 --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt @@ -0,0 +1,47 @@ +/*************************************************************************************** + * Copyright (c) 2022 Ankitects Pty Ltd * + * * + * 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 . * + ****************************************************************************************/ + +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 + } + } +} diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt index fd35d53005df..fc02f4fa6d45 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.kt @@ -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.floatingactionbutton.FloatingActionButton import com.google.android.material.snackbar.Snackbar @@ -97,6 +98,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 @@ -113,7 +115,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) { @@ -256,6 +263,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) { @@ -1271,7 +1282,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) + } + } + } } } @@ -2628,6 +2647,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 diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt index c9923f0b7fb4..a46c3db6dc07 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt @@ -99,7 +99,7 @@ suspend fun Backend.withProgress( /** * Run the provided operation in the background, showing a progress - * window. + * window. Progress info is polled from the backend. */ suspend fun AnkiActivity.runInBackgroundWithProgress( backend: Backend, @@ -122,6 +122,24 @@ suspend fun AnkiActivity.runInBackgroundWithProgress( } } +/** + * Run the provided operation in the background, showing a progress + * window with the provided message. + */ +suspend fun 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 withProgressDialog( context: AnkiActivity, onCancel: (() -> Unit)?, diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 0e082000fb25..5223ca3b0db5 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -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 @@ -2623,6 +2634,7 @@ open class DeckPicker : override fun opExecuted(changes: OpChanges, handler: Any?) { if (changes.studyQueues && handler !== this) { + invalidateOptionsMenu() updateDeckList() } } diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/BackendUndo.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/BackendUndo.kt new file mode 100644 index 000000000000..4f0fac94c3c8 --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/BackendUndo.kt @@ -0,0 +1,73 @@ +/*************************************************************************************** + * Copyright (c) 2022 Ankitects Pty Ltd * + * * + * 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 . * + ****************************************************************************************/ + +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()) +} diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt index 6c5fde5f5c4b..7e41d902b44b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/Collection.kt @@ -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) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt index 161018589563..728c9126fd2c 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/CollectionV16.kt @@ -16,7 +16,7 @@ package com.ichi2.libanki import android.content.Context -import com.ichi2.async.CancelListener.Companion.isCancelled +import android.content.res.Resources import com.ichi2.async.CollectionTask import com.ichi2.libanki.backend.* import com.ichi2.libanki.backend.model.toProtoBuf @@ -25,6 +25,7 @@ import com.ichi2.libanki.utils.TimeManager import net.ankiweb.rsdroid.Backend import net.ankiweb.rsdroid.RustCleanup import net.ankiweb.rsdroid.exceptions.BackendInvalidInputException +import timber.log.Timber class CollectionV16( context: Context, @@ -173,4 +174,21 @@ class CollectionV16( TimeManager.time.intTimeMS() ) } + + override fun undoAvailable(): Boolean { + val status = undoStatus() + Timber.i("undo: %s, %s", status, super.undoAvailable()) + if (status.undo != null) { + // any legacy undo state is invalid after a backend op + clearUndo() + return true + } + // if no backend undo state, try legacy undo state + return super.undoAvailable() + } + + override fun undoName(res: Resources): String { + val status = undoStatus() + return status.undo ?: super.undoName(res) + } } diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt index bea31e111624..b678905ee1ba 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/DecksV16.kt @@ -45,6 +45,7 @@ import com.ichi2.libanki.Utils.ids2str import com.ichi2.libanki.backend.BackendUtils import com.ichi2.libanki.backend.exception.DeckRenameException import com.ichi2.libanki.utils.* +import com.ichi2.libanki.utils.TimeManager.time import com.ichi2.utils.CollectionUtils import com.ichi2.utils.JSONArray import com.ichi2.utils.JSONObject @@ -235,7 +236,14 @@ class DecksV16(private val col: CollectionV16) : @Throws(DeckRenameException::class) fun save(g: DeckV16) { - this.update(g, preserve_usn = false) + // legacy code expects preserve_usn=false behaviour, but that + // causes a backup entry to be created, which invalidates the + // v2 review history. So we manually update the usn/mtime here + g.getJsonObject().run { + put("mod", time.intTime()) + put("usn", col.usn()) + } + this.update(g, preserve_usn = true) } @RustCleanup("unused in V16") @@ -421,6 +429,9 @@ class DecksV16(private val col: CollectionV16) : } @Throws(DeckRenameException::class) + /** This skips the backend undo queue, so is required instead of save() + * to avoid clobbering the v2 review queue. + */ override fun update(g: Deck) { // we preserve USN here as this method is used for syncing and merging update(DeckV16.Generic(g), preserve_usn = true) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt index 14f54a3df79e..175fdc85ad67 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/ModelsV16.kt @@ -119,7 +119,12 @@ class ModelsV16(col: CollectionV16) : ModelManager(col) { Timber.w("a null model is no longer supported - data is automatically flushed") return } - update(m, preserve_usn_and_mtime = false) + // legacy code expects preserve_usn=false behaviour, but that + // causes a backup entry to be created, which invalidates the + // v2 review history. So we manually update the usn/mtime here + m.put("mod", TimeManager.time.intTime()) + m.put("usn", col.usn()) + update(m, preserve_usn_and_mtime = true) } @RustCleanup("not required - java only") diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustConfigBackend.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustConfigBackend.kt index fae783290862..ecf00d12130b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustConfigBackend.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustConfigBackend.kt @@ -59,7 +59,7 @@ class RustConfigBackend(private val backend: Backend) { } fun set(key: str, value: Any?) { - backend.setConfigJson(key, to_json_bytes(value), true) + backend.setConfigJson(key, to_json_bytes(value), false) } fun remove(key: str) { diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt index 322337121e32..66ae896f8fbe 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerKeyboardInputTest.kt @@ -34,6 +34,7 @@ import com.ichi2.anki.servicelayer.SchedulerService.SuspendNote import com.ichi2.libanki.Card import com.ichi2.utils.Computation import com.ichi2.utils.KotlinCleanup +import kotlinx.coroutines.Job import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.Test @@ -368,8 +369,9 @@ class ReviewerKeyboardInputTest : RobolectricTest() { handleKeyPress(buttonCode, '\u0000') } - override fun undo() { + override fun undo(): Job? { undoCalled = true + return null } val suspendNoteCalled: Boolean diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerNoParamTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerNoParamTest.kt index 7a44d93f510e..0942ed1245c5 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerNoParamTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerNoParamTest.kt @@ -154,7 +154,8 @@ class ReviewerNoParamTest : RobolectricTest() { val hideCount = reviewer.delayedHideCount - reviewer.executeCommand(ViewerCommand.UNDO) + awaitJob(reviewer.undo()) + advanceRobolectricLooperWithSleep() assertThat("Hide should be called after answering a card", reviewer.delayedHideCount, greaterThan(hideCount)) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerTest.kt index a8bc601b1dfa..ce9a4ab01e7f 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/ReviewerTest.kt @@ -286,8 +286,7 @@ class ReviewerTest : RobolectricTest() { } private fun undo(reviewer: Reviewer) { - reviewer.undo() - waitForAsyncTasksToComplete() + awaitJob(reviewer.undo()) } @Suppress("SameParameterValue") diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt index 59564cc2c1ad..2d7190daa011 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt @@ -48,6 +48,8 @@ import com.ichi2.testutils.TaskSchedulerRule import com.ichi2.utils.Computation import com.ichi2.utils.InMemorySQLiteOpenHelperFactory import com.ichi2.utils.JSONException +import kotlinx.coroutines.Job +import kotlinx.coroutines.runBlocking import net.ankiweb.rsdroid.BackendException import net.ankiweb.rsdroid.testing.RustBackendLoader import org.hamcrest.Matcher @@ -59,6 +61,7 @@ import org.robolectric.Shadows import org.robolectric.android.controller.ActivityController import org.robolectric.shadows.ShadowDialog import org.robolectric.shadows.ShadowLog +import org.robolectric.shadows.ShadowLooper import timber.log.Timber import java.util.concurrent.locks.ReentrantLock import kotlin.concurrent.withLock @@ -206,6 +209,17 @@ open class RobolectricTest : CollectionGetter { return dialog.view.contentLayout.findViewById(R.id.md_text_message).text.toString() } + fun awaitJob(job: Job?) { + job?.let { + runBlocking { + while (!job.isCompleted) { + waitForAsyncTasksToComplete() + ShadowLooper.runUiThreadTasksIncludingDelayedTasks() + } + } + } + } + // Robolectric needs a manual advance with the new PAUSED looper mode companion object { private var mBackground = true From e432986b12f3d04b9d5182395ea842df04bf7d8f Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Thu, 30 Jun 2022 16:07:15 +1000 Subject: [PATCH 11/15] Add `@BlocksSchemaUpgrade` --- .../src/main/java/com/ichi2/anki/BackendUndo.kt | 3 ++- .../src/main/java/com/ichi2/anki/ModelBrowser.kt | 3 ++- .../src/main/java/com/ichi2/utils/KotlinCleanup.kt | 13 +++++++++++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt b/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt index 1698db1edca5..ee302b20221b 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/BackendUndo.kt @@ -20,6 +20,7 @@ import com.ichi2.anki.UIUtils.showSimpleSnackbar import com.ichi2.libanki.CollectionV16 import com.ichi2.libanki.undoNew import com.ichi2.libanki.undoableOp +import com.ichi2.utils.BlocksSchemaUpgrade import net.ankiweb.rsdroid.BackendException suspend fun AnkiActivity.backendUndoAndShowPopup(col: CollectionV16): Boolean { @@ -36,7 +37,7 @@ suspend fun AnkiActivity.backendUndoAndShowPopup(col: CollectionV16): Boolean { ) true } catch (exc: BackendException) { - // FIXME: -Backend module should export this as a separate Exception + @BlocksSchemaUpgrade("Backend module should export this as a separate Exception") if (exc.localizedMessage == "UndoEmpty") { // backend undo queue empty false diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt index a6a4e84d2a57..2aa6c63e3f71 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt @@ -47,6 +47,7 @@ import com.ichi2.libanki.StdModels import com.ichi2.libanki.Utils import com.ichi2.libanki.utils.TimeManager import com.ichi2.ui.FixedEditText +import com.ichi2.utils.BlocksSchemaUpgrade import com.ichi2.utils.KotlinCleanup import com.ichi2.utils.displayKeyboard import com.ichi2.widget.WidgetStatus.update @@ -427,7 +428,7 @@ class ModelBrowser : AnkiActivity() { */ private fun openTemplateEditor() { if (!BackendFactory.defaultLegacySchema) { - // this screen needs rewriting for the new backend + @BlocksSchemaUpgrade("this screen needs rewriting for the new backend") AlertDialog.Builder(this).setTitle("Not yet supported on new backend").show() return } diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/KotlinCleanup.kt b/AnkiDroid/src/main/java/com/ichi2/utils/KotlinCleanup.kt index 07213b89f4c7..c6f1b8691421 100644 --- a/AnkiDroid/src/main/java/com/ichi2/utils/KotlinCleanup.kt +++ b/AnkiDroid/src/main/java/com/ichi2/utils/KotlinCleanup.kt @@ -27,3 +27,16 @@ package com.ichi2.utils @Repeatable @MustBeDocumented annotation class KotlinCleanup(val value: String) + +/** This must be fixed/implemented before AnkiDroid can switch to using the new backend code + * by default. + */ +@Target( + AnnotationTarget.CLASS, AnnotationTarget.FUNCTION, + AnnotationTarget.VALUE_PARAMETER, AnnotationTarget.EXPRESSION, + AnnotationTarget.FIELD, AnnotationTarget.PROPERTY, AnnotationTarget.LOCAL_VARIABLE +) +@Retention(AnnotationRetention.SOURCE) +@Repeatable +@MustBeDocumented +annotation class BlocksSchemaUpgrade(val value: String) From 18eb265b11585808e00876ab58a1a49d2fac84a7 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Mon, 18 Jul 2022 13:49:01 +1000 Subject: [PATCH 12/15] Support deleting/undoing deck from deck picker + Delete the cards inside the default deck if user tries to delete it Closes #11703 --- .../main/java/com/ichi2/anki/DeckPicker.kt | 32 ++++++++++++++----- .../java/com/ichi2/anki/DeckPickerTest.kt | 7 +++- .../anki/dialogs/CreateDeckDialogTest.kt | 8 +++-- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 5223ca3b0db5..9fe2a3ab77ef 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -90,11 +90,8 @@ import com.ichi2.async.CollectionTask.* import com.ichi2.async.Connection.CancellableTaskListener import com.ichi2.async.Connection.ConflictResolution import com.ichi2.compat.CompatHelper.Companion.sdkVersion -import com.ichi2.libanki.ChangeManager +import com.ichi2.libanki.* import com.ichi2.libanki.Collection.CheckDatabaseResult -import com.ichi2.libanki.Consts -import com.ichi2.libanki.Decks -import com.ichi2.libanki.Utils import com.ichi2.libanki.importer.AnkiPackageImporter import com.ichi2.libanki.sched.AbstractDeckTreeNode import com.ichi2.libanki.sched.TreeNode @@ -106,6 +103,7 @@ import com.ichi2.ui.BadgeDrawableBuilder import com.ichi2.utils.* import com.ichi2.utils.Permissions.hasStorageAccessPermission import com.ichi2.widget.WidgetStatus +import kotlinx.coroutines.Job import net.ankiweb.rsdroid.BackendFactory import net.ankiweb.rsdroid.RustCleanup import timber.log.Timber @@ -2241,15 +2239,32 @@ open class DeckPicker : createDeckDialog.showDialog() } - fun confirmDeckDeletion(did: Long) { + fun confirmDeckDeletion(did: Long): Job? { + if (!BackendFactory.defaultLegacySchema) { + dismissAllDialogFragments() + // No confirmation required, as undoable + return launchCatchingCollectionTask { col -> + val changes = runInBackgroundWithProgress { + undoableOp { + col.newDecks.removeDecks(listOf(did)) + } + } + showSimpleSnackbar( + this@DeckPicker, + col.tr.browsingCardsDeleted(changes.count), + false + ) + } + } + val res = resources if (!colIsOpen()) { - return + return null } if (did == 1L) { showSimpleSnackbar(this, R.string.delete_deck_default_deck, true) dismissAllDialogFragments() - return + return null } // Get the number of cards contained in this deck and its subdecks val cnt = DeckService.countCardsInDeckTree(col, did) @@ -2258,7 +2273,7 @@ open class DeckPicker : if (cnt == 0 && !isDyn) { deleteDeck(did) dismissAllDialogFragments() - return + return null } // Otherwise we show a warning and require confirmation val msg: String @@ -2269,6 +2284,7 @@ open class DeckPicker : res.getQuantityString(R.plurals.delete_deck_message, cnt, deckName, cnt) } showDialogFragment(DeckPickerConfirmDeleteDeckDialog.newInstance(msg, did)) + return null } /** diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt index 4c8de11c91c1..9e8af07fdf1b 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/DeckPickerTest.kt @@ -17,6 +17,7 @@ import com.ichi2.testutils.BackupManagerTestUtilities import com.ichi2.testutils.DbUtils import com.ichi2.utils.KotlinCleanup import com.ichi2.utils.ResourceLoader +import net.ankiweb.rsdroid.BackendFactory import org.apache.commons.exec.OS import org.hamcrest.MatcherAssert.* import org.hamcrest.Matchers.* @@ -207,13 +208,17 @@ class DeckPickerTest : RobolectricTest() { val deckPicker = startActivityNormallyOpenCollectionWithIntent( DeckPicker::class.java, Intent() ) - deckPicker.confirmDeckDeletion(did) + awaitJob(deckPicker.confirmDeckDeletion(did)) advanceRobolectricLooperWithSleep() assertThat("deck was deleted", col.decks.count(), `is`(1)) } @Test fun deletion_of_filtered_deck_shows_warning_issue_10238() { + if (!BackendFactory.defaultLegacySchema) { + // undoable + return + } // Filtered decks contain their own options, deleting one can cause a significant loss of work. // And they are more likely to be empty temporarily val did = addDynamicDeck("filtered") diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CreateDeckDialogTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CreateDeckDialogTest.kt index aa2c6993d3df..839f52da905c 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CreateDeckDialogTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CreateDeckDialogTest.kt @@ -164,7 +164,7 @@ class CreateDeckDialogTest : RobolectricTest() { // After the last deck was created, delete a deck if (decks.count() >= 10) { - deckPicker.confirmDeckDeletion(did) + awaitJob(deckPicker.confirmDeckDeletion(did)) assertEquals(deckCounter.decrementAndGet(), decks.count()) assertEquals(deckCounter.get(), decks.count()) @@ -188,7 +188,8 @@ class CreateDeckDialogTest : RobolectricTest() { deckPicker.updateDeckList() assertTrue(deckPicker.searchDecksIcon!!.isVisible) - deckPicker.confirmDeckDeletion(decks.id("Deck0::Deck1")) + awaitJob(deckPicker.confirmDeckDeletion(decks.id("Deck0::Deck1"))) + assertEquals(2, decks.count()) deckPicker.updateDeckList() assertFalse(deckPicker.searchDecksIcon!!.isVisible) @@ -201,7 +202,8 @@ class CreateDeckDialogTest : RobolectricTest() { deckPicker.updateDeckList() assertTrue(deckPicker.searchDecksIcon!!.isVisible) - deckPicker.confirmDeckDeletion(decks.id("Deck0::Deck1")) + awaitJob(deckPicker.confirmDeckDeletion(decks.id("Deck0::Deck1"))) + assertEquals(2, decks.count()) deckPicker.updateDeckList() assertFalse(deckPicker.searchDecksIcon!!.isVisible) From f9939a7e67071e58ef7066f29935d18931a91b22 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 6 Jul 2022 15:51:42 +1000 Subject: [PATCH 13/15] Re-enable card template editor in new schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CardTemplateEditorTest is no longer failing with the latest code 🎉 --- .../src/main/java/com/ichi2/anki/ModelBrowser.kt | 8 -------- .../java/com/ichi2/anki/CardTemplateEditorTest.kt | 12 ------------ 2 files changed, 20 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt b/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt index 2aa6c63e3f71..b3dc5dde4ccd 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/ModelBrowser.kt @@ -26,7 +26,6 @@ import android.widget.AdapterView.OnItemLongClickListener import androidx.activity.result.ActivityResult import androidx.activity.result.contract.ActivityResultContracts import androidx.appcompat.app.ActionBar -import androidx.appcompat.app.AlertDialog import com.afollestad.materialdialogs.MaterialDialog import com.afollestad.materialdialogs.customview.customView import com.ichi2.anim.ActivityTransitionAnimation @@ -47,11 +46,9 @@ import com.ichi2.libanki.StdModels import com.ichi2.libanki.Utils import com.ichi2.libanki.utils.TimeManager import com.ichi2.ui.FixedEditText -import com.ichi2.utils.BlocksSchemaUpgrade import com.ichi2.utils.KotlinCleanup import com.ichi2.utils.displayKeyboard import com.ichi2.widget.WidgetStatus.update -import net.ankiweb.rsdroid.BackendFactory import timber.log.Timber import java.lang.RuntimeException import java.util.ArrayList @@ -427,11 +424,6 @@ class ModelBrowser : AnkiActivity() { * the user to edit the current note's templates. */ private fun openTemplateEditor() { - if (!BackendFactory.defaultLegacySchema) { - @BlocksSchemaUpgrade("this screen needs rewriting for the new backend") - AlertDialog.Builder(this).setTitle("Not yet supported on new backend").show() - return - } val intent = Intent(this, CardTemplateEditor::class.java) intent.putExtra("modelId", mCurrentID) launchActivityForResultWithAnimation(intent, mEditTemplateResultLauncher, ActivityTransitionAnimation.Direction.START) diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt index 5716ed587c67..914e904c4b5e 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/CardTemplateEditorTest.kt @@ -26,7 +26,6 @@ import com.ichi2.anki.dialogs.DeckSelectionDialog.SelectableDeck import com.ichi2.libanki.Model import com.ichi2.testutils.assertFalse import com.ichi2.utils.JSONObject -import net.ankiweb.rsdroid.BackendFactory import org.hamcrest.MatcherAssert import org.hamcrest.Matchers import org.junit.Test @@ -352,17 +351,6 @@ class CardTemplateEditorTest : RobolectricTest() { assertEquals("Change in database despite no change?", collectionBasicModelOriginal.toString().trim { it <= ' ' }, getCurrentDatabaseModelCopy(modelName).toString().trim { it <= ' ' }) assertEquals("Model should have 2 templates still", 2, testEditor.tempModel?.templateCount) - if (!BackendFactory.defaultLegacySchema) { - // the new backend behaves differently, which breaks these tests: - // - multiple templates with identical question format can't be saved - // - if that check is patched out, the test fails later with 3 cards remaining instead - // of 2 after deleting - - // rather than attempting to fix this, it's probably worth rewriting this screen - // to use the backend logic and cutting out these tests - return - } - // Add a template - click add, click confirm for card add, click confirm again for full sync addCardType(testEditor, shadowTestEditor) assertTrue("Model should have changed", testEditor.modelHasChanged()) From 63531c33cc4212e6fe6dbcf1ba8ff6a4ce50fd9f Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 6 Jul 2022 15:00:09 +1000 Subject: [PATCH 14/15] Fix full download crashing in legacy schema case mCol was null. Technically libanki code should not be making reference to GUI code, but this code should be retired once AD has migrated to the new background syncing code anyway. --- AnkiDroid/src/main/java/com/ichi2/libanki/sync/FullSyncer.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/libanki/sync/FullSyncer.kt b/AnkiDroid/src/main/java/com/ichi2/libanki/sync/FullSyncer.kt index c3976ab7d95b..52c946640183 100644 --- a/AnkiDroid/src/main/java/com/ichi2/libanki/sync/FullSyncer.kt +++ b/AnkiDroid/src/main/java/com/ichi2/libanki/sync/FullSyncer.kt @@ -107,7 +107,7 @@ class FullSyncer(col: Collection?, hkey: String?, con: Connection, hostNum: Host mCon.publishProgress(R.string.sync_check_download_file) var tempDb: DB? = null try { - tempDb = DB.withAndroidFramework(mCol!!.context, tpath) + tempDb = DB.withAndroidFramework(AnkiDroidApp.getInstance(), tpath) if (!"ok".equals(tempDb.queryString("PRAGMA integrity_check"), ignoreCase = true)) { Timber.e("Full sync - downloaded file corrupt") return ConnectionResultType.REMOTE_DB_ERROR From 47448c626132b183ed1e2a0473a4b0db3dfa1a38 Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Wed, 6 Jul 2022 18:14:47 +1000 Subject: [PATCH 15/15] Fix full download crashing in new schema case This worked in my initial testing, but with a smaller collection, the UI is now racing with the download on my machine: - getSyncStatus() was assuming col.db would still be open by the time it gets around to calling methods on it - onCreateOptionsMenu() likewise assumes that if colIsOpen() returns true, it is free to call methods on it A better way way to fix this would be to wrap all collection access in a mutex, so that the collection is never touched without the lock being acquired. Code holding the mutex could then be guaranteed that the collection is open and valid for the duration of its run. Changing all that legacy code is a big task however. I've added a lock to the upload case as well, as it's possible it's likewise buggy and just not triggering on my machine. --- .../src/main/java/com/ichi2/anki/DeckPicker.kt | 2 +- AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt | 18 ++++++++++++------ .../main/java/com/ichi2/utils/SyncStatus.kt | 9 +++++---- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 9fe2a3ab77ef..89d6dbe96ad4 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -619,7 +619,7 @@ open class DeckPicker : return true } }) - if (colIsOpen()) { + if (colIsOpen() && !CollectionHelper.getInstance().isCollectionLocked) { displaySyncBadge(menu) // Show / hide undo diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt index 6254682b95d4..ea74974e3ca3 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt @@ -166,16 +166,19 @@ private suspend fun handleDownload( extractProgress = fullDownloadProgress(col.tr.syncDownloadingFromAnkiweb()), onCancel = ::cancelSync ) { - col.createBackup( - BackupManager.getBackupDirectoryFromCollection(col.path), - force = true, - waitForCompletion = true - ) - col.close(save = true, downgrade = false, forFullSync = true) + val helper = CollectionHelper.getInstance() + helper.lockCollection() try { + col.createBackup( + BackupManager.getBackupDirectoryFromCollection(col.path), + force = true, + waitForCompletion = true + ) + col.close(save = true, downgrade = false, forFullSync = true) col.fullDownload(auth) } finally { col.reopen(afterFullSync = true) + helper.unlockCollection() } } @@ -193,11 +196,14 @@ private suspend fun handleUpload( extractProgress = fullDownloadProgress(col.tr.syncUploadingToAnkiweb()), onCancel = ::cancelSync ) { + val helper = CollectionHelper.getInstance() + helper.lockCollection() col.close(save = true, downgrade = false, forFullSync = true) try { col.fullUpload(auth) } finally { col.reopen(afterFullSync = true) + helper.unlockCollection() } } Timber.i("Full Upload Completed") diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/SyncStatus.kt b/AnkiDroid/src/main/java/com/ichi2/utils/SyncStatus.kt index 3f6d0420acfe..eac41f2acbf0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/utils/SyncStatus.kt +++ b/AnkiDroid/src/main/java/com/ichi2/utils/SyncStatus.kt @@ -30,14 +30,15 @@ enum class SyncStatus { @JvmStatic fun getSyncStatus(getCol: Supplier): SyncStatus { - val col: Collection - col = try { - getCol.get() + return try { + val col = getCol.get() + // may fail when the collection is closed for a full sync, + // as col.db is null + getSyncStatus(col) } catch (e: Exception) { Timber.w(e) return INCONCLUSIVE } - return getSyncStatus(col) } @JvmStatic