From 119cd8ace97d9dfef6c6d5f5fb3aca727a6cc31f Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Sat, 6 Oct 2018 15:24:12 +0200 Subject: [PATCH] QgisApp::addVectorLayer(): add |layername= to OGR datasets (fixes #20031) Do it also in case of datasets that have a single layer, in case they might later be edited to have more layers (except for a few drivers known to be always single layer) --- src/app/qgisapp.cpp | 115 ++++++++++++++++----------- tests/src/app/CMakeLists.txt | 1 + tests/src/app/testqgisapp.cpp | 141 ++++++++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 44 deletions(-) create mode 100644 tests/src/app/testqgisapp.cpp diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp index 9311418113a..5a645f95435 100644 --- a/src/app/qgisapp.cpp +++ b/src/app/qgisapp.cpp @@ -4441,6 +4441,68 @@ QString QgisApp::crsAndFormatAdjustedLayerUri( const QString &uri, const QString return newuri; } +static QStringList splitSubLayerDef( const QString &subLayerDef ) +{ + QStringList elements = subLayerDef.split( QgsDataProvider::SUBLAYER_SEPARATOR ); + // merge back parts of the name that may have been split + while ( elements.size() > 5 ) + { + elements[1] += ":" + elements[2]; + elements.removeAt( 2 ); + } + return elements; +} + +static void setupVectorLayer( const QString &vectorLayerPath, + const QStringList &sublayers, + QgsVectorLayer *&layer, + const QString &providerKey, + const QgsVectorLayer::LayerOptions &options ) +{ + //set friendly name for datasources with only one layer + QgsSettings settings; + QStringList elements = splitSubLayerDef( sublayers.at( 0 ) ); + QString rawLayerName = elements.size() >= 2 ? elements.at( 1 ) : QString(); + QString subLayerNameFormatted = rawLayerName; + if ( settings.value( QStringLiteral( "qgis/formatLayerName" ), false ).toBool() ) + { + subLayerNameFormatted = QgsMapLayer::formatLayerName( subLayerNameFormatted ); + } + + if ( elements.size() >= 4 && layer->name().compare( rawLayerName, Qt::CaseInsensitive ) != 0 + && layer->name().compare( subLayerNameFormatted, Qt::CaseInsensitive ) != 0 ) + { + layer->setName( QStringLiteral( "%1 %2" ).arg( layer->name(), rawLayerName ) ); + } + + // Systematically add a layername= option to OGR datasets in case + // the current single layer dataset becomes layer a multi-layer one. + // Except for a few select extensions, known to be always single layer dataset. + QFileInfo fi( vectorLayerPath ); + QString ext = fi.suffix().toLower(); + if ( providerKey == QLatin1String( "ogr" ) && + ext != QLatin1String( "shp" ) && + ext != QLatin1String( "mif" ) && + ext != QLatin1String( "tab" ) && + ext != QLatin1String( "csv" ) && + ext != QLatin1String( "geojson" ) && + ! vectorLayerPath.contains( QStringLiteral( "layerid=" ) ) && + ! vectorLayerPath.contains( QStringLiteral( "layername=" ) ) ) + { + auto uriParts = QgsProviderRegistry::instance()->decodeUri( + layer->providerType(), layer->dataProvider()->dataSourceUri() ); + QString composedURI( uriParts.value( QLatin1String( "path" ) ).toString() ); + composedURI += "|layername=" + rawLayerName; + + auto newLayer = qgis::make_unique( composedURI, layer->name(), QStringLiteral( "ogr" ), options ); + if ( newLayer && newLayer->isValid() ) + { + delete layer; + layer = newLayer.release(); + } + } +} + bool QgisApp::addVectorLayers( const QStringList &layerQStringList, const QString &enc, const QString &dataSourceType ) { bool wasfrozen = mMapCanvas->isFrozen(); @@ -4523,19 +4585,8 @@ bool QgisApp::addVectorLayers( const QStringList &layerQStringList, const QStrin } else if ( !sublayers.isEmpty() ) // there is 1 layer of data available { - //set friendly name for datasources with only one layer - QStringList elements = sublayers.at( 0 ).split( QgsDataProvider::SUBLAYER_SEPARATOR ); - QString subLayerNameFormatted = elements.size() >= 2 ? elements.at( 1 ) : QString(); - if ( settings.value( QStringLiteral( "qgis/formatLayerName" ), false ).toBool() ) - { - subLayerNameFormatted = QgsMapLayer::formatLayerName( subLayerNameFormatted ); - } - - if ( elements.size() >= 4 && layer->name().compare( elements.at( 1 ), Qt::CaseInsensitive ) != 0 - && layer->name().compare( subLayerNameFormatted, Qt::CaseInsensitive ) != 0 ) - { - layer->setName( QStringLiteral( "%1 %2" ).arg( layer->name(), elements.at( 1 ) ) ); - } + setupVectorLayer( src, sublayers, layer, + QStringLiteral( "ogr" ), options ); myList << layer; } @@ -4938,14 +4989,7 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer ) // OGR provider returns items in this format: // ::: - QStringList elements = sublayer.split( QgsDataProvider::SUBLAYER_SEPARATOR ); - // merge back parts of the name that may have been split - while ( elements.size() > 5 ) - { - elements[1] += ":" + elements[2]; - elements.removeAt( 2 ); - } - + QStringList elements = splitSubLayerDef( sublayer ); if ( elements.count() >= 4 ) { QgsSublayersDialog::LayerDefinition def; @@ -4978,18 +5022,11 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer ) if ( !chooseSublayersDialog.exec() ) return; - QString uri = layer->source(); QString name = layer->name(); - //the separator char & was changed to | to be compatible - //with url for protocol drivers - if ( uri.contains( '|', Qt::CaseSensitive ) ) - { - // If we get here, there are some options added to the filename. - // A valid uri is of the form: filename&option1=value1&option2=value2,... - // We want only the filename here, so we get the first part of the split. - QStringList theURIParts = uri.split( '|' ); - uri = theURIParts.at( 0 ); - } + + auto uriParts = QgsProviderRegistry::instance()->decodeUri( + layer->providerType(), layer->dataProvider()->dataSourceUri() ); + QString uri( uriParts.value( QLatin1String( "path" ) ).toString() ); // The uri must contain the actual uri of the vectorLayer from which we are // going to load the sublayers. @@ -10679,18 +10716,8 @@ QgsVectorLayer *QgisApp::addVectorLayer( const QString &vectorLayerPath, const Q QStringList sublayers = layer->dataProvider()->subLayers(); if ( !sublayers.isEmpty() ) { - QStringList elements = sublayers.at( 0 ).split( QgsDataProvider::SUBLAYER_SEPARATOR ); - QString subLayerNameFormatted = elements.size() >= 2 ? elements.at( 1 ) : QString(); - if ( settings.value( QStringLiteral( "qgis/formatLayerName" ), false ).toBool() ) - { - subLayerNameFormatted = QgsMapLayer::formatLayerName( subLayerNameFormatted ); - } - - if ( elements.size() >= 4 && layer->name().compare( elements.at( 1 ), Qt::CaseInsensitive ) != 0 - && layer->name().compare( subLayerNameFormatted, Qt::CaseInsensitive ) != 0 ) - { - layer->setName( QStringLiteral( "%1 %2" ).arg( layer->name(), elements.at( 1 ) ) ); - } + setupVectorLayer( vectorLayerPath, sublayers, layer, + providerKey, options ); } myList << layer; diff --git a/tests/src/app/CMakeLists.txt b/tests/src/app/CMakeLists.txt index fa74a667b8a..c5a51ac7b92 100644 --- a/tests/src/app/CMakeLists.txt +++ b/tests/src/app/CMakeLists.txt @@ -92,6 +92,7 @@ ENDMACRO (ADD_QGIS_TEST) IF (WITH_BINDINGS) ADD_QGIS_TEST(apppythontest testqgisapppython.cpp) ENDIF () +ADD_QGIS_TEST(qgisapp testqgisapp.cpp) ADD_QGIS_TEST(qgisappclipboard testqgisappclipboard.cpp) ADD_QGIS_TEST(attributetabletest testqgsattributetable.cpp) ADD_QGIS_TEST(applocatorfilters testqgsapplocatorfilters.cpp) diff --git a/tests/src/app/testqgisapp.cpp b/tests/src/app/testqgisapp.cpp new file mode 100644 index 00000000000..f153656bf48 --- /dev/null +++ b/tests/src/app/testqgisapp.cpp @@ -0,0 +1,141 @@ +/*************************************************************************** + testqgisapp.cpp + -------------------------------------- + Date : 6 october 2018 + Copyright : (C) 2018 by Even Rouault + Email : even.rouault at spatialys.com + *************************************************************************** + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + ***************************************************************************/ +#include "qgstest.h" + +#include +#include "gdal.h" + +#include "qgisapp.h" +#include "qgsproject.h" +#include "qgsmaplayerstore.h" + +/** + * \ingroup UnitTests + * This is a unit test for the QgisApp + */ +class TestQgisApp : public QObject +{ + Q_OBJECT + + public: + TestQgisApp(); + + private slots: + void initTestCase();// will be called before the first testfunction is executed. + void cleanupTestCase();// will be called after the last testfunction was executed. + void init(); // will be called before each testfunction is executed. + void cleanup(); // will be called after every testfunction. + + void addVectorLayerShp(); + void addVectorLayerGeopackageSingleLayer(); + void addVectorLayerGeopackageSingleLayerAlreadyLayername(); + void addVectorLayerInvalid(); + + private: + QgisApp *mQgisApp = nullptr; + QString mTestDataDir; +}; + +TestQgisApp::TestQgisApp() = default; + +//runs before all tests +void TestQgisApp::initTestCase() +{ + // Set up the QgsSettings environment + QCoreApplication::setOrganizationName( QStringLiteral( "QGIS" ) ); + QCoreApplication::setOrganizationDomain( QStringLiteral( "qgis.org" ) ); + QCoreApplication::setApplicationName( QStringLiteral( "QGIS-TEST" ) ); + + qDebug() << "TestQgisApp::initTestCase()"; + // init QGIS's paths - true means that all path will be inited from prefix + QgsApplication::init(); + QgsApplication::initQgis(); + mTestDataDir = QStringLiteral( TEST_DATA_DIR ) + '/'; //defined in CmakeLists.txt + mQgisApp = new QgisApp(); +} + +//runs after all tests +void TestQgisApp::cleanupTestCase() +{ + QgsApplication::exitQgis(); +} + +void TestQgisApp::init() +{ + GDALDriverH hDriver = GDALGetDriverByName( "GPKG" ); + QVERIFY( hDriver ); + GDALDatasetH hDS = GDALCreate( hDriver, "/vsimem/test.gpkg", 0, 0, 0, GDT_Unknown, nullptr ); + QVERIFY( hDS ); + GDALDatasetCreateLayer( hDS, "my_layer", nullptr, wkbPoint, nullptr ); + GDALClose( hDS ); +} + +void TestQgisApp::cleanup() +{ + GDALDriverH hDriver = GDALGetDriverByName( "GPKG" ); + QVERIFY( hDriver ); + GDALDeleteDataset( hDriver, "/vsimem/test.gpkg" ); +} + +void TestQgisApp::addVectorLayerShp() +{ + QString filePath = mTestDataDir + QStringLiteral( "points.shp" ); + QgsVectorLayer *layer = mQgisApp->addVectorLayer( filePath, "test", QStringLiteral( "ogr" ) ); + QVERIFY( layer->isValid() ); + + // No need for |layerName= for shapefiles + QVERIFY( layer->source().endsWith( QLatin1String( "points.shp" ) ) ); + + // cleanup + QgsProject::instance()->layerStore()->removeMapLayers( QStringList() << layer->id() ); +} + +void TestQgisApp::addVectorLayerGeopackageSingleLayer() +{ + QString filePath = QLatin1String( "/vsimem/test.gpkg" ); + QgsVectorLayer *layer = mQgisApp->addVectorLayer( filePath, "test", QStringLiteral( "ogr" ) ); + QVERIFY( layer->isValid() ); + + // Need for |layerName= for geopackage + QVERIFY( layer->source().endsWith( QLatin1String( "/vsimem/test.gpkg|layername=my_layer" ) ) ); + + // cleanup + QgsProject::instance()->layerStore()->removeMapLayers( QStringList() << layer->id() ); +} + +void TestQgisApp::addVectorLayerGeopackageSingleLayerAlreadyLayername() +{ + QString filePath = QLatin1String( "/vsimem/test.gpkg|layername=my_layer" ); + QgsVectorLayer *layer = mQgisApp->addVectorLayer( filePath, "test", QStringLiteral( "ogr" ) ); + QVERIFY( layer->isValid() ); + + // Need for |layerName= for geopackage + QVERIFY( layer->source().endsWith( QLatin1String( "/vsimem/test.gpkg|layername=my_layer" ) ) ); + + // cleanup + QgsProject::instance()->layerStore()->removeMapLayers( QStringList() << layer->id() ); +} + +void TestQgisApp::addVectorLayerInvalid() +{ + QgsVectorLayer *layer = mQgisApp->addVectorLayer( "i_do_not_exist", "test", QStringLiteral( "ogr" ) ); + QVERIFY( !layer ); + + layer = mQgisApp->addVectorLayer( "/vsimem/test.gpkg|layername=invalid_layer_name", "test", QStringLiteral( "ogr" ) ); + QVERIFY( !layer ); +} + +QGSTEST_MAIN( TestQgisApp ) +#include "testqgisapp.moc"