Implemented optional parameters for rest routes

main.cc

@@ -166,7 +166,7 @@ private:
     void setupRoutes() {
         using namespace Net::Rest;
 
-        Routes::Post(router, "/record/:name", &StatsEndpoint::doRecordMetric, this);
+        Routes::Post(router, "/record/:name/:value?", &StatsEndpoint::doRecordMetric, this);
         Routes::Get(router, "/value/:name", &StatsEndpoint::doGetMetric, this);
     }
 
@@ -176,13 +176,19 @@ private:
             return metric.name() == name;
         });
 
+        int val = 1;
+        if (request.hasParam(":value")) {
+            auto value = request.param(":value");
+            val = value.as<int>();
+        }
+
         if (it == std::end(metrics)) {
-            metrics.push_back(Metric(std::move(name), 1));
-            response.send(Http::Code::Created);
+            metrics.push_back(Metric(std::move(name), val));
+            response.send(Http::Code::Created, std::to_string(val));
         }
         else {
             auto &metric = *it;
-            metric.incr(1);
+            metric.incr(val);
             response.send(Http::Code::Ok, std::to_string(metric.value()));
         }
 
@@ -196,11 +202,11 @@ private:
 
         if (it == std::end(metrics)) {
             response.send(Http::Code::Not_Found);
+        } else {
+            const auto& metric = *it;
+            response.send(Http::Code::Ok, std::to_string(metric.value()));
         }
 
-        const auto& metric = *it;
-        response.send(Http::Code::Ok, std::to_string(metric.value()));
-
     }
 
     class Metric {

src/router.cc

@@ -5,6 +5,7 @@
 */
 
 #include "router.h"
+#include <algorithm>
 
 namespace Net {
 
@@ -14,56 +15,160 @@ Request::Request(
         const Http::Request& request,
         std::vector<TypedParam>&& params)
     : Http::Request(request)
+    , params_(std::move(params))
 {
-    for (auto&& param: params) {
-        params_.insert(std::make_pair(param.name(), std::move(param)));
-    }
+}
+
+bool
+Request::hasParam(std::string name) const {
+    auto it = std::find_if(params_.begin(), params_.end(), [&](const TypedParam& param) {
+            return param.name() == name;
+    });
+
+    return it != std::end(params_);
 }
 
 TypedParam
 Request::param(std::string name) const {
-    auto it = params_.find(std::move(name));
+    auto it = std::find_if(params_.begin(), params_.end(), [&](const TypedParam& param) {
+            return param.name() == name;
+    });
+
     if (it == std::end(params_)) {
         throw std::runtime_error("Unknown parameter");
     }
 
-    return it->second;
+    return *it;
 }
 
-std::pair<bool, std::vector<TypedParam>>
-Router::Route::match(const Http::Request& req) const
+Router::Route::Fragment::Fragment(std::string value)
 {
-    auto reqParts = splitUrl(req.resource());
-    if (reqParts.size() != parts_.size()) return std::make_pair(false, std::vector<TypedParam>());
+    if (value.empty())
+        throw std::runtime_error("Invalid empty fragment");
 
-    auto isUrlParameter = [](const std::string& str) {
-        return str.size() >= 1 && str[0] == ':';
-    };
+    init(std::move(value));
+}
 
-    std::vector<TypedParam> extractedParams;
-    for (std::vector<std::string>::size_type i = 0; i < reqParts.size(); ++i) {
-        if (!isUrlParameter(parts_[i])) {
-            if (reqParts[i] != parts_[i]) return std::make_pair(false, std::vector<TypedParam>());
-            continue;
+bool
+Router::Route::Fragment::match(const std::string& raw) const {
+    if (flags.hasFlag(Flag::Fixed)) {
+        if (!flags.hasFlag(Flag::Optional)) {
+            return raw == value_;
         }
+    }
+    else if (flags.hasFlag(Flag::Parameter)) {
+        return true;
+    }
+
+    return false;
+}
 
-        TypedParam param(parts_[i], reqParts[i]);
-        extractedParams.push_back(std::move(param));
+bool
+Router::Route::Fragment::match(const Fragment& other) const {
+    return match(other.value());
+}
+
+void
+Router::Route::Fragment::init(std::string value) {
+    if (value[0] == ':')
+        flags.setFlag(Flag::Parameter);
+    else
+        flags.setFlag(Flag::Fixed);
+
+    // Let's search for any '?'
+    auto pos = value.find('?');
+    if (pos != std::string::npos) {
+        if (value[0] != ':')
+            throw std::runtime_error("Only optional parameters are currently supported");
+
+        if (pos != value.size() - 1)
+            throw std::runtime_error("? should be at the end of the string");
+
+        value_ = value.substr(0, pos);
+        flags.setFlag(Flag::Optional);
+    } else {
+        value_ = std::move(value);
     }
 
-    return make_pair(true, std::move(extractedParams));
+    checkInvariant();
 }
 
-std::vector<std::string>
-Router::Route::splitUrl(const std::string& resource) const {
-    std::istringstream iss(resource);
+void
+Router::Route::Fragment::checkInvariant() const {
+    auto check = [this](std::initializer_list<Flag> exclusiveFlags) {
+        for (auto flag: exclusiveFlags) {
+            if (!flags.hasFlag(flag)) return;
+        }
+
+        throw std::logic_error(
+                std::string("Invariant violated: invalid combination of flags for fragment ") + value_);
+    };
+
+    check({ Flag::Fixed, Flag::Optional });
+    check({ Flag::Fixed, Flag::Parameter });
+}
+
+std::vector<Router::Route::Fragment>
+Router::Route::Fragment::fromUrl(const std::string& url) {
+    std::vector<Router::Route::Fragment> fragments;
+
+    std::istringstream iss(url);
     std::string p;
-    std::vector<std::string> parts;
+
     while (std::getline(iss, p, '/')) {
-        parts.push_back(std::move(p));
+        if (p.empty()) continue;
+
+        fragments.push_back(Fragment(std::move(p)));
     }
 
-    return parts;
+    return fragments;
+}
+
+bool
+Router::Route::Fragment::isParameter() const {
+    return flags.hasFlag(Flag::Parameter);
+}
+
+bool
+Router::Route::Fragment::isOptional() const {
+    return isParameter() && flags.hasFlag(Flag::Optional);
+}
+
+std::pair<bool, std::vector<TypedParam>>
+Router::Route::match(const Http::Request& req) const
+{
+    return match(req.resource());
+}
+
+std::pair<bool, std::vector<TypedParam>>
+Router::Route::match(const std::string& req) const
+{
+    auto reqFragments = Fragment::fromUrl(req);
+    if (reqFragments.size() > fragments_.size())
+        return std::make_pair(false, std::vector<TypedParam>());
+
+    std::vector<TypedParam> extractedParams;
+
+    for (std::vector<Fragment>::size_type i = 0; i < fragments_.size(); ++i) {
+        const auto& fragment = fragments_[i];
+        if (i >= reqFragments.size()) {
+            if (fragment.isOptional())
+                continue;
+
+            return std::make_pair(false, std::vector<TypedParam>());
+        }
+
+        const auto& reqFragment = reqFragments[i];
+        if (!fragment.match(reqFragment))
+            return std::make_pair(false, std::vector<TypedParam>());
+
+        if (fragment.isParameter()) {
+            extractedParams.push_back(TypedParam(fragment.value(), reqFragment.value()));
+        }
+
+    }
+
+    return make_pair(true, std::move(extractedParams));
 }
 
 Router::HttpHandler::HttpHandler(
@@ -85,8 +190,11 @@ Router::HttpHandler::onRequest(
         std::tie(match, params) = route.match(req);
         if (match) {
             route.invokeHandler(Request(req, std::move(params)), std::move(response));
+            return;
         }
     }
+
+    response.send(Http::Code::Not_Found, "Could not find a matching route");
 }
 
 void

src/router.h

@@ -9,11 +9,31 @@
 #include <string>
 #include "http.h"
 #include "http_defs.h"
+#include "flags.h"
 
 namespace Net {
 
 namespace Rest {
 
+namespace details {
+    template<typename T> struct LexicalCast {
+        static T cast(const std::string& value) {
+            std::istringstream iss(value);
+            T out;
+            if (!(iss >> out))
+                throw std::runtime_error("Bad lexical cast");
+            return out;
+        }
+    };
+
+    template<>
+    struct LexicalCast<std::string> {
+        static std::string cast(const std::string& value) {
+            return value;
+        }
+    };
+}
+
 class TypedParam {
 public:
     TypedParam(const std::string& name, const std::string& value)
@@ -23,8 +43,7 @@ public:
 
     template<typename T>
     T as() const {
-        /* @FixMe: use some sort of lexical casting here */
-        return value_;
+        return details::LexicalCast<T>::cast(value_);
     }
 
     std::string name() const {
@@ -42,11 +61,11 @@ public:
             const Http::Request& request, 
             std::vector<TypedParam>&& params);
 
+    bool hasParam(std::string name) const;
     TypedParam param(std::string name) const;
 
 private:
-
-    std::unordered_map<std::string, TypedParam> params_;
+    std::vector<TypedParam> params_;
 };
 
 class Router {
@@ -58,32 +77,64 @@ public:
             : resource_(std::move(resource))
             , method_(method)
             , handler_(std::move(handler))
-            , parts_(splitUrl(resource_))
+            , fragments_(Fragment::fromUrl(resource_))
         {
         }
 
         std::pair<bool, std::vector<TypedParam>>
         match(const Http::Request& req) const;
 
+        std::pair<bool, std::vector<TypedParam>>
+        match(const std::string& req) const;
+
         template<typename... Args>
         void invokeHandler(Args&& ...args) const {
             handler_(std::forward<Args>(args)...);
         }
 
     private:
-        std::vector<std::string> splitUrl(const std::string& resource) const;
+        struct Fragment {
+            explicit Fragment(std::string value);
+
+            bool match(const std::string& raw) const;
+            bool match(const Fragment& other) const;
+
+            bool isParameter() const;
+            bool isOptional() const;
+
+            std::string value() const {
+                return value_;
+            }
+
+            static std::vector<Fragment> fromUrl(const std::string& url);
+
+        private:
+            enum class Flag {
+                None      = 0x0,
+                Fixed     = 0x1,
+                Parameter = Fixed << 1,
+                Optional  = Parameter << 1
+            };
+
+            void init(std::string value);
+
+            void checkInvariant() const;
+
+            Flags<Flag> flags;
+            std::string value_;
+        };
 
         std::string resource_;
         Net::Http::Method method_;
         Handler handler_;
         /* @Performance: since we know that resource_ will live as long as the vector underneath,
-         * we would benefit from std::experimental::string_view to store parts of the resource.
+         * we would benefit from std::experimental::string_view to store fragments.
          *
          * We could use string_view instead of allocating strings everytime. However, string_view is
          * only available in c++17, so I might have to come with my own lightweight implementation of
          * it
          */
-        std::vector<std::string> parts_;
+        std::vector<Fragment> fragments_;
     };
 
     class HttpHandler : public Net::Http::Handler {
@@ -131,6 +182,16 @@ namespace Routes {
         Post(router, std::move(resource), std::bind(handler, obj, std::placeholders::_1, std::placeholders::_2));
     }
 
+    template<typename Handler, typename Obj>
+    void Put(Router& router, std::string resource, Handler handler, Obj obj) {
+        Put(router, std::move(resource), std::bind(handler, obj, std::placeholders::_1, std::placeholders::_2));
+    }
+
+    template<typename Handler, typename Obj>
+    void Delete(Router& router, std::string resource, Handler handler, Obj obj) {
+        Delete(router, std::move(resource), std::bind(handler, obj, std::placeholders::_1, std::placeholders::_2));
+    }
+
 } // namespace Routing
 
 } // namespace Rest

tests/CMakeLists.txt

@@ -13,3 +13,7 @@ add_test( async_test run_async_test )
 add_executable( run_typeid_test typeid_test.cc )
 target_link_libraries(run_typeid_test gtest gtest_main)
 add_test( typeid_test run_typeid_test )
+
+add_executable( run_router_test router_test.cc )
+target_link_libraries(run_router_test gtest gtest_main net)
+add_test( router_test run_router_test )

tests/router_test.cc

+/* router_test.cc
+   Mathieu Stefani, 06 janvier 2016
+   
+   Unit tests for the rest router
+*/
+
+
+#include "gtest/gtest.h"
+#include "router.h"
+#include <algorithm>
+
+using namespace Net::Rest;
+
+bool match(const Router::Route& route, const std::string& req) {
+    return route.match(req).first;
+}
+
+bool match(
+        const Router::Route& route, const std::string& req,
+        std::initializer_list<std::pair<std::string, std::string>> list)
+{
+    bool ok;
+    std::vector<TypedParam> params;
+    std::tie(ok, params) = route.match(req);
+
+    if (!ok) return false;
+
+    for (const auto& p: list) {
+        auto it = std::find_if(params.begin(), params.end(), [&](const TypedParam& param) {
+            return param.name() == p.first;
+        });
+        if (it == std::end(params)) {
+            std::cerr << "Did not find param '" << p.first << "'" << std::endl;
+            return false;
+        }
+        if (it->as<std::string>() != p.second) {
+            std::cerr << "Param '" << p.first << "' mismatched ("
+                      << p.second << " != " << it->as<std::string>() << ")" << std::endl;
+            return false;
+        }
+
+    }
+    return true;
+}
+
+
+
+Router::Route
+makeRoute(std::string value) {
+    auto noop = [](const Net::Http::Request&, Net::Http::Response) { };
+    return Router::Route(value, Net::Http::Method::Get, noop);
+}
+
+TEST(router_test, test_fixed_routes) {
+    auto r1 = makeRoute("/v1/hello");
+    ASSERT_TRUE(match(r1, "/v1/hello"));
+    ASSERT_FALSE(match(r1, "/v2/hello"));
+    ASSERT_FALSE(match(r1, "/v1/hell0"));
+
+    auto r2 = makeRoute("/a/b/c");
+    ASSERT_TRUE(match(r2, "/a/b/c"));
+}
+
+TEST(router_test, test_parameters) {
+    auto r1 = makeRoute("/v1/hello/:name");
+    ASSERT_TRUE(match(r1, "/v1/hello/joe", {
+            { ":name", "joe" }
+    }));
+
+    auto r2 = makeRoute("/greetings/:from/:to");
+    ASSERT_TRUE(match(r2, "/greetings/foo/bar", {
+            { ":from", "foo" },
+            { ":to"   , "bar" }
+    }));
+}
+
+TEST(router_test, test_optional) {
+    auto r1 = makeRoute("/get/:key?");
+    ASSERT_TRUE(match(r1, "/get"));
+    ASSERT_TRUE(match(r1, "/get/"));
+    ASSERT_TRUE(match(r1, "/get/foo", {
+            { ":key", "foo" }
+    }));
+    ASSERT_TRUE(match(r1, "/get/foo/", {
+            { ":key", "foo" }
+    }));
+
+    ASSERT_FALSE(match(r1, "/get/foo/bar"));
+}