diff --git a/data/osm_test_data/named_address.osm b/data/osm_test_data/named_address.osm new file mode 100644 index 0000000000..62f044edd8 --- /dev/null +++ b/data/osm_test_data/named_address.osm @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/generator/feature_builder.cpp b/generator/feature_builder.cpp index c7d215416b..3d0842ff7c 100644 --- a/generator/feature_builder.cpp +++ b/generator/feature_builder.cpp @@ -197,6 +197,23 @@ bool FeatureBuilder::PreSerialize() if (!m_params.IsValid()) return false; + auto const checkHouseNumber = [this]() + { + if (!m_params.house.IsEmpty()) + { + // Hack/Patch here. Convert non-number into default name for the search index. + // Happens with building-address: https://github.com/organicmaps/organicmaps/issues/4994 + /// @todo Refactor to store raw name: and addr: values in FeatureBuilderParams and make one + /// _finalization_ function here. + auto const & hn = m_params.house.Get(); + if (FeatureParams::LooksLikeHouseNumber(hn) || !m_params.SetDefaultNameIfEmpty(hn)) + return true; + else + m_params.house.Clear(); + } + return false; + }; + // Conform serialization logic (see HeaderMask::HEADER_MASK_HAS_ADDINFO): // - rank (city) is stored only for Point // - ref (road number, address range) is stored only for Line @@ -204,9 +221,9 @@ bool FeatureBuilder::PreSerialize() switch (m_params.GetGeomType()) { case GeomType::Point: - // Store house number like HeaderGeomType::PointEx. - if (!m_params.house.IsEmpty()) + if (checkHouseNumber()) { + // Store house number like HeaderGeomType::PointEx. m_params.SetGeomTypePointEx(); m_params.rank = 0; } @@ -242,8 +259,22 @@ bool FeatureBuilder::PreSerialize() } case GeomType::Area: + checkHouseNumber(); + + if (!m_params.ref.empty()) + { + auto const & types = GetTypes(); + if (m_params.name.IsEmpty() && + (ftypes::IsRailwayStationChecker::Instance()(types) || + ftypes::IsPlatformChecker::Instance()(types))) + { + m_params.name.AddString(StringUtf8Multilang::kDefaultCode, m_params.ref); + } + + m_params.ref.clear(); + } + m_params.rank = 0; - m_params.ref.clear(); break; default: diff --git a/generator/generator_tests/feature_builder_test.cpp b/generator/generator_tests/feature_builder_test.cpp index 205bdf2f69..8df07f2b0d 100644 --- a/generator/generator_tests/feature_builder_test.cpp +++ b/generator/generator_tests/feature_builder_test.cpp @@ -38,8 +38,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_ManyTypes) AddTypes(params, arr); params.FinishAddingTypes(); - params.AddHouseNumber("75"); - params.AddHouseName("Best House"); + params.SetHouseNumberAndHouseName("75", "Best House"); params.AddName("default", "Name"); fb1.SetParams(params); @@ -224,8 +223,7 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_SerializeLocalityObjectForBuildi AddTypes(params, arr); params.FinishAddingTypes(); - params.AddHouseNumber("75"); - params.AddHouseName("Best House"); + params.SetHouseNumberAndHouseName("75", "Best House"); params.AddName("default", "Name"); fb.AddOsmId(base::MakeOsmNode(1)); @@ -244,6 +242,44 @@ UNIT_CLASS_TEST(TestWithClassificator, FBuilder_SerializeLocalityObjectForBuildi TEST(fb.PreSerializeAndRemoveUselessNamesForMwm(buffer), ()); } +UNIT_TEST(LooksLikeHouseNumber) +{ + TEST(FeatureParams::LooksLikeHouseNumber("1 bis"), ()); + TEST(FeatureParams::LooksLikeHouseNumber("18-20"), ()); + + // Brno (Czech) has a lot of fancy samples. + TEST(FeatureParams::LooksLikeHouseNumber("ev.8"), ()); + TEST(FeatureParams::LooksLikeHouseNumber("D"), ()); + TEST(FeatureParams::LooksLikeHouseNumber("A5"), ()); + + TEST(!FeatureParams::LooksLikeHouseNumber("Building 2"), ()); + TEST(!FeatureParams::LooksLikeHouseNumber("Unit 3"), ()); +} + +UNIT_CLASS_TEST(TestWithClassificator, FBuilder_HouseName) +{ + FeatureBuilder fb; + FeatureBuilderParams params; + + base::StringIL arr[] = {{ "building" }}; + AddTypes(params, arr); + params.FinishAddingTypes(); + + params.SetHouseNumberAndHouseName("", "St. Nicholas Lodge"); + + fb.SetParams(params); + fb.AssignArea({{0, 0}, {0, 1}, {1, 1}, {1, 0}, {0, 0}}, {}); + fb.SetArea(); + + TEST(fb.RemoveInvalidTypes(), ()); + TEST(fb.IsValid(), ()); + + TEST(fb.PreSerializeAndRemoveUselessNamesForIntermediate(), ()); + TEST(fb.IsValid(), ()); + TEST_EQUAL(fb.GetName(StringUtf8Multilang::kDefaultCode), "St. Nicholas Lodge", ()); + TEST(fb.GetParams().house.IsEmpty(), ()); +} + UNIT_CLASS_TEST(TestWithClassificator, FBuilder_SerializeAccuratelyForIntermediate) { FeatureBuilder fb1; diff --git a/generator/generator_tests/osm_type_test.cpp b/generator/generator_tests/osm_type_test.cpp index 707585d047..308501a785 100644 --- a/generator/generator_tests/osm_type_test.cpp +++ b/generator/generator_tests/osm_type_test.cpp @@ -243,6 +243,27 @@ UNIT_CLASS_TEST(TestWithClassificator, OsmType_Address) TEST_EQUAL(params.GetAddressData().Get(AddrType::Street), "Leutschenbachstrasse", ()); TEST_EQUAL(params.GetAddressData().Get(AddrType::Postcode), "8050", ()); } + + { + Tags const tags = { + {"addr:city", "Šķaune"}, + {"addr:country", "LV"}, + {"addr:district", "Krāslavas novads"}, + {"addr:housename", "Rozemnieki"}, + {"addr:postcode", "LV-5695"}, + {"addr:subdistrict", "Šķaunes pagasts"}, + {"ref:LV:addr", "104934702"}, + }; + + auto const params = GetFeatureBuilderParams(tags); + + TEST_EQUAL(params.m_types.size(), 1, (params)); + TEST(params.IsTypeExist(addrType), ()); + + TEST_EQUAL(params.house.Get(), "Rozemnieki", ()); + TEST(params.GetAddressData().Get(AddrType::Street).empty(), ()); + TEST_EQUAL(params.GetAddressData().Get(AddrType::Postcode), "LV-5695", ()); + } } UNIT_CLASS_TEST(TestWithClassificator, OsmType_PlaceState) diff --git a/generator/generator_tests/raw_generator_test.cpp b/generator/generator_tests/raw_generator_test.cpp index fb6d856f28..33831544a2 100644 --- a/generator/generator_tests/raw_generator_test.cpp +++ b/generator/generator_tests/raw_generator_test.cpp @@ -904,4 +904,33 @@ UNIT_CLASS_TEST(TestRawGenerator, Addr_Interpolation) TEST_EQUAL(count, 1, ()); } +// https://github.com/organicmaps/organicmaps/issues/4994 +UNIT_CLASS_TEST(TestRawGenerator, NamedAddress) +{ + std::string const mwmName = "Address"; + + BuildFB("./data/osm_test_data/named_address.osm", mwmName); + + uint32_t const addrType = classif().GetTypeByPath({"building", "address"}); + + size_t withName = 0, withNumber = 0; + ForEachFB(mwmName, [&](feature::FeatureBuilder const & fb) + { + TEST_EQUAL(fb.GetGeomType(), feature::GeomType::Point, ()); + TEST(fb.HasType(addrType), ()); + + TEST(fb.GetParams().house.IsEmpty() != fb.GetName().empty(), ()); + if (fb.GetParams().house.IsEmpty()) + ++withName; + else + ++withNumber; + + TEST(fb.GetAddressData().Get(feature::AddressData::Type::Street).empty(), ()); + TEST_EQUAL(fb.GetAddressData().Get(feature::AddressData::Type::Postcode), "LV-5695", ()); + }); + + TEST_EQUAL(withName, 3, ()); + TEST_EQUAL(withNumber, 0, ()); +} + } // namespace raw_generator_tests diff --git a/generator/osm2type.cpp b/generator/osm2type.cpp index d3714c811e..685a2f6494 100644 --- a/generator/osm2type.cpp +++ b/generator/osm2type.cpp @@ -1069,16 +1069,17 @@ void GetNameAndType(OsmElement * p, FeatureBuilderParams & params, namesExtractor.Finish(); // Stage3: Process base feature tags. + std::string houseName, houseNumber; TagProcessor(p).ApplyRules({ {"addr:housenumber", "*", - [¶ms](string & k, string & v) { - params.AddHouseName(v); + [&houseNumber](string & k, string & v) { + houseNumber = std::move(v); k.clear(); v.clear(); }}, {"addr:housename", "*", - [¶ms](string & k, string & v) { - params.AddHouseName(v); + [&houseName](string & k, string & v) { + houseName = std::move(v); k.clear(); v.clear(); }}, @@ -1119,6 +1120,8 @@ void GetNameAndType(OsmElement * p, FeatureBuilderParams & params, }}, }); + params.SetHouseNumberAndHouseName(std::move(houseNumber), std::move(houseName)); + // Stage4: Match tags to classificator feature types via mapcss-mapping.csv. MatchTypes(p, params, filterType); diff --git a/indexer/feature_data.cpp b/indexer/feature_data.cpp index 6bfe70358d..cfa29a6262 100644 --- a/indexer/feature_data.cpp +++ b/indexer/feature_data.cpp @@ -241,6 +241,16 @@ void FeatureParamsBase::MakeZero() name.Clear(); } +bool FeatureParamsBase::SetDefaultNameIfEmpty(std::string const & s) +{ + std::string_view existing; + if (name.GetString(StringUtf8Multilang::kDefaultCode, existing)) + return existing == s; + + name.AddString(StringUtf8Multilang::kDefaultCode, s); + return true; +} + bool FeatureParamsBase::operator == (FeatureParamsBase const & rhs) const { return (name == rhs.name && house == rhs.house && ref == rhs.ref && @@ -296,49 +306,55 @@ bool FeatureParams::AddName(string_view lang, string_view s) return true; } -bool FeatureParams::AddHouseName(string const & s) +bool FeatureParams::LooksLikeHouseNumber(std::string const & hn) { - if (IsDummyName(s) || name.FindString(s) != StringUtf8Multilang::kUnsupportedLanguageCode) - return false; + // Very naive implementation to _lightly_ promote hn -> name (for search index) if suitable. + /// @todo Conform with search::LooksLikeHouseNumber. - // Most names are house numbers by statistics. - if (house.IsEmpty() && AddHouseNumber(s)) - return true; + ASSERT(!hn.empty(), ()); + size_t const sz = hn.size(); + return strings::IsASCIIDigit(hn[0]) || + (sz == 1 && strings::IsASCIILatin(hn[0])) || + std::count_if(hn.begin(), hn.end(), &strings::IsASCIIDigit) > 0.2 * sz; +} - // If we got a clear number, replace the house number with it. - // Example: housename=16th Street, housenumber=34 - if (strings::IsASCIINumeric(s)) +char const * FeatureParams::kHNLogTag = "HNLog"; + +void FeatureParams::SetHouseNumberAndHouseName(std::string houseNumber, std::string houseName) +{ + if (IsDummyName(houseName) || name.FindString(houseName) != StringUtf8Multilang::kUnsupportedLanguageCode) + houseName.clear(); + + if (houseName.empty() && houseNumber.empty()) + return; + + if (houseName.empty()) + AddHouseNumber(houseNumber); + else if (houseNumber.empty()) + AddHouseNumber(houseName); + else { - string housename(house.Get()); - if (AddHouseNumber(s)) + if (!LooksLikeHouseNumber(houseNumber) && LooksLikeHouseNumber(houseName)) { - // Duplicating code to avoid changing the method header. - string_view dummy; - if (!name.GetString(StringUtf8Multilang::kDefaultCode, dummy)) - name.AddString(StringUtf8Multilang::kDefaultCode, housename); - return true; + LOG(LWARNING, (kHNLogTag, "Fancy house name:", houseName, "and number:", houseNumber)); + houseNumber.swap(houseName); } - } - // Add as a default name if we don't have it yet. - string_view dummy; - if (!name.GetString(StringUtf8Multilang::kDefaultCode, dummy)) - { - name.AddString(StringUtf8Multilang::kDefaultCode, s); - return true; + AddHouseNumber(std::move(houseNumber)); + SetDefaultNameIfEmpty(houseName); } - - return false; } bool FeatureParams::AddHouseNumber(string houseNumber) { - ASSERT(!houseNumber.empty(), ("This check should be done by the caller.")); - ASSERT_NOT_EQUAL(houseNumber.front(), ' ', ("Trim should be done by the caller.")); + ASSERT(!houseNumber.empty(), ()); // Negative house numbers are not supported. if (houseNumber.front() == '-' || houseNumber.find("－") == 0) + { + LOG(LWARNING, (kHNLogTag, "Negative house number:", houseNumber)); return false; + } // Replace full-width digits, mostly in Japan, by ascii-ones. strings::NormalizeDigits(houseNumber); @@ -350,12 +366,10 @@ bool FeatureParams::AddHouseNumber(string houseNumber) ++i; houseNumber.erase(0, i); - if (any_of(houseNumber.cbegin(), houseNumber.cend(), &strings::IsASCIIDigit)) - { - house.Set(houseNumber); - return true; - } - return false; + // Assign house number as-is to create building-address if needed. + // Final checks are made in FeatureBuilder::PreSerialize(). + house.Set(houseNumber); + return true; } void FeatureParams::SetGeomType(feature::GeomType t) diff --git a/indexer/feature_data.hpp b/indexer/feature_data.hpp index 10d0a75b84..8929212850 100644 --- a/indexer/feature_data.hpp +++ b/indexer/feature_data.hpp @@ -154,6 +154,7 @@ struct FeatureParamsBase FeatureParamsBase() : layer(feature::LAYER_EMPTY), rank(0) {} void MakeZero(); + bool SetDefaultNameIfEmpty(std::string const & s); bool operator == (FeatureParamsBase const & rhs) const; @@ -226,13 +227,17 @@ struct FeatureParamsBase class FeatureParams : public FeatureParamsBase { + static char const * kHNLogTag; + public: using Types = std::vector; void ClearName(); bool AddName(std::string_view lang, std::string_view s); - bool AddHouseName(std::string const & s); + + static bool LooksLikeHouseNumber(std::string const & hn); + void SetHouseNumberAndHouseName(std::string houseNumber, std::string houseName); bool AddHouseNumber(std::string houseNumber); void SetGeomType(feature::GeomType t);