This is an automated email from the ASF dual-hosted git repository. kenhuuu pushed a commit to branch http-req-id-header in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 66109d536cfe7cf06d782191adb53044aa74d46a Author: Ken Hu <[email protected]> AuthorDate: Wed Apr 17 16:44:24 2024 -0700 Introduce Netty handler for managing request IDs. Request IDs are now generated by the server and returned as a header. This handler handles both of these operations to centralize the implementation. --- .../apache/tinkerpop/gremlin/driver/Client.java | 4 +- .../gremlin/server/channel/HttpChannelizer.java | 4 ++ .../server/handler/HttpGremlinEndpointHandler.java | 41 +++++------ .../gremlin/server/handler/HttpHandlerUtil.java | 3 +- .../server/handler/HttpRequestIdHandler.java | 50 +++++++++++++ .../server/handler/HttpRequestMessageDecoder.java | 5 +- .../tinkerpop/gremlin/server/handler/StateKey.java | 7 ++ .../server/GremlinServerHttpIntegrateTest.java | 35 +++++---- .../server/handler/HttpRequestIdHandlerTest.java | 82 ++++++++++++++++++++++ .../handler/HttpRequestMessageDecoderTest.java | 8 +-- .../org/apache/tinkerpop/gremlin/util/Tokens.java | 3 +- .../gremlin/util/message/RequestMessageV4.java | 12 +--- .../gremlin/util/message/ResponseMessage.java | 4 -- .../ser/AbstractGraphSONMessageSerializerV4.java | 3 - .../ser/binary/RequestMessageSerializerV4.java | 3 - .../gremlin/util/message/RequestMessageV4Test.java | 11 +-- 16 files changed, 196 insertions(+), 79 deletions(-) diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java index 88f935fd20..bc718945c0 100644 --- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java +++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java @@ -359,7 +359,7 @@ public abstract class Client { // options.getTimeout().ifPresent(timeout -> request.add(Tokens.ARGS_EVAL_TIMEOUT, timeout)); options.getParameters().ifPresent(params -> request.addBindings(params)); options.getAliases().ifPresent(aliases -> {if (aliases.get("g") != null) request.addG(aliases.get("g")); }); - options.getOverrideRequestId().ifPresent(request::overrideRequestId); +// options.getOverrideRequestId().ifPresent(request::overrideRequestId); // options.getUserAgent().ifPresent(userAgent -> request.addArg(Tokens.ARGS_USER_AGENT, userAgent)); options.getLanguage().ifPresent(lang -> request.addLanguage(lang)); // options.getMaterializeProperties().ifPresent(mp -> request.addArg(Tokens.ARGS_MATERIALIZE_PROPERTIES, mp)); @@ -656,7 +656,7 @@ public abstract class Client { // apply settings if they were made available options.getBatchSize().ifPresent(batchSize -> request.addChunkSize(batchSize)); // options.getTimeout().ifPresent(timeout -> request.add(Tokens.ARGS_EVAL_TIMEOUT, timeout)); - options.getOverrideRequestId().ifPresent(request::overrideRequestId); +// options.getOverrideRequestId().ifPresent(request::overrideRequestId); // options.getUserAgent().ifPresent(userAgent -> request.add(Tokens.ARGS_USER_AGENT, userAgent)); // options.getMaterializeProperties().ifPresent(mp -> request.addArg(Tokens.ARGS_MATERIALIZE_PROPERTIES, mp)); diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/channel/HttpChannelizer.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/channel/HttpChannelizer.java index 4d631bfdd0..e512e993fa 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/channel/HttpChannelizer.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/channel/HttpChannelizer.java @@ -30,6 +30,7 @@ import org.apache.tinkerpop.gremlin.server.handler.AbstractAuthenticationHandler import org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthenticationHandler; import org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthorizationHandler; import org.apache.tinkerpop.gremlin.server.handler.HttpRequestCheckingHandler; +import org.apache.tinkerpop.gremlin.server.handler.HttpRequestIdHandler; import org.apache.tinkerpop.gremlin.server.handler.HttpRequestMessageDecoder; import org.apache.tinkerpop.gremlin.server.handler.HttpUserAgentHandler; import org.apache.tinkerpop.gremlin.server.handler.HttpGremlinEndpointHandler; @@ -55,6 +56,8 @@ public class HttpChannelizer extends AbstractChannelizer { private HttpRequestCheckingHandler httpRequestCheckingHandler = new HttpRequestCheckingHandler(); private HttpRequestMessageDecoder httpRequestMessageDecoder = new HttpRequestMessageDecoder(serializers); + private HttpRequestIdHandler httpRequestIdHandler = new HttpRequestIdHandler(); + @Override public void init(final ServerGremlinExecutor serverGremlinExecutor) { super.init(serverGremlinExecutor); @@ -71,6 +74,7 @@ public class HttpChannelizer extends AbstractChannelizer { if (logger.isDebugEnabled()) pipeline.addLast(new LoggingHandler("http-io", LogLevel.DEBUG)); + pipeline.addLast("http-requestid-handler", httpRequestIdHandler); pipeline.addLast("http-keepalive-handler", new HttpServerKeepAliveHandler()); pipeline.addLast("http-cors-handler", new CorsHandler(CorsConfigBuilder.forAnyOrigin().build())); diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java index a6909e908a..2196795250 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java @@ -81,6 +81,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.concurrent.Future; import java.util.concurrent.FutureTask; import java.util.concurrent.RejectedExecutionException; @@ -219,7 +220,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ final String msgInvalid = String.format("Message could not be parsed. Check the format of the request. [%s]", requestMessage); throw new ProcessingException(msgInvalid, - ResponseMessage.build(requestMessage) + ResponseMessage.buildV4() .code(ResponseStatusCode.REQUEST_ERROR_MALFORMED_REQUEST) .statusMessage(msgInvalid) .create()); @@ -227,7 +228,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ final String msgDefault = String.format("Message with gremlin of type [%s] is not recognized.", requestMessage.getGremlinType()); throw new ProcessingException(msgDefault, - ResponseMessage.build(requestMessage) + ResponseMessage.buildV4() .code(ResponseStatusCode.REQUEST_ERROR_MALFORMED_REQUEST) .statusMessage(msgDefault) .create()); @@ -258,7 +259,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ if (!requestCtx.getStartedResponse()) { final String errorMessage = String.format("A timeout occurred during traversal evaluation of [%s] - consider increasing the limit given to evaluationTimeout", requestMessage); writeError(requestCtx, - ResponseMessage.build(requestMessage) + ResponseMessage.buildV4() .code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) .statusMessage(errorMessage) .create(), @@ -268,7 +269,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ } } catch (RejectedExecutionException ree) { writeError(requestCtx, - ResponseMessage.build(requestMessage).code(ResponseStatusCode.TOO_MANY_REQUESTS).statusMessage("Rate limiting").create(), + ResponseMessage.buildV4().code(ResponseStatusCode.TOO_MANY_REQUESTS).statusMessage("Rate limiting").create(), serializer.getValue1()); } } @@ -284,7 +285,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ final Optional<Throwable> possibleSpecialException = determineIfSpecialException(t); if (possibleSpecialException.isPresent()) { final Throwable special = possibleSpecialException.get(); - final ResponseMessage.Builder specialResponseMsg = ResponseMessage.build(requestMessage). + final ResponseMessage.Builder specialResponseMsg = ResponseMessage.buildV4(). statusMessage(special.getMessage()). statusAttributeException(special); if (special instanceof TemporaryException) { @@ -301,13 +302,13 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ t = ExceptionHelper.getRootCause(t); if (t instanceof TooLongFrameException) { - return ResponseMessage.build(requestMessage) + return ResponseMessage.buildV4() .code(ResponseStatusCode.SERVER_ERROR) .statusMessage(t.getMessage() + " - increase the maxContentLength") .create(); } else if (t instanceof InterruptedException || t instanceof TraversalInterruptedException) { final String errorMessage = String.format("A timeout occurred during traversal evaluation of [%s] - consider increasing the limit given to evaluationTimeout", requestMessage); - errorResponseMessage = ResponseMessage.build(requestMessage) + errorResponseMessage = ResponseMessage.buildV4() .code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) .statusMessage(errorMessage) .statusAttributeException(t) @@ -315,23 +316,23 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ } else if (t instanceof TimedInterruptTimeoutException) { // occurs when the TimedInterruptCustomizerProvider is in play logMessage = String.format("A timeout occurred within the script during evaluation of [%s] - consider increasing the limit given to TimedInterruptCustomizerProvider", requestMessage); - errorResponseMessage = ResponseMessage.build(requestMessage).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) + errorResponseMessage = ResponseMessage.buildV4().code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) .statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider") .statusAttributeException(t).create(); } else if (t instanceof TimeoutException) { logMessage = String.format("Script evaluation exceeded the configured threshold for request [%s]", requestMessage); - errorResponseMessage = ResponseMessage.build(requestMessage).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) + errorResponseMessage = ResponseMessage.buildV4().code(ResponseStatusCode.SERVER_ERROR_TIMEOUT) .statusMessage(t.getMessage()) .statusAttributeException(t).create(); } else if (t instanceof MultipleCompilationErrorsException && t.getMessage().contains("Method too large") && ((MultipleCompilationErrorsException) t).getErrorCollector().getErrorCount() == 1) { final String errorMessage = String.format("The Gremlin statement that was submitted exceeds the maximum compilation size allowed by the JVM, please split it into multiple smaller statements - %s", requestMessage.trimMessage(1021)); logMessage = errorMessage; - errorResponseMessage = ResponseMessage.build(requestMessage).code(ResponseStatusCode.SERVER_ERROR_EVALUATION) + errorResponseMessage = ResponseMessage.buildV4().code(ResponseStatusCode.SERVER_ERROR_EVALUATION) .statusMessage(errorMessage) .statusAttributeException(t).create(); } else { - errorResponseMessage = ResponseMessage.build(requestMessage) + errorResponseMessage = ResponseMessage.buildV4() .code(ResponseStatusCode.SERVER_ERROR_EVALUATION) .statusMessage((t.getMessage() == null) ? t.toString() : t.getMessage()) .statusAttributeException(t) @@ -349,20 +350,20 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ final Map bindings = (Map) message.getFields().get(Tokens.ARGS_BINDINGS); if (IteratorUtils.anyMatch(bindings.keySet().iterator(), k -> null == k || !(k instanceof String))) { final String msg = String.format("The [%s] message is using one or more invalid binding keys - they must be of type String and cannot be null", Tokens.OPS_EVAL); - throw new ProcessingException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } final Set<String> badBindings = IteratorUtils.set(IteratorUtils.<String>filter(bindings.keySet().iterator(), INVALID_BINDINGS_KEYS::contains)); if (!badBindings.isEmpty()) { final String msg = String.format("The [%s] message supplies one or more invalid parameters key of [%s] - these are reserved names.", Tokens.OPS_EVAL, badBindings); - throw new ProcessingException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } // ignore control bindings that get passed in with the "#jsr223" prefix - those aren't used in compilation if (IteratorUtils.count(IteratorUtils.filter(bindings.keySet().iterator(), k -> !k.toString().startsWith("#jsr223"))) > settings.maxParameters) { final String msg = String.format("The [%s] message contains %s bindings which is more than is allowed by the server %s configuration", Tokens.OPS_EVAL, bindings.size(), settings.maxParameters); - throw new ProcessingException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } } @@ -381,19 +382,19 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ if (!(requestMsg.getGremlin() instanceof Bytecode)) { final String msg = String.format("A [%s] message requires a gremlin argument that is of type %s.", Tokens.OPS_BYTECODE, Tokens.ARGS_GREMLIN, Bytecode.class.getSimpleName()); - throw new ProcessingException(msg, ResponseMessage.build(requestMsg).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } final Optional<String> alias = requestMsg.optionalField(Tokens.ARGS_G); if (!alias.isPresent()) { final String msg = String.format("A [%s] message requires a [%s] argument.", Tokens.OPS_BYTECODE, Tokens.ARGS_G); - throw new ProcessingException(msg, ResponseMessage.build(requestMsg).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } final String traversalSourceName = alias.get(); if (!ctx.getGraphManager().getTraversalSourceNames().contains(traversalSourceName)) { final String msg = String.format("The traversal source [%s] for alias [%s] is not configured on the server.", traversalSourceName, Tokens.VAL_TRAVERSAL_SOURCE_ALIAS); - throw new ProcessingException(msg, ResponseMessage.build(requestMsg).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); + throw new ProcessingException(msg, ResponseMessage.buildV4().code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create()); } final TraversalSource g = ctx.getGraphManager().getTraversalSource(traversalSourceName); @@ -407,13 +408,13 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ } catch (ScriptException ex) { logger.error("Traversal contains a lambda that cannot be compiled", ex); throw new ProcessingException("Traversal contains a lambda that cannot be compiled", - ResponseMessage.build(requestMsg).code(ResponseStatusCode.SERVER_ERROR_EVALUATION) + ResponseMessage.buildV4().code(ResponseStatusCode.SERVER_ERROR_EVALUATION) .statusMessage(ex.getMessage()) .statusAttributeException(ex).create()); } catch (Exception ex) { logger.error("Could not deserialize the Traversal instance", ex); throw new ProcessingException("Could not deserialize the Traversal instance", - ResponseMessage.build(requestMsg).code(ResponseStatusCode.SERVER_ERROR_SERIALIZATION) + ResponseMessage.buildV4().code(ResponseStatusCode.SERVER_ERROR_SERIALIZATION) .statusMessage(ex.getMessage()) .statusAttributeException(ex).create()); } @@ -457,7 +458,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ if (!found) { final String error = String.format("Could not alias [%s] to [%s] as [%s] not in the Graph or TraversalSource global bindings", Tokens.ARGS_G, aliased, aliased); - throw new ProcessingException(error, ResponseMessage.build(msg) + throw new ProcessingException(error, ResponseMessage.buildV4() .code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(error).create()); } } diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java index 209503a0d3..61f16280cc 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpHandlerUtil.java @@ -80,8 +80,7 @@ import static io.netty.handler.codec.http.LastHttpContent.EMPTY_LAST_CONTENT; public class HttpHandlerUtil { private static final Logger logger = LoggerFactory.getLogger(HttpHandlerUtil.class); static final Meter errorMeter = MetricManager.INSTANCE.getMeter(name(GremlinServer.class, "errors")); - private static final String ARGS_BINDINGS_DOT = Tokens.ARGS_BINDINGS + "."; - private static final String ARGS_ALIASES_DOT = Tokens.ARGS_ALIASES + "."; + /** * A generic mapper to return JSON errors in specific cases. */ diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandler.java new file mode 100644 index 0000000000..30e103b56c --- /dev/null +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandler.java @@ -0,0 +1,50 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.tinkerpop.gremlin.server.handler; + +import io.netty.channel.ChannelDuplexHandler; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelPromise; +import io.netty.handler.codec.http.HttpResponse; + +import java.util.UUID; + +/** + * A handler that generates a Request ID for the incoming HTTP request and injects the same ID into the response header. + */ [email protected] +public class HttpRequestIdHandler extends ChannelDuplexHandler { + public static String REQUEST_ID_HEADER_NAME = "Gremlin-RequestId"; + + @Override + public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { + ctx.attr(StateKey.REQUEST_ID).set(UUID.randomUUID()); + ctx.fireChannelRead(msg); + } + + @Override + public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) throws Exception { + if (msg instanceof HttpResponse) { + HttpResponse response = (HttpResponse) msg; + response.headers().add(REQUEST_ID_HEADER_NAME, ctx.attr(StateKey.REQUEST_ID).get()); + } + ctx.write(msg, promise); + } +} diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java index 29595107c7..cefc04f41f 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java @@ -218,13 +218,10 @@ public class HttpRequestMessageDecoder extends MessageToMessageDecoder<FullHttpR final JsonNode languageNode = body.get(Tokens.ARGS_LANGUAGE); final String language = null == languageNode ? "gremlin-groovy" : languageNode.asText(); - final JsonNode requestIdNode = body.get(Tokens.REQUEST_ID); - final UUID requestId = null == requestIdNode ? UUID.randomUUID() : UUID.fromString(requestIdNode.asText()); - final JsonNode chunkSizeNode = body.get(Tokens.ARGS_BATCH_SIZE); final Integer chunkSize = null == chunkSizeNode ? null : chunkSizeNode.asInt(); - final RequestMessageV4.Builder builder = RequestMessageV4.build(scriptNode.asText()).overrideRequestId(requestId) + final RequestMessageV4.Builder builder = RequestMessageV4.build(scriptNode.asText()) .addBindings(bindings).addLanguage(language); if (null != g) builder.addG(g); if (null != chunkSize) builder.addChunkSize(chunkSize); diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/StateKey.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/StateKey.java index 9e83999160..7ee47f87e3 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/StateKey.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/StateKey.java @@ -25,6 +25,8 @@ import org.apache.tinkerpop.gremlin.server.auth.Authenticator; import org.apache.tinkerpop.gremlin.util.ser.MessageTextSerializerV4; import org.javatuples.Pair; +import java.util.UUID; + /** * Keys used in the various handlers to store state in the pipeline. * @@ -54,6 +56,11 @@ public final class StateKey { */ public static final AttributeKey<RequestMessage> REQUEST_MESSAGE = AttributeKey.valueOf("request"); + /** + * The key for the current request ID. + */ + public static final AttributeKey<UUID> REQUEST_ID = AttributeKey.valueOf("requestId"); + /** * The key for the current {@link AuthenticatedUser}. */ diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java index 23c77c1ad3..bd7e85170b 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java @@ -70,12 +70,14 @@ import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import static org.apache.tinkerpop.gremlin.server.handler.HttpRequestIdHandler.REQUEST_ID_HEADER_NAME; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.hamcrest.core.StringRegularExpression.matchesRegex; import static org.hamcrest.core.StringStartsWith.startsWith; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -229,6 +231,7 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra try (final CloseableHttpResponse response = httpclient.execute(httpget)) { assertEquals(405, response.getStatusLine().getStatusCode()); + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); } } @@ -241,18 +244,7 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(401, response.getStatusLine().getStatusCode()); - } - } - - @Test - public void should400OnPOSTWithNonUUIDRequestId() throws Exception { - final CloseableHttpClient httpclient = HttpClients.createDefault(); - final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); - httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"2-1\", \"requestId\":\"nonsense\"}", Consts.UTF_8)); - - try (final CloseableHttpResponse response = httpclient.execute(httppost)) { - assertEquals(400, response.getStatusLine().getStatusCode()); + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); } } @@ -865,6 +857,7 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); // Temporarily 200 OK. + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); final String json = EntityUtils.toString(response.getEntity()); final JsonNode node = mapper.readTree(json); assertEquals("Division by zero", node.get("status").get("message").asText()); @@ -1196,6 +1189,7 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); assertTrue(response.getEntity().isChunked()); final String json = EntityUtils.toString(response.getEntity()); @@ -1223,6 +1217,7 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); assertTrue(response.getEntity().isChunked()); final String json = EntityUtils.toString(response.getEntity()); @@ -1383,6 +1378,22 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra } } + @Test + public void shouldIgnoreRequestIdInPostRequest() throws Exception { + final UUID requestId = UUID.fromString("1e55c495-22d5-4a39-934a-a2744ba010ef"); + final String body = "{ \"gremlin\": \"" + "g.V()" + "\", \"requestId\": \"" + requestId + "\", \"g\": \"gmodern" + "\", \"language\": \"gremlin-lang\"}"; + final CloseableHttpClient httpclient = HttpClients.createDefault(); + final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); + httppost.addHeader("Content-Type", "application/json"); + httppost.setEntity(new StringEntity(body, Consts.UTF_8)); + + try (final CloseableHttpResponse response = httpclient.execute(httppost)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + assertTrue(response.containsHeader(REQUEST_ID_HEADER_NAME)); + assertNotEquals(requestId, UUID.fromString(response.getLastHeader(REQUEST_ID_HEADER_NAME).getValue())); + } + } + @Test public void should100onExpectContinueRequest() throws Exception { final GraphSONMessageSerializerV4 serializer = new GraphSONMessageSerializerV4(); diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandlerTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandlerTest.java new file mode 100644 index 0000000000..0fc2247b6e --- /dev/null +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestIdHandlerTest.java @@ -0,0 +1,82 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.tinkerpop.gremlin.server.handler; + +import io.netty.buffer.ByteBuf; +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.http.DefaultFullHttpRequest; +import io.netty.handler.codec.http.DefaultFullHttpResponse; +import io.netty.handler.codec.http.FullHttpRequest; +import io.netty.handler.codec.http.FullHttpResponse; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.handler.codec.http.HttpResponse; +import io.netty.handler.codec.http.HttpResponseStatus; +import io.netty.handler.codec.http.HttpServerCodec; +import io.netty.handler.codec.http.HttpVersion; +import io.netty.util.CharsetUtil; +import org.junit.Test; + +import java.util.UUID; + +import static org.apache.tinkerpop.gremlin.server.handler.HttpRequestIdHandler.REQUEST_ID_HEADER_NAME; +import static org.junit.Assert.assertTrue; + +public class HttpRequestIdHandlerTest { + @Test + public void shouldProvideRequestIdToFollowingHandlers() { + final HttpRequestIdHandler httpRequestIdHandler = new HttpRequestIdHandler(); + final EmbeddedChannel testChannel = new EmbeddedChannel(new HttpServerCodec(), httpRequestIdHandler); + + final ByteBuf buffer = testChannel.alloc().buffer(); + buffer.writeCharSequence("abc", CharsetUtil.UTF_8); + final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/gremlin", buffer); + + testChannel.writeInbound(httpRequest); + testChannel.finish(); + + assertTrue(testChannel.readInbound() instanceof HttpRequest); + assertTrue(testChannel.attr(StateKey.REQUEST_ID) != null); + + buffer.release(); + } + + @Test + public void shouldInjectRequestIdHeaderToOutgoingWrites() { + final HttpRequestIdHandler httpRequestIdHandler = new HttpRequestIdHandler(); + final EmbeddedChannel testServerChannel = new EmbeddedChannel(httpRequestIdHandler); + + final ByteBuf buffer = testServerChannel.alloc().buffer(); + buffer.writeCharSequence("abc", CharsetUtil.UTF_8); + final FullHttpRequest httpRequest = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/gremlin", buffer); + + final FullHttpResponse httpResponse = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK); + + testServerChannel.writeInbound(httpRequest); + testServerChannel.writeOutbound(httpResponse); + final HttpResponse updatedResponse = testServerChannel.readOutbound(); + + testServerChannel.finish(); + assertTrue(updatedResponse.headers().contains(REQUEST_ID_HEADER_NAME)); + UUID.fromString(updatedResponse.headers().get(REQUEST_ID_HEADER_NAME)); + + httpRequest.release(); + httpResponse.release(); + } +} diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java index e7e43467b1..f3c421c28e 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoderTest.java @@ -72,9 +72,7 @@ public class HttpRequestMessageDecoderTest { final HttpRequestMessageDecoder requestMessageDecoder = new HttpRequestMessageDecoder(serializers); final EmbeddedChannel testChannel = new EmbeddedChannel(new HttpServerCodec(), new HttpObjectAggregator(Integer.MAX_VALUE), requestMessageDecoder); - final RequestMessageV4 request = RequestMessageV4.build("g.V()") - .overrideRequestId(UUID.randomUUID()) - .create(); + final RequestMessageV4 request = RequestMessageV4.build("g.V()").create(); final ByteBuf buffer = graphSONSerializer.serializeRequestMessageV4(request, allocator); @@ -98,9 +96,7 @@ public class HttpRequestMessageDecoderTest { final HttpRequestMessageDecoder requestMessageDecoder = new HttpRequestMessageDecoder(serializers); final EmbeddedChannel testChannel = new EmbeddedChannel(new HttpServerCodec(), new HttpObjectAggregator(Integer.MAX_VALUE), requestMessageDecoder); - final RequestMessageV4 request = RequestMessageV4.build("g.V()") - .overrideRequestId(UUID.randomUUID()) - .create(); + final RequestMessageV4 request = RequestMessageV4.build("g.V()").addLanguage("gremlin-lang").create(); final ByteBuf buffer = graphBinarySerializer.serializeRequestMessageV4(request, allocator); diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java index 04064357d5..e1a122e143 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/Tokens.java @@ -55,8 +55,7 @@ public final class Tokens { public static final String ARGS_BINDINGS = "bindings"; /** - * Argument name that allows definition of alias names for {@link Graph} and {@link TraversalSource} objects on - * the remote system. + * @Deprecated */ public static final String ARGS_ALIASES = "aliases"; diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java index 4718716ae4..65c2b0029b 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java @@ -29,7 +29,7 @@ import java.util.Optional; import java.util.UUID; /** - * The model for a request message in the HTTP body that is sent to the server. + * The model for a request message in the HTTP body that is sent to the server beginning in 4.0.0. */ public final class RequestMessageV4 { /** @@ -152,16 +152,6 @@ public final class RequestMessageV4 { this.fields.put(Tokens.REQUEST_ID, UUID.randomUUID()); } - /** - * Override the request identifier with a specified one, otherwise the {@link Builder} will randomly generate - * a {@link UUID}. - */ - public Builder overrideRequestId(final UUID requestId) { - Objects.requireNonNull(requestId, "requestId argument cannot be null."); - this.fields.put(Tokens.REQUEST_ID, requestId); - return this; - } - public Builder addLanguage(final String language) { Objects.requireNonNull(language, "language argument cannot be null."); this.fields.put(Tokens.ARGS_LANGUAGE, language); diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/ResponseMessage.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/ResponseMessage.java index 6dc2780dd5..94dbca4a3a 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/ResponseMessage.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/ResponseMessage.java @@ -111,10 +111,6 @@ public final class ResponseMessage { return new Builder(requestMessage); } - public static Builder build(final RequestMessageV4 requestMessage) { - return new Builder(requestMessage); - } - public static Builder build(final UUID requestId) { return new Builder(requestId); } diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java index 513d8cca80..489bf0ac4f 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/AbstractGraphSONMessageSerializerV4.java @@ -286,9 +286,6 @@ public abstract class AbstractGraphSONMessageSerializerV4 extends AbstractGraphS public RequestMessageV4 createObject(final Map<String, Object> data) { RequestMessageV4.Builder builder = RequestMessageV4.build(data.get(SerTokens.TOKEN_GREMLIN)); - if (data.containsKey(SerTokens.TOKEN_REQUEST)) { - builder.overrideRequestId(UUID.fromString(data.get(SerTokens.TOKEN_REQUEST).toString())); - } if (data.containsKey(SerTokens.TOKEN_LANGUAGE)) { builder.addLanguage(data.get(SerTokens.TOKEN_LANGUAGE).toString()); } diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializerV4.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializerV4.java index addf5cbc53..5c083f5cd1 100644 --- a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializerV4.java +++ b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/binary/RequestMessageSerializerV4.java @@ -63,9 +63,6 @@ public class RequestMessageSerializerV4 { } final RequestMessageV4.Builder builder = RequestMessageV4.build(gremlin); - if (fields.containsKey(SerTokens.TOKEN_REQUEST)) { - builder.overrideRequestId(UUID.fromString(fields.get(SerTokens.TOKEN_REQUEST).toString())); - } if (fields.containsKey(SerTokens.TOKEN_LANGUAGE)) { builder.addLanguage(fields.get(SerTokens.TOKEN_LANGUAGE).toString()); } diff --git a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java index f9e9de312f..5b17aabde6 100644 --- a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java +++ b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java @@ -30,13 +30,6 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; public class RequestMessageV4Test { - @Test - public void shouldOverrideRequest() { - final UUID request = UUID.randomUUID(); - final RequestMessageV4 msg = RequestMessageV4.build("x").overrideRequestId(request).create(); - assertEquals(request, msg.getRequestId()); - } - @Test public void shouldSetScriptGremlin() { final String script = "g.V().both()"; @@ -93,17 +86,15 @@ public class RequestMessageV4Test { @Test public void shouldGetFields() { final String g = "gmodern"; - final UUID rId = UUID.randomUUID(); final String lang = "lang"; final String query = "g.V()"; final Map<String, Object> bindings = new HashMap<>(); bindings.put("b", "c"); bindings.put("g", "gmodern"); - final RequestMessageV4 msg = RequestMessageV4.build(query).addG(g).addBindings(bindings).addLanguage(lang).overrideRequestId(rId).create(); + final RequestMessageV4 msg = RequestMessageV4.build(query).addG(g).addBindings(bindings).addLanguage(lang).create(); final Map<String, Object> fields = msg.getFields(); assertEquals(g, fields.get(Tokens.ARGS_G)); - assertEquals(rId, fields.get(Tokens.REQUEST_ID)); assertEquals(lang, fields.get(Tokens.ARGS_LANGUAGE)); assertEquals(bindings, fields.get(Tokens.ARGS_BINDINGS)); assertEquals(query, msg.getGremlin());
