Cleanup archive structure.

Author: chinmaygarde

impeller/archivist/BUILD.gn

@@ -34,7 +34,11 @@ impeller_component("archivist") {
 
 impeller_component("archivist_unittests") {
   testonly = true
-  sources = [ "archivist_unittests.cc" ]
+  sources = [
+    "archivist_fixture.cc",
+    "archivist_fixture.h",
+    "archivist_unittests.cc",
+  ]
   deps = [
     ":archivist",
     "//flutter/testing",

impeller/archivist/archive.cc

@@ -14,8 +14,8 @@
 
 namespace impeller {
 
-Archive::Archive(const std::string& path, bool recreate)
-    : database_(std::make_unique<ArchiveDatabase>(path, recreate)) {}
+Archive::Archive(const std::string& path)
+    : database_(std::make_unique<ArchiveDatabase>(path)) {}
 
 Archive::~Archive() {
   FML_DCHECK(transaction_count_ == 0)
@@ -64,7 +64,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
   /*
    *  We need to bind the primary key only if the item does not provide its own
    */
-  if (!definition.autoAssignName &&
+  if (!definition.auto_key &&
       !statement.WriteValue(ArchiveClassRegistration::NameIndex, itemName)) {
     return false;
   }
@@ -79,8 +79,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
 
   int64_t lastInsert = database_->GetLastInsertRowID();
 
-  if (!definition.autoAssignName &&
-      lastInsert != static_cast<int64_t>(itemName)) {
+  if (!definition.auto_key && lastInsert != static_cast<int64_t>(itemName)) {
     return false;
   }
 
@@ -256,7 +255,7 @@ bool ArchiveLocation::ReadVectorKeys(Archivable::ArchiveName name,
     return false;
   }
 
-  const auto& keys = vector.keys();
+  const auto& keys = vector.GetKeys();
 
   std::move(keys.begin(), keys.end(), std::back_inserter(members));
 

impeller/archivist/archive.h

@@ -24,17 +24,17 @@ struct ArchiveDef {
   using Member = uint64_t;
   using Members = std::vector<Member>;
 
-  const ArchiveDef* superClass;
-  const std::string className;
-  const bool autoAssignName;
+  const ArchiveDef* isa = nullptr;
+  const std::string table_name;
+  const bool auto_key = true;
   const Members members;
 };
 
 static const Archivable::ArchiveName ArchiveNameAuto = 0;
 
 class Archive {
  public:
-  Archive(const std::string& path, bool recreate);
+  Archive(const std::string& path);
 
   ~Archive();
 

impeller/archivist/archive_class_registration.cc

@@ -11,13 +11,12 @@
 
 namespace impeller {
 
-static const char* const ArchiveColumnPrefix = "item";
-static const char* const ArchivePrimaryKeyColumnName = "name";
-static const char* const ArchiveTablePrefix = "RL_";
+static const char* const ArchiveColumnPrefix = "col_";
+static const char* const ArchivePrimaryKeyColumnName = "primary_key";
 
 ArchiveClassRegistration::ArchiveClassRegistration(ArchiveDatabase& database,
                                                    ArchiveDef definition)
-    : database_(database), class_name_(definition.className) {
+    : database_(database), class_name_(definition.table_name) {
   /*
    *  Each class in the archive class hierarchy is assigned an entry in the
    *  class map.
@@ -31,11 +30,11 @@ ArchiveClassRegistration::ArchiveClassRegistration(ArchiveDatabase& database,
     for (const auto& member : current->members) {
       map[member] = currentMember++;
     }
-    class_map_[current->className] = map;
-    current = current->superClass;
+    class_map_[current->table_name] = map;
+    current = current->isa;
   }
 
-  is_ready_ = CreateTable(definition.autoAssignName);
+  is_ready_ = CreateTable(definition.auto_key);
 }
 
 const std::string& ArchiveClassRegistration::GetClassName() const {
@@ -81,8 +80,8 @@ bool ArchiveClassRegistration::CreateTable(bool autoIncrement) {
    *  Table names cannot participate in parameter substitution, so we prepare
    *  a statement and check its validity before running.
    */
-  stream << "CREATE TABLE IF NOT EXISTS " << ArchiveTablePrefix
-         << class_name_.c_str() << " (" << ArchivePrimaryKeyColumnName;
+  stream << "CREATE TABLE IF NOT EXISTS " << class_name_.c_str() << " ("
+         << ArchivePrimaryKeyColumnName;
 
   if (autoIncrement) {
     stream << " INTEGER PRIMARY KEY AUTOINCREMENT, ";
@@ -120,7 +119,7 @@ ArchiveStatement ArchiveClassRegistration::GetQueryStatement(
       stream << ",";
     }
   }
-  stream << " FROM " << ArchiveTablePrefix << class_name_;
+  stream << " FROM " << class_name_;
 
   if (single) {
     stream << " WHERE " << ArchivePrimaryKeyColumnName << " = ?";
@@ -135,8 +134,7 @@ ArchiveStatement ArchiveClassRegistration::GetQueryStatement(
 
 ArchiveStatement ArchiveClassRegistration::GetInsertStatement() const {
   std::stringstream stream;
-  stream << "INSERT OR REPLACE INTO " << ArchiveTablePrefix << class_name_
-         << " VALUES ( ?, ";
+  stream << "INSERT OR REPLACE INTO " << class_name_ << " VALUES ( ?, ";
   for (size_t i = 0; i < member_count_; i++) {
     stream << "?";
     if (i != member_count_ - 1) {

impeller/archivist/archive_database.cc

@@ -18,11 +18,7 @@ namespace impeller {
 
 #define DB_HANDLE reinterpret_cast<sqlite3*>(database_)
 
-ArchiveDatabase::ArchiveDatabase(const std::string& filename, bool recreate) {
-  if (recreate) {
-    ::remove(filename.c_str());
-  }
-
+ArchiveDatabase::ArchiveDatabase(const std::string& filename) {
   if (::sqlite3_initialize() != SQLITE_OK) {
     VALIDATION_LOG << "Could not initialize sqlite.";
     return;
@@ -82,7 +78,7 @@ static inline const ArchiveClassRegistration* RegistrationIfReady(
 
 const ArchiveClassRegistration* ArchiveDatabase::GetRegistrationForDefinition(
     const ArchiveDef& definition) {
-  auto found = registrations_.find(definition.className);
+  auto found = registrations_.find(definition.table_name);
   if (found != registrations_.end()) {
     /*
      *  This class has already been registered.
@@ -96,7 +92,7 @@ const ArchiveClassRegistration* ArchiveDatabase::GetRegistrationForDefinition(
   auto registration = std::unique_ptr<ArchiveClassRegistration>(
       new ArchiveClassRegistration(*this, definition));
   auto res =
-      registrations_.emplace(definition.className, std::move(registration));
+      registrations_.emplace(definition.table_name, std::move(registration));
 
   /*
    *  If the new class registation is ready, return it to the caller.

impeller/archivist/archive_database.h

@@ -18,7 +18,7 @@ struct ArchiveDef;
 
 class ArchiveDatabase {
  public:
-  ArchiveDatabase(const std::string& filename, bool recreate);
+  ArchiveDatabase(const std::string& filename);
 
   ~ArchiveDatabase();
 

impeller/archivist/archive_vector.cc

@@ -24,7 +24,7 @@ Archivable::ArchiveName ArchiveVector::GetArchiveName() const {
   return ArchiveNameAuto;
 }
 
-const std::vector<int64_t> ArchiveVector::keys() const {
+const std::vector<int64_t> ArchiveVector::GetKeys() const {
   return keys_;
 }
 

impeller/archivist/archive_vector.h

@@ -15,7 +15,7 @@ class ArchiveVector : public Archivable {
 
   ArchiveName GetArchiveName() const override;
 
-  const std::vector<int64_t> keys() const;
+  const std::vector<int64_t> GetKeys() const;
 
   bool Write(ArchiveLocation& item) const override;
 

impeller/archivist/archivist_fixture.cc

@@ -0,0 +1,38 @@
+// Copyright 2013 The Flutter 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 "impeller/archivist/archivist_fixture.h"
+
+#include "flutter/fml/paths.h"
+
+namespace impeller {
+namespace testing {
+
+ArchivistFixture::ArchivistFixture() {
+  std::stringstream stream;
+  stream << flutter::testing::GetCurrentTestName() << ".db";
+  archive_file_name_ = fml::paths::JoinPaths(
+      {flutter::testing::GetFixturesPath(), stream.str()});
+}
+
+ArchivistFixture::~ArchivistFixture() = default;
+
+const std::string ArchivistFixture::GetArchiveFileName() const {
+  return archive_file_name_;
+}
+
+void ArchivistFixture::SetUp() {
+  DeleteArchiveFile();
+}
+
+void ArchivistFixture::TearDown() {
+  // DeleteArchiveFile();
+}
+
+void ArchivistFixture::DeleteArchiveFile() const {
+  fml::UnlinkFile(archive_file_name_.c_str());
+}
+
+}  // namespace testing
+}  // namespace impeller

impeller/archivist/archivist_fixture.h

@@ -0,0 +1,36 @@
+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#pragma once
+
+#include "flutter/fml/macros.h"
+#include "flutter/testing/testing.h"
+
+namespace impeller {
+namespace testing {
+
+class ArchivistFixture : public ::testing::Test {
+ public:
+  ArchivistFixture();
+
+  ~ArchivistFixture();
+
+  // |::testing::Test|
+  void SetUp() override;
+
+  // |::testing::Test|
+  void TearDown() override;
+
+  const std::string GetArchiveFileName() const;
+
+ private:
+  std::string archive_file_name_;
+
+  void DeleteArchiveFile() const;
+
+  FML_DISALLOW_COPY_AND_ASSIGN(ArchivistFixture);
+};
+
+}  // namespace testing
+}  // namespace impeller