From de32614653a90fb4ecd59f73f20fbdfff3687588 Mon Sep 17 00:00:00 2001 From: vng Date: Sat, 8 May 2021 10:21:30 +0300 Subject: [PATCH] [OH] Fixed ranges like this "Mo-Sa 08:00-20:00; Dec 24 Mo-Sa 08:00-14:00". Signed-off-by: vng --- .github/workflows/linux-check.yaml | 2 + .../opening_hours_tests.cpp | 61 +++++++++++- 3party/opening_hours/parse_opening_hours.cpp | 3 +- 3party/opening_hours/rules_evaluation.cpp | 93 ++++++++++++++----- 4 files changed, 132 insertions(+), 27 deletions(-) diff --git a/.github/workflows/linux-check.yaml b/.github/workflows/linux-check.yaml index 7afa365516..a0bcc251a7 100644 --- a/.github/workflows/linux-check.yaml +++ b/.github/workflows/linux-check.yaml @@ -61,6 +61,8 @@ jobs: - name: Tests shell: bash + # Separate run of OH boost-based test + run: ./build/opening_hours_tests # generator_integration_tests - https://github.com/organicmaps/organicmaps/issues/225 # routing_integration_tests - https://github.com/organicmaps/organicmaps/issues/221 # routing_quality_tests - https://github.com/organicmaps/organicmaps/issues/215 diff --git a/3party/opening_hours/opening_hours_tests/opening_hours_tests.cpp b/3party/opening_hours/opening_hours_tests/opening_hours_tests.cpp index a1ac06ab5c..1559a805dc 100644 --- a/3party/opening_hours/opening_hours_tests/opening_hours_tests.cpp +++ b/3party/opening_hours/opening_hours_tests/opening_hours_tests.cpp @@ -871,11 +871,16 @@ BOOST_AUTO_TEST_CASE(OpeningHoursMonthdayRanges_TestParseUnparse) BOOST_AUTO_TEST_CASE(OpeningHoursYearRanges_TestParseUnparse) { + /// @todo Single year was removed here: + /// https://github.com/organicmaps/organicmaps/commit/ebe26a41da0744b3bc81d6b213406361f14d39b2 + /* { auto const rule = "1995"; auto const parsedUnparsed = ParseAndUnparse(rule); BOOST_CHECK_EQUAL(parsedUnparsed, rule); } + */ + { auto const rule = "1997+"; auto const parsedUnparsed = ParseAndUnparse(rule); @@ -1269,9 +1274,13 @@ BOOST_AUTO_TEST_CASE(OpeningHours_TestIsActive) BOOST_CHECK(GetTimeTuple("2015", fmt, time)); BOOST_CHECK(!IsActive(ranges[0], time)); } + + /// @todo Single year was removed here: + /// https://github.com/organicmaps/organicmaps/commit/ebe26a41da0744b3bc81d6b213406361f14d39b2 + /* { TYearRanges ranges; - BOOST_CHECK(Parse("2011", ranges)); + BOOST_REQUIRE(Parse("2011", ranges)); std::tm time; auto const fmt = "%Y"; @@ -1281,6 +1290,8 @@ BOOST_AUTO_TEST_CASE(OpeningHours_TestIsActive) BOOST_CHECK(GetTimeTuple("2012", fmt, time)); BOOST_CHECK(!IsActive(ranges[0], time)); } + */ + /// See https://en.wikipedia.org/wiki/ISO_week_date#First_week { TWeekRanges ranges; @@ -1495,7 +1506,7 @@ BOOST_AUTO_TEST_CASE(OpeningHours_TestIsOpen) BOOST_CHECK(IsOpen(rules, "2010-05-05 19:15")); BOOST_CHECK(IsClosed(rules, "2010-05-05 12:15")); - BOOST_CHECK(IsClosed(rules, "2010-04-10 15:15")); + BOOST_CHECK(IsClosed(rules, "2010-04-10 15:15")); // Saturday /// If no selectors with `open' modifier match than state is closed. BOOST_CHECK(IsClosed(rules, "2010-04-10 11:15")); @@ -1520,9 +1531,50 @@ BOOST_AUTO_TEST_CASE(OpeningHours_TestIsOpen) { TRuleSequences rules; BOOST_CHECK(Parse("PH open", rules)); + BOOST_CHECK(Parse("PH closed", rules)); + BOOST_CHECK(Parse("PH on", rules)); + BOOST_CHECK(Parse("PH off", rules)); - // Holidays are not supported yet. - BOOST_CHECK(IsClosed(rules, "2015-11-08 12:30")); + /// @todo Single PH entries are not supported yet, always closed?! + BOOST_CHECK(IsClosed(rules, "2021-05-07 11:23")); // Friday + BOOST_CHECK(IsClosed(rules, "2015-11-08 12:30")); // Sunday + } + { + TRuleSequences rules; + BOOST_CHECK(Parse("Mo-Sa 08:00-20:00; Dec Mo-Sa 08:00-14:00; Dec 25 off", rules)); + + BOOST_CHECK(IsClosed(rules, "2020-12-25 11:11")); + + BOOST_CHECK(IsOpen(rules, "2020-12-24 13:50")); // Thursday + BOOST_CHECK(IsClosed(rules, "2020-12-24 14:10")); // Thursday + BOOST_CHECK(IsClosed(rules, "2020-12-27 12:00")); // Sunday + + BOOST_CHECK(IsOpen(rules, "2021-05-07 13:50")); // Friday + BOOST_CHECK(IsOpen(rules, "2021-05-08 19:40")); // Saturdaya + BOOST_CHECK(IsClosed(rules, "2021-05-09 12:00")); // Sunday + } + + /// @todo Synthetic example with ill-formed OH, but documented behaviour. + /// @see rules_evaluation.cpp/IsR1IncludesR2 + /* + { + TRuleSequences rules; + BOOST_CHECK(Parse("Mo-Sa 08:00-20:00; Fr 08:00-14:00", rules)); + + BOOST_CHECK(IsOpen(rules, "2021-05-07 13:50")); // Friday + BOOST_CHECK(IsClosed(rules, "2021-05-07 14:10")); // Friday + } + */ + + { + TRuleSequences rules; + BOOST_CHECK(Parse("Mo-Sa 08:00-20:00; Dec 24 Mo-Sa 08:00-14:00; PH off", rules)); + + BOOST_CHECK(IsClosed(rules, "2020-12-24 14:10")); // Thursday + + BOOST_CHECK(IsOpen(rules, "2021-05-07 11:12")); // Friday + BOOST_CHECK(IsOpen(rules, "2021-05-08 13:14")); // Saturday + BOOST_CHECK(IsClosed(rules, "2021-05-09 15:16")); // Sunday } { TRuleSequences rules; @@ -1553,6 +1605,7 @@ BOOST_AUTO_TEST_CASE(OpeningHours_TestIsOpen) BOOST_CHECK(Parse("10:00-12:00", rules)); BOOST_CHECK(IsOpen(rules, "2013-12-12 10:01")); + BOOST_CHECK(IsClosed(rules, "2013-12-12 12:01")); } { TRuleSequences rules; diff --git a/3party/opening_hours/parse_opening_hours.cpp b/3party/opening_hours/parse_opening_hours.cpp index 4126b43001..a03130c514 100644 --- a/3party/opening_hours/parse_opening_hours.cpp +++ b/3party/opening_hours/parse_opening_hours.cpp @@ -78,7 +78,7 @@ namespace parsing ; rule_modifier = - (charset::no_case[lit("open")] + (charset::no_case[lit("open") | lit("on")] [bind(&RuleSequence::SetModifier, _r1, Modifier::Open)] >> -(comment [bind(&RuleSequence::SetModifierComment, _r1, _1)])) @@ -117,6 +117,7 @@ namespace parsing bool Parse(std::string const & str, TRuleSequences & context) { + context.clear(); return osmoh::ParseImpl(str, context); } } // namespace osmoh diff --git a/3party/opening_hours/rules_evaluation.cpp b/3party/opening_hours/rules_evaluation.cpp index bc6865b269..7897cbd838 100644 --- a/3party/opening_hours/rules_evaluation.cpp +++ b/3party/opening_hours/rules_evaluation.cpp @@ -323,60 +323,109 @@ bool IsActiveAny(std::vector const & selectors, std::tm const & date) return selectors.empty(); } -bool IsActive(RuleSequence const & rule, time_t const timestamp) +bool IsActiveAny(Timespan const & span, std::tm const & time) { return IsActive(span, time); } + +bool IsDayActive(RuleSequence const & rule, std::tm const & dt) +{ + return IsActiveAny(rule.GetYears(), dt) && IsActiveAny(rule.GetMonths(), dt) && + IsActiveAny(rule.GetWeeks(), dt) && IsActive(rule.GetWeekdays(), dt); +} + +template +std::pair MakeActiveResult(RuleSequence const & rule, std::tm const & dt, TTimeSpans const & times) +{ + if (IsDayActive(rule, dt)) + return { true, IsActiveAny(times, dt) }; + else + return {false, false}; +} + +/// @return [day active, time active]. +std::pair IsActiveImpl(RuleSequence const & rule, time_t const timestamp) { if (rule.IsTwentyFourHours()) - return true; - - auto const checkIsActive = [](RuleSequence const & rule, std::tm const & dt) - { - return IsActiveAny(rule.GetYears(), dt) && IsActiveAny(rule.GetMonths(), dt) && - IsActiveAny(rule.GetWeeks(), dt) && IsActive(rule.GetWeekdays(), dt); - }; + return {true, true}; auto const dateTimeTM = MakeTimetuple(timestamp); if (!HasExtendedHours(rule)) - return checkIsActive(rule, dateTimeTM) && IsActiveAny(rule.GetTimes(), dateTimeTM); + return MakeActiveResult(rule, dateTimeTM, rule.GetTimes()); TTimespans originalNormalizedSpans; Timespan additionalSpan; SplitExtendedHours(rule.GetTimes(), originalNormalizedSpans, additionalSpan); - if (checkIsActive(rule, dateTimeTM) && IsActiveAny(originalNormalizedSpans, dateTimeTM)) - return true; + auto const res1 = MakeActiveResult(rule, dateTimeTM, originalNormalizedSpans); + if (res1.first && res1.second) + return res1; time_t constexpr twentyFourHoursShift = 24 * 60 * 60; auto const dateTimeTMShifted = MakeTimetuple(timestamp - twentyFourHoursShift); - if (checkIsActive(rule, dateTimeTMShifted) && - IsActive(additionalSpan, dateTimeTMShifted)) + auto const res2 = MakeActiveResult(rule, dateTimeTMShifted, additionalSpan); + return { res1.first || res2.first, res2.second }; +} + +/// Check if r1 is more general range and includes r2. +bool IsR1IncludesR2(RuleSequence const & r1, RuleSequence const & r2) +{ + /// @todo Very naive implementation, but ok in most cases. + if ((r1.GetYears().empty() && !r2.GetYears().empty()) || + (r1.GetMonths().empty() && !r2.GetMonths().empty()) || + (r1.GetWeeks().empty() && !r2.GetWeeks().empty()) || + (r1.GetWeekdays().IsEmpty() && !r2.GetWeekdays().IsEmpty())) { return true; } - return false; } +bool IsActive(RuleSequence const & rule, time_t const timestamp) +{ + auto const res = IsActiveImpl(rule, timestamp); + return res.first && res.second; +} + RuleState GetState(TRuleSequences const & rules, time_t const timestamp) { - auto emptyRuleIt = rules.rend(); + RuleSequence const * emptyRule = nullptr; + RuleSequence const * dayMatchedRule = nullptr; + for (auto it = rules.rbegin(); it != rules.rend(); ++it) { - if (IsActive(*it, timestamp)) + auto const & rule = *it; + auto const res = IsActiveImpl(rule, timestamp); + if (!res.first) + continue; + + bool const empty = rule.IsEmpty(); + if (res.second) { - if (it->IsEmpty() && emptyRuleIt == rules.rend()) - emptyRuleIt = it; + if (empty && !emptyRule) + emptyRule = &rule; else - return ModifierToRuleState(it->GetModifier()); + { + // Should understand if previous 'rule vs dayMatchedRule' should work + // like 'general x BUT specific y' or like 'x OR y'. + + if (dayMatchedRule && IsR1IncludesR2(rule, *dayMatchedRule)) + return RuleState::Closed; + + return ModifierToRuleState(rule.GetModifier()); + } + } + else if (!empty && !dayMatchedRule && ModifierToRuleState(rule.GetModifier()) == RuleState::Open) + { + // Save recent day-matched rule with Open state, but not time-matched. + dayMatchedRule = &rule; } } - if (emptyRuleIt != rules.rend()) + if (emptyRule) { - if (emptyRuleIt->HasComment()) + if (emptyRule->HasComment()) return RuleState::Unknown; else - return ModifierToRuleState(emptyRuleIt->GetModifier()); + return ModifierToRuleState(emptyRule->GetModifier()); } return (rules.empty()