fix: await remote even when instance is present in memory cache (#358)

GetDutchie · Jul 10, 2023 · de8c83e · de8c83e
1 parent 4e1efca
commit de8c83e
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 6 deletions.
diff --git a/packages/brick_offline_first/CHANGELOG.md b/packages/brick_offline_first/CHANGELOG.md
@@ -1,6 +1,9 @@
 ## Unreleased
 
+## 3.1.0
+
 * Apply standardized lints
+* Respect `awaitRemote` policy for `OfflineFirstRepository#get` - fixes a bug where an instance in memory cache would early-return before hitting remote despite the requested `await` or `alwaysHydrate` policy
 
 ## 3.0.3
 

diff --git a/packages/brick_offline_first/lib/src/offline_first_repository.dart b/packages/brick_offline_first/lib/src/offline_first_repository.dart
@@ -175,18 +175,24 @@ abstract class OfflineFirstRepository<RepositoryModel extends OfflineFirstModel>
     query = (withPolicy ?? Query()).copyWith(action: QueryAction.get);
     logger.finest('#get: $TModel $query');
 
-    if (memoryCacheProvider.canFind<TModel>(query)) {
+    final requireRemote = policy == OfflineFirstGetPolicy.awaitRemote;
+    final hydrateUnexisting = policy == OfflineFirstGetPolicy.awaitRemoteWhenNoneExist;
+    final alwaysHydrate = policy == OfflineFirstGetPolicy.alwaysHydrate;
+
+    if (memoryCacheProvider.canFind<TModel>(query) && !requireRemote) {
       final memoryCacheResults = memoryCacheProvider.get<TModel>(query: query, repository: this);
 
+      if (alwaysHydrate) {
+        // start round trip for fresh data
+        // ignore: unawaited_futures
+        hydrate<TModel>(query: query, deserializeSqlite: !seedOnly);
+      }
+
       if (memoryCacheResults?.isNotEmpty ?? false) return memoryCacheResults!;
     }
 
     final modelExists = await exists<TModel>(query: query);
 
-    final requireRemote = policy == OfflineFirstGetPolicy.awaitRemote;
-    final hydrateUnexisting = policy == OfflineFirstGetPolicy.awaitRemoteWhenNoneExist;
-    final alwaysHydrate = policy == OfflineFirstGetPolicy.alwaysHydrate;
-
     if (requireRemote || (hydrateUnexisting && !modelExists)) {
       return await hydrate<TModel>(query: query, deserializeSqlite: !seedOnly);
     } else if (alwaysHydrate) {

diff --git a/packages/brick_offline_first/pubspec.yaml b/packages/brick_offline_first/pubspec.yaml
@@ -5,7 +5,7 @@ homepage: https://github.com/GetDutchie/brick/tree/main/packages/brick_offline_f
 issue_tracker: https://github.com/GetDutchie/brick/issues
 repository: https://github.com/GetDutchie/brick
 
-version: 3.0.3
+version: 3.1.0
 
 environment:
   sdk: ">=2.12.0 <4.0.0"

diff --git a/packages/brick_offline_first/test/offline_first/helpers/test_domain.dart b/packages/brick_offline_first/test/offline_first/helpers/test_domain.dart
@@ -11,6 +11,8 @@ import 'package:brick_sqlite/memory_cache_provider.dart';
 import '__mocks__.dart';
 
 class TestProvider extends Provider<TestModel> {
+  var methodsCalled = <String>[];
+
   @override
   final TestModelDictionary modelDictionary;
 
@@ -22,6 +24,7 @@ class TestProvider extends Provider<TestModel> {
     Query? query,
     ModelRepository<TestModel>? repository,
   }) {
+    methodsCalled.add('delete');
     if (TestRepository.throwOnNextRemoteMutation) throw const SocketException('Remote failed');
     return true;
   }
@@ -31,6 +34,7 @@ class TestProvider extends Provider<TestModel> {
     Query? query,
     ModelRepository<TestModel>? repository,
   }) async {
+    methodsCalled.add('get');
     final adapter = modelDictionary.adapterFor[T]!;
     final data = [
       {'name': 'SqliteName'}
@@ -48,6 +52,7 @@ class TestProvider extends Provider<TestModel> {
     Query? query,
     ModelRepository<TestModel>? repository,
   }) {
+    methodsCalled.add('upsert');
     if (TestRepository.throwOnNextRemoteMutation) throw const SocketException('Remote failed');
     return Future<T>.value(instance);
   }

diff --git a/packages/brick_offline_first/test/offline_first/offline_first_repository_test.dart b/packages/brick_offline_first/test/offline_first/offline_first_repository_test.dart
@@ -2,10 +2,12 @@ import 'dart:io';
 
 import 'package:brick_core/query.dart';
 import 'package:brick_offline_first/src/offline_first_policy.dart';
+import 'package:brick_sqlite/db.dart';
 import 'package:sqflite_common_ffi/sqflite_ffi.dart';
 import 'package:test/test.dart';
 
 import 'helpers/__mocks__.dart';
+import 'helpers/test_domain.dart';
 
 void main() {
   sqfliteFfiInit();
@@ -19,6 +21,7 @@ void main() {
 
     tearDown(() {
       TestRepository.throwOnNextRemoteMutation = false;
+      (TestRepository().remoteProvider as TestProvider).methodsCalled.clear();
     });
 
     test('#applyPolicyToQuery', () async {
@@ -81,6 +84,29 @@ void main() {
         expect(findByName.first.name, horse.name);
       });
 
+      test('OfflineFirstGetPolicy.awaitRemote', () async {
+        TestRepository().memoryCacheProvider.managedModelTypes.add(Horse);
+
+        try {
+          final fetchFirst =
+              await TestRepository().get<Horse>(policy: OfflineFirstGetPolicy.awaitRemote);
+          expect(fetchFirst, isNotEmpty);
+          final fetchMemory = TestRepository().memoryCacheProvider.get<Horse>(
+                query: Query.where(InsertTable.PRIMARY_KEY_FIELD, fetchFirst.first.primaryKey),
+              );
+          expect(fetchMemory, isNotEmpty);
+          final fetchAgain =
+              await TestRepository().get<Horse>(policy: OfflineFirstGetPolicy.awaitRemote);
+
+          // The TestProvider does not have unique keys, so Brick can't compare based on the name,
+          // giving the appearance of two distinct records
+          expect(fetchAgain, hasLength(2));
+          expect((TestRepository().remoteProvider as TestProvider).methodsCalled, hasLength(2));
+        } finally {
+          TestRepository().memoryCacheProvider.managedModelTypes.remove(Horse);
+        }
+      });
+
       test('OfflineFirstGetPolicy.localOnly', () async {
         final results = await TestRepository().get<Horse>(policy: OfflineFirstGetPolicy.localOnly);