From 2bd18332aadb04168941d7cc5aa0dbba623ebaeb Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Mon, 7 Feb 2022 11:41:29 +1000 Subject: [PATCH] [expressions] Make file function (such as base_file_name) gracefully handle map layer values These are treated as the file path to the layer. E.g. base_file_name(@some_variable_which_is_a_layer) will return the base file name of the layer's source --- resources/function_help/json/base_file_name | 2 +- resources/function_help/json/exif | 2 +- resources/function_help/json/exif_geotag | 2 +- resources/function_help/json/file_exists | 2 +- resources/function_help/json/file_name | 2 +- resources/function_help/json/file_path | 2 +- resources/function_help/json/file_size | 2 +- resources/function_help/json/file_suffix | 2 +- resources/function_help/json/is_directory | 2 +- resources/function_help/json/is_file | 2 +- src/core/expression/qgsexpressionfunction.cpp | 74 +++++++++++++++---- src/core/expression/qgsexpressionutils.cpp | 21 ++++++ src/core/expression/qgsexpressionutils.h | 7 ++ tests/src/core/testqgsexpression.cpp | 35 +++++++++ 14 files changed, 134 insertions(+), 23 deletions(-) diff --git a/resources/function_help/json/base_file_name b/resources/function_help/json/base_file_name index 9bc65be2e79..8b3ad921889 100644 --- a/resources/function_help/json/base_file_name +++ b/resources/function_help/json/base_file_name @@ -5,7 +5,7 @@ "description": "Returns the base name of the file without the directory or file suffix.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "base_file_name('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/exif b/resources/function_help/json/exif index 93316cadf4b..5c18379121f 100644 --- a/resources/function_help/json/exif +++ b/resources/function_help/json/exif @@ -5,7 +5,7 @@ "description": "Retrieves exif tag values from an image file.", "arguments": [{ "arg": "path", - "description": "An image file path." + "description": "An image file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }, { "arg": "tag", "optional": true, diff --git a/resources/function_help/json/exif_geotag b/resources/function_help/json/exif_geotag index ef99de2e957..86cce098c8a 100644 --- a/resources/function_help/json/exif_geotag +++ b/resources/function_help/json/exif_geotag @@ -5,7 +5,7 @@ "description": "Creates a point geometry from the exif geotags of an image file.", "arguments": [{ "arg": "path", - "description": "An image file path." + "description": "An image file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "geom_to_wkt(exif_geotag('/my/photo.jpg'))", diff --git a/resources/function_help/json/file_exists b/resources/function_help/json/file_exists index 829303ce431..210ddcd82db 100644 --- a/resources/function_help/json/file_exists +++ b/resources/function_help/json/file_exists @@ -5,7 +5,7 @@ "description": "Returns true if a file path exists.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "file_exists('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/file_name b/resources/function_help/json/file_name index 8625e52c439..bd5575b124c 100644 --- a/resources/function_help/json/file_name +++ b/resources/function_help/json/file_name @@ -5,7 +5,7 @@ "description": "Returns the name of a file (including the file extension), excluding the directory.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "file_name('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/file_path b/resources/function_help/json/file_path index 91ef102af46..e61acb2f16b 100644 --- a/resources/function_help/json/file_path +++ b/resources/function_help/json/file_path @@ -5,7 +5,7 @@ "description": "Returns the directory component of a file path. This does not include the file name.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "file_path('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/file_size b/resources/function_help/json/file_size index afcf386498a..8303f30e438 100644 --- a/resources/function_help/json/file_size +++ b/resources/function_help/json/file_size @@ -5,7 +5,7 @@ "description": "Returns the size (in bytes) of a file.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "file_size('/home/qgis/data/country_boundaries.geojson')", diff --git a/resources/function_help/json/file_suffix b/resources/function_help/json/file_suffix index 5c765580184..4f0863d4348 100644 --- a/resources/function_help/json/file_suffix +++ b/resources/function_help/json/file_suffix @@ -5,7 +5,7 @@ "description": "Returns the file suffix (extension) from a file path.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "file_suffix('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/is_directory b/resources/function_help/json/is_directory index 7a7087599dd..fbfbe50177c 100644 --- a/resources/function_help/json/is_directory +++ b/resources/function_help/json/is_directory @@ -5,7 +5,7 @@ "description": "Returns true if a path corresponds to a directory.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "is_directory('/home/qgis/data/country_boundaries.shp')", diff --git a/resources/function_help/json/is_file b/resources/function_help/json/is_file index 575a2c3f9a6..af538fadcc6 100644 --- a/resources/function_help/json/is_file +++ b/resources/function_help/json/is_file @@ -5,7 +5,7 @@ "description": "Returns true if a path corresponds to a file.", "arguments": [{ "arg": "path", - "description": "a file path" + "description": "a file path or a map layer value. If a map layer layer value is specified then the file source of the layer will be used." }], "examples": [{ "expression": "is_file('/home/qgis/data/country_boundaries.shp')", diff --git a/src/core/expression/qgsexpressionfunction.cpp b/src/core/expression/qgsexpressionfunction.cpp index 6e451fb24a1..91e1b9d8fa8 100644 --- a/src/core/expression/qgsexpressionfunction.cpp +++ b/src/core/expression/qgsexpressionfunction.cpp @@ -2464,14 +2464,24 @@ static QVariant fcnDateTimeFromEpoch( const QVariantList &values, const QgsExpre static QVariant fcnExif( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - QString filepath = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString filepath = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `exif` requires a value which represents a possible file path" ) ); + return QVariant(); + } QString tag = QgsExpressionUtils::getStringValue( values.at( 1 ), parent ); return !tag.isNull() ? QgsExifTools::readTag( filepath, tag ) : QVariant( QgsExifTools::readTags( filepath ) ); } static QVariant fcnExifGeoTag( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - QString filepath = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString filepath = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `exif_geotag` requires a value which represents a possible file path" ) ); + return QVariant(); + } bool ok; return QVariant::fromValue( QgsGeometry( new QgsPoint( QgsExifTools::getGeoTag( filepath, ok ) ) ) ); } @@ -6515,53 +6525,91 @@ static QVariant fcnEnvVar( const QVariantList &values, const QgsExpressionContex static QVariant fcnBaseFileName( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `base_file_name` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).completeBaseName(); } static QVariant fcnFileSuffix( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `file_suffix` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).completeSuffix(); } static QVariant fcnFileExists( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); - return QFileInfo::exists( file ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `file_exists` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo::exists( file ); } static QVariant fcnFileName( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); - return QFileInfo( file ).fileName(); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `file_name` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).fileName(); } static QVariant fcnPathIsFile( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `is_file` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).isFile(); } static QVariant fcnPathIsDir( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `is_directory` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).isDir(); } static QVariant fcnFilePath( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `file_path` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QDir::toNativeSeparators( QFileInfo( file ).path() ); } static QVariant fcnFileSize( const QVariantList &values, const QgsExpressionContext *, QgsExpression *parent, const QgsExpressionNodeFunction * ) { - const QString file = QgsExpressionUtils::getStringValue( values.at( 0 ), parent ); + const QString file = QgsExpressionUtils::getFilePathValue( values.at( 0 ), parent ); + if ( parent->hasEvalError() ) + { + parent->setEvalErrorString( QObject::tr( "Function `file_size` requires a value which represents a possible file path" ) ); + return QVariant(); + } return QFileInfo( file ).size(); } -static QVariant fcnHash( const QString str, const QCryptographicHash::Algorithm algorithm ) +static QVariant fcnHash( const QString &str, const QCryptographicHash::Algorithm algorithm ) { return QString( QCryptographicHash::hash( str.toUtf8(), algorithm ).toHex() ); } diff --git a/src/core/expression/qgsexpressionutils.cpp b/src/core/expression/qgsexpressionutils.cpp index 7ed2b3a2a3c..90e50cf8723 100644 --- a/src/core/expression/qgsexpressionutils.cpp +++ b/src/core/expression/qgsexpressionutils.cpp @@ -17,6 +17,7 @@ #include "qgsexpressionnode.h" #include "qgsvectorlayer.h" #include "qgscolorrampimpl.h" +#include "qgsproviderregistry.h" ///@cond PRIVATE @@ -49,6 +50,26 @@ QgsGradientColorRamp QgsExpressionUtils::getRamp( const QVariant &value, QgsExpr return QgsGradientColorRamp(); } +QString QgsExpressionUtils::getFilePathValue( const QVariant &value, QgsExpression *parent ) +{ + // if it's a map layer, return the file path of that layer... + QString res; + if ( QgsMapLayer *layer = getMapLayer( value, parent ) ) + { + const QVariantMap parts = QgsProviderRegistry::instance()->decodeUri( layer->providerType(), layer->source() ); + res = parts.value( QStringLiteral( "path" ) ).toString(); + } + + if ( res.isEmpty() ) + res = value.toString(); + + if ( res.isEmpty() && !value.isNull() ) + { + parent->setEvalErrorString( QObject::tr( "Cannot convert value to a file path" ) ); + } + return res; +} + ///@endcond std::tuple QgsExpressionUtils::determineResultType( const QString &expression, const QgsVectorLayer *layer, QgsFeatureRequest request, QgsExpressionContext context, bool *foundFeatures ) diff --git a/src/core/expression/qgsexpressionutils.h b/src/core/expression/qgsexpressionutils.h index 1e4f2b6271f..c53fe09d3f4 100644 --- a/src/core/expression/qgsexpressionutils.h +++ b/src/core/expression/qgsexpressionutils.h @@ -419,6 +419,13 @@ class CORE_EXPORT QgsExpressionUtils return qobject_cast( getMapLayer( value, e ) ); } + /** + * Tries to convert a \a value to a file path. + * + * \since QGIS 3.24 + */ + static QString getFilePathValue( const QVariant &value, QgsExpression *parent ); + static QVariantList getListValue( const QVariant &value, QgsExpression *parent ) { if ( value.type() == QVariant::List || value.type() == QVariant::StringList ) diff --git a/tests/src/core/testqgsexpression.cpp b/tests/src/core/testqgsexpression.cpp index 755adcce197..b93d0b6ef8f 100644 --- a/tests/src/core/testqgsexpression.cpp +++ b/tests/src/core/testqgsexpression.cpp @@ -1990,35 +1990,43 @@ class TestQgsExpression: public QObject QTest::newRow( "base_file_name(NULL)" ) << QStringLiteral( "base_file_name(NULL)" ) << false << QVariant(); QTest::newRow( "base_file_name('/home/qgis/test.qgs')" ) << QStringLiteral( "base_file_name('/home/qgis/test.qgs')" ) << false << QVariant( "test" ); QTest::newRow( "base_file_name(points.shp)" ) << QStringLiteral( "base_file_name('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( "points" ); + QTest::newRow( "base_file_name(map layer)" ) << QStringLiteral( "base_file_name('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( "points" ); QTest::newRow( "file_exists(NULL)" ) << QStringLiteral( "file_exists(NULL)" ) << false << QVariant(); QTest::newRow( "file_exists('/home/qgis/test.qgs')" ) << QStringLiteral( "file_exists('/home/qgis/test.qgs')" ) << false << QVariant( false ); QTest::newRow( "file_exists(points.shp)" ) << QStringLiteral( "file_exists('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( true ); + QTest::newRow( "file_exists(map layer)" ) << QStringLiteral( "file_exists('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( true ); QTest::newRow( "file_name(5)" ) << QStringLiteral( "file_name(5)" ) << false << QVariant( "5" ); QTest::newRow( "file_name(NULL)" ) << QStringLiteral( "file_name(NULL)" ) << false << QVariant(); QTest::newRow( "file_name('/home/qgis/test.qgs')" ) << QStringLiteral( "file_name('/home/qgis/test.qgs')" ) << false << QVariant( "test.qgs" ); QTest::newRow( "file_name(points.shp)" ) << QStringLiteral( "file_name('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( "points.shp" ); + QTest::newRow( "file_name(map layer)" ) << QStringLiteral( "file_name('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( "points.shp" ); QTest::newRow( "file_path(5)" ) << QStringLiteral( "file_path(5)" ) << false << QVariant( "." ); QTest::newRow( "file_path(NULL)" ) << QStringLiteral( "file_path(NULL)" ) << false << QVariant(); QTest::newRow( "file_path('/home/qgis/test.qgs')" ) << QStringLiteral( "file_path('/home/qgis/test.qgs')" ) << false << QVariant( "/home/qgis" ); QTest::newRow( "file_path(points.shp)" ) << QStringLiteral( "file_path('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( TEST_DATA_DIR ); + QTest::newRow( "file_path(map layer)" ) << QStringLiteral( "file_path('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( TEST_DATA_DIR ); QTest::newRow( "file_size(5)" ) << QStringLiteral( "file_size(5)" ) << false << QVariant( 0LL ); QTest::newRow( "file_size(NULL)" ) << QStringLiteral( "file_size(NULL)" ) << false << QVariant(); QTest::newRow( "file_size('/home/qgis/test.qgs')" ) << QStringLiteral( "file_size('/home/qgis/test.qgs')" ) << false << QVariant( 0LL ); QTest::newRow( "file_size(points.shp)" ) << QStringLiteral( "file_size('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( 576LL ); + QTest::newRow( "file_size(map layer)" ) << QStringLiteral( "file_size('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( 576LL ); QTest::newRow( "file_suffix(5)" ) << QStringLiteral( "file_suffix(5)" ) << false << QVariant( "" ); QTest::newRow( "file_suffix(NULL)" ) << QStringLiteral( "file_suffix(NULL)" ) << false << QVariant(); QTest::newRow( "file_suffix('/home/qgis/test.qgs')" ) << QStringLiteral( "file_suffix('/home/qgis/test.qgs')" ) << false << QVariant( "qgs" ); QTest::newRow( "file_suffix(points.shp)" ) << QStringLiteral( "file_suffix('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( "shp" ); + QTest::newRow( "file_suffix(map layer)" ) << QStringLiteral( "file_suffix('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( "shp" ); QTest::newRow( "is_directory(5)" ) << QStringLiteral( "is_directory(5)" ) << false << QVariant( false ); QTest::newRow( "is_directory(NULL)" ) << QStringLiteral( "is_directory(NULL)" ) << false << QVariant(); QTest::newRow( "is_directory('/home/qgis/test.qgs')" ) << QStringLiteral( "is_directory('/home/qgis/test.qgs')" ) << false << QVariant( false ); QTest::newRow( "is_directory(points.shp)" ) << QStringLiteral( "is_directory('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( false ); QTest::newRow( "is_directory(valid)" ) << QStringLiteral( "is_directory('%1')" ).arg( TEST_DATA_DIR ) << false << QVariant( true ); + QTest::newRow( "is_directory(map layer)" ) << QStringLiteral( "is_directory('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( false ); QTest::newRow( "is_file(5)" ) << QStringLiteral( "is_file(5)" ) << false << QVariant( false ); QTest::newRow( "is_file(NULL)" ) << QStringLiteral( "is_file(NULL)" ) << false << QVariant(); QTest::newRow( "is_file('/home/qgis/test.qgs')" ) << QStringLiteral( "is_file('/home/qgis/test.qgs')" ) << false << QVariant( false ); QTest::newRow( "is_file(points.shp)" ) << QStringLiteral( "is_file('%1/points.shp')" ).arg( TEST_DATA_DIR ) << false << QVariant( true ); QTest::newRow( "is_file(valid)" ) << QStringLiteral( "is_file('%1')" ).arg( TEST_DATA_DIR ) << false << QVariant( false ); + QTest::newRow( "is_file(map layer)" ) << QStringLiteral( "is_file('%1')" ).arg( mPointsLayer->id() ) << false << QVariant( true ); // hash functions QTest::newRow( "md5(NULL)" ) << QStringLiteral( "md5(NULL)" ) << false << QVariant(); @@ -4297,6 +4305,33 @@ class TestQgsExpression: public QObject #endif } + void testGetFilePathValue() + { + QgsExpression exp; + // NULL value + QString path = QgsExpressionUtils::getFilePathValue( QVariant(), &exp ); + QVERIFY( path.isEmpty() ); + QVERIFY( !exp.hasEvalError() ); + + // value which CANNOT be a file path + path = QgsExpressionUtils::getFilePathValue( QVariant::fromValue( QgsGeometry() ), &exp ); + QVERIFY( path.isEmpty() ); + QVERIFY( exp.hasEvalError() ); + QCOMPARE( exp.evalErrorString(), QStringLiteral( "Cannot convert value to a file path" ) ); + + // good value + exp = QgsExpression(); + path = QgsExpressionUtils::getFilePathValue( QVariant::fromValue( QStringLiteral( "/home/me/mine.txt" ) ), &exp ); + QCOMPARE( path, QStringLiteral( "/home/me/mine.txt" ) ); + QVERIFY( !exp.hasEvalError() ); + + // with map layer pointer -- should use layer path + exp = QgsExpression(); + path = QgsExpressionUtils::getFilePathValue( QVariant::fromValue( mPointsLayer ), &exp ); + QCOMPARE( path, QStringLiteral( TEST_DATA_DIR ) + QStringLiteral( "/points.shp" ) ); + QVERIFY( !exp.hasEvalError() ); + } + void test_env() { QgsExpressionContext context;