diff --git a/impeller/archivist/archivable.h b/impeller/archivist/archivable.h index 4a85166e20f44..783c138920471 100644 --- a/impeller/archivist/archivable.h +++ b/impeller/archivist/archivable.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include @@ -22,15 +23,15 @@ struct ArchiveDef { class ArchiveLocation; +using PrimaryKey = std::optional; + //------------------------------------------------------------------------------ /// @brief Instances of `Archivable`s can be read from and written to a /// persistent archive. /// class Archivable { public: - using ArchiveName = uint64_t; - - virtual ArchiveName GetArchivePrimaryKey() const = 0; + virtual PrimaryKey GetPrimaryKey() const = 0; virtual bool Write(ArchiveLocation& item) const = 0; diff --git a/impeller/archivist/archive.cc b/impeller/archivist/archive.cc index b6c6a9069eea3..c10646e081e3d 100644 --- a/impeller/archivist/archive.cc +++ b/impeller/archivist/archive.cc @@ -10,6 +10,7 @@ #include "impeller/archivist/archive_class_registration.h" #include "impeller/archivist/archive_database.h" #include "impeller/archivist/archive_location.h" +#include "impeller/base/validation.h" namespace impeller { @@ -50,7 +51,13 @@ std::optional Archive::ArchiveInstance( return std::nullopt; } - auto primary_key = archivable.GetArchivePrimaryKey(); + auto primary_key = archivable.GetPrimaryKey(); + + if (!definition.auto_key && !primary_key.has_value()) { + VALIDATION_LOG << "Archive definition specified that primary keys will be " + "explicitly specified but none was provided when asked."; + return std::nullopt; + } /* * The lifecycle of the archive item is tied to this scope and there is no @@ -65,7 +72,7 @@ std::optional Archive::ArchiveInstance( */ if (!definition.auto_key && !statement.WriteValue(ArchiveClassRegistration::kPrimaryKeyIndex, - primary_key)) { + primary_key.value())) { return std::nullopt; } @@ -79,7 +86,8 @@ std::optional Archive::ArchiveInstance( int64_t lastInsert = database_->GetLastInsertRowID(); - if (!definition.auto_key && lastInsert != static_cast(primary_key)) { + if (!definition.auto_key && + lastInsert != static_cast(primary_key.value())) { return std::nullopt; } @@ -93,7 +101,7 @@ std::optional Archive::ArchiveInstance( } bool Archive::UnarchiveInstance(const ArchiveDef& definition, - Archivable::ArchiveName name, + PrimaryKey name, Archivable& archivable) { UnarchiveStep stepper = [&archivable](ArchiveLocation& item) { archivable.Read(item); @@ -105,7 +113,7 @@ bool Archive::UnarchiveInstance(const ArchiveDef& definition, size_t Archive::UnarchiveInstances(const ArchiveDef& definition, Archive::UnarchiveStep stepper, - std::optional primary_key) { + PrimaryKey primary_key) { if (!IsValid()) { return 0; } diff --git a/impeller/archivist/archive.h b/impeller/archivist/archive.h index 3927159abd059..18d7b9ad1cd66 100644 --- a/impeller/archivist/archive.h +++ b/impeller/archivist/archive.h @@ -35,7 +35,7 @@ class Archive { template ::value>> - bool Read(Archivable::ArchiveName name, T& archivable) { + bool Read(PrimaryKey name, T& archivable) { const ArchiveDef& def = T::ArchiveDefinition; return UnarchiveInstance(def, name, archivable); } @@ -60,12 +60,12 @@ class Archive { const Archivable& archivable); bool UnarchiveInstance(const ArchiveDef& definition, - Archivable::ArchiveName name, + PrimaryKey name, Archivable& archivable); size_t UnarchiveInstances(const ArchiveDef& definition, UnarchiveStep stepper, - std::optional primary_key = std::nullopt); + PrimaryKey primary_key = std::nullopt); FML_DISALLOW_COPY_AND_ASSIGN(Archive); }; diff --git a/impeller/archivist/archive_location.cc b/impeller/archivist/archive_location.cc index 382863848550e..85080f9bab458 100644 --- a/impeller/archivist/archive_location.cc +++ b/impeller/archivist/archive_location.cc @@ -12,15 +12,15 @@ namespace impeller { ArchiveLocation::ArchiveLocation(Archive& context, ArchiveStatement& statement, const ArchiveClassRegistration& registration, - std::optional name) + PrimaryKey name) : context_(context), statement_(statement), registration_(registration), name_(name), current_class_(registration.GetClassName()) {} -Archivable::ArchiveName ArchiveLocation::GetPrimaryKey() const { - return name_.value_or(0u); +PrimaryKey ArchiveLocation::GetPrimaryKey() const { + return name_; } bool ArchiveLocation::Write(ArchiveDef::Member member, @@ -79,7 +79,7 @@ std::optional ArchiveLocation::WriteVectorKeys( return context_.ArchiveInstance(ArchiveVector::ArchiveDefinition, vector); } -bool ArchiveLocation::ReadVectorKeys(Archivable::ArchiveName name, +bool ArchiveLocation::ReadVectorKeys(PrimaryKey name, std::vector& members) { ArchiveVector vector; if (!context_.UnarchiveInstance(ArchiveVector::ArchiveDefinition, name, diff --git a/impeller/archivist/archive_location.h b/impeller/archivist/archive_location.h index 598254b45ca3f..2afdb4faa1f25 100644 --- a/impeller/archivist/archive_location.h +++ b/impeller/archivist/archive_location.h @@ -20,7 +20,7 @@ class ArchiveStatement; class ArchiveLocation { public: - Archivable::ArchiveName GetPrimaryKey() const; + PrimaryKey GetPrimaryKey() const; template ::value>> bool Write(ArchiveDef::Member member, T item) { @@ -166,7 +166,7 @@ class ArchiveLocation { Archive& context_; ArchiveStatement& statement_; const ArchiveClassRegistration& registration_; - std::optional name_; + PrimaryKey name_; std::string current_class_; friend class Archive; @@ -174,7 +174,7 @@ class ArchiveLocation { ArchiveLocation(Archive& context, ArchiveStatement& statement, const ArchiveClassRegistration& registration, - std::optional name); + PrimaryKey name); bool WriteIntegral(ArchiveDef::Member member, int64_t item); @@ -182,8 +182,7 @@ class ArchiveLocation { std::optional WriteVectorKeys(std::vector&& members); - bool ReadVectorKeys(Archivable::ArchiveName name, - std::vector& members); + bool ReadVectorKeys(PrimaryKey name, std::vector& members); bool Write(ArchiveDef::Member member, const ArchiveDef& otherDef, diff --git a/impeller/archivist/archive_vector.cc b/impeller/archivist/archive_vector.cc index 92117a1bee746..697d93e32b924 100644 --- a/impeller/archivist/archive_vector.cc +++ b/impeller/archivist/archive_vector.cc @@ -17,13 +17,14 @@ ArchiveVector::ArchiveVector() {} const ArchiveDef ArchiveVector::ArchiveDefinition = { /* .superClass = */ nullptr, - /* .className = */ "Meta_Vector", + /* .className = */ "_IPLR_meta_vector_items_", /* .autoAssignName = */ true, /* .members = */ {0}, }; -Archivable::ArchiveName ArchiveVector::GetArchivePrimaryKey() const { - return 0; +PrimaryKey ArchiveVector::GetPrimaryKey() const { + // Archive definition says the keys will be auto assigned. + return std::nullopt; } const std::vector ArchiveVector::GetKeys() const { diff --git a/impeller/archivist/archive_vector.h b/impeller/archivist/archive_vector.h index 303fb1d9fe468..e7fd59bd156c9 100644 --- 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 GetArchivePrimaryKey() const override; + PrimaryKey GetPrimaryKey() const override; const std::vector GetKeys() const; diff --git a/impeller/archivist/archivist_unittests.cc b/impeller/archivist/archivist_unittests.cc index 241c3fb64513c..5e8d17a5b6106 100644 --- a/impeller/archivist/archivist_unittests.cc +++ b/impeller/archivist/archivist_unittests.cc @@ -14,7 +14,7 @@ namespace impeller { namespace testing { -static Archivable::ArchiveName LastSample = 0; +static int64_t LastSample = 0; class Sample : public Archivable { public: @@ -23,7 +23,7 @@ class Sample : public Archivable { uint64_t GetSomeData() const { return some_data_; } // |Archivable| - ArchiveName GetArchivePrimaryKey() const override { return name_; } + PrimaryKey GetPrimaryKey() const override { return name_; } // |Archivable| bool Write(ArchiveLocation& item) const override { @@ -40,7 +40,7 @@ class Sample : public Archivable { private: uint64_t some_data_; - ArchiveName name_ = ++LastSample; + PrimaryKey name_ = ++LastSample; FML_DISALLOW_COPY_AND_ASSIGN(Sample); }; @@ -87,12 +87,12 @@ TEST_F(ArchiveTest, ReadData) { size_t count = 50; - std::vector keys; + std::vector keys; std::vector values; for (size_t i = 0; i < count; i++) { Sample sample(i + 1); - keys.push_back(sample.GetArchivePrimaryKey()); + keys.push_back(sample.GetPrimaryKey().value()); values.push_back(sample.GetSomeData()); ASSERT_TRUE(archive.Write(sample)); } @@ -110,7 +110,7 @@ TEST_F(ArchiveTest, ReadDataWithNames) { size_t count = 8; - std::vector keys; + std::vector keys; std::vector values; keys.reserve(count); @@ -118,7 +118,7 @@ TEST_F(ArchiveTest, ReadDataWithNames) { for (size_t i = 0; i < count; i++) { Sample sample(i + 1); - keys.push_back(sample.GetArchivePrimaryKey()); + keys.push_back(sample.GetPrimaryKey().value()); 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.GetArchivePrimaryKey()); + ASSERT_EQ(keys[i], sample.GetPrimaryKey()); } }