Fix cache synchronizing side effect when open folder

lib/features/thread/data/repository/thread_repository_impl.dart

@@ -96,15 +96,17 @@ class ThreadRepositoryImpl extends ThreadRepository {
       await _updateEmailCache(accountId, session.username, newCreated: networkEmailResponse.emailList);
     }
 
-    if (localEmailResponse.hasState() && getLatestChanges) {
+    if (localEmailResponse.hasState()) {
       log('ThreadRepositoryImpl::getAllEmail(): filter = ${emailFilter?.mailboxId} local has state: ${localEmailResponse.state}');
-      await _synchronizeCacheWithChanges(
+      if (getLatestChanges) {
-        session,
+        await _synchronizeCacheWithChanges(
-        accountId,
+          session,
-        localEmailResponse.state!,
+          accountId,
-        propertiesCreated: propertiesCreated,
+          localEmailResponse.state!,
-        propertiesUpdated: propertiesUpdated
+          propertiesCreated: propertiesCreated,
-      );
+          propertiesUpdated: propertiesUpdated
+        );
+      }
     } else {
       if (networkEmailResponse != null) {
         log('ThreadRepositoryImpl::getAllEmail(): filter = ${emailFilter?.mailboxId} no local state -> update from network: ${networkEmailResponse.state}');

test/features/thread/data/repository/thread_repository_impl_test.dart

@@ -3,12 +3,17 @@ import 'package:flutter_test/flutter_test.dart';
 import 'package:jmap_dart_client/jmap/core/id.dart';
 import 'package:jmap_dart_client/jmap/core/state.dart';
 import 'package:jmap_dart_client/jmap/mail/email/email.dart';
+import 'package:jmap_dart_client/jmap/mail/mailbox/mailbox.dart';
 import 'package:mockito/annotations.dart';
 import 'package:mockito/mockito.dart';
 import 'package:tmail_ui_user/features/mailbox/data/datasource/state_datasource.dart';
 import 'package:tmail_ui_user/features/thread/data/datasource/thread_datasource.dart';
 import 'package:tmail_ui_user/features/thread/data/model/email_change_response.dart';
 import 'package:tmail_ui_user/features/thread/data/repository/thread_repository_impl.dart';
+import 'package:tmail_ui_user/features/thread/domain/constants/thread_constants.dart';
+import 'package:tmail_ui_user/features/thread/domain/model/email_filter.dart';
+import 'package:tmail_ui_user/features/thread/domain/model/email_response.dart';
+import 'package:tmail_ui_user/features/thread/domain/model/filter_message_option.dart';
 
 import '../../../../fixtures/account_fixtures.dart';
 import '../../../../fixtures/session_fixtures.dart';
@@ -19,23 +24,30 @@ import 'thread_repository_impl_test.mocks.dart';
   MockSpec<StateDataSource>(),
 ])
 void main() {
-  final threadDataSource = MockThreadDataSource();
+  late MockThreadDataSource threadDataSource;
-  final stateDataSource = MockStateDataSource();
+  late MockStateDataSource stateDataSource;
-  final threadRepository = ThreadRepositoryImpl(
+  late ThreadRepositoryImpl threadRepository;
-    {
-      DataSourceType.network: threadDataSource,
-      DataSourceType.local: threadDataSource,
-    },
-    stateDataSource,
-  );
 
-  group('thread repository impl test:', () {
+  setUp(() {
-    test(
+    threadDataSource = MockThreadDataSource();
-      'should not call threadDatasource.getChanges '
+    stateDataSource = MockStateDataSource();
-      'when getAllEmail is called '
+    threadRepository = ThreadRepositoryImpl(
-      'and getLatestChanges is false',
+      {
-    () async {
+        DataSourceType.network: threadDataSource,
-      // arrange
+        DataSourceType.local: threadDataSource,
+      },
+      stateDataSource,
+    );
+  });
+
+  tearDown(() {
+    reset(threadDataSource);
+    reset(stateDataSource);
+  });
+
+  group('getAllEmail:', () {
+    test('when local cache is empty should fetch from network', () async {
+      // Arrange
       when(threadDataSource.getAllEmailCache(
         any,
         any,
@@ -43,47 +55,58 @@ void main() {
         inMailboxId: anyNamed('inMailboxId'),
         limit: anyNamed('limit'),
         sort: anyNamed('sort'),
-      )).thenAnswer(
+      )).thenAnswer((_) => Future.value([]));
-        (_) => Future.value(List.generate(30, (index) => Email(id: EmailId(Id('$index'))))),
-      );
-      when(stateDataSource.getState(
-        any,
-        any,
-        any,
-      )).thenAnswer((_) => Future.value(State('some-state')));
-      when(threadDataSource.getChanges(
-        any,
-        any,
-        any,
-        propertiesCreated: anyNamed('propertiesCreated'),
-        propertiesUpdated: anyNamed('propertiesUpdated'),
-      )).thenAnswer(
-        (_) => Future.value(EmailChangeResponse(hasMoreChanges: false)),
-      );
 
-      // act
+      when(stateDataSource.getState(any, any, any))
-      await threadRepository.getAllEmail(
+          .thenAnswer((_) => Future.value(null));
-        SessionFixtures.aliceSession,
-        AccountFixtures.aliceAccountId,
-        getLatestChanges: false,
-      ).last;
 
-      // assert
+      final networkEmails = List.generate(
-      verifyNever(threadDataSource.getChanges(
+        20, 
+        (index) => Email(id: EmailId(Id('network_$index')))
+      );
+      when(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      )).thenAnswer((_) => Future.value(EmailsResponse(
+          emailList: networkEmails,
+          state: State('network_state'))));
+
+      // Act
+      final responses = await threadRepository
+        .getAllEmail(
+          SessionFixtures.aliceSession,
+          AccountFixtures.aliceAccountId,
+        )
+        .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      expect(responses[0].emailList, networkEmails);
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: networkEmails,
+      ));
+      verify(stateDataSource.saveState(
         any,
         any,
         any,
-        propertiesCreated: anyNamed('propertiesCreated'),
-        propertiesUpdated: anyNamed('propertiesUpdated'),
       ));
     });
 
-    test(
+    test('when local cache has fewer than default limit emails '
-      'should call threadDatasource.getChanges '
+         'and no need getLatestChanges '
-      'when getAllEmail is called '
+         'should fetch from network '
-      'and getLatestChanges is true',
+         'and state should not be saved',
-    () async {
+        () async {
-      // arrange
+      // Arrange
+      final localEmails =
+          List.generate(5, (index) => Email(id: EmailId(Id('local_$index'))));
       when(threadDataSource.getAllEmailCache(
         any,
         any,
@@ -91,32 +114,273 @@ void main() {
         inMailboxId: anyNamed('inMailboxId'),
         limit: anyNamed('limit'),
         sort: anyNamed('sort'),
-      )).thenAnswer(
+      )).thenAnswer((_) => Future.value(localEmails));
-        (_) => Future.value(List.generate(30, (index) => Email(id: EmailId(Id('$index'))))),
+
+      when(stateDataSource.getState(any, any, any))
+        .thenAnswer((_) => Future.value(State('local_state')));
+
+      final networkEmails = List.generate(
+        ThreadConstants.defaultLimit.value as int,
+        (index) => Email(id: EmailId(Id('network_$index')))
       );
-      when(stateDataSource.getState(
+      when(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      )).thenAnswer((_) => Future.value(EmailsResponse(
+          emailList: networkEmails,
+          state: State('network_state'))));
+
+      // Act
+      final responses = await threadRepository
+        .getAllEmail(
+          SessionFixtures.aliceSession,
+          AccountFixtures.aliceAccountId,
+          getLatestChanges: false,
+        )
+        .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      expect(responses[0].emailList, networkEmails);
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: networkEmails,
+      ));
+      verifyNever(stateDataSource.saveState(
         any,
         any,
         any,
-      )).thenAnswer((_) => Future.value(State('some-state')));
+      ));
+    });
+
+    test('when local cache has sufficient emails '
+         'and no need getLatestChanges '
+         'should not fetch from network ',
+        () async {
+      // Arrange
+      final localEmails = List.generate(
+        ThreadConstants.defaultLimit.value as int,
+        (index) => Email(id: EmailId(Id('local_$index')))
+      );
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+        .thenAnswer((_) => Future.value(State('local_state')));
+
+      // Act
+      final responses = await threadRepository
+        .getAllEmail(
+          SessionFixtures.aliceSession,
+          AccountFixtures.aliceAccountId,
+          getLatestChanges: false
+        )
+        .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      expect(responses[0].emailList, localEmails);
+      verifyNever(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      ));
+      verifyNever(stateDataSource.saveState(
+        any,
+        any,
+        any,
+      ));
+    });
+
+    test('when local cache has insufficient emails '
+         'and no need getLatestChanges '
+         'should fetch from network '
+         'and state should not be saved',
+        () async {
+      // Arrange
+      final localEmails = List.generate(
+        5,
+        (index) => Email(id: EmailId(Id('local_$index')))
+      );
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+        .thenAnswer((_) => Future.value(State('local_state')));
+
+      final networkEmails = List.generate(
+        ThreadConstants.defaultLimit.value as int,
+        (index) => Email(id: EmailId(Id('network_$index')))
+      );
+      when(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      )).thenAnswer((_) => Future.value(EmailsResponse(
+          emailList: networkEmails,
+          state: State('network_state'))));
+
+      // Act
+      final responses = await threadRepository
+        .getAllEmail(
+          SessionFixtures.aliceSession,
+          AccountFixtures.aliceAccountId,
+          getLatestChanges: false
+        )
+        .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      expect(responses[0].emailList, networkEmails);
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: networkEmails,
+      ));
+      verifyNever(stateDataSource.saveState(
+        any,
+        any,
+        any,
+      ));
+    });
+
+    // why need to fetch first page if filter option is not "all"?
+    // try to cache all data of this folder, not miss any messages
+    test('when filter option is not "all" '
+         'and no need getLatestChanges '
+         'should fetch first page of all messages for this folder', () async {
+      // Arrange
+      final mailboxId = MailboxId(Id('mailbox_id'));
+      final localEmails = List.generate(
+        10,
+        (index) => Email(id: EmailId(Id('local_$index')))
+      );
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+        .thenAnswer((_) => Future.value(State('local_state')));
+
+      final networkEmails = List.generate(
+        ThreadConstants.defaultLimit.value as int,
+        (index) => Email(id: EmailId(Id('network_$index')))
+      );
+      when(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      )).thenAnswer((_) => Future.value(EmailsResponse(
+          emailList: networkEmails,
+          state: State('network_state'))));
+
+      // Act
+      final responses = await threadRepository
+        .getAllEmail(
+          SessionFixtures.aliceSession,
+          AccountFixtures.aliceAccountId,
+          emailFilter: EmailFilter(filterOption: FilterMessageOption.unread, mailboxId: mailboxId),
+          getLatestChanges: false,
+        )
+        .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      verify(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        filter: anyNamed('filter'),
+      )).called(2); // Once for initial fetch, once for first page
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: networkEmails,
+      )).called(2);
+      verifyNever(stateDataSource.saveState(
+        any,
+        any,
+        any,
+      ));
+    });
+
+    test('when local has mail in cache and getLatestChanges is true should synchronize cache', () async {
+      // Arrange
+      final localEmails = List.generate(
+        ThreadConstants.defaultLimit.value as int,
+        (index) => Email(id: EmailId(Id('local_$index'))));
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+          .thenAnswer((_) => Future.value(State('local_state')));
+
+      final changedEmails =
+          List.generate(5, (index) => Email(id: EmailId(Id('changed_$index'))));
       when(threadDataSource.getChanges(
         any,
         any,
         any,
         propertiesCreated: anyNamed('propertiesCreated'),
         propertiesUpdated: anyNamed('propertiesUpdated'),
-      )).thenAnswer(
+      )).thenAnswer((_) => Future.value(EmailChangeResponse(
-        (_) => Future.value(EmailChangeResponse(hasMoreChanges: false)),
+          hasMoreChanges: false,
-      );
+          created: changedEmails,
+          newStateEmail: State('new_state'))));
 
-      // act
+      // Act
-      await threadRepository.getAllEmail(
+      final responses = await threadRepository
-        SessionFixtures.aliceSession,
+          .getAllEmail(
-        AccountFixtures.aliceAccountId,
+            SessionFixtures.aliceSession,
-        getLatestChanges: true,
+            AccountFixtures.aliceAccountId,
-      ).last;
+            getLatestChanges: true,
+          )
+          .toList();
 
-      // assert
+      // Assert
+      expect(responses.length, 2);
+      verifyNever(threadDataSource.getAllEmail(any, any));
       verify(threadDataSource.getChanges(
         any,
         any,
@@ -124,6 +388,147 @@ void main() {
         propertiesCreated: anyNamed('propertiesCreated'),
         propertiesUpdated: anyNamed('propertiesUpdated'),
       ));
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: changedEmails,
+      ));
+      verify(stateDataSource.saveState(any, any, any));
+    });
+
+    test('when getChanges has more changes should fetch all changes', () async {
+      // Arrange
+      final localEmails = List.generate(
+          ThreadConstants.defaultLimit.value as int,
+          (index) => Email(id: EmailId(Id('local_$index'))));
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+          .thenAnswer((_) => Future.value(State('local_state')));
+
+      final firstChanges = List.generate(
+          5, (index) => Email(id: EmailId(Id('change1_$index'))));
+      final secondChanges = List.generate(
+          5, (index) => Email(id: EmailId(Id('change2_$index'))));
+
+      var callCount = 0;
+      when(threadDataSource.getChanges(
+        any,
+        any,
+        any,
+        propertiesCreated: anyNamed('propertiesCreated'),
+        propertiesUpdated: anyNamed('propertiesUpdated'),
+      )).thenAnswer((_) {
+        callCount++;
+        if (callCount == 1) {
+          return Future.value(EmailChangeResponse(
+            hasMoreChanges: true,
+            created: firstChanges,
+            newStateChanges: State('intermediate_state'),
+            newStateEmail: State('intermediate_state_email'),
+          ));
+        } else {
+          return Future.value(EmailChangeResponse(
+            hasMoreChanges: false,
+            created: secondChanges,
+            newStateChanges: State('final_state'),
+            newStateEmail: State('final_state_email'),
+          ));
+        }
+      });
+
+      // Act
+      final responses = await threadRepository
+          .getAllEmail(
+            SessionFixtures.aliceSession,
+            AccountFixtures.aliceAccountId,
+            getLatestChanges: true,
+          )
+          .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      verify(threadDataSource.getChanges(
+        any,
+        any,
+        any,
+        propertiesCreated: anyNamed('propertiesCreated'),
+        propertiesUpdated: anyNamed('propertiesUpdated'),
+      )).called(2);
+      verify(threadDataSource.update(
+        any,
+        any,
+        created: anyNamed('created'),
+      ));
+      verify(stateDataSource.saveState(any, any, any));
+    });
+
+    test('when local has mail in cache but network errors '
+         'should return email from local', () async {
+      // Arrange
+      final localEmails = List.generate(
+          ThreadConstants.defaultLimit.value as int,
+          (index) => Email(id: EmailId(Id('local_$index'))));
+      when(threadDataSource.getAllEmailCache(
+        any,
+        any,
+        filterOption: anyNamed('filterOption'),
+        inMailboxId: anyNamed('inMailboxId'),
+        limit: anyNamed('limit'),
+        sort: anyNamed('sort'),
+      )).thenAnswer((_) => Future.value(localEmails));
+
+      when(stateDataSource.getState(any, any, any))
+          .thenAnswer((_) => Future.value(State('local_state')));
+
+      when(threadDataSource.getAllEmail(
+        any,
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      )).thenThrow(Exception('Network error'));
+
+      // Act
+      final responses = await threadRepository
+          .getAllEmail(
+            SessionFixtures.aliceSession,
+            AccountFixtures.aliceAccountId,
+            getLatestChanges: false,
+          )
+          .toList();
+
+      // Assert
+      expect(responses.length, 2);
+      expect(responses[0].emailList, localEmails);
+      verifyNever(threadDataSource.getAllEmail(
+        any, 
+        any,
+        limit: anyNamed('limit'),
+        position: anyNamed('position'),
+        sort: anyNamed('sort'),
+        filter: anyNamed('filter'),
+        properties: anyNamed('properties'),
+      ));
+      verifyNever(threadDataSource.update(
+        any,
+        any,
+        created: anyNamed('created'),
+      ));
+      verifyNever(stateDataSource.saveState(
+        any,
+        any,
+        any,
+      ));
     });
   });
 }