From f6e0efd45580054d1f965389e527c422df3dfe13 Mon Sep 17 00:00:00 2001 From: Andrew Cholewa Date: Wed, 18 Jul 2018 14:38:26 -0500 Subject: [PATCH] Adds tests. -- Tests have been added for unit tests, but not the direct test on `DataServlet` that Michael recommended because of how brittle such a test would be. --- .../exception/DataExceptionHandler.java | 2 +- .../exception/FiliDataExceptionHandler.java | 14 +-- .../webservice/web/endpoints/DataServlet.java | 4 +- .../FiliDataExceptionHandlerSpec.groovy | 98 +++++++++++++++++++ .../FiliDimensionExceptionHandlerSpec.groovy | 91 +++++++++++++++++ .../FiliJobsExceptionHandlerSpec.groovy | 66 +++++++++++++ .../FiliMetricExceptionHandlerSpec.groovy | 85 ++++++++++++++++ .../FiliSlicesExceptionHandlerSpec.groovy | 58 +++++++++++ .../FiliTablesExceptionHandlerSpec.groovy | 83 ++++++++++++++++ 9 files changed, 490 insertions(+), 11 deletions(-) create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDataExceptionHandlerSpec.groovy create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDimensionExceptionHandlerSpec.groovy create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliJobsExceptionHandlerSpec.groovy create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliMetricExceptionHandlerSpec.groovy create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliSlicesExceptionHandlerSpec.groovy create mode 100644 fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliTablesExceptionHandlerSpec.groovy diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/exception/DataExceptionHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/exception/DataExceptionHandler.java index 741330ca00..eb5f5ddcb1 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/exception/DataExceptionHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/exception/DataExceptionHandler.java @@ -32,7 +32,7 @@ public interface DataExceptionHandler { * @param uriInfo URiInfo of the request * @param writer A tool for serializing JSON */ - void handleException( + void handleThrowable( Throwable e, AsyncResponse asyncResponse, Optional apiRequest, diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/exception/FiliDataExceptionHandler.java b/fili-core/src/main/java/com/yahoo/bard/webservice/exception/FiliDataExceptionHandler.java index 94fd553c5f..501a88e81d 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/exception/FiliDataExceptionHandler.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/exception/FiliDataExceptionHandler.java @@ -39,13 +39,13 @@ public class FiliDataExceptionHandler implements DataExceptionHandler { private static final Logger LOG = LoggerFactory.getLogger(FiliDataExceptionHandler.class); @Override - public void handleException( - Throwable e, - AsyncResponse asyncResponse, - Optional apiRequest, - ContainerRequestContext containerRequestContext, - UriInfo uriInfo, - ObjectWriter writer + public void handleThrowable( + Throwable e, + AsyncResponse asyncResponse, + Optional apiRequest, + ContainerRequestContext containerRequestContext, + UriInfo uriInfo, + ObjectWriter writer ) { if (e instanceof RequestValidationException) { LOG.debug(e.getMessage(), e); diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java index bdc4e6b15e..1638d880c5 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DataServlet.java @@ -4,8 +4,6 @@ import static com.yahoo.bard.webservice.web.handlers.workflow.DruidWorkflow.REQUEST_WORKFLOW_TIMER; -import static javax.ws.rs.core.Response.Status.BAD_REQUEST; - import com.yahoo.bard.webservice.application.MetricRegistryFactory; import com.yahoo.bard.webservice.application.ObjectMappersSuite; import com.yahoo.bard.webservice.async.MetadataHttpResponseChannel; @@ -454,7 +452,7 @@ public void getData( throw new IllegalStateException("No request handler accepted request."); } } catch (Throwable e) { - exceptionHandler.handleException( + exceptionHandler.handleThrowable( e, asyncResponse, Optional.ofNullable(apiRequest), diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDataExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDataExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..ec850f7e19 --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDataExceptionHandlerSpec.groovy @@ -0,0 +1,98 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.fasterxml.jackson.databind.ObjectWriter +import com.yahoo.bard.webservice.application.ObjectMappersSuite +import com.yahoo.bard.webservice.data.dimension.TimeoutException +import com.yahoo.bard.webservice.table.resolver.NoMatchFoundException +import com.yahoo.bard.webservice.util.JsonSlurper +import com.yahoo.bard.webservice.web.RequestValidationException +import spock.lang.Shared +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.container.AsyncResponse +import javax.ws.rs.core.Response + +class FiliDataExceptionHandlerSpec extends Specification { + + @Shared + ObjectMappersSuite MAPPERS = new ObjectMappersSuite() + @Shared + ObjectWriter writer = MAPPERS.getMapper().writer() + @Shared + JsonSlurper json = new JsonSlurper() + + AsyncResponse response = Mock(AsyncResponse) + + FiliDataExceptionHandler dataExceptionHandler = new FiliDataExceptionHandler() + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException( + status, + "Error", + "Error" + ) + and: "A mock AsyncResponse to resume" + + when: "We handle the exception" + dataExceptionHandler.handleThrowable(exception, response, Optional.empty(), null, null, writer) + + then: "The response is resumed" + 1 * response.resume(_) >> { + assert it[0].status == status.statusCode + assert json.parseText(it[0].entity).description == "Error" + return true + } + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "NoMatchFoundException returns an Internal Server Error"() { + given: + NoMatchFoundException exception = new NoMatchFoundException("NoMatch") + + when: + dataExceptionHandler.handleThrowable(exception, response, Optional.empty(), null, null, writer) + + then: + 1 * response.resume(_) >> { + assert it[0].status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + assert json.parseText(it[0].entity).description == "NoMatch" + return true + } + } + + def "TimeoutException returns a Gateway Timeout"() { + given: + TimeoutException exception = new TimeoutException("Timeout") + + when: + dataExceptionHandler.handleThrowable(exception, response, Optional.empty(), null, null, writer) + + then: + 1 * response.resume(_) >> { + assert it[0].status == Response.Status.GATEWAY_TIMEOUT.statusCode + assert json.parseText(it[0].entity).description == "Timeout" + return true + } + } + + def "A Throwable returns a Bad Request"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + dataExceptionHandler.handleThrowable(throwable, response, Optional.empty(), null, null, writer) + + then: + 1 * response.resume(_) >> { + assert it[0].status == Response.Status.BAD_REQUEST.statusCode + assert json.parseText(it[0].entity).description == "Throw" + return true + } + } +} diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDimensionExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDimensionExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..69668afaf1 --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliDimensionExceptionHandlerSpec.groovy @@ -0,0 +1,91 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.yahoo.bard.webservice.data.dimension.Dimension +import com.yahoo.bard.webservice.data.dimension.DimensionDictionary +import com.yahoo.bard.webservice.web.ErrorMessageFormat +import com.yahoo.bard.webservice.web.RequestValidationException +import com.yahoo.bard.webservice.web.ResponseCode +import com.yahoo.bard.webservice.web.RowLimitReachedException +import com.yahoo.bard.webservice.web.apirequest.DimensionsApiRequest +import com.yahoo.bard.webservice.web.apirequest.DimensionsApiRequestImpl + +import com.fasterxml.jackson.core.JsonProcessingException + +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.core.Response + +class FiliDimensionExceptionHandlerSpec extends Specification { + + FiliDimensionExceptionHandler dimensionExceptionHandler = new FiliDimensionExceptionHandler() + + Dimension dimension = Stub(Dimension) { + getApiName() >> "dimension" + toString() >> "dimension" + } + + Optional request = Optional.of(new DimensionsApiRequestImpl( + "dimension", + "", + "json", + "", + "", + new DimensionDictionary([dimension] as Set), + null + )) + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException(status, "Error", "Error") + + when: + Response response = dimensionExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == status.statusCode + response.entity == "Error" + + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "RowLimitReached returns an Insufficient Storage"() { + given: + RowLimitReachedException exception = new RowLimitReachedException("RowLimit") + + when: + Response response = dimensionExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == ResponseCode.INSUFFICIENT_STORAGE.statusCode + response.entity == "Row limit exceeded for dimension dimension: RowLimit" + } + + def "JsonProcessing returns an Internal Server Error"() { + given: + JsonProcessingException exception = new JsonProcessingException("JsonError") + + when: + Response response = dimensionExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == ErrorMessageFormat.INTERNAL_SERVER_ERROR_ON_JSON_PROCESSING.format("JsonError") + } + + def "A Throwable returns a Bad Request"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + Response response = dimensionExceptionHandler.handleThrowable(throwable, request, null, null) + + then: + response.status == Response.Status.BAD_REQUEST.statusCode + response.entity == ErrorMessageFormat.REQUEST_PROCESSING_EXCEPTION.format("Throw") + } +} \ No newline at end of file diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliJobsExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliJobsExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..517638ea62 --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliJobsExceptionHandlerSpec.groovy @@ -0,0 +1,66 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.yahoo.bard.webservice.application.ObjectMappersSuite +import com.yahoo.bard.webservice.util.JsonSlurper +import com.yahoo.bard.webservice.web.RequestValidationException +import com.yahoo.bard.webservice.web.apirequest.JobsApiRequest +import com.yahoo.bard.webservice.web.apirequest.JobsApiRequestImpl +import spock.lang.Shared +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.core.Response + +class FiliJobsExceptionHandlerSpec extends Specification { + + @Shared + ObjectMappersSuite objectMappersSuite = new ObjectMappersSuite() + + @Shared + JsonSlurper json = new JsonSlurper() + + FiliJobsExceptionHandler jobsExceptionHandler = new FiliJobsExceptionHandler(objectMappersSuite) + + Optional request = Optional.of( + new JobsApiRequestImpl( + "json", + "500", + "", + "", + "", + null, + null, + null + ) + ) + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException(status, "Error", "Error") + + when: + Response response = jobsExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == status.statusCode + json.parseText(response.entity).description == "Error" + + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "A Throwable returns an Internal Server Error"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + Response response = jobsExceptionHandler.handleThrowable(throwable, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == "Throw" + } +} \ No newline at end of file diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliMetricExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliMetricExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..1d87630b82 --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliMetricExceptionHandlerSpec.groovy @@ -0,0 +1,85 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.yahoo.bard.webservice.data.metric.LogicalMetric +import com.yahoo.bard.webservice.data.metric.LogicalMetricInfo +import com.yahoo.bard.webservice.data.metric.MetricDictionary +import com.yahoo.bard.webservice.data.metric.TemplateDruidQuery +import com.yahoo.bard.webservice.data.metric.mappers.NoOpResultSetMapper +import com.yahoo.bard.webservice.web.ErrorMessageFormat +import com.yahoo.bard.webservice.web.RequestValidationException +import com.yahoo.bard.webservice.web.apirequest.MetricsApiRequest +import com.yahoo.bard.webservice.web.apirequest.MetricsApiRequestImpl +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.core.Response + +class FiliMetricExceptionHandlerSpec extends Specification { + + FiliMetricExceptionHandler metricsExceptionHandler = new FiliMetricExceptionHandler() + + LogicalMetric metric = new LogicalMetric( + Stub(TemplateDruidQuery), + new NoOpResultSetMapper(), + new LogicalMetricInfo("metric") + ) + + MetricDictionary dictionary = new MetricDictionary() + + Optional request + + def setup() { + dictionary.put("metric", metric) + request = Optional.of(new MetricsApiRequestImpl( + "metric", + "json", + "", + "", + dictionary, + null + )) + } + + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException(status, "Error", "Error") + + when: + Response response = metricsExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == status.statusCode + response.entity == "Error" + + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "IOException returns an Internal Server Errror"() { + given: + IOException exception = new IOException("IOException") + + when: + Response response = metricsExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == "Internal server error. IOException : $exception.message" + } + + def "A Throwable returns a Bad Request"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + Response response = metricsExceptionHandler.handleThrowable(throwable, request, null, null) + + then: + response.status == Response.Status.BAD_REQUEST.statusCode + response.entity == ErrorMessageFormat.REQUEST_PROCESSING_EXCEPTION.format("Throw") + } +} \ No newline at end of file diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliSlicesExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliSlicesExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..f43bc21ca9 --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliSlicesExceptionHandlerSpec.groovy @@ -0,0 +1,58 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.yahoo.bard.webservice.web.RequestValidationException +import com.yahoo.bard.webservice.web.apirequest.SlicesApiRequest +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.core.Response + + +class FiliSlicesExceptionHandlerSpec extends Specification { + + FiliSlicesExceptionHandler slicesExceptionHandler = new FiliSlicesExceptionHandler() + + Optional request = Optional.of(Stub(SlicesApiRequest)) + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException(status, "Error", "Error") + + when: + Response response = slicesExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == status.statusCode + response.entity == "Error" + + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "IOException returns an Internal Server Error"() { + given: + IOException exception = new IOException("IOException") + + when: + Response response = slicesExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == "Internal server error. IOException : $exception.message" + } + + def "A Throwable returns an Internal Server Error"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + Response response = slicesExceptionHandler.handleThrowable(throwable, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == "Throw" + } +} \ No newline at end of file diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliTablesExceptionHandlerSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliTablesExceptionHandlerSpec.groovy new file mode 100644 index 0000000000..041f406c7a --- /dev/null +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/exception/FiliTablesExceptionHandlerSpec.groovy @@ -0,0 +1,83 @@ +// Copyright 2018 Oath Inc. +// Licensed under the terms of the Apache license. Please see LICENSE.md file distributed with this work for terms. +package com.yahoo.bard.webservice.exception + +import com.fasterxml.jackson.core.JsonProcessingException +import com.yahoo.bard.webservice.data.metric.MetricDictionary +import com.yahoo.bard.webservice.data.time.DefaultTimeGrain +import com.yahoo.bard.webservice.table.LogicalTable +import com.yahoo.bard.webservice.table.TableGroup +import com.yahoo.bard.webservice.web.ErrorMessageFormat +import com.yahoo.bard.webservice.web.RequestValidationException +import com.yahoo.bard.webservice.web.ResponseFormatType +import com.yahoo.bard.webservice.web.apirequest.TablesApiRequest +import com.yahoo.bard.webservice.web.apirequest.TablesApiRequestImpl +import com.yahoo.bard.webservice.web.filters.ApiFilters +import spock.lang.Specification +import spock.lang.Unroll + +import javax.ws.rs.core.Response + +class FiliTablesExceptionHandlerSpec extends Specification { + + FiliTablesExceptionHandler tablesExceptionHandler = new FiliTablesExceptionHandler() + + Optional request = Optional.of(new TablesApiRequestImpl( + ResponseFormatType.JSON, + Optional.empty(), + null, + null, + [] as Set, + new LogicalTable( + "table", + DefaultTimeGrain.DAY, + new TableGroup([] as LinkedHashSet, [] as Set), Stub(MetricDictionary) + ), + DefaultTimeGrain.DAY, + [] as Set, + [] as Set, + [] as Set, + new ApiFilters() + )) + + + @Unroll + def "The handler forwards the #status from a RequestValidationException as the status of the request"() { + given: "The ResponseValidationException to validate" + RequestValidationException exception = new RequestValidationException(status, "Error", "Error") + + when: + Response response = tablesExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == status.statusCode + response.entity == "Error" + + where: + status << [Response.Status.BAD_REQUEST, Response.Status.NOT_IMPLEMENTED] + } + + def "JsonProcessing returns an Internal Server Error"() { + given: + JsonProcessingException exception = new JsonProcessingException("JsonError") + + when: + Response response = tablesExceptionHandler.handleThrowable(exception, request, null, null) + + then: + response.status == Response.Status.INTERNAL_SERVER_ERROR.statusCode + response.entity == ErrorMessageFormat.INTERNAL_SERVER_ERROR_ON_JSON_PROCESSING.format("JsonError") + } + + def "A Throwable returns a Bad Request"() { + given: + Throwable throwable = new Throwable("Throw") + + when: + Response response = tablesExceptionHandler.handleThrowable(throwable, request, null, null) + + then: + response.status == Response.Status.BAD_REQUEST.statusCode + response.entity == ErrorMessageFormat.REQUEST_PROCESSING_EXCEPTION.format("Throw") + } +} \ No newline at end of file