[processing] Fix QgsProcessingProvider::isSupportedOutputValue handling

nyalldawson · nyalldawson · commit 4e8c94fb5c17 · 2019-03-20T12:58:54.000+10:00
of optional output parameters Refs #21374
diff --git a/src/core/processing/qgsprocessingprovider.cpp b/src/core/processing/qgsprocessingprovider.cpp
@@ -109,12 +109,26 @@ QStringList QgsProcessingProvider::supportedOutputTableExtensions() const
 
 bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error ) const
 {
-  QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context );
   error.clear();
+  QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context ).trimmed();
+
+  if ( outputPath.isEmpty() )
+  {
+    if ( parameter->flags() & QgsProcessingParameterDefinition::FlagOptional )
+    {
+      return true;
+    }
+    else
+    {
+      error = tr( "Missing parameter value %1" ).arg( parameter->description() );
+      return false;
+    }
+  }
+
   if ( parameter->type() == QgsProcessingParameterVectorDestination::typeName()
        ||  parameter->type() == QgsProcessingParameterFeatureSink::typeName() )
   {
-    if ( outputPath.isEmpty() || outputPath.startsWith( QLatin1String( "memory:" ) ) )
+    if ( outputPath.startsWith( QLatin1String( "memory:" ) ) )
     {
       if ( !supportsNonFileBasedOutput() )
       {
@@ -145,7 +159,7 @@ bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue,
 
     if ( !supportedOutputVectorLayerExtensions().contains( extension, Qt::CaseInsensitive ) )
     {
-      error = tr( "%1 files are not supported as outputs for this algorithm" ).arg( extension );
+      error = tr( "“.%1” files are not supported as outputs for this algorithm" ).arg( extension );
       return false;
     }
     return true;
@@ -156,7 +170,7 @@ bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue,
     const QString extension = fi.completeSuffix();
     if ( !supportedOutputRasterLayerExtensions().contains( extension, Qt::CaseInsensitive ) )
     {
-      error = tr( "%1 files are not supported as outputs for this algorithm" ).arg( extension );
+      error = tr( "“.%1” files are not supported as outputs for this algorithm" ).arg( extension );
       return false;
     }
     return true;
diff --git a/tests/src/analysis/testqgsprocessing.cpp b/tests/src/analysis/testqgsprocessing.cpp
@@ -5319,6 +5319,8 @@ void TestQgsProcessing::parameterVectorOut()
   def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), QgsProcessing::TypeVectorAnyGeometry, QString(), true ) );
   DummyProvider3 provider;
   QString error;
+  QVERIFY( provider.isSupportedOutputValue( QVariant(), def.get(), context, error ) ); // optional
+  QVERIFY( provider.isSupportedOutputValue( QString(), def.get(), context, error ) ); // optional
   QVERIFY( !provider.isSupportedOutputValue( "d:/test.shp", def.get(), context, error ) );
   QVERIFY( !provider.isSupportedOutputValue( "d:/test.SHP", def.get(), context, error ) );
   QVERIFY( !provider.isSupportedOutputValue( "ogr:d:/test.shp", def.get(), context, error ) );
@@ -5327,6 +5329,9 @@ void TestQgsProcessing::parameterVectorOut()
   QVERIFY( provider.isSupportedOutputValue( "d:/test.MIF", def.get(), context, error ) );
   QVERIFY( provider.isSupportedOutputValue( "ogr:d:/test.MIF", def.get(), context, error ) );
   QVERIFY( provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.MIF" ), def.get(), context, error ) );
+  def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), QgsProcessing::TypeVectorAnyGeometry, QString(), false ) );
+  QVERIFY( !provider.isSupportedOutputValue( QVariant(), def.get(), context, error ) ); // non-optional
+  QVERIFY( !provider.isSupportedOutputValue( QString(), def.get(), context, error ) ); // non-optional
 
   provider.loadAlgorithms();
   def->mOriginalProvider = &provider;
@@ -5442,6 +5447,8 @@ void TestQgsProcessing::parameterRasterOut()
 
   DummyProvider3 provider;
   QString error;
+  QVERIFY( !provider.isSupportedOutputValue( QVariant(), def.get(), context, error ) );
+  QVERIFY( !provider.isSupportedOutputValue( QString(), def.get(), context, error ) );
   QVERIFY( !provider.isSupportedOutputValue( "d:/test.tif", def.get(), context, error ) );
   QVERIFY( !provider.isSupportedOutputValue( "d:/test.TIF", def.get(), context, error ) );
   QVERIFY( !provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.tif" ), def.get(), context, error ) );

Original file line number	Diff line number	Diff line change
`@@ -109,12 +109,26 @@ QStringList QgsProcessingProvider::supportedOutputTableExtensions() const`
`109`	`109`
`110`	`110`	`bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error ) const`
`111`	`111`	`{`
`112`		`- QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context );`
`113`	`112`	`error.clear();`
	`113`	`+ QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context ).trimmed();`
	`114`	`+`
	`115`	`+ if ( outputPath.isEmpty() )`
	`116`	`+ {`
	`117`	`+ if ( parameter->flags() & QgsProcessingParameterDefinition::FlagOptional )`
	`118`	`+ {`
	`119`	`+ return true;`
	`120`	`+ }`
	`121`	`+ else`
	`122`	`+ {`
	`123`	`+ error = tr( "Missing parameter value %1" ).arg( parameter->description() );`
	`124`	`+ return false;`
	`125`	`+ }`
	`126`	`+ }`
	`127`	`+`
`114`	`128`	`if ( parameter->type() == QgsProcessingParameterVectorDestination::typeName()`
`115`	`129`	`\|\| parameter->type() == QgsProcessingParameterFeatureSink::typeName() )`
`116`	`130`	`{`
`117`		`- if ( outputPath.isEmpty() \|\| outputPath.startsWith( QLatin1String( "memory:" ) ) )`
	`131`	`+ if ( outputPath.startsWith( QLatin1String( "memory:" ) ) )`
`118`	`132`	`{`
`119`	`133`	`if ( !supportsNonFileBasedOutput() )`
`120`	`134`	`{`
`@@ -145,7 +159,7 @@ bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue,`
`145`	`159`
`146`	`160`	`if ( !supportedOutputVectorLayerExtensions().contains( extension, Qt::CaseInsensitive ) )`
`147`	`161`	`{`
`148`		`- error = tr( "%1 files are not supported as outputs for this algorithm" ).arg( extension );`
	`162`	`+ error = tr( "“.%1” files are not supported as outputs for this algorithm" ).arg( extension );`
`149`	`163`	`return false;`
`150`	`164`	`}`
`151`	`165`	`return true;`
`@@ -156,7 +170,7 @@ bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue,`
`156`	`170`	`const QString extension = fi.completeSuffix();`
`157`	`171`	`if ( !supportedOutputRasterLayerExtensions().contains( extension, Qt::CaseInsensitive ) )`
`158`	`172`	`{`
`159`		`- error = tr( "%1 files are not supported as outputs for this algorithm" ).arg( extension );`
	`173`	`+ error = tr( "“.%1” files are not supported as outputs for this algorithm" ).arg( extension );`
`160`	`174`	`return false;`
`161`	`175`	`}`
`162`	`176`	`return true;`