Merge pull request #1431 from Karry/malformed-opening-hours

improve opening hours parsing

Tests/src/OpeningHours.cpp

@@ -46,6 +46,32 @@ TEST_CASE("Parse simple opening hours")
   REQUIRE(oh->GetRules()[0].intervals[1].to.minute==30);
 }
 
+TEST_CASE("Parse simple opening hours with space")
+{
+  // space between time intervals should not be there, but be tolerant
+  auto oh=osmscout::OpeningHours::Parse("Mo-Fr 08:00-12:00, 13:00-17:30");
+  REQUIRE(oh != std::nullopt);
+
+  REQUIRE(oh->GetRules().size()==5);
+  REQUIRE(oh->GetRules()[0].day==osmscout::OpeningHours::WeekDay::Monday);
+  REQUIRE(oh->GetRules()[1].day==osmscout::OpeningHours::WeekDay::Tuesday);
+  REQUIRE(oh->GetRules()[2].day==osmscout::OpeningHours::WeekDay::Wednesday);
+  REQUIRE(oh->GetRules()[3].day==osmscout::OpeningHours::WeekDay::Thursday);
+  REQUIRE(oh->GetRules()[4].day==osmscout::OpeningHours::WeekDay::Friday);
+
+  REQUIRE(oh->GetRules()[0].intervals.size()==2);
+
+  REQUIRE(oh->GetRules()[0].intervals[0].from.hour==8);
+  REQUIRE(oh->GetRules()[0].intervals[0].from.minute==0);
+  REQUIRE(oh->GetRules()[0].intervals[0].to.hour==12);
+  REQUIRE(oh->GetRules()[0].intervals[0].to.minute==0);
+
+  REQUIRE(oh->GetRules()[0].intervals[1].from.hour==13);
+  REQUIRE(oh->GetRules()[0].intervals[1].from.minute==0);
+  REQUIRE(oh->GetRules()[0].intervals[1].to.hour==17);
+  REQUIRE(oh->GetRules()[0].intervals[1].to.minute==30);
+}
+
 TEST_CASE("Parse multiple rules")
 {
   auto oh=osmscout::OpeningHours::Parse("Mo-Fr 08:00-18:00; Sa 08:00-12:00");
@@ -79,3 +105,10 @@ TEST_CASE("Closed at public holidays")
   REQUIRE(oh->GetRules()[5].day==osmscout::OpeningHours::WeekDay::PublicHoliday);
   REQUIRE(oh->GetRules()[5].intervals.empty());
 }
+
+TEST_CASE("Wrong rule separator")
+{
+  // this value use comma instead of semicolon
+  auto oh = osmscout::OpeningHours::Parse("Mo-Th 11:00-24:00, Sa 11:30-24:00, Su 11:30-22:00, Fr 11:00-01:00");
+  REQUIRE(oh == std::nullopt);
+}

Tests/src/StringUtils.cpp

@@ -41,6 +41,36 @@ TEST_CASE("Split string by multi-character separator")
   REQUIRE(*(it++) == "gravel");
 }
 
+TEST_CASE("Split empty string to pair")
+{
+  auto elements = osmscout::SplitStringToPair("", ";");
+  REQUIRE_FALSE(elements.has_value());
+}
+
+TEST_CASE("Split string with multiple separators to pair")
+{
+  auto elements = osmscout::SplitStringToPair("a;b;c;", ";");
+  REQUIRE(elements.has_value());
+  REQUIRE(std::get<0>(elements.value())=="a");
+  REQUIRE(std::get<1>(elements.value())=="b;c;");
+}
+
+TEST_CASE("Split string with separator on the end")
+{
+  auto elements = osmscout::SplitStringToPair("a;", ";");
+  REQUIRE(elements.has_value());
+  REQUIRE(std::get<0>(elements.value())=="a");
+  REQUIRE(std::get<1>(elements.value()).empty());
+}
+
+TEST_CASE("Split string just with separator")
+{
+  auto elements = osmscout::SplitStringToPair(";", ";");
+  REQUIRE(elements.has_value());
+  REQUIRE(std::get<0>(elements.value()).empty());
+  REQUIRE(std::get<1>(elements.value()).empty());
+}
+
 TEST_CASE("Transliterate diacritics")
 {
   try {

libosmscout/include/osmscout/util/String.h

@@ -25,6 +25,8 @@
 #include <memory>
 #include <string>
 #include <chrono>
+#include <optional>
+#include <utility>
 
 #include <osmscout/CoreFeatures.h>
 
@@ -323,6 +325,17 @@ namespace osmscout {
                                                          const std::string& separator,
                                                          int maxSize=-1);
 
+  /**
+   * Split string by separator to two parts. Unlike SplitString with maxSize=2,
+   * second element contains the rest of the string after first separator.
+   * When no separator found, nullopt is returned.
+   *
+   * \note when str is empty nullopt is returned
+   * \note separator must not be empty
+   * \note when string ends with separator, and there is just one, second element is empty
+   */
+  extern OSMSCOUT_API std::optional<std::pair<std::string,std::string>> SplitStringToPair(const std::string& str,
+                                                                                          const std::string& separator);
 
   /**
    * \ingroup Util

libosmscout/src/osmscout/util/OpeningHours.cpp

@@ -80,7 +80,7 @@ std::optional<std::vector<OpeningHours::WeekDay>> ParseDayDescription(const std:
 
     if (dayStr.find('-') != std::string::npos) {
       // dayDescription is day interval (Mo-Su)
-      auto daysIntervalParts = SplitString(dayStr, "-", 2);
+      auto daysIntervalParts = SplitString(dayStr, "-");
       if (daysIntervalParts.size() != 2) {
         log.Warn() << "Cannot parse day description: " << dayDescription;
         return std::nullopt;
@@ -109,7 +109,7 @@ std::optional<std::vector<OpeningHours::WeekDay>> ParseDayDescription(const std:
 // example: 08:00
 std::optional<OpeningHours::DayTime> ParseTime(const std::string &timeStr)
 {
-  auto timesStr=SplitString(timeStr, ":", 2);
+  auto timesStr=SplitString(timeStr, ":");
   if (timesStr.size()!=2) {
     log.Warn() << "Cannot parse time: " << timeStr;
     return std::nullopt;
@@ -133,7 +133,7 @@ std::optional<OpeningHours::DayTime> ParseTime(const std::string &timeStr)
 // example: 08:00-12:00
 std::optional<OpeningHours::TimeInterval> ParseTimeRange(const std::string &rangeStr)
 {
-  auto timesStr=SplitString(rangeStr, "-", 2);
+  auto timesStr=SplitString(rangeStr, "-");
   if (timesStr.size()!=2) {
     log.Warn() << "Cannot parse time interval: " << rangeStr;
     return std::nullopt;
@@ -149,15 +149,19 @@ std::optional<OpeningHours::TimeInterval> ParseTimeRange(const std::string &rang
   return OpeningHours::TimeInterval{*from, *to};
 }
 
-// example: 08:00-12:00,13:00-17:30
+// examples:
+// 08:00-12:00
+// 08:00-12:00,13:00-17:30
+// 08:00-12:00, 13:00-17:30
+// off
 std::optional<std::vector<OpeningHours::TimeInterval>> ParseTimeDescription(const std::string &timeDescription) {
   std::vector<OpeningHours::TimeInterval> result;
   if (timeDescription=="off") {
     return result;
   }
   auto rangesStr=SplitString(timeDescription, ",");
   for (const auto &rangeStr:rangesStr) {
-    auto range=ParseTimeRange(rangeStr);
+    auto range=ParseTimeRange(Trim(rangeStr));
     if (!range) {
       return std::nullopt;
     }
@@ -185,16 +189,16 @@ std::optional<OpeningHours> OpeningHours::Parse(const std::string &str, bool exp
   auto rulesStr = SplitString(str, ";");
   for (std::string &ruleStr:rulesStr){
     ruleStr=Trim(ruleStr);
-    auto ruleSplit=SplitString(ruleStr, " ", 2);
-    if (ruleSplit.size()!=2) {
+    auto ruleSplit=SplitStringToPair(ruleStr, " ");
+    if (!ruleSplit.has_value()) {
       log.Warn() << "Cannot parse opening hours rule: " << ruleStr;
       return std::nullopt;
     }
-    auto days=ParseDayDescription(ruleSplit.front());
+    auto days=ParseDayDescription(std::get<0>(ruleSplit.value()));
     if (!days) {
       return std::nullopt;
     }
-    auto timeIntervals=ParseTimeDescription(ruleSplit.back());
+    auto timeIntervals=ParseTimeDescription(std::get<1>(ruleSplit.value()));
     if (!timeIntervals) {
       return std::nullopt;
     }

libosmscout/src/osmscout/util/String.cpp

@@ -425,6 +425,20 @@ namespace osmscout {
     return result;
   }
 
+  std::optional<std::pair<std::string,std::string>> SplitStringToPair(const std::string& str,
+                                                                      const std::string& separator)
+  {
+    assert(!separator.empty());
+
+    std::string::size_type pos = str.find(separator);
+
+    if (pos == std::string::npos) {
+      return std::nullopt;
+    }
+    return std::make_pair(str.substr(0, pos),
+                          str.substr(pos + separator.length()));
+  }
+
   std::string GetFirstInStringList(const std::string& stringList,
                                    const std::string& divider)
   {