Skip to content

Commit

Permalink
Update to Anki 2.1.54 backend
Browse files Browse the repository at this point in the history
Squashes a few commits:

Move the legacy schema toggle into BackendFactory

Simplify backend handling; rework collection instantiation

When the Rust code was initially introduced, it was not clear whether
it would be usable on all devices, or whether it would need to be removed
for some reason. This no doubt influenced the design of the existing API,
which tries to make it easy to swap the Rust code out with something else.

Unfortunately this approach has some downsides:

- It makes it somewhat harder to follow, as method calls jump through
multiple interfaces before they're actually sent to the backend.
- It makes utilizing new methods considerably more cumbersome.

For example, take the extract_av_tags() call. It follows the following
path:

collection method or method in related helper class:
https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/TemplateManager.kt#L242

to generic interface:
https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackend.kt#L83

to specific implementation:
https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidV16Backend.kt#L57

and if it's unusable with the legacy schema (which I don't believe is
actually true in this case), it also needs to be added to the other
implementation:
https://github.com/ankidroid/Anki-Android/blob/cea79e1b077bc30e7eed8f37529002aae416d34d/AnkiDroid/src/main/java/com/ichi2/libanki/backend/RustDroidBackend.kt#L87

and then finally, a method in the backend module is invoked.

The backend module has code generation so that invoking a backend method
is as simple as making a method call, but currently you have to weave
the call through 3 or so levels of indirection before you can actually
use it. With something like 170 available methods, that's a fair amount
of extra work required.

Rather than trying to insulate libanki from the backend code, this PR
drops some of the indirection in favour of the approach the desktop takes:
libanki is the insulation layer; it can call freely into the backend methods,
but consumers (eg the GUI code) are expected to only call methods on the
collection, and not access the backend directly.

In addition to the above, collection initialization has been reworked
to be more similar to the computer version. Instead of the collection
being created from a database object, a backend is passed into the
collection creation, and the collection takes care of creating a DB
instance that wraps the backend.

Remove always-on isUsingRustBackend

Drop the legacy upgrade/initialization code

Schema 11 was introduced in 2012, and decks that still are <11 are very
rare. The desktop dropped support for schema 10 back in early 2020.

This also removes the need to modify SCHEMA_VERSION when switching
between TESTING_USE_V16_BACKEND.

Remove DOWNGRADE_REQUIRED and slightly simplify startup error handling

- The backend automatically downgrades when required, and possible. No
backup is required, as the downgrade happens in a single transaction,
and the downgrade code has proven itself over time.
- Store the type of failure in getColSafe(), so it can be checked later.

Update to work with desktop 2.1.53 code

Depends on ankidroid/Anki-Android-Backend#202

Due to the removal and change of a few backend methods, syncing, importing
and the card templates screen will not work when the schema16 setting is
active (actually schema18 now). To get them working again, those code
paths will need to switch to the backend implementations.

A few notes:
- Downgrading happens automatically when loading the collection in schema11
mode, so the extra code dealing with downgrades & "can downgrade" reporting
can be stripped.
- Added the ability to run col.set_config("key", JSONObject.NULL), as
unit tests were attempting to write the entire collection config, which is
no longer supported.
- All tests pass on both old and new backends, though the latter required
disabling a few failed tests when running with the new schema
(eg notetype updating).

Integrates, and thus closes ankidroid#11579 and closes ankidroid#11581

Remove the time argument to Storage.collection()

Collection does not currently take a time argument, and relies on the
global object instead, so this change brings Storage in line with it.

Reuse the backend when closing+reopening a collection

Avoids having to re-initialize the translations, and reduces leaks
(the import + export code leaks backends still)
  • Loading branch information
dae authored and dorrin-sot committed Jul 14, 2022
1 parent 821afeb commit 464e484
Show file tree
Hide file tree
Showing 71 changed files with 681 additions and 1,408 deletions.
6 changes: 4 additions & 2 deletions AnkiDroid/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ dependencies {

// Backend libraries

implementation 'com.google.protobuf:protobuf-java:3.21.1' // This is required when loading from a file
implementation 'com.google.protobuf:protobuf-kotlin:3.21.2' // This is required when loading from a file

// To test with locally-built versions:
// - use the docs in Anki-Android-Backend to build a local version
Expand Down Expand Up @@ -333,7 +333,9 @@ dependencies {

// May need a resolution strategy for support libs to our versions
androidTestImplementation 'androidx.test.espresso:espresso-core:3.4.0'
androidTestImplementation 'androidx.test.espresso:espresso-contrib:3.4.0'
androidTestImplementation('androidx.test.espresso:espresso-contrib:3.4.0') {
exclude module: "protobuf-lite"
}
androidTestImplementation 'androidx.test.ext:junit:1.1.3'
androidTestImplementation 'androidx.test:rules:1.4.0'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.ichi2.anki.tests.InstrumentedTest;
import com.ichi2.libanki.DB;

import net.ankiweb.rsdroid.database.AnkiSupportSQLiteDatabase;

import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -34,7 +36,9 @@
import java.io.FileOutputStream;
import java.util.Random;

import androidx.annotation.NonNull;
import androidx.sqlite.db.SupportSQLiteDatabase;
import androidx.sqlite.db.SupportSQLiteOpenHelper;
import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.rule.GrantPermissionRule;

Expand All @@ -55,8 +59,10 @@ public void testDBCorruption() throws Exception {
SQLiteDatabase.deleteDatabase(illFatedDBFile);
Assert.assertFalse("database exists already", illFatedDBFile.exists());

TestDB illFatedDB = new TestDB(illFatedDBFile.getCanonicalPath());
Assert.assertFalse("database should not be corrupt yet", illFatedDB.mDatabaseIsCorrupt);
TestCallback callback = new TestCallback(1);
DB illFatedDB = new DB(AnkiSupportSQLiteDatabase.withFramework(getTestContext(), illFatedDBFile.getCanonicalPath(), callback));

Assert.assertFalse("database should not be corrupt yet", callback.mDatabaseIsCorrupt);

// Scribble in it
byte[] b = new byte[1024];
Expand All @@ -75,7 +81,7 @@ public void testDBCorruption() throws Exception {
// do nothing, it is expected
}

Assert.assertTrue("database corruption not detected", illFatedDB.mDatabaseIsCorrupt);
Assert.assertTrue("database corruption not detected", callback.mDatabaseIsCorrupt);

// our handler avoids deleting databases, in contrast with default handler
Assert.assertTrue("database incorrectly deleted on corruption", illFatedDBFile.exists());
Expand All @@ -87,29 +93,17 @@ public void testDBCorruption() throws Exception {


// Test fixture that lets us inspect corruption handler status
public static class TestDB extends DB {

public class TestCallback extends AnkiSupportSQLiteDatabase.DefaultDbCallback {
private boolean mDatabaseIsCorrupt = false;

private TestDB(String ankiFilename) {
super(ankiFilename);
public TestCallback(int version) {
super(version);
}

@Override
protected SupportSQLiteOpenHelperCallback getDBCallback() {
return new TestSupportSQLiteOpenHelperCallback(1);
}

public class TestSupportSQLiteOpenHelperCallback extends SupportSQLiteOpenHelperCallback {
private TestSupportSQLiteOpenHelperCallback(int version) {
super(version);
}

@Override
public void onCorruption(SupportSQLiteDatabase db) {
mDatabaseIsCorrupt = true;
super.onCorruption(db);
}
public void onCorruption(SupportSQLiteDatabase db) {
mDatabaseIsCorrupt = true;
super.onCorruption(db);
}
}
}
12 changes: 1 addition & 11 deletions AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import android.app.Application;
import android.content.Context;
import android.content.Intent;
import android.content.IntentFilter;
import android.content.SharedPreferences;
import android.content.res.Configuration;
import android.content.res.Resources;
Expand All @@ -45,6 +44,7 @@
import com.ichi2.anki.services.NotificationService;
import com.ichi2.compat.CompatHelper;
import com.ichi2.themes.Themes;
import com.ichi2.libanki.Consts;
import com.ichi2.utils.AdaptionUtil;
import com.ichi2.utils.ExceptionUtil;
import com.ichi2.utils.LanguageUtil;
Expand Down Expand Up @@ -80,16 +80,6 @@ public class AnkiDroidApp extends Application {
*/
public static boolean TESTING_SCOPED_STORAGE = false;

/**
* Toggles opening the collection using schema 16 via the Rust backend
* and using the V16 versions of the major 'col' classes: models, decks, dconf, conf, tags
*
* UNSTABLE: DO NOT USE THIS ON A COLLECTION YOU CARE ABOUT.
*
* Set this and {@link com.ichi2.libanki.Consts#SCHEMA_VERSION} to 16.
*/
public static boolean TESTING_USE_V16_BACKEND = false;

public static final String XML_CUSTOM_NAMESPACE = "http://arbitrary.app.namespace/com.ichi2.anki";

// Tag for logging messages.
Expand Down
18 changes: 0 additions & 18 deletions AnkiDroid/src/main/java/com/ichi2/anki/BackupManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package com.ichi2.anki

import android.content.SharedPreferences
import androidx.annotation.VisibleForTesting
import com.ichi2.anki.exception.OutOfSpaceException
import com.ichi2.compat.CompatHelper
import com.ichi2.libanki.Collection
import com.ichi2.libanki.Utils
Expand All @@ -38,23 +37,6 @@ import java.util.zip.ZipEntry
import java.util.zip.ZipOutputStream

open class BackupManager {
@Throws(OutOfSpaceException::class)
fun performDowngradeBackupInForeground(path: String): Boolean {
val colFile = File(path)
if (!hasFreeDiscSpace(colFile)) {
Timber.w("Could not backup: no free disc space")
throw OutOfSpaceException()
}
val backupFile = getBackupFile(colFile, "ankiDroidv16.colpkg")
return try {
performBackup(colFile, backupFile)
} catch (e: Exception) {
Timber.w(e)
CrashReportService.sendExceptionReport(e, "performBackupInForeground")
false
}
}

/**
* Attempts to create a backup in a background thread. Returns `true` if the process is started.
*
Expand Down
71 changes: 42 additions & 29 deletions AnkiDroid/src/main/java/com/ichi2/anki/CollectionHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.ichi2.preferences.PreferenceExtensions;
import com.ichi2.utils.FileUtil;

import net.ankiweb.rsdroid.Backend;
import net.ankiweb.rsdroid.BackendException;

import java.io.File;
Expand All @@ -43,9 +44,6 @@
import androidx.annotation.VisibleForTesting;
import timber.log.Timber;

import static com.ichi2.libanki.Consts.SCHEMA_VERSION;
import static com.ichi2.libanki.Consts.SCHEMA_DOWNGRADE_SUPPORTED_VERSION;

/**
* Singleton which opens, stores, and closes the reference to the Collection.
*/
Expand Down Expand Up @@ -76,6 +74,14 @@ public class CollectionHelper {
*/
private boolean mCollectionLocked;

/**
* If the last call to getColSafe() failed, this stores the error type. This only exists
* to enable better error reporting during startup; in the future it would be better if
* callers check the exception themselves via a helper routine, instead of relying on a null
* return.
*/
private static @Nullable CollectionOpenFailure mLastOpenFailure;

@Nullable
public static Long getCollectionSize(Context context) {
try {
Expand All @@ -100,6 +106,14 @@ public synchronized boolean isCollectionLocked() {
}


/**
* If the last call to getColSafe() failed, this contains the error type.
*/
@Nullable
public static CollectionOpenFailure getLastOpenFailure() {
return mLastOpenFailure;
}

/**
* Lazy initialization holder class idiom. High performance and thread safe way to create singleton.
*/
Expand All @@ -123,11 +137,19 @@ public static CollectionHelper getInstance() {
*/
private Collection openCollection(Context context, String path) {
Timber.i("Begin openCollection: %s", path);
Collection collection = Storage.collection(context, path, false, true);
Collection collection = Storage.collection(context, path, false, true, currentBackend());
Timber.i("End openCollection: %s", path);
return collection;
}

private @Nullable Backend currentBackend() {
if (mCollection != null) {
return mCollection.getBackend();
} else {
return null;
}
}

/**
* 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
Expand Down Expand Up @@ -193,12 +215,19 @@ public synchronized Time getTimeSafe(Context context) {
* @return
*/
public synchronized Collection getColSafe(Context context) {
mLastOpenFailure = null;
try {
return getCol(context);
} catch (BackendException.BackendDbException.BackendDbLockedException e) {
mLastOpenFailure = CollectionOpenFailure.LOCKED;
Timber.w(e);
return null;
} catch (BackendException.BackendDbException.BackendDbFileTooNewException e) {
mLastOpenFailure = CollectionOpenFailure.FILE_TOO_NEW;
Timber.w(e);
return null;
} catch (Exception e) {
mLastOpenFailure = CollectionOpenFailure.CORRUPT;
Timber.w(e);
CrashReportService.sendExceptionReport(e, "CollectionHelper.getColSafe");
return null;
Expand All @@ -220,8 +249,7 @@ public synchronized void closeCollection(boolean save, String reason) {
* @return Whether or not {@link Collection} and its child database are open.
*/
public boolean colIsOpen() {
return mCollection != null && !mCollection.isDbClosed() &&
mCollection.getDb().getDatabase() != null && mCollection.getDb().getDatabase().isOpen();
return mCollection != null && !mCollection.isDbClosed();
}

/**
Expand Down Expand Up @@ -405,7 +433,6 @@ public static String getAppSpecificInternalAnkiDroidDirectory(@NonNull Context c
return new File(getCurrentAnkiDroidDirectory(context), COLLECTION_FILENAME).getAbsolutePath();
}


/**
* @return the absolute path to the AnkiDroid directory.
*/
Expand Down Expand Up @@ -542,37 +569,23 @@ public static void loadCollectionComplete(Collection col) {
col.getModels();
}

public static DatabaseVersion isFutureAnkiDroidVersion(Context context) throws UnknownDatabaseVersionException {
int databaseVersion = getDatabaseVersion(context);

if (databaseVersion > SCHEMA_VERSION && databaseVersion != SCHEMA_DOWNGRADE_SUPPORTED_VERSION) {
return DatabaseVersion.FUTURE_NOT_DOWNGRADABLE;
} else if (databaseVersion == SCHEMA_DOWNGRADE_SUPPORTED_VERSION) {
return DatabaseVersion.FUTURE_DOWNGRADABLE;
} else {
return DatabaseVersion.USABLE;
}
}


public static int getDatabaseVersion(Context context) throws UnknownDatabaseVersionException {
try {
Collection col = getInstance().mCollection;
return col.queryVer();
} catch (Exception e) {
Timber.w(e, "Failed to query version");
// fallback to a pure DB implementation
return Storage.getDatabaseVersion(getCollectionPath(context));
}
// backend can't open a schema version outside range, so fall back to a pure DB implementation
return Storage.getDatabaseVersion(context, getCollectionPath(context));
}

public enum DatabaseVersion {
USABLE,
FUTURE_DOWNGRADABLE,
FUTURE_NOT_DOWNGRADABLE,
UNKNOWN
}

public enum CollectionOpenFailure {
FILE_TOO_NEW,
CORRUPT,
LOCKED
}

@VisibleForTesting(otherwise = VisibleForTesting.NONE)
public void setColForTests(Collection col) {
this.mCollection = col;
Expand Down
7 changes: 0 additions & 7 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ import androidx.core.content.ContextCompat
import androidx.core.content.pm.ShortcutInfoCompat
import androidx.core.content.pm.ShortcutManagerCompat
import androidx.core.graphics.drawable.IconCompat
import androidx.lifecycle.lifecycleScope
import androidx.recyclerview.widget.DividerItemDecoration
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
Expand Down Expand Up @@ -107,7 +106,6 @@ import com.ichi2.ui.BadgeDrawableBuilder
import com.ichi2.utils.*
import com.ichi2.utils.Permissions.hasStorageAccessPermission
import com.ichi2.widget.WidgetStatus
import kotlinx.coroutines.launch
import timber.log.Timber
import java.io.File
import kotlin.math.abs
Expand Down Expand Up @@ -474,11 +472,6 @@ open class DeckPicker : NavigationDrawerActivity(), StudyOptionsListener, SyncEr
Timber.i("Displaying database locked error")
showDatabaseErrorDialog(DatabaseErrorDialog.DIALOG_DB_LOCKED)
}
DATABASE_DOWNGRADE_REQUIRED -> { // This has a callback to continue with handleStartup
lifecycleScope.launch {
InitialActivity.downgradeBackend(this@DeckPicker)
}
}
WEBVIEW_FAILED -> MaterialDialog.Builder(this)
.title(R.string.ankidroid_init_failed_webview_title)
.content(getString(R.string.ankidroid_init_failed_webview, AnkiDroidApp.getWebViewErrorMessage()))
Expand Down
Loading

0 comments on commit 464e484

Please sign in to comment.