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

@@ -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<QgsVectorLayer>( 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
+        setupVectorLayer( src, sublayers, layer,
-        QStringList elements = sublayers.at( 0 ).split( QgsDataProvider::SUBLAYER_SEPARATOR );
+                          QStringLiteral( "ogr" ), options );
-        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 ) ) );
-        }
 
         myList << layer;
       }
@@ -4938,14 +4989,7 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer )
     // OGR provider returns items in this format:
     // <layer_index>:<name>:<feature_count>:<geom_type>
 
-    QStringList elements = sublayer.split( QgsDataProvider::SUBLAYER_SEPARATOR );
+    QStringList elements = splitSubLayerDef( sublayer );
-    // merge back parts of the name that may have been split
-    while ( elements.size() > 5 )
-    {
-      elements[1] += ":" + elements[2];
-      elements.removeAt( 2 );
-    }
-
     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
+  auto uriParts = QgsProviderRegistry::instance()->decodeUri(
-  if ( uri.contains( '|', Qt::CaseSensitive ) )
+                    layer->providerType(), layer->dataProvider()->dataSourceUri() );
-  {
+  QString uri( uriParts.value( QLatin1String( "path" ) ).toString() );
-    // 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 );
-  }
 
   // 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 );
+        setupVectorLayer( vectorLayerPath, sublayers, layer,
-        QString subLayerNameFormatted = elements.size() >= 2 ? elements.at( 1 ) : QString();
+                          providerKey, options );
-        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 ) ) );
-        }
       }
 
       myList << layer;

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)

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 <QApplication>
+#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"