From 1066e768c9aba5586a00875c847e737a780d2c1c Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Thu, 13 Aug 2020 17:33:07 +0200 Subject: [PATCH] Fix cleaning up of tags that are not needed any more --- .../thingmanagerimplementation.cpp | 1 - libnymea-core/jsonrpc/tagshandler.cpp | 6 +-- libnymea-core/nymeacore.cpp | 11 ++++ libnymea-core/tagging/tagsstorage.cpp | 1 + tests/auto/tags/testtags.cpp | 54 +++++++++++++++++++ 5 files changed, 69 insertions(+), 4 deletions(-) diff --git a/libnymea-core/integrations/thingmanagerimplementation.cpp b/libnymea-core/integrations/thingmanagerimplementation.cpp index 0c858a83..516b9da7 100644 --- a/libnymea-core/integrations/thingmanagerimplementation.cpp +++ b/libnymea-core/integrations/thingmanagerimplementation.cpp @@ -712,7 +712,6 @@ ThingSetupInfo* ThingManagerImplementation::addConfiguredThingInternal(const Thi // set settings (init with defaults) ParamList settings = buildParams(thingClass.settingsTypes(), ParamList()); - qCDebug(dcThingManager()) << "Adding thing settings" << settings << thingId; thing->setSettings(settings); ThingSetupInfo *info = setupThing(thing); diff --git a/libnymea-core/jsonrpc/tagshandler.cpp b/libnymea-core/jsonrpc/tagshandler.cpp index 92f37e70..67b40cc1 100644 --- a/libnymea-core/jsonrpc/tagshandler.cpp +++ b/libnymea-core/jsonrpc/tagshandler.cpp @@ -105,14 +105,14 @@ JsonReply *TagsHandler::GetTags(const QVariantMap ¶ms) const { QVariantList ret; foreach (const Tag &tag, NymeaCore::instance()->tagsStorage()->tags()) { - if (params.contains("thingId") && params.value("thingId").toString() != tag.thingId().toString()) { + if (params.contains("thingId") && params.value("thingId").toUuid() != tag.thingId()) { continue; } - if (params.contains("deviceId") && params.value("deviceId").toString() != tag.thingId().toString()) { + if (params.contains("deviceId") && params.value("deviceId").toUuid() != tag.thingId()) { // nymea < 0.19 continue; } - if (params.contains("ruleId") && params.value("ruleId").toString() != tag.ruleId().toString()) { + if (params.contains("ruleId") && params.value("ruleId").toUuid() != tag.ruleId()) { continue; } if (params.contains("appId") && params.value("appId").toString() != tag.appId()) { diff --git a/libnymea-core/nymeacore.cpp b/libnymea-core/nymeacore.cpp index 377f3cae..ca566bc3 100644 --- a/libnymea-core/nymeacore.cpp +++ b/libnymea-core/nymeacore.cpp @@ -838,6 +838,17 @@ void NymeaCore::thingManagerLoaded() } } } + + foreach (const Tag &tag, m_tagsStorage->tags()) { + if (!tag.ruleId().isNull() && !m_ruleEngine->findRule(tag.ruleId()).isValid()) { + qCDebug(dcCore()) << "Cleaning up stale rule tag" << tag; + m_tagsStorage->removeTag(tag); + } + if (!tag.thingId().isNull() && !m_thingManager->findConfiguredThing(tag.thingId())) { + qCDebug(dcCore()) << "Cleaning up stale thing tag" << tag.tagId(); + m_tagsStorage->removeTag(tag); + } + } } } diff --git a/libnymea-core/tagging/tagsstorage.cpp b/libnymea-core/tagging/tagsstorage.cpp index 44aaa867..7e395da7 100644 --- a/libnymea-core/tagging/tagsstorage.cpp +++ b/libnymea-core/tagging/tagsstorage.cpp @@ -41,6 +41,7 @@ TagsStorage::TagsStorage(ThingManager *thingManager, RuleEngine *ruleEngine, QOb m_ruleEngine(ruleEngine) { connect(thingManager, &ThingManager::thingRemoved, this, &TagsStorage::thingRemoved); + connect(ruleEngine, &RuleEngine::ruleRemoved, this, &TagsStorage::ruleRemoved); NymeaSettings settings(NymeaSettings::SettingsRoleTags); diff --git a/tests/auto/tags/testtags.cpp b/tests/auto/tags/testtags.cpp index 8d607bd8..f453dfad 100644 --- a/tests/auto/tags/testtags.cpp +++ b/tests/auto/tags/testtags.cpp @@ -51,6 +51,8 @@ private slots: void removeTag(); + void ruleTagIsRemovedOnRuleRemove(); + private: QVariantMap createThingTag(const QString &thingId, const QString &appId, const QString &tagId, const QString &value); bool compareThingTag(const QVariantMap &tag, const QUuid &thingId, const QString &appId, const QString &tagId, const QString &value); @@ -248,5 +250,57 @@ void TestTags::removeTag() QCOMPARE(tagsList.count(), 0); } +void TestTags::ruleTagIsRemovedOnRuleRemove() +{ + // Create a rule + QVariantMap params; + params.insert("name", "testrule"); + QVariantMap action; + action.insert("thingId", m_mockThingId); + action.insert("actionTypeId", mockWithoutParamsActionTypeId); + QVariantList actions = {action}; + params.insert("actions", actions); + QVariant response = injectAndWait("Rules.AddRule", params); + verifyError(response, "ruleError", "RuleErrorNoError"); + QUuid ruleId = response.toMap().value("params").toMap().value("ruleId").toUuid(); + + // Tag the rule + params.clear(); + QVariantMap tag; + tag.insert("appId", "testtags"); + tag.insert("ruleId", ruleId); + tag.insert("tagId", "testtag"); + tag.insert("value", "blabla"); + params.insert("tag", tag); + response = injectAndWait("Tags.AddTag", params); + verifyTagError(response, TagsStorage::TagErrorNoError); + + // Make sure the tag is here + params.clear(); + params.insert("appId", "testtags"); + params.insert("ruleId", ruleId); + params.insert("tagId", "testtag"); + response = injectAndWait("Tags.GetTags", params); + verifyTagError(response, TagsStorage::TagErrorNoError); + QVERIFY2(response.toMap().value("params").toMap().value("tags").toList().count() == 1, "Tag not found!"); + qCDebug(dcTests()) << "Get tag reply" << qUtf8Printable(QJsonDocument::fromVariant(response).toJson()); + + // Remove the rule + params.clear(); + params.insert("ruleId", ruleId); + response = injectAndWait("Rules.RemoveRule", params); + verifyError(response, "ruleError", "RuleErrorNoError"); + + // Make sure the tag disappeared + params.clear(); + params.insert("appId", "testtags"); + params.insert("ruleId", ruleId); + params.insert("tagId", "testtag"); + response = injectAndWait("Tags.GetTags", params); + verifyTagError(response, TagsStorage::TagErrorNoError); + QVERIFY2(response.toMap().value("params").toMap().value("tags").toList().count() == 0, "Tag has not been cleaned up!"); + qCDebug(dcTests()) << "Get tag reply" << qUtf8Printable(QJsonDocument::fromVariant(response).toJson()); +} + #include "testtags.moc" QTEST_MAIN(TestTags)