diff --git a/core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/OperationChainHandler.java b/core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/OperationChainHandler.java index ba0cd9bfc48..1187cd7a5dc 100644 --- a/core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/OperationChainHandler.java +++ b/core/store/src/main/java/uk/gov/gchq/gaffer/store/operation/handler/OperationChainHandler.java @@ -23,6 +23,7 @@ import uk.gov.gchq.gaffer.operation.Operation; import uk.gov.gchq.gaffer.operation.OperationChain; import uk.gov.gchq.gaffer.operation.OperationException; +import uk.gov.gchq.gaffer.operation.graph.OperationView; import uk.gov.gchq.gaffer.store.Context; import uk.gov.gchq.gaffer.store.Store; import uk.gov.gchq.gaffer.store.operation.OperationChainValidator; @@ -52,6 +53,10 @@ public OUT doOperation(final OperationChain operationChain, final Context c // OpenTelemetry hooks Span span = OtelUtil.startSpan(this.getClass().getName(), op.getClass().getName()); span.setAttribute("jobId", context.getJobId()); + if (op instanceof OperationView && ((OperationView) op).getView() != null) { + span.setAttribute("view", ((OperationView) op).getView().toString()); + } + // Sets the span to current so parent child spans are auto linked try (Scope scope = span.makeCurrent()) { updateOperationInput(op, result); diff --git a/rest-api/spring-rest/src/main/java/uk/gov/gchq/gaffer/rest/controller/GremlinController.java b/rest-api/spring-rest/src/main/java/uk/gov/gchq/gaffer/rest/controller/GremlinController.java index aa0bc4945c5..ab2b0850d27 100644 --- a/rest-api/spring-rest/src/main/java/uk/gov/gchq/gaffer/rest/controller/GremlinController.java +++ b/rest-api/spring-rest/src/main/java/uk/gov/gchq/gaffer/rest/controller/GremlinController.java @@ -34,10 +34,12 @@ import org.opencypher.gremlin.server.jsr223.CypherPlugin; import org.opencypher.gremlin.translation.CypherAst; import org.opencypher.gremlin.translation.translator.Translator; +import org.opencypher.v9_0.util.SyntaxException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; @@ -58,7 +60,6 @@ import uk.gov.gchq.koryphe.tuple.n.Tuple2; import java.io.IOException; -import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.HashMap; import java.util.LinkedList; @@ -145,16 +146,25 @@ public String explain(@RequestHeader final HttpHeaders httpHeaders, @RequestBody @io.swagger.v3.oas.annotations.Operation( summary = "Run a Gremlin Query", description = "Runs a Gremlin Groovy script and outputs the result as GraphSONv3 JSON") - public ResponseEntity execute(@RequestHeader final HttpHeaders httpHeaders, + public ResponseEntity execute( + @RequestHeader final HttpHeaders httpHeaders, @RequestBody final String gremlinQuery) throws IOException { + // Set up preExecuteSetUp(httpHeaders); - // Write to output stream for response - StreamingResponseBody responseBody = outputStream -> GRAPHSON_V3_WRITER.writeObject( - outputStream, - runGremlinQuery(gremlinQuery).get0()); + HttpStatus status = HttpStatus.OK; + StreamingResponseBody responseBody; + try { + Object result = runGremlinQuery(gremlinQuery).get0(); + // Write to output stream for response + responseBody = os -> GRAPHSON_V3_WRITER.writeObject(os, result); + } catch (final GafferRuntimeException e) { + status = HttpStatus.INTERNAL_SERVER_ERROR; + responseBody = os -> os.write( + new JSONObject().put("simpleMessage", e.getMessage()).toString().getBytes(StandardCharsets.UTF_8)); + } - return ResponseEntity.ok() + return ResponseEntity.status(status) .contentType(APPLICATION_NDJSON) .body(responseBody); } @@ -198,20 +208,32 @@ public String cypherExplain(@RequestHeader final HttpHeaders httpHeaders, @Reque summary = "Run a Cypher Query", description = "Translates a Cypher query to Gremlin and executes it returning a GraphSONv3 JSON result." + "Note will always append a '.toList()' to the translation") - public ResponseEntity cypherExecute(@RequestHeader final HttpHeaders httpHeaders, + public ResponseEntity cypherExecute( + @RequestHeader final HttpHeaders httpHeaders, @RequestBody final String cypherQuery) throws IOException { + // Set up preExecuteSetUp(httpHeaders); - final CypherAst ast = CypherAst.parse(cypherQuery); - // Translate the cypher to gremlin, always add a .toList() otherwise Gremlin - // wont execute it as its lazy - final String translation = ast.buildTranslation(Translator.builder().gremlinGroovy().enableCypherExtensions().build()) + ".toList()"; - // Write to output stream for response - StreamingResponseBody responseBody = outputStream -> GRAPHSON_V3_WRITER.writeObject( - outputStream, - runGremlinQuery(translation).get0()); - return ResponseEntity.ok() + HttpStatus status = HttpStatus.OK; + StreamingResponseBody responseBody; + try { + final CypherAst ast = CypherAst.parse(cypherQuery); + // Translate the cypher to gremlin, always add a .toList() otherwise Gremlin + // wont execute it as its lazy + final String translation = ast.buildTranslation( + Translator.builder().gremlinGroovy().enableCypherExtensions().build()) + ".toList()"; + // Run Query + Object result = runGremlinQuery(translation).get0(); + // Write to output stream for response + responseBody = os -> GRAPHSON_V3_WRITER.writeObject(os, result); + } catch (final GafferRuntimeException | SyntaxException e) { + status = HttpStatus.INTERNAL_SERVER_ERROR; + responseBody = os -> os.write( + new JSONObject().put("simpleMessage", e.getMessage()).toString().getBytes(StandardCharsets.UTF_8)); + } + + return ResponseEntity.status(status) .contentType(APPLICATION_NDJSON) .body(responseBody); } @@ -286,7 +308,7 @@ private Tuple2 runGremlinQuery(final String gremlinQuery) { // OpenTelemetry hooks Span span = OtelUtil.startSpan( - this.getClass().getName(), "Gremlin Request: " + UUID.nameUUIDFromBytes(gremlinQuery.getBytes(Charset.defaultCharset()))); + this.getClass().getName(), "Gremlin Request: " + UUID.nameUUIDFromBytes(gremlinQuery.getBytes(StandardCharsets.UTF_8))); span.setAttribute("gaffer.gremlin.query", gremlinQuery); // tuple to hold the result and explain @@ -296,7 +318,7 @@ private Tuple2 runGremlinQuery(final String gremlinQuery) { try (Scope scope = span.makeCurrent(); GremlinExecutor gremlinExecutor = getGremlinExecutor()) { // Execute the query - Object result = gremlinExecutor.eval(gremlinQuery).join(); + Object result = gremlinExecutor.eval(gremlinQuery).get(); // Store the result and explain for returning pair.put0(result); @@ -319,6 +341,8 @@ private Tuple2 runGremlinQuery(final String gremlinQuery) { span.setStatus(StatusCode.ERROR, e.getMessage()); span.recordException(e); throw new GafferRuntimeException(GENERAL_ERROR_MSG + e.getMessage(), e); + } finally { + span.end(); } return pair; diff --git a/rest-api/spring-rest/src/test/java/uk/gov/gchq/gaffer/rest/controller/GremlinControllerTest.java b/rest-api/spring-rest/src/test/java/uk/gov/gchq/gaffer/rest/controller/GremlinControllerTest.java index 873777f582c..95a52e784d0 100644 --- a/rest-api/spring-rest/src/test/java/uk/gov/gchq/gaffer/rest/controller/GremlinControllerTest.java +++ b/rest-api/spring-rest/src/test/java/uk/gov/gchq/gaffer/rest/controller/GremlinControllerTest.java @@ -146,7 +146,7 @@ void shouldReturnExplainOfValidGremlinQuery() throws Exception { } @Test - void shouldRejectMalformedGremlinQuery() throws Exception { + void shouldRejectMalformedGremlinQueryFromExplain() throws Exception { // Given String gremlinString = "g.V().stepDoesNotExist().toList()"; @@ -163,6 +163,25 @@ void shouldRejectMalformedGremlinQuery() throws Exception { assertThat(result.getResponse().getStatus()).isEqualTo(500); } + @Test + void shouldRejectMalformedGremlinQueryFromExecute() throws Exception { + // Given + String gremlinString = "g.V().stepDoesNotExist().toList()"; + + // When + MvcResult result = mockMvc + .perform(MockMvcRequestBuilders + .post(GREMLIN_EXECUTE_ENDPOINT) + .content(gremlinString) + .contentType(TEXT_PLAIN_VALUE) + .accept(APPLICATION_NDJSON)) + .andReturn(); + + // Then + // Expect a server error response + assertThat(result.getResponse().getStatus()).isEqualTo(500); + } + @Test void shouldExecuteValidCypherQuery() throws Exception { String cypherString = "MATCH (p:person) WHERE ID(p) = '" + MARKO.getId() + "' RETURN p"; @@ -285,7 +304,7 @@ void shouldReturnExplainOfCypherQueryWithExtensions() throws Exception { } @Test - void shouldRejectMalformedCypherQuery() throws Exception { + void shouldRejectMalformedCypherQueryFromExplain() throws Exception { // Given String cypherString = "MATCH (p:person) WHERE RETURN p"; @@ -302,5 +321,24 @@ void shouldRejectMalformedCypherQuery() throws Exception { assertThat(result.getResponse().getStatus()).isEqualTo(500); } + @Test + void shouldRejectMalformedCypherQueryFromExecute() throws Exception { + // Given + String cypherString = "MATCH (p:person) WHERE RETURN p"; + + // When + MvcResult result = mockMvc + .perform(MockMvcRequestBuilders + .post(CYPHER_EXECUTE_ENDPOINT) + .content(cypherString) + .contentType(TEXT_PLAIN_VALUE) + .accept(APPLICATION_NDJSON)) + .andReturn(); + + // Then + // Expect a server error response + assertThat(result.getResponse().getStatus()).isEqualTo(500); + } + }