Drop legacy schema, and update to 2.1.66 backend #14171
Conversation
dae
changed the title
dae
force-pushed
the
drop-legacy-schema
branch
2 times, most recently
from
af0fce0
to
3f558b5
Compare
|
Ok, I think this is in decent shape now. Especially happy about having gotten rid of AsyncTask. The 2.1.66 release is likely a number of weeks off still, so one option would be to merge that backend PR without waiting for an official release. That would allow the CI tests to run, and unblock the merging of this when you're ready for the 2.17 split. This PR touches quite a few files, so hopefully it doesn't develop too many conflicts before it can be merged in. |
BrayanDSO
added
the
Blocked by dependency
|
I’m going to wait for the dependency to be solved before reviewing. But, as a note: you only need to remove resources from the |
|
Ah, I just used AndroidStudio's tool for removing unused resources for that. Can clean that up before merge, and it may be that we want to exclude some of those strings from garbage collection (eg, the "try anyway?" string was unused by the new sync path and thus got removed, but if the plan is to add that offline check back in, the string could be preserved for now. My vote on that particular case would be not to add it back; online checks tend to be unreliable, and sync has a short timeout+is cancellable. But there may be other strings in a similar situation. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Thanks Brayan, I'll look into these issues. I rolled back the resource removal commit for now; we can return to that when this PR is closer to merge. For reference, it can be found here: 3f558b5 I've cherry-picked your changes; not sure why you couldn't push into the branch directly. The removal of advanced statistics took a conflict due to the resource changes; hopefully the tweak I made is correct. I didn't deliberately remove you as the author of that commit, sorry - I must have messed up something with git. |
|
Web pages issue should be fixed after updating to the latest backend commit and rebuilding it. Re: importing, are you sure the file to import is valid? Would you be able to share it? |
Yeah, I can restore legacy backups in Here's a file. I've renamed the extension from .colpkg to .zip to allow uploading it in GitHub collection-2023-07-25-15-39.zip re: now the web pages work, thanks |
|
Thanks for that. Looks like AnkiDroid was creating backups that the desktop couldn't import, since they were missing an expected file. I've pushed a fix, and it should work after you update+rebuild the backend. |
|
Added some more changes into my branch . I still can't push them here ( BrayanDSO@ebaf34a isn't essential to this PR, but I pushed it here so it doesn't break any of the previous commits if it got merged first OutdatedA problem that I had is that test_field_named_frontSide test_field_named_frontSide Inside the app, if I add
|
|
Those tests should be fixed after you update+rebuild the backend. |
|
Worked. Thanks |
|
Sorry, missed your comment about the extra commits - I've merged them in, skipping the StdModels move, as it breaks the build (testing with tools/test-commits.sh HEAD). I think the reason you're unable to push is because of https://github.com/orgs/community/discussions/5634. I'll grant you access to the repo, so you should be able to push future changes into it. |
|
I'm laptopless, but I believe that these need to be fixed in v3 sched: |
|
Huge! |
* Update target API to 23 * Update to 2.1.66, and switch to new schema by default + Temporarily disable unused resource warnings * Drop legacy syncing code * Drop legacy import/export code * Drop legacy tag handling * Drop legacy config handling * Drop legacy database check * Stub out unused v1 scheduler code Can't be fully removed, as we still need to be able to open a v1 collection so the user can upgrade. moveVersions test has been removed, as backend code is used for moving outside of unit tests. * Drop legacy deck handling * Drop legacy search code * Drop legacy notetype code This breaks the card template editor, so I've temporarily disabled it in the GUI. Getting it working again will require switching to the new template rendering code in the new backend. Also breaks an "empty cards" action in CardContentProvider, as I was not sure what it was trying to accomplish. * Drop legacy media code This removes oakkitten's custom media checking code, as keeping it would require keeping a bunch of the duplicated Kotlin logic. His comment about cancellation is incorrect: the call can be cancelled with backend. setWantsAbort() on a background thread. And since the app enforces filenames are normalized when they're added anyway, I don't think avoiding automatic normalization is worth the duplicated code. * Drop legacy Collection/DB code * Drop most references to defaultLegacySchema * Remove legacy schema tests from CI CI won't work yet, because backend version needs updating * Fix import CSV call * Remove some unused symbols from anki module * Drop majority of old stats code * Remove unused symbols from libanki * Remove some broken Android tests, and fix one checkIfStudyOptionsIsDisplayedOnTablet() is also consistently failing, but was doing so before I started on these changes. * Move config methods into col.config * Simplify config API - Use kotlinx.serialization so that arbitrary typed objects can be (de)serialized - There is a single generic get(), that always returns an optional value, which will be null if the key is missing, is null, or is not the correct type. There will always be collections that have invalid data, so the calling code always needs to be prepared to substitute a reasonable default in such cases. - Expose the typed getBool() method from the backend. * Remove unused legacy importer * Drop jackson * Models -> Notetypes * Remove some more unused symbols * Remove generic type aliases * Rename Model -> Notetype I had to leave ModelTest.kt's filename alone, as changing it to Notetype.kt reorders the unit tests, and causes about 4 tests to start flaking. * Get card template editor preview working again * Enable undo on edits; fix test hang * Use backend for bulk suspend/undo By default, undoableOp() notifies all screens listening for opExecuted, so the refreshing happens automatically, and the manual refresh code at the end of the old routine is no longer required. It is possible to bypass this when you want to manually update the UI, but this is probably not worth attempting until the card browser is either switched over to a recycler view, or reimplemented as a web component. The LongArrays have been switched to simple lists, as Google's protobuf code expects Iterable<Long> * Use backend for marking/undo, and change deck/undo * Migrate reviewer actions/undo to backend * Migrate remainder of undo code to backend, apart from v2 undo Couple of notes: - The legacy v2 undo no longer returns the undone card, and instead rebuilds the queue. In some circumstances (eg a learning card has become due), this can result in a different card being shown after undoing. This is not ideal, but v2 does not have long to live at this point. - The reset/reposition tests were reusing the old card cache values, which have to be reloaded after an operation. - A test in AbstractSched had to be moved to ReviewerTest so it can have access to an activity. - UndoTest has been removed, as it mainly focused on removed code. * GetCard() -> getNextCardAndRedraw() * render_output -> renderOutput() * Drop AsyncTask Probably the hardest part of AnkiDroid's code base to follow; very glad to see the end of it. Closes ankidroid#7108 * Make note addition undoable * Make bulk tag update undoable * Remove remaining explicit transaction handling The backend automatically wraps backend actions in a transaction, and removing the explicit calls will allow us to drop the redundant mutex that rsdroid acquires. * Remove remaining unused classes/methods/properties * Remove FunctionalInterfaces * Switch to backend answer comparison * Remove unused locking code + legacy storage code * Switch ContentProviderTest to new schema, and fix bug * Remove legacy Deck Options settings * fix: remove minSdkVersion < 23 code * Use short time in snackbars People complained about snackbars interrupting their reviews before * fix: don't duplicate undo label It were showing a message like `Undo Undo Add Note` * Remove "Advanced statistics" * Remove legacy_schema local property * Leave zip validation up to the backend The backend takes care of validating the zip file and ensuring files aren't written outside of the media folder. I've expanded the mime match to include the zip mime, on the assumption that's what the mime type is being changed to on some devices. If that proves to be insufficient, a much simpler approach would be to look for 50 4B 03 04 at the start of the file to determine if it's a zip file or not. This leaves only Mani's add-on code using commons-compression, so you could potentially switch to a simpler tgz-specific library in the future if that is easier. * Disable two flaky tests They started to flaky reliably after the legacy deck options test was removed. * Improve logging of HTTP requests * Remove ModelBrowser Replaced by ManageNotetypes * Remove legacy CardInfo replaced by com.ichi2.anki.pages.CardInfo * Change comments at AdvancedSettingsFragment * test: check if prefs analytics list don't have extra elements * ContextCompat.getColor -> getColor that compat method is for implementations in API < 23 * Remove more unused files * refactor: move SECONDS_PER_DAY to a separate util file in order to be able to remove Stats.kt later * refactor: remove Stats and OverviewStatsBuilder * Access sched.card directly in tests The helper was adding a 500ms sleep on every fetch, which slowed the scheduler tests down considerably. * Make v3 scheduler the default; drop support for v2 The v3 scheduler was originally released in 2021, and we've been waiting for a stable AnkiDroid release to support it before we could switch users over to it. Now that 2.16 is out, we can finally push v3 out across the ecosystem. While we could theoretically make v3 the default without removing v2, here are the reasons why I think we're better off switching to v3 only: - AnkiWeb's review interface will likely switch over to v3-only in the coming weeks, and AnkiMobile will likely drop v2 around the same time 2.17 comes out. - v2 and v3 differ in a few ways that makes maintaining the two separate paths more complicated: things like the different undo paths, counts not including the current card, and the way the v3 scheduler supports custom scheduler js. 2.17 is a chance to clean up a lot of old cruft in the code base, and the old scheduling code is part of that. - v2 and v3 are compatible with each other, and don't require a full sync to change, so it doesn't break syncing with old clients (though depending on settings, due counts may differ, which needs explanation) I had to disable a couple of tests for this that we'll probably want to restore in some form: - corruptVersion16CollectionShowsDatabaseError() needs an update, and it may be time to rip out the old CollectionHelper colIsOpen(), getColSafe() and so on, migrating code that uses it over to withCol() instead. The 'collection inaccessible' dialog also needs a rethink - perhaps the various exceptions could be handled in launchCatchingTask instead. - testUndoResetsCardCountsToCorrectValue() is failing because initLayout() creates a gesture listener, which fails with an error about the looper not being initialized. I am not sure what's going on there - one option would be to move the test to androidTest. * Drop v2 sched file; rename files * Drop AbstractSched and legacy undo code/queue code * Migrate BaseSched into Scheduler * More unused symbol removal * Remove DeckTreeNode * Remove unused processChildren() * Remove shouldDisplayCounts(), as counts always available * Remove manual hashCode()/compareTo() implementations * Drop AbstractDeckTreeNode * Turn DeckNode into a wrapper for DeckTreeNode * Drop TreeNode and simplify filtering * Remove `New card position` global preference Overridden by per-deck configuration Closes ankidroid#12319 * fix: remove chess.css from card_template_html * Store current queue state in reviewer Prerequisite for solving ankidroid#12620 Also dropped answerButtons(), as it's always 4 * Use local HTTP server for serving flashcard content Prerequisite for solving ankidroid#12620 * Implement support for custom JS scheduling Enables FSRS and closes ankidroid#12620 * Remove more unused code * Remove separate Deck(Config)V16 objects and more unused code * Move some NotetypeJson methods into its file * Finish tidying Decks.kt * Drop redundant col property on Collection * Remove some usages of CollectionGetter * Rename col->getColUnsafe to encourage migration * Remove Reviewer.sched * Handle deck tree not initialized in empty collection * refactor: move syncStatus() to Collection to avoid direct calls to the backend * fix: do not show error when updating the menu if there is no internet Reproduction steps: 1. have the collection synced 2. disable the device internet connection 3. Restart the app * Remove unused resources and re-enable lint * Restore sync cancel strings * Defer loading webpage until server has initialized * Ignore translations without a base entry ankidroid#14171 (comment) * Remove the SortOrder deprecation * Remove the apparently-unused Backup.kt * Remove last remaining use of Collection's context field * Stop passing Context to backend The library loading is now AnkiDroid's responsibility, so the backend does not require access to an Android context. Also remove the unused context field from Collection. * Use backend for rendering next time labels * Use backend for rendering finished message The custom study & unbury descriptions are slightly different, but should suffice until AnkiDroid can start using the Svelte finished screen. * Run SchedulerTest without Robolectric Halves the run time. To do this, I implemented a new JvmTest class and copied some of the helpers in RobolectricTest into it. * Remove unused Kotlin implementation of template parsing * Move Util code dealing with resources out of libanki * Migrate remaining libanki tests to raw JVM + Remove some commented-out tests Total savings are a drop from about 3m40s to 2m40s, with the scheduler test class dropping its runtime by about half. * Lazy-initialize col in tests * Restore 'experimental' string * Add comments to update-localizations.py * Restore AcraAnalyticsInteraction.kt --------- Co-authored-by: Brayan Oliveira <[email protected]>
|
Hi there @dae! This is the OpenCollective Notice for PRs merged from 2023-08-01 through 2023-08-31 If you are interested in compensation for this work, the process with details is here: We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month Please note that GSoC contributions are okay for this process. Our philosophy is that our users have donated to AnkiDroid for all contributions. The only PRs that will not go through the OpenCollective process are ones directly related to am accepted GSoC project from a selected participant, since those receive a stipend from GSoC itself. Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding. Thanks! |
Felt like a change of pace today, so got started on this.
Blocked until ankidroid/Anki-Android-Backend#294 is merged.