More use of optionals in archivist.

Author: chinmaygarde

impeller/archivist/archive.cc

@@ -25,11 +25,11 @@ bool Archive::IsValid() const {
   return database_->IsValid();
 }
 
-bool Archive::ArchiveInstance(const ArchiveDef& definition,
-                              const Archivable& archivable,
-                              int64_t& lastInsertIDOut) {
+std::optional<int64_t /* row id */> Archive::ArchiveInstance(
+    const ArchiveDef& definition,
+    const Archivable& archivable) {
   if (!IsValid()) {
-    return false;
+    return std::nullopt;
   }
 
   auto transaction = database_->CreateTransaction(transaction_count_);
@@ -38,7 +38,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
       database_->GetRegistrationForDefinition(definition);
 
   if (registration == nullptr) {
-    return false;
+    return std::nullopt;
   }
 
   auto statement = registration->CreateInsertStatement();
@@ -47,7 +47,7 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
     /*
      *  Must be able to reset the statement for a new write
      */
-    return false;
+    return std::nullopt;
   }
 
   auto primary_key = archivable.GetArchivePrimaryKey();
@@ -66,32 +66,30 @@ bool Archive::ArchiveInstance(const ArchiveDef& definition,
   if (!definition.auto_key &&
       !statement.WriteValue(ArchiveClassRegistration::kPrimaryKeyIndex,
                             primary_key)) {
-    return false;
+    return std::nullopt;
   }
 
   if (!archivable.Write(item)) {
-    return false;
+    return std::nullopt;
   }
 
   if (statement.Execute() != ArchiveStatement::Result::kDone) {
-    return false;
+    return std::nullopt;
   }
 
   int64_t lastInsert = database_->GetLastInsertRowID();
 
   if (!definition.auto_key && lastInsert != static_cast<int64_t>(primary_key)) {
-    return false;
+    return std::nullopt;
   }
 
-  lastInsertIDOut = lastInsert;
-
   /*
    *  If any of the nested calls fail, we would have already checked for the
    *  failure and returned.
    */
   transaction.MarkWritesAsReadyForCommit();
 
-  return true;
+  return lastInsert;
 }
 
 bool Archive::UnarchiveInstance(const ArchiveDef& definition,
@@ -107,7 +105,7 @@ bool Archive::UnarchiveInstance(const ArchiveDef& definition,
 
 size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
                                    Archive::UnarchiveStep stepper,
-                                   Archivable::ArchiveName name) {
+                                   std::optional<int64_t> primary_key) {
   if (!IsValid()) {
     return 0;
   }
@@ -119,7 +117,7 @@ size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
     return 0;
   }
 
-  const bool isQueryingSingle = name != ArchiveNameAuto;
+  const bool isQueryingSingle = primary_key.has_value();
 
   auto statement = registration->CreateQueryStatement(isQueryingSingle);
 
@@ -129,11 +127,11 @@ size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
 
   if (isQueryingSingle) {
     /*
-     *  If a single statement is being queried for, bind the name as a statement
-     *  argument.
+     *  If a single statement is being queried for, bind the primary key as a
+     * statement argument.
      */
     if (!statement.WriteValue(ArchiveClassRegistration::kPrimaryKeyIndex,
-                              name)) {
+                              primary_key.value())) {
       return 0;
     }
   }
@@ -157,7 +155,7 @@ size_t Archive::UnarchiveInstances(const ArchiveDef& definition,
     /*
      *  Prepare a fresh archive item for the given statement
      */
-    ArchiveLocation item(*this, statement, *registration, name);
+    ArchiveLocation item(*this, statement, *registration, primary_key);
 
     if (!stepper(item)) {
       break;

impeller/archivist/archive.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <type_traits>
 #include <vector>
@@ -17,8 +18,6 @@ namespace impeller {
 class ArchiveLocation;
 class ArchiveDatabase;
 
-static const Archivable::ArchiveName ArchiveNameAuto = 0;
-
 class Archive {
  public:
   Archive(const std::string& path);
@@ -31,8 +30,7 @@ class Archive {
             class = std::enable_if<std::is_base_of<Archivable, T>::value>>
   bool Write(const T& archivable) {
     const ArchiveDef& def = T::ArchiveDefinition;
-    int64_t unused_last = 0;
-    return ArchiveInstance(def, archivable, unused_last);
+    return ArchiveInstance(def, archivable).has_value();
   }
 
   template <class T,
@@ -42,13 +40,13 @@ class Archive {
     return UnarchiveInstance(def, name, archivable);
   }
 
-  using UnarchiveStep = std::function<bool /*continue*/ (ArchiveLocation&)>;
+  using UnarchiveStep = std::function<bool(ArchiveLocation&)>;
 
   template <class T,
             class = std::enable_if<std::is_base_of<Archivable, T>::value>>
   size_t Read(UnarchiveStep stepper) {
     const ArchiveDef& def = T::ArchiveDefinition;
-    return UnarchiveInstances(def, stepper, ArchiveNameAuto);
+    return UnarchiveInstances(def, stepper);
   }
 
  private:
@@ -57,15 +55,17 @@ class Archive {
 
   friend class ArchiveLocation;
 
-  bool ArchiveInstance(const ArchiveDef& definition,
-                       const Archivable& archivable,
-                       int64_t& lastInsertID);
+  std::optional<int64_t /* row id */> ArchiveInstance(
+      const ArchiveDef& definition,
+      const Archivable& archivable);
+
   bool UnarchiveInstance(const ArchiveDef& definition,
                          Archivable::ArchiveName name,
                          Archivable& archivable);
+
   size_t UnarchiveInstances(const ArchiveDef& definition,
                             UnarchiveStep stepper,
-                            Archivable::ArchiveName optionalName);
+                            std::optional<int64_t> primary_key = std::nullopt);
 
   FML_DISALLOW_COPY_AND_ASSIGN(Archive);
 };

impeller/archivist/archive_class_registration.cc

@@ -49,24 +49,24 @@ bool ArchiveClassRegistration::IsValid() const {
   return is_ready_;
 }
 
-ArchiveClassRegistration::ColumnResult ArchiveClassRegistration::FindColumn(
+std::optional<size_t> ArchiveClassRegistration::FindColumnIndex(
     const std::string& className,
     ArchiveDef::Member member) const {
   auto found = class_map_.find(className);
 
   if (found == class_map_.end()) {
-    return {0, false};
+    return std::nullopt;
   }
 
   const auto& memberToColumns = found->second;
 
   auto foundColumn = memberToColumns.find(member);
 
   if (foundColumn == memberToColumns.end()) {
-    return {0, false};
+    return std::nullopt;
   }
 
-  return {foundColumn->second, true};
+  return foundColumn->second;
 }
 
 bool ArchiveClassRegistration::CreateTable(bool autoIncrement) {

impeller/archivist/archive_class_registration.h

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <map>
+#include <optional>
 
 #include "flutter/fml/macros.h"
 #include "impeller/archivist/archive.h"
@@ -16,9 +17,8 @@ class ArchiveClassRegistration {
 
   bool IsValid() const;
 
-  using ColumnResult = std::pair<size_t, bool>;
-  ColumnResult FindColumn(const std::string& className,
-                          ArchiveDef::Member member) const;
+  std::optional<size_t> FindColumnIndex(const std::string& className,
+                                        ArchiveDef::Member member) const;
 
   const std::string& GetClassName() const;
 

impeller/archivist/archive_location.cc

@@ -12,44 +12,44 @@ namespace impeller {
 ArchiveLocation::ArchiveLocation(Archive& context,
                                  ArchiveStatement& statement,
                                  const ArchiveClassRegistration& registration,
-                                 Archivable::ArchiveName name)
+                                 std::optional<int64_t> name)
     : context_(context),
       statement_(statement),
       registration_(registration),
       name_(name),
       current_class_(registration.GetClassName()) {}
 
 Archivable::ArchiveName ArchiveLocation::GetPrimaryKey() const {
-  return name_;
+  return name_.value_or(0u);
 }
 
 bool ArchiveLocation::Write(ArchiveDef::Member member,
                             const std::string& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.WriteValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.WriteValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::WriteIntegral(ArchiveDef::Member member, int64_t item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.WriteValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.WriteValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Write(ArchiveDef::Member member, double item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.WriteValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.WriteValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Write(ArchiveDef::Member member, const Allocation& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.WriteValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.WriteValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Write(ArchiveDef::Member member,
                             const ArchiveDef& otherDef,
                             const Archivable& other) {
-  auto found = registration_.FindColumn(current_class_, member);
+  auto index = registration_.FindColumnIndex(current_class_, member);
 
-  if (!found.second) {
+  if (!index.has_value()) {
     return false;
   }
 
@@ -58,86 +58,76 @@ bool ArchiveLocation::Write(ArchiveDef::Member member,
    *  have a name that is auto assigned. In that case, we cannot ask it before
    *  archival (via `other.archiveName()`).
    */
-  int64_t lastInsert = 0;
-  if (!context_.ArchiveInstance(otherDef, other, lastInsert)) {
+  auto row_id = context_.ArchiveInstance(otherDef, other);
+  if (!row_id.has_value()) {
     return false;
   }
 
   /*
    *  Bind the name of the serializable
    */
-  if (!statement_.WriteValue(found.first, lastInsert)) {
+  if (!statement_.WriteValue(index.value(), row_id.value())) {
     return false;
   }
 
   return true;
 }
 
-std::pair<bool, int64_t> ArchiveLocation::WriteVectorKeys(
+std::optional<int64_t> ArchiveLocation::WriteVectorKeys(
     std::vector<int64_t>&& members) {
   ArchiveVector vector(std::move(members));
-  int64_t vectorID = 0;
-  if (!context_.ArchiveInstance(ArchiveVector::ArchiveDefinition,  //
-                                vector,                            //
-                                vectorID)) {
-    return {false, 0};
-  }
-  return {true, vectorID};
+  return context_.ArchiveInstance(ArchiveVector::ArchiveDefinition, vector);
 }
 
 bool ArchiveLocation::ReadVectorKeys(Archivable::ArchiveName name,
                                      std::vector<int64_t>& members) {
   ArchiveVector vector;
-
   if (!context_.UnarchiveInstance(ArchiveVector::ArchiveDefinition, name,
                                   vector)) {
     return false;
   }
-
   const auto& keys = vector.GetKeys();
-
   std::move(keys.begin(), keys.end(), std::back_inserter(members));
-
   return true;
 }
 
 bool ArchiveLocation::Read(ArchiveDef::Member member, std::string& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.ReadValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.ReadValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::ReadIntegral(ArchiveDef::Member member, int64_t& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.ReadValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.ReadValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Read(ArchiveDef::Member member, double& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.ReadValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.ReadValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Read(ArchiveDef::Member member, Allocation& item) {
-  auto found = registration_.FindColumn(current_class_, member);
-  return found.second ? statement_.ReadValue(found.first, item) : false;
+  auto index = registration_.FindColumnIndex(current_class_, member);
+  return index.has_value() ? statement_.ReadValue(index.value(), item) : false;
 }
 
 bool ArchiveLocation::Read(ArchiveDef::Member member,
                            const ArchiveDef& otherDef,
                            Archivable& other) {
-  auto found = registration_.FindColumn(current_class_, member);
+  auto index = registration_.FindColumnIndex(current_class_, member);
 
   /*
    *  Make sure a member is present at that column
    */
-  if (!found.second) {
+  if (!index.has_value()) {
     return false;
   }
 
   /*
    *  Try to find the foreign key in the current items row
    */
   int64_t foreignKey = 0;
-  if (!statement_.ReadValue(found.first, foreignKey)) {
+  if (!statement_.ReadValue(index.value(), foreignKey)) {
     return false;
   }