Fix bugs in InstallIconsOperation and add unit tests

- Fix conflicting naming and reference of operation value in
  InstallIconsOperation::targetDirectory().
- Fix bug related to appending of "icons" suffix to paths in
  XDG_DATA_HOME, the logic was glued to old default values which have
  been since changed.
- Remove unused variable in class destructor and remove unneeded
  default implementation.

Also add unit and CLI tests for InstallIconsOperation and move
addToFileMap() to common functions so it can be used in multiple tests.

Task-number: QTIFW-1715
Task-number: QTIFW-1465
Change-Id: I5281ca7a5dec1453f49fbe2cb2852d6467bf0c97
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

11 files changed, 304 insertions, 41 deletions

diff --git a/src/libs/installer/installiconsoperation.cpp b/src/libs/installer/installiconsoperation.cpp
index 7b02bb964..08ac6e8d5 100644
--- a/src/libs/installer/installiconsoperation.cpp
+++ b/src/libs/installer/installiconsoperation.cpp
@@ -1,6 +1,6 @@
 /**************************************************************************
 **
-** Copyright (C) 2017 The Qt Company Ltd.
+** Copyright (C) 2020 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the Qt Installer Framework.
@@ -40,13 +40,13 @@ using namespace QInstaller;
 QString InstallIconsOperation::targetDirectory()
 {
     // we're not searching for the first time, let's re-use the old value
-    if (hasValue(QLatin1String("targetdirectory")))
-        return value(QLatin1String("targetdirectory")).toString();
+    if (hasValue(QLatin1String("directory")))
+        return value(QLatin1String("directory")).toString();
 
     QStringList XDG_DATA_HOME = QString::fromLocal8Bit(qgetenv("XDG_DATA_HOME"))
                                                         .split(QLatin1Char(':'),
         QString::SkipEmptyParts);
-    XDG_DATA_HOME.push_back(QDir::home().absoluteFilePath(QLatin1String(".local/share/icons"))); // default path
+    XDG_DATA_HOME.push_back(QDir::home().absoluteFilePath(QLatin1String(".local/share"))); // default path
 
     QString directory;
     const QStringList& directories = XDG_DATA_HOME;
@@ -54,11 +54,7 @@ QString InstallIconsOperation::targetDirectory()
         if (it->isEmpty())
             continue;
 
-        // our default dirs are correct, XDG_DATA_HOME set via env needs "icon" at the end
-        if ((it + 1 == directories.end()) || (it + 2 == directories.end()) || (it + 3 == directories.end()))
-            directory = QDir(*it).absolutePath();
-        else
-            directory = QDir(*it).absoluteFilePath(QLatin1String("icons"));
+        directory = QDir(*it).absoluteFilePath(QLatin1String("icons"));
 
         QDir dir(directory);
         // let's see if this dir exists or we're able to create it
@@ -90,11 +86,6 @@ InstallIconsOperation::InstallIconsOperation(PackageManagerCore *core)
     setName(QLatin1String("InstallIcons"));
 }
 
-InstallIconsOperation::~InstallIconsOperation()
-{
-    const QStringList backupFiles = value(QLatin1String("backupfiles")).toStringList();
-}
-
 void InstallIconsOperation::backup()
 {
     // we backup on the fly
diff --git a/src/libs/installer/installiconsoperation.h b/src/libs/installer/installiconsoperation.h
index 5f8dff514..7a4d1cc90 100644
--- a/src/libs/installer/installiconsoperation.h
+++ b/src/libs/installer/installiconsoperation.h
@@ -1,6 +1,6 @@
 /**************************************************************************
 **
-** Copyright (C) 2017 The Qt Company Ltd.
+** Copyright (C) 2020 The Qt Company Ltd.
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the Qt Installer Framework.
@@ -40,7 +40,6 @@ class INSTALLER_EXPORT InstallIconsOperation : public QObject, public Operation
     Q_OBJECT
 public:
     explicit InstallIconsOperation(PackageManagerCore *core);
-    ~InstallIconsOperation();
 
     void backup();
     bool performOperation();
diff --git a/tests/auto/installer/copydirectoryoperation/tst_copydirectoryoperation.cpp b/tests/auto/installer/copydirectoryoperation/tst_copydirectoryoperation.cpp
index aae0aa9a4..02d95cfdb 100644
--- a/tests/auto/installer/copydirectoryoperation/tst_copydirectoryoperation.cpp
+++ b/tests/auto/installer/copydirectoryoperation/tst_copydirectoryoperation.cpp
@@ -26,6 +26,8 @@
 **
 **************************************************************************/
 
+#include "../shared/commonfunctions.h"
+
 #include <fileutils.h>
 #include <copydirectoryoperation.h>
 #include <binarycontent.h>
@@ -36,7 +38,6 @@
 #include <QObject>
 #include <QDir>
 #include <QFile>
-#include <QCryptographicHash>
 #include <QTest>
 
 using namespace KDUpdater;
@@ -161,11 +162,11 @@ private slots:
         // Matches path in component install script
         QFileInfo targetInfo(installDir + QDir::toNativeSeparators("/directory"));
         QMap<QString, QByteArray> targetMap;
-        addToFileMap(QDir(targetInfo.absoluteFilePath()), targetInfo, targetMap);
+        VerifyInstaller::addToFileMap(QDir(targetInfo.absoluteFilePath()), targetInfo, targetMap);
 
         QFileInfo destinationInfo(installDir + QDir::toNativeSeparators("/destination/directory"));
         QMap<QString, QByteArray> destinationMap;
-        addToFileMap(QDir(destinationInfo.absoluteFilePath()), destinationInfo, destinationMap);
+        VerifyInstaller::addToFileMap(QDir(destinationInfo.absoluteFilePath()), destinationInfo, destinationMap);
 
         QVERIFY(targetMap == destinationMap);
 
@@ -180,27 +181,6 @@ private slots:
     }
 
 private:
-    void addToFileMap(const QDir &baseDir, const QFileInfo &fileInfo, QMap<QString, QByteArray> &map)
-    {
-        QDir directory(fileInfo.absoluteFilePath());
-        directory.setFilter(QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::AllDirs | QDir::Files);
-        QFileInfoList fileInfoList = directory.entryInfoList();
-
-        foreach (const QFileInfo &info, fileInfoList) {
-            if (info.isDir()) {
-                map.insert(baseDir.relativeFilePath(info.filePath()), QByteArray());
-                addToFileMap(baseDir, info, map);
-            } else {
-                QCryptographicHash hash(QCryptographicHash::Sha1);
-                QFile file(info.absoluteFilePath());
-                QVERIFY(file.open(QIODevice::ReadOnly));
-                QVERIFY(hash.addData(&file));
-                map.insert(baseDir.relativeFilePath(info.filePath()), hash.result().toHex());
-                file.close();
-            }
-        }
-    }
-
     QString m_sourcePath;
     QString m_destinationPath;
 };
diff --git a/tests/auto/installer/installer.pro b/tests/auto/installer/installer.pro
index faa908f01..766e8814e 100644
--- a/tests/auto/installer/installer.pro
+++ b/tests/auto/installer/installer.pro
@@ -39,7 +39,9 @@ win32 {
 }
 
 linux-g++* {
-    SUBDIRS += createdesktopentryoperation
+    SUBDIRS += \
+        createdesktopentryoperation \
+        installiconsoperation
 }
 
 scriptengine.depends += unicodeexecutable
diff --git a/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1content.7z b/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1content.7z
new file mode 100644
index 000000000..9d58b172e
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1content.7z
diff --git a/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1meta.7z b/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1meta.7z
new file mode 100644
index 000000000..74e201fc3
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/data/repository/A/1.0.2-1meta.7z
diff --git a/tests/auto/installer/installiconsoperation/data/repository/Updates.xml b/tests/auto/installer/installiconsoperation/data/repository/Updates.xml
new file mode 100644
index 000000000..a1c8f6aa2
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/data/repository/Updates.xml
@@ -0,0 +1,14 @@
+<Updates>
+ <ApplicationName>{AnyApplication}</ApplicationName>
+ <ApplicationVersion>1.0.0</ApplicationVersion>
+ <PackageUpdate>
+  <Name>A</Name>
+  <DisplayName>A</DisplayName>
+  <Description>Example component A</Description>
+  <Version>1.0.2-1</Version>
+  <ReleaseDate>2015-01-01</ReleaseDate>
+  <Default>true</Default>
+  <Script>script.qs</Script>
+  <DownloadableArchives>content.7z</DownloadableArchives>
+ </PackageUpdate>
+</Updates>
diff --git a/tests/auto/installer/installiconsoperation/installiconsoperation.pro b/tests/auto/installer/installiconsoperation/installiconsoperation.pro
new file mode 100644
index 000000000..6e6a45f39
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/installiconsoperation.pro
@@ -0,0 +1,10 @@
+include(../../qttest.pri)
+
+QT -= gui
+QT += testlib
+
+SOURCES = tst_installiconsoperation.cpp
+
+RESOURCES += \
+    settings.qrc \
+    ..\shared\config.qrc
diff --git a/tests/auto/installer/installiconsoperation/settings.qrc b/tests/auto/installer/installiconsoperation/settings.qrc
new file mode 100644
index 000000000..10cc3c3bb
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/settings.qrc
@@ -0,0 +1,7 @@
+<RCC>
+    <qresource prefix="/">
+        <file>data/repository/Updates.xml</file>
+        <file>data/repository/A/1.0.2-1content.7z</file>
+        <file>data/repository/A/1.0.2-1meta.7z</file>
+    </qresource>
+</RCC>
diff --git a/tests/auto/installer/installiconsoperation/tst_installiconsoperation.cpp b/tests/auto/installer/installiconsoperation/tst_installiconsoperation.cpp
new file mode 100644
index 000000000..1c68f25ab
--- /dev/null
+++ b/tests/auto/installer/installiconsoperation/tst_installiconsoperation.cpp
@@ -0,0 +1,236 @@
+/**************************************************************************
+**
+** Copyright (C) 2020 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt Installer Framework.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+**************************************************************************/
+
+#include "../shared/commonfunctions.h"
+
+#include <installiconsoperation.h>
+
+#include <packagemanagercore.h>
+#include <binarycontent.h>
+#include <settings.h>
+#include <fileutils.h>
+#include <init.h>
+#include <component.h>
+
+#include <QObject>
+#include <QTest>
+#include <QFile>
+
+using namespace KDUpdater;
+using namespace QInstaller;
+
+class tst_installiconsoperation : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void initTestCase()
+    {
+        m_testIconSourcePath = qApp->applicationDirPath() + "/icons";
+        m_testIconSubdirectoryPath = m_testIconSourcePath + "/test";
+
+        m_testIconFilePaths.append(m_testIconSubdirectoryPath
+            + QDir::separator() + "vendor-icon1.png");
+        m_testIconFilePaths.append(m_testIconSubdirectoryPath
+            + QDir::separator() + "vendor-icon2.png");
+        m_testIconFilePaths.append(m_testIconSubdirectoryPath
+            + QDir::separator() + "vendor-icon3.png");
+    }
+
+    void testMissingArguments()
+    {
+        InstallIconsOperation op(nullptr);
+
+        op.backup();
+        QVERIFY(op.testOperation());
+        QVERIFY(!op.performOperation());
+
+        QCOMPARE(UpdateOperation::Error(op.error()), UpdateOperation::InvalidArguments);
+        QCOMPARE(op.errorString(), QString("Invalid arguments in InstallIcons: "
+                                           "0 arguments given, 1 or 2 arguments expected "
+                                           "in the form: <source path> [vendor prefix]."));
+
+        op.setArguments(QStringList() << "");
+        QVERIFY(!op.performOperation());
+
+        QCOMPARE(UpdateOperation::Error(op.error()), UpdateOperation::InvalidArguments);
+        QCOMPARE(op.errorString(), QString("Invalid Argument: source directory must not be empty."));
+    }
+
+    void testPerformingFromCLI()
+    {
+        QInstaller::init(); //This will eat debug output
+        PackageManagerCore *core = new PackageManagerCore(BinaryContent::MagicInstallerMarker, QList<OperationBlob> ());
+        core->setAllowedRunningProcesses(QStringList() << QCoreApplication::applicationFilePath());
+        QSet<Repository> repoList;
+        Repository repo = Repository::fromUserInput(":///data/repository");
+        repoList.insert(repo);
+        core->settings().setDefaultRepositories(repoList);
+
+        QString installDir = QInstaller::generateTemporaryFileName();
+        QVERIFY(QDir().mkpath(installDir));
+        core->setValue(scTargetDir, installDir);
+        core->installDefaultComponentsSilently();
+
+        InstallIconsOperation *installIconsOp = nullptr;
+        OperationList operations = core->componentByName("A")->operations();
+        foreach (Operation *op, operations) {
+            if (op->name() == QLatin1String("InstallIcons"))
+                installIconsOp = dynamic_cast<InstallIconsOperation *>(op);
+        }
+        QVERIFY(installIconsOp);
+
+        // As the original directory containing icons will be deleted by the operation,
+        // we will use a copy with the exact same contents.
+        QFileInfo fakeSourceInfo(installDir + "/icons_copy/test");
+        QMap<QString, QByteArray> fakeSourceMap;
+        VerifyInstaller::addToFileMap(QDir(fakeSourceInfo.absoluteFilePath()), fakeSourceInfo, fakeSourceMap);
+
+        QFileInfo destinationInfo(installIconsOp->value("directory").toString() + "/test");
+        QMap<QString, QByteArray> destinationMap;
+        VerifyInstaller::addToFileMap(QDir(destinationInfo.absoluteFilePath()), destinationInfo, destinationMap);
+
+        QVERIFY(fakeSourceMap == destinationMap);
+
+        core->setPackageManager();
+        core->commitSessionOperations();
+        core->uninstallComponentsSilently(QStringList() << "A");
+        QVERIFY(!destinationInfo.exists());
+
+        QDir dir(installDir);
+        QVERIFY(dir.removeRecursively());
+        core->deleteLater();
+    }
+
+    void testInstallIconsWithUndo()
+    {
+        QDir testIconDir(m_testIconSourcePath);
+        QVERIFY(testIconDir.mkpath(m_testIconSubdirectoryPath));
+
+        // Populate source directory
+        foreach (const QString &filePath, m_testIconFilePaths) {
+            QFile file(filePath);
+            QVERIFY(file.open(QFileDevice::ReadWrite));
+            file.close();
+        }
+
+        PackageManagerCore *core = new PackageManagerCore();
+        InstallIconsOperation op(core);
+        op.setArguments(QStringList() << m_testIconSourcePath);
+
+        QFileInfo sourceInfo(m_testIconSubdirectoryPath);
+        QMap<QString, QByteArray> sourceMap;
+        VerifyInstaller::addToFileMap(QDir(sourceInfo.absoluteFilePath()), sourceInfo, sourceMap);
+
+        QVERIFY2(op.performOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(m_testIconSourcePath));
+
+        QFileInfo destinationInfo(op.value("directory").toString() + "/test");
+        QMap<QString, QByteArray> destinationMap;
+        VerifyInstaller::addToFileMap(QDir(destinationInfo.absoluteFilePath()), destinationInfo, destinationMap);
+
+        QVERIFY2(op.undoOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(op.value("directory").toString() + "/test"));
+        QVERIFY(QFileInfo().exists(m_testIconSourcePath));
+
+        QVERIFY(sourceMap == destinationMap);
+        QVERIFY(testIconDir.removeRecursively());
+
+        core->deleteLater();
+    }
+
+    void testChangeVendorPrefix()
+    {
+        QDir testIconDir(m_testIconSourcePath);
+        QVERIFY(testIconDir.mkpath(m_testIconSubdirectoryPath));
+
+        QFile file(m_testIconFilePaths.first());
+        QVERIFY(file.open(QFileDevice::ReadWrite));
+        file.close();
+
+        PackageManagerCore *core = new PackageManagerCore();
+        InstallIconsOperation op(core);
+        op.setArguments(QStringList() << m_testIconSourcePath << "testVendor");
+
+        QVERIFY2(op.performOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(m_testIconSourcePath));
+        QVERIFY(QFileInfo().exists(op.value("directory").toString() + "/test/testVendor-icon1.png"));
+
+        QVERIFY2(op.undoOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(op.value("directory").toString() + "/test"));
+        QVERIFY(QFileInfo().exists(m_testIconSourcePath));
+
+        QVERIFY(testIconDir.removeRecursively());
+
+        core->deleteLater();
+    }
+
+    void testValidTargetDirectory()
+    {
+        QDir testIconDir(m_testIconSourcePath);
+        QVERIFY(testIconDir.mkpath(m_testIconSubdirectoryPath));
+
+        PackageManagerCore *core = new PackageManagerCore();
+        InstallIconsOperation op(core);
+        op.setArguments(QStringList() << m_testIconSourcePath);
+
+        QVERIFY2(op.performOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(m_testIconSourcePath));
+
+        QString targetIconsDirectory = op.value("directory").toString();
+        QVERIFY(QFileInfo(targetIconsDirectory).exists());
+        QStringList directories = QString::fromLocal8Bit(qgetenv("XDG_DATA_HOME"))
+            .split(QLatin1Char(':'), QString::SkipEmptyParts);
+        // Default path if XDG_DATA_HOME is not set
+        directories.append(QDir::home().absoluteFilePath(QLatin1String(".local/share")));
+        bool validPath = false;
+        foreach (const QString &dir, directories) {
+            // Icon directory should be one of the expected locations
+            if (targetIconsDirectory == QDir(dir).absoluteFilePath("icons")) {
+                validPath = true;
+                break;
+            }
+        }
+        QVERIFY(validPath);
+
+        QVERIFY2(op.undoOperation(), op.errorString().toLatin1());
+        QVERIFY(!QFileInfo().exists(op.value("directory").toString() + "/test"));
+
+        QVERIFY(testIconDir.removeRecursively());
+        core->deleteLater();
+    }
+
+private:
+    QString m_testIconSourcePath;
+    QString m_testIconSubdirectoryPath;
+    QStringList m_testIconFilePaths;
+};
+
+QTEST_MAIN(tst_installiconsoperation)
+
+#include "tst_installiconsoperation.moc"
diff --git a/tests/auto/installer/shared/commonfunctions.h b/tests/auto/installer/shared/commonfunctions.h
index bf96c5648..aac1e11e4 100644
--- a/tests/auto/installer/shared/commonfunctions.h
+++ b/tests/auto/installer/shared/commonfunctions.h
@@ -33,6 +33,8 @@
 #include <QTest>
 #include <QString>
 #include <QStringList>
+#include <QCryptographicHash>
+#include <QFile>
 
 struct VerifyInstaller
 {
@@ -67,5 +69,27 @@ struct VerifyInstaller
         QDir dir(installDir);
         QCOMPARE(dir.entryList(QStringList() << "*.*", QDir::Files).count(), fileList.count());
     }
+
+    static void addToFileMap(const QDir &baseDir, const QFileInfo &fileInfo, QMap<QString, QByteArray> &map)
+    {
+        QDir directory(fileInfo.absoluteFilePath());
+        directory.setFilter(QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::AllDirs | QDir::Files);
+        QFileInfoList fileInfoList = directory.entryInfoList();
+
+        foreach (const QFileInfo &info, fileInfoList) {
+            if (info.isDir()) {
+                map.insert(baseDir.relativeFilePath(info.filePath()), QByteArray());
+                addToFileMap(baseDir, info, map);
+            } else {
+                QCryptographicHash hash(QCryptographicHash::Sha1);
+                QFile file(info.absoluteFilePath());
+                QVERIFY(file.open(QIODevice::ReadOnly));
+                QVERIFY(hash.addData(&file));
+                map.insert(baseDir.relativeFilePath(info.filePath()), hash.result().toHex());
+                file.close();
+            }
+        }
+    }
+
 };
 #endif