[Sync] Adding storage key concept for ModelTypeServices.

components/sync_driver/device_info_service_unittest.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/sync_driver/device_info_service.h"
 
 #include <map>
 #include <memory>
 #include <set>
 #include <string>
@@ skipping 26 matching lines @@
 using syncer_v2::EntityChangeList;
 using syncer_v2::EntityData;
 using syncer_v2::EntityDataMap;
 using syncer_v2::EntityDataPtr;
 using syncer_v2::MetadataBatch;
 using syncer_v2::MetadataChangeList;
 using syncer_v2::ModelTypeChangeProcessor;
 using syncer_v2::ModelTypeService;
 using syncer_v2::ModelTypeStore;
 using syncer_v2::ModelTypeStoreTestUtil;
-using syncer_v2::TagAndData;
+using syncer_v2::KeyAndData;
 using sync_driver::DeviceInfo;
 using sync_driver::DeviceInfoTracker;
 using sync_driver::LocalDeviceInfoProviderMock;
 using sync_pb::DataTypeState;
 using sync_pb::DeviceInfoSpecifics;
 using sync_pb::EntitySpecifics;
 
-using ClientTagList = ModelTypeService::ClientTagList;
+using StorageKeyList = ModelTypeService::StorageKeyList;
 using RecordList = ModelTypeStore::RecordList;
 using Result = ModelTypeStore::Result;
 using StartCallback = ModelTypeChangeProcessor::StartCallback;
 using WriteBatch = ModelTypeStore::WriteBatch;
 
 namespace {
 
 std::unique_ptr<DeviceInfo> CreateDeviceInfo() {
   return base::MakeUnique<DeviceInfo>(
       "guid_1", "client_1", "Chromium 10k", "Chrome 10k",
@@ skipping 23 matching lines @@
   ASSERT_EQ(specifics.signin_scoped_device_id(),
             model.signin_scoped_device_id());
 }
 
 void AssertExpectedFromDataBatch(
     std::map<std::string, DeviceInfoSpecifics> expected,
     SyncError error,
     std::unique_ptr<DataBatch> batch) {
   ASSERT_FALSE(error.IsSet());
   while (batch->HasNext()) {
-    const TagAndData& pair = batch->Next();
+    const KeyAndData& pair = batch->Next();
     std::map<std::string, DeviceInfoSpecifics>::iterator iter =
         expected.find(pair.first);
     ASSERT_NE(iter, expected.end());
     AssertEqual(iter->second, pair.second->specifics.device_info());
     // Removing allows us to verify we don't see the same item multiple times,
     // and that we saw everything we expected.
     expected.erase(iter);
   }
   ASSERT_TRUE(expected.empty());
 }
@@ skipping 138 matching lines @@
 
   // Override to allow specific cache guids.
   DeviceInfoSpecifics GenerateTestSpecifics(const std::string& guid) {
     DeviceInfoSpecifics specifics(GenerateTestSpecifics());
     specifics.set_cache_guid(guid);
     return specifics;
   }
 
   // Helper method to reduce duplicated code between tests. Wraps the given
   // specifics object in an EntityData and EntityChange of type ACTION_ADD, and
   // pushes them onto the given change list. The corresponding client tag the

[skym, 2016/08/10 19:23:23]

Some "tag" naming that didn't get updated here.

[skym, 2016/08/11 16:28:48]

Done.

   // service determines is returned. Instance method because we need access to
   // service to generate the tag.

[skym, 2016/08/10 19:23:23]

And here.

[skym, 2016/08/11 16:28:47]

Done.

   std::string PushBackEntityChangeAdd(const DeviceInfoSpecifics& specifics,
                                       EntityChangeList* changes) {
     EntityDataPtr ptr = SpecificsToEntity(specifics);
-    const std::string tag = service()->GetClientTag(ptr.value());
-    changes->push_back(EntityChange::CreateAdd(tag, ptr));
-    return tag;
+    changes->push_back(EntityChange::CreateAdd(specifics.cache_guid(), ptr));
+    return specifics.cache_guid();
   }
 
   // Allows access to the store before that will ultimately be used to
   // initialize the service.
   ModelTypeStore* store() {
     EXPECT_TRUE(store_);
     return store_.get();
   }
 
   // Get the number of times the service notifies observers of changes.
@@ skipping 203 matching lines @@
   store()->WriteData(batch.get(), specifics2.cache_guid(),
                      specifics2.SerializeAsString());
   store()->WriteData(batch.get(), specifics3.cache_guid(),
                      specifics3.SerializeAsString());
   store()->CommitWriteBatch(std::move(batch),
                             base::Bind(&AssertResultIsSuccess));
 
   InitializeAndPump();
 
   std::map<std::string, DeviceInfoSpecifics> expected;
-  expected[CacheGuidToTag(specifics1.cache_guid())] = specifics1;
-  expected[CacheGuidToTag(specifics3.cache_guid())] = specifics3;
-  ClientTagList client_tags;
-  client_tags.push_back(CacheGuidToTag(specifics1.cache_guid()));
-  client_tags.push_back(CacheGuidToTag(specifics3.cache_guid()));
-  service()->GetData(client_tags,
+  expected[specifics1.cache_guid()] = specifics1;
+  expected[specifics3.cache_guid()] = specifics3;
+  StorageKeyList storage_keys;
+  storage_keys.push_back(specifics1.cache_guid());
+  storage_keys.push_back(specifics3.cache_guid());
+  service()->GetData(storage_keys,
                      base::Bind(&AssertExpectedFromDataBatch, expected));
 }
 
 TEST_F(DeviceInfoServiceTest, GetDataMissing) {
   InitializeAndPump();
   std::map<std::string, DeviceInfoSpecifics> expected;
-  ClientTagList client_tags;
-  client_tags.push_back(CacheGuidToTag("tag1"));
-  service()->GetData(client_tags,
+  StorageKeyList storage_keys;
+  storage_keys.push_back("tag1");

[skym, 2016/08/10 19:23:23]

And here.

[skym, 2016/08/11 16:28:48]

Done.

+  service()->GetData(storage_keys,
                      base::Bind(&AssertExpectedFromDataBatch, expected));
 }
 
 TEST_F(DeviceInfoServiceTest, GetAllData) {
   std::unique_ptr<WriteBatch> batch = store()->CreateWriteBatch();
   DeviceInfoSpecifics specifics1(GenerateTestSpecifics());
   DeviceInfoSpecifics specifics2(GenerateTestSpecifics());
-  const std::string& tag1 = CacheGuidToTag(specifics1.cache_guid());
-  const std::string& tag2 = CacheGuidToTag(specifics2.cache_guid());
+  const std::string& guid1 = specifics1.cache_guid();
+  const std::string& guid2 = specifics2.cache_guid();
   store()->WriteData(batch.get(), specifics1.cache_guid(),
                      specifics1.SerializeAsString());
   store()->WriteData(batch.get(), specifics2.cache_guid(),
                      specifics2.SerializeAsString());
   store()->CommitWriteBatch(std::move(batch),
                             base::Bind(&AssertResultIsSuccess));
 
   InitializeAndPump();
 
   std::map<std::string, DeviceInfoSpecifics> expected;
-  expected[tag1] = specifics1;
-  expected[tag2] = specifics2;
-  ClientTagList client_tags;
-  client_tags.push_back(tag1);
-  client_tags.push_back(tag2);
-  service()->GetData(client_tags,
+  expected[guid1] = specifics1;
+  expected[guid2] = specifics2;
+  StorageKeyList storage_keys;
+  storage_keys.push_back(guid1);
+  storage_keys.push_back(guid2);
+  service()->GetData(storage_keys,
                      base::Bind(&AssertExpectedFromDataBatch, expected));
 }
 
 TEST_F(DeviceInfoServiceTest, ApplySyncChangesEmpty) {
   InitializeAndPump();
   const SyncError error = service()->ApplySyncChanges(
       service()->CreateMetadataChangeList(), EntityChangeList());
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(0, change_count());
 }
 
 TEST_F(DeviceInfoServiceTest, ApplySyncChangesInMemory) {
   InitializeAndPump();
 
   DeviceInfoSpecifics specifics = GenerateTestSpecifics();
   EntityChangeList add_changes;
-  const std::string tag = PushBackEntityChangeAdd(specifics, &add_changes);
+  PushBackEntityChangeAdd(specifics, &add_changes);
   SyncError error = service()->ApplySyncChanges(
       service()->CreateMetadataChangeList(), add_changes);
 
   EXPECT_FALSE(error.IsSet());
   std::unique_ptr<DeviceInfo> info =
       service()->GetDeviceInfo(specifics.cache_guid());
   ASSERT_TRUE(info);
   AssertEqual(specifics, *info.get());
   EXPECT_EQ(1, change_count());
 
   EntityChangeList delete_changes;
-  delete_changes.push_back(EntityChange::CreateDelete(tag));
+  delete_changes.push_back(EntityChange::CreateDelete(specifics.cache_guid()));
   error = service()->ApplySyncChanges(service()->CreateMetadataChangeList(),
                                       delete_changes);
 
   EXPECT_FALSE(error.IsSet());
   EXPECT_FALSE(service()->GetDeviceInfo(specifics.cache_guid()));
   EXPECT_EQ(2, change_count());
 }
 
 TEST_F(DeviceInfoServiceTest, ApplySyncChangesStore) {
   InitializeAndPump();
 
   DeviceInfoSpecifics specifics = GenerateTestSpecifics();
   EntityChangeList data_changes;
-  const std::string tag = PushBackEntityChangeAdd(specifics, &data_changes);
+  PushBackEntityChangeAdd(specifics, &data_changes);
   DataTypeState state;
   state.set_encryption_key_name("ekn");
   std::unique_ptr<MetadataChangeList> metadata_changes(
       service()->CreateMetadataChangeList());
   metadata_changes->UpdateDataTypeState(state);
 
   const SyncError error =
       service()->ApplySyncChanges(std::move(metadata_changes), data_changes);
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(1, change_count());
@@ skipping 12 matching lines @@
 
 TEST_F(DeviceInfoServiceTest, ApplySyncChangesWithLocalGuid) {
   InitializeAndPumpAndStart();
 
   // The point of this test is to try to apply remote changes that have the same
   // cache guid as the local device. The service should ignore these changes
   // since only it should be performing writes on its data.
   DeviceInfoSpecifics specifics =
       GenerateTestSpecifics(local_device()->GetLocalDeviceInfo()->guid());
   EntityChangeList change_list;
-  const std::string tag = PushBackEntityChangeAdd(specifics, &change_list);
+  PushBackEntityChangeAdd(specifics, &change_list);
 
   // Should have a single change from reconciliation.
   EXPECT_TRUE(
       service()->GetDeviceInfo(local_device()->GetLocalDeviceInfo()->guid()));
   EXPECT_EQ(1, change_count());
   // Ensure |last_updated| is about now, plus or minus a little bit.
   Time last_updated(
       syncer::ProtoTimeToTime(processor()
                                   ->put_map()
                                   .begin()
                                   ->second->specifics.device_info()
                                   .last_updated_timestamp()));
   EXPECT_LT(Time::Now() - TimeDelta::FromMinutes(1), last_updated);
   EXPECT_GT(Time::Now() + TimeDelta::FromMinutes(1), last_updated);
 
   EXPECT_FALSE(
       service()
           ->ApplySyncChanges(service()->CreateMetadataChangeList(), change_list)
           .IsSet());
   EXPECT_EQ(1, change_count());
 
   change_list.clear();
-  change_list.push_back(EntityChange::CreateDelete(tag));
+  change_list.push_back(EntityChange::CreateDelete(specifics.cache_guid()));
   EXPECT_FALSE(
       service()
           ->ApplySyncChanges(service()->CreateMetadataChangeList(), change_list)
           .IsSet());
   EXPECT_EQ(1, change_count());
 }
 
 TEST_F(DeviceInfoServiceTest, ApplyDeleteNonexistent) {
   InitializeAndPumpAndStart();
   EXPECT_EQ(1, change_count());
   EntityChangeList delete_changes;
-  delete_changes.push_back(EntityChange::CreateDelete(CacheGuidToTag("tag")));
+  delete_changes.push_back(EntityChange::CreateDelete("guid"));
   const SyncError error = service()->ApplySyncChanges(
       service()->CreateMetadataChangeList(), delete_changes);
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(1, change_count());
 }
 
 TEST_F(DeviceInfoServiceTest, MergeEmpty) {
   InitializeAndPumpAndStart();
   EXPECT_EQ(1, change_count());
   const SyncError error = service()->MergeSyncData(
       service()->CreateMetadataChangeList(), EntityDataMap());
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(1, change_count());
   EXPECT_EQ(1u, processor()->put_map().size());
   EXPECT_EQ(0u, processor()->delete_set().size());
 }
 
 TEST_F(DeviceInfoServiceTest, MergeWithData) {
   const DeviceInfoSpecifics unique_local(GenerateTestSpecifics("unique_local"));

[maxbogue, 2016/08/10 23:56:03]

Could you create const strings here for the three strings that are re-used in
this test case?

[skym, 2016/08/11 16:28:48]

Kinda done, removed all the duplicated string usages.

   const DeviceInfoSpecifics conflict_local(GenerateTestSpecifics("conflict"));
   DeviceInfoSpecifics conflict_remote(GenerateTestSpecifics("conflict"));
   DeviceInfoSpecifics unique_remote(GenerateTestSpecifics("unique_remote"));
 
   std::unique_ptr<WriteBatch> batch = store()->CreateWriteBatch();
   store()->WriteData(batch.get(), unique_local.cache_guid(),
                      unique_local.SerializeAsString());
   store()->WriteData(batch.get(), conflict_local.cache_guid(),
                      conflict_local.SerializeAsString());
   store()->CommitWriteBatch(std::move(batch),
                             base::Bind(&AssertResultIsSuccess));
 
   InitializeAndPumpAndStart();
   EXPECT_EQ(1, change_count());
 
   EntityDataMap remote_input;
-  remote_input[CacheGuidToTag(conflict_remote.cache_guid())] =
+  remote_input[conflict_remote.cache_guid()] =
       SpecificsToEntity(conflict_remote);
-  remote_input[CacheGuidToTag(unique_remote.cache_guid())] =
-      SpecificsToEntity(unique_remote);
+  remote_input[unique_remote.cache_guid()] = SpecificsToEntity(unique_remote);
 
   DataTypeState state;
   state.set_encryption_key_name("ekn");
   std::unique_ptr<MetadataChangeList> metadata_changes(
       service()->CreateMetadataChangeList());
   metadata_changes->UpdateDataTypeState(state);
 
   const SyncError error =
       service()->MergeSyncData(std::move(metadata_changes), remote_input);
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(2, change_count());
 
   // The remote should beat the local in conflict.
   EXPECT_EQ(4u, service()->GetAllDeviceInfo().size());
   AssertEqual(unique_local, *service()->GetDeviceInfo("unique_local").get());
   AssertEqual(unique_remote, *service()->GetDeviceInfo("unique_remote").get());
   AssertEqual(conflict_remote, *service()->GetDeviceInfo("conflict").get());
 
   // Service should have told the processor about the existance of unique_local.
   EXPECT_TRUE(processor()->delete_set().empty());
   EXPECT_EQ(2u, processor()->put_map().size());
-  const auto& it = processor()->put_map().find(CacheGuidToTag("unique_local"));
+  const auto& it = processor()->put_map().find("unique_local");
   ASSERT_NE(processor()->put_map().end(), it);
   AssertEqual(unique_local, it->second->specifics.device_info());
 
   RestartService();
   ASSERT_EQ(state.encryption_key_name(),
             processor()->metadata()->GetDataTypeState().encryption_key_name());
 }
 
 TEST_F(DeviceInfoServiceTest, MergeLocalGuid) {
   const DeviceInfo* local_device_info = local_device()->GetLocalDeviceInfo();
   std::unique_ptr<DeviceInfoSpecifics> specifics(
       CopyToSpecifics(*local_device_info));
   specifics->set_last_updated_timestamp(syncer::TimeToProtoTime(Time::Now()));
   const std::string guid = local_device_info->guid();
 
   std::unique_ptr<WriteBatch> batch = store()->CreateWriteBatch();
   store()->WriteData(batch.get(), guid, specifics->SerializeAsString());
   store()->CommitWriteBatch(std::move(batch),
                             base::Bind(&AssertResultIsSuccess));
 
   InitializeAndPumpAndStart();
 
   EntityDataMap remote_input;
-  remote_input[CacheGuidToTag(guid)] = SpecificsToEntity(*specifics);
+  remote_input[guid] = SpecificsToEntity(*specifics);
 
   const SyncError error = service()->MergeSyncData(
       service()->CreateMetadataChangeList(), remote_input);
   EXPECT_FALSE(error.IsSet());
   EXPECT_EQ(0, change_count());
   EXPECT_EQ(1u, service()->GetAllDeviceInfo().size());
   EXPECT_TRUE(processor()->delete_set().empty());
   EXPECT_TRUE(processor()->put_map().empty());
 }
 
@@ skipping 31 matching lines @@
   specifics.set_cache_guid("non-local");
   PushBackEntityChangeAdd(specifics, &change_list);
   service()->ApplySyncChanges(service()->CreateMetadataChangeList(),
                               change_list);
   EXPECT_EQ(1, service()->CountActiveDevices());
 }
 
 }  // namespace
 
 }  // namespace sync_driver_v2