From f4e8f83f67ba9b89b579c23d7ab740bb66caaacf Mon Sep 17 00:00:00 2001 From: Clay Jensen-Reimann Date: Wed, 8 Aug 2018 11:38:02 -0500 Subject: [PATCH 1/3] Expose failure reason in ElideResponse - Provide hook for building custom responses in JsonApiEndpoint --- .gitignore | 13 ++++++++----- changelog.md | 5 +++++ .../src/main/java/com/yahoo/elide/Elide.java | 17 +++++++++++------ .../java/com/yahoo/elide/ElideResponse.java | 4 +++- .../yahoo/elide/resources/JsonApiEndpoint.java | 9 ++++++--- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/.gitignore b/.gitignore index d8a913614c..247fd189be 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,15 @@ +.gradle/ +.idea/ +.sonar/ +bin/ target/ test-output/ -.settings -.classpath + .checkstyle -.idea/ -.sonar/ +.classpath +.settings + *.iml -bin dependency-reduced-pom.xml *.jar *.class diff --git a/changelog.md b/changelog.md index db8a7ca5a8..7df5dfbd63 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,10 @@ # Change Log +## 4.2.7 +**Features** +* Provide failure reason in ElideResponse +* Expose response building in JsonApiEndpoint to allow for customization of response behavior + ## 4.2.6 **Fixes** * Fix NPE serializing Dates diff --git a/elide-core/src/main/java/com/yahoo/elide/Elide.java b/elide-core/src/main/java/com/yahoo/elide/Elide.java index f5b501807f..09792a6684 100644 --- a/elide-core/src/main/java/com/yahoo/elide/Elide.java +++ b/elide-core/src/main/java/com/yahoo/elide/Elide.java @@ -212,7 +212,7 @@ protected ElideResponse handleRequest(boolean isReadOnly, Object opaqueUser, } tx.flush(requestScope); - ElideResponse response = buildResponse(responder.get()); + ElideResponse response = buildResponse(responder.get(), null); requestScope.runQueuedPreCommitTriggers(); auditLogger.commit(requestScope); @@ -236,7 +236,7 @@ protected ElideResponse handleRequest(boolean isReadOnly, Object opaqueUser, } catch (JsonPatchExtensionException e) { log.debug("JSON patch extension exception caught", e); - return buildResponse(e.getResponse()); + return buildResponse(e.getResponse(), e); } catch (HttpStatusException e) { log.debug("Caught HTTP status exception", e); @@ -276,19 +276,24 @@ protected ElideResponse buildErrorResponse(HttpStatusException error, boolean is ErrorObjects errors = ErrorObjects.builder().addError() .withDetail(isVerbose ? error.getVerboseMessage() : error.toString()).build(); JsonNode responseBody = mapper.getObjectMapper().convertValue(errors, JsonNode.class); - return buildResponse(Pair.of(error.getStatus(), responseBody)); + return buildResponse(Pair.of(error.getStatus(), responseBody), error); } - return buildResponse(isVerbose ? error.getVerboseErrorResponse() : error.getErrorResponse()); + return buildResponse(isVerbose ? error.getVerboseErrorResponse() : error.getErrorResponse(), error); } + @Deprecated protected ElideResponse buildResponse(Pair response) { + return buildResponse(response, null); + } + + protected ElideResponse buildResponse(Pair response, Throwable failureReason) { try { JsonNode responseNode = response.getRight(); Integer responseCode = response.getLeft(); String body = responseNode == null ? null : mapper.writeJsonApiDocument(responseNode); - return new ElideResponse(responseCode, body); + return new ElideResponse(responseCode, body, failureReason); } catch (JsonProcessingException e) { - return new ElideResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, e.toString()); + return new ElideResponse(HttpStatus.SC_INTERNAL_SERVER_ERROR, e.toString(), e); } } diff --git a/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java b/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java index 2ff96c1449..169f0ceb98 100644 --- a/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java +++ b/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java @@ -13,6 +13,7 @@ public class ElideResponse { @Getter private final int responseCode; @Getter private final String body; + @Getter private final Throwable failureReason; /** * Constructor. @@ -20,8 +21,9 @@ public class ElideResponse { * @param responseCode HTTP response code * @param body returned body string */ - public ElideResponse(int responseCode, String body) { + public ElideResponse(int responseCode, String body, Throwable failureReason) { this.responseCode = responseCode; this.body = body; + this.failureReason = failureReason; } } diff --git a/elide-core/src/main/java/com/yahoo/elide/resources/JsonApiEndpoint.java b/elide-core/src/main/java/com/yahoo/elide/resources/JsonApiEndpoint.java index d2d9030ad4..4d015d32f1 100644 --- a/elide-core/src/main/java/com/yahoo/elide/resources/JsonApiEndpoint.java +++ b/elide-core/src/main/java/com/yahoo/elide/resources/JsonApiEndpoint.java @@ -42,7 +42,7 @@ public class JsonApiEndpoint { public JsonApiEndpoint(@Named("elide") Elide elide, @Named("elideUserExtractionFunction") DefaultOpaqueUserFunction getUser) { this.elide = elide; - this.getUser = getUser == null ? v -> null : getUser; + this.getUser = getUser == null ? ctx -> null : getUser; } /** @@ -121,7 +121,10 @@ public Response delete( return build(elide.delete(path, jsonApiDocument, getUser.apply(securityContext))); } - private static Response build(ElideResponse response) { - return Response.status(response.getResponseCode()).entity(response.getBody()).build(); + protected Response build(ElideResponse response) { + return Response + .status(response.getResponseCode()) + .entity(response.getBody()) + .build(); } } From 2f5a70ed08d0bb21be44a78a525a715b01a84cb5 Mon Sep 17 00:00:00 2001 From: Clay Jensen-Reimann Date: Wed, 8 Aug 2018 13:55:22 -0500 Subject: [PATCH 2/3] Respond to comments from @wcekan and @aklish --- .../src/main/java/com/yahoo/elide/Elide.java | 1 - .../java/com/yahoo/elide/ElideResponse.java | 18 ++++++++++++++++-- .../yahoo/elide/graphql/GraphQLEndpoint.java | 13 ++++++------- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/elide-core/src/main/java/com/yahoo/elide/Elide.java b/elide-core/src/main/java/com/yahoo/elide/Elide.java index 09792a6684..48f974d1aa 100644 --- a/elide-core/src/main/java/com/yahoo/elide/Elide.java +++ b/elide-core/src/main/java/com/yahoo/elide/Elide.java @@ -51,7 +51,6 @@ */ @Slf4j public class Elide { - @Getter private final ElideSettings elideSettings; @Getter private final AuditLogger auditLogger; @Getter private final DataStore dataStore; diff --git a/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java b/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java index 169f0ceb98..3c95f82598 100644 --- a/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java +++ b/elide-core/src/main/java/com/yahoo/elide/ElideResponse.java @@ -7,23 +7,37 @@ import lombok.Getter; +import java.util.Optional; + /** * Elide response object. */ public class ElideResponse { @Getter private final int responseCode; @Getter private final String body; - @Getter private final Throwable failureReason; + @Getter private final Optional failureReason; + + /** + * Constructor. + * + * @param responseCode HTTP response code + * @param body returned body string + */ + @Deprecated + public ElideResponse(int responseCode, String body) { + this(responseCode, body, null); + } /** * Constructor. * * @param responseCode HTTP response code * @param body returned body string + * @param failureReason the reason the request failed */ public ElideResponse(int responseCode, String body, Throwable failureReason) { this.responseCode = responseCode; this.body = body; - this.failureReason = failureReason; + this.failureReason = Optional.ofNullable(failureReason); } } diff --git a/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java b/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java index 811fe30fe9..1654dfc5f6 100644 --- a/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java +++ b/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java @@ -10,6 +10,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.JsonNodeFactory; +import com.google.common.collect.ImmutableMap; import com.yahoo.elide.Elide; import com.yahoo.elide.ElideSettings; import com.yahoo.elide.core.DataStoreTransaction; @@ -70,7 +71,6 @@ public class GraphQLEndpoint { public GraphQLEndpoint( @Named("elide") Elide elide, @Named("elideUserExtractionFunction") DefaultOpaqueUserFunction getUser) { - log.error("Started ~~"); this.elide = elide; this.elideSettings = elide.getElideSettings(); this.getUser = getUser; @@ -179,13 +179,12 @@ private Response executeGraphQLRequest( requestScope.getPermissionExecutor().executeCommitChecks(); if (query.trim().startsWith(MUTATION)) { if (!result.getErrors().isEmpty()) { - HashMap abortedResponseObject = new HashMap() { - { - put("errors", result.getErrors()); - put("data", null); - } - }; + Map abortedResponseObject = ImmutableMap.of( + "errors", result.getErrors(), + "data", null + ); // Do not commit. Throw OK response to process tx.close correctly. + // (default transaction implementations throw an IOException if you leave a dangling SQL transaction) throw new WebApplicationException( Response.ok(mapper.writeValueAsString(abortedResponseObject)).build()); } From c8ebfbf8ba7a170a505ee5f6c7ec98f5f3d828a1 Mon Sep 17 00:00:00 2001 From: Clay Jensen-Reimann Date: Wed, 8 Aug 2018 14:45:35 -0500 Subject: [PATCH 3/3] Fix checkstyle --- .../main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java b/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java index 1654dfc5f6..03cfccdf97 100644 --- a/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java +++ b/elide-graphql/src/main/java/com/yahoo/elide/graphql/GraphQLEndpoint.java @@ -39,9 +39,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.SecurityContext; - import java.io.IOException; -import java.util.HashMap; import java.util.Iterator; import java.util.Map; import java.util.function.Function; @@ -184,7 +182,7 @@ private Response executeGraphQLRequest( "data", null ); // Do not commit. Throw OK response to process tx.close correctly. - // (default transaction implementations throw an IOException if you leave a dangling SQL transaction) + // (default implementations throw an IOException if you leave a dangling SQL transaction) throw new WebApplicationException( Response.ok(mapper.writeValueAsString(abortedResponseObject)).build()); }