From 97d6060e596d1b044f84e7d140b26200ef56f65e Mon Sep 17 00:00:00 2001 From: Even Rouault Date: Mon, 13 May 2019 09:33:38 -0500 Subject: [PATCH] identify(): take into account the authority passed in (fixes #1465) When identifying an object that has already a code with authority A but the authority of interest passed was B, then it was not checking that A != B, and did not try to search in the objects of B. --- src/iso19111/crs.cpp | 134 +++++++++++++++++++++++++-------------- test/unit/test_c_api.cpp | 32 ++++++++++ test/unit/test_crs.cpp | 32 ++++++++-- 3 files changed, 147 insertions(+), 51 deletions(-) diff --git a/src/iso19111/crs.cpp b/src/iso19111/crs.cpp index 1ef7dcdacf..476bc72b1d 100644 --- a/src/iso19111/crs.cpp +++ b/src/iso19111/crs.cpp @@ -1387,6 +1387,36 @@ GeodeticCRSNNPtr GeodeticCRS::createEPSG_4978() { // --------------------------------------------------------------------------- +//! @cond Doxygen_Suppress + +static bool hasCodeCompatibleOfAuthorityFactory( + const common::IdentifiedObject *obj, + const io::AuthorityFactoryPtr &authorityFactory) { + const auto &ids = obj->identifiers(); + if (!ids.empty() && authorityFactory->getAuthority().empty()) { + return true; + } + for (const auto &id : ids) { + if (*(id->codeSpace()) == authorityFactory->getAuthority()) { + return true; + } + } + return false; +} + +static bool hasCodeCompatibleOfAuthorityFactory( + const metadata::IdentifierNNPtr &id, + const io::AuthorityFactoryPtr &authorityFactory) { + if (authorityFactory->getAuthority().empty()) { + return true; + } + return *(id->codeSpace()) == authorityFactory->getAuthority(); +} + +//! @endcond + +// --------------------------------------------------------------------------- + /** \brief Identify the CRS with reference CRSs. * * The candidate CRSs are either hard-coded, or looked in the database when @@ -1530,19 +1560,22 @@ GeodeticCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { searchByEllipsoid(); } } - } else if (!identifiers().empty()) { + } else if (hasCodeCompatibleOfAuthorityFactory(this, + authorityFactory)) { // If the CRS has already an id, check in the database for the // official object, and verify that they are equivalent. for (const auto &id : identifiers()) { - try { - auto crs = io::AuthorityFactory::create( - authorityFactory->databaseContext(), - *id->codeSpace()) - ->createGeodeticCRS(id->code()); - bool match = _isEquivalentTo(crs.get(), crsCriterion); - res.emplace_back(crs, match ? 100 : 25); - return res; - } catch (const std::exception &) { + if (hasCodeCompatibleOfAuthorityFactory(id, authorityFactory)) { + try { + auto crs = io::AuthorityFactory::create( + authorityFactory->databaseContext(), + *id->codeSpace()) + ->createGeodeticCRS(id->code()); + bool match = _isEquivalentTo(crs.get(), crsCriterion); + res.emplace_back(crs, match ? 100 : 25); + return res; + } catch (const std::exception &) { + } } } } else { @@ -2299,20 +2332,23 @@ VerticalCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { const bool unsignificantName = thisName.empty() || ci_equal(thisName, "unknown") || ci_equal(thisName, "unnamed"); - if (!identifiers().empty()) { + if (hasCodeCompatibleOfAuthorityFactory(this, authorityFactory)) { // If the CRS has already an id, check in the database for the // official object, and verify that they are equivalent. for (const auto &id : identifiers()) { - try { - auto crs = io::AuthorityFactory::create( - authorityFactory->databaseContext(), - *id->codeSpace()) - ->createVerticalCRS(id->code()); - bool match = _isEquivalentTo( - crs.get(), util::IComparable::Criterion::EQUIVALENT); - res.emplace_back(crs, match ? 100 : 25); - return res; - } catch (const std::exception &) { + if (hasCodeCompatibleOfAuthorityFactory(id, authorityFactory)) { + try { + auto crs = io::AuthorityFactory::create( + authorityFactory->databaseContext(), + *id->codeSpace()) + ->createVerticalCRS(id->code()); + bool match = _isEquivalentTo( + crs.get(), + util::IComparable::Criterion::EQUIVALENT); + res.emplace_back(crs, match ? 100 : 25); + return res; + } catch (const std::exception &) { + } } } } else if (!unsignificantName) { @@ -3100,21 +3136,24 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { ci_equal(thisName, "unnamed"); bool foundEquivalentName = false; - if (!identifiers().empty()) { + if (hasCodeCompatibleOfAuthorityFactory(this, authorityFactory)) { // If the CRS has already an id, check in the database for the // official object, and verify that they are equivalent. for (const auto &id : identifiers()) { - try { - auto crs = io::AuthorityFactory::create( - authorityFactory->databaseContext(), - *id->codeSpace()) - ->createProjectedCRS(id->code()); - bool match = _isEquivalentTo( - crs.get(), util::IComparable::Criterion:: - EQUIVALENT_EXCEPT_AXIS_ORDER_GEOGCRS); - res.emplace_back(crs, match ? 100 : 25); - return res; - } catch (const std::exception &) { + if (hasCodeCompatibleOfAuthorityFactory(id, authorityFactory)) { + try { + auto crs = io::AuthorityFactory::create( + authorityFactory->databaseContext(), + *id->codeSpace()) + ->createProjectedCRS(id->code()); + bool match = _isEquivalentTo( + crs.get(), + util::IComparable::Criterion:: + EQUIVALENT_EXCEPT_AXIS_ORDER_GEOGCRS); + res.emplace_back(crs, match ? 100 : 25); + return res; + } catch (const std::exception &) { + } } } } else if (!unsignificantName) { @@ -3189,8 +3228,8 @@ ProjectedCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { // Sort results res.sort(lambdaSort); - if (identifiers().empty() && !foundEquivalentName && - (res.empty() || res.front().second < 50)) { + if (!hasCodeCompatibleOfAuthorityFactory(this, authorityFactory) && + !foundEquivalentName && (res.empty() || res.front().second < 50)) { std::set> alreadyKnown; for (const auto &pair : res) { const auto &ids = pair.first->identifiers(); @@ -3448,20 +3487,23 @@ CompoundCRS::identify(const io::AuthorityFactoryPtr &authorityFactory) const { ci_equal(thisName, "unnamed"); bool foundEquivalentName = false; - if (!identifiers().empty()) { + if (hasCodeCompatibleOfAuthorityFactory(this, authorityFactory)) { // If the CRS has already an id, check in the database for the // official object, and verify that they are equivalent. for (const auto &id : identifiers()) { - try { - auto crs = io::AuthorityFactory::create( - authorityFactory->databaseContext(), - *id->codeSpace()) - ->createCompoundCRS(id->code()); - bool match = _isEquivalentTo( - crs.get(), util::IComparable::Criterion::EQUIVALENT); - res.emplace_back(crs, match ? 100 : 25); - return res; - } catch (const std::exception &) { + if (hasCodeCompatibleOfAuthorityFactory(id, authorityFactory)) { + try { + auto crs = io::AuthorityFactory::create( + authorityFactory->databaseContext(), + *id->codeSpace()) + ->createCompoundCRS(id->code()); + bool match = _isEquivalentTo( + crs.get(), + util::IComparable::Criterion::EQUIVALENT); + res.emplace_back(crs, match ? 100 : 25); + return res; + } catch (const std::exception &) { + } } } } else if (!unsignificantName) { diff --git a/test/unit/test_c_api.cpp b/test/unit/test_c_api.cpp index bea3eaa156..b9ea0bd5c5 100644 --- a/test/unit/test_c_api.cpp +++ b/test/unit/test_c_api.cpp @@ -1650,6 +1650,38 @@ TEST_F(CApi, proj_identify) { ObjListKeeper keeper_res(res); EXPECT_EQ(res, nullptr); } + { + auto obj2 = proj_create( + m_ctxt, "+proj=longlat +datum=WGS84 +no_defs +type=crs"); + ObjectKeeper keeper2(obj2); + ASSERT_NE(obj2, nullptr); + int *confidence = nullptr; + auto res = proj_identify(m_ctxt, obj2, nullptr, nullptr, &confidence); + ObjListKeeper keeper_res(res); + EXPECT_EQ(proj_list_get_count(res), 4); + proj_int_list_destroy(confidence); + } + { + auto obj2 = proj_create_from_database(m_ctxt, "IGNF", "ETRS89UTM28", + PJ_CATEGORY_CRS, false, nullptr); + ObjectKeeper keeper2(obj2); + ASSERT_NE(obj2, nullptr); + int *confidence = nullptr; + auto res = proj_identify(m_ctxt, obj2, "EPSG", nullptr, &confidence); + ObjListKeeper keeper_res(res); + EXPECT_EQ(proj_list_get_count(res), 1); + auto gotCRS = proj_list_get(m_ctxt, res, 0); + ASSERT_NE(gotCRS, nullptr); + ObjectKeeper keeper_gotCRS(gotCRS); + auto auth = proj_get_id_auth_name(gotCRS, 0); + ASSERT_TRUE(auth != nullptr); + EXPECT_EQ(auth, std::string("EPSG")); + auto code = proj_get_id_code(gotCRS, 0); + ASSERT_TRUE(code != nullptr); + EXPECT_EQ(code, std::string("25828")); + EXPECT_EQ(confidence[0], 70); + proj_int_list_destroy(confidence); + } } // --------------------------------------------------------------------------- diff --git a/test/unit/test_crs.cpp b/test/unit/test_crs.cpp index fbbd4f6421..0f0304b1c2 100644 --- a/test/unit/test_crs.cpp +++ b/test/unit/test_crs.cpp @@ -1984,13 +1984,35 @@ TEST(crs, projectedCRS_identify_no_db) { TEST(crs, projectedCRS_identify_db) { auto dbContext = DatabaseContext::create(); auto factoryEPSG = AuthorityFactory::create(dbContext, "EPSG"); + auto factoryIGNF = AuthorityFactory::create(dbContext, "IGNF"); + auto factoryAnonymous = AuthorityFactory::create(dbContext, std::string()); { // Identify by existing code - auto res = - factoryEPSG->createProjectedCRS("2172")->identify(factoryEPSG); - ASSERT_EQ(res.size(), 1U); - EXPECT_EQ(res.front().first->getEPSGCode(), 2172); - EXPECT_EQ(res.front().second, 100); + auto crs = factoryEPSG->createProjectedCRS("2172"); + { + auto res = crs->identify(factoryEPSG); + ASSERT_EQ(res.size(), 1U); + EXPECT_EQ(res.front().first->getEPSGCode(), 2172); + EXPECT_EQ(res.front().second, 100); + } + { + auto res = crs->identify(factoryAnonymous); + ASSERT_EQ(res.size(), 1U); + } + { + auto res = crs->identify(factoryIGNF); + ASSERT_EQ(res.size(), 0U); + } + } + { + // Identify by existing code + auto crs = factoryIGNF->createProjectedCRS("ETRS89UTM28"); + { + auto res = crs->identify(factoryEPSG); + ASSERT_EQ(res.size(), 1U); + EXPECT_EQ(res.front().first->getEPSGCode(), 25828); + EXPECT_EQ(res.front().second, 70); + } } { // Non-existing code