Merge pull request #9215 from elpaso/release-3_4-backports

Release 3 4 backports

Author: elpaso

python/core/auto_generated/processing/qgsprocessingalgorithm.sip.in

@@ -63,7 +63,7 @@ a pre-initialized copy of the algorithm.
 
 
 
-    QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const /TransferBack/;
+    QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const throw( QgsProcessingException ) /TransferBack/;
 %Docstring
 Creates a copy of the algorithm, ready for execution.
 
@@ -76,6 +76,9 @@ and outputs according to this configuration. This is generally used only for
 algorithms in a model, allowing them to adjust their behavior at run time
 according to some user configuration.
 
+Raises a QgsProcessingException if a new algorithm instance could not be created,
+e.g. if there is an issue with the subclass' createInstance() method.
+
 .. seealso:: :py:func:`initAlgorithm`
 %End
 
@@ -401,7 +404,7 @@ Associates this algorithm with its provider. No transfer of ownership is involve
 
   protected:
 
-    virtual QgsProcessingAlgorithm *createInstance() const = 0 /Factory/;
+    virtual QgsProcessingAlgorithm *createInstance() const = 0 /Factory,VirtualErrorHandler=processing_exception_handler/;
 %Docstring
 Creates a new instance of the algorithm class.
 

python/plugins/processing/gui/AlgorithmDialog.py

@@ -263,8 +263,11 @@ def on_complete(ok, results):
                     self.showLog()
 
                     task = QgsProcessingAlgRunnerTask(self.algorithm(), parameters, self.context, self.feedback)
-                    task.executed.connect(on_complete)
-                    self.setCurrentTask(task)
+                    if task.isCanceled():
+                        on_complete(False, {})
+                    else:
+                        task.executed.connect(on_complete)
+                        self.setCurrentTask(task)
                 else:
                     self.proxy_progress = QgsProxyProgressTask(QCoreApplication.translate("AlgorithmDialog", "Executing “{}”").format(self.algorithm().displayName()))
                     QgsApplication.taskManager().addTask(self.proxy_progress)

src/analysis/vector/geometry_checker/qgsgeometrygapcheck.cpp

@@ -45,7 +45,7 @@ void QgsGeometryGapCheck::collectErrors( const QMap<QString, QgsFeaturePool *> &
   {
     geomList.append( layerFeature.geometry().constGet()->clone() );
 
-    if ( feedback->isCanceled() )
+    if ( feedback && feedback->isCanceled() )
     {
       qDeleteAll( geomList );
       geomList.clear();

src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp

@@ -43,7 +43,7 @@ void QgsGeometryMissingVertexCheck::collectErrors( const QMap<QString, QgsFeatur
 
   for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeature : layerFeatures )
   {
-    if ( feedback->isCanceled() )
+    if ( feedback && feedback->isCanceled() )
     {
       break;
     }
@@ -152,7 +152,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
 
     if ( featurePool->getFeature( fid, compareFeature, feedback ) )
     {
-      if ( feedback->isCanceled() )
+      if ( feedback && feedback->isCanceled() )
         break;
 
       QgsVertexIterator vertexIterator = compareFeature.geometry().vertices();

src/analysis/vector/geometry_checker/qgsgeometryoverlapcheck.cpp

@@ -35,7 +35,7 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
   QList<QString> layerIds = featureIds.keys();
   for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureA : layerFeaturesA )
   {
-    if ( feedback->isCanceled() )
+    if ( feedback && feedback->isCanceled() )
       break;
 
     // Ensure each pair of layers only gets compared once: remove the current layer from the layerIds, but add it to the layerList for layerFeaturesB
@@ -52,7 +52,7 @@ void QgsGeometryOverlapCheck::collectErrors( const QMap<QString, QgsFeaturePool
     const QgsGeometryCheckerUtils::LayerFeatures layerFeaturesB( featurePools, QList<QString>() << layerFeatureA.layer()->id() << layerIds, bboxA, compatibleGeometryTypes(), mContext );
     for ( const QgsGeometryCheckerUtils::LayerFeature &layerFeatureB : layerFeaturesB )
     {
-      if ( feedback->isCanceled() )
+      if ( feedback && feedback->isCanceled() )
         break;
 
       // > : only report overlaps within same layer once

src/app/qgsvectorlayerloadstyledialog.cpp

@@ -56,15 +56,21 @@ QgsVectorLayerLoadStyleDialog::QgsVectorLayerLoadStyleDialog( QgsVectorLayer *la
   connect( mStyleTypeComboBox, qgis::overload<int>::of( &QComboBox::currentIndexChanged ), this, [ = ]( int )
   {
     QgsVectorLayerProperties::StyleType type = currentStyleType();
-    mFromFileWidget->setVisible( type != QgsVectorLayerProperties::DB );
-    mFromDbWidget->setVisible( type == QgsVectorLayerProperties::DB );
-    mDeleteButton->setVisible( type == QgsVectorLayerProperties::DB && mLayer->dataProvider()->isDeleteStyleFromDatabaseSupported() );
-    mStyleCategoriesListView->setEnabled( currentStyleType() != QgsVectorLayerProperties::SLD );
+    mFromFileWidget->setVisible( type != QgsVectorLayerProperties::StyleType::DB );
+    mFromDbWidget->setVisible( type == QgsVectorLayerProperties::StyleType::DB );
+    mDeleteButton->setVisible( type == QgsVectorLayerProperties::StyleType::DB && mLayer->dataProvider()->isDeleteStyleFromDatabaseSupported() );
+    mStyleCategoriesListView->setEnabled( currentStyleType() != QgsVectorLayerProperties::StyleType::SLD );
     updateLoadButtonState();
   } );
   mStyleTypeComboBox->addItem( tr( "from file" ), QgsVectorLayerProperties::QML ); // QML is used as entry, but works for SLD too, see currentStyleType()
   if ( mLayer->dataProvider()->isSaveAndLoadStyleToDatabaseSupported() )
-    mStyleTypeComboBox->addItem( tr( "from database (%1)" ).arg( providerName ), QgsVectorLayerProperties::DB );
+  {
+    mStyleTypeComboBox->addItem( tr( "from database (%1)" ).arg( providerName ), QgsVectorLayerProperties::StyleType::DB );
+    if ( settings.value( QStringLiteral( "style/lastLoadStyleTypeSelection" ) ) == QgsVectorLayerProperties::StyleType::DB )
+    {
+      mStyleTypeComboBox->setCurrentIndex( mStyleTypeComboBox->findData( QgsVectorLayerProperties::StyleType::DB ) );
+    }
+  }
 
   // fill style categories
   mModel = new QgsMapLayerStyleCategoriesModel( this );
@@ -105,6 +111,11 @@ QgsVectorLayerLoadStyleDialog::QgsVectorLayerLoadStyleDialog( QgsVectorLayer *la
   connect( mButtonBox, &QDialogButtonBox::helpRequested, this, &QgsVectorLayerLoadStyleDialog::showHelp );
   connect( mLoadButton, &QPushButton::clicked, this, &QDialog::accept );
   connect( mDeleteButton, &QPushButton::clicked, this, &QgsVectorLayerLoadStyleDialog::deleteStyleFromDB );
+  connect( this, &QgsVectorLayerLoadStyleDialog::rejected, [ = ]
+  {
+    QgsSettings().setValue( QStringLiteral( "style/lastLoadStyleTypeSelection" ), currentStyleType() );
+  } );
+
   setTabOrder( mRelatedTable, mOthersTable );
 
   restoreGeometry( settings.value( QStringLiteral( "Windows/vectorLayerLoadStyle/geometry" ) ).toByteArray() );
@@ -113,8 +124,7 @@ QgsVectorLayerLoadStyleDialog::QgsVectorLayerLoadStyleDialog( QgsVectorLayer *la
 
 QgsVectorLayerLoadStyleDialog::~QgsVectorLayerLoadStyleDialog()
 {
-  QgsSettings settings;
-  settings.setValue( QStringLiteral( "Windows/vectorLayerLoadStyle/geometry" ), saveGeometry() );
+  QgsSettings().setValue( QStringLiteral( "Windows/vectorLayerLoadStyle/geometry" ), saveGeometry() );
 }
 
 QgsMapLayer::StyleCategories QgsVectorLayerLoadStyleDialog::styleCategories() const
@@ -246,7 +256,9 @@ void QgsVectorLayerLoadStyleDialog::selectionChanged( QTableWidget *styleTable )
 
 void QgsVectorLayerLoadStyleDialog::accept()
 {
-  QgsSettings().setFlagValue( QStringLiteral( "style/lastStyleCategories" ), styleCategories() );
+  QgsSettings settings;
+  settings.setFlagValue( QStringLiteral( "style/lastStyleCategories" ), styleCategories() );
+  settings.setValue( QStringLiteral( "style/lastLoadStyleTypeSelection" ), currentStyleType() );
   QDialog::accept();
 }
 
@@ -274,7 +286,7 @@ void QgsVectorLayerLoadStyleDialog::deleteStyleFromDB()
     mRelatedTable->setRowCount( 0 );
     mOthersTable->setRowCount( 0 );
 
-    //Fill UI widgets again from DB. Other users might have change the styles meanwhile.
+    //Fill UI widgets again from DB. Other users might have changed the styles meanwhile.
     QString errorMsg;
     QStringList ids, names, descriptions;
     //get the list of styles in the db

src/core/processing/qgsprocessingalgorithm.cpp

@@ -37,6 +37,8 @@ QgsProcessingAlgorithm::~QgsProcessingAlgorithm()
 QgsProcessingAlgorithm *QgsProcessingAlgorithm::create( const QVariantMap &configuration ) const
 {
   std::unique_ptr< QgsProcessingAlgorithm > creation( createInstance() );
+  if ( ! creation )
+    throw QgsProcessingException( QObject::tr( "Error creating algorithm from createInstance()" ) );
   creation->setProvider( provider() );
   creation->initAlgorithm( configuration );
   return creation.release();

src/core/processing/qgsprocessingalgorithm.h

@@ -121,9 +121,12 @@ class CORE_EXPORT QgsProcessingAlgorithm
      * algorithms in a model, allowing them to adjust their behavior at run time
      * according to some user configuration.
      *
+     * Raises a QgsProcessingException if a new algorithm instance could not be created,
+     * e.g. if there is an issue with the subclass' createInstance() method.
+     *
      * \see initAlgorithm()
      */
-    QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_TRANSFERBACK;
+    QgsProcessingAlgorithm *create( const QVariantMap &configuration = QVariantMap() ) const SIP_THROW( QgsProcessingException ) SIP_TRANSFERBACK;
 
     /**
      * Returns the algorithm name, used for identifying the algorithm. This string
@@ -408,7 +411,7 @@ class CORE_EXPORT QgsProcessingAlgorithm
      *
      * This method should return a 'pristine' instance of the algorithm class.
      */
-    virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY;
+    virtual QgsProcessingAlgorithm *createInstance() const = 0 SIP_FACTORY SIP_VIRTUALERRORHANDLER( processing_exception_handler );
 
     /**
      * Initializes the algorithm using the specified \a configuration.

src/core/processing/qgsprocessingalgrunnertask.cpp

@@ -27,20 +27,30 @@ QgsProcessingAlgRunnerTask::QgsProcessingAlgRunnerTask( const QgsProcessingAlgor
   , mParameters( parameters )
   , mContext( context )
   , mFeedback( feedback )
-  , mAlgorithm( algorithm->create() )
 {
   if ( !mFeedback )
   {
     mOwnedFeedback.reset( new QgsProcessingFeedback() );
     mFeedback = mOwnedFeedback.get();
   }
-  if ( !mAlgorithm->prepare( mParameters, context, mFeedback ) )
+  try
+  {
+    mAlgorithm.reset( algorithm->create() );
+    if ( !( mAlgorithm && mAlgorithm->prepare( mParameters, context, mFeedback ) ) )
+      cancel();
+  }
+  catch ( QgsProcessingException &e )
+  {
+    QgsMessageLog::logMessage( e.what(), QObject::tr( "Processing" ), Qgis::Critical );
+    mFeedback->reportError( e.what() );
     cancel();
+  }
 }
 
 void QgsProcessingAlgRunnerTask::cancel()
 {
-  mFeedback->cancel();
+  if ( mFeedback )
+    mFeedback->cancel();
   QgsTask::cancel();
 }
 

src/core/symbology/qgsrulebasedrenderer.cpp

@@ -246,7 +246,7 @@ QgsLegendSymbolList QgsRuleBasedRenderer::Rule::legendSymbolItems( int currentLe
 
 bool QgsRuleBasedRenderer::Rule::isFilterOK( const QgsFeature &f, QgsRenderContext *context ) const
 {
-  if ( ! mFilter || mElseRule )
+  if ( ! mFilter || mElseRule || ! context )
     return true;
 
   context->expressionContext().setFeature( f );
@@ -647,7 +647,7 @@ QSet<QString> QgsRuleBasedRenderer::Rule::legendKeysForFeature( const QgsFeature
 QgsRuleBasedRenderer::RuleList QgsRuleBasedRenderer::Rule::rulesForFeature( const QgsFeature &feature, QgsRenderContext *context, bool onlyActive )
 {
   RuleList lst;
-  if ( !isFilterOK( feature, context ) )
+  if ( ! isFilterOK( feature, context ) || ( context && ! isScaleOK( context->rendererScale() ) ) )
     return lst;
 
   if ( mSymbol )

src/providers/ogr/qgsogrprovider.cpp

@@ -6199,7 +6199,7 @@ QGISEXTERN int listStyles( const QString &uri, QStringList &ids, QStringList &na
 
       listTimestamp.append( ts );
       mapIdToStyleName[fid] = styleName;
-      mapIdToDescription[fid] = styleName;
+      mapIdToDescription[fid] = description;
       mapTimestampToId[ts].append( fid );
     }
   }

tests/src/python/CMakeLists.txt

@@ -146,6 +146,7 @@ ADD_PYTHON_TEST(PyQgsPointDisplacementRenderer test_qgspointdisplacementrenderer
 ADD_PYTHON_TEST(PyQgsPostgresDomain test_qgspostgresdomain.py)
 ADD_PYTHON_TEST(PyQgsProcessingRecentAlgorithmLog test_qgsprocessingrecentalgorithmslog.py)
 ADD_PYTHON_TEST(PyQgsProcessingInPlace test_qgsprocessinginplace.py)
+ADD_PYTHON_TEST(PyQgsProcessingAlgRunner test_qgsprocessingalgrunner.py)
 ADD_PYTHON_TEST(PyQgsProjectionSelectionWidgets test_qgsprojectionselectionwidgets.py)
 ADD_PYTHON_TEST(PyQgsProjectMetadata test_qgsprojectmetadata.py)
 ADD_PYTHON_TEST(PyQgsRange test_qgsrange.py)

tests/src/python/test_provider_ogr_gpkg.py

@@ -489,7 +489,7 @@ def testStyle(self):
         self.assertEqual(errmsg, "")
         self.assertEqual(idlist, ['1', '3', '4', '2'])
         self.assertEqual(namelist, ['name', 'name2', 'name3', 'name_test2'])
-        self.assertEqual(desclist, ['description_bis', 'description2', 'description3', 'name_test2'])
+        self.assertEqual(desclist, ['description_bis', 'description2', 'description3', 'description_test2'])
 
         # Check that layers_style table is not list in subLayers()
         vl = QgsVectorLayer(tmpfile, 'test', 'ogr')

tests/src/python/test_qgsprocessingalgrunner.py

@@ -0,0 +1,110 @@
+# -*- coding: utf-8 -*-
+"""QGIS Unit tests for Processing algorithm runner(s).
+
+.. note:: This program is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 2 of the License, or
+(at your option) any later version.
+"""
+__author__ = 'Alessandro Pasotti'
+__date__ = '2019-02'
+__copyright__ = 'Copyright 2019, The QGIS Project'
+# This will get replaced with a git SHA1 when you do a git archive
+__revision__ = '$Format:%H$'
+
+import re
+from qgis.PyQt.QtCore import QCoreApplication
+from qgis.testing import start_app, unittest
+from qgis.core import QgsProcessingAlgRunnerTask
+
+from processing.core.Processing import Processing
+from processing.core.ProcessingConfig import ProcessingConfig
+from qgis.testing import start_app, unittest
+from qgis.analysis import QgsNativeAlgorithms
+from qgis.core import (
+    QgsApplication,
+    QgsSettings,
+    QgsProcessingContext,
+    QgsProcessingAlgRunnerTask,
+    QgsProcessingAlgorithm,
+    QgsProject,
+    QgsProcessingFeedback,
+)
+
+start_app()
+
+
+class ConsoleFeedBack(QgsProcessingFeedback):
+
+    _error = ''
+
+    def reportError(self, error, fatalError=False):
+        self._error = error
+        print(error)
+
+
+class CrashingProcessingAlgorithm(QgsProcessingAlgorithm):
+    """
+    Wrong class in factory createInstance()
+    """
+
+    INPUT = 'INPUT'
+    OUTPUT = 'OUTPUT'
+
+    def tr(self, string):
+        return QCoreApplication.translate('Processing', string)
+
+    def createInstance(self):
+        """Wrong!"""
+        return ExampleProcessingAlgorithm()
+
+    def name(self):
+        return 'mycrashingscript'
+
+    def displayName(self):
+        return self.tr('My Crashing Script')
+
+    def group(self):
+        return self.tr('Example scripts')
+
+    def groupId(self):
+        return 'examplescripts'
+
+    def shortHelpString(self):
+        return self.tr("Example algorithm short description")
+
+    def initAlgorithm(self, config=None):
+        pass
+
+    def processAlgorithm(self, parameters, context, feedback):
+        return {self.OUTPUT: 'an_id'}
+
+
+class TestQgsProcessingAlgRunner(unittest.TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        """Run before all tests"""
+        QCoreApplication.setOrganizationName("QGIS_Test")
+        QCoreApplication.setOrganizationDomain(
+            "QGIS_TestPyQgsProcessingInPlace.com")
+        QCoreApplication.setApplicationName("QGIS_TestPyQgsProcessingInPlace")
+        QgsSettings().clear()
+        Processing.initialize()
+        QgsApplication.processingRegistry().addProvider(QgsNativeAlgorithms())
+        cls.registry = QgsApplication.instance().processingRegistry()
+
+    def test_bad_script_dont_crash(self):  # spellok
+        """Test regression #21270 (segfault)"""
+
+        context = QgsProcessingContext()
+        context.setProject(QgsProject.instance())
+        feedback = ConsoleFeedBack()
+
+        task = QgsProcessingAlgRunnerTask(CrashingProcessingAlgorithm(), {}, context=context, feedback=feedback)
+        self.assertTrue(task.isCanceled())
+        self.assertIn('name \'ExampleProcessingAlgorithm\' is not defined', feedback._error)
+
+
+if __name__ == '__main__':
+    unittest.main()