diff --git a/src/core/scriptmodel.cpp b/src/core/scriptmodel.cpp index 7bd930e..f14a212 100644 --- a/src/core/scriptmodel.cpp +++ b/src/core/scriptmodel.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,55 @@ #include #include +namespace { +struct RecursionPair { + const QJSValue* a; + const QJSValue* b; + const RecursionPair* previous; +}; + +bool qjsValueStructuralEq( + const QJSValue& a, + const QJSValue& b, + const RecursionPair* activePair = nullptr +) { + if (a.strictlyEquals(b)) return true; + if (!a.isObject() || !b.isObject() || a.isArray() != b.isArray()) return false; + + for (const auto* pair = activePair; pair != nullptr; pair = pair->previous) { + if (a.strictlyEquals(*pair->a) || b.strictlyEquals(*pair->b)) return false; + } + + const auto nextActivePair = RecursionPair {.a = &a, .b = &b, .previous = activePair}; + + auto aIter = QJSValueIterator(a); + auto bIter = QJSValueIterator(b); + + while (aIter.hasNext()) { + if (!bIter.hasNext()) return false; + + aIter.next(); + bIter.next(); + + const auto aName = aIter.name(); + const auto bName = bIter.name(); + const auto aValue = aIter.value(); + + if (aName == bName) { + const auto bValue = bIter.value(); + if (!qjsValueStructuralEq(aValue, bValue, &nextActivePair)) return false; + } else { + if (!b.hasOwnProperty(aName)) return false; + + const auto bValue = b.property(aName); + if (!qjsValueStructuralEq(aValue, bValue, &nextActivePair)) return false; + } + } + + return !bIter.hasNext(); +} +} // namespace + bool ScriptModel::updateValuesUnique(const QList& newValues) { auto anyChanges = false; @@ -22,14 +72,23 @@ bool ScriptModel::updateValuesUnique(const QList& newValues) { auto iter = this->mValues.begin(); auto newIter = newValues.begin(); - auto getCmpKey = [this](const QJSValue& v) { - if (v.hasProperty(this->cmpKey)) return v.property(this->cmpKey); - else return v; + auto comparisonValue = [this](const QJSValue& value) { + if (value.hasProperty(this->cmpKey)) return value.property(this->cmpKey); + + return value; }; auto valueCmp = [&, this](const QJSValue& a, const QJSValue& b) { - if (!this->cmpKey.isEmpty()) return getCmpKey(a).strictlyEquals(getCmpKey(b)); - return a.strictlyEquals(b); + if (a.strictlyEquals(b)) return true; + + const auto aValue = this->cmpKey.isEmpty() ? a : comparisonValue(a); + const auto bValue = this->cmpKey.isEmpty() ? b : comparisonValue(b); + + if (this->mComparisonMode == ObjectComparison::Identity) { + return aValue.strictlyEquals(bValue); + } else { + return qjsValueStructuralEq(aValue, bValue); + } }; auto eqPredicate = [&](const QJSValue& b) { @@ -187,6 +246,12 @@ void ScriptModel::setObjectProp(const QString& objectProp) { emit this->objectPropChanged(); } +void ScriptModel::setComparisonMode(ObjectComparison::Enum comparisonMode) { + if (comparisonMode == this->mComparisonMode) return; + this->mComparisonMode = comparisonMode; + emit this->comparisonModeChanged(); +} + qint32 ScriptModel::rowCount(const QModelIndex& parent) const { if (parent != QModelIndex()) return 0; return static_cast(this->mValues.length()); diff --git a/src/core/scriptmodel.hpp b/src/core/scriptmodel.hpp index 06758d8..6e60b69 100644 --- a/src/core/scriptmodel.hpp +++ b/src/core/scriptmodel.hpp @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -10,6 +12,21 @@ #include #include +///! @@ScriptModel value comparison mode. +namespace ObjectComparison { // NOLINT +Q_NAMESPACE; +QML_ELEMENT; + +enum Enum : quint8 { + /// Compare values using strict JavaScript equality. + Identity = 0, + /// Compare records and arrays structurally. + Structure = 1, +}; +Q_ENUM_NS(Enum); + +} // namespace ObjectComparison + ///! QML model reflecting a javascript expression /// ScriptModel is a QML [Data Model] that generates model operations based on changes /// to a javascript expression attached to @@values. @@ -44,6 +61,7 @@ /// [Data Model]: https://doc.qt.io/qt-6/qtquick-modelviewsdata-modelview.html#qml-data-models class ScriptModel: public QAbstractListModel { Q_OBJECT; + // clang-format off /// The list of values to reflect in the model. /// > [!WARNING] ScriptModel currently only works with lists of *unique* values. /// > There must not be any duplicates in the given list, or behavior of the model is undefined. @@ -79,6 +97,12 @@ class ScriptModel: public QAbstractListModel { /// /// Defaults to `""`, meaning no key. Q_PROPERTY(QString objectProp READ objectProp WRITE setObjectProp NOTIFY objectPropChanged); + /// How values should be compared. Defaults to `ObjectComparison.Structure`. + /// + /// Identity based comparison is faster if usable for a given model, and often + /// achievable with @@objectProp. + Q_PROPERTY(ObjectComparison::Enum comparisonMode READ comparisonMode WRITE setComparisonMode NOTIFY comparisonModeChanged); + // clang-format on QML_ELEMENT; public: @@ -94,6 +118,9 @@ public: [[nodiscard]] QString objectProp() const { return this->cmpKey; } void setObjectProp(const QString& objectProp); + [[nodiscard]] ObjectComparison::Enum comparisonMode() const { return this->mComparisonMode; } + void setComparisonMode(ObjectComparison::Enum comparisonMode); + [[nodiscard]] qint32 rowCount(const QModelIndex& parent) const override; [[nodiscard]] QVariant data(const QModelIndex& index, qint32 role) const override; [[nodiscard]] QHash roleNames() const override; @@ -101,10 +128,12 @@ public: signals: void valuesChanged(); void objectPropChanged(); + void comparisonModeChanged(); private: QList mValues; QString cmpKey; + ObjectComparison::Enum mComparisonMode = ObjectComparison::Structure; bool isModifying = false; std::optional> stagedValues; diff --git a/src/core/test/scriptmodel.cpp b/src/core/test/scriptmodel.cpp index 6a5c5f1..7b23610 100644 --- a/src/core/test/scriptmodel.cpp +++ b/src/core/test/scriptmodel.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -15,8 +16,6 @@ #include "../scriptmodel.hpp" -using OpList = QList; - bool ModelOperation::operator==(const ModelOperation& other) const { return other.operation == this->operation && other.index == this->index && other.length == this->length && other.destIndex == this->destIndex; @@ -44,9 +43,38 @@ QDebug& operator<<(QDebug& debug, const ModelOperation& op) { return debug; } +ChangeObserver::ChangeObserver(QAbstractItemModel* model) { + QObject::connect(model, &QAbstractItemModel::rowsInserted, this, &ChangeObserver::onInsert); + QObject::connect(model, &QAbstractItemModel::rowsRemoved, this, &ChangeObserver::onRemove); + QObject::connect(model, &QAbstractItemModel::rowsMoved, this, &ChangeObserver::onMove); +} + +void ChangeObserver::onInsert(const QModelIndex& parent, int first, int last) { + QCOMPARE(parent, QModelIndex()); + this->mOperations.emplaceBack(ModelOperation::Insert, first, last - first + 1); +} + +void ChangeObserver::onRemove(const QModelIndex& parent, int first, int last) { + QCOMPARE(parent, QModelIndex()); + this->mOperations.emplaceBack(ModelOperation::Remove, first, last - first + 1); +} + +void ChangeObserver::onMove( + const QModelIndex& sourceParent, + int sourceStart, + int sourceEnd, + const QModelIndex& destParent, + int destStart +) { + QCOMPARE(sourceParent, QModelIndex()); + QCOMPARE(destParent, QModelIndex()); + this->mOperations + .emplaceBack(ModelOperation::Move, sourceStart, sourceEnd - sourceStart + 1, destStart); +} + // NOLINTNEXTLINE(misc-use-internal-linkage) QDebug& operator<<(QDebug& debug, const QJSValueList& list) { - auto str = QString(); + QString str; for (const auto& var: list) { if (var.isString()) { @@ -139,55 +167,117 @@ void TestScriptModel::unique() { return list; }; - auto oldlist = strToJsValueList(oldstr); - auto newlist = strToJsValueList(newstr); + auto oldList = strToJsValueList(oldstr); + auto newList = strToJsValueList(newstr); - auto model = ScriptModel(); + ScriptModel model; auto modelTester = QAbstractItemModelTester(&model); + auto observer = ChangeObserver(&model); - OpList actualOperations; - - auto onInsert = [&](const QModelIndex& parent, int first, int last) { - QCOMPARE(parent, QModelIndex()); - actualOperations << ModelOperation(ModelOperation::Insert, first, last - first + 1); - }; - - auto onRemove = [&](const QModelIndex& parent, int first, int last) { - QCOMPARE(parent, QModelIndex()); - actualOperations << ModelOperation(ModelOperation::Remove, first, last - first + 1); - }; - - auto onMove = [&](const QModelIndex& sourceParent, - int sourceStart, - int sourceEnd, - const QModelIndex& destParent, - int destStart) { - QCOMPARE(sourceParent, QModelIndex()); - QCOMPARE(destParent, QModelIndex()); - actualOperations << ModelOperation( - ModelOperation::Move, - sourceStart, - sourceEnd - sourceStart + 1, - destStart - ); - }; - - QObject::connect(&model, &QAbstractItemModel::rowsInserted, &model, onInsert); - QObject::connect(&model, &QAbstractItemModel::rowsRemoved, &model, onRemove); - QObject::connect(&model, &QAbstractItemModel::rowsMoved, &model, onMove); - - model.setValues(oldlist); - QVERIFY(qjsValueListsEqual(model.values(), oldlist)); + model.setValues(oldList); + QVERIFY(qjsValueListsEqual(model.values(), oldList)); QCOMPARE_EQ( - actualOperations, - OpList({{ModelOperation::Insert, 0, static_cast(oldlist.length())}}) + observer.operations(), + OpList({{ModelOperation::Insert, 0, static_cast(oldList.length())}}) ); - actualOperations.clear(); + observer.clear(); - model.setValues(newlist); - QVERIFY(qjsValueListsEqual(model.values(), newlist)); - QCOMPARE_EQ(actualOperations, operations); + model.setValues(newList); + QVERIFY(qjsValueListsEqual(model.values(), newList)); + QCOMPARE_EQ(observer.operations(), operations); +} + +void TestScriptModel::structuralEquality() { + ScriptModel model; + QJSEngine engine; + + auto object = engine.newObject(); + object.setProperty("foo", "bar"); + object.setProperty("bar", "baz"); + object.setProperty("subobject", engine.newObject()); + + auto replacement = engine.newObject(); + replacement.setProperty("subobject", engine.newObject()); + replacement.setProperty("bar", "baz"); + replacement.setProperty("foo", "bar"); + + auto oldList = QList {object}; + + auto newList = QList {replacement}; + + auto modelTester = QAbstractItemModelTester(&model); + auto observer = ChangeObserver(&model); + + model.setValues(oldList); + QVERIFY(qjsValueListsEqual(model.values(), oldList)); + QCOMPARE_EQ( + observer.operations(), + OpList({{ModelOperation::Insert, 0, static_cast(oldList.length())}}) + ); + + observer.clear(); + + model.setValues(newList); + QVERIFY(qjsValueListsEqual(model.values(), newList)); + QCOMPARE_EQ(observer.operations(), OpList()); + + auto structurallyEquals = [](const QJSValue& a, const QJSValue& b) { + ScriptModel model; + model.setValues({a}); + + auto observer = ChangeObserver(&model); + model.setValues({b}); + return observer.operations().isEmpty(); + }; + + auto different = engine.newObject(); + different.setProperty("foo", "bar"); + different.setProperty("bar", "baz"); + auto differentSubobject = engine.newObject(); + differentSubobject.setProperty("value", 1); + different.setProperty("subobject", differentSubobject); + QVERIFY(!structurallyEquals(object, different)); + + auto cycleA = engine.newObject(); + cycleA.setProperty("self", cycleA); + auto cycleB = engine.newObject(); + cycleB.setProperty("self", cycleB); + QVERIFY(!structurallyEquals(cycleA, cycleB)); + + const auto arrayA = engine.evaluate("[1, { value: 'x' }]"); + const auto arrayB = engine.evaluate("[1, { value: 'x' }]"); + QVERIFY(structurallyEquals(arrayA, arrayB)); + + auto sparseArrayA = engine.newArray(1); + auto sparseArrayB = engine.newArray(2); + QVERIFY(!structurallyEquals(sparseArrayA, sparseArrayB)); +} + +void TestScriptModel::comparisonModes() { + QJSEngine engine; + auto makeValue = [&engine]() { + auto value = engine.newObject(); + value.setProperty("nested", engine.newObject()); + return value; + }; + + ScriptModel model; + QCOMPARE_EQ(model.comparisonMode(), ObjectComparison::Structure); + + model.setValues({makeValue()}); + model.setComparisonMode(ObjectComparison::Identity); + QCOMPARE_EQ(model.comparisonMode(), ObjectComparison::Identity); + + auto observer = ChangeObserver(&model); + model.setValues({makeValue()}); + QVERIFY(!observer.operations().isEmpty()); + + model.setComparisonMode(ObjectComparison::Structure); + + observer.clear(); + model.setValues({makeValue()}); + QCOMPARE_EQ(observer.operations(), OpList()); } QTEST_MAIN(TestScriptModel); diff --git a/src/core/test/scriptmodel.hpp b/src/core/test/scriptmodel.hpp index 3b50b32..27f4a24 100644 --- a/src/core/test/scriptmodel.hpp +++ b/src/core/test/scriptmodel.hpp @@ -1,6 +1,8 @@ #pragma once +#include #include +#include #include #include #include @@ -26,6 +28,29 @@ struct ModelOperation { [[nodiscard]] bool operator==(const ModelOperation& other) const; }; +using OpList = QList; + +class ChangeObserver: public QObject { +public: + explicit ChangeObserver(QAbstractItemModel* model); + + [[nodiscard]] const OpList& operations() const { return this->mOperations; } + void clear() { this->mOperations.clear(); } + +private: + void onInsert(const QModelIndex& parent, int first, int last); + void onRemove(const QModelIndex& parent, int first, int last); + void onMove( + const QModelIndex& sourceParent, + int sourceStart, + int sourceEnd, + const QModelIndex& destParent, + int destStart + ); + + OpList mOperations; +}; + QDebug& operator<<(QDebug& debug, const ModelOperation& op); class TestScriptModel: public QObject { @@ -34,4 +59,6 @@ class TestScriptModel: public QObject { private slots: static void unique_data(); // NOLINT static void unique(); + static void structuralEquality(); + static void comparisonModes(); };