Skip to content
Permalink
Browse files

Merge pull request #7977 from rouault/fix_ogrdataitems_layername

[OGR provider] Make sure to use layername= syntax in URI of multilayer datasources (fixes #19885)
  • Loading branch information
rouault committed Sep 21, 2018
2 parents eb24bdb + ea2cc36 commit bb40385d3399b75ffc3bb128c20115e06aed671c
@@ -21,7 +21,7 @@ The method createDataItem() is ever called only if capabilities() return non-zer
There are two occasions when createDataItem() is called:
1. to create root items (passed path is empty, parent item is null).
2. to create items in directory structure. For this capabilities have to return at least
of the following: QgsDataProider.Dir or QgsDataProvider.File. Passed path is the file
of the following: QgsDataProvider.Dir or QgsDataProvider.File. Passed path is the file
or directory being inspected, parent item is a valid QgsDirectoryItem

.. versionadded:: 2.10
@@ -1457,15 +1457,10 @@ template <TYPE>

if (tobj == NULL || PyList_SetItem(l, i, tobj) < 0)
{
Py_DECREF(tobj);
Py_DECREF(l);

if (!tobj)
delete t;

return NULL;
}

Py_DECREF(tobj);
}

return l;
@@ -36,7 +36,7 @@ typedef bool handlesDirectoryPath_t( const QString &path ) SIP_SKIP;
* There are two occasions when createDataItem() is called:
* 1. to create root items (passed path is empty, parent item is null).
* 2. to create items in directory structure. For this capabilities have to return at least
* of the following: QgsDataProider::Dir or QgsDataProvider::File. Passed path is the file
* of the following: QgsDataProvider::Dir or QgsDataProvider::File. Passed path is the file
* or directory being inspected, parent item is a valid QgsDirectoryItem
*
* \since QGIS 2.10
@@ -371,7 +371,10 @@ void QgsOgrLayerItem::deleteLayer()

// -------

static QgsOgrLayerItem *dataItemForLayer( QgsDataItem *parentItem, QString name, QString path, GDALDatasetH hDataSource, int layerId, bool isSubLayer = false )
static QgsOgrLayerItem *dataItemForLayer( QgsDataItem *parentItem, QString name,
QString path, GDALDatasetH hDataSource,
int layerId,
bool isSubLayer, bool uniqueNames )
{
OGRLayerH hLayer = GDALDatasetGetLayer( hDataSource, layerId );
OGRFeatureDefnH hDef = OGR_L_GetLayerDefn( hLayer );
@@ -401,16 +404,22 @@ static QgsOgrLayerItem *dataItemForLayer( QgsDataItem *parentItem, QString name,

QString layerUri = path;

if ( name.isEmpty() )
if ( isSubLayer )
{
// we are in a collection
name = QString::fromUtf8( OGR_FD_GetName( hDef ) );
QgsDebugMsg( "OGR layer name : " + name );

layerUri += "|layerid=" + QString::number( layerId );

if ( !uniqueNames )
{
layerUri += "|layerid=" + QString::number( layerId );
}
else
{
layerUri += "|layername=" + name;
}
path += '/' + name;
}
Q_ASSERT( !name.isEmpty() );

QgsDebugMsgLevel( "OGR layer uri : " + layerUri, 2 );

@@ -433,10 +442,26 @@ QVector<QgsDataItem *> QgsOgrDataCollectionItem::createChildren()
return children;
int numLayers = GDALDatasetGetLayerCount( hDataSource.get() );

// Check if layer names are unique, so we can use |layername= in URI
QMap< QString, int > mapLayerNameToCount;
bool uniqueNames = true;
for ( int i = 0; i < numLayers; ++i )
{
OGRLayerH hLayer = GDALDatasetGetLayer( hDataSource.get(), i );
OGRFeatureDefnH hDef = OGR_L_GetLayerDefn( hLayer );
QString layerName = QString::fromUtf8( OGR_FD_GetName( hDef ) );
++mapLayerNameToCount[layerName];
if ( mapLayerNameToCount[layerName] > 1 )
{
uniqueNames = false;
break;
}
}

children.reserve( numLayers );
for ( int i = 0; i < numLayers; ++i )
{
QgsOgrLayerItem *item = dataItemForLayer( this, QString(), mPath, hDataSource.get(), i, true );
QgsOgrLayerItem *item = dataItemForLayer( this, QString(), mPath, hDataSource.get(), i, true, uniqueNames );
children.append( item );
}

@@ -477,13 +502,10 @@ bool QgsOgrDataCollectionItem::createConnection( const QString &name, const QStr

// ---------------------------------------------------------------------------

QGISEXTERN int dataCapabilities()
{
return QgsDataProvider::File | QgsDataProvider::Dir;
}

QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
QgsDataItem *QgsOgrDataItemProvider::createDataItem( const QString &pathIn, QgsDataItem *parentItem )
{
QString path( pathIn );
if ( path.isEmpty() )
return nullptr;

@@ -607,7 +629,7 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
// class
// TODO: add more OGR supported multiple layers formats here!
QStringList ogrSupportedDbLayersExtensions;
ogrSupportedDbLayersExtensions << QStringLiteral( "gpkg" ) << QStringLiteral( "sqlite" ) << QStringLiteral( "db" ) << QStringLiteral( "gdb" );
ogrSupportedDbLayersExtensions << QStringLiteral( "gpkg" ) << QStringLiteral( "sqlite" ) << QStringLiteral( "db" ) << QStringLiteral( "gdb" ) << QStringLiteral( "kml" );
QStringList ogrSupportedDbDriverNames;
ogrSupportedDbDriverNames << QStringLiteral( "GPKG" ) << QStringLiteral( "db" ) << QStringLiteral( "gdb" );

@@ -688,12 +710,12 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
}
else
{
item = dataItemForLayer( parentItem, name, path, hDS.get(), 0 );
item = dataItemForLayer( parentItem, name, path, hDS.get(), 0, false, true );
}
return item;
}

QGISEXTERN bool handlesDirectoryPath( const QString &path )
bool QgsOgrDataItemProvider::handlesDirectoryPath( const QString &path )
{
QFileInfo info( path );
QString suffix = info.suffix().toLower();
@@ -102,5 +102,19 @@ class QgsOgrDataCollectionItem : public QgsDataCollectionItem

};

//! Provider for OGR root data item
class QgsOgrDataItemProvider : public QgsDataItemProvider
{
public:
QString name() override { return QStringLiteral( "OGR" ); }

int capabilities() override { return QgsDataProvider::File | QgsDataProvider::Dir; }

QgsDataItem *createDataItem( const QString &path, QgsDataItem *parentItem ) override;

bool handlesDirectoryPath( const QString &path ) override;
};



#endif // QGSOGRDATAITEMS_H
@@ -3375,10 +3375,10 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
return true;
}


QGISEXTERN QList< QgsDataItemProvider * > *dataItemProviders()
{
QList< QgsDataItemProvider * > *providers = new QList< QgsDataItemProvider * >();
*providers << new QgsOgrDataItemProvider;
*providers << new QgsGeoPackageDataItemProvider;
return providers;
}
@@ -19,7 +19,8 @@

from osgeo import gdal, ogr # NOQA
from qgis.PyQt.QtCore import QVariant
from qgis.core import (QgsFeature, QgsFeatureRequest, QgsField, QgsSettings, QgsDataProvider,
from qgis.core import (QgsApplication,
QgsFeature, QgsFeatureRequest, QgsField, QgsSettings, QgsDataProvider,
QgsVectorDataProvider, QgsVectorLayer, QgsWkbTypes, QgsNetworkAccessManager)
from qgis.testing import start_app, unittest

@@ -404,6 +405,34 @@ def testEditGeoJsonAddFieldAndThenAddFeatures(self):
vl = QgsVectorLayer(datasource, 'test', 'ogr')
self.assertEqual(len(vl.fields()), 2)

def testDataItems(self):

registry = QgsApplication.dataItemProviderRegistry()
ogrprovider = next(provider for provider in registry.providers() if provider.name() == 'OGR')

# Single layer
item = ogrprovider.createDataItem(os.path.join(TEST_DATA_DIR, 'lines.shp'), None)
self.assertTrue(item.uri().endswith('lines.shp'))

# Multiple layer
item = ogrprovider.createDataItem(os.path.join(TEST_DATA_DIR, 'multilayer.kml'), None)
children = item.createChildren()
self.assertEqual(len(children), 2)
self.assertIn('multilayer.kml|layername=Layer1', children[0].uri())
self.assertIn('multilayer.kml|layername=Layer2', children[1].uri())

# Multiple layer (geopackage)
tmpfile = os.path.join(self.basetestpath, 'testDataItems.gpkg')
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('Layer1', geom_type=ogr.wkbPoint)
lyr = ds.CreateLayer('Layer2', geom_type=ogr.wkbPoint)
ds = None
item = ogrprovider.createDataItem(tmpfile, None)
children = item.createChildren()
self.assertEqual(len(children), 2)
self.assertIn('testDataItems.gpkg|layername=Layer1', children[0].uri())
self.assertIn('testDataItems.gpkg|layername=Layer2', children[1].uri())


if __name__ == '__main__':
unittest.main()
@@ -0,0 +1,24 @@
<?xml version="1.0" encoding="UTF-8"?>
<kml xmlns="http://www.opengis.net/kml/2.2" xmlns:kml="http://www.opengis.net/kml/2.2">
<Document>
<name>Test</name>
<Folder id="Layer1">
<name>Layer1</name>
<Placemark>
<name>Placemark1</name>
<Point>
<coordinates>2,49,0</coordinates>
</Point>
</Placemark>
</Folder>
<Folder id="Layer2">
<name>Layer2</name>
<Placemark>
<name>Placemark2</name>
<Point>
<coordinates>3,50,0</coordinates>
</Point>
</Placemark>
</Folder>
</Document>
</kml>

0 comments on commit bb40385

Please sign in to comment.
You can’t perform that action at this time.