Cleanup archive structure.

engine/src/flutter/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",

engine/src/flutter/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));
 

engine/src/flutter/impeller/archivist/archive.h

@@ -24,9 +24,9 @@ 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;
 };
 
@@ -34,7 +34,7 @@ static const Archivable::ArchiveName ArchiveNameAuto = 0;
 
 class Archive {
  public:
-  Archive(const std::string& path, bool recreate);
+  Archive(const std::string& path);
 
   ~Archive();
 

engine/src/flutter/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) {

engine/src/flutter/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.

engine/src/flutter/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();
 

engine/src/flutter/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_;
 }
 

engine/src/flutter/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;
 

engine/src/flutter/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

engine/src/flutter/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

engine/src/flutter/impeller/archivist/archivist_unittests.cc

@@ -8,6 +8,7 @@
 #include "flutter/fml/macros.h"
 #include "flutter/testing/testing.h"
 #include "impeller/archivist/archive.h"
+#include "impeller/archivist/archivist_fixture.h"
 
 namespace impeller {
 namespace testing {
@@ -44,114 +45,89 @@ class Sample : public Archivable {
 };
 
 const ArchiveDef Sample::ArchiveDefinition = {
-    .superClass = nullptr,
-    .className = "Sample",
-    .autoAssignName = false,
+    .isa = nullptr,
+    .table_name = "Sample",
+    .auto_key = false,
     .members = {999},
 };
 
-TEST(ArchiveTest, SimpleInitialization) {
-  auto name = "/tmp/sample.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
-  }
-  ASSERT_EQ(::remove(name), 0);
+using ArchiveTest = ArchivistFixture;
+
+TEST_F(ArchiveTest, SimpleInitialization) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
 }
 
-TEST(ArchiveTest, AddStorageClass) {
-  auto name = "/tmp/sample2.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
-  }
-  ASSERT_EQ(::remove(name), 0);
+TEST_F(ArchiveTest, AddStorageClass) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
 }
 
-TEST(ArchiveTest, AddData) {
-  auto name = "/tmp/sample3.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
-    Sample sample;
+TEST_F(ArchiveTest, AddData) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
+  Sample sample;
+  ASSERT_TRUE(archive.Write(sample));
+}
+
+TEST_F(ArchiveTest, AddDataMultiple) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
+
+  for (size_t i = 0; i < 100; i++) {
+    Sample sample(i + 1);
     ASSERT_TRUE(archive.Write(sample));
   }
-  ASSERT_EQ(::remove(name), 0);
 }
 
-TEST(ArchiveTest, AddDataMultiple) {
-  auto name = "/tmp/sample4.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
+TEST_F(ArchiveTest, ReadData) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
 
-    for (size_t i = 0; i < 100; i++) {
-      Sample sample(i + 1);
-      ASSERT_TRUE(archive.Write(sample));
-    }
+  size_t count = 50;
+
+  std::vector<Archivable::ArchiveName> keys;
+  std::vector<uint64_t> values;
+
+  for (size_t i = 0; i < count; i++) {
+    Sample sample(i + 1);
+    keys.push_back(sample.GetArchiveName());
+    values.push_back(sample.GetSomeData());
+    ASSERT_TRUE(archive.Write(sample));
+  }
+
+  for (size_t i = 0; i < count; i++) {
+    Sample sample;
+    ASSERT_TRUE(archive.Read(keys[i], sample));
+    ASSERT_EQ(values[i], sample.GetSomeData());
   }
-  ASSERT_EQ(::remove(name), 0);
 }
 
-TEST(ArchiveTest, ReadData) {
-  auto name = "/tmp/sample5.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
+TEST_F(ArchiveTest, ReadDataWithNames) {
+  Archive archive(GetArchiveFileName().c_str());
+  ASSERT_TRUE(archive.IsReady());
 
-    size_t count = 50;
+  size_t count = 8;
 
-    std::vector<Archivable::ArchiveName> keys;
-    std::vector<uint64_t> values;
+  std::vector<Archivable::ArchiveName> keys;
+  std::vector<uint64_t> values;
 
-    for (size_t i = 0; i < count; i++) {
-      Sample sample(i + 1);
-      keys.push_back(sample.GetArchiveName());
-      values.push_back(sample.GetSomeData());
-      ASSERT_TRUE(archive.Write(sample));
-    }
+  keys.reserve(count);
+  values.reserve(count);
 
-    for (size_t i = 0; i < count; i++) {
-      Sample sample;
-      ASSERT_TRUE(archive.Read(keys[i], sample));
-      ASSERT_EQ(values[i], sample.GetSomeData());
-    }
+  for (size_t i = 0; i < count; i++) {
+    Sample sample(i + 1);
+    keys.push_back(sample.GetArchiveName());
+    values.push_back(sample.GetSomeData());
+    ASSERT_TRUE(archive.Write(sample));
   }
-  ASSERT_EQ(::remove(name), 0);
-}
 
-/*
- *  This shouldn't be slow. Need to cache compiled statements.
- */
-TEST(ArchiveTest, ReadDataWithNames) {
-  auto name = "/tmp/sample6.db";
-  {
-    Archive archive(name, true);
-    ASSERT_TRUE(archive.IsReady());
-
-    size_t count = 8;
-
-    std::vector<Archivable::ArchiveName> keys;
-    std::vector<uint64_t> values;
-
-    keys.reserve(count);
-    values.reserve(count);
-
-    for (size_t i = 0; i < count; i++) {
-      Sample sample(i + 1);
-      keys.push_back(sample.GetArchiveName());
-      values.push_back(sample.GetSomeData());
-      ASSERT_TRUE(archive.Write(sample));
-    }
-
-    for (size_t i = 0; i < count; i++) {
-      Sample sample;
-      ASSERT_TRUE(archive.Read(keys[i], sample));
-      ASSERT_EQ(values[i], sample.GetSomeData());
-      ASSERT_EQ(keys[i], sample.GetArchiveName());
-    }
+  for (size_t i = 0; i < count; i++) {
+    Sample sample;
+    ASSERT_TRUE(archive.Read(keys[i], sample));
+    ASSERT_EQ(values[i], sample.GetSomeData());
+    ASSERT_EQ(keys[i], sample.GetArchiveName());
   }
-  ASSERT_EQ(::remove(name), 0);
 }
 
 }  // namespace testing