Updated /scheduler endopint to use Request::acceptsMediaType. Review: https://reviews.apache.org/r/37403
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/61f71f05 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/61f71f05 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/61f71f05 Branch: refs/heads/master Commit: 61f71f05ad2a7e9205565437b3243aa84072bf84 Parents: ec3318e Author: Isabel Jimenez <[email protected]> Authored: Thu Aug 13 10:37:19 2015 -0700 Committer: Benjamin Mahler <[email protected]> Committed: Thu Aug 13 11:52:56 2015 -0700 ---------------------------------------------------------------------- src/master/http.cpp | 27 ++++++---- src/tests/http_api_tests.cpp | 106 +++++++++++++++++++++++++++++++++++++- 2 files changed, 121 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/61f71f05/src/master/http.cpp ---------------------------------------------------------------------- diff --git a/src/master/http.cpp b/src/master/http.cpp index a359e74..49c18ce 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -71,6 +71,7 @@ using process::http::BadRequest; using process::http::InternalServerError; using process::http::NotFound; using process::http::NotImplemented; +using process::http::NotAcceptable; using process::http::OK; using process::http::Pipe; using process::http::TemporaryRedirect; @@ -313,7 +314,7 @@ void Master::Http::log(const Request& request) } -// TODO(ijiminez): Add some information or pointers to help +// TODO(ijimenez): Add some information or pointers to help // users understand the HTTP Event/Call API. const string Master::Http::SCHEDULER_HELP = HELP( TLDR( @@ -376,17 +377,21 @@ Future<Response> Master::Http::scheduler(const Request& request) const error.get().message); } - // Default to sending back JSON. - ContentType responseContentType = ContentType::JSON; - - // TODO(anand): Use request.acceptsMediaType() once available. - Option<string> acceptType = request.headers.get("Accept"); - - if (acceptType.get() == APPLICATION_PROTOBUF) { - responseContentType = ContentType::PROTOBUF; - } - if (call.type() == scheduler::Call::SUBSCRIBE) { + // We default to JSON since an empty 'Accept' header + // results in all media types considered acceptable. + ContentType responseContentType; + + if (request.acceptsMediaType(APPLICATION_JSON)) { + responseContentType = ContentType::JSON; + } else if (request.acceptsMediaType(APPLICATION_PROTOBUF)) { + responseContentType = ContentType::PROTOBUF; + } else { + return NotAcceptable( + string("Expecting 'Accept' to allow ") + + "'" + APPLICATION_PROTOBUF + "' or '" + APPLICATION_JSON + "'"); + } + Pipe pipe; OK ok; http://git-wip-us.apache.org/repos/asf/mesos/blob/61f71f05/src/tests/http_api_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/http_api_tests.cpp b/src/tests/http_api_tests.cpp index 1d11d29..a51231e 100644 --- a/src/tests/http_api_tests.cpp +++ b/src/tests/http_api_tests.cpp @@ -27,6 +27,7 @@ #include <process/http.hpp> #include <process/pid.hpp> +#include <stout/gtest.hpp> #include <stout/json.hpp> #include <stout/lambda.hpp> #include <stout/recordio.hpp> @@ -41,7 +42,6 @@ #include "tests/utils.hpp" using mesos::internal::master::DEFAULT_HEARTBEAT_INTERVAL; - using mesos::internal::master::Master; using mesos::internal::recordio::Reader; @@ -54,6 +54,7 @@ using process::Future; using process::PID; using process::http::BadRequest; +using process::http::NotAcceptable; using process::http::OK; using process::http::Pipe; using process::http::Response; @@ -603,6 +604,109 @@ TEST_P(HttpApiTest, UpdatePidToHttpSchedulerWithoutForce) Shutdown(); } + +TEST_P(HttpApiTest, NotAcceptable) +{ + // HTTP schedulers cannot yet authenticate. + master::Flags flags = CreateMasterFlags(); + flags.authenticate_frameworks = false; + + Try<PID<Master> > master = StartMaster(flags); + ASSERT_SOME(master); + + // Retrieve the parameter passed as content type to this test. + const string contentType = GetParam(); + + hashmap<string, string> headers; + headers["Accept"] = "foo"; + + // Only subscribe needs to 'Accept' json or protobuf. + Call call; + call.set_type(Call::SUBSCRIBE); + + Call::Subscribe* subscribe = call.mutable_subscribe(); + subscribe->mutable_framework_info()->CopyFrom(DEFAULT_V1_FRAMEWORK_INFO); + + Future<Response> response = process::http::streaming::post( + master.get(), + "api/v1/scheduler", + headers, + serialize(call, contentType), + contentType); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(NotAcceptable().status, response); +} + + +TEST_P(HttpApiTest, NoAcceptHeader) +{ + // HTTP schedulers cannot yet authenticate. + master::Flags flags = CreateMasterFlags(); + flags.authenticate_frameworks = false; + + Try<PID<Master> > master = StartMaster(flags); + ASSERT_SOME(master); + + // Retrieve the parameter passed as content type to this test. + const string contentType = GetParam(); + + // No 'Accept' header leads to all media types considered + // acceptable. JSON will be chosen by default. + hashmap<string, string> headers; + + // Only subscribe needs to 'Accept' json or protobuf. + Call call; + call.set_type(Call::SUBSCRIBE); + + Call::Subscribe* subscribe = call.mutable_subscribe(); + subscribe->mutable_framework_info()->CopyFrom(DEFAULT_V1_FRAMEWORK_INFO); + + Future<Response> response = process::http::streaming::post( + master.get(), + "api/v1/scheduler", + headers, + serialize(call, contentType), + contentType); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); + EXPECT_SOME_EQ(APPLICATION_JSON, response.get().headers.get("Content-Type")); +} + + +TEST_P(HttpApiTest, DefaultAccept) +{ + // HTTP schedulers cannot yet authenticate. + master::Flags flags = CreateMasterFlags(); + flags.authenticate_frameworks = false; + + Try<PID<Master> > master = StartMaster(flags); + ASSERT_SOME(master); + + hashmap<string, string> headers; + headers["Accept"] = "*/*"; + + // Only subscribe needs to 'Accept' json or protobuf. + Call call; + call.set_type(Call::SUBSCRIBE); + + Call::Subscribe* subscribe = call.mutable_subscribe(); + subscribe->mutable_framework_info()->CopyFrom(DEFAULT_V1_FRAMEWORK_INFO); + + // Retrieve the parameter passed as content type to this test. + const string contentType = GetParam(); + + Future<Response> response = process::http::streaming::post( + master.get(), + "api/v1/scheduler", + headers, + serialize(call, contentType), + contentType); + + AWAIT_EXPECT_RESPONSE_STATUS_EQ(OK().status, response); + EXPECT_SOME_EQ(APPLICATION_JSON, response.get().headers.get("Content-Type")); +} + + } // namespace tests { } // namespace internal { } // namespace mesos {
