-
-
Notifications
You must be signed in to change notification settings - Fork 448
fix: remove broken stale request check and handle network errors in autocomplete #7441
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
base: develop
Are you sure you want to change the base?
Changes from 7 commits
daf43e7
3611e24
2cc94a8
09b4200
018aa19
afdf76b
9e09e0a
9e84865
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import 'dart:async'; | ||
| import 'dart:convert'; | ||
|
|
||
| import 'package:smooth_app/database/abstract_sql_dao.dart'; | ||
| import 'package:smooth_app/database/local_database.dart'; | ||
| import 'package:sqflite/sqflite.dart'; | ||
|
|
||
| class DaoNamespace extends AbstractSqlDao { | ||
| DaoNamespace(super.localDatabase); | ||
|
|
||
| static const String _TABLE_NAMESPACE = 'autocomplete_namespace'; | ||
| static const String _TABLE_NAMESPACE_COLUMN_ID = 'id'; | ||
| static const String _TABLE_NAMESPACE_COLUMN_NAMESPACE = 'namespace'; | ||
|
|
||
| static FutureOr<void> onUpgrade( | ||
| final Database db, | ||
| final int oldVersion, | ||
| final int newVersion, | ||
| ) async { | ||
| if (oldVersion < 10) { | ||
| await db.execute( | ||
| 'create table $_TABLE_NAMESPACE(' | ||
| '$_TABLE_NAMESPACE_COLUMN_ID INTEGER PRIMARY KEY AUTOINCREMENT' | ||
| ',$_TABLE_NAMESPACE_COLUMN_NAMESPACE TEXT NOT NULL UNIQUE' | ||
| ')', | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Returns the namespace id for the given [namespace] string. | ||
| Future<int> getOrCreateId(final String namespace) async { | ||
| final List<Map<String, dynamic>> rows = await localDatabase.database.query( | ||
| _TABLE_NAMESPACE, | ||
| columns: <String>[_TABLE_NAMESPACE_COLUMN_ID], | ||
| where: '$_TABLE_NAMESPACE_COLUMN_NAMESPACE = ?', | ||
| whereArgs: <String>[namespace], | ||
| ); | ||
| if (rows.isNotEmpty) { | ||
| return rows.first[_TABLE_NAMESPACE_COLUMN_ID] as int; | ||
| } | ||
| return localDatabase.database.insert(_TABLE_NAMESPACE, <String, dynamic>{ | ||
| _TABLE_NAMESPACE_COLUMN_NAMESPACE: namespace, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| class DaoAutocompleteCache extends AbstractSqlDao { | ||
| DaoAutocompleteCache(super.localDatabase); | ||
|
|
||
| static const String _TABLE_CACHE = 'autocomplete_cache'; | ||
| static const String _TABLE_CACHE_COLUMN_NAMESPACE_ID = 'namespace_id'; | ||
| static const String _TABLE_CACHE_COLUMN_QUERY = 'query'; | ||
| static const String _TABLE_CACHE_COLUMN_RESULTS = 'results'; | ||
| static const String _TABLE_CACHE_COLUMN_LAST_UPDATE = 'last_update'; | ||
|
|
||
| static const List<String> _columnsCache = <String>[ | ||
| _TABLE_CACHE_COLUMN_NAMESPACE_ID, | ||
| _TABLE_CACHE_COLUMN_QUERY, | ||
| _TABLE_CACHE_COLUMN_RESULTS, | ||
| _TABLE_CACHE_COLUMN_LAST_UPDATE, | ||
| ]; | ||
|
|
||
| static FutureOr<void> onUpgrade( | ||
| final Database db, | ||
| final int oldVersion, | ||
| final int newVersion, | ||
| ) async { | ||
| if (oldVersion < 10) { | ||
| await db.execute( | ||
| 'create table $_TABLE_CACHE(' | ||
| '$_TABLE_CACHE_COLUMN_NAMESPACE_ID INTEGER NOT NULL' | ||
| ',$_TABLE_CACHE_COLUMN_QUERY TEXT NOT NULL' | ||
| ',$_TABLE_CACHE_COLUMN_RESULTS TEXT NOT NULL' | ||
| ',$_TABLE_CACHE_COLUMN_LAST_UPDATE INTEGER NOT NULL' | ||
| ',PRIMARY KEY' | ||
| '($_TABLE_CACHE_COLUMN_NAMESPACE_ID' | ||
| ',$_TABLE_CACHE_COLUMN_QUERY) ON CONFLICT REPLACE' | ||
| ')', | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Returns cached results for [namespaceId] and [query]. | ||
| /// Returns null if not found. | ||
| Future<List<String>?> get(final int namespaceId, final String query) async { | ||
| final List<Map<String, dynamic>> rows = await localDatabase.database.query( | ||
| _TABLE_CACHE, | ||
| columns: _columnsCache, | ||
| where: | ||
| '$_TABLE_CACHE_COLUMN_NAMESPACE_ID = ?' | ||
| ' AND $_TABLE_CACHE_COLUMN_QUERY = ?', | ||
| whereArgs: <dynamic>[namespaceId, query], | ||
| ); | ||
| if (rows.isEmpty) { | ||
| return null; | ||
| } | ||
| final String json = rows.first[_TABLE_CACHE_COLUMN_RESULTS] as String; | ||
| return (jsonDecode(json) as List<dynamic>).cast<String>(); | ||
| } | ||
|
|
||
| /// Stores [results] for [namespaceId] and [query]. | ||
| Future<void> put( | ||
| final int namespaceId, | ||
| final String query, | ||
| final List<String> results, | ||
| ) async { | ||
| await localDatabase.database.insert(_TABLE_CACHE, <String, dynamic>{ | ||
| _TABLE_CACHE_COLUMN_NAMESPACE_ID: namespaceId, | ||
| _TABLE_CACHE_COLUMN_QUERY: query, | ||
| _TABLE_CACHE_COLUMN_RESULTS: jsonEncode(results), | ||
| _TABLE_CACHE_COLUMN_LAST_UPDATE: LocalDatabase.nowInMillis(), | ||
| }, conflictAlgorithm: ConflictAlgorithm.replace); | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| } | ||
| } | ||
|
Sherley-Sonali marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| import 'package:openfoodfacts/openfoodfacts.dart'; | ||
| import 'package:smooth_app/pages/folksonomy/folksonomy_autocompleter.dart'; | ||
| import 'package:smooth_app/query/product_query.dart'; | ||
|
|
||
| /// Abstract interface for server-backed autocomplete with cache namespace support. | ||
| abstract class ServerSuggestion { | ||
| String getNamespace(String soFar); | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| Future<List<String>> getSuggestionsFromServer(String soFar); | ||
| } | ||
|
|
||
| /// Implementation for TagType autocompleters (categories, labels, origins, etc.) | ||
| class TagTypeServerSuggestion implements ServerSuggestion { | ||
| TagTypeServerSuggestion({required this.tagType, required this.productType}); | ||
|
|
||
| final TagType tagType; | ||
| final ProductType productType; | ||
|
|
||
| OpenFoodFactsLanguage get _language => ProductQuery.getLanguage(); | ||
| OpenFoodFactsCountry? get _country => ProductQuery.getCountry(); | ||
| UriProductHelper get _uriHelper => | ||
| ProductQuery.getUriProductHelper(productType: productType); | ||
|
|
||
| @override | ||
| String getNamespace(String soFar) { | ||
| return '${_uriHelper.domain}|tagtype|${tagType.offTag}|${_language.offTag}|${_country?.offTag ?? ''}'; | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>> getSuggestionsFromServer(String soFar) async { | ||
| final TagTypeAutocompleter autocompleter = TagTypeAutocompleter( | ||
| tagType: tagType, | ||
| language: _language, | ||
| country: _country, | ||
| uriHelper: _uriHelper, | ||
| limit: 15, | ||
| ); | ||
| return autocompleter.getSuggestions(soFar); | ||
| } | ||
| } | ||
|
|
||
| /// Implementation for TaxonomyName autocompleters (brands) | ||
| class TaxonomyServerSuggestion implements ServerSuggestion { | ||
| TaxonomyServerSuggestion({ | ||
| required this.taxonomyNames, | ||
| required this.productType, | ||
| }); | ||
|
|
||
| final List<TaxonomyName> taxonomyNames; | ||
| final ProductType productType; | ||
|
|
||
| OpenFoodFactsLanguage get _language => OpenFoodFactsLanguage.ENGLISH; | ||
| UriProductHelper get _uriHelper => | ||
| ProductQuery.getUriProductHelper(productType: productType); | ||
|
|
||
| @override | ||
| String getNamespace(String soFar) { | ||
| return '${_uriHelper.domain}|taxonomy|${taxonomyNames.map((final TaxonomyName t) => t.offTag).join(',')}|${_language.offTag}'; | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>> getSuggestionsFromServer(String soFar) async { | ||
| final TaxonomyNameAutocompleter autocompleter = TaxonomyNameAutocompleter( | ||
| taxonomyNames: taxonomyNames, | ||
| language: _language, | ||
| uriHelper: _uriHelper, | ||
| limit: 25, | ||
| fuzziness: Fuzziness.none, | ||
| user: ProductQuery.getReadUser(), | ||
| ); | ||
| return autocompleter.getSuggestions(soFar); | ||
| } | ||
| } | ||
|
|
||
| /// Implementation for folksonomy keys autocompleter | ||
| class FolksonomyKeysServerSuggestion implements ServerSuggestion { | ||
| const FolksonomyKeysServerSuggestion(); | ||
|
|
||
| UriHelper get _uriHelper => ProductQuery.uriFolksonomyHelper; | ||
|
|
||
| @override | ||
| String getNamespace(String soFar) { | ||
| return '${_uriHelper.host}|folksonomy|keys'; | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>> getSuggestionsFromServer(String soFar) async { | ||
| const FolksonomyKeysAutocompleter autocompleter = | ||
| FolksonomyKeysAutocompleter(limit: 10); | ||
| return autocompleter.getSuggestions(soFar); | ||
| } | ||
| } | ||
|
|
||
| /// Implementation for folksonomy values autocompleter | ||
| class FolksonomyValuesServerSuggestion implements ServerSuggestion { | ||
| FolksonomyValuesServerSuggestion({required String Function() keyProvider}) | ||
| : _keyProvider = keyProvider; | ||
|
|
||
| final String Function() _keyProvider; | ||
|
|
||
| UriHelper get _uriHelper => ProductQuery.uriFolksonomyHelper; | ||
|
|
||
| @override | ||
| String getNamespace(String soFar) { | ||
| final String key = _keyProvider().trim(); | ||
| return '${_uriHelper.host}|folksonomy|values|$key'; | ||
| } | ||
|
|
||
| @override | ||
| Future<List<String>> getSuggestionsFromServer(String soFar) async { | ||
| final FolksonomyValuesAutocompleter autocompleter = | ||
| FolksonomyValuesAutocompleter(keyProvider: _keyProvider, limit: 10); | ||
| return autocompleter.getSuggestions(soFar); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,47 @@ | ||||||||||||||||||||||
| import 'package:smooth_app/database/dao_autocomplete.dart'; | ||||||||||||||||||||||
| import 'package:smooth_app/database/local_database.dart'; | ||||||||||||||||||||||
| import 'package:smooth_app/pages/input/server_suggestion.dart'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| class SuggestionCache { | ||||||||||||||||||||||
| SuggestionCache({ | ||||||||||||||||||||||
| required this.serverSuggestion, | ||||||||||||||||||||||
| required this.localDatabase, | ||||||||||||||||||||||
| }); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| final ServerSuggestion serverSuggestion; | ||||||||||||||||||||||
| final LocalDatabase localDatabase; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // namespace string → namespace DB id (avoids repeated DB lookup per session) | ||||||||||||||||||||||
| final Map<String, int> _namespaceIdCache = <String, int>{}; | ||||||||||||||||||||||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Future<int> _getNamespaceId(final String namespace) async { | ||||||||||||||||||||||
| final int? cached = _namespaceIdCache[namespace]; | ||||||||||||||||||||||
| if (cached != null) { | ||||||||||||||||||||||
| return cached; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| final int id = await DaoNamespace(localDatabase).getOrCreateId(namespace); | ||||||||||||||||||||||
| _namespaceIdCache[namespace] = id; | ||||||||||||||||||||||
| return id; | ||||||||||||||||||||||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Future<List<String>> getSuggestions(final String soFar) async { | ||||||||||||||||||||||
| final String namespace = serverSuggestion.getNamespace(soFar); | ||||||||||||||||||||||
| final int namespaceId = await _getNamespaceId(namespace); | ||||||||||||||||||||||
|
Comment on lines
+26
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Just a suggestion. |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| final DaoAutocompleteCache daoCache = DaoAutocompleteCache(localDatabase); | ||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
cf. |
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // 1. DB | ||||||||||||||||||||||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||
| final List<String>? db = await daoCache.get(namespaceId, soFar); | ||||||||||||||||||||||
| if (db != null) { | ||||||||||||||||||||||
| return db; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // 2. Server | ||||||||||||||||||||||
| final List<String> results = await serverSuggestion | ||||||||||||||||||||||
| .getSuggestionsFromServer(soFar); | ||||||||||||||||||||||
| if (results.isNotEmpty) { | ||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't agree at all: knowing that there's no result is important. |
||||||||||||||||||||||
| await daoCache.put(namespaceId, soFar, results); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| return results; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's typically why I asked you to create 2 separate files. Which for some reason you ignored, and I'm a bit puzzled with that.
When I review, the whole file is considered as changed, and I have to read again ALL the file, when you may have simply edited one class.