Skip to content

Commit

Permalink
Fail consistently on network error and grid transformations
Browse files Browse the repository at this point in the history 
Currently when we need to use a remote grid that can't be opened, we
return HUGE_VAL coordinates values and a proj_errno = PROJ_ERR_OTHER_NETWORK_ERROR,
(I guess) as expected... But if we do following proj_trans() calls on the same
transformation object, the grid transformation is ignored and we
fallback to other methods (Helmert, ballpark, etc.).
Fix that by consistently returning the same error values as the initial
failed call.

Fixes pyproj4/pyproj#705
  • Loading branch information
@rouault
rouault committed Oct 31, 2024
1 parent de73f24 commit 941ef08
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 12 deletions.
6 changes: 4 additions & 2 deletions src/iso19111/c_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,13 @@ PJ *pj_obj_create(PJ_CONTEXT *ctx, const BaseObjectNNPtr &objIn) {
PROJStringFormatter::Convention::PROJ_5,
std::move(dbContext));
auto projString = coordop->exportToPROJString(formatter.get());
if (proj_context_is_network_enabled(ctx)) {
const bool defer_grid_opening_backup = ctx->defer_grid_opening;
if (!defer_grid_opening_backup &&
proj_context_is_network_enabled(ctx)) {
ctx->defer_grid_opening = true;
}
auto pj = pj_create_internal(ctx, projString.c_str());
ctx->defer_grid_opening = false;
ctx->defer_grid_opening = defer_grid_opening_backup;
if (pj) {
pj->iso_obj = objIn;
pj->iso_obj_is_coordinate_operation = true;
Expand Down
2 changes: 1 addition & 1 deletion src/transformations/gridshift.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -725,11 +725,11 @@ PJ_XYZ gridshiftData::grid_apply_internal(

bool gridshiftData::loadGridsIfNeeded(PJ *P) {
if (m_defer_grid_opening) {
m_defer_grid_opening = false;
m_grids = pj_generic_grid_init(P, "grids");
if (proj_errno(P)) {
return false;
}
m_defer_grid_opening = false;
bool isProjectedCoord;
if (!checkGridTypes(P, isProjectedCoord)) {
return false;
Expand Down
4 changes: 2 additions & 2 deletions src/transformations/hgridshift.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ static PJ_XYZ pj_hgridshift_forward_3d(PJ_LPZ lpz, PJ *P) {
point.lpz = lpz;

if (Q->defer_grid_opening) {
Q->defer_grid_opening = false;
Q->grids = pj_hgrid_init(P, "grids");
if (proj_errno(P)) {
return proj_coord_error().xyz;
}
Q->defer_grid_opening = false;
}

if (!Q->grids.empty()) {
Expand All @@ -53,11 +53,11 @@ static PJ_LPZ pj_hgridshift_reverse_3d(PJ_XYZ xyz, PJ *P) {
point.xyz = xyz;

if (Q->defer_grid_opening) {
Q->defer_grid_opening = false;
Q->grids = pj_hgrid_init(P, "grids");
if (proj_errno(P)) {
return proj_coord_error().lpz;
}
Q->defer_grid_opening = false;
}

if (!Q->grids.empty()) {
Expand Down
4 changes: 2 additions & 2 deletions src/transformations/vgridshift.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ static PJ_XYZ pj_vgridshift_forward_3d(PJ_LPZ lpz, PJ *P) {
point.lpz = lpz;

if (Q->defer_grid_opening) {
Q->defer_grid_opening = false;
Q->grids = pj_vgrid_init(P, "grids");
deal_with_vertcon_gtx_hack(P);
if (proj_errno(P)) {
return proj_coord_error().xyz;
}
Q->defer_grid_opening = false;
}

if (!Q->grids.empty()) {
Expand All @@ -81,12 +81,12 @@ static PJ_LPZ pj_vgridshift_reverse_3d(PJ_XYZ xyz, PJ *P) {
point.xyz = xyz;

if (Q->defer_grid_opening) {
Q->defer_grid_opening = false;
Q->grids = pj_vgrid_init(P, "grids");
deal_with_vertcon_gtx_hack(P);
if (proj_errno(P)) {
return proj_coord_error().lpz;
}
Q->defer_grid_opening = false;
}

if (!Q->grids.empty()) {
Expand Down
2 changes: 1 addition & 1 deletion src/transformations/xyzgridshift.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ struct xyzgridshiftData {
static bool get_grid_values(PJ *P, xyzgridshiftData *Q, const PJ_LP &lp,
double &dx, double &dy, double &dz) {
if (Q->defer_grid_opening) {
Q->defer_grid_opening = false;
Q->grids = pj_generic_grid_init(P, "grids");
if (proj_errno(P)) {
return false;
}
Q->defer_grid_opening = false;
}

GenericShiftGridSet *gridset = nullptr;
Expand Down
213 changes: 209 additions & 4 deletions test/unit/test_network.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1234,26 +1234,231 @@ TEST(networking, network_endpoint_env_variable) {

#ifdef CURL_ENABLED

TEST(networking, network_endpoint_api) {
TEST(networking, network_endpoint_api_and_not_reachable_gridshift) {
auto ctx = proj_context_create();
proj_grid_cache_set_enable(ctx, false);
proj_context_set_enable_network(ctx, true);
proj_context_set_url_endpoint(ctx, "http://0.0.0.0");

// NAD83 to NAD83(HARN) in West-Virginia. Using wvhpgn.tif
// NAD83 to NAD83(HARN) using
// us_noaa_nadcon5_nad83_1986_nad83_harn_conus.tif
auto P = proj_create_crs_to_crs(ctx, "EPSG:4269", "EPSG:4152", nullptr);
ASSERT_NE(P, nullptr);

PJ_COORD c;
c.xyz.x = 40; // lat
c.xyz.y = -80; // long
c.xyz.z = 0;
c = proj_trans(P, PJ_FWD, c);

{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NAD83 to NAD83(HARN) (47)");
}

proj_errno_reset(P);

// Check again. Cf https://github.com/pyproj4/pyproj/issues/705
{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NAD83 to NAD83(HARN) (47)");
}

proj_errno_reset(P);

// Check also reverse direction
{
PJ_COORD c2 = proj_trans(P, PJ_INV, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NAD83 to NAD83(HARN) (47)");
}

proj_destroy(P);
proj_context_destroy(ctx);
}

EXPECT_EQ(c.xyz.x, HUGE_VAL);
#endif

// ---------------------------------------------------------------------------

#ifdef CURL_ENABLED

TEST(networking, network_endpoint_api_and_not_reachable_xyzgridshift) {
auto ctx = proj_context_create();
proj_grid_cache_set_enable(ctx, false);
proj_context_set_enable_network(ctx, true);
proj_context_set_url_endpoint(ctx, "http://0.0.0.0");

// NTF to RGF93 using fr_ign_gr3df97a.tif
auto P = proj_create_crs_to_crs(ctx, "EPSG:4275", "EPSG:4171", nullptr);
ASSERT_NE(P, nullptr);

PJ_COORD c;
c.xyz.x = 49; // lat
c.xyz.y = 2; // long
c.xyz.z = 0;

{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NTF to RGF93 v1 (1)");
}

proj_errno_reset(P);

// Check again. Cf https://github.com/pyproj4/pyproj/issues/705
{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NTF to RGF93 v1 (1)");
}

proj_errno_reset(P);

// Check also reverse direction
{
PJ_COORD c2 = proj_trans(P, PJ_INV, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"NTF to RGF93 v1 (1)");
}

proj_destroy(P);
proj_context_destroy(ctx);
}

#endif

// ---------------------------------------------------------------------------

#ifdef CURL_ENABLED

TEST(networking, network_endpoint_api_and_not_reachable_hgridshift) {
auto ctx = proj_context_create();
proj_grid_cache_set_enable(ctx, false);
proj_context_set_enable_network(ctx, true);
proj_context_set_url_endpoint(ctx, "http://0.0.0.0");

// MGI to ETRS89 using at_bev_AT_GIS_GRID_2021_09_28.tif
auto P = proj_create_crs_to_crs(ctx, "EPSG:4312", "EPSG:4258", nullptr);
ASSERT_NE(P, nullptr);

PJ_COORD c;
c.xyz.x = 48; // lat
c.xyz.y = 15; // long
c.xyz.z = 0;

{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"MGI to ETRS89 (8)");
}

proj_errno_reset(P);

// Check again. Cf https://github.com/pyproj4/pyproj/issues/705
{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"MGI to ETRS89 (8)");
}

proj_errno_reset(P);

// Check also reverse direction
{
PJ_COORD c2 = proj_trans(P, PJ_INV, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"MGI to ETRS89 (8)");
}

proj_destroy(P);
proj_context_destroy(ctx);
}

#endif

// ---------------------------------------------------------------------------

#ifdef CURL_ENABLED

TEST(networking, network_endpoint_api_and_not_reachable_vgridshift) {
auto ctx = proj_context_create();
proj_grid_cache_set_enable(ctx, false);
proj_context_set_enable_network(ctx, true);
proj_context_set_url_endpoint(ctx, "http://0.0.0.0");

// "POSGAR 2007 to SRVN16 height (1)" using ar_ign_GEOIDE-Ar16.tif
auto P = proj_create_crs_to_crs(ctx, "EPSG:5342", "EPSG:9521", nullptr);
ASSERT_NE(P, nullptr);

PJ_COORD c;
c.xyz.x = -40; // lat
c.xyz.y = -60; // long
c.xyz.z = 0;

{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"POSGAR 2007 to SRVN16 height (1)");
}

proj_errno_reset(P);

// Check again. Cf https://github.com/pyproj4/pyproj/issues/705
{
PJ_COORD c2 = proj_trans(P, PJ_FWD, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"POSGAR 2007 to SRVN16 height (1)");
}

proj_errno_reset(P);

// Check also reverse direction
{
PJ_COORD c2 = proj_trans(P, PJ_INV, c);
EXPECT_EQ(c2.xyz.x, HUGE_VAL);
EXPECT_EQ(proj_errno(P), PROJ_ERR_OTHER_NETWORK_ERROR);
EXPECT_STREQ(
proj_pj_info(proj_trans_get_last_used_operation(P)).description,
"POSGAR 2007 to SRVN16 height (1)");
}

proj_destroy(P);
proj_context_destroy(ctx);
}

#endif
Expand Down

0 comments on commit 941ef08

Please sign in to comment.