Merge lp:~gerboland/unity-api/adjust-application-api into lp:unity-api

[Gerry Boland]

ApplicationManagerInterface: adjust API to use appId for app start/stop/focus. Return ApplicationInfoInterface only in get(index) and the new findApplication(appId) methods.

ApplicationManagerInterface: adjust API to use appId for app start/stop/focus. Return ApplicationInfoInterface only in get(index) and the new findApplication(appId) methods.

[lp:~gerboland/unity-api/adjust-application-api]

96. By Gerry Boland on 2013-09-03

Missing full stop

[Michael Zanetti (mzanetti)]

9 + Q_PROPERTY(QString focusedApplication READ focusedApplication NOTIFY focusedApplicationChanged)

As we still support ApplicationInfo objects I'd probably rename this to focusedApplicationId

30 - Q_INVOKABLE virtual unity::shell::application::ApplicationInfoInterface *get(int index) const = 0;
33 + Q_INVOKABLE virtual unity::shell::application::ApplicationInfoInterface *get(const int index) const = 0;

I don't really see the point of this. It only restricts the usage of the variable in the actual implementation without any benefit (as we're creating a copy of that integer anyways). But not a real problem either.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:96
http://jenkins.qa.ubuntu.com/job/unity-api-ci/115/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/72
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/72/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-i386-ci/73

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-api-ci/115/rebuild

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

> 9 + Q_PROPERTY(QString focusedApplication READ focusedApplication NOTIFY
> focusedApplicationChanged)
>
> As we still support ApplicationInfo objects I'd probably rename this to
> focusedApplicationId

+1

> 30 - Q_INVOKABLE virtual
> unity::shell::application::ApplicationInfoInterface *get(int index) const = 0;
> 33 + Q_INVOKABLE virtual
> unity::shell::application::ApplicationInfoInterface *get(const int index)
> const = 0;
>
> I don't really see the point of this. It only restricts the usage of the
> variable in the actual implementation without any benefit (as we're creating a
> copy of that integer anyways). But not a real problem either.

Well I did it as I can't imagine an implementation that will actually modify that value :) It's a possible micro-optimization, but not worth fighting over either. I'll remove to reduce the diff

[lp:~gerboland/unity-api/adjust-application-api]

97. By Gerry Boland on 2013-09-03

Remove const from get(index)

98. By Gerry Boland on 2013-09-03

Fix doc errors

99. By Gerry Boland on 2013-09-03

Rename property focusedApplication to focusedApplicationId

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:99
http://jenkins.qa.ubuntu.com/job/unity-api-ci/116/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/73
        deb: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-armhf-ci/73/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-api-saucy-i386-ci/74

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-api-ci/116/rebuild

review: Approve (continuous-integration)

[Michael Zanetti (mzanetti)]

lgtm

review: Approve

=== modified file 'include/unity/shell/application/ApplicationManagerInterface.h'
--- include/unity/shell/application/ApplicationManagerInterface.h	2013-08-29 12:46:31 +0000
+++ include/unity/shell/application/ApplicationManagerInterface.h	2013-09-03 13:52:17 +0000
@@ -56,7 +56,7 @@
      *
      * Use focusApplication() and unfocusCurrentApplication() to modify this.
      */
-    Q_PROPERTY(unity::shell::application::ApplicationInfoInterface* focusedApplication READ focusedApplication NOTIFY focusedApplicationChanged)
+    Q_PROPERTY(QString focusedApplicationId READ focusedApplicationId NOTIFY focusedApplicationIdChanged)
 
 protected:
     /// @cond
@@ -105,25 +105,36 @@
         return rowCount();
     }
 
-    virtual unity::shell::application::ApplicationInfoInterface *focusedApplication() const = 0;
+    virtual QString focusedApplicationId() const = 0;
     /// @endcond
 
     /**
-     * @brief Get an ApplicationInfo item.
+     * @brief Get an ApplicationInfo item (using stack index).
      *
      * Note: QML requires the full namespace in the return value.
      *
      * @param index the index of the item to get
-     * @returns The item.
+     * @returns The item, or null if not found.
      */
     Q_INVOKABLE virtual unity::shell::application::ApplicationInfoInterface *get(int index) const = 0;
 
     /**
+     * @brief Get an ApplicationInfo item (using the appId).
+     *
+     * Note: QML requires the full namespace in the return value.
+     *
+     * @param appId the appId of the item to get
+     * @returns The item, or null if not found.
+     */
+    Q_INVOKABLE virtual unity::shell::application::ApplicationInfoInterface *findApplication(const QString &appId) const = 0;
+
+    /**
      * @brief Focus the given application.
      *
-     * @param application The application to be focused.
+     * @param appId The application to be focused.
+     * @returns True if appId found and application focused, else false.
      */
-    Q_INVOKABLE virtual void focusApplication(ApplicationInfoInterface *application) = 0;
+    Q_INVOKABLE virtual bool focusApplication(const QString &appId) = 0;
 
     /**
      * @brief Unfocus the currently focused application.
@@ -135,15 +146,17 @@
      *
      * @param appId The appId for the application to be spawned.
      * @param arguments Any arguments to be passed to the process.
+     * @returns True if application start successful, else false.
      */
-    Q_INVOKABLE virtual unity::shell::application::ApplicationInfoInterface* startApplication(const QString &appId, const QStringList &arguments) = 0;
+    Q_INVOKABLE virtual bool startApplication(const QString &appId, const QStringList &arguments) = 0;
 
     /**
       * @brief Stops an application.
       *
-      * @param application The application to be stopped.
+      * @param appId The application to be stopped.
+      * @returns True if application stop successful, else false (i.e. false if application was not running).
       */
-    Q_INVOKABLE virtual void stopApplication(ApplicationInfoInterface* application) = 0;
+    Q_INVOKABLE virtual bool stopApplication(const QString &appId) = 0;
 
 Q_SIGNALS:
     /// @cond
@@ -153,7 +166,7 @@
     /**
      * @brief Will be emitted whenever the focused application changes.
      */
-    void focusedApplicationChanged();
+    void focusedApplicationIdChanged();
 
 protected:
     /// @cond

=== modified file 'test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.cpp'
--- test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.cpp	2013-08-29 12:54:56 +0000
+++ test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.cpp	2013-09-03 13:52:17 +0000
@@ -75,33 +75,48 @@
 {
     if (index < 0 || index >= m_list.count())
     {
-        return 0;
+        return nullptr;
     }
     return m_list.at(index);
 }
 
-unity::shell::application::ApplicationInfoInterface *MockApplicationManager::focusedApplication() const
-{
-    return m_list.first();
-}
-
-void MockApplicationManager::focusApplication(ApplicationInfoInterface *application)
-{
-    Q_UNUSED(application)
+unity::shell::application::ApplicationInfoInterface *MockApplicationManager::findApplication(const QString &appId) const
+{
+    for (auto app : m_list)
+    {
+        if (app->appId() == appId)
+        {
+            return app;
+        }
+    }
+    return nullptr;
+}
+
+QString MockApplicationManager::focusedApplicationId() const
+{
+    auto first = m_list.first();
+    return (first) ? first->appId() : QString();
+}
+
+bool MockApplicationManager::focusApplication(const QString &appId)
+{
+    Q_UNUSED(appId)
+    return true;
 }
 
 void MockApplicationManager::unfocusCurrentApplication()
 {
 }
 
-unity::shell::application::ApplicationInfoInterface *MockApplicationManager::startApplication(const QString &appId, const QStringList &arguments)
+bool MockApplicationManager::startApplication(const QString &appId, const QStringList &arguments)
 {
     Q_UNUSED(appId)
     Q_UNUSED(arguments)
-    return 0;
+    return true;
 }
 
-void MockApplicationManager::stopApplication(unity::shell::application::ApplicationInfoInterface *application)
+bool MockApplicationManager::stopApplication(const QString &appId)
 {
-    Q_UNUSED(application)
+    Q_UNUSED(appId)
+    return true;
 }

=== modified file 'test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.h'
--- test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.h	2013-08-29 12:54:56 +0000
+++ test/qmltest/mocks/plugins/Unity/Application/Mocks/MockApplicationManager.h	2013-09-03 13:52:17 +0000
@@ -38,17 +38,19 @@
 
     QVariant data(const QModelIndex& index, int role) const;
 
-    unity::shell::application::ApplicationInfoInterface *focusedApplication() const;
-
-    Q_INVOKABLE unity::shell::application::ApplicationInfoInterface *get(int index) const;
-
-    Q_INVOKABLE void focusApplication(unity::shell::application::ApplicationInfoInterface *application);
+    QString focusedApplicationId() const;
+
+    Q_INVOKABLE unity::shell::application::ApplicationInfoInterface *get(const int index) const;
+
+    Q_INVOKABLE unity::shell::application::ApplicationInfoInterface *findApplication(const QString &appId) const;
+
+    Q_INVOKABLE bool focusApplication(const QString &appId);
 
     Q_INVOKABLE void unfocusCurrentApplication();
 
-    Q_INVOKABLE unity::shell::application::ApplicationInfoInterface *startApplication(const QString &appId, const QStringList &arguments);
+    Q_INVOKABLE bool startApplication(const QString &appId, const QStringList &arguments);
 
-    Q_INVOKABLE void stopApplication(unity::shell::application::ApplicationInfoInterface *application);
+    Q_INVOKABLE bool stopApplication(const QString &appId);
 
 private:
     QList<MockApplicationInfo*> m_list;

=== modified file 'test/qmltest/unity/shell/application/tst_Application.qml'
--- test/qmltest/unity/shell/application/tst_Application.qml	2013-08-29 12:38:58 +0000
+++ test/qmltest/unity/shell/application/tst_Application.qml	2013-09-03 13:52:17 +0000
@@ -86,6 +86,7 @@
         function test_model_methods_data() {
             return [
                 { tag: "ApplicationManager.methods[get]", method: "get" },
+                { tag: "ApplicationManager.methods[findApplication]", method: "findApplication" },
                 { tag: "ApplicationManager.methods[focusApplication]", method: "focusApplication" },
                 { tag: "ApplicationManager.methods[unfocusCurrentApplication]", method: "unfocusCurrentApplication" },
                 { tag: "ApplicationManager.methods[startApplication]", method: "startApplication" },
@@ -102,7 +103,7 @@
         function test_model_properties_data() {
             return [
                 { tag: "ApplicationManager.count", property: "count", type: "number" },
-                { tag: "ApplicationManager.focusedApplication", property: "focusedApplication", type: "unity::shell::application::ApplicationInfoInterface" },
+                { tag: "ApplicationManager.focusedApplicationId", property: "focusedApplicationId", type: "string" },
             ];
         }