NOISSUE improve GradleSpecifier and handle broken library names

8 files changed, 68 insertions, 48 deletions

diff --git a/api/logic/minecraft/GradleSpecifier.h b/api/logic/minecraft/GradleSpecifier.h
index 959325c6..60e0a726 100644
--- a/api/logic/minecraft/GradleSpecifier.h
+++ b/api/logic/minecraft/GradleSpecifier.h
@@ -18,32 +18,35 @@ struct GradleSpecifier
     {
         /*
         org.gradle.test.classifiers : service : 1.0 : jdk15 @ jar
-        DEBUG   0 "org.gradle.test.classifiers:service:1.0:jdk15@jar" 
-        DEBUG   1 "org.gradle.test.classifiers" 
-        DEBUG   2 "service" 
-        DEBUG   3 "1.0" 
-        DEBUG   4 ":jdk15" 
-        DEBUG   5 "jdk15" 
-        DEBUG   6 "@jar" 
-        DEBUG   7 "jar"
+         0 "org.gradle.test.classifiers:service:1.0:jdk15@jar"
+         1 "org.gradle.test.classifiers"
+         2 "service"
+         3 "1.0"
+         4 "jdk15"
+         5 "jar"
         */
-        QRegExp matcher("([^:@]+):([^:@]+):([^:@]+)" "(:([^:@]+))?" "(@([^:@]+))?");
+        QRegExp matcher("([^:@]+):([^:@]+):([^:@]+)" "(?::([^:@]+))?" "(?:@([^:@]+))?");
         m_valid = matcher.exactMatch(value);
+        if(!m_valid) {
+            m_invalidValue = value;
+            return *this;
+        }
         auto elements = matcher.capturedTexts();
         m_groupId = elements[1];
         m_artifactId = elements[2];
         m_version = elements[3];
-        m_classifier = elements[5];
-        if(!elements[7].isEmpty())
+        m_classifier = elements[4];
+        if(!elements[5].isEmpty())
         {
-            m_extension = elements[7];
+            m_extension = elements[5];
         }
         return *this;
     }
-    operator QString() const
+    QString serialize() const
     {
-        if(!m_valid)
-            return "INVALID";
+        if(!m_valid) {
+            return m_invalidValue;
+        }
         QString retval = m_groupId + ":" + m_artifactId + ":" + m_version;
         if(!m_classifier.isEmpty())
         {
@@ -57,6 +60,9 @@ struct GradleSpecifier
     }
     QString getFileName() const
     {
+        if(!m_valid) {
+            return QString();
+        }
         QString filename = m_artifactId + '-' + m_version;
         if(!m_classifier.isEmpty())
         {
@@ -67,8 +73,9 @@ struct GradleSpecifier
     }
     QString toPath(const QString & filenameOverride = QString()) const
     {
-        if(!m_valid)
-            return "INVALID";
+        if(!m_valid) {
+            return QString();
+        }
         QString filename;
         if(filenameOverride.isEmpty())
         {
@@ -134,6 +141,7 @@ struct GradleSpecifier
         return true;
     }
 private:
+    QString m_invalidValue;
     QString m_groupId;
     QString m_artifactId;
     QString m_version;
diff --git a/api/logic/minecraft/GradleSpecifier_test.cpp b/api/logic/minecraft/GradleSpecifier_test.cpp
index f49ec718..0900c9d8 100644
--- a/api/logic/minecraft/GradleSpecifier_test.cpp
+++ b/api/logic/minecraft/GradleSpecifier_test.cpp
@@ -31,7 +31,7 @@ slots:
     {
         QFETCH(QString, through);
 
-        QString converted = GradleSpecifier(through);
+        QString converted = GradleSpecifier(through).serialize();
 
         QCOMPARE(converted, through);
     }
@@ -68,7 +68,8 @@ slots:
 
         GradleSpecifier spec(input);
         QVERIFY(!spec.valid());
-        QCOMPARE(spec.operator QString(), QString("INVALID"));
+        QCOMPARE(spec.serialize(), input);
+        QCOMPARE(spec.toPath(), QString());
     }
 };
 
diff --git a/api/logic/minecraft/Library.cpp b/api/logic/minecraft/Library.cpp
index b3c7657c..f2293679 100644
--- a/api/logic/minecraft/Library.cpp
+++ b/api/logic/minecraft/Library.cpp
@@ -94,13 +94,13 @@ QList< std::shared_ptr< NetAction > > Library::getDownloads(
             auto rawSha1 = QByteArray::fromHex(sha1.toLatin1());
             auto dl = Net::Download::makeCached(url, entry, options);
             dl->addValidator(new Net::ChecksumValidator(QCryptographicHash::Sha1, rawSha1));
-            qDebug() << "Checksummed Download for:" << rawName() << "storage:" << storage << "url:" << url;
+            qDebug() << "Checksummed Download for:" << rawName().serialize() << "storage:" << storage << "url:" << url;
             out.append(dl);
         }
         else
         {
             out.append(Net::Download::makeCached(url, entry, options));
-            qDebug() << "Download for:" << rawName() << "storage:" << storage << "url:" << url;
+            qDebug() << "Download for:" << rawName().serialize() << "storage:" << storage << "url:" << url;
         }
         return true;
     };
@@ -145,7 +145,7 @@ QList< std::shared_ptr< NetAction > > Library::getDownloads(
             }
             else
             {
-                qDebug() << "Ignoring native library" << m_name << "because it has no classifier for current OS";
+                qDebug() << "Ignoring native library" << m_name.serialize() << "because it has no classifier for current OS";
             }
         }
         else
@@ -157,7 +157,7 @@ QList< std::shared_ptr< NetAction > > Library::getDownloads(
             }
             else
             {
-                qDebug() << "Ignoring java library" << m_name << "because it has no artifact";
+                qDebug() << "Ignoring java library" << m_name.serialize() << "because it has no artifact";
             }
         }
     }
diff --git a/api/logic/minecraft/Library_test.cpp b/api/logic/minecraft/Library_test.cpp
index c3d6150d..75bb4db1 100644
--- a/api/logic/minecraft/Library_test.cpp
+++ b/api/logic/minecraft/Library_test.cpp
@@ -18,7 +18,8 @@ private:
         jsonFile.open(QIODevice::ReadOnly);
         auto data = jsonFile.readAll();
         jsonFile.close();
-        return MojangVersionFormat::libraryFromJson(QJsonDocument::fromJson(data).object(), file);
+        ProblemContainer problems;
+        return MojangVersionFormat::libraryFromJson(problems, QJsonDocument::fromJson(data).object(), file);
     }
     // get absolute path to expected storage, assuming default cache prefix
     QStringList getStorage(QString relative)
diff --git a/api/logic/minecraft/MojangVersionFormat.cpp b/api/logic/minecraft/MojangVersionFormat.cpp
index 33d3c54c..f9cb2228 100644
--- a/api/logic/minecraft/MojangVersionFormat.cpp
+++ b/api/logic/minecraft/MojangVersionFormat.cpp
@@ -220,7 +220,7 @@ VersionFilePtr MojangVersionFormat::versionFileFromJson(const QJsonDocument &doc
         {
             auto libObj = requireObject(libVal);
 
-            auto lib = MojangVersionFormat::libraryFromJson(libObj, filename);
+            auto lib = MojangVersionFormat::libraryFromJson(*out, libObj, filename);
             out->libraries.append(lib);
         }
     }
@@ -283,14 +283,18 @@ QJsonDocument MojangVersionFormat::versionFileToJson(const VersionFilePtr &patch
     }
 }
 
-LibraryPtr MojangVersionFormat::libraryFromJson(const QJsonObject &libObj, const QString &filename)
+LibraryPtr MojangVersionFormat::libraryFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename)
 {
     LibraryPtr out(new Library());
     if (!libObj.contains("name"))
     {
         throw JSONValidationError(filename + "contains a library that doesn't have a 'name' field");
     }
-    out->m_name = libObj.value("name").toString();
+    auto rawName = libObj.value("name").toString();
+    out->m_name = rawName;
+    if(!out->m_name.valid()) {
+        problems.addProblem(ProblemSeverity::Error, QObject::tr("Library %1 name is broken and cannot be processed.").arg(rawName));
+    }
 
     Bits::readString(libObj, "url", out->m_repositoryURL);
     if (libObj.contains("extract"))
@@ -333,7 +337,7 @@ LibraryPtr MojangVersionFormat::libraryFromJson(const QJsonObject &libObj, const
 QJsonObject MojangVersionFormat::libraryToJson(Library *library)
 {
     QJsonObject libRoot;
-    libRoot.insert("name", (QString)library->m_name);
+    libRoot.insert("name", library->m_name.serialize());
     if (!library->m_repositoryURL.isEmpty())
     {
         libRoot.insert("url", library->m_repositoryURL);
diff --git a/api/logic/minecraft/MojangVersionFormat.h b/api/logic/minecraft/MojangVersionFormat.h
index 76c529e9..2871dae4 100644
--- a/api/logic/minecraft/MojangVersionFormat.h
+++ b/api/logic/minecraft/MojangVersionFormat.h
@@ -3,6 +3,7 @@
 #include <minecraft/VersionFile.h>
 #include <minecraft/Library.h>
 #include <QJsonDocument>
+#include <ProblemProvider.h>
 
 #include "multimc_logic_export.h"
 
@@ -20,6 +21,6 @@ public:
     static QJsonDocument versionFileToJson(const VersionFilePtr &patch);
 
     // libraries
-    static LibraryPtr libraryFromJson(const QJsonObject &libObj, const QString &filename);
+    static LibraryPtr libraryFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename);
     static QJsonObject libraryToJson(Library *library);
 };
diff --git a/api/logic/minecraft/OneSixVersionFormat.cpp b/api/logic/minecraft/OneSixVersionFormat.cpp
index 7ac9e2db..d6aaa790 100644
--- a/api/logic/minecraft/OneSixVersionFormat.cpp
+++ b/api/logic/minecraft/OneSixVersionFormat.cpp
@@ -13,9 +13,9 @@ static void readString(const QJsonObject &root, const QString &key, QString &var
     }
 }
 
-LibraryPtr OneSixVersionFormat::libraryFromJson(const QJsonObject &libObj, const QString &filename)
+LibraryPtr OneSixVersionFormat::libraryFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename)
 {
-    LibraryPtr out = MojangVersionFormat::libraryFromJson(libObj, filename);
+    LibraryPtr out = MojangVersionFormat::libraryFromJson(problems, libObj, filename);
     readString(libObj, "MMC-hint", out->m_hint);
     readString(libObj, "MMC-absulute_url", out->m_absoluteURL);
     readString(libObj, "MMC-absoluteUrl", out->m_absoluteURL);
@@ -115,7 +115,7 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc
         {
             QJsonObject libObj = requireObject(libVal);
             // parse the jarmod
-            auto lib = OneSixVersionFormat::jarModFromJson(libObj, filename);
+            auto lib = OneSixVersionFormat::jarModFromJson(*out, libObj, filename);
             // and add to jar mods
             out->jarMods.append(lib);
         }
@@ -126,7 +126,7 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc
         {
             QJsonObject libObj = requireObject(libVal);
             // parse the jarmod
-            auto lib = OneSixVersionFormat::plusJarModFromJson(libObj, filename, out->name);
+            auto lib = OneSixVersionFormat::plusJarModFromJson(*out, libObj, filename, out->name);
             // and add to jar mods
             out->jarMods.append(lib);
         }
@@ -138,20 +138,20 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc
         {
             QJsonObject libObj = requireObject(libVal);
             // parse the jarmod
-            auto lib = OneSixVersionFormat::modFromJson(libObj, filename);
+            auto lib = OneSixVersionFormat::modFromJson(*out, libObj, filename);
             // and add to jar mods
             out->mods.append(lib);
         }
     }
 
-    auto readLibs = [&](const char * which, QList<LibraryPtr> & out)
+    auto readLibs = [&](const char * which, QList<LibraryPtr> & outList)
     {
         for (auto libVal : requireArray(root.value(which)))
         {
             QJsonObject libObj = requireObject(libVal);
             // parse the library
-            auto lib = libraryFromJson(libObj, filename);
-            out.append(lib);
+            auto lib = libraryFromJson(*out, libObj, filename);
+            outList.append(lib);
         }
     };
     bool hasPlusLibs = root.contains("+libraries");
@@ -180,7 +180,7 @@ VersionFilePtr OneSixVersionFormat::versionFileFromJson(const QJsonDocument &doc
     if(root.contains("mainJar"))
     {
         QJsonObject libObj = requireObject(root, "mainJar");
-        out->mainJar = libraryFromJson(libObj, filename);
+        out->mainJar = libraryFromJson(*out, libObj, filename);
     }
     // else reconstruct it from downloads and id ... if that's available
     else if(!out->minecraftVersion.isEmpty())
@@ -330,8 +330,12 @@ QJsonDocument OneSixVersionFormat::versionFileToJson(const VersionFilePtr &patch
     }
 }
 
-LibraryPtr OneSixVersionFormat::plusJarModFromJson(const QJsonObject &libObj, const QString &filename, const QString &originalName)
-{
+LibraryPtr OneSixVersionFormat::plusJarModFromJson(
+    ProblemContainer & problems,
+    const QJsonObject &libObj,
+    const QString &filename,
+    const QString &originalName
+) {
     LibraryPtr out(new Library());
     if (!libObj.contains("name"))
     {
@@ -366,9 +370,9 @@ LibraryPtr OneSixVersionFormat::plusJarModFromJson(const QJsonObject &libObj, co
     return out;
 }
 
-LibraryPtr OneSixVersionFormat::jarModFromJson(const QJsonObject& libObj, const QString& filename)
+LibraryPtr OneSixVersionFormat::jarModFromJson(ProblemContainer & problems, const QJsonObject& libObj, const QString& filename)
 {
-    return libraryFromJson(libObj, filename);
+    return libraryFromJson(problems, libObj, filename);
 }
 
 
@@ -377,9 +381,9 @@ QJsonObject OneSixVersionFormat::jarModtoJson(Library *jarmod)
     return libraryToJson(jarmod);
 }
 
-LibraryPtr OneSixVersionFormat::modFromJson(const QJsonObject& libObj, const QString& filename)
+LibraryPtr OneSixVersionFormat::modFromJson(ProblemContainer & problems, const QJsonObject& libObj, const QString& filename)
 {
-    return  libraryFromJson(libObj, filename);
+    return  libraryFromJson(problems, libObj, filename);
 }
 
 QJsonObject OneSixVersionFormat::modtoJson(Library *jarmod)
diff --git a/api/logic/minecraft/OneSixVersionFormat.h b/api/logic/minecraft/OneSixVersionFormat.h
index 14ae385c..1a091d88 100644
--- a/api/logic/minecraft/OneSixVersionFormat.h
+++ b/api/logic/minecraft/OneSixVersionFormat.h
@@ -4,6 +4,7 @@
 #include <minecraft/PackProfile.h>
 #include <minecraft/Library.h>
 #include <QJsonDocument>
+#include <ProblemProvider.h>
 
 class OneSixVersionFormat
 {
@@ -13,17 +14,17 @@ public:
     static QJsonDocument versionFileToJson(const VersionFilePtr &patch);
 
     // libraries
-    static LibraryPtr libraryFromJson(const QJsonObject &libObj, const QString &filename);
+    static LibraryPtr libraryFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename);
     static QJsonObject libraryToJson(Library *library);
 
     // DEPRECATED: old 'plus' jar mods generated by the application
-    static LibraryPtr plusJarModFromJson(const QJsonObject &libObj, const QString &filename, const QString &originalName);
+    static LibraryPtr plusJarModFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename, const QString &originalName);
 
     // new jar mods derived from libraries
-    static LibraryPtr jarModFromJson(const QJsonObject &libObj, const QString &filename);
+    static LibraryPtr jarModFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename);
     static QJsonObject jarModtoJson(Library * jarmod);
 
     // mods, also derived from libraries
-    static LibraryPtr modFromJson(const QJsonObject &libObj, const QString &filename);
+    static LibraryPtr modFromJson(ProblemContainer & problems, const QJsonObject &libObj, const QString &filename);
     static QJsonObject modtoJson(Library * jarmod);
 };