Limit the length of history

openfoodfacts / smooth-app

🤳🥫 The new Open Food Facts mobile application for Android and iOS, crafted with Flutter and Dart

https://world.openfoodfacts.org/open-food-facts-mobile-app?utm_source=off&utf_medium=web&utm_campaign=github-repo

Apache License 2.0

835 stars 279 forks source link

Limit the length of history #1840

Open g123k opened 2 years ago

g123k commented 2 years ago

It seems there is no limit on how many products are saved in history. We should implement a limit on this?

TODO: Discuss how many items (1000?)

teolemon commented 2 years ago

What would be the WHYs on that ?
- Performance in Flutter ?
- The volume of queries during migration ?

M123-dev commented 2 years ago

What should we do if our fixed limit is reached?

Don't allow further items? - Probably best solution for normal lists.

But what if someone wants a bigger list

Implement some kind of LRU algorithm to remove unused items? - Probably best for history.

But how to inform the user that we removed a item, just because

We had the same suggestion for limiting items in the scan carousel and I have to say I don't see the point of adding additional logic for limiting something like that.

It's of course a different thing when talking about cache or reloading all products in the history, but I wouldn't add a limit here.

g123k commented 2 years ago

This issue is only for Smoothie, nothing related to the migration itself. But as the app grows, it's not a good behavior to keep many items in a history (only the recent items are necessary)

monsieurtanuki commented 2 years ago

I see those potential issues:

for the moment, if we have a barcode in a list that means that we have the corresponding product in database too. Each time it's about 25Kb, for data that can be stalled. Potentially a lot a wasted space, that the user cannot currently remove (unless the whole app data is cleared)
as we currently use hive for lists, we load all list contents (but not products, that are in lazy mode) at init time - slower startup with bigger lists
I cannot think of a use-case where keeping a history of 1000 products makes sense, but I guess you guys have more experience with the android and ios apps and know how far the users may go

My suggestions:

switch back to sqflite (= instant startup)?
make it possible for lists not to always have the corresponding products?
implement a user-defined list length limit, with a LRU?
implement a user-defined Mb limit, with a LRU?

M123-dev commented 2 years ago

@monsieurtanuki

switch back to sqflite (= instant startup)?

I don't have a problem with that, I think the idea of web support has been forgotten for now. In terms of performance, it almost only has advantages. The only question is how long that would take, and how hard the switch is. Since the release is just around the corner, around 1 to 2 weeks at most.

It's probably smart to switch beforehand, but not if it causes any bugs after shipping.

make it possible for lists not to always have the corresponding products?

Thats definitely a good idea. We don't need a inifinite big cache just because a user has scanned probably thousands of products.

implement a user-defined list length limit, with a LRU?

I guess a limit in the app settings for history will be enouph, lists easy to delete as a whole if they aren't needed anymore.

implement a user-defined Mb limit, with a LRU?

💯

Any opinion on that @g123k

monsieurtanuki commented 2 years ago

@M123-dev @g123k The key issue is that local lists need locally stored products. Which may not be a problem with "decent" users that do not keep thousands of products (30kb each), so let's keep it that way for the moment.

We can

add settings like "delete old history items" / "delete old queries"
systematically remove a product if it's not in a list anymore ("history" being a list)