From f3149d9f5cede94eeb32d5346902985119efff48 Mon Sep 17 00:00:00 2001 From: Vladyslav Sydorov Date: Fri, 10 Jan 2025 11:49:25 +0100 Subject: [PATCH] Fix layer paths resolving to matching CWD names - Remove redundant second call to absoluteToRelativeUri in QgsMapLayer::writeLayerXml - Add a guard in QgsPathResolver::writePath to prevent this happening again when absoluteToRelativeUri is called two times. - Add a regression test --- src/core/qgsmaplayer.cpp | 3 +- src/core/qgspathresolver.cpp | 6 +++ tests/src/core/testqgsproject.cpp | 80 +++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/core/qgsmaplayer.cpp b/src/core/qgsmaplayer.cpp index e1cfb6040556..e64a79feb9ea 100644 --- a/src/core/qgsmaplayer.cpp +++ b/src/core/qgsmaplayer.cpp @@ -779,8 +779,7 @@ bool QgsMapLayer::writeLayerXml( QDomElement &layerElement, QDomDocument &docume QDomElement dataSource = document.createElement( QStringLiteral( "datasource" ) ); const QgsDataProvider *provider = dataProvider(); const QString providerKey = provider ? provider->name() : QString(); - const QString srcRaw = encodedSource( source(), context ); - const QString src = providerKey.isEmpty() ? srcRaw : QgsProviderRegistry::instance()->absoluteToRelativeUri( providerKey, srcRaw, context ); + const QString src = encodedSource( source(), context ); const QDomText dataSourceText = document.createTextNode( src ); dataSource.appendChild( dataSourceText ); layerElement.appendChild( dataSource ); diff --git a/src/core/qgspathresolver.cpp b/src/core/qgspathresolver.cpp index 1781d1b78eae..92b519b24ea5 100644 --- a/src/core/qgspathresolver.cpp +++ b/src/core/qgspathresolver.cpp @@ -290,6 +290,12 @@ QString QgsPathResolver::writePath( const QString &s ) const } const QFileInfo srcFileInfo( srcPath ); + // Guard against relative paths: If srcPath is already relative, QFileInfo will match + // files in the working directory, instead of project directory. Avoid by returning early. + if ( !srcFileInfo.isAbsolute() ) + { + return srcPath; + } if ( srcFileInfo.exists() ) srcPath = srcFileInfo.canonicalFilePath(); diff --git a/tests/src/core/testqgsproject.cpp b/tests/src/core/testqgsproject.cpp index 4efe6afd7870..db73afbf14ac 100644 --- a/tests/src/core/testqgsproject.cpp +++ b/tests/src/core/testqgsproject.cpp @@ -65,6 +65,7 @@ class TestQgsProject : public QObject void testAttachmentIdentifier(); void testEmbeddedGroupWithJoins(); void testAsynchronousLayerLoading(); + void regression60100(); }; void TestQgsProject::init() @@ -1103,5 +1104,84 @@ void TestQgsProject::testAsynchronousLayerLoading() } +void TestQgsProject::regression60100() +{ +/* + * Regression test for QGIS issue #60100 (https://github.com/qgis/QGIS/issues/60100) + * This test ensures that when saving a QGIS project with relative paths, + * the correct layer datasource is preserved, even when the current working + * directory (CWD) contains a file with the same name as the layer datasource. + * + * Previous behavior: + * - If a file with the same name as a layer datasource existed in the CWD, + * the layer path in the saved project would point to the file in the CWD, + * rather than the intended file in the project directory (PROJDIR). + * + * Test steps: + * 1. Create a temporary directory structure with two subfolders: WORKDIR and PROJDIR. + * 2. Copy a `points.geojson` file to both WORKDIR and PROJDIR. + * 3. Create a new QGIS project in PROJDIR and add the `points.geojson` file from PROJDIR as a layer. + * 4. Change the working directory to WORKDIR and save the project. + * 5. Verify that the saved project references the correct datasource (`./points.geojson` in PROJDIR) + * and does not erroneously reference the file in WORKDIR. + */ + + // Create directory structure with 2 subfolders + const QTemporaryDir baseDir; + const QDir base( baseDir.path() ); + base.mkdir( QStringLiteral( "WORKDIR" ) ); + base.mkdir( QStringLiteral( "PROJDIR" ) ); + const QString workDirPath = baseDir.path() + QStringLiteral( "/WORKDIR" ); + const QString projDirPath = baseDir.path() + QStringLiteral( "/PROJDIR" ); + + // Save our old CWD and switch to the new WORKDIR + const QString oldCWD = QDir::currentPath(); + QVERIFY( QDir::setCurrent( workDirPath ) ); + + // Copy points.geojson to both subfolders + const QString testDataDir( TEST_DATA_DIR ); + const QString pointsPath = testDataDir + QStringLiteral( "/points.geojson" ); + QFile::copy( pointsPath, workDirPath + QStringLiteral( "/points.geojson" ) ); + QFile::copy( pointsPath, projDirPath + QStringLiteral( "/points.geojson" ) ); + + // Create a new/empty project in PROJDIR + const QString projectPath = projDirPath + QStringLiteral( "/project.qgs" ); + std::unique_ptr project = std::make_unique(); + + // Add the local points.geojson (in PROJDIR) as a layer + std::unique_ptr layer = std::make_unique( + projDirPath + QStringLiteral( "/points.geojson" ), + QStringLiteral( "Test Points" ), + QStringLiteral( "ogr" ) + ); + project->addMapLayer( layer.release() ); + + // Write (save) the project to disk. This used to pick up the WRONG file and save it to the proj. + project->write( projectPath ); + + // Restore old working directory + QVERIFY( QDir::setCurrent( oldCWD ) ); + + // Verify the layer path in the project file + QDomDocument doc; + QFile projectFile( projectPath ); + bool res = projectFile.open( QIODevice::ReadOnly ); + Q_ASSERT( res ); + res = static_cast( doc.setContent( &projectFile ) ); + Q_ASSERT( res ); + projectFile.close(); + + const QDomElement docElem = doc.documentElement(); + const QDomElement layersElem = docElem.firstChildElement( QStringLiteral( "projectlayers" ) ); + QDomElement layerElem = layersElem.firstChildElement(); + while ( !layerElem.isNull() ) + { + const QString layerSource = layerElem.firstChildElement( QStringLiteral( "datasource" ) ).text(); + // Should NOT be "../WORKDIR/points.geojson" + QCOMPARE( layerSource, QStringLiteral( "./points.geojson" ) ); + layerElem = layerElem.nextSiblingElement(); + } +} + QGSTEST_MAIN( TestQgsProject ) #include "testqgsproject.moc"