Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix incorrect attempts to load outputs from models to projects #57701

Merged
merged 3 commits into from
Jun 7, 2024
Merged

Fix incorrect attempts to load outputs from models to projects #57701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ed5a41f
Fix all outputs from models are set to load into project
nyalldawson Jun 6, 2024
e6baabe
Add testOnly option to parameterAsOutputLayer
nyalldawson Jun 7, 2024
dcf88dd
Ensure calling isSupportedOutputValue doesn't add layer to project
nyalldawson Jun 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion python/PyQt6/core/auto_generated/processing/qgsprocessingparameters.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,10 +1189,13 @@ need to handle deletion of the returned layer.
Evaluates the parameter with matching ``definition`` to a output layer destination.
%End

static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context );
static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context, bool testOnly = false );
%Docstring
Evaluates the parameter with matching ``definition`` and ``value`` to a output layer destination.

Since QGIS 3.38 the ``testOnly`` argument can be set to ``True`` to evaluate the parameter to an output layer destination for advance testing only. This
prevents default behavior such as output post-processing which would otherwise occur.

.. versionadded:: 3.4
%End

Expand Down
5 changes: 4 additions & 1 deletion python/core/auto_generated/processing/qgsprocessingparameters.sip.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,10 +1189,13 @@ need to handle deletion of the returned layer.
Evaluates the parameter with matching ``definition`` to a output layer destination.
%End

static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context );
static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context, bool testOnly = false );
%Docstring
Evaluates the parameter with matching ``definition`` and ``value`` to a output layer destination.

Since QGIS 3.38 the ``testOnly`` argument can be set to ``True`` to evaluate the parameter to an output layer destination for advance testing only. This
prevents default behavior such as output post-processing which would otherwise occur.

.. versionadded:: 3.4
%End

Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/gdal/Datasources2Vrt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def supportedOutputRasterLayerExtensions(self):
return ['vrt']

def isSupportedOutputValue(self, value, context):
output_path = QgsProcessingParameters.parameterAsOutputLayer(self, value, context)
output_path = QgsProcessingParameters.parameterAsOutputLayer(self, value, context, testOnly=True)
if pathlib.Path(output_path).suffix.lower() != '.vrt':
return False, QCoreApplication.translate("GdalAlgorithm", 'Output filename must use a .vrt extension')
return True, ''
Expand Down
2 changes: 1 addition & 1 deletion python/plugins/processing/algs/gdal/buildvrt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def parameterAsOutputLayer(self, definition, value, context):
return super(QgsProcessingParameterRasterDestination, self).parameterAsOutputLayer(definition, value, context)

def isSupportedOutputValue(self, value, context):
output_path = QgsProcessingParameters.parameterAsOutputLayer(self, value, context)
output_path = QgsProcessingParameters.parameterAsOutputLayer(self, value, context, testOnly=True)
if pathlib.Path(output_path).suffix.lower() != '.vrt':
return False, QCoreApplication.translate("GdalAlgorithm", 'Output filename must use a .vrt extension')
return True, ''
Expand Down
5 changes: 3 additions & 2 deletions src/core/processing/models/qgsprocessingmodelalgorithm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,10 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
break;
}

if ( !results.value( outputIt->childOutputName() ).toString().isEmpty() )
const QString outputLayer = results.value( outputIt->childOutputName() ).toString();
if ( !outputLayer.isEmpty() && context.willLoadLayerOnCompletion( outputLayer ) )
{
QgsProcessingContext::LayerDetails &details = context.layerToLoadOnCompletionDetails( results.value( outputIt->childOutputName() ).toString() );
QgsProcessingContext::LayerDetails &details = context.layerToLoadOnCompletionDetails( outputLayer );
details.groupName = mOutputGroup;
if ( outputSortKey > 0 )
details.layerSortKey = outputSortKey;
Expand Down
5 changes: 3 additions & 2 deletions src/core/processing/qgsprocessingparameters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ QString QgsProcessingParameters::parameterAsOutputLayer( const QgsProcessingPara
return parameterAsOutputLayer( definition, val, context );
}

QString QgsProcessingParameters::parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context )
QString QgsProcessingParameters::parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context, bool testOnly )
{
QVariant val = value;

Expand Down Expand Up @@ -992,7 +992,8 @@ QString QgsProcessingParameters::parameterAsOutputLayer( const QgsProcessingPara
else if ( definition && definition->type() == QgsProcessingParameterVectorTileDestination::typeName() )
layerTypeHint = QgsProcessingUtils::LayerHint::VectorTile;

context.addLayerToLoadOnCompletion( dest, QgsProcessingContext::LayerDetails( destName, destinationProject, outputName, layerTypeHint ) );
if ( !testOnly )
context.addLayerToLoadOnCompletion( dest, QgsProcessingContext::LayerDetails( destName, destinationProject, outputName, layerTypeHint ) );
}

return dest;
Expand Down
6 changes: 5 additions & 1 deletion src/core/processing/qgsprocessingparameters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1303,9 +1303,13 @@ class CORE_EXPORT QgsProcessingParameters

/**
* Evaluates the parameter with matching \a definition and \a value to a output layer destination.
*
* Since QGIS 3.38 the \a testOnly argument can be set to TRUE to evaluate the parameter to an output layer destination for advance testing only. This
* prevents default behavior such as output post-processing which would otherwise occur.
*
* \since QGIS 3.4
*/
static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context );
static QString parameterAsOutputLayer( const QgsProcessingParameterDefinition *definition, const QVariant &value, QgsProcessingContext &context, bool testOnly = false );

/**
* Evaluates the parameter with matching \a definition to a file based output destination.
Expand Down
2 changes: 1 addition & 1 deletion src/core/processing/qgsprocessingprovider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ QStringList QgsProcessingProvider::supportedOutputTableExtensions() const
bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error ) const
{
error.clear();
QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context ).trimmed();
QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context, true ).trimmed();

if ( outputPath.isEmpty() )
{
Expand Down
11 changes: 9 additions & 2 deletions tests/src/analysis/testqgsprocessing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8090,6 +8090,11 @@ void TestQgsProcessing::parameterVectorOut()
QCOMPARE( context.layersToLoadOnCompletion().values().at( 0 ).name, QStringLiteral( "desc" ) );
QCOMPARE( context.layersToLoadOnCompletion().values().at( 0 ).layerTypeHint, QgsProcessingUtils::LayerHint::Vector );

// if we set testOnly = true, then layer should not be loaded on completion
context.setLayersToLoadOnCompletion( {} );
QCOMPARE( QgsProcessingParameters::parameterAsOutputLayer( def.get(), QVariant::fromValue( fs ), context, true ), QStringLiteral( "test.shp" ) );
QCOMPARE( context.layersToLoadOnCompletion().size(), 0 );

// with name overloading
QgsProcessingContext context2;
fs = QgsProcessingOutputLayerDefinition( QStringLiteral( "test.shp" ) );
Expand Down Expand Up @@ -8117,19 +8122,21 @@ void TestQgsProcessing::parameterVectorOut()
def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), Qgis::ProcessingSourceType::VectorAnyGeometry, QString(), true ) );
DummyProvider3 provider;
QString error;
context.setLayersToLoadOnCompletion( {} );
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 ) );
QVERIFY( !provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.SHP" ), def.get(), context, error ) );
QVERIFY( !provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.SHP", &p ), def.get(), context, error ) );
QVERIFY( provider.isSupportedOutputValue( "d:/test.mif", def.get(), context, error ) );
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 ) );
QVERIFY( provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.MIF", &p ), def.get(), context, error ) );
def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), Qgis::ProcessingSourceType::VectorAnyGeometry, QString(), false ) );
QVERIFY( !provider.isSupportedOutputValue( QVariant(), def.get(), context, error ) ); // non-optional
QVERIFY( !provider.isSupportedOutputValue( QString(), def.get(), context, error ) ); // non-optional
QVERIFY( context.layersToLoadOnCompletion().isEmpty() );

provider.loadAlgorithms();
def->mOriginalProvider = &provider;
Expand Down
32 changes: 29 additions & 3 deletions tests/src/analysis/testqgsprocessingmodelalgorithm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2295,9 +2295,9 @@ void TestQgsProcessingModelAlgorithm::modelOutputs()
p.addMapLayer( layer3111 );
context.setProject( &p );
params.insert( QStringLiteral( "INPUT" ), QStringLiteral( "v1" ) );
params.insert( QStringLiteral( "cx2:a" ), QgsProcessing::TEMPORARY_OUTPUT );
params.insert( QStringLiteral( "cx3:b" ), QgsProcessing::TEMPORARY_OUTPUT );
params.insert( QStringLiteral( "cx4:c" ), QgsProcessing::TEMPORARY_OUTPUT );
params.insert( QStringLiteral( "cx2:a" ), QgsProcessingOutputLayerDefinition( QgsProcessing::TEMPORARY_OUTPUT, &p ) );
params.insert( QStringLiteral( "cx3:b" ), QgsProcessingOutputLayerDefinition( QgsProcessing::TEMPORARY_OUTPUT, &p ) );
params.insert( QStringLiteral( "cx4:c" ), QgsProcessingOutputLayerDefinition( QgsProcessing::TEMPORARY_OUTPUT, &p ) );

QVariantMap results = m.run( params, context, &feedback );
const QString destA = results.value( QStringLiteral( "cx2:a" ) ).toString();
Expand All @@ -2314,6 +2314,32 @@ void TestQgsProcessingModelAlgorithm::modelOutputs()
QVERIFY( !destC.isEmpty() );
QCOMPARE( context.layerToLoadOnCompletionDetails( destC ).groupName, QStringLiteral( "output group" ) );
QCOMPARE( context.layerToLoadOnCompletionDetails( destC ).layerSortKey, 1 );

// not all layers are set to load in project
QgsProcessingContext context2;
context2.setProject( &p );
params.clear();
params.insert( QStringLiteral( "INPUT" ), QStringLiteral( "v1" ) );
// should not be loaded on completion:
params.insert( QStringLiteral( "cx2:a" ), QgsProcessing::TEMPORARY_OUTPUT );
// should be loaded on completion:
params.insert( QStringLiteral( "cx3:b" ), QgsProcessingOutputLayerDefinition( QgsProcessing::TEMPORARY_OUTPUT, &p ) );
params.insert( QStringLiteral( "cx4:c" ), QgsProcessingOutputLayerDefinition( QgsProcessing::TEMPORARY_OUTPUT, &p ) );

QVariantMap results2 = m.run( params, context2, &feedback );
const QString destA2 = results2.value( QStringLiteral( "cx2:a" ) ).toString();
QVERIFY( !destA2.isEmpty() );
QVERIFY( !context2.willLoadLayerOnCompletion( destA2 ) );

const QString destB2 = results2.value( QStringLiteral( "cx3:b" ) ).toString();
QVERIFY( !destB2.isEmpty() );
QCOMPARE( context2.layerToLoadOnCompletionDetails( destB2 ).groupName, QStringLiteral( "output group" ) );
QCOMPARE( context2.layerToLoadOnCompletionDetails( destB2 ).layerSortKey, 0 );

const QString destC2 = results2.value( QStringLiteral( "cx4:c" ) ).toString();
QVERIFY( !destC2.isEmpty() );
QCOMPARE( context2.layerToLoadOnCompletionDetails( destC2 ).groupName, QStringLiteral( "output group" ) );
QCOMPARE( context2.layerToLoadOnCompletionDetails( destC2 ).layerSortKey, 1 );
}


Expand Down
Loading