Skip to content

Commit

Permalink
[processing] do not allow using unsupported file formats
Browse files Browse the repository at this point in the history 
Show warning message if user selects incompatible output file format

fixes #21089

(cherry picked from commit 13bff96)
  • Loading branch information
@volaya @nyalldawson
volaya authored and nyalldawson committed Feb 25, 2019
1 parent 201fdce commit 96b25ab
Show file tree
Hide file tree
Showing 9 changed files with 152 additions and 12 deletions.
9 changes: 9 additions & 0 deletions python/core/auto_generated/processing/qgsprocessingprovider.sip.in
View file
Expand Up @@ -138,6 +138,15 @@ formats for geometry-less layers can override this method to return a different
.. seealso:: :py:func:`supportsNonFileBasedOutput`

.. versionadded:: 3.4.3
%End

virtual bool isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error /Out/ ) const;
%Docstring
Returns true if the specified ``outputValue`` is of a supported file format for the given destination ``parameter``.

If the output value is not supported, ``error`` will be set to a descriptive message explaining why.

.. versionadded:: 3.6
%End

virtual QString defaultVectorFileExtension( bool hasGeometry = true ) const;
Expand Down
2 changes: 2 additions & 0 deletions python/plugins/processing/algs/gdal/GdalAlgorithmDialog.py
View file
Expand Up @@ -142,3 +142,5 @@ def parametersHaveChanged(self):
self.text.setPlainText(" ".join(commands))
except AlgorithmDialogBase.InvalidParameterValue as e:
self.text.setPlainText(self.tr("Invalid value for parameter '{0}'").format(e.parameter.description()))
except AlgorithmDialogBase.InvalidOutputExtension as e:
self.text.setPlainText(e.message)
23 changes: 22 additions & 1 deletion python/plugins/processing/gui/AlgorithmDialog.py
View file
Expand Up @@ -25,6 +25,7 @@

__revision__ = '$Format:%H$'

import os
from pprint import pformat
import time

Expand All @@ -44,6 +45,7 @@
QgsProcessingParameterFeatureSink,
QgsProcessingParameterRasterDestination,
QgsProcessingAlgorithm,
QgsProcessingParameters,
QgsProxyProgressTask,
QgsTaskManager)
from qgis.gui import (QgsGui,
Expand Down Expand Up @@ -154,11 +156,18 @@ def getParameterValues(self):
if self.mainWidget().checkBoxes[param.name()].isChecked():
dest_project = QgsProject.instance()

value = self.mainWidget().outputWidgets[param.name()].getValue()
widget = self.mainWidget().outputWidgets[param.name()]
value = widget.getValue()

if value and isinstance(value, QgsProcessingOutputLayerDefinition):
value.destinationProject = dest_project
if value:
parameters[param.name()] = value
if param.isDestination():
context = dataobjects.createContext()
ok, error = self.algorithm().provider().isSupportedOutputValue(value, param, context)
if not ok:
raise AlgorithmDialogBase.InvalidOutputExtension(widget, error)

return self.algorithm().preprocessParameters(parameters)

Expand Down Expand Up @@ -294,6 +303,18 @@ def on_complete(ok, results):
self.messageBar().clearWidgets()
self.messageBar().pushMessage("", self.tr("Wrong or missing parameter value: {0}").format(e.parameter.description()),
level=Qgis.Warning, duration=5)
except AlgorithmDialogBase.InvalidOutputExtension as e:
try:
self.buttonBox().accepted.connect(lambda e=e:
e.widget.setPalette(QPalette()))
palette = e.widget.palette()
palette.setColor(QPalette.Base, QColor(255, 255, 0))
e.widget.setPalette(palette)
except:
pass
self.messageBar().clearWidgets()
self.messageBar().pushMessage("", e.message,
level=Qgis.Warning, duration=5)

def finish(self, successful, result, context, feedback):
keepOpen = not successful or ProcessingConfig.getSetting(ProcessingConfig.KEEP_DIALOG_OPEN)
Expand Down
6 changes: 6 additions & 0 deletions python/plugins/processing/gui/AlgorithmDialogBase.py
View file
Expand Up @@ -32,3 +32,9 @@ class InvalidParameterValue(Exception):

def __init__(self, param, widget):
(self.parameter, self.widget) = (param, widget)

class InvalidOutputExtension(Exception):

def __init__(self, widget, message):
self.widget = widget
self.message = message
60 changes: 60 additions & 0 deletions src/core/processing/qgsprocessingprovider.cpp
View file
Expand Up @@ -107,6 +107,66 @@ QStringList QgsProcessingProvider::supportedOutputTableExtensions() const
return supportedOutputVectorLayerExtensions();
}

bool QgsProcessingProvider::isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error ) const
{
QString outputPath = QgsProcessingParameters::parameterAsOutputLayer( parameter, outputValue, context );
error.clear();
if ( parameter->type() == QgsProcessingParameterVectorDestination::typeName()
|| parameter->type() == QgsProcessingParameterFeatureSink::typeName() )
{
if ( outputPath.isEmpty() || outputPath.startsWith( QLatin1String( "memory:" ) ) )
{
if ( !supportsNonFileBasedOutput() )
{
error = tr( "This algorithm only supports disk-based outputs" );
return false;
}
return true;
}

QString providerKey;
QString uri;
QString layerName;
QMap<QString, QVariant> options;
bool useWriter = false;
QString format;
QString extension;
QgsProcessingUtils::parseDestinationString( outputPath, providerKey, uri, layerName, format, options, useWriter, extension );

if ( providerKey != QLatin1String( "ogr" ) )
{
if ( !supportsNonFileBasedOutput() )
{
error = tr( "This algorithm only supports disk-based outputs" );
return false;
}
return true;
}

if ( !supportedOutputVectorLayerExtensions().contains( extension, Qt::CaseInsensitive ) )
{
error = tr( "%1 files are not supported as outputs for this algorithm" ).arg( extension );
return false;
}
return true;
}
else if ( parameter->type() == QgsProcessingParameterRasterDestination::typeName() )
{
QFileInfo fi( outputPath );
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 );
return false;
}
return true;
}
else
{
return true;
}
}

QString QgsProcessingProvider::defaultVectorFileExtension( bool hasGeometry ) const
{
QgsSettings settings;
Expand Down
9 changes: 9 additions & 0 deletions src/core/processing/qgsprocessingprovider.h
View file
Expand Up @@ -140,6 +140,15 @@ class CORE_EXPORT QgsProcessingProvider : public QObject
*/
virtual QStringList supportedOutputTableExtensions() const;

/**
* Returns true if the specified \a outputValue is of a supported file format for the given destination \a parameter.
*
* If the output value is not supported, \a error will be set to a descriptive message explaining why.
*
* \since QGIS 3.6
*/
virtual bool isSupportedOutputValue( const QVariant &outputValue, const QgsProcessingDestinationParameter *parameter, QgsProcessingContext &context, QString &error SIP_OUT ) const;

/**
* Returns the default file extension to use for vector outputs created by the
* provider.
Expand Down
14 changes: 10 additions & 4 deletions src/core/processing/qgsprocessingutils.cpp
View file
Expand Up @@ -481,8 +481,9 @@ QString QgsProcessingUtils::stringToPythonLiteral( const QString &string )
return s;
}

void QgsProcessingUtils::parseDestinationString( QString &destination, QString &providerKey, QString &uri, QString &layerName, QString &format, QMap<QString, QVariant> &options, bool &useWriter )
void QgsProcessingUtils::parseDestinationString( QString &destination, QString &providerKey, QString &uri, QString &layerName, QString &format, QMap<QString, QVariant> &options, bool &useWriter, QString &extension )
{
extension.clear();
QRegularExpression splitRx( QStringLiteral( "^(.{3,}?):(.*)$" ) );
QRegularExpressionMatch match = splitRx.match( destination );
if ( match.hasMatch() )
Expand All @@ -504,7 +505,12 @@ void QgsProcessingUtils::parseDestinationString( QString &destination, QString &
options.insert( QStringLiteral( "layerName" ), layerName );
}
uri = dsUri.database();
format = QgsVectorFileWriter::driverForExtension( QFileInfo( uri ).completeSuffix() );
extension = QFileInfo( uri ).completeSuffix();
format = QgsVectorFileWriter::driverForExtension( extension );
}
else
{
extension = QFileInfo( uri ).completeSuffix();
}
options.insert( QStringLiteral( "update" ), true );
}
Expand All @@ -516,7 +522,6 @@ void QgsProcessingUtils::parseDestinationString( QString &destination, QString &
providerKey = QStringLiteral( "ogr" );
QRegularExpression splitRx( QStringLiteral( "^(.*)\\.(.*?)$" ) );
QRegularExpressionMatch match = splitRx.match( destination );
QString extension;
if ( match.hasMatch() )
{
extension = match.captured( 2 );
Expand Down Expand Up @@ -574,8 +579,9 @@ QgsFeatureSink *QgsProcessingUtils::createFeatureSink( QString &destination, Qgs
QString uri;
QString layerName;
QString format;
QString extension;
bool useWriter = false;
parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );

QgsFields newFields = fields;
if ( useWriter && providerKey == QLatin1String( "ogr" ) )
Expand Down
3 changes: 2 additions & 1 deletion src/core/processing/qgsprocessingutils.h
View file
Expand Up @@ -318,9 +318,10 @@ class CORE_EXPORT QgsProcessingUtils
*/
static QgsMapLayer *loadMapLayerFromString( const QString &string, LayerHint typeHint = UnknownType );

static void parseDestinationString( QString &destination, QString &providerKey, QString &uri, QString &layerName, QString &format, QMap<QString, QVariant> &options, bool &useWriter );
static void parseDestinationString( QString &destination, QString &providerKey, QString &uri, QString &layerName, QString &format, QMap<QString, QVariant> &options, bool &useWriter, QString &extension );

friend class TestQgsProcessing;
friend class QgsProcessingProvider;

};

Expand Down
38 changes: 32 additions & 6 deletions tests/src/analysis/testqgsprocessing.cpp
View file
Expand Up @@ -1514,59 +1514,66 @@ void TestQgsProcessing::parseDestinationString()
QString layerName;
QString format;
QVariantMap options;
QString extension;
bool useWriter = false;

// simple shapefile output
QString destination = QStringLiteral( "d:/test.shp" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( destination, QStringLiteral( "d:/test.shp" ) );
QCOMPARE( providerKey, QStringLiteral( "ogr" ) );
QCOMPARE( uri, QStringLiteral( "d:/test.shp" ) );
QCOMPARE( format, QStringLiteral( "ESRI Shapefile" ) );
QCOMPARE( extension, QStringLiteral( "shp" ) );
QVERIFY( useWriter );

// postgis output
destination = QStringLiteral( "postgis:dbname='db' host=DBHOST port=5432 table=\"calcs\".\"output\" (geom) sql=" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( providerKey, QStringLiteral( "postgres" ) );
QCOMPARE( uri, QStringLiteral( "dbname='db' host=DBHOST port=5432 table=\"calcs\".\"output\" (geom) sql=" ) );
QVERIFY( !useWriter );
QVERIFY( extension.isEmpty() );
// postgres key should also work
destination = QStringLiteral( "postgres:dbname='db' host=DBHOST port=5432 table=\"calcs\".\"output\" (geom) sql=" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( providerKey, QStringLiteral( "postgres" ) );
QCOMPARE( uri, QStringLiteral( "dbname='db' host=DBHOST port=5432 table=\"calcs\".\"output\" (geom) sql=" ) );
QVERIFY( !useWriter );
QVERIFY( extension.isEmpty() );

// full uri shp output
options.clear();
destination = QStringLiteral( "ogr:d:/test.shp" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( providerKey, QStringLiteral( "ogr" ) );
QCOMPARE( uri, QStringLiteral( "d:/test.shp" ) );
QCOMPARE( options.value( QStringLiteral( "update" ) ).toBool(), true );
QVERIFY( !options.contains( QStringLiteral( "layerName" ) ) );
QVERIFY( !useWriter );
QCOMPARE( extension, QStringLiteral( "shp" ) );

// full uri geopackage output
options.clear();
destination = QStringLiteral( "ogr:d:/test.gpkg" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( providerKey, QStringLiteral( "ogr" ) );
QCOMPARE( uri, QStringLiteral( "d:/test.gpkg" ) );
QCOMPARE( options.value( QStringLiteral( "update" ) ).toBool(), true );
QVERIFY( !options.contains( QStringLiteral( "layerName" ) ) );
QVERIFY( !useWriter );
QCOMPARE( extension, QStringLiteral( "gpkg" ) );

// full uri geopackage table output with layer name
options.clear();
destination = QStringLiteral( "ogr:dbname='d:/package.gpkg' table=\"mylayer\" (geom) sql=" );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter );
QgsProcessingUtils::parseDestinationString( destination, providerKey, uri, layerName, format, options, useWriter, extension );
QCOMPARE( providerKey, QStringLiteral( "ogr" ) );
QCOMPARE( uri, QStringLiteral( "d:/package.gpkg" ) );
QCOMPARE( options.value( QStringLiteral( "update" ) ).toBool(), true );
QCOMPARE( options.value( QStringLiteral( "layerName" ) ).toString(), QStringLiteral( "mylayer" ) );
QVERIFY( !useWriter );
QCOMPARE( extension, QStringLiteral( "gpkg" ) );
}

void TestQgsProcessing::createFeatureSink()
Expand Down Expand Up @@ -5311,6 +5318,16 @@ void TestQgsProcessing::parameterVectorOut()

def.reset( new QgsProcessingParameterVectorDestination( "with_geom", QString(), QgsProcessing::TypeVectorAnyGeometry, QString(), true ) );
DummyProvider3 provider;
QString error;
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( "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 ) );

provider.loadAlgorithms();
def->mOriginalProvider = &provider;
QCOMPARE( def->supportedOutputVectorLayerExtensions().count(), 2 );
Expand Down Expand Up @@ -5423,6 +5440,15 @@ void TestQgsProcessing::parameterRasterOut()
QCOMPARE( fromCode->flags(), def->flags() );
QCOMPARE( fromCode->defaultValue(), def->defaultValue() );

DummyProvider3 provider;
QString 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 ) );
QVERIFY( provider.isSupportedOutputValue( "d:/test.mig", def.get(), context, error ) );
QVERIFY( provider.isSupportedOutputValue( "d:/test.MIG", def.get(), context, error ) );
QVERIFY( provider.isSupportedOutputValue( QgsProcessingOutputLayerDefinition( "d:/test.MIG" ), def.get(), context, error ) );

// test layers to load on completion
def.reset( new QgsProcessingParameterRasterDestination( "x", QStringLiteral( "desc" ), QStringLiteral( "default.tif" ), true ) );
QgsProcessingOutputLayerDefinition fs = QgsProcessingOutputLayerDefinition( QStringLiteral( "test.tif" ) );
Expand Down

0 comments on commit 96b25ab

Please sign in to comment.