From afd1b3b6e95728f6bc1df76d647979a904234c23 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Wed, 3 Apr 2024 12:10:16 +0200 Subject: [PATCH] QmlDesigner: Lookup the default property in prototypes too The default property can be set too in any prototypes. Look them up there too. Because it is expensive we cache them in the node meta info to make hasDefaultProperty() followed by defaultProperty() cheaper. Change-Id: I3b9ec90fc1bc5f0228dad3b580c335734f03821d Reviewed-by: Thomas Hartmann Reviewed-by: Qt CI Patch Build Bot --- .../designercore/include/nodemetainfo.h | 2 + .../designercore/metainfo/nodemetainfo.cpp | 14 +++- .../projectstorage/projectstorage.h | 56 ++++++++++++++-- .../projectstorage/projectstorageinfotypes.h | 18 ++---- .../projectstorage/projectstorageinterface.h | 1 + tests/unit/tests/mocks/projectstoragemock.cpp | 6 +- tests/unit/tests/mocks/projectstoragemock.h | 4 ++ .../tests/printers/gtest-creator-printing.cpp | 2 +- .../unittests/model/nodelistproperty-test.cpp | 4 +- .../projectstorage/projectstorage-test.cpp | 64 ++++++++++++++++--- 10 files changed, 135 insertions(+), 36 deletions(-) diff --git a/src/plugins/qmldesigner/designercore/include/nodemetainfo.h b/src/plugins/qmldesigner/designercore/include/nodemetainfo.h index 23110175c1b..648c4be9a82 100644 --- a/src/plugins/qmldesigner/designercore/include/nodemetainfo.h +++ b/src/plugins/qmldesigner/designercore/include/nodemetainfo.h @@ -265,12 +265,14 @@ public: private: const Storage::Info::Type &typeData() const; + PropertyDeclarationId defaultPropertyDeclarationId() const; bool isSubclassOf(const TypeName &type, int majorVersion = -1, int minorVersion = -1) const; private: TypeId m_typeId; NotNullPointer m_projectStorage = {}; mutable std::optional m_typeData; + mutable std::optional m_defaultPropertyId; std::shared_ptr m_privateData; }; diff --git a/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp b/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp index 845a1db1e52..adea8baf255 100644 --- a/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp +++ b/src/plugins/qmldesigner/designercore/metainfo/nodemetainfo.cpp @@ -1835,7 +1835,7 @@ PropertyName NodeMetaInfo::defaultPropertyName() const { if constexpr (useProjectStorage()) { if (isValid()) { - if (auto name = m_projectStorage->propertyName(typeData().defaultPropertyId)) { + if (auto name = m_projectStorage->propertyName(defaultPropertyDeclarationId())) { return name->toQByteArray(); } } @@ -1851,7 +1851,7 @@ PropertyMetaInfo NodeMetaInfo::defaultProperty() const { if constexpr (useProjectStorage()) { if (isValid()) { - return PropertyMetaInfo(typeData().defaultPropertyId, m_projectStorage); + return PropertyMetaInfo(defaultPropertyDeclarationId(), m_projectStorage); } } else { return property(defaultPropertyName()); @@ -1862,7 +1862,7 @@ PropertyMetaInfo NodeMetaInfo::defaultProperty() const bool NodeMetaInfo::hasDefaultProperty() const { if constexpr (useProjectStorage()) - return isValid() && bool(typeData().defaultPropertyId); + return isValid() && bool(defaultPropertyDeclarationId()); else return !defaultPropertyName().isEmpty(); } @@ -2089,6 +2089,14 @@ const Storage::Info::Type &NodeMetaInfo::typeData() const return *m_typeData; } +PropertyDeclarationId NodeMetaInfo::defaultPropertyDeclarationId() const +{ + if (!m_defaultPropertyId) + m_defaultPropertyId = m_projectStorage->defaultPropertyDeclarationId(m_typeId); + + return *m_defaultPropertyId; +} + bool NodeMetaInfo::isSubclassOf(const TypeName &type, int majorVersion, int minorVersion) const { if (!isValid()) { diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h index 83e11f952bc..ad606bcb90c 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorage.h @@ -408,6 +408,22 @@ public: return propertyDeclarationId; } + PropertyDeclarationId defaultPropertyDeclarationId(TypeId typeId) const override + { + using NanotraceHR::keyValue; + NanotraceHR::Tracer tracer{"get default property declaration id"_t, + projectStorageCategory(), + keyValue("type id", typeId)}; + + auto propertyDeclarationId = Sqlite::withDeferredTransaction(database, [&] { + return fetchDefaultPropertyDeclarationId(typeId); + }); + + tracer.end(keyValue("property declaration id", propertyDeclarationId)); + + return propertyDeclarationId; + } + std::optional propertyDeclaration( PropertyDeclarationId propertyDeclarationId) const override { @@ -2379,17 +2395,42 @@ private: return PropertyDeclarationId{}; } - PropertyDeclarationId fetchPropertyDeclarationId(TypeId baseTypeId, + PropertyDeclarationId fetchPropertyDeclarationId(TypeId typeId, Utils::SmallStringView propertyName) const { auto propertyDeclarationId = selectPropertyDeclarationIdByTypeIdAndNameStatement - .template value(baseTypeId, - propertyName); + .template value(typeId, propertyName); if (propertyDeclarationId) return propertyDeclarationId; - return fetchNextPropertyDeclarationId(baseTypeId, propertyName); + return fetchNextPropertyDeclarationId(typeId, propertyName); + } + + PropertyDeclarationId fetchNextDefaultPropertyDeclarationId(TypeId baseTypeId) const + { + auto range = selectPrototypeAndExtensionIdsStatement.template range(baseTypeId); + + for (TypeId prototype : range) { + auto propertyDeclarationId = selectDefaultPropertyDeclarationIdStatement + .template value(prototype); + + if (propertyDeclarationId) + return propertyDeclarationId; + } + + return PropertyDeclarationId{}; + } + + PropertyDeclarationId fetchDefaultPropertyDeclarationId(TypeId typeId) const + { + auto propertyDeclarationId = selectDefaultPropertyDeclarationIdStatement + .template value(typeId); + + if (propertyDeclarationId) + return propertyDeclarationId; + + return fetchNextDefaultPropertyDeclarationId(typeId); } void synchronizePropertyDeclarationsInsertProperty( @@ -4842,9 +4883,10 @@ public: "UPDATE types SET defaultPropertyId=?2 WHERE typeId=?1", database}; WriteStatement<1> updateDefaultPropertyIdToNullStatement{ "UPDATE types SET defaultPropertyId=NULL WHERE defaultPropertyId=?1", database}; - mutable ReadStatement<4, 1> selectInfoTypeByTypeIdStatement{ - "SELECT defaultPropertyId, sourceId, traits, annotationTraits FROM types WHERE typeId=?", - database}; + mutable ReadStatement<3, 1> selectInfoTypeByTypeIdStatement{ + "SELECT sourceId, traits, annotationTraits FROM types WHERE typeId=?", database}; + mutable ReadStatement<1, 1> selectDefaultPropertyDeclarationIdStatement{ + "SELECT defaultPropertyId FROM types WHERE typeId=?", database}; mutable ReadStatement<1, 1> selectPrototypeIdsForTypeIdInOrderStatement{ "WITH RECURSIVE " " all_prototype_and_extension(typeId, prototypeId) AS (" diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinfotypes.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinfotypes.h index 3be6fc4cae4..9f0c134ed39 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinfotypes.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinfotypes.h @@ -503,18 +503,13 @@ public: class Type { public: - Type(PropertyDeclarationId defaultPropertyId, - SourceId sourceId, - long long typeTraits, - long long typeAnnotationTraits) - : defaultPropertyId{defaultPropertyId} - , sourceId{sourceId} + Type(SourceId sourceId, long long typeTraits, long long typeAnnotationTraits) + : sourceId{sourceId} , traits{typeTraits, typeAnnotationTraits} {} - Type(PropertyDeclarationId defaultPropertyId, SourceId sourceId, TypeTraits traits) - : defaultPropertyId{defaultPropertyId} - , sourceId{sourceId} + Type(SourceId sourceId, TypeTraits traits) + : sourceId{sourceId} , traits{traits} {} @@ -523,14 +518,11 @@ public: { using NanotraceHR::dictonary; using NanotraceHR::keyValue; - auto dict = dictonary(keyValue("default property id", type.defaultPropertyId), - keyValue("source id", type.sourceId), - keyValue("traits", type.traits)); + auto dict = dictonary(keyValue("source id", type.sourceId), keyValue("traits", type.traits)); convertToString(string, dict); } - PropertyDeclarationId defaultPropertyId; SourceId sourceId; TypeTraits traits; }; diff --git a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h index 13a40695bf9..a67b6296083 100644 --- a/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h +++ b/src/plugins/qmldesigner/designercore/projectstorage/projectstorageinterface.h @@ -55,6 +55,7 @@ public: virtual PropertyDeclarationId propertyDeclarationId(TypeId typeId, ::Utils::SmallStringView propertyName) const = 0; + virtual PropertyDeclarationId defaultPropertyDeclarationId(TypeId typeId) const = 0; virtual std::optional type(TypeId typeId) const = 0; virtual Utils::PathString typeIconPath(TypeId typeId) const = 0; virtual Storage::Info::TypeHints typeHints(TypeId typeId) const = 0; diff --git a/tests/unit/tests/mocks/projectstoragemock.cpp b/tests/unit/tests/mocks/projectstoragemock.cpp index 48357ff3e63..6d5304879e6 100644 --- a/tests/unit/tests/mocks/projectstoragemock.cpp +++ b/tests/unit/tests/mocks/projectstoragemock.cpp @@ -293,8 +293,10 @@ TypeId ProjectStorageMock::createType(ModuleId moduleId, defaultPropertyTypeId); } - ON_CALL(*this, type(Eq(typeId))) - .WillByDefault(Return(Storage::Info::Type{defaultPropertyDeclarationId, sourceId, typeTraits})); + ON_CALL(*this, type(Eq(typeId))).WillByDefault(Return(Storage::Info::Type{sourceId, typeTraits})); + + ON_CALL(*this, defaultPropertyDeclarationId(Eq(typeId))) + .WillByDefault(Return(defaultPropertyDeclarationId)); ON_CALL(*this, isBasedOn(Eq(typeId), Eq(typeId))).WillByDefault(Return(true)); diff --git a/tests/unit/tests/mocks/projectstoragemock.h b/tests/unit/tests/mocks/projectstoragemock.h index a68e37c2a15..b534814c1d3 100644 --- a/tests/unit/tests/mocks/projectstoragemock.h +++ b/tests/unit/tests/mocks/projectstoragemock.h @@ -187,6 +187,10 @@ public: propertyDeclarationId, (QmlDesigner::TypeId typeId, ::Utils::SmallStringView propertyName), (const, override)); + MOCK_METHOD(QmlDesigner::PropertyDeclarationId, + defaultPropertyDeclarationId, + (QmlDesigner::TypeId typeId), + (const, override)); MOCK_METHOD(std::optional, type, (QmlDesigner::TypeId typeId), diff --git a/tests/unit/tests/printers/gtest-creator-printing.cpp b/tests/unit/tests/printers/gtest-creator-printing.cpp index 1bff6df492c..40f9c8db6da 100644 --- a/tests/unit/tests/printers/gtest-creator-printing.cpp +++ b/tests/unit/tests/printers/gtest-creator-printing.cpp @@ -675,7 +675,7 @@ std::ostream &operator<<(std::ostream &out, const PropertyDeclaration &propertyD std::ostream &operator<<(std::ostream &out, const Type &type) { - return out << "(" << type.defaultPropertyId << ")"; + return out << "(" << type.sourceId << ")"; } std::ostream &operator<<(std::ostream &out, const ExportedTypeName &name) diff --git a/tests/unit/tests/unittests/model/nodelistproperty-test.cpp b/tests/unit/tests/unittests/model/nodelistproperty-test.cpp index 6783bde3e75..aac2e729a29 100644 --- a/tests/unit/tests/unittests/model/nodelistproperty-test.cpp +++ b/tests/unit/tests/unittests/model/nodelistproperty-test.cpp @@ -61,8 +61,8 @@ protected: ++defaultPropertyIdNumber); ON_CALL(projectStorageMock, typeId(Eq(moduleId), Eq(typeName), _)).WillByDefault(Return(typeId)); - ON_CALL(projectStorageMock, type(Eq(typeId))) - .WillByDefault(Return(Info::Type{defaultPropertyId, QmlDesigner::SourceId{}, {}})); + ON_CALL(projectStorageMock, defaultPropertyDeclarationId(Eq(typeId))) + .WillByDefault(Return(defaultPropertyId)); ON_CALL(projectStorageMock, propertyName(Eq(defaultPropertyId))) .WillByDefault(Return(defaultPeopertyName)); } diff --git a/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp b/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp index 4b3f0a18692..125e8df3aeb 100644 --- a/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp +++ b/tests/unit/tests/unittests/projectstorage/projectstorage-test.cpp @@ -253,17 +253,15 @@ MATCHER(StringsAreSorted, std::string(negation ? "isn't sorted" : "is sorted")) }); } -MATCHER_P3(IsInfoType, - defaultPropertyId, +MATCHER_P2(IsInfoType, sourceId, traits, std::string(negation ? "isn't " : "is ") - + PrintToString(Storage::Info::Type{defaultPropertyId, sourceId, traits})) + + PrintToString(Storage::Info::Type{sourceId, traits})) { const Storage::Info::Type &type = arg; - return type.defaultPropertyId == defaultPropertyId && type.sourceId == sourceId - && type.traits == traits; + return type.sourceId == sourceId && type.traits == traits; } class ProjectStorage : public testing::Test @@ -6683,12 +6681,10 @@ TEST_F(ProjectStorage, get_type) auto package{createSimpleSynchronizationPackage()}; storage.synchronize(package); auto typeId = fetchTypeId(sourceId1, "QQuickItem"); - auto defaultPropertyName = storage.fetchTypeByTypeId(typeId).defaultPropertyName; - auto defaultPropertyId = storage.propertyDeclarationId(typeId, defaultPropertyName); auto type = storage.type(typeId); - ASSERT_THAT(type, Optional(IsInfoType(defaultPropertyId, sourceId1, TypeTraitsKind::Reference))); + ASSERT_THAT(type, Optional(IsInfoType(sourceId1, TypeTraitsKind::Reference))); } TEST_F(ProjectStorage, dont_get_type_for_invalid_id) @@ -6701,6 +6697,58 @@ TEST_F(ProjectStorage, dont_get_type_for_invalid_id) ASSERT_THAT(type, Eq(std::nullopt)); } +TEST_F(ProjectStorage, get_default_property_declarartion_id) +{ + auto package{createSimpleSynchronizationPackage()}; + storage.synchronize(package); + auto typeId = fetchTypeId(sourceId1, "QQuickItem"); + auto defaultPropertyName = storage.fetchTypeByTypeId(typeId).defaultPropertyName; + auto defaultPropertyId = storage.propertyDeclarationId(typeId, defaultPropertyName); + + auto propertyId = storage.defaultPropertyDeclarationId(typeId); + + ASSERT_THAT(propertyId, defaultPropertyId); +} + +TEST_F(ProjectStorage, get_default_property_declarartion_id_in_base_type) +{ + auto package{createSynchronizationPackageWithAliases()}; + storage.synchronize(package); + auto baseTypeId = fetchTypeId(sourceId1, "QQuickItem"); + auto defaultPropertyName = storage.fetchTypeByTypeId(baseTypeId).defaultPropertyName; + auto defaultPropertyId = storage.propertyDeclarationId(baseTypeId, defaultPropertyName); + auto typeId = fetchTypeId(sourceId3, "QAliasItem"); + + auto propertyId = storage.defaultPropertyDeclarationId(typeId); + + ASSERT_THAT(propertyId, defaultPropertyId); +} + +TEST_F(ProjectStorage, do_not_get_default_property_declarartion_id_wrong_type_in_property_chain) +{ + auto package{createSynchronizationPackageWithAliases()}; + package.types[1].defaultPropertyName = "objects"; + storage.synchronize(package); + auto baseTypeId = fetchTypeId(sourceId1, "QQuickItem"); + auto defaultPropertyName = storage.fetchTypeByTypeId(baseTypeId).defaultPropertyName; + auto defaultPropertyId = storage.propertyDeclarationId(baseTypeId, defaultPropertyName); + auto typeId = fetchTypeId(sourceId3, "QAliasItem"); + + auto propertyId = storage.defaultPropertyDeclarationId(typeId); + + ASSERT_THAT(propertyId, defaultPropertyId); +} + +TEST_F(ProjectStorage, get_invalid_default_property_declarartion_id_for_invalid_type) +{ + auto package{createSimpleSynchronizationPackage()}; + storage.synchronize(package); + + auto propertyId = storage.defaultPropertyDeclarationId(TypeId()); + + ASSERT_FALSE(propertyId); +} + TEST_F(ProjectStorage, get_common_type) { auto package{createSimpleSynchronizationPackage()};