Clearer names for primary keys.

egramond · Apr 27, 2022 · 547fc00 · 547fc00
1 parent 89589cc
commit 547fc00
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 26 deletions.
diff --git a/impeller/archivist/archivable.h b/impeller/archivist/archivable.h
@@ -30,7 +30,7 @@ class Archivable {
  public:
   using ArchiveName = uint64_t;
 
-  virtual ArchiveName GetArchiveName() const = 0;
+  virtual ArchiveName GetArchivePrimaryKey() const = 0;
 
   virtual bool Write(ArchiveLocation& item) const = 0;
 

diff --git a/impeller/archivist/archive.cc b/impeller/archivist/archive.cc
@@ -43,7 +43,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
     return false;
   }
 
-  auto statement = registration->GetInsertStatement();
+  auto statement = registration->CreateInsertStatement();
 
   if (!statement.IsValid() || !statement.Reset()) {
     /*
@@ -52,21 +52,22 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
     return false;
   }
 
-  auto itemName = archivable.GetArchiveName();
+  auto primary_key = archivable.GetArchivePrimaryKey();
 
   /*
    *  The lifecycle of the archive item is tied to this scope and there is no
    *  way for the user to create an instance of an archive item. So its safe
    *  for its members to be references. It does not manage the lifetimes of
    *  anything.
    */
-  ArchiveLocation item(*this, statement, *registration, itemName);
+  ArchiveLocation item(*this, statement, *registration, primary_key);
 
   /*
    *  We need to bind the primary key only if the item does not provide its own
    */
   if (!definition.auto_key &&
-      !statement.WriteValue(ArchiveClassRegistration::NameIndex, itemName)) {
+      !statement.WriteValue(ArchiveClassRegistration::kPrimaryKeyIndex,
+                            primary_key)) {
     return false;
   }
 
@@ -80,7 +81,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
 
   int64_t lastInsert = database_->GetLastInsertRowID();
 
-  if (!definition.auto_key && lastInsert != static_cast<int64_t>(itemName)) {
+  if (!definition.auto_key && lastInsert != static_cast<int64_t>(primary_key)) {
     return false;
   }
 
@@ -122,7 +123,7 @@ size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
 
   const bool isQueryingSingle = name != ArchiveNameAuto;
 
-  auto statement = registration->GetQueryStatement(isQueryingSingle);
+  auto statement = registration->CreateQueryStatement(isQueryingSingle);
 
   if (!statement.IsValid() || !statement.Reset()) {
     return 0;
@@ -133,7 +134,8 @@ size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
      *  If a single statement is being queried for, bind the name as a statement
      *  argument.
      */
-    if (!statement.WriteValue(ArchiveClassRegistration::NameIndex, name)) {
+    if (!statement.WriteValue(ArchiveClassRegistration::kPrimaryKeyIndex,
+                              name)) {
       return 0;
     }
   }
@@ -181,7 +183,7 @@ ArchiveLocation::ArchiveLocation(Archive& context,
       name_(name),
       current_class_(registration.GetClassName()) {}
 
-Archivable::ArchiveName ArchiveLocation::Name() const {
+Archivable::ArchiveName ArchiveLocation::GetPrimaryKey() const {
   return name_;
 }
 
@@ -226,7 +228,7 @@ bool ArchiveLocation::Write(ArchiveDef::Member member,
   }
 
   /*
-   *  Bind the name of the serialiable
+   *  Bind the name of the serializable
    */
   if (!statement_.WriteValue(found.first, lastInsert)) {
     return false;

diff --git a/impeller/archivist/archive_class_registration.cc b/impeller/archivist/archive_class_registration.cc
@@ -109,7 +109,7 @@ bool ArchiveClassRegistration::CreateTable(bool autoIncrement) {
   return statement.Execute() == ArchiveStatement::Result::kDone;
 }
 
-ArchiveStatement ArchiveClassRegistration::GetQueryStatement(
+ArchiveStatement ArchiveClassRegistration::CreateQueryStatement(
     bool single) const {
   std::stringstream stream;
   stream << "SELECT " << ArchivePrimaryKeyColumnName << ", ";
@@ -132,7 +132,7 @@ ArchiveStatement ArchiveClassRegistration::GetQueryStatement(
   return database_.CreateStatement(stream.str());
 }
 
-ArchiveStatement ArchiveClassRegistration::GetInsertStatement() const {
+ArchiveStatement ArchiveClassRegistration::CreateInsertStatement() const {
   std::stringstream stream;
   stream << "INSERT OR REPLACE INTO " << class_name_ << " VALUES ( ?, ";
   for (size_t i = 0; i < member_count_; i++) {

diff --git a/impeller/archivist/archive_class_registration.h b/impeller/archivist/archive_class_registration.h
@@ -12,6 +12,10 @@ namespace impeller {
 
 class ArchiveClassRegistration {
  public:
+  static constexpr size_t kPrimaryKeyIndex = 0u;
+
+  bool IsValid() const;
+
   using ColumnResult = std::pair<size_t, bool>;
   ColumnResult FindColumn(const std::string& className,
                           ArchiveDef::Member member) const;
@@ -20,13 +24,9 @@ class ArchiveClassRegistration {
 
   size_t GetMemberCount() const;
 
-  bool IsValid() const;
-
-  ArchiveStatement GetInsertStatement() const;
-
-  ArchiveStatement GetQueryStatement(bool single) const;
+  ArchiveStatement CreateInsertStatement() const;
 
-  static const size_t NameIndex = 0;
+  ArchiveStatement CreateQueryStatement(bool single) const;
 
  private:
   using MemberColumnMap = std::map<ArchiveDef::Member, size_t>;

diff --git a/impeller/archivist/archive_location.h b/impeller/archivist/archive_location.h
@@ -159,7 +159,7 @@ class ArchiveLocation {
     return success;
   }
 
-  Archivable::ArchiveName Name() const;
+  Archivable::ArchiveName GetPrimaryKey() const;
 
  private:
   Archive& context_;

diff --git a/impeller/archivist/archive_vector.cc b/impeller/archivist/archive_vector.cc
@@ -22,7 +22,7 @@ const ArchiveDef ArchiveVector::ArchiveDefinition = {
     /* .members = */ {0},
 };
 
-Archivable::ArchiveName ArchiveVector::GetArchiveName() const {
+Archivable::ArchiveName ArchiveVector::GetArchivePrimaryKey() const {
   return ArchiveNameAuto;
 }
 

diff --git a/impeller/archivist/archive_vector.h b/impeller/archivist/archive_vector.h
@@ -13,7 +13,7 @@ class ArchiveVector : public Archivable {
  public:
   static const ArchiveDef ArchiveDefinition;
 
-  ArchiveName GetArchiveName() const override;
+  ArchiveName GetArchivePrimaryKey() const override;
 
   const std::vector<int64_t> GetKeys() const;
 

diff --git a/impeller/archivist/archivist_unittests.cc b/impeller/archivist/archivist_unittests.cc
@@ -23,7 +23,7 @@ class Sample : public Archivable {
   uint64_t GetSomeData() const { return some_data_; }
 
   // |Archivable|
-  ArchiveName GetArchiveName() const override { return name_; }
+  ArchiveName GetArchivePrimaryKey() const override { return name_; }
 
   // |Archivable|
   bool Write(ArchiveLocation& item) const override {
@@ -32,7 +32,7 @@ class Sample : public Archivable {
 
   // |Archivable|
   bool Read(ArchiveLocation& item) override {
-    name_ = item.Name();
+    name_ = item.GetPrimaryKey();
     return item.Read(999, some_data_);
   };
 
@@ -92,7 +92,7 @@ TEST_F(ArchiveTest, ReadData) {
 
   for (size_t i = 0; i < count; i++) {
     Sample sample(i + 1);
-    keys.push_back(sample.GetArchiveName());
+    keys.push_back(sample.GetArchivePrimaryKey());
     values.push_back(sample.GetSomeData());
     ASSERT_TRUE(archive.Write(sample));
   }
@@ -118,7 +118,7 @@ TEST_F(ArchiveTest, ReadDataWithNames) {
 
   for (size_t i = 0; i < count; i++) {
     Sample sample(i + 1);
-    keys.push_back(sample.GetArchiveName());
+    keys.push_back(sample.GetArchivePrimaryKey());
     values.push_back(sample.GetSomeData());
     ASSERT_TRUE(archive.Write(sample));
   }
@@ -127,7 +127,7 @@ TEST_F(ArchiveTest, ReadDataWithNames) {
     Sample sample;
     ASSERT_TRUE(archive.Read(keys[i], sample));
     ASSERT_EQ(values[i], sample.GetSomeData());
-    ASSERT_EQ(keys[i], sample.GetArchiveName());
+    ASSERT_EQ(keys[i], sample.GetArchivePrimaryKey());
   }
 }