aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKaren Arutyunov <karen@codesynthesis.com>2017-04-19 20:48:38 +0300
committerKaren Arutyunov <karen@codesynthesis.com>2017-04-19 20:57:18 +0300
commitefb9c3e0e6b612d5bfadc7a2b984c14b5439335c (patch)
treeafac11df4d8c5e1079b367a2cfd17008a317147a
parent6be5bc707876ece1cd09d7c304ba559512ef5257 (diff)
Fix repository root module to handle retries properly
-rw-r--r--mod/mod-repository-root9
-rw-r--r--mod/mod-repository-root.cxx81
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> package_version_details_;
shared_ptr<repository_details> repository_details_;
shared_ptr<options::repository_root> 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<module> 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:
+ //
+ // <package-name>
+ // <package-name>/<package-version>
+ // <package-name>/<package-version>/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;
}