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

fix: 4066 - top 1K pre-download and full refresh as background tasks #4131

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • This is the first step of Replace the current server call for offline products to something more scalable #4066: the top 1K pre-download and the full refresh download are now background tasks, instead of letting the user wait in front of an endless progress indicator.
  • So far we only used background tasks that were barcode-centered (e.g. edit the category for this barcode). This is not the case anymore as here we're not focused on a specific barcode. That required a bit of refactoring of the background tasks.
  • The next steps would be
    • to optimize the downloads - instead of downloading 1K products or refreshing 2K products, we could do that by chunks of 100 products. More iterations, but less demands on server, smaller downloads and smaller local memory usage.
    • to split the downloads in separate tasks - in order no to block the user that edits a product and would have to implicitly wait that the 2K refresh task is over before the server actually receives the edit as a task.
    • perhaps send a notification to the user at the end

Screenshot

(just tell me which labels you would like and I will put them in the next commit).

Refresh database:

snackbar task
Screenshot_2023-06-12-17-42-54 Screenshot_2023-06-12-17-43-25

Top 1K download:

snackbar task
Screenshot_2023-06-12-17-41-04 Screenshot_2023-06-12-17-41-10

Part of

Files

Deleted file:

  • products_preload_helper.dart

New files:

  • background_task.dart: Abstract background task.
  • background_task_full_refresh.dart: Background task about refreshing all the already downloaded products.
  • background_task_offline.dart: Background task about pre-downloading top n products for offline usage.

Impacted files:

  • background_task_barcode.dart
  • background_task_crop.dart: refactored
  • background_task_details.dart: refactored
  • background_task_hunger_games.dart: refactored
  • background_task_image.dart: refactored
  • background_task_manager.dart
  • background_task_refresh_later.dart: refactored
  • background_task_unselect.dart: refactored
  • background_task_upload.dart: refactored
  • offline_data_page.dart: now using background task for offline and full refresh tasks
  • offline_tasks_page.dart: dealing with the "no barcode" case
  • operation_type.dart: added values offline and fullRefresh; added processName field; added a BackgroundTask constructor
  • product_refresher.dart: bug fix

Deleted file:
* `products_preload_helper.dart`

New files:
* `background_task.dart`: Abstract background task.
* `background_task_full_refresh.dart`: Background task about refreshing all the already downloaded products.
* `background_task_offline.dart`: Background task about pre-downloading top n products for offline usage.

Impacted files:
* `background_task_barcode.dart`
* `background_task_crop.dart`: refactored
* `background_task_details.dart`: refactored
* `background_task_hunger_games.dart`: refactored
* `background_task_image.dart`: refactored
* `background_task_manager.dart`
* `background_task_refresh_later.dart`: refactored
* `background_task_unselect.dart`: refactored
* `background_task_upload.dart`: refactored
* `offline_data_page.dart`: now using background task for offline and full refresh tasks
* `offline_tasks_page.dart`: dealing with the "no barcode" case
* `operation_type.dart`: added values `offline` and `fullRefresh`; added `processName` field; added a `BackgroundTask` constructor
* `product_refresher.dart`: bug fix
@codecov-commenter
Copy link

codecov-commenter commented Jun 12, 2023

Codecov Report

Merging #4131 (2204874) into develop (abab934) will increase coverage by 0.04%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4131      +/-   ##
===========================================
+ Coverage    10.88%   10.92%   +0.04%     
===========================================
  Files          273      275       +2     
  Lines        13568    13517      -51     
===========================================
  Hits          1477     1477              
+ Misses       12091    12040      -51     
Impacted Files Coverage Δ
...ges/smooth_app/lib/background/background_task.dart 0.00% <0.00%> (ø)
...th_app/lib/background/background_task_barcode.dart 0.00% <0.00%> (ø)
...mooth_app/lib/background/background_task_crop.dart 0.00% <0.00%> (ø)
...th_app/lib/background/background_task_details.dart 0.00% <0.00%> (ø)
...p/lib/background/background_task_full_refresh.dart 0.00% <0.00%> (ø)
...p/lib/background/background_task_hunger_games.dart 0.00% <0.00%> (ø)
...ooth_app/lib/background/background_task_image.dart 0.00% <0.00%> (ø)
...th_app/lib/background/background_task_manager.dart 0.00% <0.00%> (ø)
...th_app/lib/background/background_task_offline.dart 0.00% <0.00%> (ø)
.../lib/background/background_task_refresh_later.dart 0.00% <0.00%> (ø)
... and 11 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@teolemon
Copy link
Member

@stephanegigandet What do you prefer?
The next steps would be
to optimize the downloads - instead of downloading 1K products or refreshing 2K products, we could do that by chunks of 100 products. More iterations, but less demands on server, smaller downloads and smaller local memory usage.

@g123k g123k mentioned this pull request Jun 13, 2023
Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small question, but otherwise LGTM

languageCode: ProductQuery.getLanguage().offTag,
user: jsonEncode(ProductQuery.getUser().toJson()),
country: ProductQuery.getCountry()!.offTag,
stamp: ';fullRefresh',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By curiosity, why do you start it with ; ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before that PR we always had a single barcode involved, which meant a stamp like $barcode;whatever (useful to check if a stamp is related to a barcode).
As there are no single barcodes for full refresh or top 1k, I somehow kept the same kind of stamp but without barcode.
Could be improved, but good enough for the moment.

@monsieurtanuki monsieurtanuki merged commit 55b4894 into openfoodfacts:develop Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants