Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix potentially ambiguous sublayer names when querying sublayers in
an vsi archive file
  • Loading branch information
nyalldawson committed Jul 12, 2021
1 parent 28e8b4d commit 5b921dc
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 18 deletions.
6 changes: 3 additions & 3 deletions src/core/providers/gdal/qgsgdalprovider.cpp
Expand Up @@ -3660,8 +3660,6 @@ QList<QgsProviderSublayerDetails> QgsGdalProviderMetadata::querySublayers( const
dataset.reset( QgsGdalProviderBase::gdalOpen( gdalUri, GDAL_OF_READONLY ) );
if ( !dataset )
{
if ( CPLGetLastErrorNo() != CPLE_OpenFailed )
QgsDebugMsg( QStringLiteral( "Error querying sublayers: %1 " ).arg( QString::fromUtf8( CPLGetLastErrorMsg() ) ) );
return {};
}

Expand All @@ -3681,7 +3679,9 @@ QList<QgsProviderSublayerDetails> QgsGdalProviderMetadata::querySublayers( const
const QVariantMap parts = decodeUri( uri );
if ( !parts.value( QStringLiteral( "vsiSuffix" ) ).toString().isEmpty() )
{
name = QgsProviderUtils::suggestLayerNameFromFilePath( parts.value( QStringLiteral( "vsiSuffix" ) ).toString() );
name = parts.value( QStringLiteral( "vsiSuffix" ) ).toString();
if ( name.startsWith( '/' ) )
name = name.mid( 1 );
}
else if ( parts.contains( QStringLiteral( "path" ) ) )
{
Expand Down
3 changes: 2 additions & 1 deletion src/core/providers/ogr/qgsogrprovidermetadata.cpp
Expand Up @@ -1187,7 +1187,8 @@ QList<QgsProviderSublayerDetails> QgsOgrProviderMetadata::querySublayers( const
// remove non-matching, unwanted layers
res.erase( std::remove_if( res.begin(), res.end(), [ = ]( const QgsProviderSublayerDetails & sublayer )
{
return sublayer.name() != originalUriLayerName;
const QVariantMap uriParts = decodeUri( sublayer.uri() );
return uriParts.value( QStringLiteral( "layerName" ) ).toString() != originalUriLayerName && sublayer.name() != originalUriLayerName;
} ), res.end() );
}

Expand Down
18 changes: 16 additions & 2 deletions src/core/providers/ogr/qgsogrproviderutils.cpp
Expand Up @@ -2376,7 +2376,14 @@ QList< QgsProviderSublayerDetails > QgsOgrProviderUtils::querySubLayerList( int

QgsProviderSublayerDetails details;
details.setLayerNumber( i );
details.setName( layerName );
if ( parts.value( QStringLiteral( "vsiSuffix" ) ).toString().isEmpty() )
details.setName( layerName );
else
{
if ( parts.value( QStringLiteral( "layerName" ) ).toString().isEmpty() )
parts.insert( QStringLiteral( "layerName" ), layerName );
details.setName( parts.value( QStringLiteral( "vsiSuffix" ) ).toString().mid( 1 ) );
}
details.setFeatureCount( layerFeatureCount );
details.setWkbType( QgsOgrUtils::ogrGeometryTypeToQgsWkbType( layerGeomType ) );
details.setGeometryColumnName( geometryColumnName );
Expand Down Expand Up @@ -2458,7 +2465,14 @@ QList< QgsProviderSublayerDetails > QgsOgrProviderUtils::querySubLayerList( int

QgsProviderSublayerDetails details;
details.setLayerNumber( i );
details.setName( layerName );
if ( parts.value( QStringLiteral( "vsiSuffix" ) ).toString().isEmpty() )
details.setName( layerName );
else
{
if ( parts.value( QStringLiteral( "layerName" ) ).toString().isEmpty() )
parts.insert( QStringLiteral( "layerName" ), layerName );
details.setName( parts.value( QStringLiteral( "vsiSuffix" ) ).toString().mid( 1 ) );
}
details.setFeatureCount( fCount.value( countIt.key() ) );
details.setWkbType( QgsOgrUtils::ogrGeometryTypeToQgsWkbType( ( bIs25D ) ? wkbSetZ( countIt.key() ) : countIt.key() ) );
details.setGeometryColumnName( geometryColumnName );
Expand Down
12 changes: 6 additions & 6 deletions tests/src/core/testqgsgdalprovider.cpp
Expand Up @@ -551,7 +551,7 @@ void TestQgsGdalProvider::testGdalProviderQuerySublayers()
res = gdalMetadata->querySublayers( QStringLiteral( TEST_DATA_DIR ) + "/zip/landsat_b1.zip" );
QCOMPARE( res.count(), 1 );
QCOMPARE( res.at( 0 ).layerNumber(), 1 );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b1" ) );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b1.tif" ) );
QCOMPARE( res.at( 0 ).description(), QString() );
QCOMPARE( res.at( 0 ).uri(), QStringLiteral( "/vsizip/%1/zip/landsat_b1.zip/landsat_b1.tif" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 0 ).providerKey(), QStringLiteral( "gdal" ) );
Expand All @@ -563,23 +563,23 @@ void TestQgsGdalProvider::testGdalProviderQuerySublayers()
res = gdalMetadata->querySublayers( QStringLiteral( TEST_DATA_DIR ) + "/zip/testtar.tgz" );
QCOMPARE( res.count(), 3 );
QCOMPARE( res.at( 0 ).layerNumber(), 1 );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b2" ) );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "folder/folder2/landsat_b2.tif" ) );
QCOMPARE( res.at( 0 ).description(), QString() );
QCOMPARE( res.at( 0 ).uri(), QStringLiteral( "/vsitar/%1/zip/testtar.tgz/folder/folder2/landsat_b2.tif" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 0 ).providerKey(), QStringLiteral( "gdal" ) );
QCOMPARE( res.at( 0 ).type(), QgsMapLayerType::RasterLayer );
rl.reset( qgis::down_cast< QgsRasterLayer * >( res.at( 0 ).toLayer( options ) ) );
QVERIFY( rl->isValid() );
QCOMPARE( res.at( 1 ).layerNumber(), 1 );
QCOMPARE( res.at( 1 ).name(), QStringLiteral( "landsat_b1" ) );
QCOMPARE( res.at( 1 ).name(), QStringLiteral( "landsat_b1.tif" ) );
QCOMPARE( res.at( 1 ).description(), QString() );
QCOMPARE( res.at( 1 ).uri(), QStringLiteral( "/vsitar/%1/zip/testtar.tgz/landsat_b1.tif" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 1 ).providerKey(), QStringLiteral( "gdal" ) );
QCOMPARE( res.at( 1 ).type(), QgsMapLayerType::RasterLayer );
rl.reset( qgis::down_cast< QgsRasterLayer * >( res.at( 1 ).toLayer( options ) ) );
QVERIFY( rl->isValid() );
QCOMPARE( res.at( 2 ).layerNumber(), 1 );
QCOMPARE( res.at( 2 ).name(), QStringLiteral( "landsat_b1" ) );
QCOMPARE( res.at( 2 ).name(), QStringLiteral( "landsat_b1.vrt" ) );
QCOMPARE( res.at( 2 ).description(), QString() );
QCOMPARE( res.at( 2 ).uri(), QStringLiteral( "/vsitar/%1/zip/testtar.tgz/landsat_b1.vrt" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 2 ).providerKey(), QStringLiteral( "gdal" ) );
Expand All @@ -591,7 +591,7 @@ void TestQgsGdalProvider::testGdalProviderQuerySublayers()
res = gdalMetadata->querySublayers( QStringLiteral( "/vsitar/" ) + QStringLiteral( TEST_DATA_DIR ) + "/zip/testtar.tgz/folder/folder2/landsat_b2.tif" );
QCOMPARE( res.count(), 1 );
QCOMPARE( res.at( 0 ).layerNumber(), 1 );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b2" ) );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "folder/folder2/landsat_b2.tif" ) );
QCOMPARE( res.at( 0 ).description(), QString() );
QCOMPARE( res.at( 0 ).uri(), QStringLiteral( "/vsitar/%1/zip/testtar.tgz/folder/folder2/landsat_b2.tif" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 0 ).providerKey(), QStringLiteral( "gdal" ) );
Expand All @@ -601,7 +601,7 @@ void TestQgsGdalProvider::testGdalProviderQuerySublayers()
res = gdalMetadata->querySublayers( QStringLiteral( "/vsitar/" ) + QStringLiteral( TEST_DATA_DIR ) + "/zip/testtar.tgz/landsat_b1.tif" );
QCOMPARE( res.count(), 1 );
QCOMPARE( res.at( 0 ).layerNumber(), 1 );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b1" ) );
QCOMPARE( res.at( 0 ).name(), QStringLiteral( "landsat_b1.tif" ) );
QCOMPARE( res.at( 0 ).description(), QString() );
QCOMPARE( res.at( 0 ).uri(), QStringLiteral( "/vsitar/%1/zip/testtar.tgz/landsat_b1.tif" ).arg( QStringLiteral( TEST_DATA_DIR ) ) );
QCOMPARE( res.at( 0 ).providerKey(), QStringLiteral( "gdal" ) );
Expand Down
12 changes: 6 additions & 6 deletions tests/src/python/test_provider_ogr.py
Expand Up @@ -1534,7 +1534,7 @@ def test_provider_sublayer_details(self):
res = metadata.querySublayers(os.path.join(TEST_DATA_DIR, 'zip', 'points2.zip'))
self.assertEqual(len(res), 1)
self.assertEqual(res[0].layerNumber(), 0)
self.assertEqual(res[0].name(), "points")
self.assertEqual(res[0].name(), "points.shp")
self.assertEqual(res[0].description(), '')
self.assertEqual(res[0].uri(), '/vsizip/' + TEST_DATA_DIR + "/zip/points2.zip/points.shp|layername=points")
self.assertEqual(res[0].providerKey(), "ogr")
Expand All @@ -1550,7 +1550,7 @@ def test_provider_sublayer_details(self):
res = metadata.querySublayers(os.path.join(TEST_DATA_DIR, 'zip', 'testtar.tgz'))
self.assertEqual(len(res), 2)
self.assertEqual(res[0].layerNumber(), 0)
self.assertEqual(res[0].name(), "points")
self.assertEqual(res[0].name(), "folder/points.geojson")
self.assertEqual(res[0].description(), '')
self.assertEqual(res[0].uri(), '/vsitar/' + TEST_DATA_DIR + "/zip/testtar.tgz/folder/points.geojson|layername=points")
self.assertEqual(res[0].providerKey(), "ogr")
Expand All @@ -1562,7 +1562,7 @@ def test_provider_sublayer_details(self):
self.assertTrue(vl.isValid())
self.assertEqual(vl.wkbType(), QgsWkbTypes.Point)
self.assertEqual(res[1].layerNumber(), 0)
self.assertEqual(res[1].name(), "points")
self.assertEqual(res[1].name(), "points.shp")
self.assertEqual(res[1].description(), '')
self.assertEqual(res[1].uri(), '/vsitar/' + TEST_DATA_DIR + "/zip/testtar.tgz/points.shp|layername=points")
self.assertEqual(res[1].providerKey(), "ogr")
Expand All @@ -1578,7 +1578,7 @@ def test_provider_sublayer_details(self):
res = metadata.querySublayers('/vsitar/' + os.path.join(TEST_DATA_DIR, 'zip', 'testtar.tgz') + '/folder/points.geojson')
self.assertEqual(len(res), 1)
self.assertEqual(res[0].layerNumber(), 0)
self.assertEqual(res[0].name(), "points")
self.assertEqual(res[0].name(), "folder/points.geojson")
self.assertEqual(res[0].description(), '')
self.assertEqual(res[0].uri(), '/vsitar/' + TEST_DATA_DIR + "/zip/testtar.tgz/folder/points.geojson|layername=points")
self.assertEqual(res[0].providerKey(), "ogr")
Expand All @@ -1594,7 +1594,7 @@ def test_provider_sublayer_details(self):
res = metadata.querySublayers('/vsitar/' + os.path.join(TEST_DATA_DIR, 'zip', 'testtar.tgz') + '/points.shp')
self.assertEqual(len(res), 1)
self.assertEqual(res[0].layerNumber(), 0)
self.assertEqual(res[0].name(), "points")
self.assertEqual(res[0].name(), "points.shp")
self.assertEqual(res[0].description(), '')
self.assertEqual(res[0].uri(), '/vsitar/' + TEST_DATA_DIR + "/zip/testtar.tgz/points.shp|layername=points")
self.assertEqual(res[0].providerKey(), "ogr")
Expand All @@ -1611,7 +1611,7 @@ def test_provider_sublayer_details(self):
res = metadata.querySublayers('/vsitar/' + os.path.join(TEST_DATA_DIR, 'zip', 'testtar.tgz') + '/points.shp|layername=points')
self.assertEqual(len(res), 1)
self.assertEqual(res[0].layerNumber(), 0)
self.assertEqual(res[0].name(), "points")
self.assertEqual(res[0].name(), "points.shp")
self.assertEqual(res[0].description(), '')
self.assertEqual(res[0].uri(), '/vsitar/' + TEST_DATA_DIR + "/zip/testtar.tgz/points.shp|layername=points")
self.assertEqual(res[0].providerKey(), "ogr")
Expand Down

0 comments on commit 5b921dc

Please sign in to comment.