Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the V3 scheduler; drop support for V1 #11808

Merged
merged 10 commits into from
Jul 27, 2022
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,12 @@
import com.ichi2.libanki.StdModels;
import com.ichi2.libanki.Utils;

import com.ichi2.utils.BlocksSchemaUpgrade;
import com.ichi2.utils.JSONArray;
import com.ichi2.utils.JSONObject;

import net.ankiweb.rsdroid.BackendFactory;

import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
Expand Down Expand Up @@ -135,7 +139,9 @@ public static java.util.Collection<Object[]> initParameters() {
* Initially create one note for each model.
*/
@Before
@BlocksSchemaUpgrade("some of these tests are failing; need to investigate why")
dae marked this conversation as resolved.
Show resolved Hide resolved
public void setUp() throws Exception {
assumeThat(BackendFactory.getDefaultLegacySchema(), is(true));
Timber.i("setUp()");
mCreatedNotes = new ArrayList<>();
final Collection col = getCol();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ package com.ichi2.anki.tests

import com.ichi2.libanki.Storage
import net.ankiweb.rsdroid.BackendException
import net.ankiweb.rsdroid.BackendFactory
import org.hamcrest.MatcherAssert
import org.hamcrest.Matchers.equalTo
import org.junit.Assume.assumeThat
import org.junit.Rule
import org.junit.Test
import org.junit.rules.Timeout
Expand All @@ -35,6 +37,7 @@ class RustTest : InstrumentedTest() {
@Test
@Throws(BackendException::class, IOException::class)
fun collectionIsVersion11AfterOpen() {
assumeThat(BackendFactory.defaultLegacySchema, equalTo(true))
// This test will be decommissioned, but before we get an upgrade strategy, we need to ensure we're not upgrading the database.
val path = Shared.getTestFilePath(testContext, "initial_version_2_12_1.anki2")
val collection = Storage.collection(testContext, path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
import com.ichi2.utils.JSONException;
import com.ichi2.utils.JSONObject;

import net.ankiweb.rsdroid.BackendFactory;

import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
Expand All @@ -57,8 +59,10 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeThat;

@RunWith(AndroidJUnit4.class)
public class ImportTest extends InstrumentedTest {
Expand Down Expand Up @@ -86,6 +90,8 @@ public class ImportTest extends InstrumentedTest {
@Before
public void setUp() throws IOException {
mTestCol = getEmptyCol();
// the backend provides its own importing methods
assumeThat(BackendFactory.getDefaultLegacySchema(), is(true));
}

@After
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import com.ichi2.libanki.Note;
import com.ichi2.libanki.exception.EmptyMediaException;

import net.ankiweb.rsdroid.BackendFactory;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
Expand All @@ -47,6 +49,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeThat;


/**
Expand Down Expand Up @@ -221,6 +224,8 @@ private List<String> removed(Collection d) {

@Test
public void testChanges() throws IOException, EmptyMediaException {
// legacy code, not used by backend
assumeThat(BackendFactory.getDefaultLegacySchema(), is(true));
assertNotNull(mTestCol.getMedia()._changed());
assertEquals(0, added(mTestCol).size());
assertEquals(0, removed(mTestCol).size());
Expand Down
3 changes: 3 additions & 0 deletions AnkiDroid/src/androidTest/java/com/ichi2/libanki/DBTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@ package com.ichi2.libanki

import androidx.test.ext.junit.runners.AndroidJUnit4
import com.ichi2.anki.tests.InstrumentedTest
import net.ankiweb.rsdroid.BackendFactory
import org.hamcrest.MatcherAssert.assertThat
import org.hamcrest.Matchers.*
import org.junit.Assume.assumeThat
import org.junit.Test
import org.junit.runner.RunWith
import java.util.*
Expand All @@ -28,6 +30,7 @@ class DBTest : InstrumentedTest() {
/** mDatabase.disableWriteAheadLogging(); is called in DB init */
@Test
fun writeAheadLoggingIsDisabled() {
assumeThat(BackendFactory.defaultLegacySchema, equalTo(true))
// An old comment noted that explicitly disabling the WAL was no longer necessary after API 16:
// https://github.com/ankidroid/Anki-Android/commit/6e34663ba9d09dc8b023230811c3185b72ee7eec#diff-4fdbf41d84a547a45edad66ae1f543128d1118b0e831a12916b4fac11b483688

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ import com.ichi2.libanki.Collection
import com.ichi2.libanki.Consts.BUTTON_TYPE
import com.ichi2.libanki.Sound.SoundSide
import com.ichi2.libanki.sched.AbstractSched
import com.ichi2.libanki.sched.SchedV2
import com.ichi2.themes.Themes
import com.ichi2.themes.Themes.getResFromAttr
import com.ichi2.ui.FixedEditText
Expand Down Expand Up @@ -634,8 +635,8 @@ abstract class AbstractFlashcardViewer :
super.onDestroy()
// Tells the scheduler there is no more current cards. 0 is
// not a valid id.
if (sched != null) {
sched!!.discardCurrentCard()
if (sched != null && sched is SchedV2) {
(sched!! as SchedV2).discardCurrentCard()
}
Timber.d("onDestroy()")
mTTS.releaseTts(this)
Expand Down Expand Up @@ -2582,7 +2583,7 @@ abstract class AbstractFlashcardViewer :
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(
GetCard().runWithHandler(
answerCardHandler(false)
)
}
Expand Down
4 changes: 2 additions & 2 deletions AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ public void onCreate() {
// Forget the last deck that was used in the CardBrowser
CardBrowser.clearLastDeckId();

LanguageUtil.setDefaultBackendLanguages();

// Create the AnkiDroid directory if missing. Send exception report if inaccessible.
if (Permissions.hasStorageAccessPermission(this)) {
try {
Expand Down Expand Up @@ -317,8 +319,6 @@ public static Context updateContextWithLanguage(@NonNull Context remoteContext)
preferences = getSharedPrefs(remoteContext);
}
Configuration langConfig = getLanguageConfig(remoteContext.getResources().getConfiguration(), preferences);
// TODO: support fallback languages (backend already automatically adds English to the end)
BackendFactory.INSTANCE.setDefaultLanguagesFromLocales(Arrays.asList(langConfig.locale));
return remoteContext.createConfigurationContext(langConfig);
} catch (Exception e) {
Timber.e(e, "failed to update context with new language");
Expand Down
5 changes: 3 additions & 2 deletions AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,12 @@ synchronized void discardBackend() {

/**
* Get the single instance of the {@link Collection}, creating it if necessary (lazy initialization).
* @param context context which can be used to get the setting for the path to the Collection
* @param _context is no longer used, as the global AnkidroidApp instance is used instead
* @return instance of the Collection
*/
public synchronized Collection getCol(Context context) {
public synchronized Collection getCol(Context _context) {
// Open collection
Context context = AnkiDroidApp.getInstance();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one - thanks - hopefully avoids the semi-false positives I was seeing with LeakCanary which were making me think things were crashing

if (!colIsOpen()) {
String path = getCollectionPath(context);
// Check that the directory has been created and initialized
Expand Down
29 changes: 28 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/CoroutineHelpers.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@ import androidx.appcompat.app.AlertDialog
import androidx.lifecycle.coroutineScope
import anki.collection.Progress
import com.ichi2.anki.UIUtils.showSimpleSnackbar
import com.ichi2.libanki.Collection
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
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine

/**
* Launch a job that catches any uncaught errors and reports them to the user.
Expand Down Expand Up @@ -127,7 +130,7 @@ suspend fun <T> AnkiActivity.runInBackgroundWithProgress(
* window with the provided message.
*/
suspend fun <T> AnkiActivity.runInBackgroundWithProgress(
message: String = "",
message: String = resources.getString(R.string.dialog_processing),
op: suspend () -> T
): T = withProgressDialog(
context = this@runInBackgroundWithProgress,
Expand Down Expand Up @@ -205,3 +208,27 @@ private fun ProgressContext.updateDialog(dialog: android.app.ProgressDialog) {
@Suppress("Deprecation") // ProgressDialog deprecation
dialog.setMessage(text + progressText)
}

/**
* If a full sync is not already required, confirm the user wishes to proceed.
* If the user agrees, the schema is bumped and the routine will return true.
* On false, calling routine should abort.
*/
suspend fun AnkiActivity.userAcceptsSchemaChange(col: Collection): Boolean {
if (col.schemaChanged()) {
return true
}
return suspendCoroutine { coroutine ->
AlertDialog.Builder(this)
// generic message
.setMessage(col.tr.deckConfigWillRequireFullSync())
.setPositiveButton(R.string.dialog_ok) { _, _ ->
col.modSchemaNoCheck()
coroutine.resume(true)
}
.setNegativeButton(R.string.dialog_cancel) { _, _ ->
coroutine.resume(false)
}
.show()
}
}
60 changes: 60 additions & 0 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import android.view.WindowManager.BadTokenException
import android.widget.*
import androidx.annotation.StringRes
import androidx.annotation.VisibleForTesting
import androidx.appcompat.app.AlertDialog
import androidx.appcompat.widget.SearchView
import androidx.core.app.ActivityCompat
import androidx.core.app.ActivityCompat.OnRequestPermissionsResultCallback
Expand Down Expand Up @@ -1950,11 +1951,41 @@ open class DeckPicker :
}
}

private fun promptUserToUpdateScheduler() {
val builder = AlertDialog.Builder(this)
.setMessage(col.tr.schedulingUpdateRequired())
.setPositiveButton(R.string.dialog_ok) { _, _ ->
launchCatchingTask {
if (!userAcceptsSchemaChange(col)) {
return@launchCatchingTask
}
runInBackgroundWithProgress {
CollectionHelper.getInstance().updateScheduler(this@DeckPicker)
}
showThemedToast(this@DeckPicker, col.tr.schedulingUpdateDone(), false)
refreshState()
}
}
.setNegativeButton(R.string.dialog_cancel) { _, _ ->
// nothing to do
}
if (AdaptionUtil.hasWebBrowser(this)) {
builder.setNeutralButton(col.tr.schedulingUpdateMoreInfoButton()) { _, _ ->
this.openUrl(Uri.parse("https://faqs.ankiweb.net/the-anki-2.1-scheduler.html#updating"))
mikehardy marked this conversation as resolved.
Show resolved Hide resolved
}
}
builder.show()
}

private fun handleDeckSelection(did: Long, selectionType: DeckSelectionType) {
// Clear the undo history when selecting a new deck
if (col.decks.selected() != did) {
col.clearUndo()
}
if (col.get_config_int("schedVer") == 1) {
promptUserToUpdateScheduler()
return
}
// Select the deck
col.decks.select(did)
// Also forget the last deck used by the Browser
Expand Down Expand Up @@ -2655,3 +2686,32 @@ open class DeckPicker :
}
}
}

/** Upgrade from v1 to v2 scheduler.
* Caller must have confirmed schema modification already.
*/
@KotlinCleanup("move into CollectionHelper once it's converted to Kotlin")
@Synchronized
fun CollectionHelper.updateScheduler(context: Context) {
if (BackendFactory.defaultLegacySchema) {
// We'll need to temporarily update to the latest schema.
closeCollection(true, "sched upgrade")
discardBackend()
BackendFactory.defaultLegacySchema = false
// Ensure collection closed if upgrade fails, and schema reverted
// even if close fails.
try {
try {
getCol(context).sched.upgradeToV2()
} finally {
closeCollection(true, "sched upgrade")
}
} finally {
BackendFactory.defaultLegacySchema = true
discardBackend()
}
} else {
// Can upgrade directly
getCol(context).sched.upgradeToV2()
}
}
7 changes: 6 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/Preferences.kt
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,13 @@ class Preferences : AnkiActivity() {
// It's only possible to change the language by recreating the activity,
// so do it if the language has changed.
// TODO recreate the activity and keep its previous state instead of just closing it
languageSelection.setOnPreferenceChangeListener { _ ->
languageSelection.setOnPreferenceChangeListener { _, newValue ->
LanguageUtil.setDefaultBackendLanguages(newValue as String)
val helper = CollectionHelper.getInstance()
helper.closeCollection(true, "language change")
helper.discardBackend()
(requireActivity() as Preferences).closePreferences()
true
}
}
}
Expand Down
1 change: 0 additions & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/Reviewer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,6 @@ open class Reviewer : AbstractFlashcardViewer() {
Counts.Queue.NEW -> mNewCount!!.setSpan(UnderlineSpan(), 0, mNewCount!!.length, 0)
Counts.Queue.LRN -> mLrnCount!!.setSpan(UnderlineSpan(), 0, mLrnCount!!.length, 0)
Counts.Queue.REV -> mRevCount!!.setSpan(UnderlineSpan(), 0, mRevCount!!.length, 0)
else -> Timber.w("Unknown card type %s", sched!!.countIdx(mCurrentCard!!))
}
mTextBarNew.text = mNewCount
mTextBarLearn.text = mLrnCount
Expand Down
2 changes: 1 addition & 1 deletion AnkiDroid/src/main/java/com/ichi2/async/CollectionTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ open class CollectionTask<Progress, Result>(val task: TaskDelegateBase<Progress,
Timber.d("doInBackgroundLoadDeckCounts")
return try {
// Get due tree
col.sched.deckDueTree(collectionTask)!!
col.sched.deckDueTree(collectionTask)
} catch (e: RuntimeException) {
Timber.e(e, "doInBackgroundLoadDeckCounts - error")
null
Expand Down
Loading