Remove processing specific methods for adding/removing providers

QgsProcessingRegistry methods should always be used instead
qgis · Apr 7, 2017 · ebd8519 · ebd8519
1 parent fdcf04f
commit ebd8519
Show file tree

Hide file tree

Showing 15 changed files with 80 additions and 37 deletions.
diff --git a/python/plugins/processing/ProcessingPlugin.py b/python/plugins/processing/ProcessingPlugin.py
@@ -163,6 +163,7 @@ def unload(self):
         self.iface.unregisterOptionsWidgetFactory(self.options_factory)
 
         removeMenus()
+        Processing.deinitialize()
 
     def openCommander(self):
         if self.commander is None:

diff --git a/python/plugins/processing/algs/exampleprovider/ExampleAlgorithmProvider.py b/python/plugins/processing/algs/exampleprovider/ExampleAlgorithmProvider.py
@@ -48,6 +48,8 @@ def load(self):
         ProcessingConfig.addSetting(Setting('Example algorithms',
                                             ExampleAlgorithmProvider.MY_DUMMY_SETTING,
                                             'Example setting', 'Default value'))
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/algs/exampleprovider/ProcessingExampleProviderPlugin.py b/python/plugins/processing/algs/exampleprovider/ProcessingExampleProviderPlugin.py
@@ -30,6 +30,7 @@
 import sys
 import inspect
 
+from qgis.core import QgsApplication
 from processing.core.Processing import Processing
 from exampleprovider.ExampleAlgorithmProvider import ExampleAlgorithmProvider
 
@@ -45,7 +46,7 @@ def __init__(self):
         self.provider = ExampleAlgorithmProvider()
 
     def initGui(self):
-        Processing.addProvider(self.provider)
+        QgsApplication.processingRegistry().addProvider(self.provider)
 
     def unload(self):
-        Processing.removeProvider(self.provider)
+        QgsApplication.processingRegistry().removeProvider(self.provider)
diff --git a/python/plugins/processing/algs/gdal/GdalAlgorithmProvider.py b/python/plugins/processing/algs/gdal/GdalAlgorithmProvider.py
@@ -110,6 +110,8 @@ def load(self):
             GdalUtils.GDAL_HELP_PATH,
             self.tr('Location of GDAL docs'),
             GdalUtils.gdalHelpPath()))
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/algs/grass7/Grass7AlgorithmProvider.py b/python/plugins/processing/algs/grass7/Grass7AlgorithmProvider.py
@@ -69,6 +69,8 @@ def load(self):
             Grass7Utils.GRASS_HELP_PATH,
             self.tr('Location of GRASS docs'),
             Grass7Utils.grassHelpPath()))
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/algs/r/RAlgorithmProvider.py b/python/plugins/processing/algs/r/RAlgorithmProvider.py
@@ -84,6 +84,8 @@ def load(self):
                 RUtils.R_USE64, self.tr('Use 64 bit version'), False))
         ProviderActions.registerProviderActions(self, self.actions)
         ProviderContextMenuActions.registerProviderContextMenuActions(self.contextMenuActions)
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/algs/saga/SagaAlgorithmProvider.py b/python/plugins/processing/algs/saga/SagaAlgorithmProvider.py
@@ -66,6 +66,8 @@ def load(self):
         ProcessingConfig.addSetting(Setting("SAGA",
                                             SagaUtils.SAGA_LOG_CONSOLE,
                                             self.tr('Log console output'), True))
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/core/Processing.py b/python/plugins/processing/core/Processing.py
@@ -66,37 +66,6 @@
 
 class Processing(object):
 
-    @staticmethod
-    def addProvider(provider):
-        """Use this method to add algorithms from external providers.
-        """
-        if QgsApplication.processingRegistry().providerById(provider.id()):
-            return
-        try:
-            if provider.load():
-                ProcessingConfig.readSettings()
-                provider.refreshAlgorithms()
-                QgsApplication.processingRegistry().addProvider(provider)
-                PROVIDERS.append(provider)
-        except:
-            ProcessingLog.addToLog(
-                ProcessingLog.LOG_ERROR,
-                Processing.tr('Could not load provider: {0}\n{0}').format(
-                    provider.name(), traceback.format_exc()
-                )
-            )
-            Processing.removeProvider(provider)
-
-    @staticmethod
-    def removeProvider(provider):
-        """Use this method to remove a provider.
-
-        This method should be called when unloading a plugin that
-        contributes a provider.
-        """
-        provider.unload()
-        QgsApplication.processingRegistry().removeProvider(provider.id())
-
     @staticmethod
     def activateProvider(providerOrName, activate=True):
         provider_id = providerOrName.id() if isinstance(providerOrName, QgsProcessingProvider) else providerOrName
@@ -115,12 +84,19 @@ def initialize():
             return
         # Add the basic providers
         for c in QgsProcessingProvider.__subclasses__():
-            Processing.addProvider(c())
+            p = c()
+            PROVIDERS.append(p)
+            QgsApplication.processingRegistry().addProvider(p)
         # And initialize
         ProcessingConfig.initialize()
         ProcessingConfig.readSettings()
         RenderingStyles.loadStyles()
 
+    @staticmethod
+    def deinitialize():
+        for p in PROVIDERS:
+            QgsApplication.processingRegistry().removeProvider(p)
+
     @staticmethod
     def addScripts(folder):
         Processing.initialize()

diff --git a/python/plugins/processing/modeler/ModelerAlgorithmProvider.py b/python/plugins/processing/modeler/ModelerAlgorithmProvider.py
@@ -61,6 +61,8 @@ def load(self):
                                             ModelerUtils.defaultModelsFolder(), valuetype=Setting.MULTIPLE_FOLDERS))
         ProviderActions.registerProviderActions(self, self.actions)
         ProviderContextMenuActions.registerProviderContextMenuActions(self.contextMenuActions)
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/python/plugins/processing/preconfigured/PreconfiguredAlgorithmProvider.py b/python/plugins/processing/preconfigured/PreconfiguredAlgorithmProvider.py
@@ -57,6 +57,8 @@ def loadAlgorithms(self):
 
     def load(self):
         ProviderContextMenuActions.registerProviderContextMenuActions(self.contextMenuActions)
+        self.refreshAlgorithms()
+        return True
 
     def unload(self):
         ProviderContextMenuActions.deregisterProviderContextMenuActions(self.contextMenuActions)

diff --git a/python/plugins/processing/script/ScriptAlgorithmProvider.py b/python/plugins/processing/script/ScriptAlgorithmProvider.py
@@ -67,6 +67,8 @@ def load(self):
                                             ScriptUtils.defaultScriptsFolder(), valuetype=Setting.MULTIPLE_FOLDERS))
         ProviderActions.registerProviderActions(self, self.actions)
         ProviderContextMenuActions.registerProviderContextMenuActions(self.contextMenuActions)
+        ProcessingConfig.readSettings()
+        self.refreshAlgorithms()
         return True
 
     def unload(self):

diff --git a/src/core/processing/qgsprocessingprovider.h b/src/core/processing/qgsprocessingprovider.h
@@ -120,12 +120,13 @@ class CORE_EXPORT QgsProcessingProvider : public QObject
     /**
      * Loads the provider. This will be called when the plugin is being loaded, and any general
      * setup actions should occur in an overridden version of this method.
-     * Subclasses should not load any algorithms in their load() implementations, as that must
-     * occur within the loadAlgorithms() method.
+     * Subclasses should not individually load any algorithms in their load() implementations, as that must
+     * occur within the loadAlgorithms() method. Instead, subclasses should call refreshAlgorithms()
+     * from any overloaded load() method to trigger an initial load of the provider's algorithms.
      * \returns true if provider could be successfully loaded
      * \see unload()
      */
-    virtual bool load() { return true; }
+    virtual bool load() { refreshAlgorithms(); return true; }
 
     /**
      * Unloads the provider. Any tear-down steps required by the provider should be implemented here.

diff --git a/src/core/processing/qgsprocessingregistry.cpp b/src/core/processing/qgsprocessingregistry.cpp
@@ -38,6 +38,9 @@ bool QgsProcessingRegistry::addProvider( QgsProcessingProvider *provider )
   if ( mProviders.contains( provider->id() ) )
     return false;
 
+  if ( !provider->load() )
+    return false;
+
   provider->setParent( this );
   mProviders[ provider->id()] = provider;
   emit providerAdded( provider->id() );
@@ -54,6 +57,8 @@ bool QgsProcessingRegistry::removeProvider( QgsProcessingProvider *provider )
   if ( !mProviders.contains( id ) )
     return false;
 
+  provider->unload();
+
   delete mProviders.take( id );
   emit providerRemoved( id );
   return true;

diff --git a/src/core/processing/qgsprocessingregistry.h b/src/core/processing/qgsprocessingregistry.h
@@ -61,6 +61,8 @@ class CORE_EXPORT QgsProcessingRegistry : public QObject
      * and the provider's parent will be set to the registry.
      * Returns false if the provider could not be added (eg if a provider with a duplicate ID already exists
      * in the registry).
+     * Adding a provider to the registry automatically triggers the providers QgsProcessingProvider::load()
+     * method to populate the provider with algorithms.
      * \see removeProvider()
      */
     bool addProvider( QgsProcessingProvider *provider SIP_TRANSFER );

diff --git a/tests/src/core/testqgsprocessing.cpp b/tests/src/core/testqgsprocessing.cpp
@@ -48,6 +48,9 @@ class DummyProvider : public QgsProcessingProvider
 
     virtual QString name() const override { return "dummy"; }
 
+    void unload() override { if ( unloaded ) { *unloaded = true; } }
+
+    bool *unloaded = nullptr;
 
   protected:
 
@@ -66,6 +69,20 @@ class DummyProvider : public QgsProcessingProvider
 
     QString mId;
 
+
+};
+
+class DummyProviderNoLoad : public DummyProvider
+{
+  public:
+
+    DummyProviderNoLoad( const QString &id ) : DummyProvider( id ) {}
+
+    bool load() override
+    {
+      return false;
+    }
+
 };
 
 class TestQgsProcessing: public QObject
@@ -136,6 +153,13 @@ void TestQgsProcessing::addProvider()
   QCOMPARE( r.providers().toSet(), QSet< QgsProcessingProvider * >() << p << p2 );
   QCOMPARE( spyProviderAdded.count(), 2 );
   delete p3;
+
+  // test that adding a provider which does not load means it is not added to registry
+  DummyProviderNoLoad *p4 = new DummyProviderNoLoad( "p4" );
+  QVERIFY( !r.addProvider( p4 ) );
+  QCOMPARE( r.providers().toSet(), QSet< QgsProcessingProvider * >() << p << p2 );
+  QCOMPARE( spyProviderAdded.count(), 2 );
+  delete p4;
 }
 
 void TestQgsProcessing::providerById()
@@ -177,11 +201,16 @@ void TestQgsProcessing::removeProvider()
   QVERIFY( r.addProvider( p2 ) );
 
   // remove one by pointer
+  bool unloaded = false;
+  p->unloaded = &unloaded;
   QVERIFY( r.removeProvider( p ) );
   QCOMPARE( spyProviderRemoved.count(), 1 );
   QCOMPARE( spyProviderRemoved.last().at( 0 ).toString(), QString( "p1" ) );
   QCOMPARE( r.providers(), QList< QgsProcessingProvider * >() << p2 );
 
+  //test that provider was unloaded
+  QVERIFY( unloaded );
+
   // should fail, already removed
   QVERIFY( !r.removeProvider( "p1" ) );
 
@@ -387,6 +416,18 @@ void TestQgsProcessing::algorithm()
   QCOMPARE( r.algorithmById( "p1:alg2" ), p->algorithm( "alg2" ) );
   QVERIFY( !r.algorithmById( "p1:alg3" ) );
   QVERIFY( !r.algorithmById( "px:alg1" ) );
+
+  //test that loading a provider triggers an algorithm refresh
+  DummyProvider *p2 = new DummyProvider( "p2" );
+  QVERIFY( p2->algorithms().isEmpty() );
+  p2->load();
+  QCOMPARE( p2->algorithms().size(), 2 );
+
+  // test that adding a provider to the registry automatically refreshes algorithms (via load)
+  DummyProvider *p3 = new DummyProvider( "p3" );
+  QVERIFY( p3->algorithms().isEmpty() );
+  r.addProvider( p3 );
+  QCOMPARE( p3->algorithms().size(), 2 );
 }
 
 QGSTEST_MAIN( TestQgsProcessing )