Fix handling of ogr sublayers with ':' in their name

Unlikely to happen, but it does occur with some layers coming from processing models. In any case we want QGIS to be super-tolerant of corner cases like this!
qgis · Nov 26, 2017 · 144e9a2 · 144e9a2
1 parent 7451422
commit 144e9a2
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 11 deletions.
diff --git a/python/core/qgsdataprovider.sip b/python/core/qgsdataprovider.sip
@@ -164,9 +164,17 @@ class QgsDataProvider : QObject
 
  Sub-layers are used when the provider's source can combine layers
  it knows about in some way before it hands them off to the provider.
+
+.. seealso:: SUBLAYER_SEPARATOR
  :rtype: list of str
 %End
 
+    static QString SUBLAYER_SEPARATOR;
+%Docstring
+ String sequence used for separating components of sublayers strings.
+.. seealso:: subLayers()
+.. versionadded:: 3.0
+%End
 
     virtual QStringList subLayerStyles() const;
 %Docstring

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -4229,7 +4229,7 @@ bool QgisApp::addVectorLayers( const QStringList &layerQStringList, const QStrin
       else if ( !sublayers.isEmpty() ) // there is 1 layer of data available
       {
         //set friendly name for datasources with only one layer
-        QStringList elements = sublayers.at( 0 ).split( ':' );
+        QStringList elements = sublayers.at( 0 ).split( QgsDataProvider::SUBLAYER_SEPARATOR );
         QString subLayerNameFormatted = elements.size() >= 2 ? QgsMapLayer::formatLayerName( elements.at( 1 ) ) : QString();
 
         if ( elements.size() >= 4 && layer->name().compare( elements.at( 1 ), Qt::CaseInsensitive ) != 0
@@ -4442,7 +4442,7 @@ void QgisApp::askUserForGDALSublayers( QgsRasterLayer *layer )
     else
     {
       // remove driver name and file name
-      name.remove( name.split( ':' )[0] );
+      name.remove( name.split( QgsDataProvider::SUBLAYER_SEPARATOR )[0] );
       name.remove( path );
     }
     // remove any : or " left over
@@ -4590,7 +4590,7 @@ void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer )
     // OGR provider returns items in this format:
     // <layer_index>:<name>:<feature_count>:<geom_type>
 
-    QStringList elements = sublayer.split( QStringLiteral( ":" ) );
+    QStringList elements = sublayer.split( QgsDataProvider::SUBLAYER_SEPARATOR );
     // merge back parts of the name that may have been split
     while ( elements.size() > 5 )
     {
@@ -10024,7 +10024,7 @@ QgsVectorLayer *QgisApp::addVectorLayer( const QString &vectorLayerPath, const Q
       QStringList sublayers = layer->dataProvider()->subLayers();
       if ( !sublayers.isEmpty() )
       {
-        QStringList elements = sublayers.at( 0 ).split( ':' );
+        QStringList elements = sublayers.at( 0 ).split( QgsDataProvider::SUBLAYER_SEPARATOR );
         QString subLayerNameFormatted = elements.size() >= 2 ? QgsMapLayer::formatLayerName( elements.at( 1 ) ) : QString();
 
         if ( elements.size() >= 4 && layer->name().compare( elements.at( 1 ), Qt::CaseInsensitive ) != 0

diff --git a/src/core/qgsdataprovider.cpp b/src/core/qgsdataprovider.cpp
@@ -15,6 +15,7 @@
 
 #include "qgsdataprovider.h"
 
+QString QgsDataProvider::SUBLAYER_SEPARATOR = QString( "!!::!!" );
 
 void QgsDataProvider::setProviderProperty( QgsDataProvider::ProviderProperty property, const QVariant &value )
 {

diff --git a/src/core/qgsdataprovider.h b/src/core/qgsdataprovider.h
@@ -218,12 +218,20 @@ class CORE_EXPORT QgsDataProvider : public QObject
      *
      * Sub-layers are used when the provider's source can combine layers
      * it knows about in some way before it hands them off to the provider.
+     *
+     * \see SUBLAYER_SEPARATOR
      */
     virtual QStringList subLayers() const
     {
       return QStringList();  // Empty
     }
 
+    /**
+     * String sequence used for separating components of sublayers strings.
+     * \see subLayers()
+     * \since QGIS 3.0
+     */
+    static QString SUBLAYER_SEPARATOR;
 
     /**
      * Sub-layer styles for each sub-layer handled by this provider,

diff --git a/src/providers/gdal/qgsgdaldataitems.cpp b/src/providers/gdal/qgsgdaldataitems.cpp
@@ -89,7 +89,7 @@ QVector<QgsDataItem *> QgsGdalLayerItem::createChildren()
       else
       {
         // remove driver name and file name and initial ':'
-        name.remove( name.split( ':' )[0] + ':' );
+        name.remove( name.split( QgsDataProvider::SUBLAYER_SEPARATOR )[0] + ':' );
         name.remove( mPath );
       }
       // remove any : or " left over

diff --git a/src/providers/ogr/qgsogrdataitems.cpp b/src/providers/ogr/qgsogrdataitems.cpp
@@ -163,7 +163,7 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
     int prevIdx = -1;
     for ( const QString &descriptor : subLayersList )
     {
-      QStringList pieces = descriptor.split( ':' );
+      QStringList pieces = descriptor.split( QgsDataProvider::SUBLAYER_SEPARATOR );
       int idx = pieces[0].toInt();
       subLayersMap.insert( idx, pieces );
       if ( pieces.count() >= 4 && idx != prevIdx )
@@ -238,7 +238,7 @@ QList<QgsOgrDbLayerInfo *> QgsOgrLayerItem::subLayers( const QString &path, cons
     const QStringList layers( rlayer.dataProvider()->subLayers( ) );
     for ( const QString &uri : layers )
     {
-      QStringList pieces = uri.split( ':' );
+      QStringList pieces = uri.split( QgsDataProvider::SUBLAYER_SEPARATOR );
       QString name = pieces.value( pieces.length() - 1 );
       QgsDebugMsgLevel( QStringLiteral( "Adding GeoPackage Raster item %1 %2 %3" ).arg( name, uri ), 3 );
       children.append( new QgsOgrDbLayerInfo( path, uri, name, QStringLiteral( "" ), QStringLiteral( "Raster" ), QgsLayerItem::LayerType::Raster ) );

diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp
@@ -749,7 +749,14 @@ void QgsOgrProvider::addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer
 
     QString geom = ogrWkbGeometryTypeName( layerGeomType );
 
-    mSubLayerList << QStringLiteral( "%1:%2:%3:%4:%5" ).arg( i ).arg( layerName, layerFeatureCount == -1 ? tr( "Unknown" ) : QString::number( layerFeatureCount ), geom, geometryColumnName );
+    QStringList parts = QStringList()
+                        << QString::number( i )
+                        << layerName
+                        << ( layerFeatureCount == -1 ? tr( "Unknown" ) : QString::number( layerFeatureCount ) )
+                        << geom
+                        << geometryColumnName;
+
+    mSubLayerList << parts.join( QgsDataProvider::SUBLAYER_SEPARATOR );
   }
   else
   {
@@ -813,12 +820,18 @@ void QgsOgrProvider::addSubLayerDetailsToSubLayerList( int i, QgsOgrLayer *layer
     {
       QString geom = ogrWkbGeometryTypeName( ( bIs25D ) ? wkbSetZ( countIt.key() ) : countIt.key() );
 
-      QString sl = QStringLiteral( "%1:%2:%3:%4:%5" ).arg( i ).arg( layerName ).arg( fCount.value( countIt.key() ) ).arg( geom, geometryColumnName );
+      QStringList parts = QStringList()
+                          << QString::number( i )
+                          << layerName
+                          << QString::number( fCount.value( countIt.key() ) )
+                          << geom
+                          << geometryColumnName;
+
+      QString sl = parts.join( QgsDataProvider::SUBLAYER_SEPARATOR );
       QgsDebugMsg( "sub layer: " + sl );
       mSubLayerList << sl;
     }
   }
-
 }
 
 QStringList QgsOgrProvider::subLayers() const

diff --git a/tests/src/core/testqgsrastersublayer.cpp b/tests/src/core/testqgsrastersublayer.cpp
@@ -138,7 +138,7 @@ void TestQgsRasterSubLayer::subLayersList()
     Q_FOREACH ( const QString &s, mpRasterLayer->subLayers() )
     {
       qDebug() << "sublayer: " << s;
-      sublayers << s.split( ':' ).last();
+      sublayers << s.split( QgsDataProvider::SUBLAYER_SEPARATOR ).last();
     }
     qDebug() << "sublayers: " << sublayers.join( QStringLiteral( "," ) );
     mReport += QStringLiteral( "sublayers:<br>%1<br>\n" ).arg( sublayers.join( QStringLiteral( "<br>" ) ) );