Skip to content

Commit bb40385

Browse files
rouaultrouault
authoredSep 21, 2018
Merge pull request #7977 from rouault/fix_ogrdataitems_layername
[OGR provider] Make sure to use layername= syntax in URI of multilayer datasources (fixes #19885)
2 parents eb24bdb + ea2cc36 commit bb40385

File tree

8 files changed

+108
-24
lines changed

8 files changed

+108
-24
lines changed
 

‎python/core/auto_generated/qgsdataitemprovider.sip.in

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ The method createDataItem() is ever called only if capabilities() return non-zer
2121
There are two occasions when createDataItem() is called:
2222
1. to create root items (passed path is empty, parent item is null).
2323
2. to create items in directory structure. For this capabilities have to return at least
24-
of the following: QgsDataProider.Dir or QgsDataProvider.File. Passed path is the file
24+
of the following: QgsDataProvider.Dir or QgsDataProvider.File. Passed path is the file
2525
or directory being inspected, parent item is a valid QgsDirectoryItem
2626

2727
.. versionadded:: 2.10

‎python/core/conversions.sip

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,15 +1457,10 @@ template <TYPE>
14571457

14581458
if (tobj == NULL || PyList_SetItem(l, i, tobj) < 0)
14591459
{
1460+
Py_DECREF(tobj);
14601461
Py_DECREF(l);
1461-
1462-
if (!tobj)
1463-
delete t;
1464-
14651462
return NULL;
14661463
}
1467-
1468-
Py_DECREF(tobj);
14691464
}
14701465

14711466
return l;

‎src/core/qgsdataitemprovider.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ typedef bool handlesDirectoryPath_t( const QString &path ) SIP_SKIP;
3636
* There are two occasions when createDataItem() is called:
3737
* 1. to create root items (passed path is empty, parent item is null).
3838
* 2. to create items in directory structure. For this capabilities have to return at least
39-
* of the following: QgsDataProider::Dir or QgsDataProvider::File. Passed path is the file
39+
* of the following: QgsDataProvider::Dir or QgsDataProvider::File. Passed path is the file
4040
* or directory being inspected, parent item is a valid QgsDirectoryItem
4141
*
4242
* \since QGIS 2.10

‎src/providers/ogr/qgsogrdataitems.cpp

Lines changed: 36 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,10 @@ void QgsOgrLayerItem::deleteLayer()
371371

372372
// -------
373373

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

402405
QString layerUri = path;
403406

404-
if ( name.isEmpty() )
407+
if ( isSubLayer )
405408
{
406409
// we are in a collection
407410
name = QString::fromUtf8( OGR_FD_GetName( hDef ) );
408411
QgsDebugMsg( "OGR layer name : " + name );
409-
410-
layerUri += "|layerid=" + QString::number( layerId );
411-
412+
if ( !uniqueNames )
413+
{
414+
layerUri += "|layerid=" + QString::number( layerId );
415+
}
416+
else
417+
{
418+
layerUri += "|layername=" + name;
419+
}
412420
path += '/' + name;
413421
}
422+
Q_ASSERT( !name.isEmpty() );
414423

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

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

445+
// Check if layer names are unique, so we can use |layername= in URI
446+
QMap< QString, int > mapLayerNameToCount;
447+
bool uniqueNames = true;
448+
for ( int i = 0; i < numLayers; ++i )
449+
{
450+
OGRLayerH hLayer = GDALDatasetGetLayer( hDataSource.get(), i );
451+
OGRFeatureDefnH hDef = OGR_L_GetLayerDefn( hLayer );
452+
QString layerName = QString::fromUtf8( OGR_FD_GetName( hDef ) );
453+
++mapLayerNameToCount[layerName];
454+
if ( mapLayerNameToCount[layerName] > 1 )
455+
{
456+
uniqueNames = false;
457+
break;
458+
}
459+
}
460+
436461
children.reserve( numLayers );
437462
for ( int i = 0; i < numLayers; ++i )
438463
{
439-
QgsOgrLayerItem *item = dataItemForLayer( this, QString(), mPath, hDataSource.get(), i, true );
464+
QgsOgrLayerItem *item = dataItemForLayer( this, QString(), mPath, hDataSource.get(), i, true, uniqueNames );
440465
children.append( item );
441466
}
442467

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

478503
// ---------------------------------------------------------------------------
479504

480-
QGISEXTERN int dataCapabilities()
481-
{
482-
return QgsDataProvider::File | QgsDataProvider::Dir;
483-
}
484505

485-
QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
506+
QgsDataItem *QgsOgrDataItemProvider::createDataItem( const QString &pathIn, QgsDataItem *parentItem )
486507
{
508+
QString path( pathIn );
487509
if ( path.isEmpty() )
488510
return nullptr;
489511

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

@@ -688,12 +710,12 @@ QGISEXTERN QgsDataItem *dataItem( QString path, QgsDataItem *parentItem )
688710
}
689711
else
690712
{
691-
item = dataItemForLayer( parentItem, name, path, hDS.get(), 0 );
713+
item = dataItemForLayer( parentItem, name, path, hDS.get(), 0, false, true );
692714
}
693715
return item;
694716
}
695717

696-
QGISEXTERN bool handlesDirectoryPath( const QString &path )
718+
bool QgsOgrDataItemProvider::handlesDirectoryPath( const QString &path )
697719
{
698720
QFileInfo info( path );
699721
QString suffix = info.suffix().toLower();

‎src/providers/ogr/qgsogrdataitems.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,5 +102,19 @@ class QgsOgrDataCollectionItem : public QgsDataCollectionItem
102102

103103
};
104104

105+
//! Provider for OGR root data item
106+
class QgsOgrDataItemProvider : public QgsDataItemProvider
107+
{
108+
public:
109+
QString name() override { return QStringLiteral( "OGR" ); }
110+
111+
int capabilities() override { return QgsDataProvider::File | QgsDataProvider::Dir; }
112+
113+
QgsDataItem *createDataItem( const QString &path, QgsDataItem *parentItem ) override;
114+
115+
bool handlesDirectoryPath( const QString &path ) override;
116+
};
117+
118+
105119

106120
#endif // QGSOGRDATAITEMS_H

‎src/providers/ogr/qgsogrprovider.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3375,10 +3375,10 @@ QGISEXTERN bool createEmptyDataSource( const QString &uri,
33753375
return true;
33763376
}
33773377

3378-
33793378
QGISEXTERN QList< QgsDataItemProvider * > *dataItemProviders()
33803379
{
33813380
QList< QgsDataItemProvider * > *providers = new QList< QgsDataItemProvider * >();
3381+
*providers << new QgsOgrDataItemProvider;
33823382
*providers << new QgsGeoPackageDataItemProvider;
33833383
return providers;
33843384
}

‎tests/src/python/test_provider_ogr.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@
1919

2020
from osgeo import gdal, ogr # NOQA
2121
from qgis.PyQt.QtCore import QVariant
22-
from qgis.core import (QgsFeature, QgsFeatureRequest, QgsField, QgsSettings, QgsDataProvider,
22+
from qgis.core import (QgsApplication,
23+
QgsFeature, QgsFeatureRequest, QgsField, QgsSettings, QgsDataProvider,
2324
QgsVectorDataProvider, QgsVectorLayer, QgsWkbTypes, QgsNetworkAccessManager)
2425
from qgis.testing import start_app, unittest
2526

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

408+
def testDataItems(self):
409+
410+
registry = QgsApplication.dataItemProviderRegistry()
411+
ogrprovider = next(provider for provider in registry.providers() if provider.name() == 'OGR')
412+
413+
# Single layer
414+
item = ogrprovider.createDataItem(os.path.join(TEST_DATA_DIR, 'lines.shp'), None)
415+
self.assertTrue(item.uri().endswith('lines.shp'))
416+
417+
# Multiple layer
418+
item = ogrprovider.createDataItem(os.path.join(TEST_DATA_DIR, 'multilayer.kml'), None)
419+
children = item.createChildren()
420+
self.assertEqual(len(children), 2)
421+
self.assertIn('multilayer.kml|layername=Layer1', children[0].uri())
422+
self.assertIn('multilayer.kml|layername=Layer2', children[1].uri())
423+
424+
# Multiple layer (geopackage)
425+
tmpfile = os.path.join(self.basetestpath, 'testDataItems.gpkg')
426+
ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile)
427+
lyr = ds.CreateLayer('Layer1', geom_type=ogr.wkbPoint)
428+
lyr = ds.CreateLayer('Layer2', geom_type=ogr.wkbPoint)
429+
ds = None
430+
item = ogrprovider.createDataItem(tmpfile, None)
431+
children = item.createChildren()
432+
self.assertEqual(len(children), 2)
433+
self.assertIn('testDataItems.gpkg|layername=Layer1', children[0].uri())
434+
self.assertIn('testDataItems.gpkg|layername=Layer2', children[1].uri())
435+
407436

408437
if __name__ == '__main__':
409438
unittest.main()

‎tests/testdata/multilayer.kml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<kml xmlns="http://www.opengis.net/kml/2.2" xmlns:kml="http://www.opengis.net/kml/2.2">
3+
<Document>
4+
<name>Test</name>
5+
<Folder id="Layer1">
6+
<name>Layer1</name>
7+
<Placemark>
8+
<name>Placemark1</name>
9+
<Point>
10+
<coordinates>2,49,0</coordinates>
11+
</Point>
12+
</Placemark>
13+
</Folder>
14+
<Folder id="Layer2">
15+
<name>Layer2</name>
16+
<Placemark>
17+
<name>Placemark2</name>
18+
<Point>
19+
<coordinates>3,50,0</coordinates>
20+
</Point>
21+
</Placemark>
22+
</Folder>
23+
</Document>
24+
</kml>

0 commit comments

Comments
 (0)
Please sign in to comment.