From 72be86dc61d1126e1e106b9f7d4f0ca0fc4569ed Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Tue, 6 Jun 2017 08:25:03 +1000 Subject: [PATCH] Only accept QgsPropertys in QgsProcessingFeatureSourceDefinition/ QgsProcessingFeatureSink, not all QVariant types Only strings/QgsPropertys are valid anyway, so instead of strings use static properties. This makes it clearer what possible values are permitted for the underlying source/sink definition. --- .../processing/qgsprocessingparameters.sip | 26 ++++--- src/core/processing/qgsprocessingparameters.h | 32 ++++++--- tests/src/core/testqgsprocessing.cpp | 70 ++++++++++++++++--- 3 files changed, 104 insertions(+), 24 deletions(-) diff --git a/python/core/processing/qgsprocessingparameters.sip b/python/core/processing/qgsprocessingparameters.sip index 22d7b0300c1c..ffd7c5cc58d2 100644 --- a/python/core/processing/qgsprocessingparameters.sip +++ b/python/core/processing/qgsprocessingparameters.sip @@ -26,14 +26,19 @@ class QgsProcessingFeatureSourceDefinition %End public: - QgsProcessingFeatureSourceDefinition( const QVariant &source = QVariant(), bool selectedFeaturesOnly = false ); + QgsProcessingFeatureSourceDefinition( const QString &source = QString(), bool selectedFeaturesOnly = false ); %Docstring - Constructor for QgsProcessingFeatureSourceDefinition. + Constructor for QgsProcessingFeatureSourceDefinition, accepting a static string source. %End - QVariant source; + QgsProcessingFeatureSourceDefinition( const QgsProperty &source, bool selectedFeaturesOnly = false ); %Docstring - Source definition. Usually set to a source layer's ID or file name. + Constructor for QgsProcessingFeatureSourceDefinition, accepting a QgsProperty source. +%End + + QgsProperty source; +%Docstring + Source definition. Usually a static property set to a source layer's ID or file name. %End bool selectedFeaturesOnly; @@ -65,14 +70,19 @@ class QgsProcessingFeatureSink %End public: - QgsProcessingFeatureSink( const QVariant &sink = QVariant(), bool loadIntoProject = false ); + QgsProcessingFeatureSink( const QString &sink = QString(), bool loadIntoProject = false ); +%Docstring + Constructor for QgsProcessingFeatureSink, accepting a static string sink. +%End + + QgsProcessingFeatureSink( const QgsProperty &sink, bool loadIntoProject = false ); %Docstring - Constructor for QgsProcessingFeatureSink. + Constructor for QgsProcessingFeatureSink, accepting a QgsProperty sink. %End - QVariant sink; + QgsProperty sink; %Docstring - Sink definition. Usually set to the destination file name for the sink's layer. + Sink definition. Usually a static property set to the destination file name for the sink's layer. %End bool loadIntoProject; diff --git a/src/core/processing/qgsprocessingparameters.h b/src/core/processing/qgsprocessingparameters.h index b7b3f3482fbb..9db4420c38de 100644 --- a/src/core/processing/qgsprocessingparameters.h +++ b/src/core/processing/qgsprocessingparameters.h @@ -46,17 +46,25 @@ class CORE_EXPORT QgsProcessingFeatureSourceDefinition public: /** - * Constructor for QgsProcessingFeatureSourceDefinition. + * Constructor for QgsProcessingFeatureSourceDefinition, accepting a static string source. */ - QgsProcessingFeatureSourceDefinition( const QVariant &source = QVariant(), bool selectedFeaturesOnly = false ) + QgsProcessingFeatureSourceDefinition( const QString &source = QString(), bool selectedFeaturesOnly = false ) + : source( QgsProperty::fromValue( source ) ) + , selectedFeaturesOnly( selectedFeaturesOnly ) + {} + + /** + * Constructor for QgsProcessingFeatureSourceDefinition, accepting a QgsProperty source. + */ + QgsProcessingFeatureSourceDefinition( const QgsProperty &source, bool selectedFeaturesOnly = false ) : source( source ) , selectedFeaturesOnly( selectedFeaturesOnly ) {} /** - * Source definition. Usually set to a source layer's ID or file name. + * Source definition. Usually a static property set to a source layer's ID or file name. */ - QVariant source; + QgsProperty source; /** * True if only selected features in the source should be used by algorithms. @@ -88,17 +96,25 @@ class CORE_EXPORT QgsProcessingFeatureSink public: /** - * Constructor for QgsProcessingFeatureSink. + * Constructor for QgsProcessingFeatureSink, accepting a static string sink. + */ + QgsProcessingFeatureSink( const QString &sink = QString(), bool loadIntoProject = false ) + : sink( QgsProperty::fromValue( sink ) ) + , loadIntoProject( loadIntoProject ) + {} + + /** + * Constructor for QgsProcessingFeatureSink, accepting a QgsProperty sink. */ - QgsProcessingFeatureSink( const QVariant &sink = QVariant(), bool loadIntoProject = false ) + QgsProcessingFeatureSink( const QgsProperty &sink, bool loadIntoProject = false ) : sink( sink ) , loadIntoProject( loadIntoProject ) {} /** - * Sink definition. Usually set to the destination file name for the sink's layer. + * Sink definition. Usually a static property set to the destination file name for the sink's layer. */ - QVariant sink; + QgsProperty sink; /** * True if sink should be loaded into the current project when the algorithm completes. diff --git a/tests/src/core/testqgsprocessing.cpp b/tests/src/core/testqgsprocessing.cpp index 7bb8d900634a..336bfc2efdd1 100644 --- a/tests/src/core/testqgsprocessing.cpp +++ b/tests/src/core/testqgsprocessing.cpp @@ -2329,9 +2329,9 @@ void TestQgsProcessing::combineLayerExtent() void TestQgsProcessing::processingFeatureSource() { - QVariant source( QStringLiteral( "test.shp" ) ); - QgsProcessingFeatureSourceDefinition fs( source, true ); - QCOMPARE( fs.source, source ); + QString sourceString = QStringLiteral( "test.shp" ); + QgsProcessingFeatureSourceDefinition fs( sourceString, true ); + QCOMPARE( fs.source.staticValue().toString(), sourceString ); QVERIFY( fs.selectedFeaturesOnly ); // test storing QgsProcessingFeatureSource in variant and retrieving @@ -2339,15 +2339,45 @@ void TestQgsProcessing::processingFeatureSource() QVERIFY( fsInVariant.isValid() ); QgsProcessingFeatureSourceDefinition fromVar = qvariant_cast( fsInVariant ); - QCOMPARE( fromVar.source, source ); + QCOMPARE( fromVar.source.staticValue().toString(), sourceString ); QVERIFY( fromVar.selectedFeaturesOnly ); + + // test evaluating parameter as source + QgsVectorLayer *layer = new QgsVectorLayer( "Point", "v1", "memory" ); + QgsFeature f( 10001 ); + f.setGeometry( QgsGeometry( new QgsPoint( 1, 2 ) ) ); + layer->dataProvider()->addFeatures( QgsFeatureList() << f ); + + QgsProject p; + p.addMapLayer( layer ); + QgsProcessingContext context; + context.setProject( &p ); + + // first using static string definition + QgsProcessingParameterDefinition *def = new QgsProcessingParameterString( QStringLiteral( "layer" ) ); + QVariantMap params; + params.insert( QStringLiteral( "layer" ), QgsProcessingFeatureSourceDefinition( layer->id(), false ) ); + std::unique_ptr< QgsFeatureSource > source( QgsProcessingParameters::parameterAsSource( def, params, context ) ); + // can't directly match it to layer, so instead just get the feature and test that it matches what we expect + QgsFeature f2; + QVERIFY( source.get() ); + QVERIFY( source->getFeatures().nextFeature( f2 ) ); + QCOMPARE( f2.geometry(), f.geometry() ); + + // next using property based definition + params.insert( QStringLiteral( "layer" ), QgsProcessingFeatureSourceDefinition( QgsProperty::fromExpression( QStringLiteral( "trim('%1' + ' ')" ).arg( layer->id() ) ), false ) ); + source.reset( QgsProcessingParameters::parameterAsSource( def, params, context ) ); + // can't directly match it to layer, so instead just get the feature and test that it matches what we expect + QVERIFY( source.get() ); + QVERIFY( source->getFeatures().nextFeature( f2 ) ); + QCOMPARE( f2.geometry(), f.geometry() ); } void TestQgsProcessing::processingFeatureSink() { - QVariant sink( QStringLiteral( "test.shp" ) ); - QgsProcessingFeatureSink fs( sink, true ); - QCOMPARE( fs.sink, sink ); + QString sinkString( QStringLiteral( "test.shp" ) ); + QgsProcessingFeatureSink fs( sinkString, true ); + QCOMPARE( fs.sink.staticValue().toString(), sinkString ); QVERIFY( fs.loadIntoProject ); // test storing QgsProcessingFeatureSink in variant and retrieving @@ -2355,8 +2385,32 @@ void TestQgsProcessing::processingFeatureSink() QVERIFY( fsInVariant.isValid() ); QgsProcessingFeatureSink fromVar = qvariant_cast( fsInVariant ); - QCOMPARE( fromVar.sink, sink ); + QCOMPARE( fromVar.sink.staticValue().toString(), sinkString ); QVERIFY( fromVar.loadIntoProject ); + + // test evaluating parameter as sink + QgsProject p; + QgsProcessingContext context; + context.setProject( &p ); + + // first using static string definition + QgsProcessingParameterDefinition *def = new QgsProcessingParameterString( QStringLiteral( "layer" ) ); + QVariantMap params; + params.insert( QStringLiteral( "layer" ), QgsProcessingFeatureSink( "memory:test", false ) ); + QString dest; + std::unique_ptr< QgsFeatureSink > sink( QgsProcessingParameters::parameterAsSink( def, params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3111" ), context, dest ) ); + QVERIFY( sink.get() ); + QgsVectorLayer *layer = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) ); + QVERIFY( layer ); + QCOMPARE( layer->crs().authid(), QStringLiteral( "EPSG:3111" ) ); + + // next using property based definition + params.insert( QStringLiteral( "layer" ), QgsProcessingFeatureSink( QgsProperty::fromExpression( QStringLiteral( "trim('memory' + ':test2')" ) ), false ) ); + sink.reset( QgsProcessingParameters::parameterAsSink( def, params, QgsFields(), QgsWkbTypes::Point, QgsCoordinateReferenceSystem( "EPSG:3113" ), context, dest ) ); + QVERIFY( sink.get() ); + QgsVectorLayer *layer2 = qobject_cast< QgsVectorLayer *>( QgsProcessingUtils::mapLayerFromString( dest, context, false ) ); + QVERIFY( layer2 ); + QCOMPARE( layer2->crs().authid(), QStringLiteral( "EPSG:3113" ) ); } QGSTEST_MAIN( TestQgsProcessing )