Deprecate DriveMetadataStore.batch_sync_origins

chrome/browser/sync_file_system/drive_metadata_store_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/sync_file_system/drive_metadata_store.h"
 
 #include <utility>
 
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
@@ skipping 114 matching lines @@
   void RestoreSyncRootDirectoryFromDB() {
     EXPECT_TRUE(ui_task_runner_->RunsTasksOnCurrentThread());
     drive_metadata_store_->RestoreSyncRootDirectory(
         base::Bind(&DriveMetadataStoreTest::DidRestoreSyncRootDirectory,
                    base::Unretained(this)));
     message_loop_.Run();
   }
 
   void DropSyncOriginsInStore() {
     EXPECT_TRUE(ui_task_runner_->RunsTasksOnCurrentThread());
-    drive_metadata_store_->batch_sync_origins_.clear();
     drive_metadata_store_->incremental_sync_origins_.clear();
     drive_metadata_store_->disabled_origins_.clear();
-    EXPECT_TRUE(drive_metadata_store_->batch_sync_origins().empty());
     EXPECT_TRUE(drive_metadata_store_->incremental_sync_origins().empty());
     EXPECT_TRUE(drive_metadata_store_->disabled_origins().empty());
   }
 
   void RestoreOriginsFromDB() {
     EXPECT_TRUE(ui_task_runner_->RunsTasksOnCurrentThread());
     drive_metadata_store_->RestoreOrigins(
         base::Bind(&DriveMetadataStoreTest::DidRestoreOrigins,
                    base::Unretained(this)));
     message_loop_.Run();
@@ skipping 60 matching lines @@
                        SyncStatusCode status) {
     *status_out = status;
     message_loop_.Quit();
   }
 
   void MarkAsCreated() {
     created_ = true;
   }
 
   void VerifyUntrackedOrigin(const GURL& origin) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsOriginDisabled(origin));
   }
 
   void VerifyIncrementalSyncOrigin(const GURL& origin,
                                    const std::string& resource_id) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_TRUE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsOriginDisabled(origin));
     EXPECT_EQ(resource_id,
               GetResourceID(metadata_store()->incremental_sync_origins(),
                             origin));
   }
 
   void VerifyDisabledOrigin(const GURL& origin,
                             const std::string& resource_id) {
-    EXPECT_FALSE(metadata_store()->IsBatchSyncOrigin(origin));
     EXPECT_FALSE(metadata_store()->IsIncrementalSyncOrigin(origin));
     EXPECT_TRUE(metadata_store()->IsOriginDisabled(origin));
     EXPECT_EQ(resource_id,
               GetResourceID(metadata_store()->disabled_origins(), origin));
   }
 
   base::FilePath base_dir() {
     return base_dir_.path();
   }
 
   DriveMetadataStore* metadata_store() {
     return drive_metadata_store_.get();
   }
 
   const DriveMetadataStore::MetadataMap& metadata_map() {
     return drive_metadata_store_->metadata_map_;
   }
 
   void VerifyReverseMap() {
-    const ResourceIdByOrigin& batch_sync_origins =
-        drive_metadata_store_->batch_sync_origins_;
     const ResourceIdByOrigin& incremental_sync_origins =
         drive_metadata_store_->incremental_sync_origins_;
     const ResourceIdByOrigin& disabled_origins =
         drive_metadata_store_->disabled_origins_;
     const OriginByResourceId& origin_by_resource_id =
         drive_metadata_store_->origin_by_resource_id_;
 
-    size_t expected_size =
-        batch_sync_origins.size() + incremental_sync_origins.size() +
-        disabled_origins.size();
+    size_t expected_size = incremental_sync_origins.size() +
+                           disabled_origins.size();
     size_t actual_size = origin_by_resource_id.size();
     EXPECT_EQ(expected_size, actual_size);
-    EXPECT_TRUE(VerifyReverseMapInclusion(batch_sync_origins,
-                                          origin_by_resource_id));
     EXPECT_TRUE(VerifyReverseMapInclusion(incremental_sync_origins,
                                           origin_by_resource_id));
     EXPECT_TRUE(VerifyReverseMapInclusion(disabled_origins,
                                           origin_by_resource_id));
   }
 
  private:
   void DidInitializeDatabase(bool* done_out,
                              SyncStatusCode* status_out,
                              bool* created_out,
@@ skipping 245 matching lines @@
                     kOrigin3, kServiceName, base::FilePath(FPL("waf"))),
                 CreateMetadata("baz", "egg", false, false)));
 
   EXPECT_EQ(SYNC_STATUS_OK, RemoveOrigin(kOrigin2));
   EXPECT_EQ(SYNC_STATUS_OK, RemoveOrigin(kOrigin3));
 
   DropDatabase();
   InitializeDatabase();
 
   // kOrigin1 should be the only one left.
-  EXPECT_EQ(0u, metadata_store()->batch_sync_origins().size());
   EXPECT_EQ(1u, metadata_store()->incremental_sync_origins().size());
   EXPECT_EQ(0u, metadata_store()->disabled_origins().size());
   EXPECT_TRUE(metadata_store()->IsIncrementalSyncOrigin(kOrigin1));
   EXPECT_EQ(1u, metadata_map().size());
 
   DriveMetadataStore::MetadataMap::const_iterator found =
       metadata_map().find(kOrigin1);
   EXPECT_TRUE(found != metadata_map().end() && found->second.size() == 1u);
 
   VerifyReverseMap();
@@ skipping 97 matching lines @@
             kDriveIncrementalSyncOriginKeyPrefix + kOrigin2.spec(),
             kResourceId2);
     EXPECT_TRUE(RevokeSyncableFileSystem(kV0ServiceName));
     MarkAsCreated();
   }
 
   InitializeDatabase();
 
   EXPECT_EQ(1, metadata_store()->GetLargestChangeStamp());
   EXPECT_EQ(kSyncRootResourceId, metadata_store()->sync_root_directory());
-  EXPECT_EQ(kResourceId1, metadata_store()->GetResourceIdForOrigin(kOrigin1));
   EXPECT_EQ(kResourceId2, metadata_store()->GetResourceIdForOrigin(kOrigin2));
 
   DriveMetadata metadata;
   EXPECT_EQ(SYNC_STATUS_OK,
             metadata_store()->ReadEntry(
                 CreateSyncableFileSystemURL(kOrigin1, kServiceName, kFile),
                 &metadata));
   EXPECT_EQ(kFileResourceId, metadata.resource_id());
   EXPECT_EQ(kFileMD5, metadata.md5_checksum());
   EXPECT_FALSE(metadata.conflicted());
   EXPECT_FALSE(metadata.to_be_fetched());
 
   VerifyReverseMap();
 }
 
+TEST_F(DriveMetadataStoreTest, DeprecateBatchSyncOrigins) {
+  // Make sure that previously saved batch sync origins were deleted from the DB
+  // as they are no longer used.
+  const char kDriveBatchSyncOriginKeyPrefix[] = "BSYNC_ORIGIN: ";
+  const GURL kOrigin1("chrome-extension://example1");
+  const std::string kSyncRootResourceId("sync_root_resource_id");
+  const std::string kResourceId1("hoge");
+  const std::string kFileResourceId("piyo");
+
+  // Purposely add in an old batch sync origin (from previous DB version).
+  leveldb::Options options;
+  options.create_if_missing = true;
+  leveldb::DB* db_ptr = NULL;
+  std::string db_dir = fileapi::FilePathToString(
+      base_dir().Append(DriveMetadataStore::kDatabaseName));
+  leveldb::Status status = leveldb::DB::Open(options, db_dir, &db_ptr);

[tzik, 2013/05/27 04:31:58]

We can open only one LevelDB instance for each path, that seems to cause the
Windows test failure.

Can we get it from metadata_store()?
Or, just
 - Close the DB before InitializeDatabase(),
 - DropDatabase(),
 - Reopen the DB,
may work.

[tzik, 2013/05/27 07:57:42]

For second option, we need to ensure the LevelDB instance was dropped before the
reopen. Since it is closed asynchronously on FILE thread.

Please insert something like below (it may need a helper function)
file_task_runner_->PostTaskAndReply(
    FROM_HERE,
    base::Bind(&Noop),
    base::Bind(&FunctionToCallMessageLoopQuit));
message_loop_->Run();

+  scoped_ptr<leveldb::DB> db(db_ptr);
+  leveldb::WriteOptions write_options;
+  db->Put(write_options,
+          kDriveBatchSyncOriginKeyPrefix + kOrigin1.spec(), kResourceId1);
+  InitializeDatabase();
+
+  // Confirm no batch sync origins rename after InitializeDatabase.
+  scoped_ptr<leveldb::Iterator> itr(db->NewIterator(leveldb::ReadOptions()));
+  for (itr->SeekToFirst(); itr->Valid(); itr->Next()) {
+    std::string key = itr->key().ToString();
+    bool batch_origin_found =
+        (key.find(kDriveBatchSyncOriginKeyPrefix) == std::string::npos);
+    EXPECT_FALSE(batch_origin_found);
+  }
+}
+
 TEST_F(DriveMetadataStoreTest, ResetOriginRootDirectory) {
   const GURL kOrigin1("chrome-extension://example1");
   const std::string kResourceId1("hoge");
   const std::string kResourceId2("fuga");
 
   InitializeDatabase();
   EXPECT_EQ(SYNC_STATUS_OK, SetLargestChangeStamp(1));
 
   metadata_store()->AddIncrementalSyncOrigin(kOrigin1, kResourceId1);
   VerifyIncrementalSyncOrigin(kOrigin1, kResourceId1);
   VerifyReverseMap();
 
   metadata_store()->SetOriginRootDirectory(kOrigin1, kResourceId2);
   VerifyIncrementalSyncOrigin(kOrigin1, kResourceId2);
   VerifyReverseMap();
 }
 
 }  // namespace sync_file_system