[GSOC] Implemented Service to store Deck Meta Data

[prateek-singh-3212]

Pull Request template

Purpose / Description

I am working on decoupling the notification ( currently It causes random notification triggring in our app). This PR also aims to solve the problem of tight coupling between notification and small widget.

Fixes

Fixes https://github.com/ankidroid/Anki-Android/issues/8114 Fixes https://github.com/ankidroid/Anki-Android/issues/6476

Approach

What is the issue? The complete issue is documented in https://github.com/ankidroid/Anki-Android/issues/6476. The main issue is that Current notification system is coupled with widget and notification is only triggered when user use widget. Secondly, Notification is triggered when user is using the app. How? Whenever deck review completes the deck the widget data is updated Hence Notification is shown (Because it is tightly coupled).

Architectural: one "unit of work" per time, rather than per deck.

ALGO FOR DECK NOTIFICATION Input: TreeMap<time, List<Long>>

We store the the time and list of decks whose notification needs to be send on that time. The map is TreeMap which stores the data in sorted key form.

Working of algo:

1. Takes the input time (EPOCH Long)

            if (currentTimeMS < scheduleTimeMS) {
                    // Scheduled time is gone for today.
                    // Only add time in time deck map. No need to reschedule work manager.
                    // Update schedule time for next day `scheduleTimeMS += 3600000`
                    // Update data in map
            } else if (scheduleTimeMS < (first key of map {Map is sorted so it will contain the most upcomming deck reminder at **TOP**})) {
                    // Scheduled time will come today only. And it will come before the current schedule deck.
                    // Updating data in map
                    // Replacing old work manager with new one.
            } else {
                    // Scheduled time will come today. And it will come after the current schedule deck.
                    // Updating data in map
            }

ALGO FOR ALL DECK NOTIFICATION All deck notification will work in the syncronization with above algo and addintion to this If Thier is no scheduled notification in next 1 hour then also work manager will run after 1 hour and check the all deck status.

How Has This Been Tested?

Tested on samsung galaxy M51(API 31)

Checklist

Please, go through these checks before submitting the PR.

• [x] You have not changed whitespace unnecessarily (it makes diffs hard to read)
• [x] You have a descriptive commit message with a short title (first line, max 50 chars).
• [x] Your code follows the style of the project (e.g. never omit braces in if statements)
• [x] You have commented your code, particularly in hard-to-understand areas
• [x] You have performed a self-review of your own code
• [ ] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• [ ] UI Changes: You have tested your change using the Google Accessibility Scanner

[prateek-singh-3212]

Please check this video for clarification. (for now I am starting this service when app starts and when onDestroy() of activity is called. But I want to change this in future.)

https://user-images.githubusercontent.com/76490368/171696647-41ba5eab-af7e-4e01-90ba-f52c55906e32.mp4

Snip of deck meta data [Shared Preference]...

creating json with this model class ⬇️ https://github.com/prateek-singh-3212/Anki-Android/blob/257bb949313aaca19c954100c90e185631996fe8/AnkiDroid/src/main/java/com/ichi2/anki/services/DeckMetaDataService.kt#L113

Json body...

{
  "deckName": "dtc",
  "did": 1621523610764,
  "eta": 0,
  "lrn": 0,
  "new": 0,
  "rev": 2
}

PS You can check logs by using tag META

[david-allison]

Tests are falling

[prateek-singh-3212]

@david-allison Fixed the Issue. Kindly check now.

[Akshay0701]

conflict is still there

[prateek-singh-3212]

@david-allison I was unable to found why tests are failing. Could you please help?

[mikehardy]

What the heck? Nothing but this over and over and over

AbstractFlashcardViewerKeyboardInputTest > spaceDoesNotShowAnswerIfTextFieldFocused FAILED
    java.lang.UnsatisfiedLinkError: 'long net.ankiweb.rsdroid.NativeMethods.openBackend(byte[])'
        at net.ankiweb.rsdroid.NativeMethods.openBackend(Native Method)
        at net.ankiweb.rsdroid.BackendV1Impl.ensureBackend(BackendV1Impl.java:91)
        at net.ankiweb.rsdroid.BackendV1Impl.openAnkiDroidCollection(BackendV1Impl.java:128)
        at net.ankiweb.rsdroid.BackendMutex.openAnkiDroidCollection(BackendMutex.java:977)
        at net.ankiweb.rsdroid.BackendUtils.openAnkiDroidCollection(BackendUtils.java:29)
        at net.ankiweb.rsdroid.database.RustV11SupportSQLiteOpenHelper.createRustSupportSQLiteDatabase(RustV11SupportSQLiteOpenHelper.java:45)
        at net.ankiweb.rsdroid.database.RustSupportSQLiteOpenHelper.getWritableDatabase(RustSupportSQLiteOpenHelper.java:69)
        at com.ichi2.libanki.DB.<init>(DB.java:80)
        at com.ichi2.libanki.backend.RustDroidBackend.openCollectionDatabase(RustDroidBackend.kt:42)
        at com.ichi2.libanki.Storage.Collection(Storage.java:97)
        at com.ichi2.anki.CollectionHelper.openCollection(CollectionHelper.java:138)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:157)
        at com.ichi2.anki.CollectionHelper.getCol(CollectionHelper.java:128)
        at com.ichi2.anki.CollectionHelper.getTimeSafe(CollectionHelper.java:190)
        at com.ichi2.anki.worker.DeckMetaDataWorker$Companion.setupNewWorker(DeckMetaDataWorker.kt:[112](https://github.com/ankidroid/Anki-Android/runs/6739991733?check_suite_focus=true#step:8:113))
        at com.ichi2.anki.AnkiDroidApp.setupDeckMetaDataWorker(AnkiDroidApp.java:443)
        at com.ichi2.anki.AnkiDroidApp.onCreate(AnkiDroidApp.java:236)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$installAndCreateApplication$2(AndroidTestEnvironment.java:350)
        at org.robolectric.util.PerfStatsCollector.measure(PerfStatsCollector.java:86)
        at org.robolectric.android.internal.AndroidTestEnvironment.installAndCreateApplication(AndroidTestEnvironment.java:350)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$createApplicationSupplier$0(AndroidTestEnvironment.java:229)
        at org.robolectric.util.PerfStatsCollector.measure(PerfStatsCollector.java:53)
        at org.robolectric.android.internal.AndroidTestEnvironment.lambda$createApplicationSupplier$1(AndroidTestEnvironment.java:226)
        at com.google.common.base.Suppliers$NonSerializableMemoizingSupplier.get(Suppliers.java:167)
        at org.robolectric.RuntimeEnvironment.getApplication(RuntimeEnvironment.java:71)
        at org.robolectric.android.internal.AndroidTestEnvironment.setUpApplicationState(AndroidTestEnvironment.java:194)
        at org.robolectric.RobolectricTestRunner.beforeTest(RobolectricTestRunner.java:325)
        at org.robolectric.internal.SandboxTestRunner$2.lambda$evaluate$0(SandboxTestRunner.java:265)
        at org.robolectric.internal.bytecode.Sandbox.lambda$runOnMainThread$0(Sandbox.java:88)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1[128](https://github.com/ankidroid/Anki-Android/runs/6739991733?check_suite_focus=true#step:8:129))
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
        at java.base/java.lang.Thread.run(Thread.java:829)

Does the ankidroid backend use a provider also, and this is disrupting it?

[david-allison]

Does the ankidroid backend use a provider also, and this is disrupting it?

No

Typical initialization is:

https://github.com/ankidroid/Anki-Android/blob/599c50575148ed8259cf8eda7fd858244f773a62/AnkiDroid/src/main/java/com/ichi2/libanki/backend/DroidBackendFactory.kt#L45-L50

https://github.com/ankidroid/Anki-Android-Backend/blob/0c095aa8eb6f6b72bd88661d44fdf9a630f34f45/rsdroid/src/main/java/net/ankiweb/rsdroid/BackendV11Factory.java#L29-L33

My first place to look would be to see if we're initialising anki-android-backend-testing library correctly:

https://github.com/ankidroid/Anki-Android/blob/8741aae7215366ed95ad466474906492e77e88c6/AnkiDroid/src/test/java/com/ichi2/anki/RobolectricTest.kt#L92-L120

---

I'd also say: we're adding a dependency on a valid collection inside AnkiDroidApp.

We need to handle if a collection is corrupt etc...

[david-allison]

@Arthur-Milchior, this is probably the time to decouple Time from the collection

[Arthur-Milchior]

@Arthur-Milchior, this is probably the time to decouple Time from the collection

Done : #11546

By the way, I trust you it was necessary. I have not looked in details why it was blocking here

[prateek-singh-3212]

@Arthur-Milchior Deleted the unessary commit and Added the documentation. Please check now.

@david-allison still tests are failing. Shoud I use different approch in fetching time?

[prateek-singh-3212]

@Arthur-Milchior @david-allison @Akshay0701 Done the atomic commits and added the documentation. I think now it will be easier for you to review

[mikehardy]

Is there some place that is tracking this discussion on whether a persistence layer is needed or not?

A change this size with questions on design and need needs to have some conclusive discussion on whether we really need it or not. GSoC has deadlines so even though we are not deadline oriented and all of us are having various personal reasons for low availability right now :weary: it seems like some sort of synchronous discussion needs to happen? Maybe on the discord channel?

Or if a solid defense for this has already been made and accepted, it should be documented so that all future readers (like myself) can say "okay, it seems iffy but everyone had the discussion and we did agree it was justified"

[prateek-singh-3212]

@mikehardy I understood your point. This is the drastic change in the AnkiDroid. I designed the complete flow of this mechanism. on the basis of concluding the following disscussion. https://github.com/ankidroid/Anki-Android/issues/6476 (The main issue is documented here) https://github.com/ankidroid/Anki-Android/issues/8114 https://github.com/ankidroid/Anki-Android/issues/4944 (UI is inspired from your disscusion with timrae)

Yes, I too feel that we all needs to be on same page. I would like to propose the of Idea of setting up meet to discuss this issue.

[mikehardy]

for the record I think it may be justified - even though it is a big change, I just want to make sure we all agree so there are no lingering doubts. I don't like lingering doubts - I prefer to be decisive even if we all discover it is a mistake later (which is okay, it happens). I'm sure you saw I mentioned it on discord. Hopefully we can settle it quickly so you are not delayed

[prateek-singh-3212]

@david-allison @Akshay0701 @mikehardy @Arthur-Milchior

What is the issue? The complete issue is documented in #6476. The main issue is that Current notification system is coupled with widget and notification is only triggered when user use widget. Secondly, Notification is triggered when user is using the app. How? Whenever deck review completes the deck the widget data is updated Hence Notification is shown (Because it is tightly coupled).

Solution of this issue

Create a class which is used to store the deck meta data. But does this solves the issue? Yes, because

1. it removes the calls which updates the data on onStop or onPause which in not optimized at all.
2. It removes the call which opens the collection after regular interval of time just to update the card count. for eg. If notification services opens the collection and after 5 min Widget update service opens the collection to get the same total cards count. (Open collection is expensive task). In future if other serivce want to access the data then it can use meta data persistant layer

Why we need to store all the decks detail? Why cant we store the Total card due count? This is because we have to show the individual deck notification to user. In future we will provide the feature to show notification of particular deck at the given time if due card is above (for eg. 10). If we did'nt store individual deck details then we encounter problem metioned in poin 2 above.

Current Solution Explanation This PR uses new librarys:

1. WorkManager: used after concluding (https://github.com/ankidroid/Anki-Android/pull/11487#discussion_r888258029)
2. Datastore Preference: Used because Shared Preference is causing the OOM Exception. See: https://discord.com/channels/368267295601983490/977324896352862258/982979582804246548

Class Structure DeckMetaWorker DeckMetaDatastore

Future Plan I this PR mereges then I will use the work manager chaning to do:

1. Update Widget
2. Send Notification

How??? After 30 min Deck Meta Data Updates ------------> Checks the is there any notification that need to be send?-------------------->Updates the widget.

More on Chaining: https://developer.android.com/topic/libraries/architecture/workmanager/how-to/chain-work

Above Solution Solves the 2 isses which I addressed ☺️

[mikehardy]

Okay, so what I understand is: 1- people want notifications at specific times, which implies waking up and working maybe when the app is not running, and WorkManager is the best choice for that by far, so that justifies the WorkManager stuff 2- people want per-deck notification settings which means we need to store and access an amount of data that is O(n) where n is number of decks and will cause OOMs so we either need to store it in the collection or we need some other place to store it

I understand from 2 above that if we are not storing it in the collection then datastore-preferences appears to be a good choice for this probably small but maybe large set of data that is nevertheless pretty simple (so SharedPreferences won't work but Room is more than we need)

So just one question then: could we / should we store this in the collection? I think it's possible to do so but right now deck conf is read in / written out as one big chunk and I think it would have all the problems SharedPreferences has with OOM risk and performance. In newer Anki rust code I think it's possible to do per-deck metadata but then we'd block a GSoC project on that which is not acceptable.

So if I understand things correctly, I believe the dependencies are justified by the requirements and the specific dependencies are the correct choices. My opinion

[Arthur-Milchior]

In the initial message you wrote:

Solution of this issue image

I think the image is missing.

I would love if for next screenshot you can activate an option to show where you click. I find hard to follow without it. You can do it at least by using accessibility feature to show where you press. I think some app have also it by default

I've a hard time following your explanation. Mostly because I don't see what you call deck "meta data". The closest thing I could imagine would be the deck object as a json, as it's mostly meta data, but it do not seems to make sens here.

I'd love if we could ensure github is self consistent. If you could copy from discord to github the explanation about OOM instead of linking to it. I would like to at some point know I don't have to chase information and can find everything at the same place

[Arthur-Milchior]

Regarding storing data in the collection, as suggested by Mike. Wouldn't there be a problem with the fact that it would require to open the actual collection? Potentially a big file, that must be entirely read to be used by sqlite. Plus it probably would require loading at least a big part of our codebase in memory. If there is a way to ensure that we can avoid actually opening the database and actually opening the CollectionHelper method to open the actual collection, I believe that it's an actual win. Or am I missing something here?

[mikehardy]

open the actual collection? Potentially a big file, that must be entirely read to be used by sqlite

That's not how the database read works with sqlite - you can have a 2GB sqlite db and open it and it doesn't map the whole thing in to RAM. It will only read the data it needs to read. I think collection open is actually pretty efficient too, I'm not worried about that. I do worry that this O(n) storage requirement would be in one single JSON object in the collection though and that (even if sqlite is efficient) would be a burden

[prateek-singh-3212]

Yes, Sorry droped some changes while rebasing. Fixed Now 🙌

[prateek-singh-3212]

@david-allison I think thir is SDK error while running android emulator test. Can you re-run it

[david-allison]

@david-allison I think thir is SDK error while running android emulator test. Can you re-run it

done

[prateek-singh-3212]

Architectural question about one "unit of work" per time, rather than per deck.

@ankidroid/gsoc-mentor Please take a look.

Gave a thought to this and designed the algo ALGO FOR DECK NOTIFICATION Input: Map<time, List<DeckId>>

We store the the time and list of decks whose notification needs to be send on that time. Calculation of time is as follows we round to minutes to it nearest %10 so decks with time 11:56 11:58 is counted in 12:00

Working of algo:

1. Takes the input time and does the round off as explained above and adds the deckid in the respective timing.
2. If (new decklist contains only 1 entry then [Which we added up] ) Then Run the work Manager and calculate the new timing to restart the work manager.
3. else (new decklist contains more than 1 entry then [Which we added up]) then do noting i.e don't run the workmanager.

Cancelation of alarm

1. Takes the input time and does the round off as explained above.
2. if (list of decks contains only 1 deck) then delete the time from map run the worker and calaculate the next time of worker to start.
3. else (list of decks contains more then 1 deck) Simply pop out that deck id from list.

ALGO FOR ALL DECK NOTIFICATION All deck notification will work in the syncronization with above algo and addintion to this If Thier is no scheduled notification in next 1 hour then also work manager will run after 1 hour and check the all deck status.

[prateek-singh-3212]

@david-allison done with major fixes. couple of them needs disscussion. Use of TreeMap is intentional as it store data in sorted form. In our case it stores the notification time in ascending order.

[prateek-singh-3212]

e: /home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/main/java/com/ichi2/anki/NotificationDatastore.kt: (139, 23): None of the following functions can be called with the arguments supplied: public constructor JSONObject(copyFrom: JSONObject) defined in com.ichi2.utils.JSONObject public constructor JSONObject(x: JSONTokener?) defined in com.ichi2.utils.JSONObject public constructor JSONObject(source: String?) defined in com.ichi2.utils.JSONObject public constructor JSONObject(copyFrom: Map<Any?, Any?>) defined in com.ichi2.utils.JSONObject

I am passing the treeMap and it raned earlier. I think I is not running because I used Type alis

[prateek-singh-3212]

I was unable to fix this issue I don'nt know why. I used type alias but I tried it to pass using type casting as mentioned below. but it didn't worked.

val mapData = data as Map<String, MutableList<Long>> 
val jsonObj = JSONObject(mapData)

@mikehardy @Akshay0701

[david-allison]

Let's avoid the alias so it compiles and I can move on with a review.

[prateek-singh-3212]

@david-allison done with the changes

[david-allison]

What was the answer to:

My main question: in your design, is TreeMap<String, List<did>> going to be a source of truth, or should it be regenerated from the user preferences? It's a concern that the system runs without any input from the current user's preferences because the two can easily get out of sync.

[prateek-singh-3212]

Use of TreeMap is intentional as it store data in sorted form (ascending). In our case it stores the notification time in ascending order so most recent notification will always at top i.e index 0. I will be stored in deck notification datastore.

PS. It makes life little bit easier as most recent notification will always at top so if we want to check upcomming notification time then we can check element at 0 index.