OGR provider MUST include layername even for single sublayer gpkg/etc…

…, in case more layers are added in future
qgis · Jul 12, 2021 · 665f6a5 · 665f6a5
1 parent 352c050
commit 665f6a5
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 19 deletions.
diff --git a/src/core/providers/ogr/qgsogrprovidermetadata.cpp b/src/core/providers/ogr/qgsogrprovidermetadata.cpp
@@ -1042,13 +1042,13 @@ QList<QgsProviderSublayerDetails> QgsOgrProviderMetadata::querySublayers( const
 
   const int layerCount = firstLayer->GetLayerCount();
 
+  QList<QgsProviderSublayerDetails> res;
   if ( layerCount == 1 )
   {
-    return QgsOgrProviderUtils::querySubLayerList( 0, firstLayer.get(), driverName, flags, false, uri, true, feedback );
+    res << QgsOgrProviderUtils::querySubLayerList( 0, firstLayer.get(), driverName, flags, false, uri, true, feedback );
   }
   else
   {
-    QList<QgsProviderSublayerDetails> res;
     // In case there is no free opened dataset in the cache, keep the first
     // layer alive while we iterate over the other layers, so that we can
     // reuse the same dataset. Can help in a particular with a FileGDB with
@@ -1077,28 +1077,37 @@ QList<QgsProviderSublayerDetails> QgsOgrProviderMetadata::querySublayers( const
 
       res << QgsOgrProviderUtils::querySubLayerList( i, i == 0 ? firstLayer.get() : layer.get(), driverName, flags, false, uri, false, feedback );
     }
+  }
 
-    // if all layernames are equal, we remove them from the uris
-    QSet< QString > layerNames;
-    for ( const QgsProviderSublayerDetails &details : std::as_const( res ) )
-    {
-      const QVariantMap parts = decodeUri( details.uri() );
-      layerNames.insert( parts.value( QStringLiteral( "layerName" ) ).toString() );
-    }
-
-    if ( layerNames.count() == 1 )
+  // Systematically add a layerName= option to all OGR sublayers 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!)
+  for ( int i = 0; i < res.count(); ++i )
+  {
+    QVariantMap parts = decodeUri( res.at( i ).uri() );
+    if ( !parts.value( QStringLiteral( "layerName" ) ).toString().isEmpty() ||
+         !parts.value( QStringLiteral( "layerId" ) ).toString().isEmpty() )
+      continue;
+
+    bool isAlwaysSingleLayerDataset = false;
+    const QFileInfo fi( parts.value( QStringLiteral( "path" ) ).toString() );
+    if ( fi.isFile() )
     {
-      // all layer names are the same, so remove them from uris
-      for ( int i = 0; i < res.size(); ++i )
-      {
-        QVariantMap parts = decodeUri( res.at( i ).uri() );
-        parts.remove( QStringLiteral( "layerName" ) );
-        res[ i ].setUri( encodeUri( parts ) );
-      }
+      const QString ext = fi.suffix().toLower();
+      isAlwaysSingleLayerDataset = ext == QLatin1String( "shp" ) ||
+                                   ext == QLatin1String( "mif" ) ||
+                                   ext == QLatin1String( "tab" ) ||
+                                   ext == QLatin1String( "csv" ) ||
+                                   ext == QLatin1String( "geojson" );
     }
+    if ( isAlwaysSingleLayerDataset )
+      continue;
 
-    return res;
+    parts.insert( QStringLiteral( "layerName" ), res.at( i ).name() );
+    res[i].setUri( encodeUri( parts ) );
   }
+
+  return res;
 }
 
 QMap<QString, QgsAbstractProviderConnection *> QgsOgrProviderMetadata::connections( bool cached )

diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py
@@ -1530,6 +1530,19 @@ def test_provider_sublayer_details(self):
         self.assertEqual(res[0].wkbType(), QgsWkbTypes.LineString)
         self.assertEqual(res[0].geometryColumnName(), '')
 
+        # single layer geopackage -- sublayers MUST have the layerName set on the uri,
+        # in case more layers are added in future to the gpkg
+        res = metadata.querySublayers(os.path.join(TEST_DATA_DIR, 'curved_polys.gpkg'))
+        self.assertEqual(len(res), 1)
+        self.assertEqual(res[0].layerNumber(), 0)
+        self.assertEqual(res[0].name(), "polys")
+        self.assertEqual(res[0].description(), '')
+        self.assertEqual(res[0].uri(), TEST_DATA_DIR + "/curved_polys.gpkg|layername=polys")
+        self.assertEqual(res[0].providerKey(), "ogr")
+        self.assertEqual(res[0].type(), QgsMapLayerType.VectorLayer)
+        self.assertEqual(res[0].wkbType(), QgsWkbTypes.CurvePolygon)
+        self.assertEqual(res[0].geometryColumnName(), 'geometry')
+
         # make sure result is valid to load layer from
         options = QgsProviderSublayerDetails.LayerOptions(QgsCoordinateTransformContext())
         vl = res[0].toLayer(options)