Fix dupe handling in requestAddLauncherToActivities; improve unit test

Summary:
The business logic was using the resolved URL in lookups in
internal data structures, instead of the internal key it
previously established equivalence to. This could lead to
junk in internal maps and unnecessarily emitted model changes.

This also improves the unit test not to require apps to be
installed, fixing the CI failure over missing Dolphin.

Reviewers: #plasma, kossebau

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D10253

libtaskmanager/autotests/launchertasksmodeltest.cpp

@@ -23,6 +23,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 #include <QTest>
 
 #include "launchertasksmodel.h"
+#include "tasktools.h"
 
 using namespace TaskManager;
 
@@ -42,16 +43,13 @@ class LauncherTasksModelTest : public QObject
 
     private:
         QStringList m_urlStrings;
-        QStringList m_mangledUrlStrings;
 };
 
 void LauncherTasksModelTest::initTestCase()
 {
     qApp->setProperty("org.kde.KActivities.core.disableAutostart", true);
-    m_urlStrings << QLatin1String("file:///usr/share/applications/org.kde.dolphin.desktop");
+    m_urlStrings << QLatin1String("file:///usr/share/applications/org.kde.systemmonitor.desktop");
-    m_mangledUrlStrings << QLatin1String("applications:org.kde.dolphin.desktop");
     m_urlStrings << QLatin1String("file:///usr/share/applications/org.kde.konversation.desktop");
-    m_mangledUrlStrings << QLatin1String("applications:org.kde.konversation.desktop");
 }
 
 void LauncherTasksModelTest::shouldRoundTripLauncherUrlList()
@@ -67,8 +65,8 @@ void LauncherTasksModelTest::shouldRoundTripLauncherUrlList()
 
     QCOMPARE(m.launcherList(), m_urlStrings);
 
-    QCOMPARE(m.data(m.index(0, 0), AbstractTasksModel::LauncherUrl).toString(), m_mangledUrlStrings.at(0));
+    QVERIFY(launcherUrlsMatch(m.data(m.index(0, 0), AbstractTasksModel::LauncherUrl).toUrl(), QUrl(m_urlStrings.at(0))));
-    QCOMPARE(m.data(m.index(1, 0), AbstractTasksModel::LauncherUrl).toString(), m_mangledUrlStrings.at(1));
+    QVERIFY(launcherUrlsMatch(m.data(m.index(1, 0), AbstractTasksModel::LauncherUrl).toUrl(), QUrl(m_urlStrings.at(1))));
 }
 
 void LauncherTasksModelTest::shouldIgnoreInvalidUrls()
@@ -128,7 +126,7 @@ void LauncherTasksModelTest::shouldAddRemoveLauncher()
     QVERIFY(added);
     QCOMPARE(launcherListChangedSpy.count(), 1);
 
-    QCOMPARE(m.launcherList().at(0), m_urlStrings.at(0));
+    QVERIFY(launcherUrlsMatch(QUrl(m.launcherList().at(0)), QUrl(m_urlStrings.at(0))));
 
     bool removed = m.requestRemoveLauncher(QUrl(m_urlStrings.at(0)));
 

libtaskmanager/launchertasksmodel.cpp

@@ -174,7 +174,8 @@ bool LauncherTasksModel::Private::requestAddLauncherToActivities(const QUrl &_ur
         if (launcherUrlsMatch(url, launcher, IgnoreQueryItems)) {
             ActivitiesSet newActivities;
 
-            if (!activitiesForLauncher.contains(url)) {
+            // Use the key we established equivalence to ('launcher').
+            if (!activitiesForLauncher.contains(launcher)) {
                 // If we don't have the activities assigned to this url
                 // for some reason
                 newActivities = activities;
@@ -185,7 +186,7 @@ bool LauncherTasksModel::Private::requestAddLauncherToActivities(const QUrl &_ur
                     // launcher should be on all activities
                     newActivities = ActivitiesSet { NULL_UUID };
 
-                } else if (isOnAllActivities(activitiesForLauncher[url])) {
+                } else if (isOnAllActivities(activitiesForLauncher[launcher])) {
                     // If we have been on all activities before, and we have
                     // been asked to be on a specific one, lets make an
                     // exception - we will set the activities to exactly
@@ -195,13 +196,13 @@ bool LauncherTasksModel::Private::requestAddLauncherToActivities(const QUrl &_ur
 
                 } else {
                     newActivities += activities;
-                    newActivities += activitiesForLauncher[url];
+                    newActivities += activitiesForLauncher[launcher];
 
                 }
             }
 
-            if (newActivities != activitiesForLauncher[url]) {
+            if (newActivities != activitiesForLauncher[launcher]) {
-                setActivitiesForLauncher(url, newActivities);
+                setActivitiesForLauncher(launcher, newActivities);
 
                 emit q->dataChanged(
                         q->index(row, 0),