Drop legacy Java backend implementation #11581
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Unsure if the Windows failure is a flaky test or a regression. |
|
This appears to have broken the emulator; investigating. |
As AnkiDroid begins to adopt more of the backend code, it will not be practical to maintain a fallback for devices that are unable to load the backend library. Assuming there are an acceptably few number of DroidBackendFactory::getInstance crash reports, cutting the Java fallback is one step towards a single concrete backend interface. Also introduce AnkidroidApp.currentBackendFactory() to get the currently active backend version, and switch a couple of calls that were using BackendFactory.createInstance() (which hard-codes V11).
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 11, 2022
This was referenced Jun 11, 2022
|
I still have a pending task of bringing up new crash reporting database after the security issue a few weeks back Once I do that (hopefully this weekend - it's nearly setup), I'd like to let this sit for a week to allow for an answer to the question "how many DroidBackendFactory::getInstance crashes do we have?". I think the answer is actually zero, it was before the security incident resulted in us wiping the crash reporting instance |
mikehardy
approved these changes
Jun 11, 2022
mikehardy
added
Needs Author Reply
|
Re the latter point, my thinking here was that: a) TESTING_USE_V16_BACKEND is defined inside AnkiDroid, and would need to be moved into AnkiDroidBackend and a new release made |
|
Preliminary information:
So I'm going to say that we don't have a rust lib problem. One user with a native lib error is a "that user had an install problem" in my opinion Will re-scan in another few days but I think going all-in on Rust is really the only way forward and the data seems to support that a stance like that is not user-hostile or aggressive in any way |
There was a problem hiding this comment.
Architecturally, I don't mind as long as there's a cleanup attribute.
When this causes a crash, the user won't have migrated to the new storage mechanism, and their collection will be safe, allowing them to downgrade.
But, we shouldn't show the regular 'collection open failed' message here, as that provides incorrect solutions to this issue.
I'd accept either:
- throw an
ErrorifNativeMethods.ensureSetup()fails (to hard crash the app) - Expose the exception from
NativeMethodsand check inDatabaseErrorDialog
I'd prefer 1 for now, with a cleanup attribute to move to 2, to retain velocity
|
Ok, good to know it can be cut. I'll circle back to this later; currently experimenting with updating the backend code to the latest desktop code. |
|
Mildly related - to your last comment - have not had time to investigate enabling Kotlin in anki-android-code yet, but hopefully soon... |
I wouldn't be opposed to doing a migration in a single commit for the backend. |
|
I managed to figure it out - had applied the plugin to the wrong module, and needed some extra steps to get generated Kotlin to work. |
agreed on that. If you're in to it, just do it, boom done. |
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 16, 2022
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
|
Closing in favor of #11644 |
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 21, 2022
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
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 25, 2022
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
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 25, 2022
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
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
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
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
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
dae
added a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 28, 2022
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
mikehardy
pushed a commit
that referenced
this pull request
Jun 29, 2022
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 #11579 and closes #11581
mikehardy
pushed a commit
that referenced
this pull request
Jun 29, 2022
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 #11579 and closes #11581 refactor: Rename Storage.java to .kt com.ichi2.libanki.Storage refactor: Convert Storage to Kotlin com.ichi2.libanki.Storage 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)
mikehardy
pushed a commit
that referenced
this pull request
Jun 29, 2022
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 #11579 and closes #11581
mikehardy
pushed a commit
that referenced
this pull request
Jun 29, 2022
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 #11579 and closes #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)
mikehardy
pushed a commit
to ankitects/Anki-Android
that referenced
this pull request
Jun 29, 2022
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)
mikehardy
pushed a commit
that referenced
this pull request
Jun 29, 2022
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 #11579 and closes #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)
mikehardy
removed
Needs Author Reply
dorrin-sot
pushed a commit
to dorrin-sot/Anki-Android
that referenced
this pull request
Jul 14, 2022
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)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As AnkiDroid begins to adopt more of the backend code, it will not be
practical to maintain a fallback for devices that are unable to load
the backend library. Assuming there are an acceptably few number of
DroidBackendFactory::getInstance crash reports, cutting the Java
fallback is one step towards a single concrete backend interface.
Also introduce AnkidroidApp.currentBackendFactory() to get the currently
active backend version, and switch a couple of calls that were using
BackendFactory.createInstance() (which hard-codes V11).