From e91aed6617416428bc790cbcafa48c2ccc357fbf Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 4 Jun 2018 17:10:10 +1000 Subject: [PATCH] [processing] Force model outputs to respect constraints set by their underlying algorithm's provider E.g. for model outputs generated by a saga algorithm, only sdat and shp files are valid outputs. So only give users choices of these instead of all formats. Also fixes temporary file names generated as part of model execution may use formats which are not compatible with the algorithm's provider. Fixes #18908 --- .../processing/qgsprocessingparameters.sip.in | 40 +++++++ .../processing/gui/ParameterGuiUtils.py | 10 +- .../models/qgsprocessingmodelalgorithm.cpp | 13 +- .../processing/qgsprocessingparameters.cpp | 67 ++++++++++- src/core/processing/qgsprocessingparameters.h | 44 +++++++ tests/src/analysis/testqgsprocessing.cpp | 111 ++++++++++++++++++ 6 files changed, 273 insertions(+), 12 deletions(-) diff --git a/python/core/auto_generated/processing/qgsprocessingparameters.sip.in b/python/core/auto_generated/processing/qgsprocessingparameters.sip.in index 5e79a968266..48bed9831ff 100644 --- a/python/core/auto_generated/processing/qgsprocessingparameters.sip.in +++ b/python/core/auto_generated/processing/qgsprocessingparameters.sip.in @@ -1901,6 +1901,19 @@ Sets whether the destination should be created by default. For optional paramete a value of false indicates that the destination should not be created by default. .. seealso:: :py:func:`createByDefault` +%End + + protected: + + QgsProcessingProvider *originalProvider() const; +%Docstring +Original (source) provider which this parameter has been derived from. +In the case of destination parameters which are part of model algorithms, this +will reflect the child algorithm's provider which actually generates the +parameter, as opposed to the provider which this parameter belongs to (i.e. +the model provider) + +.. versionadded:: 3.2 %End }; @@ -1948,6 +1961,15 @@ Returns the type name for the parameter class. virtual QString defaultFileExtension() const; + virtual QStringList supportedOutputVectorLayerExtensions() const; +%Docstring +Returns a list of the vector format file extensions supported by this parameter. + +.. seealso:: :py:func:`defaultFileExtension` + +.. versionadded:: 3.2 +%End + QgsProcessing::SourceType dataType() const; %Docstring Returns the layer type for sinks associated with the parameter. @@ -2029,6 +2051,15 @@ Returns the type name for the parameter class. virtual QString defaultFileExtension() const; + virtual QStringList supportedOutputVectorLayerExtensions() const; +%Docstring +Returns a list of the vector format file extensions supported by this parameter. + +.. seealso:: :py:func:`defaultFileExtension` + +.. versionadded:: 3.2 +%End + QgsProcessing::SourceType dataType() const; %Docstring Returns the layer type for this created vector layer. @@ -2103,6 +2134,15 @@ Returns the type name for the parameter class. virtual QString defaultFileExtension() const; + virtual QStringList supportedOutputRasterLayerExtensions() const; +%Docstring +Returns a list of the raster format file extensions supported for this parameter. + +.. seealso:: :py:func:`defaultFileExtension` + +.. versionadded:: 3.2 +%End + static QgsProcessingParameterRasterDestination *fromScriptCode( const QString &name, const QString &description, bool isOptional, const QString &definition ) /Factory/; %Docstring Creates a new parameter using the definition from a script code. diff --git a/python/plugins/processing/gui/ParameterGuiUtils.py b/python/plugins/processing/gui/ParameterGuiUtils.py index a154eeecde6..fae75573835 100644 --- a/python/plugins/processing/gui/ParameterGuiUtils.py +++ b/python/plugins/processing/gui/ParameterGuiUtils.py @@ -68,18 +68,12 @@ def getFileFilter(param): elif param.type() == 'raster': return QgsProviderRegistry.instance().fileRasterFilters() elif param.type() == 'rasterDestination': - if param.provider() is not None: - exts = param.provider().supportedOutputRasterLayerExtensions() - else: - exts = QgsRasterFileWriter.supportedFormatExtensions() + exts = param.supportedOutputRasterLayerExtensions() for i in range(len(exts)): exts[i] = tr('{0} files (*.{1})', 'ParameterRaster').format(exts[i].upper(), exts[i].lower()) return ';;'.join(exts) + ';;' + tr('All files (*.*)') elif param.type() in ('sink', 'vectorDestination'): - if param.provider() is not None: - exts = param.provider().supportedOutputVectorLayerExtensions() - else: - exts = QgsVectorFileWriter.supportedFormatExtensions() + exts = param.supportedOutputVectorLayerExtensions() for i in range(len(exts)): exts[i] = tr('{0} files (*.{1})', 'ParameterVector').format(exts[i].upper(), exts[i].lower()) return ';;'.join(exts) + ';;' + tr('All files (*.*)') diff --git a/src/core/processing/models/qgsprocessingmodelalgorithm.cpp b/src/core/processing/models/qgsprocessingmodelalgorithm.cpp index 3e05879f4fd..486d59ebb33 100644 --- a/src/core/processing/models/qgsprocessingmodelalgorithm.cpp +++ b/src/core/processing/models/qgsprocessingmodelalgorithm.cpp @@ -791,7 +791,18 @@ void QgsProcessingModelAlgorithm::updateDestinationParameters() param->setName( outputIt->childId() + ':' + outputIt->name() ); param->setDescription( outputIt->description() ); param->setDefaultValue( outputIt->defaultValue() ); - addParameter( param.release() ); + + QgsProcessingDestinationParameter *newDestParam = dynamic_cast< QgsProcessingDestinationParameter * >( param.get() ); + if ( addParameter( param.release() ) && newDestParam ) + { + if ( QgsProcessingProvider *provider = childIt->algorithm()->provider() ) + { + // we need to copy the constraints given by the provider which creates this output across + // and replace those which have been set to match the model provider's constraints + newDestParam->setSupportsNonFileBasedOutput( provider->supportsNonFileBasedOutput() ); + newDestParam->mOriginalProvider = provider; + } + } } } } diff --git a/src/core/processing/qgsprocessingparameters.cpp b/src/core/processing/qgsprocessingparameters.cpp index e69bd4920fb..04246b4fb76 100644 --- a/src/core/processing/qgsprocessingparameters.cpp +++ b/src/core/processing/qgsprocessingparameters.cpp @@ -27,6 +27,7 @@ #include "qgsreferencedgeometry.h" #include "qgsprocessingregistry.h" #include "qgsprocessingparametertype.h" +#include "qgsrasterfilewriter.h" #include @@ -3366,7 +3367,11 @@ QgsProcessingOutputDefinition *QgsProcessingParameterFeatureSink::toOutputDefini QString QgsProcessingParameterFeatureSink::defaultFileExtension() const { - if ( QgsProcessingProvider *p = provider() ) + if ( originalProvider() ) + { + return originalProvider()->defaultVectorFileExtension( hasGeometry() ); + } + else if ( QgsProcessingProvider *p = provider() ) { return p->defaultVectorFileExtension( hasGeometry() ); } @@ -3384,6 +3389,22 @@ QString QgsProcessingParameterFeatureSink::defaultFileExtension() const } } +QStringList QgsProcessingParameterFeatureSink::supportedOutputVectorLayerExtensions() const +{ + if ( originalProvider() ) + { + return originalProvider()->supportedOutputVectorLayerExtensions(); + } + else if ( QgsProcessingProvider *p = provider() ) + { + return p->supportedOutputVectorLayerExtensions(); + } + else + { + return QgsVectorFileWriter::supportedFormatExtensions(); + } +} + QgsProcessing::SourceType QgsProcessingParameterFeatureSink::dataType() const { return mDataType; @@ -3538,7 +3559,11 @@ QgsProcessingOutputDefinition *QgsProcessingParameterRasterDestination::toOutput QString QgsProcessingParameterRasterDestination::defaultFileExtension() const { - if ( QgsProcessingProvider *p = provider() ) + if ( originalProvider() ) + { + return originalProvider()->defaultRasterFileExtension(); + } + else if ( QgsProcessingProvider *p = provider() ) { return p->defaultRasterFileExtension(); } @@ -3549,6 +3574,22 @@ QString QgsProcessingParameterRasterDestination::defaultFileExtension() const } } +QStringList QgsProcessingParameterRasterDestination::supportedOutputRasterLayerExtensions() const +{ + if ( originalProvider() ) + { + return originalProvider()->supportedOutputRasterLayerExtensions(); + } + else if ( QgsProcessingProvider *p = provider() ) + { + return p->supportedOutputRasterLayerExtensions(); + } + else + { + return QgsRasterFileWriter::supportedFormatExtensions(); + } +} + QgsProcessingParameterRasterDestination *QgsProcessingParameterRasterDestination::fromScriptCode( const QString &name, const QString &description, bool isOptional, const QString &definition ) { return new QgsProcessingParameterRasterDestination( name, description, definition.isEmpty() ? QVariant() : definition, isOptional ); @@ -3886,7 +3927,11 @@ QgsProcessingOutputDefinition *QgsProcessingParameterVectorDestination::toOutput QString QgsProcessingParameterVectorDestination::defaultFileExtension() const { - if ( QgsProcessingProvider *p = provider() ) + if ( originalProvider() ) + { + return originalProvider()->defaultVectorFileExtension( hasGeometry() ); + } + else if ( QgsProcessingProvider *p = provider() ) { return p->defaultVectorFileExtension( hasGeometry() ); } @@ -3904,6 +3949,22 @@ QString QgsProcessingParameterVectorDestination::defaultFileExtension() const } } +QStringList QgsProcessingParameterVectorDestination::supportedOutputVectorLayerExtensions() const +{ + if ( originalProvider() ) + { + return originalProvider()->supportedOutputVectorLayerExtensions(); + } + else if ( QgsProcessingProvider *p = provider() ) + { + return p->supportedOutputVectorLayerExtensions(); + } + else + { + return QgsVectorFileWriter::supportedFormatExtensions(); + } +} + QgsProcessing::SourceType QgsProcessingParameterVectorDestination::dataType() const { return mDataType; diff --git a/src/core/processing/qgsprocessingparameters.h b/src/core/processing/qgsprocessingparameters.h index 1fb8e1eae00..57120ce55f1 100644 --- a/src/core/processing/qgsprocessingparameters.h +++ b/src/core/processing/qgsprocessingparameters.h @@ -1831,11 +1831,34 @@ class CORE_EXPORT QgsProcessingDestinationParameter : public QgsProcessingParame */ void setCreateByDefault( bool createByDefault ); + protected: + + /** + * Original (source) provider which this parameter has been derived from. + * In the case of destination parameters which are part of model algorithms, this + * will reflect the child algorithm's provider which actually generates the + * parameter, as opposed to the provider which this parameter belongs to (i.e. + * the model provider) + * \since QGIS 3.2 + */ + QgsProcessingProvider *originalProvider() const { return mOriginalProvider; } + private: + /** + * Original (source) provider which this parameter has been derived from. + * In the case of destination parameters which are part of model algorithms, this + * will reflect the child algorithm's provider which actually generates the + * parameter, as opposed to the provider which this parameter belongs to (i.e. + * the model provider) + */ + QgsProcessingProvider *mOriginalProvider = nullptr; + bool mSupportsNonFileBasedOutputs = true; bool mCreateByDefault = true; + friend class QgsProcessingModelAlgorithm; + friend class TestQgsProcessing; }; @@ -1872,6 +1895,13 @@ class CORE_EXPORT QgsProcessingParameterFeatureSink : public QgsProcessingDestin QgsProcessingOutputDefinition *toOutputDefinition() const override SIP_FACTORY; QString defaultFileExtension() const override; + /** + * Returns a list of the vector format file extensions supported by this parameter. + * \see defaultFileExtension() + * \since QGIS 3.2 + */ + virtual QStringList supportedOutputVectorLayerExtensions() const; + /** * Returns the layer type for sinks associated with the parameter. * \see setDataType() @@ -1940,6 +1970,13 @@ class CORE_EXPORT QgsProcessingParameterVectorDestination : public QgsProcessing QgsProcessingOutputDefinition *toOutputDefinition() const override SIP_FACTORY; QString defaultFileExtension() const override; + /** + * Returns a list of the vector format file extensions supported by this parameter. + * \see defaultFileExtension() + * \since QGIS 3.2 + */ + virtual QStringList supportedOutputVectorLayerExtensions() const; + /** * Returns the layer type for this created vector layer. * \see setDataType() @@ -2005,6 +2042,13 @@ class CORE_EXPORT QgsProcessingParameterRasterDestination : public QgsProcessing QgsProcessingOutputDefinition *toOutputDefinition() const override SIP_FACTORY; QString defaultFileExtension() const override; + /** + * Returns a list of the raster format file extensions supported for this parameter. + * \see defaultFileExtension() + * \since QGIS 3.2 + */ + virtual QStringList supportedOutputRasterLayerExtensions() const; + /** * Creates a new parameter using the definition from a script code. */ diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp index b6ade1a3ef7..016abdc724e 100644 --- a/tests/src/analysis/testqgsprocessing.cpp +++ b/tests/src/analysis/testqgsprocessing.cpp @@ -404,6 +404,62 @@ class DummyProvider3 : public QgsProcessingProvider }; +class DummyAlgorithm2 : public QgsProcessingAlgorithm +{ + public: + + DummyAlgorithm2( const QString &name ) : mName( name ) { mFlags = QgsProcessingAlgorithm::flags(); } + + void initAlgorithm( const QVariantMap & = QVariantMap() ) override + { + addParameter( new QgsProcessingParameterVectorDestination( QStringLiteral( "vector_dest" ) ) ); + addParameter( new QgsProcessingParameterRasterDestination( QStringLiteral( "raster_dest" ) ) ); + addParameter( new QgsProcessingParameterFeatureSink( QStringLiteral( "sink" ) ) ); + } + QString name() const override { return mName; } + QString displayName() const override { return mName; } + QVariantMap processAlgorithm( const QVariantMap &, QgsProcessingContext &, QgsProcessingFeedback * ) override { return QVariantMap(); } + + Flags flags() const override { return mFlags; } + DummyAlgorithm2 *createInstance() const override { return new DummyAlgorithm2( name() ); } + + QString mName; + + Flags mFlags; + +}; + + +class DummyProvider4 : public QgsProcessingProvider +{ + public: + + DummyProvider4() = default; + QString id() const override { return QStringLiteral( "dummy4" ); } + QString name() const override { return QStringLiteral( "dummy4" ); } + + bool supportsNonFileBasedOutput() const override + { + return false; + } + + QStringList supportedOutputVectorLayerExtensions() const override + { + return QStringList() << QStringLiteral( "mif" ); + } + + QStringList supportedOutputRasterLayerExtensions() const override + { + return QStringList() << QStringLiteral( "mig" ); + } + + void loadAlgorithms() override + { + QVERIFY( addAlgorithm( new DummyAlgorithm2( "alg1" ) ) ); + } + +}; + class DummyParameterType : public QgsProcessingParameterType { @@ -494,6 +550,7 @@ class TestQgsProcessing: public QObject void asPythonCommand(); void modelerAlgorithm(); void modelExecution(); + void modelWithProviderWithLimitedTypes(); void modelVectorOutputIsCompatibleType(); void modelAcceptableValues(); void tempUtils(); @@ -5807,6 +5864,7 @@ void TestQgsProcessing::modelerAlgorithm() QCOMPARE( alg7.destinationParameterDefinitions().count(), 1 ); QCOMPARE( alg7.destinationParameterDefinitions().at( 0 )->name(), QStringLiteral( "cx1:my_output" ) ); QCOMPARE( alg7.destinationParameterDefinitions().at( 0 )->description(), QStringLiteral( "my output" ) ); + QCOMPARE( static_cast< const QgsProcessingDestinationParameter * >( alg7.destinationParameterDefinitions().at( 0 ) )->originalProvider()->id(), QStringLiteral( "native" ) ); QCOMPARE( alg7.outputDefinitions().count(), 1 ); QCOMPARE( alg7.outputDefinitions().at( 0 )->name(), QStringLiteral( "cx1:my_output" ) ); QCOMPARE( alg7.outputDefinitions().at( 0 )->type(), QStringLiteral( "outputVector" ) ); @@ -6083,6 +6141,59 @@ void TestQgsProcessing::modelExecution() QCOMPARE( actualParts, expectedParts ); } +void TestQgsProcessing::modelWithProviderWithLimitedTypes() +{ + QgsApplication::processingRegistry()->addProvider( new DummyProvider4() ); + + QgsProcessingModelAlgorithm alg( "test", "testGroup" ); + QgsProcessingModelChildAlgorithm algc1; + algc1.setChildId( "cx1" ); + algc1.setAlgorithmId( "dummy4:alg1" ); + QMap algc1outputs; + QgsProcessingModelOutput algc1out1( QStringLiteral( "my_vector_output" ) ); + algc1out1.setChildId( "cx1" ); + algc1out1.setChildOutputName( "vector_dest" ); + algc1out1.setDescription( QStringLiteral( "my output" ) ); + algc1outputs.insert( QStringLiteral( "my_vector_output" ), algc1out1 ); + QgsProcessingModelOutput algc1out2( QStringLiteral( "my_raster_output" ) ); + algc1out2.setChildId( "cx1" ); + algc1out2.setChildOutputName( "raster_dest" ); + algc1out2.setDescription( QStringLiteral( "my output" ) ); + algc1outputs.insert( QStringLiteral( "my_raster_output" ), algc1out2 ); + QgsProcessingModelOutput algc1out3( QStringLiteral( "my_sink_output" ) ); + algc1out3.setChildId( "cx1" ); + algc1out3.setChildOutputName( "sink" ); + algc1out3.setDescription( QStringLiteral( "my output" ) ); + algc1outputs.insert( QStringLiteral( "my_sink_output" ), algc1out3 ); + algc1.setModelOutputs( algc1outputs ); + alg.addChildAlgorithm( algc1 ); + // verify that model has destination parameter created + QCOMPARE( alg.destinationParameterDefinitions().count(), 3 ); + QCOMPARE( alg.destinationParameterDefinitions().at( 2 )->name(), QStringLiteral( "cx1:my_vector_output" ) ); + QCOMPARE( alg.destinationParameterDefinitions().at( 2 )->description(), QStringLiteral( "my output" ) ); + QCOMPARE( static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 2 ) )->originalProvider()->id(), QStringLiteral( "dummy4" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterVectorDestination * >( alg.destinationParameterDefinitions().at( 2 ) )->supportedOutputVectorLayerExtensions(), QStringList() << QStringLiteral( "mif" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterVectorDestination * >( alg.destinationParameterDefinitions().at( 2 ) )->defaultFileExtension(), QStringLiteral( "mif" ) ); + QVERIFY( static_cast< const QgsProcessingParameterVectorDestination * >( alg.destinationParameterDefinitions().at( 2 ) )->generateTemporaryDestination().endsWith( QStringLiteral( ".mif" ) ) ); + QVERIFY( !static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 2 ) )->supportsNonFileBasedOutput() ); + + QCOMPARE( alg.destinationParameterDefinitions().at( 0 )->name(), QStringLiteral( "cx1:my_raster_output" ) ); + QCOMPARE( alg.destinationParameterDefinitions().at( 0 )->description(), QStringLiteral( "my output" ) ); + QCOMPARE( static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 0 ) )->originalProvider()->id(), QStringLiteral( "dummy4" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterRasterDestination * >( alg.destinationParameterDefinitions().at( 0 ) )->supportedOutputRasterLayerExtensions(), QStringList() << QStringLiteral( "mig" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterRasterDestination * >( alg.destinationParameterDefinitions().at( 0 ) )->defaultFileExtension(), QStringLiteral( "mig" ) ); + QVERIFY( static_cast< const QgsProcessingParameterRasterDestination * >( alg.destinationParameterDefinitions().at( 0 ) )->generateTemporaryDestination().endsWith( QStringLiteral( ".mig" ) ) ); + QVERIFY( !static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 0 ) )->supportsNonFileBasedOutput() ); + + QCOMPARE( alg.destinationParameterDefinitions().at( 1 )->name(), QStringLiteral( "cx1:my_sink_output" ) ); + QCOMPARE( alg.destinationParameterDefinitions().at( 1 )->description(), QStringLiteral( "my output" ) ); + QCOMPARE( static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 1 ) )->originalProvider()->id(), QStringLiteral( "dummy4" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterFeatureSink * >( alg.destinationParameterDefinitions().at( 1 ) )->supportedOutputVectorLayerExtensions(), QStringList() << QStringLiteral( "mif" ) ); + QCOMPARE( static_cast< const QgsProcessingParameterFeatureSink * >( alg.destinationParameterDefinitions().at( 1 ) )->defaultFileExtension(), QStringLiteral( "mif" ) ); + QVERIFY( static_cast< const QgsProcessingParameterFeatureSink * >( alg.destinationParameterDefinitions().at( 1 ) )->generateTemporaryDestination().endsWith( QStringLiteral( ".mif" ) ) ); + QVERIFY( !static_cast< const QgsProcessingDestinationParameter * >( alg.destinationParameterDefinitions().at( 1 ) )->supportsNonFileBasedOutput() ); +} + void TestQgsProcessing::modelVectorOutputIsCompatibleType() { // IMPORTANT: This method is intended to be "permissive" rather than "restrictive".