bugfix ModelsCollection::map() return type

- updated docs

docs/tinyorm/collections.mdx

@@ -396,19 +396,19 @@ The behavior is the same as for TinyBuilder's [`load`](relationships.mdx#lazy-ea
 
 #### `map()` {#method-map}
 
-The `map` method iterates through the collection and passes each model to the given lambda expression. The lambda expression is free to modify the model and return it, thus forming a new collection of modified models:
+The `map` method iterates through the collection and passes a __copy__ of each model to the given lambda expression. The lambda expression is free to modify the model and return it, thus forming a new collection of modified models:
 
     ModelsCollection<User> users {
         {{"name", "John"}, {"votes", 200}},
         {{"name", "Jack"}, {"votes", 400}},
     };
 
-    auto usersAdded = users.map([](User *const user)
+    auto usersAdded = users.map([](User &&userCopy)
     {
-        if (user->getAttribute<QString>("name") == "John")
-            (*user)["votes"] = user->getAttribute<quint64>("votes") + 1;
+        if (userCopy.getAttribute<QString>("name") == "John")
+            userCopy["votes"] = userCopy.getAttribute<quint64>("votes") + 1;
 
-        return user;
+        return std::move(userCopy);
     });
 
     /*
@@ -420,12 +420,12 @@ The `map` method iterates through the collection and passes each model to the gi
 
 The second `map` overload allows to return the `QVector<T>`:
 
-    QVector<quint64> usersAdded = users.map<quint64>([](User *const user)
+    QVector<quint64> usersAdded = users.map<quint64>([](User &&userCopy)
     {
-        const auto votesRef = (*user)["votes"];
+        const auto votesRef = userCopy["votes"];
 
-        if (user->getAttribute<QString>("name") == "John")
-             votesRef = user->getAttribute<quint64>("votes") + 1;
+        if (userCopy.getAttribute<QString>("name") == "John")
+             votesRef = userCopy.getAttribute<quint64>("votes") + 1;
 
         return votesRef->value<quint64>();
     });
@@ -438,6 +438,10 @@ Both overloads allow to pass the lambda expression with two arguments, whereas t
 Like most other collection methods, `map` returns a new collection instance; it does not modify the collection it is called on. If you want to modify the original collection in place, use the [`each`](#method-each) method.
 :::
 
+:::info
+The model copy is passed to the lambda expression even if the `map` iterates over a collection of model pointers `ModelsCollection<Model *>`. The models are dereferenced behind the scene.
+:::
+
 #### `mapWithKeys()` {#method-mapwithkeys}
 
 The `mapWithKeys` method iterates through the collection and passes each model to the given lambda expression. It returns the `std::unordered_map<K, V>` and the lambda expression should return the `std::pair<K, V>` containing a single column / value pair:

include/orm/tiny/types/modelscollection.hpp

@@ -194,18 +194,20 @@ namespace Types
         QVector<T> modelKeys() const;
 
         /*! Run a map over each of the models. */
-        ModelsCollection<ModelRawType *>
-        map(const std::function<ModelRawType *(ModelRawType *, size_type)> &callback);
+        ModelsCollection<ModelRawType>
+        map(const std::function<ModelRawType(ModelRawType &&modelCopy,
+                                             size_type)> &callback);
         /*! Run a map over each of the models. */
-        ModelsCollection<ModelRawType *>
-        map(const std::function<ModelRawType *(ModelRawType *)> &callback);
+        ModelsCollection<ModelRawType>
+        map(const std::function<ModelRawType(ModelRawType &&modelCopy)> &callback);
 
         /*! Run a map over each of the models. */
         template<typename T>
-        QVector<T> map(const std::function<T(ModelRawType *, size_type)> &callback);
+        QVector<T> map(const std::function<T(ModelRawType &&modelCopy,
+                                             size_type)> &callback);
         /*! Run a map over each of the models. */
         template<typename T>
-        QVector<T> map(const std::function<T(ModelRawType *)> &callback);
+        QVector<T> map(const std::function<T(ModelRawType &&modelCopy)> &callback);
 
         /*! Run an associative map over each of the models (keyed by primary key). */
         std::unordered_map<KeyType, ModelRawType *> mapWithModelKeys();
@@ -415,6 +417,11 @@ namespace Types
         inline ModelsCollection<ModelRawType *>
         toPointersCollection() const noexcept requires IsPointersCollection;
 
+        /*! Return a model copy. */
+        inline static ModelRawType getModelCopy(const ModelRawType &model);
+        /*! Return a model copy. */
+        inline static ModelRawType getModelCopy(const ModelRawType *model);
+
         /*! Throw if the given operator is not valid for the where() method. */
         static void throwIfInvalidWhereOperator(const QString &comparison);
     };
@@ -814,34 +821,35 @@ namespace Types
     }
 
     template<DerivedCollectionModel Model>
-    ModelsCollection<typename ModelsCollection<Model>::ModelRawType *>
+    ModelsCollection<typename ModelsCollection<Model>::ModelRawType>
     ModelsCollection<Model>::map(
-            const std::function<ModelRawType *(ModelRawType *, size_type)> &callback)
+            const std::function<ModelRawType(ModelRawType &&modelCopy,
+                                             size_type)> &callback)
     {
         const auto size = this->size();
 
-        ModelsCollection<ModelRawType *> result;
+        ModelsCollection<ModelRawType> result;
         result.reserve(size);
 
         for (size_type index = 0; index < size; ++index)
-            result.push_back(std::invoke(callback,
-                                         // Don't handle the nullptr
-                                         toPointer(this->operator[](index)), index));
+            result.push_back(std::invoke(callback, getModelCopy(this->operator[](index)),
+                                         index));
 
         return result;
     }
 
     template<DerivedCollectionModel Model>
-    ModelsCollection<typename ModelsCollection<Model>::ModelRawType *>
+    ModelsCollection<typename ModelsCollection<Model>::ModelRawType>
     ModelsCollection<Model>::map(
-            const std::function<ModelRawType *(ModelRawType *)> &callback)
+            const std::function<ModelRawType(ModelRawType &&modelCopy)> &callback)
     {
-        ModelsCollection<ModelRawType *> result;
-        result.reserve(this->size());
+        const auto size = this->size();
 
-        for (ModelLoopType model : *this)
-                                                   // Don't handle the nullptr
-            result.push_back(std::invoke(callback, toPointer(model)));
+        ModelsCollection<ModelRawType> result;
+        result.reserve(size);
+
+        for (auto &&model : *this)
+            result.push_back(std::invoke(callback, getModelCopy(model)));
 
         return result;
     }
@@ -850,7 +858,7 @@ namespace Types
     template<typename T>
     QVector<T>
     ModelsCollection<Model>::map(
-            const std::function<T(ModelRawType *, size_type)> &callback)
+            const std::function<T(ModelRawType &&modelCopy, size_type)> &callback)
     {
         const auto size = this->size();
 
@@ -858,9 +866,9 @@ namespace Types
         result.reserve(size);
 
         for (size_type index = 0; index < size; ++index)
-            result.emplace_back(std::invoke(callback,
-                                            // Don't handle the nullptr
-                                            toPointer(this->operator[](index)), index));
+            result.emplace_back(
+                        std::invoke(callback, getModelCopy(this->operator[](index)),
+                                    index));
 
         return result;
     }
@@ -868,14 +876,16 @@ namespace Types
     template<DerivedCollectionModel Model>
     template<typename T>
     QVector<T>
-    ModelsCollection<Model>::map(const std::function<T(ModelRawType *)> &callback)
+    ModelsCollection<Model>::map(
+            const std::function<T(ModelRawType &&modelCopy)> &callback)
     {
-        QVector<T> result;
-        result.reserve(this->size());
+        const auto size = this->size();
 
-        for (ModelLoopType model : *this)
-            // Don't handle the nullptr
-            result.emplace_back(std::invoke(callback, toPointer(model)));
+        QVector<T> result;
+        result.reserve(size);
+
+        for (auto &&model : *this)
+            result.emplace_back(std::invoke(callback, getModelCopy(model)));
 
         return result;
     }
@@ -1671,6 +1681,22 @@ namespace Types
         return *this;
     }
 
+    template<DerivedCollectionModel Model>
+    typename ModelsCollection<Model>::ModelRawType
+    ModelsCollection<Model>::getModelCopy(const ModelRawType &model)
+    {
+        return model;
+    }
+
+    template<DerivedCollectionModel Model>
+    typename ModelsCollection<Model>::ModelRawType
+    ModelsCollection<Model>::getModelCopy(const ModelRawType *const model)
+    {
+        Q_CHECK_PTR(model);
+
+        return *model;
+    }
+
     template<DerivedCollectionModel Model>
     void ModelsCollection<Model>::throwIfInvalidWhereOperator(const QString &comparison)
     {

tests/TinyUtils/src/common/collection.hpp

@@ -61,7 +61,7 @@ namespace TestUtils::Common
         template<typename T, typename M>
         static bool
         verifyAttributeValues(
-                const QString &attribute, const ModelsCollection<M *> &actual,
+                const QString &attribute, const ModelsCollection<M> &actual,
                 const std::unordered_set<T> &expected);
 
         /*! Get the value of the model's primary key casted to the Derived::KeyType. */
@@ -115,13 +115,13 @@ namespace TestUtils::Common
 
     template<typename T, typename M>
     bool Collection::verifyAttributeValues(
-            const QString &attribute, const ModelsCollection<M *> &actual,
+            const QString &attribute, const ModelsCollection<M> &actual,
             const std::unordered_set<T> &expected)
     {
         return std::ranges::all_of(actual,
-                                   [&attribute, &expected](M *const model)
+                                   [&attribute, &expected](const M &model)
         {
-            return expected.contains(model->template getAttribute<T>(attribute));
+            return expected.contains(model.template getAttribute<T>(attribute));
         });
     }
 

tests/auto/functional/orm/tiny/collection_models/tst_collection_models.cpp

@@ -570,18 +570,22 @@ void tst_Collection_Models::map() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map([](AlbumImage *const image)
+    const auto result = images.map([](AlbumImage &&imageCopy)
     {
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 3 || id == 6
         )
-            (*image)[NAME] = QStringLiteral("%1_id_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(id);
-        return image;
+            imageCopy[NAME] = QStringLiteral("%1_id_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(id);
+
+        return std::move(imageCopy);
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVERIFY(Common::verifyIds(result, {2, 3, 4, 5, 6}));
     QVERIFY(Common::verifyAttributeValues<QString>(
@@ -597,24 +601,27 @@ void tst_Collection_Models::map_WithIndex() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map([](AlbumImage *const image, const auto index)
+    const auto result = images.map([](AlbumImage &&imageCopy, const auto index)
     {
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 2
         )
-            (*image)[NAME] = QStringLiteral("%1_id_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(id);
+            imageCopy[NAME] = QStringLiteral("%1_id_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(id);
 
         else if (index == 3)
-            (*image)[NAME] = QStringLiteral("%1_index_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(index);
+            imageCopy[NAME] = QStringLiteral("%1_index_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(index);
 
-        return image;
+        return std::move(imageCopy);
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVERIFY(Common::verifyIds(result, {2, 3, 4, 5, 6}));
     QVERIFY(Common::verifyAttributeValues<QString>(
@@ -630,21 +637,24 @@ void tst_Collection_Models::map_CustomReturnType() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map<QString>([](AlbumImage *const image) -> QString
+    const auto result = images.map<QString>([](AlbumImage &&imageCopy) -> QString
     {
-        const auto nameRef = (*image)[NAME];
+        const auto nameRef = imageCopy[NAME];
 
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 3 || id == 6
         )
             nameRef = QStringLiteral("%1_id_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(id);
 
         return nameRef->value<QString>();
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVector<QString> expected {"album2_image1", "album2_image2_id_3", "album2_image3",
                                "album2_image4", "album2_image5_id_6"};
@@ -659,26 +669,29 @@ void tst_Collection_Models::map_CustomReturnType_WithIndex() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map<QString>([](AlbumImage *const image, const auto index)
+    const auto result = images.map<QString>([](AlbumImage &&imageCopy, const auto index)
     {
-        const auto nameRef = (*image)[NAME];
+        const auto nameRef = imageCopy[NAME];
 
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 2
         )
             nameRef = QStringLiteral("%1_id_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(id);
 
         else if (index == 3)
             nameRef = QStringLiteral("%1_index_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(index);
 
         return nameRef->value<QString>();
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVector<QString> expected {"album2_image1_id_2", "album2_image2", "album2_image3",
                                "album2_image4_index_3", "album2_image5"};

tests/auto/functional/orm/tiny/collection_relations/tst_collection_relations.cpp

@@ -751,19 +751,22 @@ void tst_Collection_Relations::map() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map([](AlbumImage *const image)
+    const auto result = images.map([](AlbumImage &&imageCopy)
     {
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 3 || id == 6
         )
-            (*image)[NAME] = QStringLiteral("%1_id_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(id);
-        return image;
+            imageCopy[NAME] = QStringLiteral("%1_id_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(id);
+        return imageCopy;
     });
 
     // Verify
     QCOMPARE(result.size(), 5);
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QVERIFY(Common::verifyIds(result, {2, 3, 4, 5, 6}));
     QVERIFY(Common::verifyAttributeValues<QString>(
                 NAME, result, {"album2_image1", "album2_image2_id_3", "album2_image3",
@@ -784,24 +787,27 @@ void tst_Collection_Relations::map_WithIndex() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map([](AlbumImage *const image, const auto index)
+    const auto result = images.map([](AlbumImage &&imageCopy, const auto index)
     {
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 2
         )
-            (*image)[NAME] = QStringLiteral("%1_id_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(id);
+            imageCopy[NAME] = QStringLiteral("%1_id_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(id);
 
         else if (index == 3)
-            (*image)[NAME] = QStringLiteral("%1_index_%2")
-                             .arg(image->getAttribute<QString>(NAME))
-                             .arg(index);
+            imageCopy[NAME] = QStringLiteral("%1_index_%2")
+                              .arg(imageCopy.getAttribute<QString>(NAME))
+                              .arg(index);
 
-        return image;
+        return imageCopy;
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVERIFY(Common::verifyIds(result, {2, 3, 4, 5, 6}));
     QVERIFY(Common::verifyAttributeValues<QString>(
@@ -823,21 +829,24 @@ void tst_Collection_Relations::map_CustomReturnType() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map<QString>([](AlbumImage *const image)
+    const auto result = images.map<QString>([](AlbumImage &&imageCopy)
     {
-        const auto nameRef = (*image)[NAME];
+        const auto nameRef = imageCopy[NAME];
 
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 3 || id == 6
         )
             nameRef = QStringLiteral("%1_id_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(id);
 
         return nameRef->value<QString>();
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVector<QString> expected {"album2_image1", "album2_image2_id_3", "album2_image3",
                                "album2_image4", "album2_image5_id_6"};
@@ -858,26 +867,29 @@ void tst_Collection_Relations::map_CustomReturnType_WithIndex() const
     QVERIFY(Common::verifyIds(images, {2, 3, 4, 5, 6}));
 
     // Get result
-    const auto result = images.map<QString>([](AlbumImage *const image, const auto index)
+    const auto result = images.map<QString>([](AlbumImage &&imageCopy, const auto index)
     {
-        const auto nameRef = (*image)[NAME];
+        const auto nameRef = imageCopy[NAME];
 
-        if (const auto id = Common::getKeyCasted(image);
+        if (const auto id = Common::getKeyCasted(imageCopy);
             id == 2
         )
             nameRef = QStringLiteral("%1_id_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(id);
 
         else if (index == 3)
             nameRef = QStringLiteral("%1_index_%2")
-                      .arg(image->getAttribute<QString>(NAME))
+                      .arg(imageCopy.getAttribute<QString>(NAME))
                       .arg(index);
 
         return nameRef->value<QString>();
     });
 
     // Verify
+    // It must return a new ModelsCollection (different memory address)
+    QVERIFY(reinterpret_cast<uintptr_t>(&result) !=
+            reinterpret_cast<uintptr_t>(&images));
     QCOMPARE(result.size(), 5);
     QVector<QString> expected {"album2_image1_id_2", "album2_image2", "album2_image3",
                                "album2_image4_index_3", "album2_image5"};