From 144e9a2e457d48ca7680d6bbc7a0c74b59a6b2ad Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sun, 26 Nov 2017 12:23:32 +1000 Subject: [PATCH 1/5] 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! --- python/core/qgsdataprovider.sip | 8 ++++++++ src/app/qgisapp.cpp | 8 ++++---- src/core/qgsdataprovider.cpp | 1 + src/core/qgsdataprovider.h | 8 ++++++++ src/providers/gdal/qgsgdaldataitems.cpp | 2 +- src/providers/ogr/qgsogrdataitems.cpp | 4 ++-- src/providers/ogr/qgsogrprovider.cpp | 19 ++++++++++++++++--- tests/src/core/testqgsrastersublayer.cpp | 2 +- 8 files changed, 41 insertions(+), 11 deletions(-) diff --git a/python/core/qgsdataprovider.sip b/python/core/qgsdataprovider.sip index 001f12f1bf82..92a014a044a7 100644 --- 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 index ca94804c47c6..58b0b8b7f608 100644 --- 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: // ::: - 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 index fd68008c4a7e..f660cdcc4d51 100644 --- 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 index f5e2f57de49b..3ac1a0425d90 100644 --- 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 index 2a6eb2ce2a8b..90fa77007ae6 100644 --- a/src/providers/gdal/qgsgdaldataitems.cpp +++ b/src/providers/gdal/qgsgdaldataitems.cpp @@ -89,7 +89,7 @@ QVector 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 index e4919d4acb08..ad04e0ab42fe 100644 --- a/src/providers/ogr/qgsogrdataitems.cpp +++ b/src/providers/ogr/qgsogrdataitems.cpp @@ -163,7 +163,7 @@ QList 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 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 index 115394e09fd0..7741183c60bf 100644 --- 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 index 2aba9585b108..d564feb42268 100644 --- 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:
%1
\n" ).arg( sublayers.join( QStringLiteral( "
" ) ) ); From 148380abef1548978b4787e201b35a3691d1467c Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sun, 26 Nov 2017 12:29:58 +1000 Subject: [PATCH 2/5] Fix inefficient QString splitting QString::split with single characters is about 10x faster than QString::split using QStrings --- src/app/qgsoptions.cpp | 4 ++-- src/app/qgsprojectproperties.cpp | 4 ++-- src/app/qgsversionmigration.cpp | 6 +++--- src/gui/qgsmetadatawidget.cpp | 2 +- src/plugins/grass/qgsgrassmapcalc.cpp | 6 +++--- src/plugins/grass/qgsgrassmodule.cpp | 2 +- src/plugins/grass/qgsgrassmoduleinput.cpp | 2 +- src/plugins/grass/qgsgrassmoduleoptions.cpp | 2 +- src/plugins/grass/qgsgrassmoduleparam.cpp | 10 +++++----- src/providers/arcgisrest/qgsafsprovider.cpp | 2 +- src/providers/arcgisrest/qgsamsdataitems.cpp | 2 +- src/providers/arcgisrest/qgsamssourceselect.cpp | 2 +- src/providers/arcgisrest/qgsarcgisrestutils.cpp | 2 +- src/providers/grass/qgsgrass.cpp | 8 ++++---- src/providers/grass/qgsgrassprovidermodule.cpp | 4 ++-- src/providers/grass/qgsgrassrasterprovider.cpp | 2 +- src/providers/wfs/qgswfsprovider.cpp | 2 +- src/server/services/wfs/qgswfsparameters.cpp | 2 +- src/server/services/wfs/qgswfstransaction.cpp | 2 +- src/server/services/wfs/qgswfstransaction_1_0_0.cpp | 2 +- src/server/services/wms/qgswmsparameters.cpp | 8 ++++---- tests/src/core/testqgsmaprendererjob.cpp | 2 +- 22 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/app/qgsoptions.cpp b/src/app/qgsoptions.cpp index 578a04c5813a..f0a508187e63 100644 --- a/src/app/qgsoptions.cpp +++ b/src/app/qgsoptions.cpp @@ -2346,11 +2346,11 @@ void QgsOptions::addScaleToScaleList( QListWidgetItem *newItem ) QListWidgetItem *duplicateItem = mListGlobalScales->findItems( newItem->text(), Qt::MatchExactly ).value( 0 ); delete duplicateItem; - int newDenominator = newItem->text().split( QStringLiteral( ":" ) ).value( 1 ).toInt(); + int newDenominator = newItem->text().split( ':' ).value( 1 ).toInt(); int i; for ( i = 0; i < mListGlobalScales->count(); i++ ) { - int denominator = mListGlobalScales->item( i )->text().split( QStringLiteral( ":" ) ).value( 1 ).toInt(); + int denominator = mListGlobalScales->item( i )->text().split( ':' ).value( 1 ).toInt(); if ( newDenominator > denominator ) break; } diff --git a/src/app/qgsprojectproperties.cpp b/src/app/qgsprojectproperties.cpp index a67b9f38db9f..a7e5b50a516c 100644 --- a/src/app/qgsprojectproperties.cpp +++ b/src/app/qgsprojectproperties.cpp @@ -1992,11 +1992,11 @@ void QgsProjectProperties::addScaleToScaleList( QListWidgetItem *newItem ) QListWidgetItem *duplicateItem = lstScales->findItems( newItem->text(), Qt::MatchExactly ).value( 0 ); delete duplicateItem; - int newDenominator = newItem->text().split( QStringLiteral( ":" ) ).value( 1 ).toInt(); + int newDenominator = newItem->text().split( ':' ).value( 1 ).toInt(); int i; for ( i = 0; i < lstScales->count(); i++ ) { - int denominator = lstScales->item( i )->text().split( QStringLiteral( ":" ) ).value( 1 ).toInt(); + int denominator = lstScales->item( i )->text().split( ':' ).value( 1 ).toInt(); if ( newDenominator > denominator ) break; } diff --git a/src/app/qgsversionmigration.cpp b/src/app/qgsversionmigration.cpp index 7b45746b9f6b..92473921c133 100644 --- a/src/app/qgsversionmigration.cpp +++ b/src/app/qgsversionmigration.cpp @@ -201,7 +201,7 @@ QgsError Qgs2To3Migration::migrateSettings() if ( line.isEmpty() ) continue; - QStringList parts = line.split( ";" ); + const QStringList parts = line.split( ';' ); Q_ASSERT_X( parts.count() == 2, "QgsVersionMigration::migrateSettings()", "Can't split line in 2 parts." ); @@ -327,10 +327,10 @@ QPair Qgs2To3Migration::transformKey( QString fullOldKey, QStr if ( newKeyPart.endsWith( "/*" ) ) { - QStringList newKeyparts = newKeyPart.split( "/" ); + QStringList newKeyparts = newKeyPart.split( '/' ); // Throw away the * newKeyparts.removeLast(); - QStringList oldKeyParts = fullOldKey.split( "/" ); + QStringList oldKeyParts = fullOldKey.split( '/' ); for ( int i = 0; i < newKeyparts.count(); ++i ) { oldKeyParts.replace( i, newKeyparts.at( i ) ); diff --git a/src/gui/qgsmetadatawidget.cpp b/src/gui/qgsmetadatawidget.cpp index 46b73b401bbd..6235b0964406 100644 --- a/src/gui/qgsmetadatawidget.cpp +++ b/src/gui/qgsmetadatawidget.cpp @@ -715,7 +715,7 @@ void QgsMetadataWidget::updatePanel() const if ( !categories.isEmpty() ) { int row = categories.at( 0 )->row(); - mCategoriesModel->setStringList( tabKeywords->item( row, 1 )->text().split( QStringLiteral( "," ) ) ); + mCategoriesModel->setStringList( tabKeywords->item( row, 1 )->text().split( ',' ) ); } else { diff --git a/src/plugins/grass/qgsgrassmapcalc.cpp b/src/plugins/grass/qgsgrassmapcalc.cpp index 0333cc9d158f..be048fa996b5 100644 --- a/src/plugins/grass/qgsgrassmapcalc.cpp +++ b/src/plugins/grass/qgsgrassmapcalc.cpp @@ -512,7 +512,7 @@ QStringList QgsGrassMapcalc::checkRegion() struct Cell_head window; - QStringList mm = obj->value().split( QStringLiteral( "@" ) ); + QStringList mm = obj->value().split( '@' ); if ( mm.size() < 1 ) continue; @@ -571,7 +571,7 @@ bool QgsGrassMapcalc::inputRegion( struct Cell_head *window, QgsCoordinateRefere struct Cell_head mapWindow; - QStringList mm = obj->value().split( QStringLiteral( "@" ) ); + QStringList mm = obj->value().split( '@' ); if ( mm.size() < 1 ) continue; @@ -652,7 +652,7 @@ void QgsGrassMapcalc::setOption() { case QgsGrassMapcalcObject::Map : { - QStringList mapMapset = mObject->value().split( QStringLiteral( "@" ) ); + QStringList mapMapset = mObject->value().split( '@' ); if ( !mMapComboBox->setCurrent( mapMapset.value( 0 ), mapMapset.value( 1 ) ) ) { mMapComboBox->setEditText( mObject->value() ); diff --git a/src/plugins/grass/qgsgrassmodule.cpp b/src/plugins/grass/qgsgrassmodule.cpp index 51e58f9ee570..fffb2f2a80bd 100644 --- a/src/plugins/grass/qgsgrassmodule.cpp +++ b/src/plugins/grass/qgsgrassmodule.cpp @@ -672,7 +672,7 @@ void QgsGrassModule::run() } else // option { - QStringList opt = arg.split( "=" ); + QStringList opt = arg.split( '=' ); //env = "GIS_OPT_" + opt.takeFirst().toUpper(); //env += "=" + opt.join( "=" ); // rejoin rest environment.insert( "GIS_OPT_" + opt.takeFirst().toUpper(), opt.join( "=" ) ); diff --git a/src/plugins/grass/qgsgrassmoduleinput.cpp b/src/plugins/grass/qgsgrassmoduleinput.cpp index b70963051eb7..2d10fb58372d 100644 --- a/src/plugins/grass/qgsgrassmoduleinput.cpp +++ b/src/plugins/grass/qgsgrassmoduleinput.cpp @@ -858,7 +858,7 @@ QgsGrassModuleInput::QgsGrassModuleInput( QgsGrassModule *module, { int mask = 0; - Q_FOREACH ( const QString &typeName, opt.split( "," ) ) + Q_FOREACH ( const QString &typeName, opt.split( ',' ) ) { mask |= QgsGrass::vectorType( typeName ); } diff --git a/src/plugins/grass/qgsgrassmoduleoptions.cpp b/src/plugins/grass/qgsgrassmoduleoptions.cpp index 2afb4b570c7e..452f0e34af2d 100644 --- a/src/plugins/grass/qgsgrassmoduleoptions.cpp +++ b/src/plugins/grass/qgsgrassmoduleoptions.cpp @@ -845,7 +845,7 @@ bool QgsGrassModuleStandardOptions::getCurrentMapRegion( QgsGrassModuleInput *in return false; } - QStringList mm = input->currentMap().split( QStringLiteral( "@" ) ); + QStringList mm = input->currentMap().split( '@' ); QString map = mm.value( 0 ); QString mapset = QgsGrass::getDefaultMapset(); if ( mm.size() > 1 ) diff --git a/src/plugins/grass/qgsgrassmoduleparam.cpp b/src/plugins/grass/qgsgrassmoduleparam.cpp index b72ebff5456f..ae6fa3935d95 100644 --- a/src/plugins/grass/qgsgrassmoduleparam.cpp +++ b/src/plugins/grass/qgsgrassmoduleparam.cpp @@ -436,7 +436,7 @@ QgsGrassModuleOption::QgsGrassModuleOption( QgsGrassModule *module, QString key, { QDomElement e = n.toElement(); QString val = e.text().trimmed(); - minMax = val.split( QStringLiteral( "-" ) ); + minMax = val.split( '-' ); if ( minMax.size() == 2 ) { mHaveLimits = true; @@ -902,7 +902,7 @@ void QgsGrassModuleGdalInput::updateQgisLayers() } else if ( vector->providerType() == QLatin1String( "ogr" ) ) { - QStringList items = provider->dataSourceUri().split( QStringLiteral( "|" ) ); + QStringList items = provider->dataSourceUri().split( '|' ); if ( items.size() > 1 ) { @@ -913,7 +913,7 @@ void QgsGrassModuleGdalInput::updateQgisLayers() for ( int i = 1; i < items.size(); i++ ) { - QStringList args = items[i].split( QStringLiteral( "=" ) ); + QStringList args = items[i].split( '=' ); if ( args.size() != 2 ) continue; @@ -1219,7 +1219,7 @@ void QgsGrassModuleSelection::onLayerChanged() { QString uri = vectorLayer->dataProvider()->dataSourceUri(); QgsDebugMsg( "uri = " + uri ); - QString layerCode = uri.split( QStringLiteral( "/" ) ).last(); + QString layerCode = uri.split( '/' ).last(); if ( mLayerInput->currentLayerCodes().contains( layerCode ) ) { // Qt::UserRole+1 may be also uri (AddLayer) but hardly matching layer id @@ -1464,7 +1464,7 @@ void QgsGrassModuleFile::browse() if ( mType == Multiple ) { - QString path = mLineEdit->text().split( QStringLiteral( "," ) ).first(); + QString path = mLineEdit->text().split( ',' ).first(); if ( path.isEmpty() ) path = lastDir; else diff --git a/src/providers/arcgisrest/qgsafsprovider.cpp b/src/providers/arcgisrest/qgsafsprovider.cpp index 7b92ed6675f2..7a5e7a2057a8 100644 --- a/src/providers/arcgisrest/qgsafsprovider.cpp +++ b/src/providers/arcgisrest/qgsafsprovider.cpp @@ -59,7 +59,7 @@ QgsAfsProvider::QgsAfsProvider( const QString &uri ) mLayerDescription = layerData[QStringLiteral( "description" )].toString(); // Set extent - QStringList coords = mSharedData->mDataSource.param( QStringLiteral( "bbox" ) ).split( QStringLiteral( "," ) ); + QStringList coords = mSharedData->mDataSource.param( QStringLiteral( "bbox" ) ).split( ',' ); if ( coords.size() == 4 ) { bool xminOk = false, yminOk = false, xmaxOk = false, ymaxOk = false; diff --git a/src/providers/arcgisrest/qgsamsdataitems.cpp b/src/providers/arcgisrest/qgsamsdataitems.cpp index c53e18f1136f..c5279dd703be 100644 --- a/src/providers/arcgisrest/qgsamsdataitems.cpp +++ b/src/providers/arcgisrest/qgsamsdataitems.cpp @@ -103,7 +103,7 @@ QVector QgsAmsConnectionItem::createChildren() QString format = QStringLiteral( "jpg" ); bool found = false; QList supportedFormats = QImageReader::supportedImageFormats(); - foreach ( const QString &encoding, serviceData["supportedImageFormatTypes"].toString().split( "," ) ) + foreach ( const QString &encoding, serviceData["supportedImageFormatTypes"].toString().split( ',' ) ) { foreach ( const QByteArray &fmt, supportedFormats ) { diff --git a/src/providers/arcgisrest/qgsamssourceselect.cpp b/src/providers/arcgisrest/qgsamssourceselect.cpp index 2c245fd54d11..fa9787529d31 100644 --- a/src/providers/arcgisrest/qgsamssourceselect.cpp +++ b/src/providers/arcgisrest/qgsamssourceselect.cpp @@ -44,7 +44,7 @@ bool QgsAmsSourceSelect::connectToService( const QgsOwsConnection &connection ) return false; } - populateImageEncodings( serviceInfoMap[QStringLiteral( "supportedImageFormatTypes" )].toString().split( QStringLiteral( "," ) ) ); + populateImageEncodings( serviceInfoMap[QStringLiteral( "supportedImageFormatTypes" )].toString().split( ',' ) ); QStringList layerErrors; foreach ( const QVariant &layerInfo, serviceInfoMap["layers"].toList() ) diff --git a/src/providers/arcgisrest/qgsarcgisrestutils.cpp b/src/providers/arcgisrest/qgsarcgisrestutils.cpp index 90162c779c1b..3b7376bf1250 100644 --- a/src/providers/arcgisrest/qgsarcgisrestutils.cpp +++ b/src/providers/arcgisrest/qgsarcgisrestutils.cpp @@ -388,7 +388,7 @@ QVariantMap QgsArcGisRestUtils::getObjects( const QString &layerurl, const QList QUrl queryUrl( layerurl + "/query" ); queryUrl.addQueryItem( QStringLiteral( "f" ), QStringLiteral( "json" ) ); queryUrl.addQueryItem( QStringLiteral( "objectIds" ), ids.join( QStringLiteral( "," ) ) ); - QString wkid = crs.indexOf( QLatin1String( ":" ) ) >= 0 ? crs.split( QStringLiteral( ":" ) )[1] : QLatin1String( "" ); + QString wkid = crs.indexOf( QLatin1String( ":" ) ) >= 0 ? crs.split( ':' )[1] : QLatin1String( "" ); queryUrl.addQueryItem( QStringLiteral( "inSR" ), wkid ); queryUrl.addQueryItem( QStringLiteral( "outSR" ), wkid ); QString outFields = fetchAttributes.join( QStringLiteral( "," ) ); diff --git a/src/providers/grass/qgsgrass.cpp b/src/providers/grass/qgsgrass.cpp index 591bdd2b5679..4e4055565ba9 100644 --- a/src/providers/grass/qgsgrass.cpp +++ b/src/providers/grass/qgsgrass.cpp @@ -1515,7 +1515,7 @@ QStringList QgsGrass::grassObjects( const QgsGrassObject &mapsetObject, QgsGrass fullName = fullName.trimmed(); if ( !fullName.isEmpty() ) { - QStringList nameMapset = fullName.split( QStringLiteral( "@" ) ); + QStringList nameMapset = fullName.split( '@' ); if ( nameMapset.value( 1 ) == mapsetObject.mapset() || nameMapset.value( 1 ).isEmpty() ) { list << nameMapset.value( 0 ); @@ -2133,7 +2133,7 @@ QgsRectangle QgsGrass::extent( const QString &gisdbase, const QString &location, try { QString str = getInfo( QStringLiteral( "window" ), gisdbase, location, mapset, map, type ); - QStringList list = str.split( QStringLiteral( "," ) ); + QStringList list = str.split( ',' ); if ( list.size() != 4 ) { throw QgsGrass::Exception( "Cannot parse GRASS map extent: " + str ); @@ -2157,7 +2157,7 @@ void QgsGrass::size( const QString &gisdbase, const QString &location, const QSt try { QString str = getInfo( QStringLiteral( "size" ), gisdbase, location, mapset, map, QgsGrassObject::Raster ); - QStringList list = str.split( QStringLiteral( "," ) ); + QStringList list = str.split( ',' ); if ( list.size() != 2 ) { throw QgsGrass::Exception( "Cannot parse GRASS map size: " + str ); @@ -2250,7 +2250,7 @@ QMap QgsGrass::query( const QString &gisdbase, const QString & try { QString str = getInfo( QStringLiteral( "query" ), gisdbase, location, mapset, map, type, x, y ); - QStringList list = str.trimmed().split( QStringLiteral( ":" ) ); + QStringList list = str.trimmed().split( ':' ); if ( list.size() == 2 ) { result[list[0]] = list[1]; diff --git a/src/providers/grass/qgsgrassprovidermodule.cpp b/src/providers/grass/qgsgrassprovidermodule.cpp index 1223253e90ec..52a0506f8a9c 100644 --- a/src/providers/grass/qgsgrassprovidermodule.cpp +++ b/src/providers/grass/qgsgrassprovidermodule.cpp @@ -559,8 +559,8 @@ QVector QgsGrassMapsetItem::createChildren() // somewhere not properly escaped (there was bug in QgsMimeDataUtils for example) QString uri = mDirPath + "/" + name + "/" + layerName; QgsLayerItem::LayerType layerType = QgsLayerItem::Vector; - QString typeName = layerName.split( QStringLiteral( "_" ) ).value( 1 ); - QString baseLayerName = layerName.split( QStringLiteral( "_" ) ).value( 0 ); + QString typeName = layerName.split( '_' ).value( 1 ); + QString baseLayerName = layerName.split( '_' ).value( 0 ); if ( typeName == QLatin1String( "point" ) || typeName == QLatin1String( "node" ) ) layerType = QgsLayerItem::Point; diff --git a/src/providers/grass/qgsgrassrasterprovider.cpp b/src/providers/grass/qgsgrassrasterprovider.cpp index 6943b6755919..ed802203f63b 100644 --- a/src/providers/grass/qgsgrassrasterprovider.cpp +++ b/src/providers/grass/qgsgrassrasterprovider.cpp @@ -675,7 +675,7 @@ double QgsGrassRasterValue::value( double x, double y, bool *ok ) // TODO: use doubles instead of strings - QStringList list = str.trimmed().split( QStringLiteral( ":" ) ); + QStringList list = str.trimmed().split( ':' ); if ( list.size() == 2 ) { if ( list[1] == QLatin1String( "error" ) ) return value; diff --git a/src/providers/wfs/qgswfsprovider.cpp b/src/providers/wfs/qgswfsprovider.cpp index 7e32c2abb820..4545c0936359 100644 --- a/src/providers/wfs/qgswfsprovider.cpp +++ b/src/providers/wfs/qgswfsprovider.cpp @@ -285,7 +285,7 @@ bool QgsWFSProvider::processSQL( const QString &sqlString, QString &errorMsg, QS if ( sql.hasParserError() ) { QString parserErrorString( sql.parserErrorString() ); - QStringList parts( parserErrorString.split( QStringLiteral( "," ) ) ); + QStringList parts( parserErrorString.split( ',' ) ); parserErrorString.clear(); Q_FOREACH ( const QString &part, parts ) { diff --git a/src/server/services/wfs/qgswfsparameters.cpp b/src/server/services/wfs/qgswfsparameters.cpp index 3453c695b2c8..22101d54c493 100644 --- a/src/server/services/wfs/qgswfsparameters.cpp +++ b/src/server/services/wfs/qgswfsparameters.cpp @@ -383,7 +383,7 @@ namespace QgsWfs if ( !bbox.isEmpty() ) { - QStringList corners = bbox.split( "," ); + const QStringList corners = bbox.split( ',' ); if ( corners.size() == 4 ) { diff --git a/src/server/services/wfs/qgswfstransaction.cpp b/src/server/services/wfs/qgswfstransaction.cpp index 55cf2f85c719..6b8166ce96de 100644 --- a/src/server/services/wfs/qgswfstransaction.cpp +++ b/src/server/services/wfs/qgswfstransaction.cpp @@ -933,7 +933,7 @@ namespace QgsWfs } // get bbox corners - QStringList corners = bbox.split( "," ); + const QStringList corners = bbox.split( ',' ); if ( corners.size() != 4 ) { throw QgsRequestNotWellFormedException( QStringLiteral( "BBOX has to be composed of 4 elements: '%1'" ).arg( bbox ) ); diff --git a/src/server/services/wfs/qgswfstransaction_1_0_0.cpp b/src/server/services/wfs/qgswfstransaction_1_0_0.cpp index 2891bc04ad14..d204edf371c4 100644 --- a/src/server/services/wfs/qgswfstransaction_1_0_0.cpp +++ b/src/server/services/wfs/qgswfstransaction_1_0_0.cpp @@ -916,7 +916,7 @@ namespace QgsWfs } // get bbox corners - QStringList corners = bbox.split( "," ); + const QStringList corners = bbox.split( ',' ); if ( corners.size() != 4 ) { throw QgsRequestNotWellFormedException( QStringLiteral( "BBOX has to be composed of 4 elements: '%1'" ).arg( bbox ) ); diff --git a/src/server/services/wms/qgswmsparameters.cpp b/src/server/services/wms/qgswmsparameters.cpp index b55d3747872c..d1290c39aa96 100644 --- a/src/server/services/wms/qgswmsparameters.cpp +++ b/src/server/services/wms/qgswmsparameters.cpp @@ -882,7 +882,7 @@ namespace QgsWms if ( !bbox.isEmpty() ) { - QStringList corners = bbox.split( "," ); + QStringList corners = bbox.split( ',' ); if ( corners.size() == 4 ) { @@ -1670,7 +1670,7 @@ namespace QgsWms QMultiMap layerFilters; Q_FOREACH ( QString f, filter ) { - QStringList splits = f.split( ":" ); + const QStringList splits = f.split( ':' ); if ( splits.size() == 2 ) { layerFilters.insert( splits[0], splits[1] ); @@ -1687,7 +1687,7 @@ namespace QgsWms QMultiMap layerSelections; Q_FOREACH ( QString s, selection ) { - QStringList splits = s.split( ":" ); + const QStringList splits = s.split( ':' ); if ( splits.size() == 2 ) { layerSelections.insert( splits[0], splits[1] ); @@ -1728,7 +1728,7 @@ namespace QgsWms it = layerSelections.find( layer ); while ( it != layerSelections.end() && it.key() == layer ) { - param.mSelection << it.value().split( "," ); + param.mSelection << it.value().split( ',' ); ++it; } } diff --git a/tests/src/core/testqgsmaprendererjob.cpp b/tests/src/core/testqgsmaprendererjob.cpp index 7cf34c590d9a..8158738b007f 100644 --- a/tests/src/core/testqgsmaprendererjob.cpp +++ b/tests/src/core/testqgsmaprendererjob.cpp @@ -294,7 +294,7 @@ void TestQgsMapRendererJob::testFourAdjacentTiles() QgsMapSettings mapSettings; //extent - QStringList rectCoords = bboxList.at( i ).split( QStringLiteral( "," ) ); + QStringList rectCoords = bboxList.at( i ).split( ',' ); if ( rectCoords.size() != 4 ) { QFAIL( "bbox string invalid" ); From 718e4985c6eb90944ded415a1138ddc187f32121 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sun, 26 Nov 2017 12:48:00 +1000 Subject: [PATCH 3/5] Fix some inefficient QString replace calls --- src/providers/gdal/qgsgdalprovider.cpp | 2 +- src/server/qgsserverrequest.cpp | 2 +- src/server/services/wfs/qgswfsgetfeature.cpp | 4 ++-- src/server/services/wfs/qgswfsparameters.cpp | 2 +- src/server/services/wfs/qgswfstransaction.cpp | 2 +- src/server/services/wfs/qgswfstransaction_1_0_0.cpp | 2 +- src/server/services/wms/qgswmsrenderer.cpp | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/providers/gdal/qgsgdalprovider.cpp b/src/providers/gdal/qgsgdalprovider.cpp index 0be036c29f6b..f59db6c80c22 100644 --- a/src/providers/gdal/qgsgdalprovider.cpp +++ b/src/providers/gdal/qgsgdalprovider.cpp @@ -1154,7 +1154,7 @@ QString QgsGdalProvider::generateBandName( int bandNumber ) const val = values.at( 1 ); if ( values.at( 0 ) == QLatin1String( "NETCDF_DIM_EXTRA" ) ) { - dimExtraValues = val.replace( QStringLiteral( "{" ), QString() ).replace( QStringLiteral( "}" ), QString() ).split( ',' ); + dimExtraValues = val.replace( '{', QString() ).replace( '}', QString() ).split( ',' ); //http://qt-project.org/doc/qt-4.8/qregexp.html#capturedTexts } else diff --git a/src/server/qgsserverrequest.cpp b/src/server/qgsserverrequest.cpp index c31a489e7609..0eed7a003eca 100644 --- a/src/server/qgsserverrequest.cpp +++ b/src/server/qgsserverrequest.cpp @@ -81,7 +81,7 @@ QMap QgsServerRequest::parameters() const { // prepare the value QString value = pair.second; - value.replace( "+", " " ); + value.replace( '+', ' ' ); mParams.insert( pair.first.toUpper(), value ); } diff --git a/src/server/services/wfs/qgswfsgetfeature.cpp b/src/server/services/wfs/qgswfsgetfeature.cpp index 35365682705c..6e059fecfea0 100644 --- a/src/server/services/wfs/qgswfsgetfeature.cpp +++ b/src/server/services/wfs/qgswfsgetfeature.cpp @@ -1298,7 +1298,7 @@ namespace QgsWfs continue; } - QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( QStringLiteral( " " ), QStringLiteral( "_" ) ) ); + QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ) ); QDomText fieldText = doc.createTextNode( featureAttributes[idx].toString() ); fieldElem.appendChild( fieldText ); typeNameElement.appendChild( fieldElem ); @@ -1399,7 +1399,7 @@ namespace QgsWfs continue; } - QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( QStringLiteral( " " ), QStringLiteral( "_" ) ) ); + QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ) ); QDomText fieldText = doc.createTextNode( featureAttributes[idx].toString() ); fieldElem.appendChild( fieldText ); typeNameElement.appendChild( fieldElem ); diff --git a/src/server/services/wfs/qgswfsparameters.cpp b/src/server/services/wfs/qgswfsparameters.cpp index 22101d54c493..6de470e923c8 100644 --- a/src/server/services/wfs/qgswfsparameters.cpp +++ b/src/server/services/wfs/qgswfsparameters.cpp @@ -383,7 +383,7 @@ namespace QgsWfs if ( !bbox.isEmpty() ) { - const QStringList corners = bbox.split( ',' ); + QStringList corners = bbox.split( ',' ); if ( corners.size() == 4 ) { diff --git a/src/server/services/wfs/qgswfstransaction.cpp b/src/server/services/wfs/qgswfstransaction.cpp index 6b8166ce96de..0d89215b5f68 100644 --- a/src/server/services/wfs/qgswfstransaction.cpp +++ b/src/server/services/wfs/qgswfstransaction.cpp @@ -933,7 +933,7 @@ namespace QgsWfs } // get bbox corners - const QStringList corners = bbox.split( ',' ); + QStringList corners = bbox.split( ',' ); if ( corners.size() != 4 ) { throw QgsRequestNotWellFormedException( QStringLiteral( "BBOX has to be composed of 4 elements: '%1'" ).arg( bbox ) ); diff --git a/src/server/services/wfs/qgswfstransaction_1_0_0.cpp b/src/server/services/wfs/qgswfstransaction_1_0_0.cpp index d204edf371c4..f279e9820db6 100644 --- a/src/server/services/wfs/qgswfstransaction_1_0_0.cpp +++ b/src/server/services/wfs/qgswfstransaction_1_0_0.cpp @@ -916,7 +916,7 @@ namespace QgsWfs } // get bbox corners - const QStringList corners = bbox.split( ',' ); + QStringList corners = bbox.split( ',' ); if ( corners.size() != 4 ) { throw QgsRequestNotWellFormedException( QStringLiteral( "BBOX has to be composed of 4 elements: '%1'" ).arg( bbox ) ); diff --git a/src/server/services/wms/qgswmsrenderer.cpp b/src/server/services/wms/qgswmsrenderer.cpp index 015043209536..5cd7203b4485 100644 --- a/src/server/services/wms/qgswmsrenderer.cpp +++ b/src/server/services/wms/qgswmsrenderer.cpp @@ -2224,7 +2224,7 @@ namespace QgsWms continue; } - QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( QStringLiteral( " " ), QStringLiteral( "_" ) ) ); + QDomElement fieldElem = doc.createElement( "qgs:" + attributeName.replace( ' ', '_' ) ); QString fieldTextString = featureAttributes.at( i ).toString(); if ( layer ) { From f04587eb670f2c5dbdee2f3b6ba316811daa7e42 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sun, 26 Nov 2017 14:00:35 +1000 Subject: [PATCH 4/5] Update tests, add new test for complex layer name --- tests/src/python/test_provider_ogr_gpkg.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/tests/src/python/test_provider_ogr_gpkg.py b/tests/src/python/test_provider_ogr_gpkg.py index 243b5d5b8641..313e1a8fded6 100644 --- a/tests/src/python/test_provider_ogr_gpkg.py +++ b/tests/src/python/test_provider_ogr_gpkg.py @@ -33,7 +33,8 @@ QgsVectorLayerExporter, QgsPointXY, QgsProject, - QgsWkbTypes) + QgsWkbTypes, + QgsDataProvider) from qgis.PyQt.QtCore import QCoreApplication, QVariant from qgis.testing import start_app, unittest @@ -113,7 +114,7 @@ def testCurveGeometryType(self): ds = None vl = QgsVectorLayer('{}'.format(tmpfile), 'test', 'ogr') - self.assertEqual(vl.dataProvider().subLayers(), ['0:test:0:CurvePolygon:geom']) + self.assertEqual(vl.dataProvider().subLayers(), [QgsDataProvider.SUBLAYER_SEPARATOR.join(['0', 'test', '0', 'CurvePolygon', 'geom'])]) f = QgsFeature() f.setGeometry(QgsGeometry.fromWkt('POLYGON ((0 0,0 1,1 1,0 0))')) vl.dataProvider().addFeatures([f]) @@ -601,6 +602,20 @@ def testReplaceLayerWhileOpen(self): features = [f for f in vl1.getFeatures(request)] self.assertEqual(len(features), 1) + def testSublayerWithComplexLayerName(self): + ''' Test reading a gpkg with a sublayer name containing : ''' + tmpfile = os.path.join(self.basetestpath, 'testGeopackageComplexLayerName.gpkg') + ds = ogr.GetDriverByName('GPKG').CreateDataSource(tmpfile) + lyr = ds.CreateLayer('layer1:', geom_type=ogr.wkbPoint, options=['GEOMETRY_NAME=geom:']) + lyr.CreateField(ogr.FieldDefn('attr', ogr.OFTInteger)) + f = ogr.Feature(lyr.GetLayerDefn()) + f.SetGeometry(ogr.CreateGeometryFromWkt('POINT(0 0)')) + lyr.CreateFeature(f) + f = None + + vl = QgsVectorLayer(u'{}'.format(tmpfile), u'layer', u'ogr') + self.assertEqual(vl.dataProvider().subLayers(), [QgsDataProvider.SUBLAYER_SEPARATOR.join(['0', 'layer1:', '1', 'Point', 'geom:'])]) + def testGeopackageManyLayers(self): ''' test opening more than 64 layers without running out of Spatialite connections ''' @@ -669,7 +684,8 @@ def testGeopackageRefreshIfTableListUpdated(self): vl2 = QgsVectorLayer(u'{}'.format(tmpfile), 'test', u'ogr') vl2.subLayers() - self.assertEqual(vl2.dataProvider().subLayers(), ['0:test:0:Point:geom', '1:test2:0:Point:geom']) + self.assertEqual(vl2.dataProvider().subLayers(), [QgsDataProvider.SUBLAYER_SEPARATOR.join(['0', 'test', '0', 'Point', 'geom']), + QgsDataProvider.SUBLAYER_SEPARATOR.join(['1', 'test2', '0', 'Point', 'geom'])]) def testGeopackageLargeFID(self): From a95aecafd6fefe701085a6259f7105147aca7438 Mon Sep 17 00:00:00 2001 From: Nyall Dawson Date: Sun, 26 Nov 2017 16:28:59 +1000 Subject: [PATCH 5/5] Fix tests --- python/plugins/processing/tools/dataobjects.py | 3 ++- tests/src/core/testqgsrastersublayer.cpp | 4 ++-- tests/src/python/test_provider_ogr.py | 6 +++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/python/plugins/processing/tools/dataobjects.py b/python/plugins/processing/tools/dataobjects.py index b9ca3f954907..c6cad5e9703f 100644 --- a/python/plugins/processing/tools/dataobjects.py +++ b/python/plugins/processing/tools/dataobjects.py @@ -32,6 +32,7 @@ from qgis.core import (QgsVectorFileWriter, QgsMapLayer, + QgsDataProvider, QgsRasterLayer, QgsWkbTypes, QgsVectorLayer, @@ -309,7 +310,7 @@ def getRasterSublayer(path, param): subLayer = subLayer[1:] else: # remove driver name and file name - subLayer.replace(subLayer.split(":")[0], "") + subLayer.replace(subLayer.split(QgsDataProvider.SUBLAYER_SEPARATOR)[0], "") subLayer.replace(path, "") # remove any : or " left over if subLayer.startswith(":"): diff --git a/tests/src/core/testqgsrastersublayer.cpp b/tests/src/core/testqgsrastersublayer.cpp index d564feb42268..77ae5d6be160 100644 --- a/tests/src/core/testqgsrastersublayer.cpp +++ b/tests/src/core/testqgsrastersublayer.cpp @@ -138,12 +138,12 @@ void TestQgsRasterSubLayer::subLayersList() Q_FOREACH ( const QString &s, mpRasterLayer->subLayers() ) { qDebug() << "sublayer: " << s; - sublayers << s.split( QgsDataProvider::SUBLAYER_SEPARATOR ).last(); + sublayers << s.split( ':' ).last(); } qDebug() << "sublayers: " << sublayers.join( QStringLiteral( "," ) ); mReport += QStringLiteral( "sublayers:
%1
\n" ).arg( sublayers.join( QStringLiteral( "
" ) ) ); mReport += QStringLiteral( "expected:
%1
\n" ).arg( expected.join( QStringLiteral( "
" ) ) ); - QVERIFY( sublayers == expected ); + QCOMPARE( sublayers, expected ); mReport += QLatin1String( "

Passed

" ); } } diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py index b674cde831df..38e42bdb18ed 100644 --- a/tests/src/python/test_provider_ogr.py +++ b/tests/src/python/test_provider_ogr.py @@ -18,7 +18,7 @@ import tempfile from osgeo import gdal, ogr # NOQA -from qgis.core import (QgsFeature, QgsFeatureRequest, QgsSettings, +from qgis.core import (QgsFeature, QgsFeatureRequest, QgsSettings, QgsDataProvider, QgsVectorDataProvider, QgsVectorLayer, QgsWkbTypes, QgsNetworkAccessManager) from qgis.testing import start_app, unittest @@ -111,7 +111,7 @@ def testMixOfPolygonCurvePolygon(self): vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr') self.assertTrue(vl.isValid()) self.assertEqual(len(vl.dataProvider().subLayers()), 1) - self.assertEqual(vl.dataProvider().subLayers()[0], '0:testMixOfPolygonCurvePolygon:4:CurvePolygon:') + self.assertEqual(vl.dataProvider().subLayers()[0], QgsDataProvider.SUBLAYER_SEPARATOR.join(['0', 'testMixOfPolygonCurvePolygon', '4', 'CurvePolygon', ''])) def testMixOfLineStringCompoundCurve(self): @@ -127,7 +127,7 @@ def testMixOfLineStringCompoundCurve(self): vl = QgsVectorLayer('{}|layerid=0'.format(datasource), 'test', 'ogr') self.assertTrue(vl.isValid()) self.assertEqual(len(vl.dataProvider().subLayers()), 1) - self.assertEqual(vl.dataProvider().subLayers()[0], '0:testMixOfLineStringCompoundCurve:5:CompoundCurve:') + self.assertEqual(vl.dataProvider().subLayers()[0], QgsDataProvider.SUBLAYER_SEPARATOR.join(['0', 'testMixOfLineStringCompoundCurve', '5', 'CompoundCurve', ''])) def testGpxElevation(self): # GPX without elevation data