From aff8cff0ecc1768d253f13625bb7887d102f862d Mon Sep 17 00:00:00 2001 From: ComixHe Date: Thu, 12 Oct 2023 18:05:00 +0800 Subject: [PATCH] refact: change underlying type of desktop entry Signed-off-by: ComixHe --- api/dbus/org.desktopspec.MimeManager1.xml | 2 +- apps/dde-application-manager/src/main.cpp | 3 +- src/applicationchecker.cpp | 25 ++++----- src/applicationmimeinfo.cpp | 4 +- src/dbus/applicationservice.cpp | 58 ++++++++++++--------- src/dbus/mimemanager1service.cpp | 2 +- src/dbus/mimemanager1service.h | 2 +- src/desktopentry.cpp | 63 ++++++++++------------- src/desktopentry.h | 31 +++++------ src/desktopfileparser.cpp | 45 ++++++++++++---- src/global.h | 6 +-- tests/main.cpp | 2 +- tests/ut_desktopentry.cpp | 17 +++--- 13 files changed, 142 insertions(+), 118 deletions(-) diff --git a/api/dbus/org.desktopspec.MimeManager1.xml b/api/dbus/org.desktopspec.MimeManager1.xml index f6f9180..6a63b9a 100644 --- a/api/dbus/org.desktopspec.MimeManager1.xml +++ b/api/dbus/org.desktopspec.MimeManager1.xml @@ -10,7 +10,7 @@ - + diff --git a/apps/dde-application-manager/src/main.cpp b/apps/dde-application-manager/src/main.cpp index 0e12ef8..790bf5b 100644 --- a/apps/dde-application-manager/src/main.cpp +++ b/apps/dde-application-manager/src/main.cpp @@ -21,7 +21,8 @@ void registerComplexDbusType() qDBusRegisterMetaType(); qRegisterMetaType(); qDBusRegisterMetaType(); - qDBusRegisterMetaType(); + qDBusRegisterMetaType(); + qRegisterMetaType(); qRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); diff --git a/src/applicationchecker.cpp b/src/applicationchecker.cpp index bd73c7b..21bab3b 100644 --- a/src/applicationchecker.cpp +++ b/src/applicationchecker.cpp @@ -15,9 +15,9 @@ bool ApplicationFilter::hiddenCheck(const std::unique_ptr &entry) if (hiddenVal.has_value()) { bool ok{false}; - hidden = hiddenVal.value().toBoolean(ok); + hidden = toBoolean(hiddenVal.value(), ok); if (!ok) { - qWarning() << "invalid hidden value:" << *hiddenVal.value().find(defaultKeyStr); + qWarning() << "invalid hidden value:" << hiddenVal.value(); return false; } } @@ -28,10 +28,9 @@ bool ApplicationFilter::tryExecCheck(const std::unique_ptr &entry) { auto tryExecVal = entry->value(DesktopFileEntryKey, "TryExec"); if (tryExecVal.has_value()) { - bool ok{false}; - auto executable = tryExecVal.value().toString(ok); - if (!ok) { - qWarning() << "invalid TryExec value:" << *tryExecVal.value().find(defaultKeyStr); + auto executable = toString(tryExecVal.value()); + if (executable.isEmpty()) { + qWarning() << "invalid TryExec value:" << tryExecVal.value(); return false; } @@ -56,10 +55,9 @@ bool ApplicationFilter::showInCheck(const std::unique_ptr &entry) bool showInCurrentDE{true}; auto onlyShowInVal = entry->value(DesktopFileEntryKey, "OnlyShowIn"); while (onlyShowInVal.has_value()) { - bool ok{false}; - auto deStr = onlyShowInVal.value().toString(ok); - if (!ok) { - qWarning() << "invalid OnlyShowIn value:" << *onlyShowInVal.value().find(defaultKeyStr); + auto deStr = toString(onlyShowInVal.value()); + if (deStr.isEmpty()) { + qWarning() << "invalid OnlyShowIn value:" << onlyShowInVal.value(); break; } @@ -79,10 +77,9 @@ bool ApplicationFilter::showInCheck(const std::unique_ptr &entry) bool notShowInCurrentDE{false}; auto notShowInVal = entry->value(DesktopFileEntryKey, "NotShowIn"); while (notShowInVal.has_value()) { - bool ok{false}; - auto deStr = notShowInVal.value().toString(ok); - if (!ok) { - qWarning() << "invalid OnlyShowIn value:" << *notShowInVal.value().find(defaultKeyStr); + auto deStr = toString(notShowInVal.value()); + if (deStr.isEmpty()) { + qWarning() << "invalid OnlyShowIn value:" << notShowInVal.value(); break; } diff --git a/src/applicationmimeinfo.cpp b/src/applicationmimeinfo.cpp index 80a65e0..7264490 100644 --- a/src/applicationmimeinfo.cpp +++ b/src/applicationmimeinfo.cpp @@ -44,8 +44,8 @@ QString toString(const MimeContent &content) noexcept for (auto it = content.constKeyValueBegin(); it != content.constKeyValueEnd(); ++it) { ret.append(QString{"[%1]\n"}.arg(it->first)); - const auto &kvPairs = it->second; - for (auto inner = kvPairs.constKeyValueBegin(); inner != kvPairs.constKeyValueEnd(); ++inner) { + const auto &QStringMap = it->second; + for (auto inner = QStringMap.constKeyValueBegin(); inner != QStringMap.constKeyValueEnd(); ++inner) { ret.append(QString{"%1="}.arg(inner->first)); for (const auto &app : inner->second) { ret.append(QString{"%2;"}.arg(app)); diff --git a/src/dbus/applicationservice.cpp b/src/dbus/applicationservice.cpp index ba79a66..9b74c91 100644 --- a/src/dbus/applicationservice.cpp +++ b/src/dbus/applicationservice.cpp @@ -180,9 +180,10 @@ QDBusObjectPath ApplicationService::Launch(const QString &action, const QStringL if (!actionExec) { break; } - execStr = actionExec->toString(ok); - if (!ok) { - qWarning() << "exec value to string failed, try default action."; + + execStr = toString(actionExec.value()); + if (execStr.isEmpty()) { + qWarning() << "exec value to string failed, try default action."; // we need this log. break; } break; @@ -196,8 +197,8 @@ QDBusObjectPath ApplicationService::Launch(const QString &action, const QStringL sendErrorReply(QDBusError::Failed, msg); return {}; } - execStr = Actions->toString(ok); - if (!ok) { + execStr = toString(Actions.value()); + if (execStr.isEmpty()) { QString msg{"maybe entry actions's format is invalid, abort launch."}; qWarning() << msg; sendErrorReply(QDBusError::Failed, msg); @@ -220,8 +221,8 @@ QDBusObjectPath ApplicationService::Launch(const QString &action, const QStringL if (terminal()) { // don't change this sequence - execCmds.push_front("-C"); // means run a shellscript - execCmds.push_front("--keep-open"); // keep terminal open, prevent exit immediately + execCmds.push_front("-C"); // means run a shellscript + execCmds.push_front("--keep-open"); // keep terminal open, prevent exit immediately execCmds.push_front("deepin-terminal"); } cmds.append(std::move(execCmds)); @@ -391,7 +392,7 @@ PropMap ApplicationService::actionName() const noexcept if (!value.has_value()) { continue; } - ret.insert(action, {std::move(value).value()}); + ret.insert(action, std::move(value).value()); } return ret; @@ -508,9 +509,9 @@ void ApplicationService::setScaleFactor(double value) noexcept return; } - if(m_customScale){ + if (m_customScale) { if (!storagePtr->updateApplicationValue(appId, ApplicationPropertiesGroup, ScaleFactor, value)) { - sendErrorReply(QDBusError::Failed,"update scaleFactor failed."); + sendErrorReply(QDBusError::Failed, "update scaleFactor failed."); return; } } else { @@ -826,9 +827,9 @@ LaunchTask ApplicationService::unescapeExec(const QString &str, const QStringLis task.command.append(std::move(execList)); return task; } - bool ok; - auto iconStr = val->toIconString(ok); - if (!ok) { + + auto iconStr = toIconString(val.value()); + if (iconStr.isEmpty()) { qDebug() << R"(Icons Convert to string failed. %i will be ignored.)"; task.command.append(std::move(execList)); return task; @@ -845,9 +846,17 @@ LaunchTask ApplicationService::unescapeExec(const QString &str, const QStringLis task.command.append(std::move(execList)); return task; } - bool ok; - auto NameStr = val->toLocaleString(getUserLocale(), ok); - if (!ok) { + + const auto &rawValue = val.value(); + if (!rawValue.canConvert()) { + qDebug() << "Name's underlying type mismatch:" + << "QStringMap" << rawValue.metaType().name(); + task.command.append(std::move(execList)); + return task; + } + + auto NameStr = toLocaleString(rawValue.value(), getUserLocale()); + if (NameStr.isEmpty()) { qDebug() << R"(Name Convert to locale string failed. %c will be ignored.)"; task.command.append(std::move(execList)); return task; @@ -897,26 +906,29 @@ QVariant ApplicationService::findEntryValue(const QString &group, switch (type) { case EntryValueType::String: { - auto valStr = val.toString(ok); - if (ok) { + auto valStr = toString(val); + if (!valStr.isEmpty()) { ret = QVariant::fromValue(valStr); } } break; case EntryValueType::LocaleString: { - auto valStr = val.toLocaleString(locale, ok); - if (ok) { + if (!val.canConvert()) { + return ret; + } + auto valStr = toLocaleString(val.value(), locale); + if (!valStr.isEmpty()) { ret = QVariant::fromValue(valStr); } } break; case EntryValueType::Boolean: { - auto valBool = val.toBoolean(ok); + auto valBool = toBoolean(val, ok); if (ok) { ret = QVariant::fromValue(valBool); } } break; case EntryValueType::IconString: { - auto valStr = val.toIconString(ok); - if (ok) { + auto valStr = toIconString(val); + if (!valStr.isEmpty()) { ret = QVariant::fromValue(valStr); } } break; diff --git a/src/dbus/mimemanager1service.cpp b/src/dbus/mimemanager1service.cpp index 92f9bac..05a8d7c 100644 --- a/src/dbus/mimemanager1service.cpp +++ b/src/dbus/mimemanager1service.cpp @@ -79,7 +79,7 @@ QString MimeManager1Service::queryFileTypeAndDefaultApplication(const QString &f return mimeType; } -void MimeManager1Service::setDefaultApplication(const KVPairs &defaultApps) noexcept +void MimeManager1Service::setDefaultApplication(const QStringMap &defaultApps) noexcept { auto &app = m_infos.front().appsList(); auto userConfig = std::find_if( diff --git a/src/dbus/mimemanager1service.h b/src/dbus/mimemanager1service.h index 8773743..b507b41 100644 --- a/src/dbus/mimemanager1service.h +++ b/src/dbus/mimemanager1service.h @@ -28,7 +28,7 @@ public Q_SLOTS: [[nodiscard]] ObjectMap listApplications(const QString &mimeType) const noexcept; [[nodiscard]] QString queryFileTypeAndDefaultApplication(const QString &filePath, QDBusObjectPath &application) const noexcept; - void setDefaultApplication(const KVPairs &defaultApps) noexcept; + void setDefaultApplication(const QStringMap &defaultApps) noexcept; void unsetDefaultApplication(const QStringList &mimeTypes) noexcept; private: diff --git a/src/desktopentry.cpp b/src/desktopentry.cpp index f76146e..96e6cf2 100644 --- a/src/desktopentry.cpp +++ b/src/desktopentry.cpp @@ -3,7 +3,6 @@ // SPDX-License-Identifier: LGPL-3.0-or-later #include "desktopentry.h" -#include "global.h" #include "desktopfileparser.h" #include #include @@ -43,17 +42,15 @@ bool DesktopEntry::checkMainEntryValidation() const noexcept qWarning() << "No Type."; for (auto tmp = it->constKeyValueBegin(); tmp != it->constKeyValueEnd(); ++tmp) { const auto &[k, v] = *tmp; - qInfo() << "keyName:" << k; - for (auto valIt = v.constKeyValueBegin(); valIt != v.constKeyValueEnd(); ++valIt) { - const auto &[key, value] = *valIt; - qInfo() << key << value; - } + qInfo() << "key:" << k << "value:" << v; } return false; } - const auto &typeStr = *type->find(defaultKeyStr); - + const auto &typeStr = type->toString(); + if (typeStr.isEmpty()) { + return false; + } if (typeStr == "Link") { if (it->find("URL") == it->end()) { return false; @@ -239,7 +236,7 @@ std::optional DesktopEntry::value(const QString &groupKey, return *it; } -QString DesktopEntry::Value::unescape(const QString &str) noexcept +QString unescape(const QString &str) noexcept { QString unescapedStr; for (qsizetype i = 0; i < str.size(); ++i) { @@ -283,63 +280,66 @@ QString DesktopEntry::Value::unescape(const QString &str) noexcept return unescapedStr; } -QString DesktopEntry::Value::toString(bool &ok) const noexcept +QString toString(const DesktopEntry::Value &value) noexcept { - ok = false; - auto str = this->find(defaultKeyStr); + QString str; - if (str == this->end()) { + if (value.canConvert()) { // get default locale + str = value.value()[defaultKeyStr]; + } else { + str = value.toString(); + } + + if (str.isEmpty()) { qWarning() << "value not found."; return {}; } - auto unescapedStr = unescape(*str); + auto unescapedStr = unescape(str); if (hasNonAsciiAndControlCharacters(unescapedStr)) { return {}; } - ok = true; return unescapedStr; } -QString DesktopEntry::Value::toLocaleString(const QLocale &locale, bool &ok) const noexcept +QString toLocaleString(const QStringMap &localeMap, const QLocale &locale) noexcept { - ok = false; - for (auto it = this->constKeyValueBegin(); it != this->constKeyValueEnd(); ++it) { + for (auto it = localeMap.constKeyValueBegin(); it != localeMap.constKeyValueEnd(); ++it) { auto [a, b] = *it; if (QLocale{a}.name() == locale.name()) { - ok = true; return unescape(b); } } - return toString(ok); + + return toString(localeMap[defaultKeyStr]); } -QString DesktopEntry::Value::toIconString(bool &ok) const noexcept +QString toIconString(const DesktopEntry::Value &value) noexcept { - return toString(ok); + return toString(value); } -bool DesktopEntry::Value::toBoolean(bool &ok) const noexcept +bool toBoolean(const DesktopEntry::Value &value, bool &ok) noexcept { ok = false; - const auto &str = (*this)[defaultKeyStr]; + const auto &str = toString(value); if (str == "true") { ok = true; return true; } + if (str == "false") { ok = true; return false; } + return false; } -float DesktopEntry::Value::toNumeric(bool &ok) const noexcept +float toNumeric(const DesktopEntry::Value &value, bool &ok) noexcept { - const auto &str = (*this)[defaultKeyStr]; - QVariant v{str}; - return v.toFloat(&ok); + return value.toFloat(&ok); } bool operator==(const DesktopEntry &lhs, const DesktopEntry &rhs) @@ -381,10 +381,3 @@ bool operator!=(const DesktopFile &lhs, const DesktopFile &rhs) { return !(lhs == rhs); } - -QDebug operator<<(QDebug debug, const DesktopEntry::Value &v) -{ - QDebugStateSaver saver{debug}; - debug << static_cast &>(v); - return debug; -} diff --git a/src/desktopentry.h b/src/desktopentry.h index 789a356..933c86f 100644 --- a/src/desktopentry.h +++ b/src/desktopentry.h @@ -13,6 +13,7 @@ #include #include #include "iniParser.h" +#include "global.h" constexpr static auto defaultKeyStr = "default"; @@ -105,21 +106,7 @@ private: class DesktopEntry { public: - class Value final : public QMap - { - public: - using QMap::QMap; - QString toString(bool &ok) const noexcept; - bool toBoolean(bool &ok) const noexcept; - QString toIconString(bool &ok) const noexcept; - float toNumeric(bool &ok) const noexcept; - QString toLocaleString(const QLocale &locale, bool &ok) const noexcept; - friend QDebug operator<<(QDebug debug, const DesktopEntry::Value &v); - - private: - [[nodiscard]] static QString unescape(const QString &str) noexcept; - }; - + using Value = QVariant; DesktopEntry() = default; DesktopEntry(const DesktopEntry &) = default; DesktopEntry(DesktopEntry &&) = default; @@ -141,8 +128,6 @@ private: bool m_parsed{false}; }; -QDebug operator<<(QDebug debug, const DesktopEntry::Value &v); - bool operator==(const DesktopEntry &lhs, const DesktopEntry &rhs); bool operator!=(const DesktopEntry &lhs, const DesktopEntry &rhs); @@ -151,4 +136,16 @@ bool operator==(const DesktopFile &lhs, const DesktopFile &rhs); bool operator!=(const DesktopFile &lhs, const DesktopFile &rhs); +QString unescape(const QString &str) noexcept; + +QString toLocaleString(const QStringMap &localeMap, const QLocale &locale) noexcept; + +QString toString(const DesktopEntry::Value &value) noexcept; + +bool toBoolean(const DesktopEntry::Value &value, bool &ok) noexcept; + +QString toIconString(const DesktopEntry::Value &value) noexcept; + +float toNumeric(const DesktopEntry::Value &value, bool &ok) noexcept; + #endif diff --git a/src/desktopfileparser.cpp b/src/desktopfileparser.cpp index fa45a7e..2bb2092 100644 --- a/src/desktopfileparser.cpp +++ b/src/desktopfileparser.cpp @@ -5,6 +5,7 @@ #include #include "desktopfileparser.h" #include "constant.h" +#include "global.h" namespace { bool isInvalidLocaleString(const QString &str) noexcept @@ -25,6 +26,12 @@ bool isInvalidLocaleString(const QString &str) noexcept return re.match(str).hasMatch(); } +bool isLocaleString(const QString &key) noexcept +{ + static QSet localeSet{"Name", "GenericName", "Comment", "Keywords"}; + return localeSet.contains(key); +} + } // namespace ParserError DesktopFileParser::parse(Groups &ret) noexcept @@ -135,25 +142,41 @@ ParserError DesktopFileParser::addEntry(typename Groups::iterator &group) noexce // NOTE: https://stackoverflow.com/a/25583104 thread_local const QRegularExpression re = _re; if (re.match(key).hasMatch()) { - qWarning() << "invalid key name, skip this line:" << line; + qWarning() << "invalid key name:" << key << ", skip this line:" << line; return ParserError::NoError; } - if (localeStr != defaultKeyStr && !isInvalidLocaleString(localeStr)) { + if (localeStr != defaultKeyStr and !isInvalidLocaleString(localeStr)) { qWarning().noquote() << QString("invalid LOCALE (%2) for key \"%1\"").arg(key, localeStr); - } - - auto keyIt = group->find(key); - if (keyIt != group->end() && keyIt->find(localeStr) != keyIt->end()) { - qWarning() << "duplicated localestring, skip this line:" << line; return ParserError::NoError; } - if (keyIt == group->end()) { - group->insert(key, {{localeStr, valueStr}}); - return ParserError::NoError; + if (auto keyIt = group->find(key); keyIt != group->end()) { + if (!isLocaleString(key)) { + qWarning() << "duplicate key:" << key << "skip."; + return ParserError::NoError; + } + + if (!keyIt->canConvert()) { + qWarning() << "underlying type of value is invalid, raw value:" << *keyIt << "skip"; + return ParserError::NoError; + } + + auto localeMap = keyIt->value(); + if (auto valueIt = localeMap.find(localeStr) != localeMap.end()) { + qWarning() << "duplicate locale key:" << key << "skip."; + return ParserError::NoError; + } + + localeMap.insert(localeStr, valueStr); + group->insert(key, QVariant::fromValue(localeMap)); + } else { + if (isLocaleString(key)) { + group->insert(key, QVariant::fromValue(QStringMap{{localeStr, valueStr}})); + } else { + group->insert(key, QVariant::fromValue(valueStr)); + } } - keyIt->insert(localeStr, valueStr); return ParserError::NoError; } diff --git a/src/global.h b/src/global.h index b867d09..f9355ff 100644 --- a/src/global.h +++ b/src/global.h @@ -29,12 +29,12 @@ Q_DECLARE_LOGGING_CATEGORY(DDEAMProf) using ObjectInterfaceMap = QMap; using ObjectMap = QMap; -using KVPairs = QMap; -using PropMap = QMap; +using QStringMap = QMap; +using PropMap = QVariantMap; Q_DECLARE_METATYPE(ObjectInterfaceMap) Q_DECLARE_METATYPE(ObjectMap) -Q_DECLARE_METATYPE(KVPairs) +Q_DECLARE_METATYPE(QStringMap) Q_DECLARE_METATYPE(PropMap) struct SystemdUnitDBusMessage diff --git a/tests/main.cpp b/tests/main.cpp index 1da4fe8..2f2b413 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -15,7 +15,7 @@ void registerComplexDbusType() // FIXME: test shouldn't associate with DBus qDBusRegisterMetaType(); qRegisterMetaType(); qDBusRegisterMetaType(); - qDBusRegisterMetaType>(); + qDBusRegisterMetaType(); qRegisterMetaType(); qDBusRegisterMetaType(); qDBusRegisterMetaType(); diff --git a/tests/ut_desktopentry.cpp b/tests/ut_desktopentry.cpp index f29a883..50136c2 100644 --- a/tests/ut_desktopentry.cpp +++ b/tests/ut_desktopentry.cpp @@ -66,19 +66,20 @@ TEST_F(TestDesktopEntry, prase) auto name = group->constFind("Name"); ASSERT_NE(name, group->cend()); - bool ok; - name->toBoolean(ok); + const auto &nameVal = *name; + + bool ok{true}; + toBoolean(nameVal, ok); EXPECT_FALSE(ok); - name->toNumeric(ok); + toNumeric(nameVal, ok); EXPECT_FALSE(ok); - auto defaultName = name->toString(ok); - ASSERT_TRUE(ok); + EXPECT_TRUE(nameVal.canConvert()); + auto defaultName = toString(nameVal); // get default locale value. EXPECT_TRUE(defaultName == "Text Editor"); - auto localeString = name->toLocaleString(QLocale{"zh_CN"}, ok); - ASSERT_TRUE(ok); - + const auto &localeMap = nameVal.value(); + auto localeString = toLocaleString(nameVal.value(), QLocale{"zh_CN"}); EXPECT_TRUE(localeString == "文本编辑器"); }