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 committed Oct 16, 2020
1 parent 9883c42 commit ae27767
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
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}
}

Expand All @@ -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;
}
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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 -----------------------
Expand Down Expand Up @@ -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();
Expand All @@ -543,7 +543,7 @@ bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id )
if ( !sDisableSrIdCache )
sSrIdCache()->insert( id, *this );

return true;
return d->mIsValid;
}
}
}
Expand All @@ -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();
Expand All @@ -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;
}
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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
Expand Down
48 changes: 26 additions & 22 deletions tests/src/core/testqgscoordinatereferencesystem.cpp
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::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();
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::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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit ae27767

Please sign in to comment.