From 262cf278ba122a3476fc76e32f0eddbfbed31571 Mon Sep 17 00:00:00 2001 From: Dat PHAM HOANG Date: Thu, 9 Jan 2025 17:50:25 +0700 Subject: [PATCH] Fix cache synchronizing side effect when open folder --- .../repository/thread_repository_impl.dart | 18 +- .../thread_repository_impl_test.dart | 539 +++++++++++++++--- 2 files changed, 482 insertions(+), 75 deletions(-) diff --git a/lib/features/thread/data/repository/thread_repository_impl.dart b/lib/features/thread/data/repository/thread_repository_impl.dart index 9b4c5d94d..72aea4364 100644 --- a/lib/features/thread/data/repository/thread_repository_impl.dart +++ b/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( - session, - accountId, - localEmailResponse.state!, - propertiesCreated: propertiesCreated, - propertiesUpdated: propertiesUpdated - ); + if (getLatestChanges) { + await _synchronizeCacheWithChanges( + session, + accountId, + localEmailResponse.state!, + propertiesCreated: propertiesCreated, + propertiesUpdated: propertiesUpdated + ); + } } else { if (networkEmailResponse != null) { log('ThreadRepositoryImpl::getAllEmail(): filter = ${emailFilter?.mailboxId} no local state -> update from network: ${networkEmailResponse.state}'); diff --git a/test/features/thread/data/repository/thread_repository_impl_test.dart b/test/features/thread/data/repository/thread_repository_impl_test.dart index 2ed0fd538..10e0435f2 100644 --- a/test/features/thread/data/repository/thread_repository_impl_test.dart +++ b/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(), ]) void main() { - final threadDataSource = MockThreadDataSource(); - final stateDataSource = MockStateDataSource(); - final threadRepository = ThreadRepositoryImpl( - { - DataSourceType.network: threadDataSource, - DataSourceType.local: threadDataSource, - }, - stateDataSource, - ); + late MockThreadDataSource threadDataSource; + late MockStateDataSource stateDataSource; + late ThreadRepositoryImpl threadRepository; - group('thread repository impl test:', () { - test( - 'should not call threadDatasource.getChanges ' - 'when getAllEmail is called ' - 'and getLatestChanges is false', - () async { - // arrange + setUp(() { + threadDataSource = MockThreadDataSource(); + stateDataSource = MockStateDataSource(); + threadRepository = ThreadRepositoryImpl( + { + DataSourceType.network: threadDataSource, + 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( - (_) => Future.value(List.generate(30, (index) => Email(id: EmailId(Id('$index'))))), + )).thenAnswer((_) => Future.value([])); + + when(stateDataSource.getState(any, any, any)) + .thenAnswer((_) => Future.value(null)); + + final networkEmails = List.generate( + 20, + (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, + ) + .toList(); + + // Assert + expect(responses.length, 2); + expect(responses[0].emailList, networkEmails); + verify(threadDataSource.update( + any, + any, + created: networkEmails, + )); + verify(stateDataSource.saveState( 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 - await threadRepository.getAllEmail( - SessionFixtures.aliceSession, - AccountFixtures.aliceAccountId, - getLatestChanges: false, - ).last; - - // assert - verifyNever(threadDataSource.getChanges( - any, - any, - any, - propertiesCreated: anyNamed('propertiesCreated'), - propertiesUpdated: anyNamed('propertiesUpdated'), )); }); - test( - 'should call threadDatasource.getChanges ' - 'when getAllEmail is called ' - 'and getLatestChanges is true', - () async { - // arrange + test('when local cache has fewer than default limit 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, @@ -91,32 +114,273 @@ void main() { inMailboxId: anyNamed('inMailboxId'), limit: anyNamed('limit'), sort: anyNamed('sort'), - )).thenAnswer( - (_) => Future.value(List.generate(30, (index) => Email(id: EmailId(Id('$index'))))), + )).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(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( - (_) => Future.value(EmailChangeResponse(hasMoreChanges: false)), - ); - - // act - await threadRepository.getAllEmail( - SessionFixtures.aliceSession, - AccountFixtures.aliceAccountId, - getLatestChanges: true, - ).last; - - // assert + )).thenAnswer((_) => Future.value(EmailChangeResponse( + hasMoreChanges: false, + created: changedEmails, + newStateEmail: State('new_state')))); + + // Act + final responses = await threadRepository + .getAllEmail( + SessionFixtures.aliceSession, + AccountFixtures.aliceAccountId, + getLatestChanges: true, + ) + .toList(); + + // 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, + )); }); }); -} \ No newline at end of file +}