refact: change underlying type of desktop entry

Signed-off-by: ComixHe <heyuming@deepin.org>

api/dbus/org.desktopspec.MimeManager1.xml

@@ -10,7 +10,7 @@
 
         <method name="setDefaultApplication">
             <arg type="a{ss}" name="defaultApps" direction="in"/>
-            <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="KVPairs"/>
+            <annotation name="org.qtproject.QtDBus.QtTypeName.In0" value="QStringMap"/>
         </method>
 
         <method name="unsetDefaultApplication">

apps/dde-application-manager/src/main.cpp

@@ -21,7 +21,8 @@ void registerComplexDbusType()
     qDBusRegisterMetaType<ObjectInterfaceMap>();
     qRegisterMetaType<ObjectMap>();
     qDBusRegisterMetaType<ObjectMap>();
-    qDBusRegisterMetaType<KVPairs>();
+    qDBusRegisterMetaType<QStringMap>();
+    qRegisterMetaType<QStringMap>();
     qRegisterMetaType<PropMap>();
     qDBusRegisterMetaType<PropMap>();
     qDBusRegisterMetaType<QDBusObjectPath>();

src/applicationchecker.cpp

@@ -15,9 +15,9 @@ bool ApplicationFilter::hiddenCheck(const std::unique_ptr<DesktopEntry> &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<DesktopEntry> &entry)
 {
     auto tryExecVal = entry->value(DesktopFileEntryKey, "TryExec");
     if (tryExecVal.has_value()) {
-        bool ok{false};
+        auto executable = toString(tryExecVal.value());
-        auto executable = tryExecVal.value().toString(ok);
+        if (executable.isEmpty()) {
-        if (!ok) {
+            qWarning() << "invalid TryExec value:" << tryExecVal.value();
-            qWarning() << "invalid TryExec value:" << *tryExecVal.value().find(defaultKeyStr);
             return false;
         }
 
@@ -56,10 +55,9 @@ bool ApplicationFilter::showInCheck(const std::unique_ptr<DesktopEntry> &entry)
     bool showInCurrentDE{true};
     auto onlyShowInVal = entry->value(DesktopFileEntryKey, "OnlyShowIn");
     while (onlyShowInVal.has_value()) {
-        bool ok{false};
+        auto deStr = toString(onlyShowInVal.value());
-        auto deStr = onlyShowInVal.value().toString(ok);
+        if (deStr.isEmpty()) {
-        if (!ok) {
+            qWarning() << "invalid OnlyShowIn value:" << onlyShowInVal.value();
-            qWarning() << "invalid OnlyShowIn value:" << *onlyShowInVal.value().find(defaultKeyStr);
             break;
         }
 
@@ -79,10 +77,9 @@ bool ApplicationFilter::showInCheck(const std::unique_ptr<DesktopEntry> &entry)
     bool notShowInCurrentDE{false};
     auto notShowInVal = entry->value(DesktopFileEntryKey, "NotShowIn");
     while (notShowInVal.has_value()) {
-        bool ok{false};
+        auto deStr = toString(notShowInVal.value());
-        auto deStr = notShowInVal.value().toString(ok);
+        if (deStr.isEmpty()) {
-        if (!ok) {
+            qWarning() << "invalid OnlyShowIn value:" << notShowInVal.value();
-            qWarning() << "invalid OnlyShowIn value:" << *notShowInVal.value().find(defaultKeyStr);
             break;
         }
 

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;
+        const auto &QStringMap = it->second;
-        for (auto inner = kvPairs.constKeyValueBegin(); inner != kvPairs.constKeyValueEnd(); ++inner) {
+        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));

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) {
+        execStr = toString(actionExec.value());
-            qWarning() << "exec value to string failed, try default action.";
+        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);
+        execStr = toString(Actions.value());
-        if (!ok) {
+        if (execStr.isEmpty()) {
             QString msg{"maybe entry actions's format is invalid, abort launch."};
             qWarning() << msg;
             sendErrorReply(QDBusError::Failed, msg);
@@ -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;
@@ -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);
+        auto iconStr = toIconString(val.value());
-        if (!ok) {
+        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);
+        const auto &rawValue = val.value();
-        if (!ok) {
+        if (!rawValue.canConvert<QStringMap>()) {
+            qDebug() << "Name's underlying type mismatch:"
+                     << "QStringMap" << rawValue.metaType().name();
+            task.command.append(std::move(execList));
+            return task;
+        }
+
+        auto NameStr = toLocaleString(rawValue.value<QStringMap>(), 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);
+        auto valStr = toString(val);
-        if (ok) {
+        if (!valStr.isEmpty()) {
             ret = QVariant::fromValue(valStr);
         }
     } break;
     case EntryValueType::LocaleString: {
-        auto valStr = val.toLocaleString(locale, ok);
+        if (!val.canConvert<QStringMap>()) {
-        if (ok) {
+            return ret;
+        }
+        auto valStr = toLocaleString(val.value<QStringMap>(), 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);
+        auto valStr = toIconString(val);
-        if (ok) {
+        if (!valStr.isEmpty()) {
             ret = QVariant::fromValue(valStr);
         }
     } break;

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(

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:

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 <QFileInfo>
 #include <QDir>
@@ -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;
+            qInfo() << "key:" << k << "value:" << v;
-            for (auto valIt = v.constKeyValueBegin(); valIt != v.constKeyValueEnd(); ++valIt) {
-                const auto &[key, value] = *valIt;
-                qInfo() << key << value;
-            }
         }
         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> 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;
+    QString str;
-    auto str = this->find(defaultKeyStr);
 
-    if (str == this->end()) {
+    if (value.canConvert<QStringMap>()) {  // get default locale
+        str = value.value<QStringMap>()[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 = localeMap.constKeyValueBegin(); it != localeMap.constKeyValueEnd(); ++it) {
-    for (auto it = this->constKeyValueBegin(); it != this->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];
+    return value.toFloat(&ok);
-    QVariant v{str};
-    return v.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<const QMap<QString, QString> &>(v);
-    return debug;
-}

src/desktopentry.h

@@ -13,6 +13,7 @@
 #include <optional>
 #include <QFile>
 #include "iniParser.h"
+#include "global.h"
 
 constexpr static auto defaultKeyStr = "default";
 
@@ -105,21 +106,7 @@ private:
 class DesktopEntry
 {
 public:
-    class Value final : public QMap<QString, QString>
+    using Value = QVariant;
-    {
-    public:
-        using QMap<QString, QString>::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;
-    };
-
     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

src/desktopfileparser.cpp

@@ -5,6 +5,7 @@
 #include <QRegularExpression>
 #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<QString> 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()) {
+    if (auto keyIt = group->find(key); keyIt != group->end()) {
-        group->insert(key, {{localeStr, valueStr}});
+        if (!isLocaleString(key)) {
+            qWarning() << "duplicate key:" << key << "skip.";
             return ParserError::NoError;
         }
 
-    keyIt->insert(localeStr, valueStr);
+        if (!keyIt->canConvert<QStringMap>()) {
+            qWarning() << "underlying type of value is invalid, raw value:" << *keyIt << "skip";
+            return ParserError::NoError;
+        }
+
+        auto localeMap = keyIt->value<QStringMap>();
+        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));
+        }
+    }
+
     return ParserError::NoError;
 }

src/global.h

@@ -29,12 +29,12 @@ Q_DECLARE_LOGGING_CATEGORY(DDEAMProf)
 
 using ObjectInterfaceMap = QMap<QString, QVariantMap>;
 using ObjectMap = QMap<QDBusObjectPath, ObjectInterfaceMap>;
-using KVPairs = QMap<QString, QString>;
+using QStringMap = QMap<QString, QString>;
-using PropMap = QMap<QString, KVPairs>;
+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

tests/main.cpp

@@ -15,7 +15,7 @@ void registerComplexDbusType()  // FIXME: test shouldn't associate with DBus
     qDBusRegisterMetaType<ObjectInterfaceMap>();
     qRegisterMetaType<ObjectMap>();
     qDBusRegisterMetaType<ObjectMap>();
-    qDBusRegisterMetaType<QMap<QString, QString>>();
+    qDBusRegisterMetaType<QStringMap>();
     qRegisterMetaType<PropMap>();
     qDBusRegisterMetaType<PropMap>();
     qDBusRegisterMetaType<QDBusObjectPath>();

tests/ut_desktopentry.cpp

@@ -66,19 +66,20 @@ TEST_F(TestDesktopEntry, prase)
     auto name = group->constFind("Name");
     ASSERT_NE(name, group->cend());
 
-    bool ok;
+    const auto &nameVal = *name;
-    name->toBoolean(ok);
+
+    bool ok{true};
+    toBoolean(nameVal, ok);
     EXPECT_FALSE(ok);
 
-    name->toNumeric(ok);
+    toNumeric(nameVal, ok);
     EXPECT_FALSE(ok);
 
-    auto defaultName = name->toString(ok);
+    EXPECT_TRUE(nameVal.canConvert<QStringMap>());
-    ASSERT_TRUE(ok);
+    auto defaultName = toString(nameVal);  // get default locale value.
     EXPECT_TRUE(defaultName == "Text Editor");
 
-    auto localeString = name->toLocaleString(QLocale{"zh_CN"}, ok);
+    const auto &localeMap = nameVal.value<QStringMap>();
-    ASSERT_TRUE(ok);
+    auto localeString = toLocaleString(nameVal.value<QStringMap>(), QLocale{"zh_CN"});
-
     EXPECT_TRUE(localeString == "文本编辑器");
 }