fix: remove broken stale request check and handle network errors in autocomplete #7441
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| 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 DaoAutocomplete extends AbstractSqlDao { | ||
|
Contributor
There was a problem hiding this comment. It would be more clear if you had one dao per table. Like in the rest of the project btw. |
||
| DaoAutocomplete(super.localDatabase); | ||
|
|
||
| // autocomplete_namespace table | ||
| static const String _TABLE_NAMESPACE = 'autocomplete_namespace'; | ||
| static const String _TABLE_NAMESPACE_COLUMN_ID = 'id'; | ||
| static const String _TABLE_NAMESPACE_COLUMN_NAMESPACE = 'namespace'; | ||
|
|
||
| // autocomplete_cache table | ||
| 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, | ||
| ]; | ||
|
|
||
| /// Session-level in-memory cache for namespace string → integer id. | ||
| /// Eliminates redundant DB lookups - there are only 7-22 namespaces | ||
| /// for a typical user so this map stays tiny for the lifetime of the | ||
| /// app process. | ||
| static final Map<String, int> _namespaceCache = <String, int>{}; | ||
|
|
||
| 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' | ||
| ')', | ||
| ); | ||
| await db.execute( | ||
| 'create index idx_namespace' | ||
| ' on $_TABLE_NAMESPACE($_TABLE_NAMESPACE_COLUMN_NAMESPACE)', | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| ); | ||
| 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 the namespace id for the given [namespace] string. | ||
| /// Creates a new row if the namespace does not exist yet. | ||
| /// Result is cached in memory so subsequent calls within the same | ||
| /// app session never hit the database. | ||
| Future<int> _getOrCreateNamespaceId(final String namespace) async { | ||
| final int? cached = _namespaceCache[namespace]; | ||
|
Contributor
There was a problem hiding this comment. I don't think it's the DAO's job to cache data. Just make DAO what they're meant for: simple access to database. In the rest of the code, I imagine something like "oh we're looking for suggestions of category in English, let's get the namespace for it from DaoNamespace, and while we're on the page let's use that same namespace while we're on looking for suggestions."
Author
There was a problem hiding this comment. DAOs are now plain DB access; caching logic moved to new SuggestionCache class |
||
| if (cached != null) { | ||
| return cached; | ||
| } | ||
| 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], | ||
| ); | ||
| final int id; | ||
| if (rows.isNotEmpty) { | ||
| id = rows.first[_TABLE_NAMESPACE_COLUMN_ID] as int; | ||
| } else { | ||
| id = await localDatabase.database.insert( | ||
| _TABLE_NAMESPACE, | ||
| <String, dynamic>{_TABLE_NAMESPACE_COLUMN_NAMESPACE: namespace}, | ||
| ); | ||
| } | ||
| _namespaceCache[namespace] = id; | ||
| return id; | ||
| } | ||
|
|
||
| /// Returns cached results for the given [namespace] and [query]. | ||
| /// Returns null if not found. | ||
| Future<List<String>?> getResults( | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| final String namespace, | ||
| final String query, | ||
| ) async { | ||
| final int namespaceId = await _getOrCreateNamespaceId(namespace); | ||
| 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 the given [namespace] and [query]. | ||
| Future<void> storeResults( | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| final String namespace, | ||
|
Sherley-Sonali marked this conversation as resolved.
Outdated
|
||
| final String query, | ||
| final List<String> results, | ||
| ) async { | ||
| final int namespaceId = await _getOrCreateNamespaceId(namespace); | ||
| 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
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Sherley-Sonali marked this conversation as resolved.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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((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); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.