Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Deprecate methods for constructing CRS from Postgis srids
Constructing CRS using Postgis srids is highly discouraged,
and instead CRSes should always be constructed using auth:id
codes or WKT strings.

QGIS 4.0: The logic should be isolated into the postgres
provider alone, and not exposed to stable API
  • Loading branch information
nyalldawson committed Dec 20, 2019
1 parent b37dd09 commit c76813c
Show file tree
Hide file tree
Showing 17 changed files with 75 additions and 112 deletions.
Expand Up @@ -349,13 +349,18 @@ and refer to QGIS internal CRS IDs.
%End


bool createFromSrid( long srid );
bool createFromSrid( long srid ) /Deprecated/;
%Docstring
Sets this CRS by lookup of the given PostGIS SRID in the CRS database.

:param srid: The PostGIS SRID for the desired spatial reference system.

:return: ``True`` on success else ``False``

.. deprecated::
Use alternative methods for SRS construction instead -- this
method was specifically created for use by the postgres provider alone,
and using it elsewhere will lead to subtle bugs.
%End

bool createFromWkt( const QString &wkt );
Expand Down
9 changes: 7 additions & 2 deletions src/core/qgscoordinatereferencesystem.cpp
Expand Up @@ -252,7 +252,7 @@ bool QgsCoordinateReferenceSystem::createFromId( const long id, CrsType type )
result = createFromSrsId( id );
break;
case PostgisCrsId:
result = createFromSrid( id );
result = createFromPostgisSrid( id );
break;
case EpsgCrsId:
result = createFromOgcWmsCrs( QStringLiteral( "EPSG:%1" ).arg( id ) );
Expand Down Expand Up @@ -292,7 +292,7 @@ bool QgsCoordinateReferenceSystem::createFromString( const QString &definition )
else if ( authName == QLatin1String( "postgis" ) )
{
const long id = match.captured( 2 ).toLong();
result = createFromId( id, PostgisCrsId );
result = createFromPostgisSrid( id );
}
else if ( authName == QLatin1String( "esri" ) || authName == QLatin1String( "osgeo" ) || authName == QLatin1String( "ignf" ) || authName == QLatin1String( "zangi" ) || authName == QLatin1String( "iau2000" ) )
{
Expand Down Expand Up @@ -498,6 +498,11 @@ void QgsCoordinateReferenceSystem::validate()
}

bool QgsCoordinateReferenceSystem::createFromSrid( const long id )
{
return createFromPostgisSrid( id );
}

bool QgsCoordinateReferenceSystem::createFromPostgisSrid( const long id )
{
QgsReadWriteLocker locker( *sSrIdCacheLock(), QgsReadWriteLocker::Read );
if ( !sDisableSrIdCache )
Expand Down
11 changes: 9 additions & 2 deletions src/core/qgscoordinatereferencesystem.h
Expand Up @@ -208,7 +208,7 @@ class CORE_EXPORT QgsCoordinateReferenceSystem
enum CrsType
{
InternalCrsId, //!< Internal ID used by QGIS in the local SQLite database
PostgisCrsId, //!< SRID used in PostGIS
PostgisCrsId, //!< SRID used in PostGIS. DEPRECATED -- DO NOT USE
EpsgCrsId //!< EPSG code
};

Expand Down Expand Up @@ -364,8 +364,12 @@ class CORE_EXPORT QgsCoordinateReferenceSystem
* Sets this CRS by lookup of the given PostGIS SRID in the CRS database.
* \param srid The PostGIS SRID for the desired spatial reference system.
* \returns TRUE on success else FALSE
*
* \deprecated Use alternative methods for SRS construction instead -- this
* method was specifically created for use by the postgres provider alone,
* and using it elsewhere will lead to subtle bugs.
*/
bool createFromSrid( long srid );
Q_DECL_DEPRECATED bool createFromSrid( long srid ) SIP_DEPRECATED;

/**
* Sets this CRS using a WKT definition.
Expand Down Expand Up @@ -933,6 +937,9 @@ class CORE_EXPORT QgsCoordinateReferenceSystem
static const QHash< long, QgsCoordinateReferenceSystem > &srIdCache();

friend class TestQgsCoordinateReferenceSystem;
friend class QgsPostgresProvider;

bool createFromPostgisSrid( const long id );
};

Q_DECLARE_METATYPE( QgsCoordinateReferenceSystem )
Expand Down
6 changes: 0 additions & 6 deletions src/providers/db2/qgsdb2provider.cpp
Expand Up @@ -548,12 +548,6 @@ QgsCoordinateReferenceSystem QgsDb2Provider::crs() const
{
if ( !mCrs.isValid() && mSRId > 0 )
{
mCrs.createFromSrid( mSRId );
if ( mCrs.isValid() )
{
return mCrs;
}

// try to load crs from the database tables as a fallback
QSqlQuery query = QSqlQuery( mDatabase );
query.setForwardOnly( true );
Expand Down
2 changes: 1 addition & 1 deletion src/providers/gpx/qgsgpxprovider.cpp
Expand Up @@ -534,7 +534,7 @@ QString QgsGPXProvider::description() const

QgsCoordinateReferenceSystem QgsGPXProvider::crs() const
{
return QgsCoordinateReferenceSystem( GEOSRID, QgsCoordinateReferenceSystem::PostgisCrsId ); // use WGS84
return QgsCoordinateReferenceSystem( QStringLiteral( "EPSG:4326" ) );
}

QgsDataProvider *QgsGpxProviderMetadata::createProvider( const QString &uri, const QgsDataProvider::ProviderOptions &options )
Expand Down
4 changes: 3 additions & 1 deletion src/providers/postgres/qgspostgresprovider.cpp
Expand Up @@ -4328,7 +4328,9 @@ QgsCoordinateReferenceSystem QgsPostgresProvider::crs() const
{
QgsCoordinateReferenceSystem srs;
int srid = mRequestedSrid.isEmpty() ? mDetectedSrid.toInt() : mRequestedSrid.toInt();
srs.createFromSrid( srid );

// TODO QGIS 4 - move the logic from createFromSridInternal to sit within the postgres provider alone
srs.createFromPostgisSrid( srid );
if ( !srs.isValid() )
{
static QMutex sMutex;
Expand Down
64 changes: 22 additions & 42 deletions tests/src/core/testqgscoordinatereferencesystem.cpp
Expand Up @@ -337,7 +337,7 @@ void TestQgsCoordinateReferenceSystem::ogcWmsCrsCache()
void TestQgsCoordinateReferenceSystem::createFromSrid()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
myCrs.createFromPostgisSrid( GEOSRID );
debugPrint( myCrs );
QVERIFY( myCrs.isValid() );
QCOMPARE( myCrs.srsid(), GEOCRS_ID );
Expand All @@ -353,24 +353,24 @@ void TestQgsCoordinateReferenceSystem::sridCache()
{
// test that crs can be retrieved correctly from cache
QgsCoordinateReferenceSystem crs;
crs.createFromSrid( 3112 );
crs.createFromPostgisSrid( 3112 );
QVERIFY( crs.isValid() );
QCOMPARE( crs.authid(), QStringLiteral( "EPSG:3112" ) );
QVERIFY( QgsCoordinateReferenceSystem::srIdCache().contains( 3112 ) );
// a second time, so crs is fetched from cache
QgsCoordinateReferenceSystem crs2;
crs2.createFromSrid( 3112 );
crs2.createFromPostgisSrid( 3112 );
QVERIFY( crs2.isValid() );
QCOMPARE( crs2.authid(), QStringLiteral( "EPSG:3112" ) );

// invalid
QgsCoordinateReferenceSystem crs3;
crs3.createFromSrid( -3141 );
crs3.createFromPostgisSrid( -3141 );
QVERIFY( !crs3.isValid() );
QVERIFY( QgsCoordinateReferenceSystem::srIdCache().contains( -3141 ) );
// a second time, so invalid crs is fetched from cache
QgsCoordinateReferenceSystem crs4;
crs4.createFromSrid( -3141 );
crs4.createFromPostgisSrid( -3141 );
QVERIFY( !crs4.isValid() );

QgsCoordinateReferenceSystem::invalidateCache();
Expand Down Expand Up @@ -611,8 +611,7 @@ void TestQgsCoordinateReferenceSystem::createFromESRIWkt()
void TestQgsCoordinateReferenceSystem::createFromSrId()
{
QgsCoordinateReferenceSystem myCrs;
QVERIFY( myCrs.createFromSrid( GEOSRID ) );
debugPrint( myCrs );
QVERIFY( myCrs.createFromPostgisSrid( GEOSRID ) );
QVERIFY( myCrs.isValid() );
QCOMPARE( myCrs.srsid(), GEOCRS_ID );
}
Expand Down Expand Up @@ -778,10 +777,10 @@ void TestQgsCoordinateReferenceSystem::fromStringCache()

void TestQgsCoordinateReferenceSystem::isValid()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
QVERIFY( myCrs.isValid() );
debugPrint( myCrs );
QgsCoordinateReferenceSystem crs( QStringLiteral( "EPSG:4326" ) );
QVERIFY( crs.isValid() );
crs = QgsCoordinateReferenceSystem( QStringLiteral( "xxxxxxxxxxxxxxx" ) );
QVERIFY( !crs.isValid() );
}

void TestQgsCoordinateReferenceSystem::validate()
Expand Down Expand Up @@ -815,20 +814,14 @@ void TestQgsCoordinateReferenceSystem::validate()

void TestQgsCoordinateReferenceSystem::equality()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
QgsCoordinateReferenceSystem myCrs2;
myCrs2.createFromSrsId( GEOCRS_ID );
debugPrint( myCrs );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
QgsCoordinateReferenceSystem myCrs2( QStringLiteral( "EPSG:4326" ) );
QVERIFY( myCrs == myCrs2 );
}
void TestQgsCoordinateReferenceSystem::noEquality()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
QgsCoordinateReferenceSystem myCrs2;
myCrs2.createFromSrsId( 4327 );
debugPrint( myCrs );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
QgsCoordinateReferenceSystem myCrs2( QStringLiteral( "EPSG:4327" ) );
QVERIFY( myCrs != myCrs2 );
}

Expand All @@ -840,8 +833,7 @@ void TestQgsCoordinateReferenceSystem::equalityInvalid()
}
void TestQgsCoordinateReferenceSystem::readWriteXml()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
QVERIFY( myCrs.isValid() );
QDomDocument document( QStringLiteral( "test" ) );
QDomElement node = document.createElement( QStringLiteral( "crs" ) );
Expand Down Expand Up @@ -1056,15 +1048,13 @@ void TestQgsCoordinateReferenceSystem::customSrsValidation()
void TestQgsCoordinateReferenceSystem::postgisSrid()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
myCrs.createFromPostgisSrid( GEOSRID );
QVERIFY( myCrs.postgisSrid() == GEOSRID );
debugPrint( myCrs );
}
void TestQgsCoordinateReferenceSystem::ellipsoidAcronym()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
debugPrint( myCrs );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
#if PROJ_VERSION_MAJOR>=6
QCOMPARE( myCrs.ellipsoidAcronym(), QStringLiteral( "EPSG:7030" ) );
#else
Expand All @@ -1081,35 +1071,26 @@ void TestQgsCoordinateReferenceSystem::ellipsoidAcronym()
}
void TestQgsCoordinateReferenceSystem::toWkt()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
QString myWkt = myCrs.toWkt();
debugPrint( myCrs );
//Note: this is not the same as geoWkt() as OGR strips off the TOWGS clause...
QString myStrippedWkt( "GEOGCS[\"WGS 84\",DATUM[\"WGS_1984\",SPHEROID"
"[\"WGS 84\",6378137,298.257223563,AUTHORITY[\"EPSG\",\"7030\"]],"
"AUTHORITY[\"EPSG\",\"6326\"]],PRIMEM[\"Greenwich\",0,AUTHORITY"
"[\"EPSG\",\"8901\"]],UNIT[\"degree\",0.0174532925199433,AUTHORITY"
"[\"EPSG\",\"9122\"]],AUTHORITY[\"EPSG\",\"4326\"]]" );
qDebug() << "wkt: " << myWkt;
qDebug() << "stripped: " << myStrippedWkt;
QVERIFY( myWkt == myStrippedWkt );
QCOMPARE( myWkt, myStrippedWkt );
}
void TestQgsCoordinateReferenceSystem::toProj()
{
QgsCoordinateReferenceSystem myCrs;
myCrs.createFromSrid( GEOSRID );
debugPrint( myCrs );
QgsCoordinateReferenceSystem myCrs( QStringLiteral( "EPSG:4326" ) );
//first proj string produced by gdal 1.8-1.9
//second by gdal 1.7
QCOMPARE( myCrs.toProj(), geoProj4() );
}
void TestQgsCoordinateReferenceSystem::isGeographic()
{
QgsCoordinateReferenceSystem geographic;
geographic.createFromSrid( GEOSRID );
QgsCoordinateReferenceSystem geographic( QStringLiteral( "EPSG:4326" ) );
QVERIFY( geographic.isGeographic() );
debugPrint( geographic );

QgsCoordinateReferenceSystem nonGeographic;
nonGeographic.createFromId( 3857, QgsCoordinateReferenceSystem::EpsgCrsId );
Expand Down Expand Up @@ -1211,8 +1192,7 @@ void TestQgsCoordinateReferenceSystem::validSrsIds()

void TestQgsCoordinateReferenceSystem::asVariant()
{
QgsCoordinateReferenceSystem original;
original.createFromSrid( 3112 );
QgsCoordinateReferenceSystem original( QStringLiteral( "EPSG:3112" ) );

//convert to and from a QVariant
QVariant var = QVariant::fromValue( original );
Expand Down
18 changes: 6 additions & 12 deletions tests/src/core/testqgscoordinatetransform.cpp
Expand Up @@ -130,10 +130,8 @@ void TestQgsCoordinateTransform::isValid()
QgsCoordinateTransform tr;
QVERIFY( !tr.isValid() );

QgsCoordinateReferenceSystem srs1;
srs1.createFromSrid( 3994 );
QgsCoordinateReferenceSystem srs2;
srs2.createFromSrid( 4326 );
QgsCoordinateReferenceSystem srs1( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem srs2( QStringLiteral( "EPSG:4326" ) );

// valid source, invalid destination
QgsCoordinateTransform tr2( srs1, QgsCoordinateReferenceSystem(), QgsProject::instance() );
Expand Down Expand Up @@ -163,10 +161,8 @@ void TestQgsCoordinateTransform::isShortCircuited()
//invalid transform shortcircuits
QVERIFY( tr.isShortCircuited() );

QgsCoordinateReferenceSystem srs1;
srs1.createFromSrid( 3994 );
QgsCoordinateReferenceSystem srs2;
srs2.createFromSrid( 4326 );
QgsCoordinateReferenceSystem srs1( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem srs2( QStringLiteral( "EPSG:4326" ) );

// valid source, invalid destination
QgsCoordinateTransform tr2( srs1, QgsCoordinateReferenceSystem(), QgsProject::instance() );
Expand Down Expand Up @@ -400,10 +396,8 @@ void TestQgsCoordinateTransform::transform()
void TestQgsCoordinateTransform::transformBoundingBox()
{
//test transforming a bounding box which crosses the 180 degree longitude line
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4326 );
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4326" ) );

QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );
QgsRectangle crossingRect( 6374985, -3626584, 7021195, -3272435 );
Expand Down
36 changes: 12 additions & 24 deletions tests/src/core/testqgsgeometry.cpp
Expand Up @@ -837,10 +837,8 @@ void TestQgsGeometry::point()
QCOMPARE( p15.boundingBox(), QgsRectangle( 21.0, 23.0, 21.0, 23.0 ) );

//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) ); // want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );
QgsPoint p16( QgsWkbTypes::PointZM, 6374985, -3626584, 1, 2 );
p16.transform( tr, QgsCoordinateTransform::ForwardTransform );
Expand Down Expand Up @@ -1881,10 +1879,8 @@ void TestQgsGeometry::circularString()
QCOMPARE( points.at( 2 ), QgsPoint( QgsWkbTypes::PointZM, 15, 10, 6, 7 ) );

//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) );// want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );

// 2d CRS transform
Expand Down Expand Up @@ -3857,10 +3853,8 @@ void TestQgsGeometry::lineString()
QCOMPARE( points.at( 2 ), QgsPoint( QgsWkbTypes::PointZM, 15, 10, 6, 7 ) );

//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) );// want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );

// 2d CRS transform
Expand Down Expand Up @@ -5793,10 +5787,8 @@ void TestQgsGeometry::polygon()

//transform
//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) ); // want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );

// 2d CRS transform
Expand Down Expand Up @@ -10405,10 +10397,8 @@ void TestQgsGeometry::compoundCurve()
QCOMPARE( points.at( 3 ), QgsPoint( QgsWkbTypes::PointZM, 25, 10, 6, 7 ) );

//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) ); // want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );

// 2d CRS transform
Expand Down Expand Up @@ -15379,10 +15369,8 @@ void TestQgsGeometry::geometryCollection()

//transform
//CRS transform
QgsCoordinateReferenceSystem sourceSrs;
sourceSrs.createFromSrid( 3994 );
QgsCoordinateReferenceSystem destSrs;
destSrs.createFromSrid( 4202 ); // want a transform with ellipsoid change
QgsCoordinateReferenceSystem sourceSrs( QStringLiteral( "EPSG:3994" ) );
QgsCoordinateReferenceSystem destSrs( QStringLiteral( "EPSG:4202" ) ); // want a transform with ellipsoid change
QgsCoordinateTransform tr( sourceSrs, destSrs, QgsProject::instance() );

// 2d CRS transform
Expand Down

0 comments on commit c76813c

Please sign in to comment.