From c52b037daa33ca8124029e4b0ff979b115f627fe Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Thu, 31 Oct 2024 23:46:04 +0100 Subject: [PATCH 1/2] Fail consistently on network error and grid transformations 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 /~https://github.com/pyproj4/pyproj/issues/705 --- src/iso19111/c_api.cpp | 6 +- src/transformations/gridshift.cpp | 2 +- src/transformations/hgridshift.cpp | 4 +- src/transformations/vgridshift.cpp | 4 +- src/transformations/xyzgridshift.cpp | 2 +- test/unit/test_network.cpp | 229 ++++++++++++++++++++++++++- 6 files changed, 232 insertions(+), 15 deletions(-) diff --git a/src/iso19111/c_api.cpp b/src/iso19111/c_api.cpp index f2eca319ae..7c11af9fa8 100644 --- a/src/iso19111/c_api.cpp +++ b/src/iso19111/c_api.cpp @@ -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; diff --git a/src/transformations/gridshift.cpp b/src/transformations/gridshift.cpp index fc516b1150..d5524571ab 100644 --- a/src/transformations/gridshift.cpp +++ b/src/transformations/gridshift.cpp @@ -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; diff --git a/src/transformations/hgridshift.cpp b/src/transformations/hgridshift.cpp index 4be74159fb..a85298b82c 100644 --- a/src/transformations/hgridshift.cpp +++ b/src/transformations/hgridshift.cpp @@ -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()) { @@ -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()) { diff --git a/src/transformations/vgridshift.cpp b/src/transformations/vgridshift.cpp index 6460bfd644..49f642d9b9 100644 --- a/src/transformations/vgridshift.cpp +++ b/src/transformations/vgridshift.cpp @@ -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()) { @@ -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()) { diff --git a/src/transformations/xyzgridshift.cpp b/src/transformations/xyzgridshift.cpp index 68281d1e7c..6b53c0df36 100644 --- a/src/transformations/xyzgridshift.cpp +++ b/src/transformations/xyzgridshift.cpp @@ -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; diff --git a/test/unit/test_network.cpp b/test/unit/test_network.cpp index 8eaeb3da80..7cad3bb190 100644 --- a/test/unit/test_network.cpp +++ b/test/unit/test_network.cpp @@ -1234,26 +1234,241 @@ 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); + c.xyzt.x = 40; // lat + c.xyzt.y = -80; // long + c.xyzt.z = 0; + c.xyzt.t = HUGE_VAL; + + { + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "NAD83 to NAD83(HARN) (47)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "NAD83 to NAD83(HARN) (47)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "NAD83 to NAD83(HARN) (47)"); + proj_destroy(last_op); + } 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.xyzt.x = 49; // lat + c.xyzt.y = 2; // long + c.xyzt.z = 0; + c.xyzt.t = HUGE_VAL; + + { + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "NTF to RGF93 v1 (1)"); + proj_destroy(last_op); + } + + 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.xyzt.x = 48; // lat + c.xyzt.y = 15; // long + c.xyzt.z = 0; + c.xyzt.t = HUGE_VAL; + + { + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, "MGI to ETRS89 (8)"); + proj_destroy(last_op); + } + + 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.xyzt.x = -40; // lat + c.xyzt.y = -60; // long + c.xyzt.z = 0; + c.xyzt.t = HUGE_VAL; + + { + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "POSGAR 2007 to SRVN16 height (1)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "POSGAR 2007 to SRVN16 height (1)"); + proj_destroy(last_op); + } + + 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); + PJ *last_op = proj_trans_get_last_used_operation(P); + EXPECT_STREQ(proj_pj_info(last_op).description, + "POSGAR 2007 to SRVN16 height (1)"); + proj_destroy(last_op); + } + + proj_destroy(P); + proj_context_destroy(ctx); } #endif From 0f543d209f6f26dced0ba2710f4942c3ed6b8dce Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Fri, 1 Nov 2024 16:10:35 +0100 Subject: [PATCH 2/2] Defered grid loading: do not try to re-open a grid that previously failed to open from the same PJ* instance --- src/transformations/gridshift.cpp | 11 ++++++++--- src/transformations/hgridshift.cpp | 21 +++++++++++++-------- src/transformations/vgridshift.cpp | 21 +++++++++++++-------- src/transformations/xyzgridshift.cpp | 11 +++++++---- 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/src/transformations/gridshift.cpp b/src/transformations/gridshift.cpp index d5524571ab..f97e8b59ce 100644 --- a/src/transformations/gridshift.cpp +++ b/src/transformations/gridshift.cpp @@ -80,6 +80,7 @@ struct GridInfo { struct gridshiftData { ListOfGenericGrids m_grids{}; bool m_defer_grid_opening = false; + int m_error_code_in_defer_grid_opening = 0; bool m_bHasHorizontalOffset = false; bool m_bHasGeographic3DOffset = false; bool m_bHasEllipsoidalHeightOffset = false; @@ -724,12 +725,16 @@ PJ_XYZ gridshiftData::grid_apply_internal( // --------------------------------------------------------------------------- bool gridshiftData::loadGridsIfNeeded(PJ *P) { - if (m_defer_grid_opening) { + if (m_error_code_in_defer_grid_opening) { + proj_errno_set(P, m_error_code_in_defer_grid_opening); + return false; + } else if (m_defer_grid_opening) { + m_defer_grid_opening = false; m_grids = pj_generic_grid_init(P, "grids"); - if (proj_errno(P)) { + m_error_code_in_defer_grid_opening = proj_errno(P); + if (m_error_code_in_defer_grid_opening) { return false; } - m_defer_grid_opening = false; bool isProjectedCoord; if (!checkGridTypes(P, isProjectedCoord)) { return false; diff --git a/src/transformations/hgridshift.cpp b/src/transformations/hgridshift.cpp index a85298b82c..45d939fc40 100644 --- a/src/transformations/hgridshift.cpp +++ b/src/transformations/hgridshift.cpp @@ -22,6 +22,7 @@ struct hgridshiftData { double t_epoch = 0; ListOfHGrids grids{}; bool defer_grid_opening = false; + int error_code_in_defer_grid_opening = 0; }; } // anonymous namespace @@ -31,11 +32,13 @@ static PJ_XYZ pj_hgridshift_forward_3d(PJ_LPZ lpz, PJ *P) { point.lpz = lpz; if (Q->defer_grid_opening) { - Q->grids = pj_hgrid_init(P, "grids"); - if (proj_errno(P)) { - return proj_coord_error().xyz; - } Q->defer_grid_opening = false; + Q->grids = pj_hgrid_init(P, "grids"); + Q->error_code_in_defer_grid_opening = proj_errno(P); + } + if (Q->error_code_in_defer_grid_opening) { + proj_errno_set(P, Q->error_code_in_defer_grid_opening); + return proj_coord_error().xyz; } if (!Q->grids.empty()) { @@ -53,11 +56,13 @@ static PJ_LPZ pj_hgridshift_reverse_3d(PJ_XYZ xyz, PJ *P) { point.xyz = xyz; if (Q->defer_grid_opening) { - Q->grids = pj_hgrid_init(P, "grids"); - if (proj_errno(P)) { - return proj_coord_error().lpz; - } Q->defer_grid_opening = false; + Q->grids = pj_hgrid_init(P, "grids"); + Q->error_code_in_defer_grid_opening = proj_errno(P); + } + if (Q->error_code_in_defer_grid_opening) { + proj_errno_set(P, Q->error_code_in_defer_grid_opening); + return proj_coord_error().lpz; } if (!Q->grids.empty()) { diff --git a/src/transformations/vgridshift.cpp b/src/transformations/vgridshift.cpp index 49f642d9b9..eef2806193 100644 --- a/src/transformations/vgridshift.cpp +++ b/src/transformations/vgridshift.cpp @@ -23,6 +23,7 @@ struct vgridshiftData { double forward_multiplier = 0; ListOfVGrids grids{}; bool defer_grid_opening = false; + int error_code_in_defer_grid_opening = 0; }; } // anonymous namespace @@ -57,12 +58,14 @@ 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; + Q->error_code_in_defer_grid_opening = proj_errno(P); + } + if (Q->error_code_in_defer_grid_opening) { + proj_errno_set(P, Q->error_code_in_defer_grid_opening); + return proj_coord_error().xyz; } if (!Q->grids.empty()) { @@ -81,12 +84,14 @@ 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; + Q->error_code_in_defer_grid_opening = proj_errno(P); + } + if (Q->error_code_in_defer_grid_opening) { + proj_errno_set(P, Q->error_code_in_defer_grid_opening); + return proj_coord_error().lpz; } if (!Q->grids.empty()) { diff --git a/src/transformations/xyzgridshift.cpp b/src/transformations/xyzgridshift.cpp index 6b53c0df36..aa682c7a53 100644 --- a/src/transformations/xyzgridshift.cpp +++ b/src/transformations/xyzgridshift.cpp @@ -45,6 +45,7 @@ struct xyzgridshiftData { bool grid_ref_is_input = true; ListOfGenericGrids grids{}; bool defer_grid_opening = false; + int error_code_in_defer_grid_opening = 0; double multiplier = 1.0; }; } // anonymous namespace @@ -54,11 +55,13 @@ 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->grids = pj_generic_grid_init(P, "grids"); - if (proj_errno(P)) { - return false; - } Q->defer_grid_opening = false; + Q->grids = pj_generic_grid_init(P, "grids"); + Q->error_code_in_defer_grid_opening = proj_errno(P); + } + if (Q->error_code_in_defer_grid_opening) { + proj_errno_set(P, Q->error_code_in_defer_grid_opening); + return false; } GenericShiftGridSet *gridset = nullptr;