aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zma...@apache.org
Subject aurora git commit: Support TBinaryProtocol over HTTP
Date Wed, 03 Aug 2016 22:36:26 GMT
Repository: aurora
Updated Branches:
  refs/heads/master a071af345 -> 0105a151b


Support TBinaryProtocol over HTTP

This replaces the `TServlet` servlet from thrift with our own servlet which
dispatches thrift responses based on the content type of the request. This
enables a client to use either the thrift json protocol or the binary protocol
when communicating with the scheduler.

Without this patch the current behaviour is:
- Regardless of content type of the request, assume the request is json and
  return json with a `application/x-thrift` content type.

With this patch the behaviour becomes:
- A request with no content type header, or a type of `application/x-thrift` or
  `application/json` or `application/vnd.apache.thrift.json` is assumed to be
  JSON.
- A request with a content type header of `application/vnd.apache.thrift.binary`
  is assumed to be binary.
- A request with an `Accept` header of `application/vnd.apache.thrift.binary`
  will have a binary response.
- A request with any other `Accept` header will have a JSON response.

Bugs closed: AURORA-1743

Reviewed at https://reviews.apache.org/r/50685/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/0105a151
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/0105a151
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/0105a151

Branch: refs/heads/master
Commit: 0105a151b0431aec413b0f46021b21b993101424
Parents: a071af3
Author: Zameer Manji <zmanji@apache.org>
Authored: Wed Aug 3 15:31:18 2016 -0700
Committer: Zameer Manji <zmanji@apache.org>
Committed: Wed Aug 3 15:31:18 2016 -0700

----------------------------------------------------------------------
 RELEASE-NOTES.md                                |   6 +
 .../aurora/scheduler/http/api/ApiModule.java    |  62 ++++++-
 .../http/api/TContentAwareServlet.java          | 166 +++++++++++++++++++
 .../scheduler/http/AbstractJettyTest.java       |   6 +
 .../apache/aurora/scheduler/http/api/ApiIT.java |  70 +++++++-
 5 files changed, 304 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/0105a151/RELEASE-NOTES.md
----------------------------------------------------------------------
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index 19d7f7e..d14c3d7 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -17,6 +17,12 @@
   from Mesos. This has affected rendering of some of the existing attributes. Furthermore,
it now
   dumps additional offer attributes including [reservations](http://mesos.apache.org/documentation/latest/reservation/)
   and [persistent volumes](http://mesos.apache.org/documentation/latest/persistent-volume/).
+* The scheduler API now accepts both thrift JSON and binary thrift. If a request is sent
with a
+  `Content-Type` header, or a `Content-Type` header of `application/x-thrift` or `application/json`
+  or `application/vnd.apache.thrift.json` the request is treated as thrift JSON. If a request
is
+  sent with a `Content-Type` header of `application/vnd.apache.thrift.binary` the request
is treated
+  as binary thrift. If the `Accept` header of the request is `application/vnd.apache.thrift.binary`
+  then the response will be binary thrift. Any other value for `Accept` will result in thrift
JSON.
 
 ### Deprecations and removals:
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/0105a151/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java b/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
index cd5adf9..e468209 100644
--- a/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java
@@ -14,6 +14,7 @@
 package org.apache.aurora.scheduler.http.api;
 
 import javax.inject.Singleton;
+import javax.ws.rs.core.MediaType;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.inject.Provides;
@@ -26,14 +27,24 @@ import org.apache.aurora.gen.AuroraAdmin;
 import org.apache.aurora.scheduler.http.CorsFilter;
 import org.apache.aurora.scheduler.http.JettyServerModule;
 import org.apache.aurora.scheduler.http.LeaderRedirectFilter;
+import org.apache.aurora.scheduler.http.api.TContentAwareServlet.ContentFactoryPair;
+import org.apache.aurora.scheduler.http.api.TContentAwareServlet.InputConfig;
+import org.apache.aurora.scheduler.http.api.TContentAwareServlet.OutputConfig;
 import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
+import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TJSONProtocol;
-import org.apache.thrift.server.TServlet;
 import org.eclipse.jetty.servlet.DefaultServlet;
 import org.eclipse.jetty.util.resource.Resource;
 
+import static javax.ws.rs.core.MediaType.APPLICATION_JSON_TYPE;
+
 public class ApiModule extends ServletModule {
   public static final String API_PATH = "/api";
+  private static final MediaType GENERIC_THRIFT = new MediaType("application", "x-thrift");
+  private static final MediaType THRIFT_JSON =
+      new MediaType("application", "vnd.apache.thrift.json");
+  private static final MediaType THRIFT_BINARY =
+      new MediaType("application", "vnd.apache.thrift.binary");
 
   /**
    * Set the {@code Access-Control-Allow-Origin} header for API requests. See
@@ -52,7 +63,7 @@ public class ApiModule extends ServletModule {
     if (ENABLE_CORS_FOR.get() != null) {
       filter(API_PATH).through(new CorsFilter(ENABLE_CORS_FOR.get()));
     }
-    serve(API_PATH).with(TServlet.class);
+    serve(API_PATH).with(TContentAwareServlet.class);
 
     filter(ApiBeta.PATH, ApiBeta.PATH + "/*").through(LeaderRedirectFilter.class);
     filter(ApiBeta.PATH, ApiBeta.PATH + "/*")
@@ -69,8 +80,49 @@ public class ApiModule extends ServletModule {
 
   @Provides
   @Singleton
-  TServlet provideApiThriftServlet(AnnotatedAuroraAdmin schedulerThriftInterface) {
-    return new TServlet(
-        new AuroraAdmin.Processor<>(schedulerThriftInterface), new TJSONProtocol.Factory());
+  TContentAwareServlet provideApiThriftServlet(AnnotatedAuroraAdmin schedulerThriftInterface)
{
+    /*
+     * For backwards compatibility the servlet is configured to assume `application/x-thrift`
and
+     * `application/json` have TJSON bodies.
+     *
+     * Requests that have the registered MIME type for apache thrift are mapped to their
respective
+     * protocols. See
+     * http://www.iana.org/assignments/media-types/application/vnd.apache.thrift.binary and
+     * http://www.iana.org/assignments/media-types/application/vnd.apache.thrift.json for
details.
+     *
+     * Responses have the registered MIME type so the client can decode appropriately.
+     *
+     * The Accept header is used to determine the response type. By default JSON is sent
for any
+     * value except for the binary thrift header.
+     */
+
+    ContentFactoryPair jsonFactory = new ContentFactoryPair(
+        new TJSONProtocol.Factory(),
+        THRIFT_JSON);
+    ContentFactoryPair binFactory = new ContentFactoryPair(
+        new TBinaryProtocol.Factory(),
+        THRIFT_BINARY);
+
+    // Which factory to use based on the Content-Type header of the request for reading the
request.
+    InputConfig inputConfig = new InputConfig(GENERIC_THRIFT, ImmutableMap.of(
+        GENERIC_THRIFT, jsonFactory,
+        THRIFT_JSON, jsonFactory,
+        APPLICATION_JSON_TYPE, jsonFactory,
+        THRIFT_BINARY, binFactory
+    ));
+
+    // Which factory to use based on the Accept header of the request for the response.
+    OutputConfig outputConfig = new OutputConfig(APPLICATION_JSON_TYPE, ImmutableMap.of(
+        APPLICATION_JSON_TYPE, jsonFactory,
+        GENERIC_THRIFT, jsonFactory,
+        THRIFT_JSON, jsonFactory,
+        THRIFT_BINARY, binFactory
+        ));
+
+    // A request without a Content-Type (like from curl) should be treated as GENERIC_THRIFT
+    return new TContentAwareServlet(
+        new AuroraAdmin.Processor<>(schedulerThriftInterface),
+        inputConfig,
+        outputConfig);
   }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/0105a151/src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java
b/src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java
new file mode 100644
index 0000000..1634cb8
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/http/api/TContentAwareServlet.java
@@ -0,0 +1,166 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.http.api;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import java.util.Optional;
+
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import javax.ws.rs.core.MediaType;
+
+import org.apache.thrift.TException;
+import org.apache.thrift.TProcessor;
+import org.apache.thrift.protocol.TProtocol;
+import org.apache.thrift.protocol.TProtocolFactory;
+import org.apache.thrift.transport.TIOStreamTransport;
+import org.apache.thrift.transport.TTransport;
+
+import static java.util.Objects.requireNonNull;
+
+import static javax.ws.rs.core.HttpHeaders.ACCEPT;
+
+/**
+ * An implementation of {@link org.apache.thrift.server.TServlet} that can handle multiple
thrift
+ * protocols. The protocols are dispatched on HTTP headers.
+ */
+public class TContentAwareServlet extends HttpServlet {
+  private final TProcessor processor;
+  private final InputConfig inputConfig;
+  private final OutputConfig outputConfig;
+
+  /**
+   * Class which contains the mapping of the factory and the content type of the output.
+   */
+  static class ContentFactoryPair implements TProtocolFactory {
+    private final TProtocolFactory factory;
+
+    private final MediaType outputType;
+
+    ContentFactoryPair(TProtocolFactory factory, MediaType outputType) {
+      this.factory = requireNonNull(factory);
+      this.outputType = requireNonNull(outputType);
+    }
+
+    MediaType getOutputType() {
+      return outputType;
+    }
+
+    @Override
+    public TProtocol getProtocol(TTransport tTransport) {
+      return factory.getProtocol(tTransport);
+    }
+  }
+
+  /**
+   * Configures how to interpret the Content-Type of the request.
+   */
+  static class InputConfig {
+    // Type to use when there is no Content-Type
+    private final MediaType defaultType;
+    // Mapping of values in Content-Type to protocol to use to deserialize
+    private final Map<MediaType, ContentFactoryPair> inputMapping;
+
+    InputConfig(MediaType defaultType, Map<MediaType, ContentFactoryPair> inputMapping)
{
+      this.defaultType = requireNonNull(defaultType);
+      this.inputMapping = requireNonNull(inputMapping);
+    }
+
+    Optional<ContentFactoryPair> getFactory(Optional<MediaType> mediaType) {
+      return Optional.ofNullable(inputMapping.get(mediaType.orElse(defaultType)));
+    }
+  }
+
+  /**
+   * Configures how to interpret the Accept header of the request. The defaultType's factory
is
+   * returned for almost all values to maintain backwards compatibility.
+   */
+  static class OutputConfig {
+    // Type to use when there is no Accept header
+    private final MediaType defaultType;
+    // Mapping of MediaTypes in the Accept header to protocol used to serialize the response
+    private final Map<MediaType, ContentFactoryPair> outputMapping;
+    private final ContentFactoryPair defaultFactory;
+
+    OutputConfig(MediaType defaultType, Map<MediaType, ContentFactoryPair> outputMapping)
{
+      this.defaultType = requireNonNull(defaultType);
+      this.outputMapping = requireNonNull(outputMapping);
+      this.defaultFactory = requireNonNull(outputMapping.get(defaultType));
+    }
+
+    ContentFactoryPair getFactory(Optional<MediaType> type) {
+      return Optional.ofNullable(outputMapping.get(type.orElse(defaultType)))
+          .orElse(defaultFactory);
+    }
+  }
+
+  TContentAwareServlet(TProcessor processor, InputConfig inputConfig, OutputConfig outputConfig)
{
+    this.processor = requireNonNull(processor);
+    this.inputConfig = requireNonNull(inputConfig);
+    this.outputConfig = requireNonNull(outputConfig);
+  }
+
+  @Override
+  protected void doPost(HttpServletRequest request, HttpServletResponse response)
+      throws ServletException, IOException {
+
+    Optional<ContentFactoryPair> factoryOptional =
+        inputConfig.getFactory(Optional.of(request.getContentType()).map(MediaType::valueOf));
+
+    if (!factoryOptional.isPresent()) {
+      response.setStatus(HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE);
+      String msg = "Unsupported Content-Type: " + request.getContentType();
+      response.getOutputStream().write(msg.getBytes(StandardCharsets.UTF_8));
+      return;
+    }
+
+    TTransport transport =
+        new TIOStreamTransport(request.getInputStream(), response.getOutputStream());
+
+    TProtocol inputProtocol = factoryOptional.get().getProtocol(transport);
+
+    Optional<String> acceptHeader = Optional.ofNullable(request.getHeader(ACCEPT));
+    Optional<MediaType> acceptType = Optional.empty();
+    if (acceptHeader.isPresent()) {
+      try {
+        acceptType = acceptHeader.map(MediaType::valueOf);
+      } catch (IllegalArgumentException e) {
+        // Thrown if the Accept header contains more than one type or something else we can't
+        // parse, we just treat is as no header (which will pick up the default value).
+        acceptType = Optional.empty();
+      }
+    }
+
+    ContentFactoryPair outputProtocolFactory = outputConfig.getFactory(acceptType);
+
+    response.setContentType(outputProtocolFactory.getOutputType().toString());
+    TProtocol outputProtocol = outputProtocolFactory.getProtocol(transport);
+    try {
+      processor.process(inputProtocol, outputProtocol);
+      response.getOutputStream().flush();
+    } catch (TException e) {
+      throw new ServletException(e);
+    }
+  }
+
+  @Override
+  protected void doGet(HttpServletRequest request, HttpServletResponse response)
+      throws ServletException, IOException {
+    doPost(request, response);
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/0105a151/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java b/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java
index 7dbe48b..c2ceb4e 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java
@@ -175,6 +175,12 @@ public abstract class AbstractJettyTest extends EasyMockTest {
     return String.format("http://%s:%s%s", httpServer.getHostText(), httpServer.getPort(),
path);
   }
 
+  protected WebResource.Builder getPlainRequestBuilder(String path) {
+    assertNotNull("HTTP server must be started first", httpServer);
+    Client client = Client.create(new DefaultClientConfig());
+    return client.resource(makeUrl(path)).getRequestBuilder();
+  }
+
   protected WebResource.Builder getRequestBuilder(String path) {
     assertNotNull("HTTP server must be started first", httpServer);
     ClientConfig config = new DefaultClientConfig();

http://git-wip-us.apache.org/repos/asf/aurora/blob/0105a151/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java b/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
index 31f5cb3..0a3ff05 100644
--- a/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java
@@ -13,8 +13,14 @@
  */
 package org.apache.aurora.scheduler.http.api;
 
+import java.util.Arrays;
+import java.util.List;
+
 import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.MediaType;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.primitives.Bytes;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
 import com.google.inject.util.Modules;
@@ -26,10 +32,15 @@ import org.apache.aurora.scheduler.thrift.aop.AnnotatedAuroraAdmin;
 import org.junit.Before;
 import org.junit.Test;
 
+import static javax.servlet.http.HttpServletResponse.SC_OK;
+import static javax.servlet.http.HttpServletResponse.SC_UNSUPPORTED_MEDIA_TYPE;
+import static javax.ws.rs.core.HttpHeaders.CONTENT_TYPE;
+
 import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 
 public class ApiIT extends AbstractJettyTest {
+  private static final String JSON_FIXTURE = "[1,\"getRoleSummary\",1,0,{}]";
   private AnnotatedAuroraAdmin thrift;
 
   @Before
@@ -58,8 +69,65 @@ public class ApiIT extends AbstractJettyTest {
     ClientResponse response = getRequestBuilder(ApiModule.API_PATH)
         .header(HttpHeaders.ACCEPT_ENCODING, "gzip")
         .type("application/x-thrift")
-        .post(ClientResponse.class, "[1,\"getRoleSummary\",1,0,{}]");
+        .post(ClientResponse.class, JSON_FIXTURE);
 
+    assertEquals(SC_OK, response.getStatus());
     assertEquals("gzip", response.getHeaders().getFirst(HttpHeaders.CONTENT_ENCODING));
   }
+
+  @Test
+  public void testThriftJsonAccepted() throws Exception {
+    expect(thrift.getRoleSummary()).andReturn(new Response());
+
+    replayAndStart();
+
+    ClientResponse response = getPlainRequestBuilder(ApiModule.API_PATH)
+        .type("application/vnd.apache.thrift.json")
+        .accept("application/vnd.apache.thrift.json")
+        .post(ClientResponse.class, JSON_FIXTURE);
+
+    assertEquals(SC_OK, response.getStatus());
+    assertEquals(
+        "application/vnd.apache.thrift.json",
+        response.getHeaders().getFirst(CONTENT_TYPE));
+  }
+
+  @Test
+  public void testUnknownContentTypeRejected() throws Exception {
+    replayAndStart();
+
+    ClientResponse response = getRequestBuilder(ApiModule.API_PATH)
+        .type(MediaType.TEXT_HTML_TYPE)
+        .post(ClientResponse.class, JSON_FIXTURE);
+
+    assertEquals(SC_UNSUPPORTED_MEDIA_TYPE, response.getStatus());
+  }
+
+  @Test
+  public void testBinaryContentTypeAccepted() throws Exception {
+    expect(thrift.getRoleSummary()).andReturn(new Response());
+
+    replayAndStart();
+
+    // This fixture represents a 'getRoleSummary' call encoded as binary thrift.
+    List<Integer> fixture = ImmutableList.<Integer>builder()
+        .addAll(ImmutableList.of(-128, 1, 0, 1, 0, 0, 0, 14, 103))
+        .addAll(ImmutableList.of(101, 116, 82, 111, 108, 101, 83, 117, 109))
+        .addAll(ImmutableList.of(109, 97, 114, 121, 0, 0, 0, 1, 0))
+        .addAll(ImmutableList.of(0, 0, 0, 0, 0))
+        .build();
+
+    // Note the array has to be exactly 27 bytes long.
+    byte[] rawBytes = Arrays.copyOf(Bytes.toArray(fixture), 27);
+
+    ClientResponse response = getPlainRequestBuilder(ApiModule.API_PATH)
+        .type("application/vnd.apache.thrift.binary")
+        .accept("application/vnd.apache.thrift.binary")
+        .post(ClientResponse.class, rawBytes);
+
+    assertEquals(SC_OK, response.getStatus());
+    assertEquals(
+        "application/vnd.apache.thrift.binary",
+        response.getHeaders().getFirst(CONTENT_TYPE));
+  }
 }


Mime
View raw message