Merge pull request #449 from LASTRADA-Software/improvement/hasmany_tests_and_fixes

[DataMapper] HasMany fixes and tests

Author: Yaraslaut

src/Lightweight/DataMapper/DataMapper.hpp

@@ -117,7 +117,7 @@ class HasMany
     struct Loader
     {
         std::function<size_t()> count {};
-        std::function<void()> all {};
+        std::function<ReferencedRecordList()> all {};
         std::function<void(std::function<void(ReferencedRecord const&)>)> each {};
 
         std::weak_ordering operator<=>(Loader const& /*other*/) const noexcept
@@ -150,7 +150,7 @@ template <typename OtherRecord>
 inline LIGHTWEIGHT_FORCE_INLINE void HasMany<OtherRecord>::RequireLoaded()
 {
     if (!_records)
-        _loader.all();
+        _records = _loader.all();
 }
 
 template <typename OtherRecord>
@@ -239,28 +239,40 @@ template <typename OtherRecord>
 inline LIGHTWEIGHT_FORCE_INLINE HasMany<OtherRecord>::iterator HasMany<OtherRecord>::begin() noexcept
 {
     RequireLoaded();
-    return _records->begin();
+    if (_records)
+        return _records->begin();
+    else
+        return iterator {};
 }
 
 template <typename OtherRecord>
 inline LIGHTWEIGHT_FORCE_INLINE HasMany<OtherRecord>::iterator HasMany<OtherRecord>::end() noexcept
 {
     RequireLoaded();
-    return _records->end();
+    if (_records)
+        return _records->end();
+    else
+        return iterator {};
 }
 
 template <typename OtherRecord>
 inline LIGHTWEIGHT_FORCE_INLINE HasMany<OtherRecord>::const_iterator HasMany<OtherRecord>::begin() const noexcept
 {
     RequireLoaded();
-    return _records->begin();
+    if (_records)
+        return _records->begin();
+    else
+        return const_iterator {};
 }
 
 template <typename OtherRecord>
 inline LIGHTWEIGHT_FORCE_INLINE HasMany<OtherRecord>::const_iterator HasMany<OtherRecord>::end() const noexcept
 {
     RequireLoaded();
-    return _records->end();
+    if (_records)
+        return _records->end();
+    else
+        return const_iterator {};
 }
 
 } // namespace Lightweight

src/Lightweight/DataMapper/HasMany.hpp

@@ -97,7 +97,7 @@ class HasManyThrough
     struct Loader
     {
         std::function<size_t()> count;
-        std::function<void()> all;
+        std::function<ReferencedRecordList()> all;
         std::function<void(std::function<void(ReferencedRecord const&)>)> each;
     };
 
@@ -140,7 +140,7 @@ class HasManyThrough
             return;
 
         if (_loader.all)
-            _loader.all();
+            _records = _loader.all();
 
         if (!_records)
             throw SqlRequireLoadedError(Reflection::TypeNameOf<std::remove_cvref_t<decltype(*this)>>);

src/Lightweight/DataMapper/HasManyThrough.hpp

@@ -68,7 +68,7 @@ class HasOneThrough
 
     struct Loader
     {
-        std::function<void()> loadReference {};
+        std::function<std::shared_ptr<ReferencedRecord>()> loadReference {};
     };
 
     /// Used internally to configure on-demand loading of the record.
@@ -84,7 +84,7 @@ class HasOneThrough
             return;
 
         if (_loader.loadReference)
-            _loader.loadReference();
+            _record = _loader.loadReference();
 
         if (!IsLoaded())
             throw SqlRequireLoadedError { Reflection::TypeNameOf<std::remove_cvref_t<decltype(*this)>> };
@@ -93,7 +93,7 @@ class HasOneThrough
     Loader _loader {};
 
     // We use shared_ptr to not require ReferencedRecord to be declared before HasOneThrough.
-    std::shared_ptr<ReferencedRecord> _record {};
+    mutable std::shared_ptr<ReferencedRecord> _record {};
 };
 
 namespace detail

src/Lightweight/DataMapper/HasOneThrough.hpp

@@ -88,7 +88,7 @@ namespace details
     template <typename Record>
     struct RecordPrimaryKeyTypeHelper
     {
-        using type = void;
+        using type = std::monostate;
     };
 
     template <typename Record>
@@ -182,10 +182,12 @@ inline LIGHTWEIGHT_FORCE_INLINE RecordPrimaryKeyType<Record> GetPrimaryKeyField(
 
     auto result = RecordPrimaryKeyType<Record> {};
     Reflection::EnumerateMembers(record, [&]<size_t I, typename FieldType>(FieldType const& field) {
-        if constexpr (IsPrimaryKey<FieldType> && std::same_as<FieldType, RecordPrimaryKeyType<Record>>)
-        {
-            result = field;
-        }
+        // std::same_as<typename FieldType::ValueType, RecordPrimaryKeyType<Record>>condition is for the case where there are
+        // multiple primary keys, we want to return the first one
+        if constexpr (IsField<FieldType>)
+            if constexpr (IsPrimaryKey<FieldType>)
+                if constexpr (std::same_as<typename FieldType::ValueType, RecordPrimaryKeyType<Record>>)
+                    result = field.Value();
     });
     return result;
 }

src/Lightweight/DataMapper/Record.hpp

@@ -163,6 +163,18 @@ static_assert(RecordPrimaryKeyIndex<PKTestMulti> == 2,
               "Primary key index should be the first primary key field in the record");
 static_assert(std::same_as<RecordPrimaryKeyType<PKTestMulti>, SqlTrimmedFixedString<10>>);
 
+struct NoPKRecord
+{
+    Field<int> fieldA;
+    Field<char> fieldB;
+};
+
+static_assert(HasPrimaryKey<PKTest0>);
+static_assert(HasPrimaryKey<PKTest1>);
+static_assert(HasPrimaryKey<PKTestMulti>);
+static_assert(HasPrimaryKey<Person>);
+static_assert(!HasPrimaryKey<NoPKRecord>);
+
 TEST_CASE_METHOD(SqlTestFixture, "Primary key access", "[DataMapper]")
 {
     PKTest0 pk0;
@@ -178,6 +190,39 @@ TEST_CASE_METHOD(SqlTestFixture, "Primary key access", "[DataMapper]")
     CHECK(pkMulti.pk1.Value() == "World");
 }
 
+TEST_CASE("GetPrimaryKeyField", "[DataMapper][Record]")
+{
+    SECTION("int PK at field index 0")
+    {
+        PKTest0 record;
+        record.pk = 42;
+        CHECK(GetPrimaryKeyField(record) == 42);
+    }
+
+    SECTION("string PK at field index 1")
+    {
+        PKTest1 record;
+        record.pk = "Hello";
+        CHECK(GetPrimaryKeyField(record) == SqlTrimmedFixedString<10> { "Hello" });
+    }
+
+    SECTION("first PK returned when multiple PKs present")
+    {
+        PKTestMulti record;
+        record.pk1 = "World";
+        record.pk2 = 3.14;
+        CHECK(GetPrimaryKeyField(record) == SqlTrimmedFixedString<10> { "World" });
+    }
+
+    SECTION("SqlGuid PK")
+    {
+        auto const guid = SqlGuid::Create();
+        Person record;
+        record.id = guid;
+        CHECK(GetPrimaryKeyField(record) == guid);
+    }
+}
+
 TEST_CASE_METHOD(SqlTestFixture, "MapFromRecordFields", "[DataMapper]")
 {
     auto const person = Person {

src/tests/DataMapper/MiscTests.cpp

@@ -461,7 +461,7 @@ struct SimpleStruct2
 };
 
 static_assert(std::cmp_equal(RecordPrimaryKeyIndex<SimpleStruct2>, static_cast<size_t>(-1)));
-static_assert(std::same_as<RecordPrimaryKeyType<SimpleStruct2>, void>);
+static_assert(std::same_as<RecordPrimaryKeyType<SimpleStruct2>, std::monostate>);
 
 std::ostream& operator<<(std::ostream& os, SimpleStruct2 const& record)
 {