From ae27767cc898e011bccd5cf0aa4c56bceb91046f Mon Sep 17 00:00:00 2001 From: rldhont Date: Fri, 16 Oct 2020 22:09:40 +0200 Subject: [PATCH] FIX: QgsCoordinateReferenceSystem::createFrom* has to return CRS's validity The methods `QgsCoordinateReferenceSystem::createFrom*` returned true when the CRS was found in cache instead of the CRS's validity. This fixed it and the tests. --- src/core/qgscoordinatereferencesystem.cpp | 28 +++++------ .../core/testqgscoordinatereferencesystem.cpp | 48 ++++++++++--------- 2 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/core/qgscoordinatereferencesystem.cpp b/src/core/qgscoordinatereferencesystem.cpp index 9176d202f70..b47aa770ff1 100644 --- a/src/core/qgscoordinatereferencesystem.cpp +++ b/src/core/qgscoordinatereferencesystem.cpp @@ -282,7 +282,7 @@ bool QgsCoordinateReferenceSystem::createFromString( const QString &definition ) { // found a match in the cache *this = crsIt.value(); - return true; + return d->mIsValid; } } locker.unlock(); @@ -398,7 +398,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs ) { // found a match in the cache *this = crsIt.value(); - return true; + return d->mIsValid; } } locker.unlock(); @@ -423,7 +423,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs ) locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableOgcCache ) sOgcCache()->insert( crs, *this ); - return true; + return d->mIsValid; } } @@ -442,7 +442,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs ) locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableOgcCache ) sOgcCache()->insert( crs, *this ); - return true; + return d->mIsValid; } } } @@ -453,7 +453,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs ) locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableOgcCache ) sOgcCache()->insert( crs, *this ); - return true; + return d->mIsValid; } // NAD27 @@ -492,7 +492,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs ) locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableOgcCache ) sOgcCache()->insert( crs, QgsCoordinateReferenceSystem() ); - return false; + return d->mIsValid; } // Misc helper functions ----------------------- @@ -523,7 +523,7 @@ bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id ) { // found a match in the cache *this = crsIt.value(); - return true; + return d->mIsValid; } } locker.unlock(); @@ -543,7 +543,7 @@ bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id ) if ( !sDisableSrIdCache ) sSrIdCache()->insert( id, *this ); - return true; + return d->mIsValid; } } } @@ -568,7 +568,7 @@ bool QgsCoordinateReferenceSystem::createFromSrsId( const long id ) { // found a match in the cache *this = crsIt.value(); - return true; + return d->mIsValid; } } locker.unlock(); @@ -587,7 +587,7 @@ bool QgsCoordinateReferenceSystem::createFromSrsId( const long id ) locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableSrsIdCache ) sSrsIdCache()->insert( id, *this ); - return true; + return d->mIsValid; } } } @@ -869,7 +869,7 @@ bool QgsCoordinateReferenceSystem::createFromWktInternal( const QString &wkt, co locker.changeMode( QgsReadWriteLocker::Write ); sWktCache()->insert( wkt, *this ); } - return true; + return d->mIsValid; } } locker.unlock(); @@ -954,7 +954,7 @@ bool QgsCoordinateReferenceSystem::createFromProj( const QString &projString, co { // found a match in the cache *this = crsIt.value(); - return true; + return d->mIsValid; } } locker.unlock(); @@ -991,7 +991,7 @@ bool QgsCoordinateReferenceSystem::createFromProj( const QString &projString, co locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableProjCache ) sProj4Cache()->insert( projString, *this ); - return true; + return d->mIsValid; } } } @@ -1654,7 +1654,7 @@ bool QgsCoordinateReferenceSystem::setWktString( const QString &wkt, bool allowP locker.changeMode( QgsReadWriteLocker::Write ); if ( !sDisableWktCache ) sWktCache()->insert( wkt, *this ); - return true; + return d->mIsValid; } } else diff --git a/tests/src/core/testqgscoordinatereferencesystem.cpp b/tests/src/core/testqgscoordinatereferencesystem.cpp index 82b5919fab2..c82873ac774 100644 --- a/tests/src/core/testqgscoordinatereferencesystem.cpp +++ b/tests/src/core/testqgscoordinatereferencesystem.cpp @@ -327,21 +327,25 @@ void TestQgsCoordinateReferenceSystem::fromOgcWmsCrs() void TestQgsCoordinateReferenceSystem::ogcWmsCrsCache() { // test that crs can be retrieved correctly from cache - QgsCoordinateReferenceSystem crs = QgsCoordinateReferenceSystem::fromOgcWmsCrs( QStringLiteral( "EPSG:4326" ) ); + QgsCoordinateReferenceSystem crs; + QVERIFY( crs.createFromOgcWmsCrs( QStringLiteral( "EPSG:4326" ) ) ); QVERIFY( crs.isValid() ); QCOMPARE( crs.authid(), QStringLiteral( "EPSG:4326" ) ); QVERIFY( QgsCoordinateReferenceSystem::ogcCache().contains( QStringLiteral( "EPSG:4326" ) ) ); // a second time, so crs is fetched from cache - QgsCoordinateReferenceSystem crs2 = QgsCoordinateReferenceSystem::fromOgcWmsCrs( QStringLiteral( "EPSG:4326" ) ); + QgsCoordinateReferenceSystem crs2; + QVERIFY( crs2.createFromOgcWmsCrs( QStringLiteral( "EPSG:4326" ) ) ); QVERIFY( crs2.isValid() ); QCOMPARE( crs2.authid(), QStringLiteral( "EPSG:4326" ) ); // invalid - QgsCoordinateReferenceSystem crs3 = QgsCoordinateReferenceSystem::fromOgcWmsCrs( QStringLiteral( "not a CRS" ) ); + QgsCoordinateReferenceSystem crs3; + QVERIFY( !crs3.createFromOgcWmsCrs( QStringLiteral( "not a CRS" ) ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::ogcCache().contains( QStringLiteral( "not a CRS" ) ) ); // a second time, so invalid crs is fetched from cache - QgsCoordinateReferenceSystem crs4 = QgsCoordinateReferenceSystem::fromOgcWmsCrs( QStringLiteral( "not a CRS" ) ); + QgsCoordinateReferenceSystem crs4; + QVERIFY( !crs4.createFromOgcWmsCrs( QStringLiteral( "not a CRS" ) ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache(); @@ -379,12 +383,12 @@ void TestQgsCoordinateReferenceSystem::sridCache() // invalid QgsCoordinateReferenceSystem crs3; - crs3.createFromPostgisSrid( -3141 ); + QVERIFY( !crs3.createFromPostgisSrid( -3141 ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::srIdCache().contains( -3141 ) ); // a second time, so invalid crs is fetched from cache QgsCoordinateReferenceSystem crs4; - crs4.createFromPostgisSrid( -3141 ); + QVERIFY( !crs4.createFromPostgisSrid( -3141 ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache(); @@ -535,24 +539,24 @@ void TestQgsCoordinateReferenceSystem::wktCache() { // test that crs can be retrieved correctly from cache QgsCoordinateReferenceSystem crs; - crs.createFromWkt( geoWkt() ); + QVERIFY( crs.createFromWkt( geoWkt() ) ); QVERIFY( crs.isValid() ); QCOMPARE( crs.srsid(), GEOCRS_ID ); QVERIFY( QgsCoordinateReferenceSystem::wktCache().contains( geoWkt() ) ); // a second time, so crs is fetched from cache QgsCoordinateReferenceSystem crs2; - crs2.createFromWkt( geoWkt() ); + QVERIFY( crs2.createFromWkt( geoWkt() ) ); QVERIFY( crs2.isValid() ); QCOMPARE( crs2.srsid(), GEOCRS_ID ); // invalid QgsCoordinateReferenceSystem crs3; - crs3.createFromWkt( QStringLiteral( "bad wkt" ) ); + QVERIFY( !crs3.createFromWkt( QStringLiteral( "bad wkt" ) ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::wktCache().contains( QStringLiteral( "bad wkt" ) ) ); // a second time, so invalid crs is fetched from cache QgsCoordinateReferenceSystem crs4; - crs4.createFromWkt( QStringLiteral( "bad wkt" ) ); + QVERIFY( !crs4.createFromWkt( QStringLiteral( "bad wkt" ) ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache(); @@ -692,24 +696,24 @@ void TestQgsCoordinateReferenceSystem::srsIdCache() { // test that crs can be retrieved correctly from cache QgsCoordinateReferenceSystem crs; - crs.createFromSrsId( GEOCRS_ID ); + QVERIFY( crs.createFromSrsId( GEOCRS_ID ) ); QVERIFY( crs.isValid() ); QCOMPARE( crs.srsid(), GEOCRS_ID ); QVERIFY( QgsCoordinateReferenceSystem::srsIdCache().contains( GEOCRS_ID ) ); // a second time, so crs is fetched from cache QgsCoordinateReferenceSystem crs2; - crs2.createFromSrsId( GEOCRS_ID ); + QVERIFY( crs2.createFromSrsId( GEOCRS_ID ) ); QVERIFY( crs2.isValid() ); QCOMPARE( crs2.srsid(), GEOCRS_ID ); // invalid QgsCoordinateReferenceSystem crs3; - crs3.createFromSrsId( -5141 ); + QVERIFY( !crs3.createFromSrsId( -5141 ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::srsIdCache().contains( -5141 ) ); // a second time, so invalid crs is fetched from cache QgsCoordinateReferenceSystem crs4; - crs4.createFromSrsId( -5141 ); + QVERIFY( !crs4.createFromSrsId( -5141 ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache(); @@ -749,24 +753,24 @@ void TestQgsCoordinateReferenceSystem::proj4Cache() { // test that crs can be retrieved correctly from cache QgsCoordinateReferenceSystem crs; - crs.createFromProj( geoProj4() ); + QVERIFY( crs.createFromProj( geoProj4() ) ); QVERIFY( crs.isValid() ); QCOMPARE( crs.srsid(), GEOCRS_ID ); QVERIFY( QgsCoordinateReferenceSystem::projCache().contains( geoProj4() ) ); // a second time, so crs is fetched from cache QgsCoordinateReferenceSystem crs2; - crs2.createFromProj( geoProj4() ); + QVERIFY( crs2.createFromProj( geoProj4() ) ); QVERIFY( crs2.isValid() ); QCOMPARE( crs2.srsid(), GEOCRS_ID ); // invalid QgsCoordinateReferenceSystem crs3; - crs3.createFromProj( QStringLiteral( "bad proj4" ) ); + QVERIFY( !crs3.createFromProj( QStringLiteral( "bad proj4" ) ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::projCache().contains( QStringLiteral( "bad proj4" ) ) ); // a second time, so invalid crs is fetched from cache QgsCoordinateReferenceSystem crs4; - crs4.createFromProj( QStringLiteral( "bad proj4" ) ); + QVERIFY( !crs4.createFromProj( QStringLiteral( "bad proj4" ) ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache(); @@ -822,24 +826,24 @@ void TestQgsCoordinateReferenceSystem::fromStringCache() { // test that crs can be retrieved correctly from cache QgsCoordinateReferenceSystem crs; - crs.createFromString( QStringLiteral( "EPSG:3113" ) ); + QVERIFY( crs.createFromString( QStringLiteral( "EPSG:3113" ) ) ); QVERIFY( crs.isValid() ); QCOMPARE( crs.authid(), QStringLiteral( "EPSG:3113" ) ); QVERIFY( QgsCoordinateReferenceSystem::stringCache().contains( QStringLiteral( "EPSG:3113" ) ) ); // a second time, so crs is fetched from cache QgsCoordinateReferenceSystem crs2; - crs2.createFromString( QStringLiteral( "EPSG:3113" ) ); + QVERIFY( crs2.createFromString( QStringLiteral( "EPSG:3113" ) ) ); QVERIFY( crs2.isValid() ); QCOMPARE( crs2.authid(), QStringLiteral( "EPSG:3113" ) ); // invalid QgsCoordinateReferenceSystem crs3; - crs3.createFromString( QStringLiteral( "bad string" ) ); + QVERIFY( !crs3.createFromString( QStringLiteral( "bad string" ) ) ); QVERIFY( !crs3.isValid() ); QVERIFY( QgsCoordinateReferenceSystem::stringCache().contains( QStringLiteral( "bad string" ) ) ); // a second time, so invalid crs is fetched from cache QgsCoordinateReferenceSystem crs4; - crs4.createFromString( QStringLiteral( "bad string" ) ); + QVERIFY( !crs4.createFromString( QStringLiteral( "bad string" ) ) ); QVERIFY( !crs4.isValid() ); QgsCoordinateReferenceSystem::invalidateCache();