Remove delete* methods from QgsTaskManager API

nyalldawson · nyalldawson · commit b6b7a7f8c658 · 2016-12-05T14:08:11.000+10:00
On further consideration allowing external control of task
deletion is a bad idea.
diff --git a/python/core/qgstaskmanager.sip b/python/core/qgstaskmanager.sip
@@ -274,25 +274,6 @@ class QgsTaskManager : QObject
      */
     long addTask( QgsTask* task /Transfer/, const QgsTaskList& dependencies = QgsTaskList() );
 
-    /** Deletes the specified task, first terminating it if it is currently
-     * running.
-     * @param id task ID
-     * @returns true if task was found and deleted
-     */
-    bool deleteTask( long id );
-
-    /** Deletes the specified task, first terminating it if it is currently
-     * running.
-     * @param task task to delete
-     * @returns true if task was contained in manager and deleted
-     */
-    bool deleteTask( QgsTask* task );
-
-    /** Deletes all tasks in the manager, first terminating them if they are currently
-     * running.
-     */
-    void deleteAllTasks();
-
     /** Returns the task with matching ID.
      * @param id task ID
      * @returns task if found, or nullptr
diff --git a/src/core/qgstaskmanager.cpp b/src/core/qgstaskmanager.cpp
@@ -172,46 +172,6 @@ long QgsTaskManager::addTask( QgsTask* task, const QgsTaskList& dependencies )
   return mNextTaskId++;
 }
 
-bool QgsTaskManager::deleteTask( long id )
-{
-  QMutexLocker ml( mTaskMutex );
-  QgsTask* task = mTasks.value( id ).task;
-  return deleteTask( task );
-}
-
-bool QgsTaskManager::deleteTask( QgsTask *task )
-{
-  if ( !task )
-    return false;
-
-  bool result = cleanupAndDeleteTask( task );
-
-  // remove from internal task list
-  QMutexLocker ml( mTaskMutex );
-  for ( QMap< long, TaskInfo >::iterator it = mTasks.begin(); it != mTasks.end(); )
-  {
-    if ( it.value().task == task )
-      it = mTasks.erase( it );
-    else
-      ++it;
-  }
-
-  return result;
-}
-
-void QgsTaskManager::deleteAllTasks()
-{
-  //first tell all tasks to cancel
-  cancelAll();
-
-  QMutexLocker ml( mTaskMutex );
-  Q_FOREACH ( QgsTask* task, tasks() )
-  {
-    deleteTask( task );
-  }
-  emit allTasksFinished();
-}
-
 QgsTask* QgsTaskManager::task( long id ) const
 {
   QMutexLocker ml( mTaskMutex );
@@ -339,6 +299,20 @@ QStringList QgsTaskManager::dependentLayers( long taskId ) const
   return mLayerDependencies.value( taskId, QStringList() );
 }
 
+QList<QgsTask*> QgsTaskManager::activeTasks() const
+{
+  QMutexLocker ml( mTaskMutex );
+  QList< QgsTask* > taskList = mActiveTasks;
+  taskList.detach();
+  return taskList;
+}
+
+int QgsTaskManager::countActiveTasks() const
+{
+  QMutexLocker ml( mTaskMutex );
+  return mActiveTasks.count();
+}
+
 void QgsTaskManager::taskProgressChanged( double progress )
 {
   QMutexLocker ml( mTaskMutex );
diff --git a/src/core/qgstaskmanager.h b/src/core/qgstaskmanager.h
@@ -305,25 +305,6 @@ class CORE_EXPORT QgsTaskManager : public QObject
      */
     long addTask( QgsTask* task, const QgsTaskList& dependencies = QgsTaskList() );
 
-    /** Deletes the specified task, first terminating it if it is currently
-     * running.
-     * @param id task ID
-     * @returns true if task was found and deleted
-     */
-    bool deleteTask( long id );
-
-    /** Deletes the specified task, first terminating it if it is currently
-     * running.
-     * @param task task to delete
-     * @returns true if task was contained in manager and deleted
-     */
-    bool deleteTask( QgsTask* task );
-
-    /** Deletes all tasks in the manager, first terminating them if they are currently
-     * running.
-     */
-    void deleteAllTasks();
-
     /** Returns the task with matching ID.
      * @param id task ID
      * @returns task if found, or nullptr
@@ -375,13 +356,13 @@ class CORE_EXPORT QgsTaskManager : public QObject
     /** Returns a list of the active (queued or running) tasks.
      * @see countActiveTasks()
      */
-    QList< QgsTask* > activeTasks() const { return mActiveTasks; }
+    QList< QgsTask* > activeTasks() const;
 
     /** Returns the number of active (queued or running) tasks.
      * @see activeTasks()
      * @see countActiveTasksChanged()
      */
-    int countActiveTasks() const { return mActiveTasks.count(); }
+    int countActiveTasks() const;
 
   signals:
 
diff --git a/tests/src/core/testqgstaskmanager.cpp b/tests/src/core/testqgstaskmanager.cpp
@@ -70,6 +70,29 @@ class TestTerminationTask : public TestTask
     }
 };
 
+class CancelableTask : public QgsTask
+{
+    Q_OBJECT
+
+  public:
+
+    ~CancelableTask()
+    {
+      int i = 1;
+      i++;
+
+    }
+
+  protected:
+
+    TaskResult run() override
+    {
+      while ( !isCancelled() )
+        {}
+      return ResultSuccess;
+    }
+};
+
 class SuccessTask : public QgsTask
 {
     Q_OBJECT
@@ -138,7 +161,6 @@ class TestQgsTaskManager : public QObject
     void taskFinished();
     void createInstance();
     void addTask();
-    void deleteTask();
     //void taskTerminationBeforeDelete();
     void taskId();
     void progressChanged();
@@ -303,42 +325,6 @@ void TestQgsTaskManager::addTask()
   QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
 }
 
-void TestQgsTaskManager::deleteTask()
-{
-  //create manager with some tasks
-  QgsTaskManager manager;
-  TestTask* task = new TestTask();
-  TestTask* task2 = new TestTask();
-  TestTask* task3 = new TestTask();
-  manager.addTask( task );
-  manager.addTask( task2 );
-  manager.addTask( task3 );
-
-  QSignalSpy spy( &manager, &QgsTaskManager::taskAboutToBeDeleted );
-
-  //try deleting a non-existant task
-  QVERIFY( !manager.deleteTask( 56 ) );
-  QCOMPARE( spy.count(), 0 );
-
-  //try deleting a task by ID
-  QVERIFY( manager.deleteTask( 1 ) );
-  QCOMPARE( manager.tasks().count(), 2 );
-  QVERIFY( !manager.task( 1 ) );
-  QCOMPARE( spy.count(), 1 );
-  QCOMPARE( spy.last().at( 0 ).toLongLong(), 1LL );
-
-  //can't delete twice
-  QVERIFY( !manager.deleteTask( 1 ) );
-  QCOMPARE( spy.count(), 1 );
-
-  //delete task by reference
-  QVERIFY( manager.deleteTask( task ) );
-  QCOMPARE( manager.tasks().count(), 1 );
-  QVERIFY( !manager.task( 0 ) );
-  QCOMPARE( spy.count(), 2 );
-  QCOMPARE( spy.last().at( 0 ).toLongLong(), 0LL );
-}
-
 #if 0
 // we don't run this by default - the sendPostedEvents call is fragile
 void TestQgsTaskManager::taskTerminationBeforeDelete()
