From 7d0d7a0c2add7c0cff7b9995b9dcca586c6ed7e5 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Fri, 13 Dec 2019 20:40:08 +0200 Subject: [PATCH 1/4] Pass last_fetch_time to fetch-sync-records (Initial); fixes brave-browser#7327 --- .../extensions/api/brave_sync_event_router.cc | 12 +++++---- .../extensions/api/brave_sync_event_router.h | 8 +++--- common/extensions/api/brave_sync.json | 4 +++ .../brave_profile_sync_service_impl.cc | 26 ++++++++++++++++--- components/brave_sync/brave_sync_prefs.cc | 12 +++++++++ components/brave_sync/brave_sync_prefs.h | 6 ++++- .../brave_sync/client/brave_sync_client.h | 7 ++--- .../client/brave_sync_client_impl.cc | 9 ++++--- .../client/brave_sync_client_impl.h | 9 ++++--- components/brave_sync/extension/background.js | 6 ++--- 10 files changed, 72 insertions(+), 27 deletions(-) diff --git a/browser/extensions/api/brave_sync_event_router.cc b/browser/extensions/api/brave_sync_event_router.cc index da2579f12ffe..47dba8b949fd 100644 --- a/browser/extensions/api/brave_sync_event_router.cc +++ b/browser/extensions/api/brave_sync_event_router.cc @@ -44,12 +44,14 @@ void BraveSyncEventRouter::GotInitData( void BraveSyncEventRouter::FetchSyncRecords( const std::vector& category_names, - const base::Time& startAt, - const int max_records) { + const base::Time& start_at, + const int max_records, + const base::Time& previous_fetch_time) { std::unique_ptr args( - extensions::api::brave_sync::OnFetchSyncRecords::Create(category_names, - startAt.ToJsTime(), static_cast(max_records)) - .release()); + extensions::api::brave_sync::OnFetchSyncRecords::Create( + category_names, start_at.ToJsTime(), static_cast(max_records), + previous_fetch_time.ToJsTime()) + .release()); std::unique_ptr event( new Event(extensions::events::FOR_TEST, extensions::api::brave_sync::OnFetchSyncRecords::kEventName, diff --git a/browser/extensions/api/brave_sync_event_router.h b/browser/extensions/api/brave_sync_event_router.h index 9ab409ee8dd8..c8736cbe1298 100644 --- a/browser/extensions/api/brave_sync_event_router.h +++ b/browser/extensions/api/brave_sync_event_router.h @@ -40,10 +40,10 @@ class BraveSyncEventRouter { const extensions::api::brave_sync::Config& config, const std::string& device_id_v2); - void FetchSyncRecords( - const std::vector& category_names, - const base::Time& startAt, - const int max_records); + void FetchSyncRecords(const std::vector& category_names, + const base::Time& start_at, + const int max_records, + const base::Time& previous_fetch_time); void ResolveSyncRecords(const std::string &category_name, const std::vector& records_and_existing_objects); diff --git a/common/extensions/api/brave_sync.json b/common/extensions/api/brave_sync.json index 9a582f180c6d..d84066603ef4 100644 --- a/common/extensions/api/brave_sync.json +++ b/common/extensions/api/brave_sync.json @@ -225,6 +225,10 @@ { "type": "number", "name": "maxRecords" + }, + { + "type": "number", + "name": "previousFetchTime" } ] }, diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index 96770c1350e8..e6e8e336094a 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -846,6 +846,13 @@ void BraveProfileSyncServiceImpl::FetchSyncRecords(const bool bookmarks, return; } + // With current logic and separate SQS queues we can do only fetch per + // category + // TODO(AlexeyBarabash): split this function into a three + DCHECK((bookmarks && !history && !preferences) || + (!bookmarks && history && !preferences) || + (!bookmarks && !history && preferences)); + std::vector category_names; if (history) { category_names.push_back(kHistorySites); // "HISTORY_SITES"; @@ -868,8 +875,16 @@ void BraveProfileSyncServiceImpl::FetchSyncRecords(const bool bookmarks, base::Time start_at_time = IsSQSReady() ? brave_sync_prefs_->GetLatestRecordTime() : base::Time(); + base::Time last_fetch_time; + if (bookmarks) { + last_fetch_time = brave_sync_prefs_->GetLastFetchTime(); + brave_sync_prefs_->SetLastFetchTime(base::Time::Now()); + } else if (preferences) { + last_fetch_time = brave_sync_prefs_->GetLastPreferencesFetchTime(); + } + brave_sync_client_->SendFetchSyncRecords(category_names, start_at_time, - max_records); + max_records, last_fetch_time); } void BraveProfileSyncServiceImpl::SendCreateDevice() { @@ -985,14 +1000,17 @@ bool BraveProfileSyncServiceImpl::IsBraveSyncEnabled() const { void BraveProfileSyncServiceImpl::FetchDevices() { DCHECK(sync_client_); - brave_sync_prefs_->SetLastFetchTime(base::Time::Now()); + + const auto last_fetch_time = brave_sync_prefs_->GetLastPreferencesFetchTime(); + brave_sync_prefs_->SetLastPreferencesFetchTime(base::Time::Now()); base::Time start_at_time = IsSQSReady() ? brave_sync_prefs_->GetLatestDeviceRecordTime() : base::Time(); brave_sync_client_->SendFetchSyncRecords( - {brave_sync::jslib_const::kPreferences}, start_at_time, 1000); + {brave_sync::jslib_const::kPreferences}, start_at_time, 1000, + last_fetch_time); } void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb, @@ -1000,7 +1018,7 @@ void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb, if (!brave_sync_prefs_->GetSyncEnabled()) return; - if (IsTimeEmpty(brave_sync_prefs_->GetLastFetchTime())) { + if (IsTimeEmpty(brave_sync_prefs_->GetLastPreferencesFetchTime())) { SendCreateDevice(); this_device_created_time_ = base::Time::Now(); } diff --git a/components/brave_sync/brave_sync_prefs.cc b/components/brave_sync/brave_sync_prefs.cc index 786b07361c1e..1922ef1d1f4b 100644 --- a/components/brave_sync/brave_sync_prefs.cc +++ b/components/brave_sync/brave_sync_prefs.cc @@ -35,6 +35,7 @@ const char kSyncLatestRecordTime[] = "brave_sync.latest_record_time"; const char kSyncLatestDeviceRecordTime[] = "brave_sync.latest_device_record_time"; const char kSyncLastFetchTime[] = "brave_sync.last_fetch_time"; +const char kSyncLastPreferencesFetchTime[] = "brave_sync.last_prefs_fetch_time"; const char kSyncLastCompactTimeBookmarks[] = "brave_sync.last_compact_time.bookmarks"; const char kSyncDeviceList[] = "brave_sync.device_list"; @@ -63,6 +64,8 @@ void Prefs::RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry) { registry->RegisterTimePref(prefs::kSyncLatestRecordTime, base::Time()); registry->RegisterTimePref(prefs::kSyncLatestDeviceRecordTime, base::Time()); registry->RegisterTimePref(prefs::kSyncLastFetchTime, base::Time()); + registry->RegisterTimePref(prefs::kSyncLastPreferencesFetchTime, + base::Time()); registry->RegisterTimePref(prefs::kSyncLastCompactTimeBookmarks, base::Time()); @@ -197,6 +200,14 @@ base::Time Prefs::GetLastFetchTime() { return pref_service_->GetTime(kSyncLastFetchTime); } +void Prefs::SetLastPreferencesFetchTime(const base::Time& time) { + pref_service_->SetTime(kSyncLastPreferencesFetchTime, time); +} + +base::Time Prefs::GetLastPreferencesFetchTime() { + return pref_service_->GetTime(kSyncLastPreferencesFetchTime); +} + void Prefs::SetLastCompactTimeBookmarks(const base::Time &time) { pref_service_->SetTime(kSyncLastCompactTimeBookmarks, time); } @@ -289,6 +300,7 @@ void Prefs::Clear() { pref_service_->ClearPref(kSyncLatestRecordTime); pref_service_->ClearPref(kSyncLatestDeviceRecordTime); pref_service_->ClearPref(kSyncLastFetchTime); + pref_service_->ClearPref(kSyncLastPreferencesFetchTime); pref_service_->ClearPref(kSyncDeviceList); pref_service_->ClearPref(kSyncApiVersion); pref_service_->ClearPref(kSyncMigrateBookmarksVersion); diff --git a/components/brave_sync/brave_sync_prefs.h b/components/brave_sync/brave_sync_prefs.h index f3d2ac869a81..6ff748115f30 100644 --- a/components/brave_sync/brave_sync_prefs.h +++ b/components/brave_sync/brave_sync_prefs.h @@ -58,8 +58,10 @@ extern const char kSyncHistoryEnabled[]; extern const char kSyncLatestRecordTime[]; // The latest time of synced device record extern const char kSyncLatestDeviceRecordTime[]; -// The time of latest fetch records operation +// The time of latest fetch records operation for bookmarks extern const char kSyncLastFetchTime[]; +// The time of latest fetch records operation for preferences +extern const char kSyncLastPreferencesFetchTime[]; // the list of all known sync devices // TODO(bridiver) - this should be a dictionary - not raw json extern const char kSyncDeviceList[]; @@ -107,6 +109,8 @@ class Prefs { base::Time GetLatestDeviceRecordTime(); void SetLastFetchTime(const base::Time &time); base::Time GetLastFetchTime(); + void SetLastPreferencesFetchTime(const base::Time& time); + base::Time GetLastPreferencesFetchTime(); void SetLastCompactTimeBookmarks(const base::Time &time); base::Time GetLastCompactTimeBookmarks(); diff --git a/components/brave_sync/client/brave_sync_client.h b/components/brave_sync/client/brave_sync_client.h index 1b8fdc034836..4d9f49c0ae11 100644 --- a/components/brave_sync/client/brave_sync_client.h +++ b/components/brave_sync/client/brave_sync_client.h @@ -72,9 +72,10 @@ class BraveSyncClient { const client_data::Config& config, const std::string& device_id_v2) = 0; virtual void SendFetchSyncRecords( - const std::vector &category_names, - const base::Time &startAt, - const int max_records) = 0; + const std::vector& category_names, + const base::Time& startAt, + const int max_records, + const base::Time& last_fetch_time) = 0; virtual void SendResolveSyncRecords( const std::string &category_name, std::unique_ptr list) = 0; diff --git a/components/brave_sync/client/brave_sync_client_impl.cc b/components/brave_sync/client/brave_sync_client_impl.cc index 0f5700dfb013..4e1ffb8241e1 100644 --- a/components/brave_sync/client/brave_sync_client_impl.cc +++ b/components/brave_sync/client/brave_sync_client_impl.cc @@ -75,11 +75,12 @@ void BraveSyncClientImpl::SendGotInitData(const Uint8Array& seed, void BraveSyncClientImpl::SendFetchSyncRecords( const std::vector& category_names, - const base::Time& startAt, - const int max_records) { + const base::Time& start_at, + const int max_records, + const base::Time& last_fetch_time) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); - brave_sync_event_router_->FetchSyncRecords(category_names, startAt, - max_records); + brave_sync_event_router_->FetchSyncRecords(category_names, start_at, + max_records, last_fetch_time); } void BraveSyncClientImpl::SendResolveSyncRecords( diff --git a/components/brave_sync/client/brave_sync_client_impl.h b/components/brave_sync/client/brave_sync_client_impl.h index 0222e1837197..a5b4e0525b14 100644 --- a/components/brave_sync/client/brave_sync_client_impl.h +++ b/components/brave_sync/client/brave_sync_client_impl.h @@ -49,9 +49,12 @@ class BraveSyncClientImpl : public BraveSyncClient, const Uint8Array& device_id, const client_data::Config& config, const std::string& device_id_v2) override; - void SendFetchSyncRecords( - const std::vector &category_names, const base::Time &startAt, - const int max_records) override; + + void SendFetchSyncRecords(const std::vector& category_names, + const base::Time& start_at, + const int max_records, + const base::Time& last_fetch_time) override; + void SendResolveSyncRecords( const std::string& category_name, std::unique_ptr records) override; diff --git a/components/brave_sync/extension/background.js b/components/brave_sync/extension/background.js index db30bca74ac0..5b8f7750fdbe 100644 --- a/components/brave_sync/extension/background.js +++ b/components/brave_sync/extension/background.js @@ -8,9 +8,9 @@ chrome.braveSync.onGotInitData.addListener(function(seed, device_id, config, dev callbackList["got-init-data"](null, seed, device_id, config, device_id_v2); }); -chrome.braveSync.onFetchSyncRecords.addListener(function(category_names, start_at, max_records) { - console.log(`"fetch-sync-records" category_names=${JSON.stringify(category_names)} start_at=${JSON.stringify(start_at)} max_records=${JSON.stringify(max_records)}`); - callbackList["fetch-sync-records"](null, category_names, start_at, max_records); +chrome.braveSync.onFetchSyncRecords.addListener(function(category_names, start_at, max_records, previous_fetch_time) { + console.log(`"fetch-sync-records" category_names=${JSON.stringify(category_names)} start_at=${JSON.stringify(start_at)} previous_fetch_time=${JSON.stringify(previous_fetch_time)} max_records=${JSON.stringify(max_records)}`); + callbackList["fetch-sync-records"](null, category_names, start_at, max_records, previous_fetch_time); }); chrome.braveSync.onResolveSyncRecords.addListener(function(category_name, recordsAndExistingObjects) { From 37c047998b17aa39164e3da23c220cc0a1b1b811 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Tue, 24 Dec 2019 16:38:05 +0200 Subject: [PATCH 2/4] split FetchSyncRecords --- .../brave_profile_sync_service_impl.cc | 77 +++++++++---------- .../brave_profile_sync_service_impl.h | 9 ++- 2 files changed, 41 insertions(+), 45 deletions(-) diff --git a/components/brave_sync/brave_profile_sync_service_impl.cc b/components/brave_sync/brave_profile_sync_service_impl.cc index e6e8e336094a..841553e68e67 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.cc +++ b/components/brave_sync/brave_profile_sync_service_impl.cc @@ -836,54 +836,38 @@ bool BraveProfileSyncServiceImpl::IsSQSReady() const { } } -void BraveProfileSyncServiceImpl::FetchSyncRecords(const bool bookmarks, - const bool history, - const bool preferences, - int max_records) { +void BraveProfileSyncServiceImpl::FetchBookmarks(int max_records) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(bookmarks || history || preferences); - if (!(bookmarks || history || preferences)) { - return; - } - // With current logic and separate SQS queues we can do only fetch per - // category - // TODO(AlexeyBarabash): split this function into a three - DCHECK((bookmarks && !history && !preferences) || - (!bookmarks && history && !preferences) || - (!bookmarks && !history && preferences)); - - std::vector category_names; - if (history) { - category_names.push_back(kHistorySites); // "HISTORY_SITES"; - } - if (bookmarks) { - category_names.push_back(kBookmarks); // "BOOKMARKS"; - - base::Time last_compact_time = - brave_sync_prefs_->GetLastCompactTimeBookmarks(); - if (tools::IsTimeEmpty(last_compact_time) || - base::Time::Now() - last_compact_time > - base::TimeDelta::FromDays(kCompactPeriodInDays)) { - brave_sync_client_->SendCompact(kBookmarks); - } - } - if (preferences) { - category_names.push_back(kPreferences); // "PREFERENCES"; + base::Time last_compact_time = + brave_sync_prefs_->GetLastCompactTimeBookmarks(); + if (tools::IsTimeEmpty(last_compact_time) || + base::Time::Now() - last_compact_time > + base::TimeDelta::FromDays(kCompactPeriodInDays)) { + brave_sync_client_->SendCompact(kBookmarks); } base::Time start_at_time = IsSQSReady() ? brave_sync_prefs_->GetLatestRecordTime() : base::Time(); - base::Time last_fetch_time; - if (bookmarks) { - last_fetch_time = brave_sync_prefs_->GetLastFetchTime(); - brave_sync_prefs_->SetLastFetchTime(base::Time::Now()); - } else if (preferences) { - last_fetch_time = brave_sync_prefs_->GetLastPreferencesFetchTime(); - } + base::Time last_fetch_time = brave_sync_prefs_->GetLastFetchTime(); + brave_sync_prefs_->SetLastFetchTime(base::Time::Now()); - brave_sync_client_->SendFetchSyncRecords(category_names, start_at_time, + brave_sync_client_->SendFetchSyncRecords({kBookmarks}, start_at_time, + max_records, last_fetch_time); +} + +void BraveProfileSyncServiceImpl::FetchHistory(int max_records) { + NOTIMPLEMENTED(); +} + +void BraveProfileSyncServiceImpl::FetchPreferences(int max_records) { + // For now "PREFERENCES" category stores only devices list + base::Time start_at_time = + IsSQSReady() ? brave_sync_prefs_->GetLatestDeviceRecordTime() + : base::Time(); + base::Time last_fetch_time = brave_sync_prefs_->GetLastPreferencesFetchTime(); + brave_sync_client_->SendFetchSyncRecords({kPreferences}, start_at_time, max_records, last_fetch_time); } @@ -1043,9 +1027,20 @@ void BraveProfileSyncServiceImpl::OnPollSyncCycle(GetRecordsCallback cb, wevent_ = wevent; const bool bookmarks = brave_sync_prefs_->GetSyncBookmarksEnabled(); + if (bookmarks) { + FetchBookmarks(1000); + } + const bool history = brave_sync_prefs_->GetSyncHistoryEnabled(); + if (history) { + FetchHistory(1000); + } + const bool preferences = brave_sync_prefs_->GetSyncSiteSettingsEnabled(); - FetchSyncRecords(bookmarks, history, preferences, 1000); + if (preferences) { + FetchPreferences(1000); + } + ResendSyncRecords(jslib_const::SyncRecordType_BOOKMARKS); } diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index 0c4b87228f83..1ca1400437b8 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -172,10 +172,11 @@ class BraveProfileSyncServiceImpl friend class ::BraveSyncServiceTest; void SignalWaitableEvent(); - void FetchSyncRecords(const bool bookmarks, - const bool history, - const bool preferences, - int max_records); + + void FetchBookmarks(int max_records); + void FetchHistory(int max_records); + void FetchPreferences(int max_records); + void FetchDevices(); void SendCreateDevice(); void SendDeleteDevice(); From b5cba61a6f18fa9518198173cacaf252c959f0a6 Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Thu, 26 Dec 2019 17:31:00 +0200 Subject: [PATCH 3/4] Tests for separate last fetch time per category --- .../brave_profile_sync_service_impl.h | 4 + .../brave_sync/brave_sync_service_unittest.cc | 81 ++++++++++++++----- components/brave_sync/test_util.h | 8 +- 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/components/brave_sync/brave_profile_sync_service_impl.h b/components/brave_sync/brave_profile_sync_service_impl.h index 1ca1400437b8..a69de24fad9d 100644 --- a/components/brave_sync/brave_profile_sync_service_impl.h +++ b/components/brave_sync/brave_profile_sync_service_impl.h @@ -49,6 +49,8 @@ FORWARD_DECLARE_TEST(BraveSyncServiceTest, SetThisDeviceCreatedTime); FORWARD_DECLARE_TEST(BraveSyncServiceTest, InitialFetchesStartWithZero); FORWARD_DECLARE_TEST(BraveSyncServiceTest, DeviceIdV2Migration); FORWARD_DECLARE_TEST(BraveSyncServiceTest, DeviceIdV2MigrationDupDeviceId); +FORWARD_DECLARE_TEST(BraveSyncServiceTest, LastFetchBookmarksTime); +FORWARD_DECLARE_TEST(BraveSyncServiceTest, LastPreferencesFetchTime); class BraveSyncServiceTest; @@ -168,6 +170,8 @@ class BraveProfileSyncServiceImpl FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, DeviceIdV2Migration); FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, DeviceIdV2MigrationDupDeviceId); + FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, LastFetchBookmarksTime); + FRIEND_TEST_ALL_PREFIXES(::BraveSyncServiceTest, LastPreferencesFetchTime); friend class ::BraveSyncServiceTest; diff --git a/components/brave_sync/brave_sync_service_unittest.cc b/components/brave_sync/brave_sync_service_unittest.cc index e506916a9f4c..07e962be897e 100644 --- a/components/brave_sync/brave_sync_service_unittest.cc +++ b/components/brave_sync/brave_sync_service_unittest.cc @@ -439,7 +439,8 @@ TEST_F(BraveSyncServiceTest, OnDeleteDevice) { ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device3", "beef03"))) .Times(1); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _)); + + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _, _)); sync_service()->OnDeleteDevice("beef03"); RecordsList resolved_records; @@ -486,7 +487,7 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenOneDevice) { EXPECT_TRUE(DevicesContains(devices.get(), "2", "beef02", "device2")); EXPECT_CALL(*sync_client(), SendSyncRecords).Times(1); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _)); + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _, _)); sync_service()->OnDeleteDevice("beef02"); RecordsList resolved_records; @@ -541,7 +542,8 @@ TEST_F(BraveSyncServiceTest, OnDeleteDeviceWhenSelfDeleted) { ContainsDeviceRecord(SyncRecord::Action::A_DELETE, "device1", "beef01"))) .Times(1); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _)); + + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, _, _, _)); sync_service()->OnDeleteDevice("beef01"); // Enabled->Disabled @@ -1029,7 +1031,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { // Expecting SendFetchSyncRecords will be invoked // after sync_service()->OnPollSyncCycle - EXPECT_EQ(brave_sync_prefs()->GetLastFetchTime(), base::Time()); + EXPECT_EQ(brave_sync_prefs()->GetLastPreferencesFetchTime(), base::Time()); EXPECT_EQ(brave_sync_prefs()->GetLatestDeviceRecordTime(), base::Time()); using brave_sync::tools::IsTimeEmpty; @@ -1039,7 +1041,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { base::BindOnce(&OnGetRecordsStub); base::WaitableEvent we; EXPECT_CALL(*sync_client(), SendSyncRecords("PREFERENCES", _)); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _)); + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _, _)); sync_service()->OnPollSyncCycle(std::move(on_get_records), &we); EXPECT_FALSE(IsTimeEmpty(sync_service()->this_device_created_time_)); @@ -1053,7 +1055,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { sync_service()->OnGetExistingObjects(kPreferences, std::move(records), device1_timestamp, false); - EXPECT_NE(brave_sync_prefs()->GetLastFetchTime(), base::Time()); + EXPECT_NE(brave_sync_prefs()->GetLastPreferencesFetchTime(), base::Time()); EXPECT_EQ(brave_sync_prefs()->GetLatestDeviceRecordTime(), device1_timestamp); // We have moved records into OnGetExistingObjects, so fill again @@ -1069,7 +1071,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { // Less than 70 seconds have passed, we should fetch with empty start_at { auto time_override = OverrideForTimeDelta(base::TimeDelta::FromSeconds(65)); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _)) + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _, _)) .Times(1); sync_service()->FetchDevices(); } @@ -1078,7 +1080,7 @@ TEST_F(BraveSyncServiceTest, GetDevicesWithFetchSyncRecords) { { auto time_override = OverrideForTimeDelta(base::TimeDelta::FromSeconds(75)); EXPECT_CALL(*sync_client(), - SendFetchSyncRecords(_, base::Time(device1_timestamp), _)) + SendFetchSyncRecords(_, base::Time(device1_timestamp), _, _)) .Times(1); sync_service()->FetchDevices(); } @@ -1089,26 +1091,26 @@ TEST_F(BraveSyncServiceTest, SendCompact) { EXPECT_EQ(brave_sync_prefs()->GetLastCompactTimeBookmarks(), base::Time()); EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1); - sync_service()->FetchSyncRecords(true, false, true, 1000); + sync_service()->FetchBookmarks(1000); // timestamp is not writtend until we get CompactedSyncCategory EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1); - sync_service()->FetchSyncRecords(true, false, true, 1000); + sync_service()->FetchBookmarks(1000); sync_service()->OnCompactComplete(kBookmarks); EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(0); EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1); - sync_service()->FetchSyncRecords(true, false, true, 1000); + sync_service()->FetchBookmarks(1000); { auto time_override = OverrideForTimeDelta(base::TimeDelta::FromDays( sync_service()->GetCompactPeriodInDaysForTests())); EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1); - sync_service()->FetchSyncRecords(true, false, true, 1000); + sync_service()->FetchBookmarks(1000); sync_service()->OnCompactComplete(kBookmarks); } EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(0); EXPECT_CALL(*sync_client(), SendFetchSyncRecords).Times(1); - sync_service()->FetchSyncRecords(true, false, true, 1000); + sync_service()->FetchBookmarks(1000); } TEST_F(BraveSyncServiceTest, MigratePrevSeed) { @@ -1121,18 +1123,18 @@ TEST_F(BraveSyncServiceTest, MigratePrevSeed) { TEST_F(BraveSyncServiceTest, InitialFetchesStartWithZero) { using brave_sync::jslib_const::kBookmarks; EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _)) + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _, _)) .Times(1); - sync_service()->FetchSyncRecords(true, false, false, 1000); + sync_service()->FetchBookmarks(1000); const auto latest_bookmark_record_time = base::Time::Now(); brave_sync_prefs()->SetLatestRecordTime(latest_bookmark_record_time); sync_service()->this_device_created_time_ = base::Time::Now(); EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); - EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _)) + EXPECT_CALL(*sync_client(), SendFetchSyncRecords(_, base::Time(), _, _)) .Times(1); - sync_service()->FetchSyncRecords(true, false, false, 1000); + sync_service()->FetchBookmarks(1000); sync_service()->OnCompactComplete(kBookmarks); // Prior 70 sec we should use null start_at and after we should use @@ -1140,9 +1142,9 @@ TEST_F(BraveSyncServiceTest, InitialFetchesStartWithZero) { { auto time_override = OverrideForTimeDelta(base::TimeDelta::FromSeconds(71)); EXPECT_CALL(*sync_client(), - SendFetchSyncRecords(_, latest_bookmark_record_time, _)) + SendFetchSyncRecords(_, latest_bookmark_record_time, _, _)) .Times(1); - sync_service()->FetchSyncRecords(true, false, false, 1000); + sync_service()->FetchBookmarks(1000); } } @@ -1171,7 +1173,7 @@ TEST_F(BraveSyncServiceTest, DeviceIdV2Migration) { SimpleDeviceRecord(SyncRecord::Action::A_CREATE, "", "1", "", "device1")); sync_service()->OnResolvedPreferences(records); - brave_sync_prefs()->SetLastFetchTime(base::Time::Now()); + brave_sync_prefs()->SetLastPreferencesFetchTime(base::Time::Now()); auto devices = brave_sync_prefs()->GetSyncDevices(); @@ -1228,7 +1230,7 @@ TEST_F(BraveSyncServiceTest, DeviceIdV2MigrationDupDeviceId) { SimpleDeviceRecord(SyncRecord::Action::A_CREATE, "", "1", "", "device2")); sync_service()->OnResolvedPreferences(records); - brave_sync_prefs()->SetLastFetchTime(base::Time::Now()); + brave_sync_prefs()->SetLastPreferencesFetchTime(base::Time::Now()); auto devices = brave_sync_prefs()->GetSyncDevices(); @@ -1259,3 +1261,40 @@ TEST_F(BraveSyncServiceTest, DeviceIdV2MigrationDupDeviceId) { base::BindOnce(&OnGetRecordsStub); sync_service()->OnPollSyncCycle(std::move(on_get_records), &we); } + +TEST_F(BraveSyncServiceTest, LastFetchBookmarksTime) { + using brave_sync::jslib_const::kBookmarks; + + EXPECT_EQ(brave_sync_prefs()->GetLastFetchTime(), base::Time()); + + EXPECT_CALL(*sync_client(), SendCompact(kBookmarks)).Times(1); + EXPECT_CALL(*sync_client(), + SendFetchSyncRecords(_, base::Time(), _, base::Time())) + .Times(1); + sync_service()->FetchBookmarks(1000); + sync_service()->OnCompactComplete(kBookmarks); + + auto first_fetch_time = brave_sync_prefs()->GetLastFetchTime(); + EXPECT_NE(first_fetch_time, base::Time()); + + EXPECT_CALL(*sync_client(), + SendFetchSyncRecords(_, base::Time(), _, first_fetch_time)) + .Times(1); + sync_service()->FetchBookmarks(1000); +} + +TEST_F(BraveSyncServiceTest, LastPreferencesFetchTime) { + EXPECT_EQ(brave_sync_prefs()->GetLastPreferencesFetchTime(), base::Time()); + + EXPECT_CALL(*sync_client(), + SendFetchSyncRecords(_, base::Time(), _, base::Time())); + + sync_service()->FetchDevices(); + + auto first_fetch_time = brave_sync_prefs()->GetLastPreferencesFetchTime(); + EXPECT_NE(first_fetch_time, base::Time()); + + EXPECT_CALL(*sync_client(), + SendFetchSyncRecords(_, base::Time(), _, first_fetch_time)); + sync_service()->FetchDevices(); +} diff --git a/components/brave_sync/test_util.h b/components/brave_sync/test_util.h index eb75bf762977..d3f6293e5aa1 100644 --- a/components/brave_sync/test_util.h +++ b/components/brave_sync/test_util.h @@ -40,9 +40,11 @@ class MockBraveSyncClient : public BraveSyncClient { const Uint8Array& device_id, const client_data::Config& config, const std::string& device_id_v2)); - MOCK_METHOD3(SendFetchSyncRecords, void( - const std::vector& category_names, const base::Time& startAt, - const int max_records)); + MOCK_METHOD4(SendFetchSyncRecords, + void(const std::vector& category_names, + const base::Time& startAt, + const int max_records, + const base::Time& last_fetch_time)); MOCK_METHOD2(SendResolveSyncRecords, void(const std::string& category_name, std::unique_ptr list)); MOCK_METHOD2(SendSyncRecords, From 320508350880c6209cd63ee067716a0e46877d9d Mon Sep 17 00:00:00 2001 From: AlexeyBarabash Date: Mon, 30 Dec 2019 14:13:06 +0200 Subject: [PATCH 4/4] Updated DEPS of brave sync lib (points to staging) --- DEPS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEPS b/DEPS index ba7a4fb4dc25..c05df645712e 100644 --- a/DEPS +++ b/DEPS @@ -15,7 +15,7 @@ deps = { "vendor/bip39wally-core-native": "https://github.com/brave-intl/bip39wally-core-native.git@13bb40a215248cfbdd87d0a6b425c8397402e9e6", "vendor/bat-native-anonize": "https://github.com/brave-intl/bat-native-anonize.git@e3742ba3e8942eea9e4755d91532491871bd3116", "vendor/bat-native-tweetnacl": "https://github.com/brave-intl/bat-native-tweetnacl.git@800f9d40b7409239ff192e0be634764e747c7a75", - "components/brave_sync/extension/brave-sync": "https://github.com/brave/sync.git@bde4bf8dcd56e41cc064d613219dc3382c178856", + "components/brave_sync/extension/brave-sync": "https://github.com/brave/sync.git@e36acfa8cbff5ae283407f6fcc6b8916351fd8e0", "vendor/bat-native-usermodel": "https://github.com/brave-intl/bat-native-usermodel.git@45e32155af9897dbe1d5534dd36697ec4728bb75", "vendor/challenge_bypass_ristretto_ffi": "https://github.com/brave-intl/challenge-bypass-ristretto-ffi.git@c396fb4eb9e9bf63b89ae5a0ec0b5f201d43c7c5", }