Push metadata checks up to InterfaceTest class for reuse

10 files changed, 182 insertions, 140 deletions

diff --git a/mpris2/interfacetest.cpp b/mpris2/interfacetest.cpp
index c6af22e..c370fc0 100644
--- a/mpris2/interfacetest.cpp
+++ b/mpris2/interfacetest.cpp
@@ -323,4 +323,140 @@ void InterfaceTest::delayedIncrementalCheck()
     outOfDateProperties.clear();
 }
 
+void InterfaceTest::checkMetadata(const QVariantMap& metadata,
+                                  QStringList* errors,
+                                  QStringList* warnings,
+                                  QStringList* infoMessages)
+{
+    if (!metadata.contains("mpris:trackid")) {
+        (*errors) << "No mpris:trackid entry";
+    } else if (metadata.value("mpris:trackid").userType() != qMetaTypeId<QDBusObjectPath>()) {
+        (*errors) << "mpris:trackid entry was not sent as a D-Bus object path (D-Bus type 'o')";
+    } else if (metadata.value("mpris:trackid").value<QDBusObjectPath>().path().isEmpty()) {
+        (*errors) << "mpris:trackid entry is an empty path";
+    }
+
+    checkMetadataEntry(metadata, "mpris:length", QVariant::LongLong, errors, warnings, infoMessages);
+
+    if (checkMetadataEntry(metadata, "mpris:artUrl", QVariant::Url, errors, warnings, infoMessages)) {
+        QString artUrl = metadata.value("mpris:artUrl").toString();
+        QUrl asUrl(artUrl, QUrl::StrictMode);
+        if (asUrl.scheme() != "file" && asUrl.scheme() != "http" && asUrl.scheme() != "https") {
+            (*infoMessages) << "mpris:artUrl has a scheme (" + asUrl.scheme() + ") which not all clients may recognise";
+        } else {
+            if (asUrl.scheme() == "file") {
+                if (!QFile::exists(asUrl.toLocalFile())) {
+                    (*infoMessages) << "mpris:artUrl references a file that does not exist";
+                }
+            }
+            // TODO: check network files
+        }
+    }
+
+    Q_FOREACH( QString key, metadata.keys() ) {
+        if (!key.startsWith("xesam:")) {
+            if (key != "mpris:trackid" &&
+                key != "mpris:length" &&
+                key != "mpris:artUrl")
+            {
+                (*warnings) << "Unrecognised entry " + key;
+            }
+        }
+    }
+
+    checkMetadataEntry(metadata, "xesam:album", QVariant::String, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:albumArtist", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:artist", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:asText", QVariant::String, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:audioBpm", QVariant::Int, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:autoRating", QVariant::Double, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:comment", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:composer", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:contentCreator", QVariant::DateTime, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:discNumber", QVariant::Int, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:firstUsed", QVariant::DateTime, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:genre", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:lastUsed", QVariant::DateTime, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:lyricist", QVariant::StringList, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:title", QVariant::String, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:trackNumber", QVariant::Int, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:url", QVariant::Url, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:useCount", QVariant::Int, errors, warnings, infoMessages);
+    checkMetadataEntry(metadata, "xesam:userRating", QVariant::Double, errors, warnings, infoMessages);
+}
+
+bool InterfaceTest::checkMetadataEntry(const QVariantMap& metadata,
+                                       const QString& entry,
+                                       QVariant::Type expType,
+                                       QStringList* errors,
+                                       QStringList* warnings,
+                                       QStringList* infoMessages)
+{
+    if (metadata.contains(entry)) {
+        QVariant value = metadata.value(entry);
+
+        bool propertyTypeError = false;
+        bool propertyTypeWarning = false;
+        QVariant::Type realExpectedType = expType;
+        if (expType == QVariant::DateTime || expType == QVariant::Url) {
+            realExpectedType = QVariant::String;
+        }
+
+        // be lax about integers
+        if (realExpectedType == QVariant::Int) {
+            if (value.type() == QVariant::UInt ||
+                value.type() == QVariant::LongLong ||
+                value.type() == QVariant::ULongLong)
+            {
+                propertyTypeWarning = true;
+            } else if (value.type() != QVariant::Int) {
+                propertyTypeError = true;
+            }
+        } else if (realExpectedType == QVariant::UInt || realExpectedType == QVariant::LongLong) {
+            if (value.type() == QVariant::ULongLong) {
+                propertyTypeWarning = true;
+            } else if (value.type() != realExpectedType) {
+                propertyTypeError = true;
+            }
+        } else if (value.type() != realExpectedType) {
+            propertyTypeError = true;
+        }
+
+        if (propertyTypeError || propertyTypeWarning) {
+            const char * gotTypeCh = QDBusMetaType::typeToSignature(value.userType());
+            QString gotType = gotTypeCh ? QString::fromAscii(gotTypeCh) : "<unknown>";
+            const char * expTypeCh = QDBusMetaType::typeToSignature(realExpectedType);
+            QString expType = expTypeCh ? QString::fromAscii(expTypeCh) : "<unknown>";
+            if (propertyTypeError) {
+                (*errors) << entry + " entry is of type '" + gotType + "' but should have been of type '" + expType + "'";
+                return false;
+            } else {
+                (*warnings) << entry + " entry is of type '" + gotType + "' but should have been of type '" + expType + "'";
+                return true;
+            }
+        }
+
+        // extra checks for special types
+        if (expType == QVariant::DateTime) {
+            QDateTime dtValue = QDateTime::fromString(value.toString(), Qt::ISODate);
+            if (!dtValue.isValid()) {
+                (*errors) << entry + " entry does not contain a valid date/time string (value was " + value.toString() + ")";
+                return false;
+            }
+        } else if (expType == QVariant::Url) {
+            if (value.toString().isEmpty()) {
+                return false;
+            } else {
+                QUrl asUrl(value.toString(), QUrl::StrictMode);
+                if (!asUrl.isValid()) {
+                    (*errors) << entry + " entry is not a valid URL";
+                    return false;
+                }
+            }
+        }
+        return true;
+    }
+    return false;
+}
 
+// vim:et:sw=4:sts=4
diff --git a/mpris2/interfacetest.h b/mpris2/interfacetest.h
index 6e4d8e8..39f0fd6 100644
--- a/mpris2/interfacetest.h
+++ b/mpris2/interfacetest.h
@@ -125,11 +125,21 @@ namespace Mpris2
             PropAllowErrors = PropAllowMissing | PropAllowReadOnly
         };
         bool getAllProps();
-        bool getProp(const QString& propName, PropErrorAllowance allowError = PropDisallowErrors);
-        bool setProp(const QString& propName, const QDBusVariant& value, PropErrorAllowance allowError = PropAllowReadOnly);
-
-        bool checkPropValid(const QString& propName, QVariant::Type expType, const QVariantMap& oldProps = QVariantMap());
-        bool checkNonEmptyStringPropValid(const QString& propName, const QVariantMap& oldProps = QVariantMap());
+        bool getProp(const QString& propName,
+                     PropErrorAllowance allowError = PropDisallowErrors);
+        bool setProp(const QString& propName,
+                     const QDBusVariant& value,
+                     PropErrorAllowance allowError = PropAllowReadOnly);
+
+        bool checkPropValid(const QString& propName,
+                            QVariant::Type expType,
+                            const QVariantMap& oldProps = QVariantMap());
+        bool checkNonEmptyStringPropValid(const QString& propName,
+                                          const QVariantMap& oldProps = QVariantMap());
+        void checkMetadata(const QVariantMap& metadata,
+                           QStringList* errors,
+                           QStringList* warnings,
+                           QStringList* infoMessages);
 
         virtual void checkProps(const QVariantMap& oldProps = QVariantMap()) = 0;
         virtual void checkUpdatedProperty(const QString& propName) = 0;
@@ -142,6 +152,13 @@ namespace Mpris2
         QStringList     propsNotUpdated;
 
     private:
+        bool checkMetadataEntry(const QVariantMap& metadata,
+                                const QString& entry,
+                                QVariant::Type type,
+                                QStringList* errors,
+                                QStringList* warnings,
+                                QStringList* infoMessages);
+
         QDBusInterface*    propsIface;
         QTimer*            delayedCheckTimer;
         QMap<QString,uint> propertyUpdateWarningCount;
@@ -149,3 +166,4 @@ namespace Mpris2
 }
 
 #endif // INTERFACETEST_H
+// vim:et:sw=4:sts=4
diff --git a/mpris2/playerinterfacetest.cpp b/mpris2/playerinterfacetest.cpp
index 08469de..d52214c 100644
--- a/mpris2/playerinterfacetest.cpp
+++ b/mpris2/playerinterfacetest.cpp
@@ -279,144 +279,23 @@ void PlayerInterfaceTest::checkMetadata(const QVariantMap& oldProps)
     }
     if (metadata.isEmpty()) {
         emit interfaceInfo(Property, "Metadata",
-                           "No metadata provided");
+                           "No metadata provided for the current track");
         return;
     }
 
-    if (!metadata.contains("mpris:trackid")) {
-        emit interfaceError(Property, "Metadata",
-                            "No mpris:trackid entry for the current track");
-    } else if (metadata.value("mpris:trackid").userType() != qMetaTypeId<QDBusObjectPath>()) {
-        emit interfaceError(Property, "Metadata",
-                            "mpris:trackid entry was not sent as a D-Bus object path (D-Bus type 'o')");
-    } else if (metadata.value("mpris:trackid").value<QDBusObjectPath>().path().isEmpty()) {
-        emit interfaceError(Property, "Metadata",
-                            "mpris:trackid entry is an empty path");
+    QStringList errors;
+    QStringList warnings;
+    QStringList infoMessages;
+    InterfaceTest::checkMetadata(metadata, &errors, &warnings, &infoMessages);
+    Q_FOREACH (const QString& message, errors) {
+        emit interfaceError(Property, "Metadata", message);
     }
-
-    checkMetadataEntry(metadata, "mpris:length", QVariant::LongLong);
-
-    if (checkMetadataEntry(metadata, "mpris:artUrl", QVariant::Url)) {
-        QString artUrl = metadata.value("mpris:artUrl").toString();
-        QUrl asUrl(artUrl, QUrl::StrictMode);
-        if (asUrl.scheme() != "file" && asUrl.scheme() != "http" && asUrl.scheme() != "https") {
-            emit interfaceInfo(Property, "Metadata",
-                                "mpris:artUrl has a scheme (" + asUrl.scheme() + ") which not all clients may recognise");
-        } else {
-            if (asUrl.scheme() == "file") {
-                if (!QFile::exists(asUrl.toLocalFile())) {
-                    emit interfaceInfo(Property, "Metadata",
-                                        "mpris:artUrl references a file that does not exist");
-                }
-            }
-            // TODO: check network files
-        }
-    }
-
-    Q_FOREACH( QString key, metadata.keys() ) {
-        if (!key.startsWith("xesam:")) {
-            if (key != "mpris:trackid" &&
-                key != "mpris:length" &&
-                key != "mpris:artUrl")
-            {
-                emit interfaceWarning(Property, "Metadata",
-                                      "Unrecognised entry " + key);
-            }
-        }
+    Q_FOREACH (const QString& message, warnings) {
+        emit interfaceWarning(Property, "Metadata", message);
     }
-
-    checkMetadataEntry(metadata, "xesam:album", QVariant::String);
-    checkMetadataEntry(metadata, "xesam:albumArtist", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:artist", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:asText", QVariant::String);
-    checkMetadataEntry(metadata, "xesam:audioBpm", QVariant::Int);
-    checkMetadataEntry(metadata, "xesam:autoRating", QVariant::Double);
-    checkMetadataEntry(metadata, "xesam:comment", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:composer", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:contentCreator", QVariant::DateTime);
-    checkMetadataEntry(metadata, "xesam:discNumber", QVariant::Int);
-    checkMetadataEntry(metadata, "xesam:firstUsed", QVariant::DateTime);
-    checkMetadataEntry(metadata, "xesam:genre", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:lastUsed", QVariant::DateTime);
-    checkMetadataEntry(metadata, "xesam:lyricist", QVariant::StringList);
-    checkMetadataEntry(metadata, "xesam:title", QVariant::String);
-    checkMetadataEntry(metadata, "xesam:trackNumber", QVariant::Int);
-    checkMetadataEntry(metadata, "xesam:url", QVariant::Url);
-    checkMetadataEntry(metadata, "xesam:useCount", QVariant::Int);
-    checkMetadataEntry(metadata, "xesam:userRating", QVariant::Double);
-}
-
-bool PlayerInterfaceTest::checkMetadataEntry(const QVariantMap& metadata, const QString& entry, QVariant::Type expType)
-{
-    if (metadata.contains(entry)) {
-        QVariant value = metadata.value(entry);
-
-        bool propertyTypeError = false;
-        bool propertyTypeWarning = false;
-        QVariant::Type realExpectedType = expType;
-        if (expType == QVariant::DateTime || expType == QVariant::Url) {
-            realExpectedType = QVariant::String;
-        }
-
-        // be lax about integers
-        if (realExpectedType == QVariant::Int) {
-            if (value.type() == QVariant::UInt ||
-                value.type() == QVariant::LongLong ||
-                value.type() == QVariant::ULongLong)
-            {
-                propertyTypeWarning = true;
-            } else if (value.type() != QVariant::Int) {
-                propertyTypeError = true;
-            }
-        } else if (realExpectedType == QVariant::UInt || realExpectedType == QVariant::LongLong) {
-            if (value.type() == QVariant::ULongLong) {
-                propertyTypeWarning = true;
-            } else if (value.type() != realExpectedType) {
-                propertyTypeError = true;
-            }
-        } else if (value.type() != realExpectedType) {
-            propertyTypeError = true;
-        }
-
-        if (propertyTypeError || propertyTypeWarning) {
-            const char * gotTypeCh = QDBusMetaType::typeToSignature(value.userType());
-            QString gotType = gotTypeCh ? QString::fromAscii(gotTypeCh) : "<unknown>";
-            const char * expTypeCh = QDBusMetaType::typeToSignature(realExpectedType);
-            QString expType = expTypeCh ? QString::fromAscii(expTypeCh) : "<unknown>";
-            if (propertyTypeError) {
-                emit interfaceError(Property, "Metadata",
-                                    entry + " entry is of type '" + gotType + "' but should have been of type '" + expType + "'");
-                return false;
-            } else {
-                emit interfaceWarning(Property, "Metadata",
-                                      entry + " entry is of type '" + gotType + "' but should have been of type '" + expType + "'");
-                return true;
-            }
-        }
-
-        // extra checks for special types
-        if (expType == QVariant::DateTime) {
-            QDateTime dtValue = QDateTime::fromString(value.toString(), Qt::ISODate);
-            if (!dtValue.isValid()) {
-                emit interfaceError(Property, "Metadata",
-                                    entry + " entry does not contain a valid date/time string (value was " + value.toString() + ")");
-                return false;
-            }
-        } else if (expType == QVariant::Url) {
-            if (value.toString().isEmpty()) {
-                return false;
-            } else {
-                QUrl asUrl(value.toString(), QUrl::StrictMode);
-                if (!asUrl.isValid()) {
-                    emit interfaceError(Property, "Metadata",
-                                        entry + " entry is not a valid URL");
-                    return false;
-                }
-            }
-        }
-        return true;
+    Q_FOREACH (const QString& message, infoMessages) {
+        emit interfaceInfo(Property, "Metadata", message);
     }
-    return false;
 }
 
 void PlayerInterfaceTest::checkPosition(const QVariantMap& oldProps)
@@ -819,3 +698,4 @@ void PlayerInterfaceTest::testSetRate(double rate)
     }
 }
 
+// vim:et:sw=4:sts=4
diff --git a/mpris2/playerinterfacetest.h b/mpris2/playerinterfacetest.h
index 34656f9..c021edc 100644
--- a/mpris2/playerinterfacetest.h
+++ b/mpris2/playerinterfacetest.h
@@ -75,7 +75,6 @@ namespace Mpris2 {
         void checkRateConsistency(const QVariantMap& oldProps = QVariantMap());
         void checkPositionConsistency(const QVariantMap& oldProps = QVariantMap());
         void checkPredictedPosition();
-        bool checkMetadataEntry(const QVariantMap& metadata, const QString& entry, QVariant::Type type);
         void updateCurrentRate();
 
         qint64 m_pos;
@@ -85,3 +84,4 @@ namespace Mpris2 {
 }
 
 #endif // MPRIS2_PLAYERINTERFACETEST_H
+// vim:et:sw=4:sts=4
diff --git a/mpris2/playertestwidget.cpp b/mpris2/playertestwidget.cpp
index ad7a0ab..5d39626 100644
--- a/mpris2/playertestwidget.cpp
+++ b/mpris2/playertestwidget.cpp
@@ -225,3 +225,5 @@ void PlayerTestWidget::testSetRate()
 {
     test->testSetRate(ui.rateSpinBox->value());
 }
+
+// vim:et:sw=4:sts=4
diff --git a/mpris2/playertestwidget.h b/mpris2/playertestwidget.h
index 6a70006..92a26e9 100644
--- a/mpris2/playertestwidget.h
+++ b/mpris2/playertestwidget.h
@@ -66,3 +66,4 @@ namespace Mpris2 {
 }
 
 #endif // MPRIS2_PLAYERTESTWIDGET_H
+// vim:et:sw=4:sts=4
diff --git a/mpris2/roottestwidget.cpp b/mpris2/roottestwidget.cpp
index ebdd144..cf70fd8 100644
--- a/mpris2/roottestwidget.cpp
+++ b/mpris2/roottestwidget.cpp
@@ -85,3 +85,4 @@ void RootTestWidget::propertiesChanged(const QStringList& properties)
     }
 }
 
+// vim:et:sw=4:sts=4
diff --git a/mpris2/roottestwidget.h b/mpris2/roottestwidget.h
index 513ecb9..00a7269 100644
--- a/mpris2/roottestwidget.h
+++ b/mpris2/roottestwidget.h
@@ -28,7 +28,7 @@ namespace Mpris2
     class RootInterfaceTest;
     class RootTestWidget : public QWidget
     {
-	    Q_OBJECT
+        Q_OBJECT
 
     public:
         RootTestWidget(RootInterfaceTest *test, QWidget *parent = 0);
@@ -48,3 +48,4 @@ namespace Mpris2
 }
 
 #endif // ROOTTESTWIDGET_H
+// vim:et:sw=4:sts=4
diff --git a/mpris2/testconsole.cpp b/mpris2/testconsole.cpp
index 5726000..a836d90 100644
--- a/mpris2/testconsole.cpp
+++ b/mpris2/testconsole.cpp
@@ -99,3 +99,5 @@ void TestConsole::interfaceInfo(InterfaceTest::LocationType locType, const QStri
     cursor.insertText(desc);
     cursor.insertBlock();
 }
+
+// vim:et:sw=4:sts=4
diff --git a/mpris2/testconsole.h b/mpris2/testconsole.h
index d62c54f..afa6da6 100644
--- a/mpris2/testconsole.h
+++ b/mpris2/testconsole.h
@@ -30,7 +30,7 @@ namespace Mpris2
 {
     class TestConsole : public QWidget
     {
-	    Q_OBJECT
+        Q_OBJECT
 
     public:
         TestConsole(QWidget* parent = 0);
@@ -52,3 +52,4 @@ namespace Mpris2
 }
 
 #endif // TESTCONSOLE_H
+// vim:et:sw=4:sts=4