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

Use backend to calculate legacy timezone #11881

Closed
wants to merge 22 commits into from
Closed

Conversation

dae
Copy link
Contributor

@dae dae commented Jul 18, 2022

I expect this fixes #9368

Based on the v3 PR, so only the last commit is relevant. Will rebase once that's merged in.

In terms of merge order, I recommend merging #11740 and #11808 before pointing AD to a new backend version, since they don't rely on any API changes. This commit depends on the updated signature of a backend method, so to avoid any commits that don't compile, this commit should be squashed with a commit that updates AD to the latest backend version.

dae added 15 commits July 18, 2022 13:43
scm/ls are fetched on demand now, since backend methods may mutate
them.
Follow-up of ankidroid@b02515b

+ Convert Optional references to native Kotlin nullable types
+ Remove some methods that have been replaced with a new API
Follow-up of ankidroid@b02515b

+ Convert Optional references to native Kotlin nullable types
- Progress is now shown in the GUI
- Cancellation of tasks is supported
- Removes some boilerplate and improves ergonomics -
runInBackgroundWithProgress is now scoped to AnkiActivity.
- When errors encountered, they are shown until user clicks OK.
- Hooks up new backend to import->colpkg option

Cancellation will not work correctly until the backend is updated,
as the cancellation routines need to be excluded from the backend
mutex - until updating, attempting to cancel will block the UI until
the operation completes.
Can be tested by importing an apkg from the deck list, then choosing
the undo option.

- If a backend undo entry is present, it takes priority over the old
undo system to ensure data integrity. AD's reviewer saves config vars and
decks as part of the review process, and it was triggering undo entries
for those changes, which prevent the v2 scheduler review from being undone.
The calls have been updated to clear the backend undo, so that the review
takes precedence. The desktop code has the same problem, and it will be
avoidable once updating to the v3 scheduler.
- Introduce a helper to await a job in unit tests, to avoid race
conditions
+ Delete the cards inside the default deck if user tries to delete it

Closes ankidroid#11703
CardTemplateEditorTest is no longer failing with the latest code 🎉
mCol was null.

Technically libanki code should not be making reference to GUI
code, but this code should be retired once AD has migrated to the
new background syncing code anyway.
This worked in my initial testing, but with a smaller collection, the
UI is now racing with the download on my machine:

- getSyncStatus() was assuming col.db would still be open by the time
it gets around to calling methods on it
- onCreateOptionsMenu() likewise assumes that if colIsOpen() returns true,
it is free to call methods on it

A better way way to fix this would be to wrap all collection access in a mutex,
so that the collection is never touched without the lock being acquired.
Code holding the mutex could then be guaranteed that the collection is open
and valid for the duration of its run. Changing all that legacy code is
a big task however.

I've added a lock to the upload case as well, as it's possible it's likewise
buggy and just not triggering on my machine.
@mikehardy
Copy link
Member

This has conflicts now but they will probably rebase out
New backend version is in process but I haven't merged the first two commits you mention yet, new backend version will be ankidroid/Anki-Android-Backend@7405e45

0.1.15-anki2.1.54

dae added 7 commits July 20, 2022 11:44
Studying with the V1 scheduler enabled is no longer possible on recent
Anki, AnkiMobile or AnkiWeb versions, so AnkiDroid is the last one
still supporting it. Pushing users to upgrade will save them from some
of the footguns V1 had, and will allow AnkiDroid to cut out some code
and tests. Many users have likely already upgraded due to the use of
the other clients.

This commit adds support for the backend upgrade code, so that learning
cards will not be reset on upgrade. To make use of this, users will be
automatically updated to the latest schema version, the scheduler
upgrade will be performed, and then they're moved back to the legacy
schema.

I've also added a helper to more ergonomically deal with schema changes.
Initial prep work for adding the V3 scheduler. Removed some methods that
are only used internally, and defined some overloads concretely.

eta() implementation in v1 appeared to be functionally identical to v2,
despite the comment.
It was introduced due to ankidroid#5666, but AnkiWeb has ignored the due counts
in the sanity check for about 18 months, so this is no longer necessary.

Also remove _updateCutoff() and _checkDay() from the public API.
- Mirrors the code layout of the desktop version, where routines like
buryCard(), that are not specific to V2 or V3, are stored in a separate
file. This leaves AbstractSched containing only the methods that will
need to be implemented separately for V3.
- Moves some methods from SchedV2 into BaseSched, as V3 will want to
use them as well for now.
- Some of the routines in BaseSched will only work with a schema upgrade.
For now, SchedV2 overrides these so they continue to work with the
legacy schema.
Like the desktop, this works by using a single set of tests that
alter some of the checks depending on the active scheduler version,
so that code duplication is avoided.
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.
@dae
Copy link
Contributor Author

dae commented Jul 20, 2022

I've updated the commit to point to that new version; tests will need to be restarted once the library is available.

@dae dae marked this pull request as ready for review July 20, 2022 02:09
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 22, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
@dae dae marked this pull request as draft July 22, 2022 04:39
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 22, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 22, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 23, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 24, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
dae added a commit to ankitects/Anki-Android that referenced this pull request Jul 24, 2022
I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881
@mikehardy mikehardy closed this in 712ec6b Jul 27, 2022
bennettj12 added a commit to bennettj12/Anki-Android that referenced this pull request Jul 30, 2022
kotlincleanup: additional uses of .apply

Reduce the API surface of AbstractSched

Initial prep work for adding the V3 scheduler. Removed some methods that
are only used internally, and defined some overloads concretely.

eta() implementation in v1 appeared to be functionally identical to v2,
despite the comment.

Remove setReportLimit()

It was introduced due to ankidroid#5666, but AnkiWeb has ignored the due counts
in the sanity check for about 18 months, so this is no longer necessary.

Also remove _updateCutoff() and _checkDay() from the public API.

Split majority of AbstractSched into BaseSched

- Mirrors the code layout of the desktop version, where routines like
buryCard(), that are not specific to V2 or V3, are stored in a separate
file. This leaves AbstractSched containing only the methods that will
need to be implemented separately for V3.
- Moves some methods from SchedV2 into BaseSched, as V3 will want to
use them as well for now.
- Some of the routines in BaseSched will only work with a schema upgrade.
For now, SchedV2 overrides these so they continue to work with the
legacy schema.

Add support for the V3 scheduler

Add V3 scheduler tests

Like the desktop, this works by using a single set of tests that
alter some of the checks depending on the active scheduler version,
so that code duplication is avoided.

Use backend to calculate legacy timezone

I expect this fixes ankidroid#9368

Bumps the backend version, since the API changed.

Closes ankidroid#11881

Backend language tweaks

- Move logic from backend into frontend, so a new backend release is
not required
- Fix mapping of Chinese languages and Hindi; check others
- Don't require a restart after changing language in prefs

Fix LeakCanary warning when changing language/v16 schema

After discarding the backend, Preferences was calling getCol and passing
itself in as the context. This caused the preferences screen to be retained
as the backend lives on. By switching to AnkiDroidApp, a leak can be
avoided.

Fix some connected tests failing when legacy_schema=false

Some were irrelevant; the ones in ContentProviderTest need investigating.

Updated strings from Crowdin

Bumped version to 2.16alpha76

more uses of .apply

reintroduced null checks and removed specified nullable type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] card due count doesn't match with Anki Desktop after sync
2 participants