From 351dfa40d0e319a4c402b892b32dc1d60fdabd3b Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Wed, 24 Jan 2024 21:14:11 +0100 Subject: [PATCH] QgsPathResolver::readPath/writePath: make it work properly with OGR datasource URI (fixes #55975) --- src/core/qgspathresolver.cpp | 42 +++++++++++++++++++----- tests/src/python/test_qgspathresolver.py | 26 ++++++++++----- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/core/qgspathresolver.cpp b/src/core/qgspathresolver.cpp index b8984c0cb8ab..224ef8a780fb 100644 --- a/src/core/qgspathresolver.cpp +++ b/src/core/qgspathresolver.cpp @@ -81,6 +81,18 @@ QString QgsPathResolver::readPath( const QString &f ) const return src; } + // If this looks like a OGR connection string, remove everything after the + // path to avoid messing with '/' characters that may be in the subset filter, + // and re-add it in a final stage. + // cf https://github.com/qgis/QGIS/issues/55975 + QString srcSuffix; + const auto posLayername = src.indexOf( "|layername=" ); + if ( posLayername > 0 ) + { + srcSuffix = src.mid( posLayername ); + src.resize( posLayername ); + } + // if this is a VSIFILE, remove the VSI prefix and append to final result QString vsiPrefix = QgsGdalUtils::vsiPrefixForPath( src ); if ( ! vsiPrefix.isEmpty() ) @@ -102,13 +114,13 @@ QString QgsPathResolver::readPath( const QString &f ) const ( src[0].isLetter() && src[1] == ':' ) ) { // UNC or absolute path - return vsiPrefix + src; + return vsiPrefix + src + srcSuffix; } #else if ( src[0] == '/' ) { // absolute path - return vsiPrefix + src; + return vsiPrefix + src + srcSuffix; } #endif @@ -120,17 +132,17 @@ QString QgsPathResolver::readPath( const QString &f ) const const QFileInfo pfi( mBaseFileName ); const QString home = pfi.absolutePath(); if ( home.isEmpty() ) - return vsiPrefix + src; + return vsiPrefix + src + srcSuffix; const QFileInfo fi( home + '/' + src ); if ( !fi.exists() ) { - return vsiPrefix + src; + return vsiPrefix + src + srcSuffix; } else { - return vsiPrefix + fi.canonicalFilePath(); + return vsiPrefix + fi.canonicalFilePath() + srcSuffix; } } @@ -139,7 +151,7 @@ QString QgsPathResolver::readPath( const QString &f ) const if ( projPath.isEmpty() ) { - return vsiPrefix + src; + return vsiPrefix + src + srcSuffix; } #if defined(Q_OS_WIN) @@ -194,7 +206,7 @@ QString QgsPathResolver::readPath( const QString &f ) const projElems.prepend( QString() ); #endif - return vsiPrefix + projElems.join( QLatin1Char( '/' ) ); + return vsiPrefix + projElems.join( QLatin1Char( '/' ) ) + srcSuffix; } QString QgsPathResolver::setPathPreprocessor( const std::function &processor ) @@ -278,6 +290,18 @@ QString QgsPathResolver::writePath( const QString &s ) const return src; } + // If this looks like a OGR connection string, remove everything after the + // path to avoid messing with '/' characters that may be in the subset filter, + // and re-add it in a final stage. + // cf https://github.com/qgis/QGIS/issues/55975 + QString srcSuffix; + const auto posLayername = src.indexOf( "|layername=" ); + if ( posLayername > 0 ) + { + srcSuffix = src.mid( posLayername ); + src.resize( posLayername ); + } + // Check if it is a publicSource uri and clean it const QUrl url { src }; QString srcPath { src }; @@ -341,7 +365,7 @@ QString QgsPathResolver::writePath( const QString &s ) const if ( n == 0 ) { // no common parts; might not even be a file - return src; + return src + srcSuffix; } if ( !projElems.isEmpty() ) @@ -366,5 +390,5 @@ QString QgsPathResolver::writePath( const QString &s ) const returnPath.append( '?' ); returnPath.append( urlQuery ); } - return returnPath; + return returnPath + srcSuffix; } diff --git a/tests/src/python/test_qgspathresolver.py b/tests/src/python/test_qgspathresolver.py index c24153dd6709..702dcb074e72 100644 --- a/tests/src/python/test_qgspathresolver.py +++ b/tests/src/python/test_qgspathresolver.py @@ -158,22 +158,32 @@ def testRelativeProject(self): os.chdir(curdir) def __test__path_writer(self, path): + pos = path.find("|layername=") + suffix = "" + if pos > 0: + suffix = path[pos:] + path = path[0:pos] if path.startswith(TEST_DATA_DIR): - return os.path.join("@TEST_DATA_DIR@", os.path.basename(path)) - return path + return os.path.join("@TEST_DATA_DIR@", os.path.basename(path)) + suffix + return path + suffix def __test_path_reader(self, path): + pos = path.find("|layername=") + suffix = "" + if pos > 0: + suffix = path[pos:] + path = path[0:pos] if path.startswith("@TEST_DATA_DIR@"): - return os.path.join(TEST_DATA_DIR, os.path.basename(path)) - return path + return os.path.join(TEST_DATA_DIR, os.path.basename(path)) + suffix + return path + suffix def testPathWriter(self): readerId = QgsPathResolver.setPathPreprocessor(self.__test_path_reader) writerId = QgsPathResolver.setPathWriter(self.__test__path_writer) - lines_shp_path = os.path.join(TEST_DATA_DIR, 'lines.shp') + uri = os.path.join(TEST_DATA_DIR, 'points_gpkg.gpkg') + "|layername=points_gpkg|subset=1=1 /* foo */" - lines_layer = QgsVectorLayer(lines_shp_path, 'Lines', 'ogr') + lines_layer = QgsVectorLayer(uri, 'Points', 'ogr') self.assertTrue(lines_layer.isValid()) p = QgsProject() p.addMapLayer(lines_layer) @@ -187,9 +197,9 @@ def testPathWriter(self): p2 = QgsProject() self.assertTrue(p2.read(temp_project_path)) - l = p2.mapLayersByName('Lines')[0] + l = p2.mapLayersByName('Points')[0] self.assertEqual(l.isValid(), True) - self.assertEqual(l.source(), lines_shp_path) + self.assertEqual(l.source(), uri) QgsPathResolver.removePathPreprocessor(readerId) QgsPathResolver.removePathWriter(writerId)