Merge pull request #377 from mmd-osm/patch/rlist

Minor router cleanups

include/cgimap/router.hpp

@@ -12,10 +12,12 @@
 
 #include "cgimap/types.hpp"
 
-#include <string>
-#include <list>
-#include <stdexcept>
 #include <iostream>
+#include <stdexcept>
+#include <string>
+#include <string_view>
+#include <utility>
+#include <vector>
 
 #include <boost/fusion/container/generation/make_cons.hpp>
 #include <boost/fusion/include/make_cons.hpp>
@@ -43,7 +45,7 @@ using boost::fusion::as_list;
 namespace result_of = boost::fusion::result_of;
 
 // iterates over the split up parts of the item being matched.
-using part_iterator = std::list<std::string>::const_iterator;
+using part_iterator = std::vector<std::string_view>::const_iterator;
 
 /**
  * thrown when a match error occurs, giving some information about the
@@ -65,7 +67,7 @@ struct error : public std::runtime_error {
 struct match_string;
 struct match_osm_id;
 struct match_begin;
-struct match_name;
+
 template <typename LeftType, typename RightType> struct match_and;
 
 /**
@@ -83,24 +85,28 @@ template <typename Self> struct ops {
   match_and<Self, match_osm_id> operator/(const match_osm_id &rhs) const {
     return match_and<Self, match_osm_id>(*static_cast<const Self *>(this), rhs);
   }
-  match_and<Self, match_name> operator/(const match_name &rhs) const {
-    return match_and<Self, match_name>(*static_cast<const Self *>(this), rhs);
-  }
 };
 
 /**
  * effectively a cons cell for sequencing matches.
  */
 template <typename LeftType, typename RightType>
 struct match_and : public ops<match_and<LeftType, RightType> > {
+
   using match_type = typename result_of::as_list<typename result_of::join<
       typename LeftType::match_type,
       typename RightType::match_type>::type>::type;
+
   match_and(const LeftType &l, const RightType &r) : lhs(l), rhs(r) {}
-  match_type match(part_iterator &begin, const part_iterator &end) const {
-    typename LeftType::match_type lval = lhs.match(begin, end);
-    typename RightType::match_type rval = rhs.match(begin, end);
-    return as_list(join(lval, rval));
+
+  std::pair<match_type, bool> match(part_iterator &begin, const part_iterator &end) const {
+    auto [ lval, lerror ] = lhs.match(begin, end);
+    if (lerror)
+      return {as_list(join(typename LeftType::match_type(), typename RightType::match_type())), true};
+    auto [ rval, rerror ] = rhs.match(begin, end);
+    if (rerror)
+      return {as_list(join(typename LeftType::match_type(), typename RightType::match_type())), true};
+    return {as_list(join(lval, rval)), false};
   }
 
 private:
@@ -117,16 +123,15 @@ struct match_string : public ops<match_string> {
 
   // implicit constructor intended, so that the use of this class is
   // hidden and easier / nicer to read.
-  match_string(const std::string &s);
   match_string(const char *s);
 
   // copy just copies the held string
   inline match_string(const match_string &m) = default;
 
-  match_type match(part_iterator &begin, const part_iterator &end) const;
+  std::pair<match_type, bool> match(part_iterator &begin, const part_iterator &end) const noexcept;
 
 private:
-  std::string str;
+  std::string_view str;
 };
 
 /**
@@ -135,16 +140,7 @@ struct match_string : public ops<match_string> {
 struct match_osm_id : public ops<match_osm_id> {
   using match_type = list<osm_nwr_id_t>;
   match_osm_id() = default;
-  match_type match(part_iterator &begin, const part_iterator &end) const;
-};
-
-/**
- * match any string.
- */
-struct match_name : public ops<match_name> {
-  using match_type = list<std::string>;
-  match_name() = default;
-  match_type match(part_iterator &begin, const part_iterator &end) const;
+  std::pair<match_type, bool> match(part_iterator &begin, const part_iterator &end) const noexcept;
 };
 
 /**
@@ -155,13 +151,14 @@ struct match_name : public ops<match_name> {
 struct match_begin : public ops<match_begin> {
   using match_type = list<>;
   match_begin() = default;
-  match_type match(part_iterator &begin, const part_iterator &end) const;
+  inline std::pair<match_type, bool> match(part_iterator &begin, const part_iterator &end) const noexcept{
+    return {match_type(), false};
+  }
 };
 
 // match items, given nicer names so that expressions are easier to read.
-static const match_begin root_;
-static const match_osm_id osm_id_;
-static const match_name name_;
+static constexpr match_begin root_;
+static constexpr match_osm_id osm_id_;
 }
 
 #endif /* ROUTER_HPP */

src/process_request.cpp

@@ -503,16 +503,34 @@ void process_request(request &req, rate_limiter &limiter,
                      oauth::store* store) {
   try {
 
-    std::set<osm_user_role_t> user_roles;
 
-    bool allow_api_write = true;
 
     // get the client IP address
     const std::string ip = fcgi_get_env(req, "REMOTE_ADDR");
 
     // fetch and parse the request method
     std::optional<http::method> maybe_method = http::parse_method(fcgi_get_env(req, "REQUEST_METHOD"));
 
+    // figure how to handle the request
+    handler_ptr_t handler = route(req);
+
+    // if handler doesn't accept this method, then return method not
+    // allowed.
+    if (!maybe_method || !handler->allows_method(*maybe_method)) {
+      process_not_allowed(req, *handler);
+      return;
+    }
+    const http::method method = *maybe_method;
+
+    // override the default access control allow methods header
+    req.set_default_methods(handler->allowed_methods());
+
+    // ------
+
+    std::set<osm_user_role_t> user_roles;
+
+    bool allow_api_write = true;
+
     auto default_transaction = factory.get_default_transaction();
 
     // create a data selection for the request
@@ -543,20 +561,6 @@ void process_request(request &req, rate_limiter &limiter,
 
     auto start_time = std::chrono::high_resolution_clock::now();
 
-    // figure how to handle the request
-    handler_ptr_t handler = route(req);
-
-    // if handler doesn't accept this method, then return method not
-    // allowed.
-    if (!maybe_method || !handler->allows_method(*maybe_method)) {
-      process_not_allowed(req, *handler);
-      return;
-    }
-    http::method method = *maybe_method;
-
-    // override the default access control allow methods header
-    req.set_default_methods(handler->allowed_methods());
-
     if (is_moderator && show_redactions_requested(req)) {
       selection->set_redactions_visible(true);
     }

src/router.cpp

@@ -9,67 +9,48 @@
 
 #include "cgimap/router.hpp"
 
+#include <algorithm>
+#include <charconv>
+
 namespace match {
 
 error::error() : std::runtime_error("error!") {}
 
-match_string::match_string(const std::string &s) : str(s) {}
-
-match_string::match_string(const char *s) : str(s) {}
+match_string::match_string(const char *s) : str(std::string_view(s)) {}
 
-match_string::match_type match_string::match(part_iterator &begin,
-                                             const part_iterator &end) const {
+std::pair<match_string::match_type, bool> match_string::match(part_iterator &begin,
+                                             const part_iterator &end) const noexcept {
   bool matches = false;
   if (begin != end) {
-    std::string bit = *begin;
-    matches = bit == str;
+    auto& bit = *begin;
+    matches = (bit == str);
     ++begin;
   }
-  if (!matches) {
-    throw error();
-  }
-  return match_type();
+  return {match_type(), !matches}; // raises error if not matching
 }
 
-match_osm_id::match_type match_osm_id::match(part_iterator &begin,
-                                             const part_iterator &end) const {
+std::pair<match_osm_id::match_type, bool> match_osm_id::match(part_iterator &begin,
+                                             const part_iterator &end) const noexcept {
   if (begin != end) {
-    try {
-      std::string bit = *begin;
-      // note that osm_nwr_id_t is actually unsigned, so we lose a bit of
-      // precision here, but it's OK since IDs are postgres 'bigint' types
-      // which are also signed, so element 2^63 is unlikely to exist.
-      auto x = std::stol(bit);
-      if (x > 0) {
-        ++begin;
-        return match_type(x);
-      }
-    } catch (std::exception &e) {
-      throw error();
+
+    auto& bit = *begin;
+
+    if (bit.end() != std::find_if(bit.begin(), bit.end(),
+        [](unsigned char c)->bool { return !isdigit(c); })) {
+      return {match_type(), true};
     }
-  }
-  throw error();
-}
 
-match_name::match_type match_name::match(part_iterator &begin,
-                                         const part_iterator &end) const {
-  if (begin != end) {
-    try {
-      std::string bit = *begin++;
-      return match_type(bit);
-    } catch (std::exception &e) {
-      throw error();
+    osm_nwr_id_t x{};
+    auto [ptr, ec] = std::from_chars(bit.data(), bit.data() + bit.size(), x);
+
+    if (ec == std::errc() && x > 0) {
+      ++begin;
+      return {match_type(x), false};
     }
   }
-  throw error();
-}
-
-match_begin::match_type match_begin::match(part_iterator &,
-                                           const part_iterator &) const {
-  return match_type();
+  return {match_type(), true};
 }
 
 extern const match_begin root_; // @suppress("Unused variable declaration in file scope")
 extern const match_osm_id osm_id_; // @suppress("Unused variable declaration in file scope")
-extern const match_name name_; // @suppress("Unused variable declaration in file scope")
 }