From 33708ea2459ebe898ed2e7c477bc0d8a0f738987 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Fri, 22 May 2020 18:37:47 +0200 Subject: [PATCH 1/4] Support OGR GPKG in datasource uri connection info Makes QgsDataSourceUri::connectionInfo usable with GPKG and OGR/Spatialite file-based DBs. With tests. Fixes #36525 --- src/core/qgsdatasourceuri.cpp | 12 ++++++++ src/core/qgsdatasourceuri.h | 4 +++ tests/src/core/testqgsdatasourceuri.cpp | 39 +++++++++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/src/core/qgsdatasourceuri.cpp b/src/core/qgsdatasourceuri.cpp index 4f64547a95b6..0f67bf916c3b 100644 --- a/src/core/qgsdatasourceuri.cpp +++ b/src/core/qgsdatasourceuri.cpp @@ -33,6 +33,7 @@ QgsDataSourceUri::QgsDataSourceUri() } QgsDataSourceUri::QgsDataSourceUri( const QString &u ) + : mRawUri( u ) { QString uri = u; int i = 0; @@ -532,6 +533,17 @@ QString QgsDataSourceUri::connectionInfo( bool expandAuthConfig ) const } } + // Is this an OGR file-based DB? + if ( connectionItems.isEmpty() ) + { + // Handles OGR geopackages, gets the raw uri and remove what is after the '|' + const QStringList parts{ mRawUri.split( '|' ) }; + if ( ! parts.isEmpty() ) + { + connectionItems.push_back( parts.at( 0 ) ); + } + } + return connectionItems.join( QStringLiteral( " " ) ); } diff --git a/src/core/qgsdatasourceuri.h b/src/core/qgsdatasourceuri.h index 3d0e34b30e7e..138e8d7aee62 100644 --- a/src/core/qgsdatasourceuri.h +++ b/src/core/qgsdatasourceuri.h @@ -66,6 +66,7 @@ class CORE_EXPORT QgsDataSourceUri /** * Returns the connection part of the URI. + * This may be a file path in case of a filesystem-based DB (spatialite or GPKG). */ QString connectionInfo( bool expandAuthConfig = true ) const; @@ -308,6 +309,9 @@ class CORE_EXPORT QgsDataSourceUri /* data */ + //! The original unmodified/unparsed uri string + QString mRawUri; + //! host name QString mHost; //! port the database server listens on diff --git a/tests/src/core/testqgsdatasourceuri.cpp b/tests/src/core/testqgsdatasourceuri.cpp index 8f61d2ad863e..e0741c9d852b 100644 --- a/tests/src/core/testqgsdatasourceuri.cpp +++ b/tests/src/core/testqgsdatasourceuri.cpp @@ -27,6 +27,15 @@ class TestQgsDataSourceUri: public QObject private slots: void checkparser(); void checkparser_data(); + + /** + * connectionInfo is used in several places to extract the + * connection part of a layer URI, this means that for a networked + * DB we get the connection string, but for a file-based DB (spatialite and + * GPKG we need to get the file path. Let's test it. + */ + void checkConnectionInfo(); + void checkConnectionInfo_data(); void checkAuthParams(); }; @@ -230,6 +239,36 @@ void TestQgsDataSourceUri::checkparser() QCOMPARE( ds.param( "myparam" ), myparam ); } +void TestQgsDataSourceUri::checkConnectionInfo_data() +{ + QTest::addColumn( "uri" ); + QTest::addColumn( "connectionInfo" ); + QTest::newRow( "PG" ) << "dbname='mydb' host='myhost' user='91divoc' password='quarantine' port='5432' mode='2' schema=myschema table=my_table (geom)" + << "dbname='mydb' host=myhost port=5432 user='91divoc' password='quarantine'"; + QTest::newRow( "PG Service" ) << "service=my_service schema=myschema table=my_table" + << "service='my_service'"; + QTest::newRow( "spatialite" ) << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db' table="somedata" (geom))" + << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db')"; + // This fail because the data source uri parsed adds another back slash, it is probably not an issue + //QTest::newRow( "spatialite on windows" ) << R"(dbname='C:\my fancy path\qgis\spatialite.db' table="somedata" (geom))" + // << R"(dbname='C:\my fancy path\qgis\spatialite.db')"; + QTest::newRow( "geopackage" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layername=my_layer" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; + QTest::newRow( "geopackage camel case" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layerName=my_layer" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; + QTest::newRow( "geopackage no layername" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; + QTest::newRow( "geopackage no path" ) << "geopackage.gpkg|layername=my_layer" + << "geopackage.gpkg"; +} + +void TestQgsDataSourceUri::checkConnectionInfo() +{ + QFETCH( QString, uri ); + QFETCH( QString, connectionInfo ); + QCOMPARE( QgsDataSourceUri( uri ).connectionInfo( false ), connectionInfo ); +} + void TestQgsDataSourceUri::checkAuthParams() { // some providers rely on the QgsDataSourceUri params for storing and retrieving username, password and authentication. From 5d0c45e4eea867818d32e0342208e74dae7838a6 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Fri, 22 May 2020 18:39:35 +0200 Subject: [PATCH 2/4] Fix list of providers that support auto transactions --- src/ui/qgsprojectpropertiesbase.ui | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/ui/qgsprojectpropertiesbase.ui b/src/ui/qgsprojectpropertiesbase.ui index 9ac955794ca5..5a222a526588 100644 --- a/src/ui/qgsprojectpropertiesbase.ui +++ b/src/ui/qgsprojectpropertiesbase.ui @@ -245,7 +245,7 @@ - 8 + 4 @@ -274,8 +274,8 @@ 0 0 - 590 - 828 + 537 + 855 @@ -897,8 +897,8 @@ 0 0 - 603 - 161 + 546 + 164 @@ -972,8 +972,8 @@ 0 0 - 290 - 543 + 269 + 553 @@ -1392,7 +1392,7 @@ - When enabled, layers from the same database connection will be put into a transaction group. Their edit state will be synchronized and changes to these layers will be sent to the provider immediately. Only supported on postgres provider. + <html><head/><body><p>When enabled, layers from the same database connection will be put into a transaction group. </p><p>Their edit state will be synchronized and changes to these layers will be sent to the provider immediately. </p><p>Only supported on:</p><ul><li>Postgres</li><li>Spatialite</li><li> GeoPackage</li><li>Oracle</li></ul></body></html> Automatically create transaction groups where possible @@ -1548,8 +1548,8 @@ 0 0 - 178 - 54 + 167 + 55 @@ -1611,7 +1611,7 @@ 0 0 671 - 3090 + 3139 From 85f9f185cca472f4043e6adbe83032da21902af4 Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Fri, 22 May 2020 18:40:16 +0200 Subject: [PATCH 3/4] Sipify --- python/core/auto_generated/qgsdatasourceuri.sip.in | 1 + 1 file changed, 1 insertion(+) diff --git a/python/core/auto_generated/qgsdatasourceuri.sip.in b/python/core/auto_generated/qgsdatasourceuri.sip.in index 8e6de1ef22cc..35f3823b193a 100644 --- a/python/core/auto_generated/qgsdatasourceuri.sip.in +++ b/python/core/auto_generated/qgsdatasourceuri.sip.in @@ -48,6 +48,7 @@ Constructor for QgsDataSourceUri which parses an input ``uri`` string. QString connectionInfo( bool expandAuthConfig = true ) const; %Docstring Returns the connection part of the URI. +This may be a file path in case of a filesystem-based DB (spatialite or GPKG). %End QString uri( bool expandAuthConfig = true ) const; From fdf91e121814457f9557a50c0005237b4121803a Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Sat, 23 May 2020 08:05:03 +0200 Subject: [PATCH 4/4] Handle file system based in connectionInfo --- src/core/qgsdatasourceuri.cpp | 17 ++++++++++-- tests/src/core/testqgsdatasourceuri.cpp | 36 ++++++++++++++++++++----- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/core/qgsdatasourceuri.cpp b/src/core/qgsdatasourceuri.cpp index 0f67bf916c3b..6524c376d40a 100644 --- a/src/core/qgsdatasourceuri.cpp +++ b/src/core/qgsdatasourceuri.cpp @@ -551,6 +551,12 @@ QString QgsDataSourceUri::uri( bool expandAuthConfig ) const { QString uri = connectionInfo( expandAuthConfig ); + // Filsystem based? + if ( uri.compare( mRawUri, Qt::CaseSensitivity::CaseInsensitive ) == 0 ) + { + return uri; + } + if ( !mKeyColumn.isEmpty() ) { uri += QStringLiteral( " key='%1'" ).arg( escape( mKeyColumn ) ); @@ -584,8 +590,15 @@ QString QgsDataSourceUri::uri( bool expandAuthConfig ) const QgsDebugMsg( QStringLiteral( "invalid uri parameter %1 skipped" ).arg( it.key() ) ); continue; } - - uri += ' ' + it.key() + "='" + escape( it.value() ) + '\''; + // filesystem based + if ( it.key() + '=' + it.value() == uri || it.key().contains( uri ) ) + { + return mRawUri; + } + else + { + uri += ' ' + it.key() + "='" + escape( it.value() ) + '\''; + } } QString columnName( mGeometryColumn ); diff --git a/tests/src/core/testqgsdatasourceuri.cpp b/tests/src/core/testqgsdatasourceuri.cpp index e0741c9d852b..f199c423823c 100644 --- a/tests/src/core/testqgsdatasourceuri.cpp +++ b/tests/src/core/testqgsdatasourceuri.cpp @@ -243,30 +243,52 @@ void TestQgsDataSourceUri::checkConnectionInfo_data() { QTest::addColumn( "uri" ); QTest::addColumn( "connectionInfo" ); + QTest::addColumn( "roundtrip" ); + + // Test roundtrip + QTest::newRow( "gpx" ) << "testdata/layers.gpx?type=track" + << "testdata/layers.gpx?type=track" + << "testdata/layers.gpx?type=track"; + QTest::newRow( "shapefile" ) << R"(/my/path/to\ my/hapefile.shp)" + << R"(/my/path/to\ my/hapefile.shp)" + << R"(/my/path/to\ my/hapefile.shp)"; + // Real DBs QTest::newRow( "PG" ) << "dbname='mydb' host='myhost' user='91divoc' password='quarantine' port='5432' mode='2' schema=myschema table=my_table (geom)" - << "dbname='mydb' host=myhost port=5432 user='91divoc' password='quarantine'"; - QTest::newRow( "PG Service" ) << "service=my_service schema=myschema table=my_table" - << "service='my_service'"; + << "dbname='mydb' host=myhost port=5432 user='91divoc' password='quarantine'" + << "dbname='mydb' host=myhost port=5432 user='91divoc' password='quarantine' mode='2' table=\"myschema\".\"my_table\" (geom)"; + QTest::newRow( "PG service" ) << "service=my_service schema=myschema table=my_table" + << "service='my_service'" + << "service='my_service' table=\"myschema\".\"my_table\""; QTest::newRow( "spatialite" ) << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db' table="somedata" (geom))" - << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db')"; + << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db')" + << R"(dbname='/home/qgis/dev/QGIS/tests/testdata/provider/spatialite.db' table="somedata" (geom))"; // This fail because the data source uri parsed adds another back slash, it is probably not an issue //QTest::newRow( "spatialite on windows" ) << R"(dbname='C:\my fancy path\qgis\spatialite.db' table="somedata" (geom))" // << R"(dbname='C:\my fancy path\qgis\spatialite.db')"; QTest::newRow( "geopackage" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layername=my_layer" - << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layername=my_layer"; + QTest::newRow( "geopackage layerid" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layerId=my_layer" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layerId=my_layer"; QTest::newRow( "geopackage camel case" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layerName=my_layer" - << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg|layerName=my_layer"; QTest::newRow( "geopackage no layername" ) << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" + << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg" << "/home/qgis/dev/QGIS/tests/testdata/provider/geopackage.gpkg"; QTest::newRow( "geopackage no path" ) << "geopackage.gpkg|layername=my_layer" - << "geopackage.gpkg"; + << "geopackage.gpkg" + << "geopackage.gpkg|layername=my_layer"; } void TestQgsDataSourceUri::checkConnectionInfo() { QFETCH( QString, uri ); QFETCH( QString, connectionInfo ); + QFETCH( QString, roundtrip ); QCOMPARE( QgsDataSourceUri( uri ).connectionInfo( false ), connectionInfo ); + QCOMPARE( QgsDataSourceUri( uri ).uri( ), roundtrip ); } void TestQgsDataSourceUri::checkAuthParams()