git commit: Add post function to http.hpp of libprocess.

[be...@apache.org]

Updated Branches:
  refs/heads/master b3ec466fa -> 115ab9450


Add post function to http.hpp of libprocess.

This adds the post equivalent of the get function to http.hpp.

The existing get method is refactored into internal::request with
arguments for method, path, query, contentType, and body.

The get and post functions can then be implemented as simple wrappers
of internal::request.

The new post function has all required arguments of path, contentType,
and body.

The get function's query argument is also changed to be Option<string>
to better indicate that it is optional.

That change requires adding a forwarding overload with the old
signature (but w/o the default parameter value) for back-compat.

There are also new unit tests to verify the existing behavior of get
and the new post behavior.

From: Charlie Carson <cc...@twitter.com>
Review: https://reviews.apache.org/r/16839


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/115ab945
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/115ab945
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/115ab945

Branch: refs/heads/master
Commit: 115ab94501c838161dd3b35dd1c889314408daf9
Parents: b3ec466
Author: Benjamin Hindman <be...@gmail.com>
Authored: Fri Jan 17 14:07:47 2014 -0800
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Fri Jan 17 14:09:43 2014 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/include/process/http.hpp | 20 ++++-
 3rdparty/libprocess/src/process.cpp          | 56 ++++++++++---
 3rdparty/libprocess/src/tests/http_tests.cpp | 95 +++++++++++++++++++++++
 3 files changed, 161 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/115ab945/3rdparty/libprocess/include/process/http.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/include/process/http.hpp b/3rdparty/libprocess/include/process/http.hpp
index 5bdd520..a8b1636 100644
--- a/3rdparty/libprocess/include/process/http.hpp
+++ b/3rdparty/libprocess/include/process/http.hpp
@@ -482,7 +482,25 @@ inline Try<std::string> decode(const std::string& s)
 Future<Response> get(
     const UPID& upid,
     const std::string& path = "",
-    const std::string& query = "");
+    const Option<std::string>& query = None());
+
+
+// Backwards compatible wrapper for the above.
+// There are clients who pass in string literals to query that cannot be
+// converted automatically to Option<string>.
+Future<Response> get(
+    const UPID& upid,
+    const std::string& path,
+    const std::string& query);
+
+
+// Sends a blocking HTTP POST request to the process with the given upid.
+// Returns the HTTP response from the process, read asyncronously.
+Future<Response> post(
+    const UPID& upid,
+    const std::string& path,
+    const std::string& contentType,
+    const std::string& body);
 
 
 // Status code reason strings, from the HTTP1.1 RFC:

http://git-wip-us.apache.org/repos/asf/mesos/blob/115ab945/3rdparty/libprocess/src/process.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp
index 8cad696..936f118 100644
--- a/3rdparty/libprocess/src/process.cpp
+++ b/3rdparty/libprocess/src/process.cpp
@@ -67,6 +67,7 @@
 #include <stout/lambda.hpp>
 #include <stout/memory.hpp> // TODO(benh): Replace shared_ptr with unique_ptr.
 #include <stout/net.hpp>
+#include <stout/option.hpp>
 #include <stout/os.hpp>
 #include <stout/strings.hpp>
 #include <stout/thread.hpp>
@@ -3616,10 +3617,14 @@ Future<Response> decode(const string& buffer)
   return response;
 }
 
-} // namespace internal {
-
 
-Future<Response> get(const UPID& upid, const string& path, const string& query)
+Future<Response> request(
+    const UPID& upid,
+    const string& method,
+    const string& path,
+    const Option<string>& query,
+    const Option<string>& contentType,
+    const Option<string>& body)
 {
   int s = socket(AF_INET, SOCK_STREAM, IPPROTO_IP);
 
@@ -3645,19 +3650,30 @@ Future<Response> get(const UPID& upid, const string& path, const string& query)
 
   std::ostringstream out;
 
-  if (query.empty()) {
-    out << "GET /" << upid.id << "/" << path << " HTTP/1.1\r\n";
-  } else {
-    out << "GET /" << upid.id << "/" << path << "?" << query << " HTTP/1.1\r\n";
+  out << method << " /" << upid.id << "/" << path;
+  if (query.isSome()) {
+    out << "?" << query.get();
   }
+  out << " HTTP/1.1\r\n";
 
   // Call inet_ntop since inet_ntoa is not thread-safe!
   char ip[INET_ADDRSTRLEN];
   PCHECK(inet_ntop(AF_INET, (in_addr *) &upid.ip, ip, INET_ADDRSTRLEN) != NULL);
 
   out << "Host: " << ip << ":" << upid.port << "\r\n"
-      << "Connection: close\r\n"
-      << "\r\n";
+      << "Connection: close\r\n";
+
+  if (contentType.isSome()) {
+    out << "Content-Type: " << contentType.get() << "\r\n";
+  }
+
+  if (body.isNone()) {
+    out << "\r\n";
+  } else {
+    out << "Content-Length: " << body.get().length() << "\r\n"
+        << "\r\n"
+        << body.get();
+  }
 
   // TODO(bmahler): Use benh's async write when it gets committed.
   const string& data = out.str();
@@ -3689,6 +3705,28 @@ Future<Response> get(const UPID& upid, const string& path, const string& query)
     .onAny(lambda::bind(&os::close, s));
 }
 
+} // namespace internal {
+
+
+Future<Response> get(const UPID& upid, const string& path, const Option<string>& query)
+{
+  return internal::request(upid, "GET", path, query, None(), None());
+}
+
+
+// Overload for back-compat.
+Future<Response> get(const UPID& upid, const string& path, const string& query)
+{
+   //In this overload empty string means no query.
+   return get(upid, path, query.empty() ? Option<string>::none() : Some(query));
+}
+
+
+Future<Response> post(const UPID& upid, const string& path, const string& contentType, const string& body)
+{
+  return internal::request(upid, "POST", path, None(), contentType, body);
+}
+
 }  // namespace http {
 
 namespace internal {

http://git-wip-us.apache.org/repos/asf/mesos/blob/115ab945/3rdparty/libprocess/src/tests/http_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp
index 68d1a1b..c27e027 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -25,6 +25,8 @@ using namespace process;
 using testing::_;
 using testing::Assign;
 using testing::DoAll;
+using testing::EndsWith;
+using testing::Invoke;
 using testing::Return;
 
 
@@ -35,10 +37,14 @@ public:
   {
     route("/body", None(), &HttpProcess::body);
     route("/pipe", None(), &HttpProcess::pipe);
+    route("/get", None(), &HttpProcess::get);
+    route("/post", None(), &HttpProcess::post);
   }
 
   MOCK_METHOD1(body, Future<http::Response>(const http::Request&));
   MOCK_METHOD1(pipe, Future<http::Response>(const http::Request&));
+  MOCK_METHOD1(get, Future<http::Response>(const http::Request&));
+  MOCK_METHOD1(post, Future<http::Response>(const http::Request&));
 };
 
 
@@ -178,3 +184,92 @@ TEST(HTTP, PathParse)
   EXPECT_ERROR(parse);
   EXPECT_EQ("Not expecting suffix 'foo/bar'", parse.error());
 }
+
+
+http::Response validateGetWithoutQuery(const http::Request& request)
+{
+  EXPECT_EQ("GET", request.method);
+  EXPECT_THAT(request.path, EndsWith("get"));
+  EXPECT_EQ("", request.body);
+  EXPECT_EQ("", request.fragment);
+  EXPECT_TRUE(request.query.empty());
+
+  return http::OK();
+}
+
+
+http::Response validateGetWithQuery(const http::Request& request)
+{
+  EXPECT_EQ("GET", request.method);
+  EXPECT_THAT(request.path, EndsWith("get"));
+  EXPECT_EQ("", request.body);
+  EXPECT_EQ("frag", request.fragment);
+  EXPECT_EQ("bar", request.query.at("foo"));
+  EXPECT_EQ(1, request.query.size());
+
+  return http::OK();
+}
+
+
+TEST(HTTP, Get)
+{
+  ASSERT_TRUE(GTEST_IS_THREADSAFE);
+
+  HttpProcess process;
+
+  spawn(process);
+
+  EXPECT_CALL(process, get(_))
+    .WillOnce(Invoke(validateGetWithoutQuery));
+
+  Future<http::Response> noQueryFuture = http::get(process.self(), "get");
+
+  AWAIT_READY(noQueryFuture);
+  ASSERT_EQ(http::statuses[200], noQueryFuture.get().status);
+
+  EXPECT_CALL(process, get(_))
+    .WillOnce(Invoke(validateGetWithQuery));
+
+  Future<http::Response> queryFuture =
+    http::get(process.self(), "get", Some("foo=bar#frag"));
+
+  AWAIT_READY(queryFuture);
+  ASSERT_EQ(http::statuses[200], queryFuture.get().status);
+
+  terminate(process);
+  wait(process);
+}
+
+
+http::Response validatePost(const http::Request& request)
+{
+  EXPECT_EQ("POST", request.method);
+  EXPECT_THAT(request.path, EndsWith("post"));
+  EXPECT_EQ("This is the payload.", request.body);
+  EXPECT_EQ("", request.fragment);
+  EXPECT_TRUE(request.query.empty());
+
+  return http::OK();
+}
+
+
+TEST(HTTP, Post)
+{
+  ASSERT_TRUE(GTEST_IS_THREADSAFE);
+
+  HttpProcess process;
+
+  spawn(process);
+
+  EXPECT_CALL(process, post(_))
+    .WillOnce(Invoke(validatePost));
+
+  Future<http::Response> future =
+    http::post(process.self(), "post", "text/plain", "This is the payload.");
+
+  AWAIT_READY(future);
+  ASSERT_EQ(http::statuses[200], future.get().status);
+
+  terminate(process);
+  wait(process);
+}