[sync] Add store version to model type store backend

Add the ModelTypeStoreSchemaDescriptor protobuf, and add a basic
framework for performing migration. Add a Paranoid PRESUBMIT test to
make sure the record identifier for the db version won't be reused.

R=pavely@chromium.org

BUG=656737

Review-Url: https://chromiumcodereview.appspot.com/2426613003
Cr-Commit-Position: refs/heads/master@{#426900}

components/sync/PRESUBMIT.py

@@ -25,6 +25,12 @@ GRANDFATHERED_MODEL_TYPES = [
   'PROXY_TABS',  # Doesn't have a root tag or notification type.
   'NIGORI']  # Model type string is 'encryption keys'.
 
+# Root tags are used as prefixes when creating storage keys, so certain strings
+# are blacklisted in order to prevent prefix collision.
+BLACKLISTED_ROOT_TAGS = [
+  '_mts_schema_descriptor'
+]
+
 # Number of distinct fields in a map entry; used to create
 # sets that check for uniqueness.
 MAP_ENTRY_FIELD_COUNT = 6
@@ -113,6 +119,7 @@ def CheckModelTypeInfoMap(input_api, output_api, model_type_file):
     entry_problems.extend(
       CheckNotificationTypeMatchesProtoMessageName(
         output_api, map_entry, proto_field_definitions))
+    entry_problems.extend(CheckRootTagNotInBlackList(output_api, map_entry))
 
     if map_entry.model_type not in GRANDFATHERED_MODEL_TYPES:
       entry_problems.extend(
@@ -343,6 +350,20 @@ def CheckRootTagMatchesModelType(output_api, map_entry):
         map_entry.affected_lines)]
   return []
 
+def CheckRootTagNotInBlackList(output_api, map_entry):
+  """ Checks that map_entry's root isn't a blacklisted string.
+  Args:
+    output_api: presubmit_support OutputAPI instance
+    map_entry: ModelTypeEnumEntry object to check
+  Returns:
+    A list of PresubmitError objects for each violation
+  """
+  if map_entry.root_tag in BLACKLISTED_ROOT_TAGS:
+    return [FormatPresubmitError(
+        output_api,'root tag "%s" is a blacklisted root tag'
+        % (map_entry.root_tag), map_entry.affected_lines)]
+  return []
+
 
 def FieldNumberToPrototypeString(field_number):
   """Converts a field number enum reference to an EntitySpecifics string.

components/sync/PRESUBMIT_test.py

@@ -8,7 +8,8 @@ import sys
 import unittest
 import PRESUBMIT
 
-sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
+sys.path.append(
+  os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
 from PRESUBMIT_test_mocks import MockOutputApi, MockChange
 
 class MockInputApi(object):
@@ -63,6 +64,7 @@ MOCK_PROTOFILE_CONTENTS = ('\n'
   'optional AutofillSpecifics autofill = 123;\n'
   'optional AppSpecifics app = 456;\n'
   'optional AppSettingSpecifics app_setting = 789;\n'
+  'optional ExtensionSettingSpecifics extension_setting = 910;\n'
   '//comment\n'
   '}\n')
 
@@ -125,6 +127,14 @@ class ModelTypeInfoChangeTest(unittest.TestCase):
     self.assertEqual(6, len(results))
     self.assertTrue('APP_SETTINGS' in results[0].message)
 
+  def testBlacklistedRootTag(self):
+    results = self._testChange('{EXTENSION_SETTING, "EXTENSION_SETTING",\n'
+      '"_mts_schema_descriptor","Extension Setting",\n'
+      'sync_pb::EntitySpecifics::kExtensionSettingFieldNumber, 6},')
+    self.assertEqual(2, len(results))
+    self.assertTrue('_mts_schema_descriptor' in results[0].message)
+    self.assertTrue("blacklist" in results[0].message)
+
   def _testChange(self, modeltype_literal):
     mock_input_api = MockInputApi()
     mock_input_api.files = [

components/sync/model/model_type_store.h

@@ -51,6 +51,7 @@ class ModelTypeStore {
   enum class Result {
     SUCCESS,
     UNSPECIFIED_ERROR,
+    SCHEMA_VERSION_TOO_HIGH,
   };
 
   // Output of read operations is passed back as list of Record structures.

components/sync/model_impl/model_type_store_backend.cc

@@ -8,6 +8,7 @@
 
 #include "base/files/file_path.h"
 #include "base/memory/ptr_util.h"
+#include "components/sync/protocol/model_type_store_schema_descriptor.pb.h"
 #include "third_party/leveldatabase/env_chromium.h"
 #include "third_party/leveldatabase/src/helpers/memenv/memenv.h"
 #include "third_party/leveldatabase/src/include/leveldb/db.h"
@@ -18,8 +19,15 @@
 #include "third_party/leveldatabase/src/include/leveldb/status.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
+using sync_pb::ModelTypeStoreSchemaDescriptor;
+
 namespace syncer {
 
+const int64_t kInvalidSchemaVersion = -1;
+const int64_t ModelTypeStoreBackend::kLatestSchemaVersion = 1;
+const char ModelTypeStoreBackend::kDBSchemaDescriptorRecordId[] =
+    "_mts_schema_descriptor";
+
 // static
 base::LazyInstance<ModelTypeStoreBackend::BackendMap>
     ModelTypeStoreBackend::backend_map_ = LAZY_INSTANCE_INITIALIZER;
@@ -80,6 +88,19 @@ ModelTypeStore::Result ModelTypeStoreBackend::Init(
     return ModelTypeStore::Result::UNSPECIFIED_ERROR;
   }
   db_.reset(db_raw);
+
+  int64_t current_version = GetStoreVersion();
+  if (current_version == kInvalidSchemaVersion) {
+    return ModelTypeStore::Result::UNSPECIFIED_ERROR;
+  }
+
+  if (current_version != kLatestSchemaVersion) {
+    ModelTypeStore::Result result =
+        Migrate(current_version, kLatestSchemaVersion);
+    if (result != ModelTypeStore::Result::SUCCESS) {
+      return result;
+    }
+  }
   return ModelTypeStore::Result::SUCCESS;
 }
 
@@ -99,8 +120,7 @@ ModelTypeStore::Result ModelTypeStoreBackend::ReadRecordsWithPrefix(
     key = prefix + id;
     leveldb::Status status = db_->Get(read_options, key, &value);
     if (status.ok()) {
-      // TODO(pavely): Use emplace_back instead of push_back once it is allowed.
-      record_list->push_back(ModelTypeStore::Record(id, value));
+      record_list->emplace_back(id, value);
     } else if (status.IsNotFound()) {
       missing_id_list->push_back(id);
     } else {
@@ -124,9 +144,7 @@ ModelTypeStore::Result ModelTypeStoreBackend::ReadAllRecordsWithPrefix(
     if (!key.starts_with(prefix_slice))
       break;
     key.remove_prefix(prefix_slice.size());
-    // TODO(pavely): Use emplace_back instead of push_back once it is allowed.
-    record_list->push_back(
-        ModelTypeStore::Record(key.ToString(), iter->value().ToString()));
+    record_list->emplace_back(key.ToString(), iter->value().ToString());
   }
   return iter->status().ok() ? ModelTypeStore::Result::SUCCESS
                              : ModelTypeStore::Result::UNSPECIFIED_ERROR;
@@ -142,4 +160,47 @@ ModelTypeStore::Result ModelTypeStoreBackend::WriteModifications(
                      : ModelTypeStore::Result::UNSPECIFIED_ERROR;
 }
 
+int64_t ModelTypeStoreBackend::GetStoreVersion() {
+  DCHECK(db_);
+  leveldb::ReadOptions read_options;
+  read_options.verify_checksums = true;
+  std::string value;
+  ModelTypeStoreSchemaDescriptor schema_descriptor;
+  leveldb::Status status =
+      db_->Get(read_options, kDBSchemaDescriptorRecordId, &value);
+  if (status.IsNotFound()) {
+    return 0;
+  } else if (!status.ok() || !schema_descriptor.ParseFromString(value)) {
+    return kInvalidSchemaVersion;
+  }
+  return schema_descriptor.version_number();
+}
+
+ModelTypeStore::Result ModelTypeStoreBackend::Migrate(int64_t current_version,
+                                                      int64_t desired_version) {
+  DCHECK(db_);
+  if (current_version == 0) {
+    if (Migrate0To1()) {
+      current_version = 1;
+    }
+  }
+  if (current_version == desired_version) {
+    return ModelTypeStore::Result::SUCCESS;
+  } else if (current_version > desired_version) {
+    return ModelTypeStore::Result::SCHEMA_VERSION_TOO_HIGH;
+  } else {
+    return ModelTypeStore::Result::UNSPECIFIED_ERROR;
+  }
+}
+
+bool ModelTypeStoreBackend::Migrate0To1() {
+  DCHECK(db_);
+  ModelTypeStoreSchemaDescriptor schema_descriptor;
+  schema_descriptor.set_version_number(1);
+  leveldb::Status status =
+      db_->Put(leveldb::WriteOptions(), kDBSchemaDescriptorRecordId,
+               schema_descriptor.SerializeAsString());
+  return status.ok();
+}
+
 }  // namespace syncer

components/sync/model_impl/model_type_store_backend.h

@@ -67,6 +67,9 @@ class ModelTypeStoreBackend
   friend class base::RefCountedThreadSafe<ModelTypeStoreBackend>;
   friend class ModelTypeStoreBackendTest;
 
+  static const int64_t kLatestSchemaVersion;
+  static const char kDBSchemaDescriptorRecordId[];
+
   explicit ModelTypeStoreBackend(const std::string& path);
   ~ModelTypeStoreBackend();
 
@@ -97,6 +100,20 @@ class ModelTypeStoreBackend
   ModelTypeStore::Result Init(const std::string& path,
                               std::unique_ptr<leveldb::Env> env);
 
+  // Attempts to read and return the database's version.
+  // If there is not a schema descriptor present, the value returned is 0.
+  // If an error occurs, the value returned is kInvalidSchemaVersion(-1).
+  int64_t GetStoreVersion();
+
+  // Migrate the db schema from |current_version| to |desired_version|,
+  // returning true on success.
+  ModelTypeStore::Result Migrate(int64_t current_version,
+                                 int64_t desired_version);
+
+  // Migrates from no version record at all (version 0) to version 1 of
+  // the schema, returning true on success.
+  bool Migrate0To1();
+
   // Macro wrapped mutex to guard against concurrent calls in debug builds.
   DFAKE_MUTEX(push_pop_);
 

components/sync/model_impl/model_type_store_backend_unittest.cc

@@ -7,10 +7,13 @@
 #include <utility>
 
 #include "base/memory/ptr_util.h"
+#include "components/sync/protocol/model_type_store_schema_descriptor.pb.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/leveldatabase/src/include/leveldb/env.h"
 #include "third_party/leveldatabase/src/include/leveldb/write_batch.h"
 
+using sync_pb::ModelTypeStoreSchemaDescriptor;
+
 namespace syncer {
 
 class ModelTypeStoreBackendTest : public testing::Test {
@@ -49,6 +52,28 @@ class ModelTypeStoreBackendTest : public testing::Test {
   std::string GetBackendPath(scoped_refptr<ModelTypeStoreBackend> backend) {
     return backend->path_;
   }
+
+  ModelTypeStore::Result Migrate(scoped_refptr<ModelTypeStoreBackend> backend,
+                                 int64_t current_version,
+                                 int64_t desired_version) {
+    return backend->Migrate(current_version, desired_version);
+  }
+
+  bool Migrate0To1(scoped_refptr<ModelTypeStoreBackend> backend) {
+    return backend->Migrate0To1();
+  }
+
+  int64_t GetStoreVersion(scoped_refptr<ModelTypeStoreBackend> backend) {
+    return backend->GetStoreVersion();
+  }
+
+  int64_t LatestVersion() {
+    return ModelTypeStoreBackend::kLatestSchemaVersion;
+  }
+
+  const char* SchemaId() {
+    return ModelTypeStoreBackend::kDBSchemaDescriptorRecordId;
+  }
 };
 
 // Test that after record is written to backend it can be read back even after
@@ -190,4 +215,33 @@ TEST_F(ModelTypeStoreBackendTest, TwoDifferentBackendTest) {
   ASSERT_FALSE(BackendExistsForPath("/test_db2"));
 }
 
+// Test that initializing the database migrates it to the latest schema version.
+TEST_F(ModelTypeStoreBackendTest, MigrateNoSchemaVersionToLatestVersionTest) {
+  scoped_refptr<ModelTypeStoreBackend> backend = GetOrCreateBackend();
+
+  ASSERT_EQ(LatestVersion(), GetStoreVersion(backend));
+}
+
+// Test that the 0 to 1 migration succeeds and sets the schema version to 1.
+TEST_F(ModelTypeStoreBackendTest, Migrate0To1Test) {
+  scoped_refptr<ModelTypeStoreBackend> backend = GetOrCreateBackend();
+
+  std::unique_ptr<leveldb::WriteBatch> write_batch(new leveldb::WriteBatch());
+  write_batch->Delete(SchemaId());
+  ModelTypeStore::Result result =
+      backend->WriteModifications(std::move(write_batch));
+  ASSERT_EQ(ModelTypeStore::Result::SUCCESS, result);
+
+  ASSERT_TRUE(Migrate0To1(backend));
+  ASSERT_EQ(1, GetStoreVersion(backend));
+}
+
+// Test that migration to an unknown version fails
+TEST_F(ModelTypeStoreBackendTest, MigrateWithHigherExistingVersionFails) {
+  scoped_refptr<ModelTypeStoreBackend> backend = GetOrCreateBackend();
+
+  ASSERT_EQ(Migrate(backend, LatestVersion() + 1, LatestVersion()),
+            ModelTypeStore::Result::SCHEMA_VERSION_TOO_HIGH);
+}
+
 }  // namespace syncer

components/sync/protocol/model_type_store_schema_descriptor.proto

+// Copyright 2016 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.
+//
+// Protocol messages used to record the state of the model type store for USS.
+// At the time of writing, the model type store uses leveldb, a schemaless
+// key-value store. This means that the database's schema is mostly implicit.
+// This descriptor isn't intended to fully describe the schema, just keep track
+// of which major changes have been applied.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+package sync_pb;
+
+message ModelTypeStoreSchemaDescriptor {
+  optional int64 version_number = 1;
+}

components/sync/protocol/protocol_sources.gni

@@ -34,6 +34,7 @@ sync_protocol_sources = [
   "//components/sync/protocol/managed_user_specifics.proto",
   "//components/sync/protocol/managed_user_whitelist_specifics.proto",
   "//components/sync/protocol/model_type_state.proto",
+  "//components/sync/protocol/model_type_store_schema_descriptor.proto",
   "//components/sync/protocol/password_specifics.proto",
   "//components/sync/protocol/preference_specifics.proto",
   "//components/sync/protocol/printer_specifics.proto",