Merge pull request #2403 from oxidase/refactor/duration-parser

Refactoring of iso_8601_grammar

CHANGELOG.md

@@ -5,6 +5,9 @@
      - new parameter `annotate` for `route` and `match` requests.  Returns additional data about each
        coordinate along the selected/matched route line.
 
+   - Profile changes:
+     - duration parser now accepts P[n]DT[n]H[n]M[n]S, P[n]W, PTHHMMSS and PTHH:MM:SS ISO8601 formats.
+
 # 5.1.0
    Changes with regard to 5.0.0
 

include/extractor/extraction_helper_functions.hpp

@@ -1,13 +1,8 @@
 #ifndef EXTRACTION_HELPER_FUNCTIONS_HPP
 #define EXTRACTION_HELPER_FUNCTIONS_HPP
 
-#include "util/cast.hpp"
-#include "util/iso_8601_duration_parser.hpp"
-
-#include <boost/algorithm/string.hpp>
-#include <boost/algorithm/string_regex.hpp>
 #include <boost/spirit/include/qi.hpp>
-#include <boost/regex.hpp>
+#include <boost/spirit/include/phoenix.hpp>
 
 #include <limits>
 #include <string>
@@ -17,83 +12,106 @@ namespace osrm
 namespace extractor
 {
 
-inline bool simple_duration_is_valid(const std::string &s)
+namespace detail
 {
-    boost::regex simple_format(
-        "((\\d|\\d\\d):(\\d|\\d\\d):(\\d|\\d\\d))|((\\d|\\d\\d):(\\d|\\d\\d))|(\\d|\\d\\d)",
-        boost::regex_constants::icase | boost::regex_constants::perl);
 
-    const bool simple_matched = regex_match(s, simple_format);
+namespace qi = boost::spirit::qi;
 
-    if (simple_matched)
+template <typename Iterator> struct iso_8601_grammar : qi::grammar<Iterator, unsigned()>
-    {
-        return true;
-    }
-    return false;
-}
-
-inline bool iso_8601_duration_is_valid(const std::string &s)
 {
-    util::iso_8601_grammar<std::string::const_iterator> iso_parser;
+    iso_8601_grammar()
-    const bool result = boost::spirit::qi::parse(s.begin(), s.end(), iso_parser);
+        : iso_8601_grammar::base_type(root)
 
-    // check if the was an error with the request
-    if (result && (0 != iso_parser.get_duration()))
     {
-        return true;
+        using qi::_1;
+        using qi::_a;
+        using qi::_b;
+        using qi::_c;
+        using qi::_pass;
+        using qi::_val;
+        using qi::eoi;
+        using qi::eps;
+        using qi::uint_;
+        using qi::char_;
+
+        hh = uint2_p[_pass = bind([](unsigned x) { return x < 24; }, _1), _val = _1];
+        mm = uint2_p[_pass = bind([](unsigned x) { return x < 60; }, _1), _val = _1];
+        ss = uint2_p[_pass = bind([](unsigned x) { return x < 60; }, _1), _val = _1];
+
+        osm_time
+            = (uint_p[_a = _1] >> eoi)                                                     [_val = _a * 60]
+            | (uint_p[_a = _1] >> ':' >> uint_p[_b = _1] >> eoi)                           [_val = _a * 3600 + _b * 60]
+            | (uint_p[_a = _1] >> ':' >> uint_p[_b = _1] >> ':' >> uint_p[_c = _1] >> eoi) [_val = _a * 3600 + _b * 60 + _c]
+            ;
+
+        alternative_time
+            = ('T' >> hh[_a = _1] >> mm[_b = _1] >> ss[_c = _1])                        [_val = _a * 3600 + _b * 60 + _c]
+            ;
+
+        extended_time
+            = ('T' >> hh[_a = _1] >> ':' >> mm[_b = _1] >> ':' >> ss[_c = _1])          [_val = _a * 3600 + _b * 60 + _c]
+            ;
+
+        standard_time
+            = ('T'
+               >> -(uint_ >> char_("Hh"))[_a = _1]
+               >> -(uint_ >> char_("Mm"))[_b = _1]
+               >> -(uint_ >> char_("Ss"))[_c = _1])                                     [_val = _a * 3600 + _b * 60 + _c]
+            ;
+
+        standard_date
+            = (uint_ >> char_("Dd"))                                                    [_val = _1 * 86400]
+            ;
+
+        standard_week
+            = (uint_ >> char_("Ww"))                                                    [_val = _1 * 604800]
+            ;
+
+        iso_period
+            = osm_time                                                                  [_val = _1]
+            | ('P' >> standard_week >> eoi)                                             [_val = _1]
+            | ('P' >> ( alternative_time[_a = 0, _b = _1]
+                      | extended_time[_a = 0, _b = _1]
+                      | (eps[_a = 0, _b = 0] >> -standard_date[_a = _1] >> -standard_time[_b = _1] ) )
+                   >> eoi)                                                              [_val = _a + _b]
+            ;
+
+        root = iso_period;
     }
-    return false;
+
+    qi::rule<Iterator, unsigned()> root;
+    qi::rule<Iterator, unsigned(), qi::locals<unsigned, unsigned>> iso_period;
+    qi::rule<Iterator, unsigned(), qi::locals<unsigned, unsigned, unsigned>> osm_time, standard_time, alternative_time, extended_time;
+    qi::rule<Iterator, unsigned()> standard_date, standard_week;
+    qi::rule<Iterator, unsigned()> hh, mm, ss;
+
+    qi::uint_parser<unsigned, 10, 1, 2> uint_p;
+    qi::uint_parser<unsigned, 10, 2, 2> uint2_p;
+};
 }
 
 inline bool durationIsValid(const std::string &s)
 {
-    return simple_duration_is_valid(s) || iso_8601_duration_is_valid(s);
+    static detail::iso_8601_grammar<std::string::const_iterator> const iso_8601_grammar;
+
+    std::string::const_iterator iter = s.begin();
+    unsigned duration = 0;
+    boost::spirit::qi::parse(iter, s.end(), iso_8601_grammar, duration);
+
+    return !s.empty() && iter == s.end();
 }
 
 inline unsigned parseDuration(const std::string &s)
 {
-    if (simple_duration_is_valid(s))
+    static detail::iso_8601_grammar<std::string::const_iterator> const iso_8601_grammar;
-    {
-        unsigned hours = 0;
-        unsigned minutes = 0;
-        unsigned seconds = 0;
-        boost::regex e(
-            "((\\d|\\d\\d):(\\d|\\d\\d):(\\d|\\d\\d))|((\\d|\\d\\d):(\\d|\\d\\d))|(\\d|\\d\\d)",
-            boost::regex_constants::icase | boost::regex_constants::perl);
 
-        std::vector<std::string> result;
+    std::string::const_iterator iter = s.begin();
-        boost::algorithm::split_regex(result, s, boost::regex(":"));
+    unsigned duration = 0;
-        const bool matched = regex_match(s, e);
+    boost::spirit::qi::parse(iter, s.end(), iso_8601_grammar, duration);
-        if (matched)
-        {
-            if (1 == result.size())
-            {
-                minutes = std::stoul(result[0]);
-            }
-            if (2 == result.size())
-            {
-                minutes = std::stoul(result[1]);
-                hours = std::stoul(result[0]);
-            }
-            if (3 == result.size())
-            {
-                seconds = std::stoul(result[2]);
-                minutes = std::stoul(result[1]);
-                hours = std::stoul(result[0]);
-            }
-            return (3600 * hours + 60 * minutes + seconds);
-        }
-    }
-    else if (iso_8601_duration_is_valid(s))
-    {
-        util::iso_8601_grammar<std::string::const_iterator> iso_parser;
-        boost::spirit::qi::parse(s.begin(), s.end(), iso_parser);
 
-        return iso_parser.get_duration();
+    return !s.empty() && iter == s.end() ? duration : std::numeric_limits<unsigned>::max();
-    }
-
-    return std::numeric_limits<unsigned>::max();
 }
+
 }
 }
 

include/util/iso_8601_duration_parser.hpp

@@ -1,82 +0,0 @@
-#ifndef ISO_8601_DURATION_PARSER_HPP
-#define ISO_8601_DURATION_PARSER_HPP
-
-#include <boost/bind.hpp>
-#include <boost/spirit/include/qi.hpp>
-#include <boost/spirit/include/qi_action.hpp>
-
-namespace osrm
-{
-namespace util
-{
-
-namespace qi = boost::spirit::qi;
-
-template <typename Iterator> struct iso_8601_grammar : qi::grammar<Iterator>
-{
-    iso_8601_grammar()
-        : iso_8601_grammar::base_type(iso_period), temp(0), hours(0), minutes(0), seconds(0)
-    {
-        iso_period = qi::lit('P') >> qi::lit('T') >>
-                     ((value >> hour >> value >> minute >> value >> second) |
-                      (value >> hour >> value >> minute) | (value >> hour >> value >> second) |
-                      (value >> hour) | (value >> minute >> value >> second) | (value >> minute) |
-                      (value >> second));
-
-        value = qi::uint_[boost::bind(&iso_8601_grammar<Iterator>::set_temp, this, ::_1)];
-        second = (qi::lit('s') |
-                  qi::lit('S'))[boost::bind(&iso_8601_grammar<Iterator>::set_seconds, this)];
-        minute = (qi::lit('m') |
-                  qi::lit('M'))[boost::bind(&iso_8601_grammar<Iterator>::set_minutes, this)];
-        hour = (qi::lit('h') |
-                qi::lit('H'))[boost::bind(&iso_8601_grammar<Iterator>::set_hours, this)];
-    }
-
-    qi::rule<Iterator> iso_period;
-    qi::rule<Iterator, std::string()> value, hour, minute, second;
-
-    unsigned temp;
-    unsigned hours;
-    unsigned minutes;
-    unsigned seconds;
-
-    void set_temp(unsigned number) { temp = number; }
-
-    void set_hours()
-    {
-        if (temp < 24)
-        {
-            hours = temp;
-        }
-    }
-
-    void set_minutes()
-    {
-        if (temp < 60)
-        {
-            minutes = temp;
-        }
-    }
-
-    void set_seconds()
-    {
-        if (temp < 60)
-        {
-            seconds = temp;
-        }
-    }
-
-    unsigned get_duration() const
-    {
-        unsigned temp = (3600 * hours + 60 * minutes + seconds);
-        if (temp == 0)
-        {
-            temp = std::numeric_limits<unsigned>::max();
-        }
-        return temp;
-    }
-};
-}
-}
-
-#endif // ISO_8601_DURATION_PARSER_HPP

unit_tests/util/duration_parsing.cpp

@@ -6,17 +6,32 @@
 BOOST_AUTO_TEST_SUITE(durations_are_valid)
 
 using namespace osrm;
-using namespace osrm::util;
 
 BOOST_AUTO_TEST_CASE(all_necessary_test)
 {
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("0"), true);
     BOOST_CHECK_EQUAL(extractor::durationIsValid("00:01"), true);
     BOOST_CHECK_EQUAL(extractor::durationIsValid("00:01:01"), true);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("61"), true);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("24:01"), true);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("00:01:60"), true);
     BOOST_CHECK_EQUAL(extractor::durationIsValid("PT15M"), true);
+
+    BOOST_CHECK_EQUAL(extractor::durationIsValid(""), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT15"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT15A"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT1H25:01"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT12501"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT0125:01"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT016001"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT240000"), false);
+    BOOST_CHECK_EQUAL(extractor::durationIsValid("PT24:00:00"), false);
 }
 
 BOOST_AUTO_TEST_CASE(common_durations_get_translated)
 {
+    BOOST_CHECK_EQUAL(extractor::parseDuration("00"), 0);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("10"), 600);
     BOOST_CHECK_EQUAL(extractor::parseDuration("00:01"), 60);
     BOOST_CHECK_EQUAL(extractor::parseDuration("00:01:01"), 61);
     BOOST_CHECK_EQUAL(extractor::parseDuration("01:01"), 3660);
@@ -29,12 +44,22 @@ BOOST_AUTO_TEST_CASE(common_durations_get_translated)
     BOOST_CHECK_EQUAL(extractor::parseDuration("PT15H"), 54000);
     BOOST_CHECK_EQUAL(extractor::parseDuration("PT1H15M"), 4500);
     BOOST_CHECK_EQUAL(extractor::parseDuration("PT1H15M1S"), 4501);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT2H25M6S"), 8706);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("P1DT2H15M1S"), 94501);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("P4D"), 345600);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT4H"), 14400);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT71M"), 4260);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT022506"), 8706);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT02:25:06"), 8706);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("P3W"), 1814400);
 }
 
 BOOST_AUTO_TEST_CASE(iso_8601_durations_case_insensitive)
 {
     BOOST_CHECK_EQUAL(extractor::parseDuration("PT15m"), 900);
     BOOST_CHECK_EQUAL(extractor::parseDuration("PT1h15m"), 4500);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("PT1h15m42s"), 4542);
+    BOOST_CHECK_EQUAL(extractor::parseDuration("P2dT1h15m42s"), 177342);
 }
 
 BOOST_AUTO_TEST_SUITE_END()