From efb9c3e0e6b612d5bfadc7a2b984c14b5439335c Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 19 Apr 2017 20:48:38 +0300 Subject: Fix repository root module to handle retries properly --- mod/mod-repository-root | 9 +++++ mod/mod-repository-root.cxx | 81 ++++++++++++++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/mod/mod-repository-root b/mod/mod-repository-root index e028fb8..57db93a 100644 --- a/mod/mod-repository-root +++ b/mod/mod-repository-root @@ -23,6 +23,8 @@ namespace brep public: repository_root (); + // Copy constructible-only type. + // // Create a shallow copy (handling instance) if initialized and a deep // copy (context exemplar) otherwise. // @@ -54,6 +56,13 @@ namespace brep shared_ptr package_version_details_; shared_ptr repository_details_; shared_ptr options_; + + // Sub-module the request is dispatched to. Initially is NULL. It is set + // by the first call to handle() to a deep copy of the selected exemplar. + // The subsequent calls of handle() (that may take place after the retry + // exception is thrown) will use the existing handler instance. + // + unique_ptr handler_; }; } diff --git a/mod/mod-repository-root.cxx b/mod/mod-repository-root.cxx index 5bb5b69..1bc10cb 100644 --- a/mod/mod-repository-root.cxx +++ b/mod/mod-repository-root.cxx @@ -40,7 +40,8 @@ namespace brep cookies () {return request_.cookies ();} virtual istream& - content (bool buffer) {return request_.content (buffer);} + content (size_t limit, size_t buffer) { + return request_.content (limit, buffer);} private: request& request_; @@ -151,15 +152,15 @@ namespace brep const path& lpath (rpath.leaf (root)); - // Delegate the request handling to the sub-module. Intercept exception - // handling to add sub-module attribution. + // Delegate the request handling to the selected sub-module. Intercept + // exception handling to add sub-module attribution. // auto handle = - [&rs, this] (module& m, request& rq, const char* name) -> bool + [&rs, this] (request& rq, const char* name) -> bool { try { - return m.handle (rq, rs, *log_); + return handler_->handle (rq, rs, *log_); } catch (const invalid_request&) { @@ -184,6 +185,13 @@ namespace brep } }; + // Note that while selecting the sub-module type for handling the request, + // we rely on the fact that the initial and all the subsequent function + // calls (that may take place after the retry exception is thrown) will + // end-up with the same type, and so using the single handler instance for + // all of these calls is safe. Note that the selection also sets up the + // handling context (sub-module name and optionally the request proxy). + // if (lpath.empty ()) { // Dispatch request handling to the repository_details or the @@ -196,23 +204,34 @@ namespace brep { if (params.front ().name == "about") { - // Cleanup not to confuse the selected module with the unknown - // parameter. - // - name_values p (params); - p.erase (p.begin ()); - - request_proxy rp (rq, p); - repository_details m (*repository_details_); - return handle (m, rp, "repository_details"); + if (handler_ == nullptr) + handler_.reset (new repository_details (*repository_details_)); + + return handle (rp, "repository_details"); } + else if (fn == "build-task") + { + if (handler_ == nullptr) + handler_.reset (new build_task (*build_task_)); - throw invalid_request (400, "unknown function"); + return handle (rp, "build_task"); + } + else if (fn == "build-result") + { + if (handler_ == nullptr) + handler_.reset (new build_result (*build_result_)); + + return handle (rp, "build_result"); + } + else + throw invalid_request (400, "unknown function"); } else { - package_search m (*package_search_); - return handle (m, rq, "package_search"); + if (handler_ == nullptr) + handler_.reset (new package_search (*package_search_)); + + return handle (rq, "package_search"); } } else @@ -227,29 +246,45 @@ namespace brep // Check if this is a package name and not a brep static content files // (CSS) directory name, a repository directory name, or a special file - // name (the one starting with '.'). + // name (the one starting with '.'). Note that HTTP request URL path + // (without the root directory) must also have one of the following + // layouts: + // + // + // / + // //log[/...] + // + // If any of the checks fails, then the handling is declined. // // @@ Shouldn't we validate that the package name is not "@", is not // digit-only, does not start with '.' while parsing and serializing // the package manifest ? Probably also need to mention these - // contraints in the manifest.txt file. + // constraints in the manifest.txt file. // if (n != "@" && n.find_first_not_of ("0123456789") != string::npos && n[0] != '.') { if (i == lpath.end ()) { - package_details m (*package_details_); - return handle (m, rq, "package_details"); + if (handler_ == nullptr) + handler_.reset (new package_details (*package_details_)); + + return handle (rq, "package_details"); } else if (++i == lpath.end ()) { - package_version_details m (*package_version_details_); - return handle (m, rq, "package_version_details"); + if (handler_ == nullptr) + handler_.reset ( + new package_version_details (*package_version_details_)); + + return handle (rq, "package_version_details"); } } } + // We shouldn't be selecting a handler if decline to handle the request. + // + assert (handler_ == nullptr); return false; } -- cgit v1.1