Skip to content

Commit

Permalink
First attempt to make api_router work with task-based route_handlers,…
Browse files Browse the repository at this point in the history 
… so that request handling can be non-blocking. The loop through the possible route_handlers becomes a lazy sequence of task continuations.

(cherry picked from commit ef01a6cdcee407b7deb3f37e387e7c9f1ac9ad5c)
  • Loading branch information
@garethsb
garethsb committed Dec 4, 2017
1 parent cf9fa3f commit 345e344
Show file tree
Hide file tree
Showing 12 changed files with 382 additions and 350 deletions.
103 changes: 66 additions & 37 deletions Development/cpprest/api_router.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,77 +38,106 @@ namespace web
{
web::http::http_response res;

if ((*this)(req, res, {}, {}))
(*this)(req, res, {}, {}).then([req, res](bool continue_matching)
{
// no route matched, or one or more routes matched, but not for the requested method, or all handlers returned true, "continue matching routes"

// if one or more routes matched, but not for the requested method, the status code and "Allow" header have been set appropriately;
// otherwise, if no route matched, or no handler set the response status code, indicate route not found
if (empty_status_code == res.status_code())
if (continue_matching)
{
res.set_status_code(status_codes::NotFound);
}
// no route matched, or one or more routes matched, but not for the requested method, or all handlers returned true, "continue matching routes"

req.reply(res);
}
// if one or more routes matched, but not for the requested method, the status code and "Allow" header have been set appropriately;
// otherwise, if no route matched, or no handler set the response status code, indicate route not found
if (empty_status_code == res.status_code())
{
res.set_status_code(status_codes::NotFound);
}

req.reply(res);
}
});
}

static const web::http::method any_method{};

bool api_router::operator()(const web::http::http_request& req, web::http::http_response& res, const utility::string_t& route_path, const route_parameters& parameters)
pplx::task<bool> api_router::operator()(web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters)
{
return (*this)(req, res, route_path, parameters, routes.begin());
}

pplx::task<bool> api_router::operator()(web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters, iterator route)
{
const utility::string_t path = get_route_relative_path(req, route_path); // required, as must live longer than the match results
for (auto route : routes)
for (; routes.end() != route; ++route)
{
utility::smatch_t route_match;
if (route_regex_match(path, route_match, utility::regex_t(route.route_pattern.first), route.flags))
if (route_regex_match(path, route_match, utility::regex_t(route->route_pattern.first), route->flags))
{
// route_path for this route handler is constructed by appending the entire matching expression
const auto merged_path = route_path + route_match.str();
// existing parameters are inserted into the new parameters rather than vice-versa so that new parameters replace existing ones with the same name
const auto merged_parameters = insert(get_parameters(route.route_pattern.second, route_match), parameters);
const auto merged_parameters = insert(get_parameters(route->route_pattern.second, route_match), parameters);

if (route.method == req.method() || any_method == route.method)
if (route->method == req.method() || any_method == route->method)
{
try
return call(route->handler, exception_handler, req, res, merged_path, merged_parameters).then([=](bool continue_matching)
{
if (!route.handler(req, res, merged_path, merged_parameters))
if (!continue_matching)
{
// short-circuit other routes, e.g. if the hander actually sent a reply rather than just modifying the response object
return false;
}
}
catch (...)
{
if (exception_handler)
{
if (!exception_handler(req, res, merged_path, merged_parameters))
{
// explanation as above
return false;
}
}
else
{
throw;
return pplx::task_from_result(false);
}
}

return (*this)(req, res, route_path, parameters, std::next(route));
});
}
else
{
handle_method_not_allowed(route, res, merged_path, merged_parameters);
handle_method_not_allowed(*route, res, merged_path, merged_parameters);
}
}
}
// no route matched, or all handlers returned true, indicating "continue matching routes"
return true;
// no route matched; indicate "continue matching routes"
return pplx::task_from_result(true);
}

pplx::task<bool> api_router::call(const route_handler& handler, const route_handler& exception_handler, web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters)
{
if (!exception_handler)
{
return handler(req, res, route_path, parameters);
}

// this handler may:
// return a task that (immediately or asynchronously) indicates "continue matching routes" or not,
// return a task that (immediately or asynchronously) contains an exception,
// throw an exception itself
// the exception handler should be applied in either of the latter cases
try
{
return handler(req, res, route_path, parameters).then([=](pplx::task<bool> continue_matching_or_exception)
{
try
{
return pplx::task_from_result(continue_matching_or_exception.get());
}
catch (...)
{
return exception_handler(req, res, route_path, parameters);
}
});
}
catch (...)
{
return exception_handler(req, res, route_path, parameters);
}
}

void api_router::handle_method_not_allowed(const route& route, web::http::http_response& res, const utility::string_t& route_path, const route_parameters& parameters)
{
// a preceding route handler may have already set a status code, but if not, this is worth reporting as a "near miss"
if (empty_status_code == res.status_code())
{
res.set_status_code(status_codes::MethodNotAllowed);
// a subsequent route handler may yet overwrite this
}
// hmm, it's not a guarantee that the route.handler for this route.method would in fact result in a response being sent,
// but it seems nice to add the "Allow" header even so?
Expand Down
7 changes: 5 additions & 2 deletions Development/cpprest/api_router.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ namespace web

// route handlers have access to the request, and a mutable response object, the route path and parameters extracted from it by the matched route pattern;
// a handler may e.g. reply to the request or initiate asynchronous processing, and returns a flag indicating whether to continue matching routes or not
typedef std::function<bool(const web::http::http_request&, web::http::http_response&, const utility::string_t&, const route_parameters&)> route_handler;
typedef std::function<pplx::task<bool>(web::http::http_request, web::http::http_response, const utility::string_t&, const route_parameters&)> route_handler;

class api_router
{
Expand All @@ -55,7 +55,7 @@ namespace web
// allow use as a handler with http_listener
void operator()(web::http::http_request req);
// allow use as a mounted handler in another api_router
bool operator()(const web::http::http_request& req, web::http::http_response& res, const utility::string_t& route_path, const route_parameters& parameters);
pplx::task<bool> operator()(web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters);

// add a method-specific handler to support requests for this route
void support(const utility::string_t& route_pattern, const web::http::method& method, route_handler handler);
Expand All @@ -77,11 +77,14 @@ namespace web
typedef route_handlers::iterator iterator;

static utility::string_t get_route_relative_path(const web::http::http_request& req, const utility::string_t& route_path);
static pplx::task<bool> call(const route_handler& handler, const route_handler& exception_handler, web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters);
static void handle_method_not_allowed(const route& route, web::http::http_response& res, const utility::string_t& route_path, const route_parameters& parameters);
static route_parameters get_parameters(const utility::named_sub_matches_t& parameter_sub_matches, const utility::smatch_t& route_match);
static route_parameters insert(route_parameters&& into, const route_parameters& range);
static bool route_regex_match(const utility::string_t& path, utility::smatch_t& route_match, const utility::regex_t& route_regex, match_flag_type flags);

pplx::task<bool> operator()(web::http::http_request req, web::http::http_response res, const utility::string_t& route_path, const route_parameters& parameters, iterator route);

// to allow routes to be added out-of-order, support() and mount() could easily be given overloads that accept and return an iterator (const_iterator in C++11)
iterator insert(iterator where, match_flag_type flags, const utility::string_t& route_pattern, const web::http::method& method, route_handler handler);

Expand Down
8 changes: 4 additions & 4 deletions Development/nmos/admin_ui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ namespace nmos

api_router admin_ui;

admin_ui.support(U("/?"), methods::GET, [](const http_request&, http_response& res, const string_t&, const route_parameters&)
admin_ui.support(U("/?"), methods::GET, [](http_request, http_response res, const string_t&, const route_parameters&)
{
set_reply(res, status_codes::OK, value_of({ JU("admin/") }));
return true;
return pplx::task_from_result(true);
});

admin_ui.support(U("/") + nmos::experimental::patterns::admin_ui.pattern + U("/?"), methods::GET, [](const http_request&, http_response& res, const string_t&, const route_parameters&)
admin_ui.support(U("/") + nmos::experimental::patterns::admin_ui.pattern + U("/?"), methods::GET, [](http_request, http_response res, const string_t&, const route_parameters&)
{
// temporarily hard-coded hack to redirect admin root to the index.html
set_reply(res, status_codes::TemporaryRedirect);
res.headers().add(web::http::header_names::location, U("/admin/index.html"));
return true;
return pplx::task_from_result(true);
});

// To serve the admin UI, only a few HTML, JavaScript and CSS files are necessary
Expand Down
8 changes: 4 additions & 4 deletions Development/nmos/api_utils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace nmos
{
using namespace web::http::experimental::listener::api_router_using_declarations;

return [&gate](const http_request& req, http_response& res, const string_t&, const route_parameters& parameters)
return [&gate](http_request req, http_response res, const string_t&, const route_parameters& parameters)
{
if (web::http::empty_status_code == res.status_code())
{
Expand Down Expand Up @@ -118,7 +118,7 @@ namespace nmos
slog::detail::logw<slog::log_statement, slog::base_gate>(gate, slog::severities::more_info, SLOG_FLF) << nmos::stash_category(nmos::categories::access) << nmos::common_log_stash(req, res) << "Sending response";

req.reply(res);
return false; // don't continue matching routes
return pplx::task_from_result(false); // don't continue matching routes
};
}

Expand All @@ -134,7 +134,7 @@ namespace nmos

api.support(U(".*"), details::make_api_finally_handler(gate));

api.set_exception_handler([&gate](const http_request& req, http_response& res, const string_t&, const route_parameters& parameters)
api.set_exception_handler([&gate](http_request req, http_response res, const string_t&, const route_parameters& parameters)
{
try
{
Expand All @@ -157,7 +157,7 @@ namespace nmos
details::set_error_reply(res, status_codes::InternalError);
}

return true; // continue matching routes, e.g. the 'finally' handler
return pplx::task_from_result(true); // continue matching routes, e.g. the 'finally' handler
});
}

Expand Down
Loading

0 comments on commit 345e344

Please sign in to comment.