Skip to content

Commit

Permalink
fix: 4222 - top barcode task refactoring (#4226)
Browse files Browse the repository at this point in the history
Impacted files:
* `app_en.arb`: added a label for "server timeout"
* `background_task_top_barcodes.dart`: now explicitly using page number parameter, as server data is not 100% reliable
* `dao_work_barcode.dart`: minor refactoring
* `offline_tasks_page.dart`: detected a typical "server timeout" error message and replaced it with a more user-friendly message
  • Loading branch information
monsieurtanuki committed Jun 25, 2023
1 parent 097a905 commit 25264c9
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 64 deletions.
101 changes: 51 additions & 50 deletions packages/smooth_app/lib/background/background_task_top_barcodes.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,23 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {
required super.work,
required super.pageSize,
required super.totalSize,
required this.pageNumber,
});

BackgroundTaskTopBarcodes.fromJson(Map<String, dynamic> json)
: super.fromJson(json);
: pageNumber = json[_jsonTagPageNumber] as int? ?? 1,
super.fromJson(json);

final int pageNumber;

static const String _jsonTagPageNumber = 'pageNumber';

@override
Map<String, dynamic> toJson() {
final Map<String, dynamic> result = super.toJson();
result[_jsonTagPageNumber] = pageNumber;
return result;
}

static const OperationType _operationType = OperationType.offlineBarcodes;

Expand All @@ -35,6 +48,7 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {
required final int pageSize,
required final int totalSize,
required final int soFarSize,
final int pageNumber = 1,
}) async {
final String uniqueId = await _operationType.getNewKey(
localDatabase,
Expand All @@ -46,6 +60,7 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {
work,
pageSize,
totalSize,
pageNumber,
);
await task.addToManager(localDatabase);
}
Expand All @@ -58,6 +73,7 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {
final String work,
final int pageSize,
final int totalSize,
final int pageNumber,
) =>
BackgroundTaskTopBarcodes._(
processName: _operationType.processName,
Expand All @@ -69,6 +85,7 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {
work: work,
pageSize: pageSize,
totalSize: totalSize,
pageNumber: pageNumber,
);

@override
Expand All @@ -79,72 +96,56 @@ class BackgroundTaskTopBarcodes extends BackgroundTaskProgressing {

@override
Future<void> execute(final LocalDatabase localDatabase) async {
final DaoWorkBarcode daoWorkBarcode = DaoWorkBarcode(localDatabase);
final int soFarBefore = await daoWorkBarcode.getCount(work);
if (soFarBefore >= totalSize) {
// not likely
return;
final SearchResult searchResult = await OpenFoodAPIClient.searchProducts(
ProductQuery.getUser(),
ProductSearchQueryConfiguration(
fields: <ProductField>[ProductField.BARCODE],
parametersList: <Parameter>[
PageSize(size: pageSize),
PageNumber(page: pageNumber),
const SortBy(option: SortOption.POPULARITY),
],
language: ProductQuery.getLanguage(),
country: ProductQuery.getCountry(),
version: ProductQuery.productQueryVersion,
),
);
if (searchResult.products == null || searchResult.count == null) {
throw Exception('Cannot download top barcodes');
}
int newTotalSize = searchResult.count!;
if (newTotalSize > totalSize) {
newTotalSize = totalSize;
}
final bool ok = await _getBarcodes(localDatabase);
if (!ok) {
// something failed, let's get out of here.
return;
final Set<String> barcodes = <String>{};
for (final Product product in searchResult.products!) {
barcodes.add(product.barcode!);
}
final DaoWorkBarcode daoWorkBarcode = DaoWorkBarcode(localDatabase);
await daoWorkBarcode.putAll(work, barcodes);
// if we haven't downloaded a full page, it means that there's no data left.
final bool fullPageDownloaded = barcodes.length == pageSize;
final int soFarAfter = await daoWorkBarcode.getCount(work);
if (soFarAfter < totalSize) {
if (soFarAfter < newTotalSize && fullPageDownloaded) {
// we still have barcodes to download
await addTask(
localDatabase: localDatabase,
work: work,
pageSize: pageSize,
totalSize: totalSize,
totalSize: newTotalSize,
soFarSize: soFarAfter,
pageNumber: pageNumber + 1,
);
} else {
// we have all the barcodes; now we need to download the products.
await BackgroundTaskDownloadProducts.addTask(
localDatabase: localDatabase,
work: work,
pageSize: pageSize,
totalSize: totalSize,
totalSize: soFarAfter,
soFarSize: 0,
downloadFlag: BackgroundTaskDownloadProducts.flagMaskExcludeKP,
);
}
}

/// Returns true if somehow we can go on with the process.
Future<bool> _getBarcodes(final LocalDatabase localDatabase) async {
final DaoWorkBarcode daoWorkBarcode = DaoWorkBarcode(localDatabase);
final int soFar = await daoWorkBarcode.getCount(work);
if (soFar >= totalSize) {
// we're done!
return true;
}
final int pageNumber = (soFar ~/ pageSize) + 1;
final ProductSearchQueryConfiguration queryConfig =
ProductSearchQueryConfiguration(
fields: <ProductField>[ProductField.BARCODE],
parametersList: <Parameter>[
PageSize(size: pageSize),
PageNumber(page: pageNumber),
const SortBy(option: SortOption.POPULARITY),
],
language: ProductQuery.getLanguage(),
country: ProductQuery.getCountry(),
version: ProductQuery.productQueryVersion,
);
final SearchResult searchResult = await OpenFoodAPIClient.searchProducts(
ProductQuery.getUser(),
queryConfig,
);
if (searchResult.products == null) {
// not expected
return false;
}
final List<String> barcodes = <String>[];
for (final Product product in searchResult.products!) {
barcodes.add(product.barcode!);
}
await daoWorkBarcode.putAll(work, barcodes);
return true;
}
}
26 changes: 12 additions & 14 deletions packages/smooth_app/lib/database/dao_work_barcode.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@ class DaoWorkBarcode extends AbstractSqlDao {
static const String _columnWork = 'work';
static const String _columnBarcode = 'barcode';

static const List<String> _columns = <String>[
_columnWork,
_columnBarcode,
];

static FutureOr<void> onUpgrade(
final Database db,
final int oldVersion,
Expand Down Expand Up @@ -95,9 +90,13 @@ class DaoWorkBarcode extends AbstractSqlDao {
int count = 0;

Future<void> rawInsert() async {
final int inserted = await databaseExecutor.rawInsert(
'insert into $_table(${_columns.join(',')}) '
'values(?,?)${',(?,?)' * (parameters.length ~/ 2 - 1)}',
if (parameters.isEmpty) {
return;
}
final int inserted = parameters.length ~/ 2;
await databaseExecutor.rawInsert(
'insert into $_table($_columnWork,$_columnBarcode) '
'values(?,?)${',(?,?)' * (inserted - 1)}',
parameters,
);
count += inserted;
Expand All @@ -111,9 +110,7 @@ class DaoWorkBarcode extends AbstractSqlDao {
parameters.clear();
}
}
if (parameters.isNotEmpty) {
await rawInsert();
}
await rawInsert();
return count;
}

Expand Down Expand Up @@ -152,6 +149,9 @@ class DaoWorkBarcode extends AbstractSqlDao {
int count = 0;

Future<void> rawDelete() async {
if (parameters.length < 2) {
return;
}
final int deleted = await databaseExecutor.delete(
_table,
where: '$_columnWork = ? '
Expand All @@ -170,9 +170,7 @@ class DaoWorkBarcode extends AbstractSqlDao {
parameters.add(work);
}
}
if (parameters.isNotEmpty) {
await rawDelete();
}
await rawDelete();
return count;
}
}
1 change: 1 addition & 0 deletions packages/smooth_app/lib/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -2078,6 +2078,7 @@
"background_task_title": "Pending contributions",
"background_task_subtitle": "Your contributions are automatically saved to our server, but not always in real-time.",
"background_task_list_empty": "No Pending Background Tasks",
"background_task_error_server_time_out": "Server timeout",
"background_task_error_no_internet": "Internet connection error. Try later.",
"background_task_operation_unknown": "unknown operation type",
"background_task_operation_details": "detailed changes",
Expand Down
5 changes: 5 additions & 0 deletions packages/smooth_app/lib/pages/offline_tasks_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ class _OfflineTaskState extends State<OfflineTaskPage> {
case BackgroundTaskManager.taskStatusStopAsap:
return appLocalizations.background_task_run_to_be_deleted;
}
// "startsWith" because there's some kind of "chr(13)" at the end.
if (status.startsWith(
'Exception: JSON expected, html found: <head><title>504 Gateway Time-out</title></head>')) {
return appLocalizations.background_task_error_server_time_out;
}
return status;
}

Expand Down

0 comments on commit 25264c9

Please sign in to comment.