From 7cf26369c452f9265f7f51eafb65278098316c60 Mon Sep 17 00:00:00 2001
From: Alessandro Pasotti <apasotti@boundlessgeo.com>
Date: Mon, 18 Sep 2017 10:42:55 +0200
Subject: [PATCH] Handle gpkg vectors and rasters with ogr data items

---
 src/providers/gdal/qgsgdaldataitems.cpp | 29 +++++++++++++++++
 src/providers/ogr/qgsogrdataitems.cpp   | 41 +++++++++++++------------
 2 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/src/providers/gdal/qgsgdaldataitems.cpp b/src/providers/gdal/qgsgdaldataitems.cpp
index e9cfaa56b37..ca1cfdd64d9 100644
--- a/src/providers/gdal/qgsgdaldataitems.cpp
+++ b/src/providers/gdal/qgsgdaldataitems.cpp
@@ -241,12 +241,32 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
 #endif
   }
 
+  // Filters out the OGR/GDAL supported formats that can contain multiple layers
+  // and should be treated like a DB: GeoPackage and SQLite
+  // NOTE: this formats are scanned for rasters too and they are handled
+  //       by the "ogr" provider. For this reason they must
+  //       be skipped by "gdal" provider or the rasters will be listed
+  //       twice. ogrSupportedDbLayersExtensions must be kept in sync
+  //       with the companion variable (same name) in the ogr provider
+  //       class
+  // TODO: add more OGR supported multiple layers formats here!
+  QStringList ogrSupportedDbLayersExtensions;
+  ogrSupportedDbLayersExtensions << QLatin1String( "gpkg" ) << QLatin1String( "sqlite" ) << QLatin1String( "db" );
+  QStringList ogrSupportedDbDriverNames;
+  ogrSupportedDbDriverNames << QLatin1String( "GPKG" ) << QLatin1String( "db" );
+
   // return item without testing if:
   // scanExtSetting
   // or zipfile and scan zip == "Basic scan"
   if ( scanExtSetting ||
        ( ( is_vsizip || is_vsitar ) && scanZipSetting == QLatin1String( "basic" ) ) )
   {
+    // Skip this layer if it's handled by ogr:
+    if ( ogrSupportedDbLayersExtensions.contains( suffix ) )
+    {
+      return nullptr;
+    }
+
     // if this is a VRT file make sure it is raster VRT to avoid duplicates
     if ( suffix == QLatin1String( "vrt" ) )
     {
@@ -283,6 +303,15 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
     return nullptr;
   }
 
+  GDALDriverH hDriver = GDALGetDatasetDriver( hDS );
+  QString ogrDriverName = GDALGetDriverShortName( hDriver );
+
+  // Skip this layer if it's handled by ogr:
+  if ( ogrSupportedDbDriverNames.contains( ogrDriverName ) )
+  {
+    return nullptr;
+  }
+
   QStringList sublayers = QgsGdalProvider::subLayers( hDS );
 
   GDALClose( hDS );
diff --git a/src/providers/ogr/qgsogrdataitems.cpp b/src/providers/ogr/qgsogrdataitems.cpp
index b04bfaba702..3743a2b31d5 100644
--- a/src/providers/ogr/qgsogrdataitems.cpp
+++ b/src/providers/ogr/qgsogrdataitems.cpp
@@ -578,6 +578,8 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
   // TODO: add more OGR supported multiple layers formats here!
   QStringList ogrSupportedDbLayersExtensions;
   ogrSupportedDbLayersExtensions << QLatin1String( "gpkg" ) << QLatin1String( "sqlite" ) << QLatin1String( "db" );
+  QStringList ogrSupportedDbDriverNames;
+  ogrSupportedDbDriverNames << QLatin1String( "GPKG" ) << QLatin1String( "db" );
 
   // Fast track: return item without testing if:
   // scanExtSetting or zipfile and scan zip == "Basic scan"
@@ -624,13 +626,16 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
       return item;
   }
 
+  // Slow track: scan file contents
+  QgsDataItem *item = nullptr;
+
   // test that file is valid with OGR
   OGRRegisterAll();
-  GDALDriverH hDriver;
+  OGRSFDriverH hDriver;
   // do not print errors, but write to debug
   CPLPushErrorHandler( CPLQuietErrorHandler );
   CPLErrorReset();
-  GDALDatasetH hDataSource = QgsOgrProviderUtils::GDALOpenWrapper( path.toUtf8().constData(), false, &hDriver );
+  OGRDataSourceH hDataSource = QgsOgrProviderUtils::GDALOpenWrapper( path.toUtf8().constData(), false, &hDriver );
   CPLPopErrorHandler();
 
   if ( ! hDataSource )
@@ -639,26 +644,24 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
     return nullptr;
   }
 
-  QgsDebugMsgLevel( QString( "GDAL Driver : %1" ).arg( GDALGetDriverShortName( hDriver ) ), 2 );
-
-  QgsDataItem *item = nullptr;
-
+  QgsDebugMsgLevel( QString( "GDAL Driver : %1" ).arg( OGR_Dr_GetName( hDriver ) ), 2 );
+  QString ogrDriverName = OGR_Dr_GetName( hDriver );
   int numLayers = OGR_DS_GetLayerCount( hDataSource );
-  if ( numLayers == 1 )
-  {
-    QgsDebugMsgLevel( QString( "using name = %1" ).arg( name ), 2 );
-    item = dataItemForLayer( parentItem, name, path, hDataSource, 0 );
-  }
-  else if ( numLayers > 1 )
-  {
-    QgsDebugMsgLevel( QString( "using name = %1" ).arg( name ), 2 );
-    item = new QgsOgrDataCollectionItem( parentItem, name, path );
-  }
-  else if ( suffix.compare( QLatin1String( "gpkg" ), Qt::CaseInsensitive ) == 0 )
+
+  // GeoPackage needs a specialized data item, mainly because of raster deletion not
+  // yet implemented in GDAL (2.2.1)
+  if ( ogrDriverName == QLatin1String( "GPKG" ) )
   {
     item = new QgsGeoPackageCollectionItem( parentItem, name, path );
   }
-
-  GDALClose( hDataSource );
+  else if ( numLayers > 1 || ogrSupportedDbDriverNames.contains( ogrDriverName ) )
+  {
+    item = new QgsOgrDataCollectionItem( parentItem, name, path );
+  }
+  else
+  {
+    item = dataItemForLayer( parentItem, name, path, hDataSource, 0 );
+  }
+  OGR_DS_Destroy( hDataSource );
   return item;
 }