Skip to content

Commit e6379fb

Browse files
nyalldawsonnyalldawson
committedFeb 7, 2022
[processing] When a model algorithm parameter value is set to be taken
from an expression, abort the model execution if the expression evaluation fails instead of treating the value as null
·
ltr-3_40final-3_24_0
1 parent c74c934 commit e6379fb

File tree

3 files changed

+38
-10
lines changed

3 files changed

+38
-10
lines changed
 

‎src/core/processing/models/qgsprocessingmodelalgorithm.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,10 @@ QgsProcessingAlgorithm::Flags QgsProcessingModelAlgorithm::flags() const
9898
return QgsProcessingAlgorithm::flags() | QgsProcessingAlgorithm::FlagNoThreading;
9999
}
100100

101-
QVariantMap QgsProcessingModelAlgorithm::parametersForChildAlgorithm( const QgsProcessingModelChildAlgorithm &child, const QVariantMap &modelParameters, const QVariantMap &results, const QgsExpressionContext &expressionContext ) const
101+
QVariantMap QgsProcessingModelAlgorithm::parametersForChildAlgorithm( const QgsProcessingModelChildAlgorithm &child, const QVariantMap &modelParameters, const QVariantMap &results, const QgsExpressionContext &expressionContext, QString &error ) const
102102
{
103-
auto evaluateSources = [ = ]( const QgsProcessingParameterDefinition * def )->QVariant
103+
error.clear();
104+
auto evaluateSources = [ =, &error ]( const QgsProcessingParameterDefinition * def )->QVariant
104105
{
105106
const QgsProcessingModelChildParameterSources paramSources = child.parameterSources().value( def->name() );
106107

@@ -129,6 +130,10 @@ QVariantMap QgsProcessingModelAlgorithm::parametersForChildAlgorithm( const QgsP
129130
{
130131
QgsExpression exp( source.expression() );
131132
paramParts << exp.evaluate( &expressionContext );
133+
if ( exp.hasEvalError() )
134+
{
135+
error = QObject::tr( "Could not evaluate expression for parameter %1 for %2: %3" ).arg( def->name(), child.description(), exp.evalErrorString() );
136+
}
132137
break;
133138
}
134139
case QgsProcessingModelChildParameterSource::ExpressionText:
@@ -329,7 +334,11 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
329334
<< createExpressionContextScopeForChildAlgorithm( childId, context, parameters, childResults );
330335
context.setExpressionContext( expContext );
331336

332-
QVariantMap childParams = parametersForChildAlgorithm( child, parameters, childResults, expContext );
337+
QString error;
338+
QVariantMap childParams = parametersForChildAlgorithm( child, parameters, childResults, expContext, error );
339+
if ( !error.isEmpty() )
340+
throw QgsProcessingException( error );
341+
333342
if ( feedback && !skipGenericLogging )
334343
feedback->setProgressText( QObject::tr( "Running %1 [%2/%3]" ).arg( child.description() ).arg( executed.count() + 1 ).arg( toExecute.count() ) );
335344

‎src/core/processing/models/qgsprocessingmodelalgorithm.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ class CORE_EXPORT QgsProcessingModelAlgorithm : public QgsProcessingAlgorithm
549549
void dependsOnChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends ) const;
550550
void dependentChildAlgorithmsRecursive( const QString &childId, QSet<QString> &depends, const QString &branch ) const;
551551

552-
QVariantMap parametersForChildAlgorithm( const QgsProcessingModelChildAlgorithm &child, const QVariantMap &modelParameters, const QVariantMap &results, const QgsExpressionContext &expressionContext ) const;
552+
QVariantMap parametersForChildAlgorithm( const QgsProcessingModelChildAlgorithm &child, const QVariantMap &modelParameters, const QVariantMap &results, const QgsExpressionContext &expressionContext, QString &error ) const;
553553

554554
/**
555555
* Returns TRUE if an output from a child algorithm is required elsewhere in

‎tests/src/analysis/testqgsprocessingmodelalgorithm.cpp

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1163,7 +1163,9 @@ void TestQgsProcessingModelAlgorithm::modelExecution()
11631163
layerDef.destinationName = "my_dest";
11641164
modelInputs.insert( "cx3:MY_OUT", QVariant::fromValue( layerDef ) );
11651165
QVariantMap childResults;
1166-
QVariantMap params = model2.parametersForChildAlgorithm( model2.childAlgorithm( QStringLiteral( "cx1" ) ), modelInputs, childResults, expContext );
1166+
QString error;
1167+
QVariantMap params = model2.parametersForChildAlgorithm( model2.childAlgorithm( QStringLiteral( "cx1" ) ), modelInputs, childResults, expContext, error );
1168+
QVERIFY( error.isEmpty() );
11671169
QCOMPARE( params.value( "DISSOLVE" ).toBool(), false );
11681170
QCOMPARE( params.value( "DISTANCE" ).toInt(), 271 );
11691171
QCOMPARE( params.value( "SEGMENTS" ).toInt(), 16 );
@@ -1218,7 +1220,8 @@ void TestQgsProcessingModelAlgorithm::modelExecution()
12181220
alg2c2.setAlgorithmId( "native:centroids" );
12191221
alg2c2.addParameterSources( "INPUT", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromChildOutput( QStringLiteral( "cx1" ), "OUTPUT" ) );
12201222
model2.addChildAlgorithm( alg2c2 );
1221-
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx2" ), modelInputs, childResults, expContext );
1223+
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx2" ), modelInputs, childResults, expContext, error );
1224+
QVERIFY( error.isEmpty() );
12221225
QCOMPARE( params.value( "INPUT" ).toString(), QStringLiteral( "dest.shp" ) );
12231226
QCOMPARE( params.value( "OUTPUT" ).toString(), QStringLiteral( "memory:Centroids" ) );
12241227
QCOMPARE( params.count(), 2 );
@@ -1269,7 +1272,8 @@ void TestQgsProcessingModelAlgorithm::modelExecution()
12691272
alg2c3.setModelOutputs( outputs3 );
12701273

12711274
model2.addChildAlgorithm( alg2c3 );
1272-
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx3" ), modelInputs, childResults, expContext );
1275+
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx3" ), modelInputs, childResults, expContext, error );
1276+
QVERIFY( error.isEmpty() );
12731277
QCOMPARE( params.value( "INPUT" ).toString(), QStringLiteral( "dest.shp" ) );
12741278
QCOMPARE( params.value( "EXPRESSION" ).toString(), QStringLiteral( "true" ) );
12751279
QVERIFY( params.value( "OUTPUT" ).canConvert<QgsProcessingOutputLayerDefinition>() );
@@ -1278,23 +1282,38 @@ void TestQgsProcessingModelAlgorithm::modelExecution()
12781282
QCOMPARE( outDef.sink.staticValue().toString(), QStringLiteral( "memory:" ) );
12791283
QCOMPARE( params.count(), 3 ); // don't want FAIL_OUTPUT set!
12801284

1285+
// a child with an expression which is invalid
1286+
QgsProcessingModelChildAlgorithm alg2c3a;
1287+
alg2c3a.setChildId( "cx3a" );
1288+
alg2c3a.setAlgorithmId( "native:extractbyexpression" );
1289+
alg2c3a.setDescription( "Extract by expression" );
1290+
alg2c3a.addParameterSources( "INPUT", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromExpression( QStringLiteral( "1/'a'" ) ) );
1291+
alg2c3a.addParameterSources( "EXPRESSION", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromStaticValue( "true" ) );
1292+
alg2c3a.addParameterSources( "OUTPUT", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromModelParameter( "MY_OUT" ) );
1293+
1294+
model2.addChildAlgorithm( alg2c3a );
1295+
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx3a" ), modelInputs, childResults, expContext, error );
1296+
QCOMPARE( error, QStringLiteral( "Could not evaluate expression for parameter INPUT for Extract by expression: Cannot convert 'a' to double" ) );
1297+
model2.removeChildAlgorithm( "cx3a" );
1298+
12811299
// a child with an static output value
12821300
QgsProcessingModelChildAlgorithm alg2c4;
12831301
alg2c4.setChildId( "cx4" );
12841302
alg2c4.setAlgorithmId( "native:extractbyexpression" );
12851303
alg2c4.addParameterSources( "OUTPUT", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromStaticValue( "STATIC" ) );
12861304
model2.addChildAlgorithm( alg2c4 );
1287-
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx4" ), modelInputs, childResults, expContext );
1305+
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx4" ), modelInputs, childResults, expContext, error );
1306+
QVERIFY( error.isEmpty() );
12881307
QCOMPARE( params.value( "OUTPUT" ).toString(), QStringLiteral( "STATIC" ) );
12891308
model2.removeChildAlgorithm( "cx4" );
12901309
// expression based output value
12911310
alg2c4.addParameterSources( "OUTPUT", QgsProcessingModelChildParameterSources() << QgsProcessingModelChildParameterSource::fromExpression( "'A' || 'B'" ) );
12921311
model2.addChildAlgorithm( alg2c4 );
1293-
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx4" ), modelInputs, childResults, expContext );
1312+
params = model2.parametersForChildAlgorithm( model2.childAlgorithm( "cx4" ), modelInputs, childResults, expContext, error );
1313+
QVERIFY( error.isEmpty() );
12941314
QCOMPARE( params.value( "OUTPUT" ).toString(), QStringLiteral( "AB" ) );
12951315
model2.removeChildAlgorithm( "cx4" );
12961316

1297-
12981317
variables = model2.variablesForChildAlgorithm( "cx3", context );
12991318
QCOMPARE( variables.count(), 17 );
13001319
QCOMPARE( variables.value( "DIST" ).source.source(), QgsProcessingModelChildParameterSource::ModelParameter );

0 commit comments

Comments
 (0)
Please sign in to comment.