Skip to content

Commit

Permalink
FIX: QgsCoordinateReferenceSystem::createFrom* has to return CRS's va…
Browse files Browse the repository at this point in the history 
…lidity

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.
  • Loading branch information
@rldhont @nyalldawson
rldhont authored and nyalldawson committed Oct 23, 2020
1 parent cb010a3 commit 2eff708
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 36 deletions.
28 changes: 14 additions & 14 deletions src/core/qgscoordinatereferencesystem.cpp
View file
Expand Up @@ -276,7 +276,7 @@ bool QgsCoordinateReferenceSystem::createFromString( const QString &definition )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand Down Expand Up @@ -386,7 +386,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand All @@ -411,7 +411,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs )
locker.changeMode( QgsReadWriteLocker::Write );
if ( !sDisableOgcCache )
sOgcCache.insert( crs, *this );
return true;
return d->mIsValid;
}
}

Expand All @@ -430,7 +430,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs )
locker.changeMode( QgsReadWriteLocker::Write );
if ( !sDisableOgcCache )
sOgcCache.insert( crs, *this );
return true;
return d->mIsValid;
}
}
}
Expand All @@ -441,7 +441,7 @@ bool QgsCoordinateReferenceSystem::createFromOgcWmsCrs( const QString &crs )
locker.changeMode( QgsReadWriteLocker::Write );
if ( !sDisableOgcCache )
sOgcCache.insert( crs, *this );
return true;
return d->mIsValid;
}

// NAD27
Expand Down Expand Up @@ -480,7 +480,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 -----------------------
Expand Down Expand Up @@ -511,7 +511,7 @@ bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand All @@ -531,7 +531,7 @@ bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id )
if ( !sDisableSrIdCache )
sSrIdCache.insert( id, *this );

return true;
return d->mIsValid;
}
}
}
Expand All @@ -556,7 +556,7 @@ bool QgsCoordinateReferenceSystem::createFromSrsId( const long id )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand All @@ -575,7 +575,7 @@ bool QgsCoordinateReferenceSystem::createFromSrsId( const long id )
locker.changeMode( QgsReadWriteLocker::Write );
if ( !sDisableSrsIdCache )
sSrsIdCache.insert( id, *this );
return true;
return d->mIsValid;
}
}
}
Expand Down Expand Up @@ -836,7 +836,7 @@ bool QgsCoordinateReferenceSystem::createFromWkt( const QString &wkt )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand Down Expand Up @@ -914,7 +914,7 @@ bool QgsCoordinateReferenceSystem::createFromProj( const QString &projString )
{
// found a match in the cache
*this = crsIt.value();
return true;
return d->mIsValid;
}
}
locker.unlock();
Expand Down Expand Up @@ -949,7 +949,7 @@ bool QgsCoordinateReferenceSystem::createFromProj( const QString &projString )
locker.changeMode( QgsReadWriteLocker::Write );
if ( !sDisableProj4Cache )
sProj4Cache.insert( projString, *this );
return true;
return d->mIsValid;
}
}
}
Expand Down Expand Up @@ -1602,7 +1602,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
Expand Down
48 changes: 26 additions & 22 deletions tests/src/core/testqgscoordinatereferencesystem.cpp
View file
Expand Up @@ -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::sOgcCache.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::sOgcCache.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();
Expand Down Expand Up @@ -379,12 +383,12 @@ void TestQgsCoordinateReferenceSystem::sridCache()

// invalid
QgsCoordinateReferenceSystem crs3;
crs3.createFromPostgisSrid( -3141 );
QVERIFY( !crs3.createFromPostgisSrid( -3141 ) );
QVERIFY( !crs3.isValid() );
QVERIFY( QgsCoordinateReferenceSystem::sSrIdCache.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();
Expand Down Expand Up @@ -528,24 +532,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::sWktCache.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::sWktCache.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();
Expand Down Expand Up @@ -685,24 +689,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::sSrsIdCache.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::sSrsIdCache.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();
Expand Down Expand Up @@ -742,24 +746,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::sProj4Cache.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::sProj4Cache.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();
Expand Down Expand Up @@ -811,24 +815,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::sStringCache.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::sStringCache.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();
Expand Down

0 comments on commit 2eff708

Please sign in to comment.