diff --git a/CHANGELOG.md b/CHANGELOG.md index 12f3c26961..02ebdde5b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,12 @@ Current starts the timer. Note: This won't work when performing timings across threads, or across contexts. Those need to be started and stopped manually. +- [Clean up logging and responses in `DimensionCacheLoaderServlet`](https://github.com/yahoo/fili/pull/163) + * Switched a number of `error`-level logs to `debug` level to line up with logging guidance when request failures + were result of client error + * Reduced some `info`-level logs down to `debug` + * Converted to 404 when error was cause by not finding a path element + ### Deprecated: - [CompositePhsyicalTable Core Components Refactor](https://github.com/yahoo/fili/pull/179) diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/application/Loader.java b/fili-core/src/main/java/com/yahoo/bard/webservice/application/Loader.java index 3a80baa7cb..d9bd0c2316 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/application/Loader.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/application/Loader.java @@ -13,7 +13,7 @@ import java.util.concurrent.ScheduledFuture; /** - * Defines a task that loads data from Druid and is scheduled to run at a given time, potentially periodically. + * Defines a task that is scheduled to run at a given time, potentially periodically. * * @param The type of the result returned by the task associated with this loader. */ diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/client/DruidClientConfigHelper.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/client/DruidClientConfigHelper.java index 6c7925ad54..561ca29ebe 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/client/DruidClientConfigHelper.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/client/DruidClientConfigHelper.java @@ -9,6 +9,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.concurrent.TimeUnit; + /** * Helper to fetch druid url and timeout settings. */ @@ -62,7 +64,7 @@ public class DruidClientConfigHelper { /** * The default timeout for queries. */ - private static final int DRUID_REQUEST_TIMEOUT_DEFAULT = 10 * 60 * 1000; // 10 Minutes, in milliseconds + private static final int DRUID_REQUEST_TIMEOUT_DEFAULT = Math.toIntExact(TimeUnit.MINUTES.toMillis(10)); /** * Fetches the druid UI request Priority. diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/DefaultQueryType.java b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/DefaultQueryType.java index a571b6ac6d..be950e5ab0 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/DefaultQueryType.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/druid/model/DefaultQueryType.java @@ -5,7 +5,7 @@ import com.yahoo.bard.webservice.util.EnumUtils; /** - * Druid queries that Fili supports out o the box. + * Druid queries that Fili supports out of the box. */ public enum DefaultQueryType implements QueryType { GROUP_BY, diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServlet.java b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServlet.java index eea2a9d19c..204ed8e676 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServlet.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServlet.java @@ -3,6 +3,8 @@ package com.yahoo.bard.webservice.web.endpoints; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static javax.ws.rs.core.Response.Status.INTERNAL_SERVER_ERROR; +import static javax.ws.rs.core.Response.Status.NOT_FOUND; import com.yahoo.bard.webservice.application.ObjectMappersSuite; import com.yahoo.bard.webservice.data.cache.DataCache; @@ -18,6 +20,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import org.joda.time.DateTime; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,8 +54,7 @@ public class DimensionCacheLoaderServlet { private final DimensionDictionary dimensionDictionary; private final ObjectMapper mapper; - @SuppressWarnings("rawtypes") - private final DataCache dataCache; + private final DataCache dataCache; /** * Constructor. @@ -61,11 +63,10 @@ public class DimensionCacheLoaderServlet { * @param dataCache A cache that we can clear if told to * @param objectMappers Shares mappers for dealing with JSON */ - @SuppressWarnings("rawtypes") // This issue is enforced by binder limitations for matching DataCaches @Inject public DimensionCacheLoaderServlet( DimensionDictionary dimensionDictionary, - @NotNull DataCache dataCache, + @NotNull DataCache dataCache, ObjectMappersSuite objectMappers ) { this.mapper = objectMappers.getMapper(); @@ -92,46 +93,40 @@ public DimensionCacheLoaderServlet( @Path("/dimensions/{dimensionName}") @Consumes("application/json") public Response updateDimensionLastUpdated(@PathParam("dimensionName") String dimensionName, String json) { - LOG.debug("Updating lastUpdated for dimension:{} using json: {}", dimensionName, json); - DimensionUpdateDate dimensionUpdateDate; try { - Dimension dimension = dimensionDictionary.findByApiName(dimensionName); // if dimension is not located return bad request response + Dimension dimension = dimensionDictionary.findByApiName(dimensionName); if (dimension == null) { - LOG.error("Dimension {} cannot be found", dimensionName); String message = String.format("Dimension %s cannot be found.", dimensionName); - return Response.status(BAD_REQUEST).entity(message).build(); + LOG.debug(message); + return Response.status(NOT_FOUND).entity(message).build(); } // Extract LastUpdated as string from the post data - dimensionUpdateDate = mapper.readValue( - json, new TypeReference() { - // No-op - } + DimensionUpdateDate dimensionUpdateDate = mapper.readValue( + json, + new TypeReference() { /* Empty class */ } ); - if (dimensionUpdateDate.getLastUpdated() == null) { - LOG.error("lastUpdated value not in json"); - return Response.status(BAD_REQUEST).entity("Last updated value not in json.").build(); + DateTime lastUpdated = dimensionUpdateDate.getLastUpdated(); + if (lastUpdated == null) { + String message = "lastUpdated value not in json"; + LOG.error(message); + return Response.status(BAD_REQUEST).entity(message).build(); + } else { + dimension.setLastUpdated(lastUpdated); } - // update last updated - dimension.setLastUpdated(dimensionUpdateDate.getLastUpdated()); - + LOG.debug("Successfully updated lastUpdated {} for dimension: {}", lastUpdated, dimensionName); + return Response.status(Status.OK).build(); } catch (IOException e) { - LOG.error("Failed to process lastUpdated date: {}", e); - return Response.status(BAD_REQUEST).entity("Failed to process lastUpdated date: " + e).build(); + String message = "Failed to process lastUpdated date"; + LOG.error(message, e); + return Response.status(INTERNAL_SERVER_ERROR).entity(message).build(); } - LOG.info( - "Successfully updated lastUpdated {} for dimension: {}", - dimensionUpdateDate.getLastUpdated(), - dimensionName - ); - return Response.status(Status.OK).build(); } - /** * Get the lastUpdated date for a particular dimension. * @@ -150,23 +145,24 @@ public Response updateDimensionLastUpdated(@PathParam("dimensionName") String di @Path("/dimensions/{dimensionName}") @Consumes("application/json") public Response getDimensionLastUpdated(@PathParam("dimensionName") String dimensionName) { + // if dimension is not located return bad request response + Dimension dimension = dimensionDictionary.findByApiName(dimensionName); + if (dimension == null) { + String message = String.format("Dimension %s cannot be found.", dimensionName); + LOG.debug(message); + return Response.status(NOT_FOUND).entity(message).build(); + } + + Map result = new LinkedHashMap<>(); + result.put("name", dimensionName); + result.put("lastUpdated", dimension.getLastUpdated() == null ? null : dimension.getLastUpdated().toString()); try { - Dimension dimension = dimensionDictionary.findByApiName(dimensionName); - // if dimension is not located return bad request response - if (dimension == null) { - String message = String.format("Dimension %s cannot be found.", dimensionName); - return Response.status(BAD_REQUEST).entity(message).build(); - } - Map result = new LinkedHashMap<>(); - String lastUpdated = (dimension.getLastUpdated() == null ? null : dimension.getLastUpdated().toString()); - result.put("name", dimensionName); - result.put("lastUpdated", lastUpdated); - // Extract LastUpdated as string from the post data - String output = mapper.writeValueAsString(result); - return Response.status(Status.OK).entity(output).build(); + return Response.status(Status.OK).entity(mapper.writeValueAsString(result)).build(); } catch (IOException e) { - return Response.status(BAD_REQUEST).entity("Exception: " + e.getMessage()).build(); + String message = String.format("Error retrieving lastUpdated for %s", dimensionName); + LOG.error(message, e); + return Response.status(INTERNAL_SERVER_ERROR).entity(message).build(); } } @@ -194,36 +190,34 @@ public Response getDimensionLastUpdated(@PathParam("dimensionName") String dimen @Consumes("application/json; charset=utf-8") public Response addReplaceDimensionRows(@PathParam("dimensionName") String dimensionName, String json) { LOG.debug("Replacing {} dimension rows with a json payload {} characters long", dimensionName, json.length()); - Map>> dimensionRows; try { - Dimension dimension = dimensionDictionary.findByApiName(dimensionName); // if dimension is not located return bad request response + Dimension dimension = dimensionDictionary.findByApiName(dimensionName); if (dimension == null) { - LOG.error("Missing dimensionRows for dimension: {}", dimensionName); - return Response.status(BAD_REQUEST).build(); + String message = String.format("Dimension %s cannot be found.", dimensionName); + LOG.debug(message); + return Response.status(NOT_FOUND).entity(message).build(); } // extract dimension rows form the post data - dimensionRows = mapper.readValue( - json, new TypeReference>>>() { - // Empty class - } + Map>> rawDimensionRows = mapper.readValue( + json, + new TypeReference>>>() { /* Empty class */ } ); - Set drs = dimensionRows.get("dimensionRows").stream() + Set dimensionRows = rawDimensionRows.get("dimensionRows").stream() .map(dimension::parseDimensionRow) .collect(Collectors.toCollection(LinkedHashSet::new)); - dimension.addAllDimensionRows(drs); + dimension.addAllDimensionRows(dimensionRows); + LOG.debug("Successfully added/replaced {} row(s) for dimension: {}", dimensionRows.size(), dimensionName); + return Response.status(Status.OK).build(); } catch (IOException e) { - LOG.error("Failed to add/replace dimension rows: {}", e); - return Response.status(BAD_REQUEST).build(); + String message = "Failed to add/replace dimension rows"; + LOG.error(message, e); + return Response.status(INTERNAL_SERVER_ERROR).entity(message).build(); } - - int dimensionRowsSize = dimensionRows.get("dimensionRows").size(); - LOG.info("Successfully added/replaced {} row(s) for dimension: {}", dimensionRowsSize, dimensionName); - return Response.status(Status.OK).build(); } /** @@ -282,49 +276,49 @@ public Response addReplaceDimensionRows(@PathParam("dimensionName") String dimen @Consumes("application/json") public Response addUpdateDimensionRows(@PathParam("dimensionName") String dimensionName, String json) { LOG.debug("Updating {} dimension rows with a json payload {} characters long", dimensionName, json.length()); - Map>> dimensionRows; try { - Dimension dimension = dimensionDictionary.findByApiName(dimensionName); // if dimension is not located return bad request response + Dimension dimension = dimensionDictionary.findByApiName(dimensionName); if (dimension == null) { - LOG.error("Missing dimensionRows for dimension: {}", dimensionName); - return Response.status(BAD_REQUEST).build(); + String message = String.format("Dimension %s cannot be found.", dimensionName); + LOG.debug(message); + return Response.status(NOT_FOUND).entity(message).build(); } // extract dimension rows form the post data - dimensionRows = mapper.readValue( - json, new TypeReference>>>() { - // Empty class - } + Map>> rawDimensionRows = mapper.readValue( + json, + new TypeReference>>>() { /* Empty class */ } ); - Set drs = new LinkedHashSet<>(); + DimensionField key = dimension.getKey(); - for (LinkedHashMap fieldnameValueMap: dimensionRows.get("dimensionRows")) { + Set dimensionRows = new LinkedHashSet<>(); + for (Map fieldnameValueMap: rawDimensionRows.get("dimensionRows")) { DimensionRow newRow = dimension.parseDimensionRow(fieldnameValueMap); DimensionRow oldRow = dimension.findDimensionRowByKeyValue(newRow.get(key)); if (oldRow == null) { - drs.add(newRow); + // It didn't exist before, so add it directly + dimensionRows.add(newRow); } else { + // The row existed before, so do an update on the existing row's data for (DimensionField field : dimension.getDimensionFields()) { - String fieldName = field.getName(); // only overwrite if the field was in the original JSON - if (fieldnameValueMap.containsKey(fieldName)) { + if (fieldnameValueMap.containsKey(field.getName())) { oldRow.put(field, newRow.get(field)); } - drs.add(oldRow); + dimensionRows.add(oldRow); } } } - dimension.addAllDimensionRows(drs); + dimension.addAllDimensionRows(dimensionRows); + LOG.debug("Successfully added/updated {} row(s) for dimension: {}", dimensionRows.size(), dimensionName); + return Response.status(Status.OK).build(); } catch (IOException e) { - LOG.error("Failed to add/update dimension rows: {}", e); - return Response.status(BAD_REQUEST).build(); + String message = "Failed to add/update dimension rows"; + LOG.error(message, e); + return Response.status(INTERNAL_SERVER_ERROR).entity(message).build(); } - - int dimensionRowsSize = dimensionRows.get("dimensionRows").size(); - LOG.info("Successfully added/updated {} row(s) for dimension: {}", dimensionRowsSize, dimensionName); - return Response.status(Status.OK).build(); } /** @@ -350,25 +344,22 @@ public Response updateDimensionLastUpdated(String json) { Map postDataMap; try { // Extract LastUpdated as string from the post data - postDataMap = mapper.readValue( - json, new TypeReference>() { - // Empty class - } - ); + postDataMap = mapper.readValue(json, new TypeReference>() { /* Empty class */ }); if (!postDataMap.containsKey("cacheStatus")) { - LOG.error("Missing cacheStatus in json: {}", json); - return Response.status(BAD_REQUEST).entity("Missing cacheStatus in json: " + json).build(); + String message = String.format("Missing cacheStatus in json: %s", json); + LOG.error(message); + return Response.status(BAD_REQUEST).entity(message).build(); } - // TODO tie this to cache status management when implemented + + LOG.debug("Successfully updated cacheStatus to: {}", postDataMap.get("cacheStatus")); + return Response.status(Status.OK).build(); } catch (IOException e) { - LOG.error("Failed to update lastUpdated: {}", e); - return Response.status(BAD_REQUEST).entity("Exception: " + e).build(); + String message = "Failed to update lastUpdated. Unable to process payload"; + LOG.error(message, e); + return Response.status(BAD_REQUEST).entity(message).build(); } - - LOG.info("Successfully updated cacheStatus to: {}", postDataMap.get("cacheStatus")); - return Response.status(Status.OK).build(); } /** @@ -384,10 +375,11 @@ public Response getMsg() { // TODO tie this cache status when implemented cacheStatus.put("cacheStatus", "Active"); try { - String output = mapper.writeValueAsString(cacheStatus); - return Response.status(Status.OK).entity(output).build(); + return Response.status(Status.OK).entity(mapper.writeValueAsString(cacheStatus)).build(); } catch (JsonProcessingException e) { - return Response.status(Status.INTERNAL_SERVER_ERROR).build(); + String message = "Unable to serialize cache status"; + LOG.error(message, e); + return Response.status(INTERNAL_SERVER_ERROR).entity(message).build(); } } diff --git a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServletSpec.groovy b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServletSpec.groovy index 04eda8ea36..2e2d535bf2 100644 --- a/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServletSpec.groovy +++ b/fili-core/src/test/groovy/com/yahoo/bard/webservice/web/endpoints/DimensionCacheLoaderServletSpec.groovy @@ -1,6 +1,7 @@ // Copyright 2016 Yahoo 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.web.endpoints + import com.yahoo.bard.webservice.application.JerseyTestBinder import com.yahoo.bard.webservice.application.ObjectMappersSuite import com.yahoo.bard.webservice.data.cache.DataCache @@ -27,7 +28,7 @@ import javax.ws.rs.core.Response.Status class DimensionCacheLoaderServletSpec extends Specification { private static final ObjectMappersSuite MAPPERS = new ObjectMappersSuite() - static final DateTimeZone ORIGINAL_TIME_ZONE = DateTimeZone.getDefault() + DateTimeZone originalTimeZone DimensionCacheLoaderServlet dimensionCacheLoaderServlet @@ -47,17 +48,16 @@ class DimensionCacheLoaderServletSpec extends Specification { DateTime lastUpdated = new DateTime(50000) def setup() { + originalTimeZone = DateTimeZone.getDefault() DateTimeZone.setDefault(DateTimeZone.forTimeZone(TimeZone.getTimeZone("America/Chicago"))) - dimensionGenderFields = new LinkedHashSet<>() - dimensionGenderFields.add(BardDimensionField.ID) - dimensionGenderFields.add(BardDimensionField.DESC) - - dimensionUserCountryFields = new LinkedHashSet<>() - dimensionUserCountryFields.add(BardDimensionField.ID) - dimensionUserCountryFields.add(BardDimensionField.DESC) - dimensionUserCountryFields.add(BardDimensionField.FIELD1) - dimensionUserCountryFields.add(BardDimensionField.FIELD2) + dimensionGenderFields = [BardDimensionField.ID, BardDimensionField.DESC] as LinkedHashSet + dimensionUserCountryFields = [ + BardDimensionField.ID, + BardDimensionField.DESC, + BardDimensionField.FIELD1, + BardDimensionField.FIELD2 + ] as LinkedHashSet Set dimensions = [] as Set @@ -101,47 +101,38 @@ class DimensionCacheLoaderServletSpec extends Specification { } def cleanup() { - DateTimeZone.setDefault(ORIGINAL_TIME_ZONE) + DateTimeZone.setDefault(originalTimeZone) dimensionGender.searchProvider.clearDimension() dimensionUserCountry.searchProvider.clearDimension() } - def "Check updateDimensionLastUpdated for dimension cache loader endpoint"() { - + def "UpdateDimensionLastUpdated works as expected"() { setup: DateTime newLastUpdated = new DateTime(10000) - String post = """{"name": "gender","lastUpdated":"$newLastUpdated"}""" - def dims = dimensionCacheLoaderServlet.dimensionDictionary.findAll() + Set dims = dimensionCacheLoaderServlet.dimensionDictionary.findAll() when: Response r = dimensionCacheLoaderServlet.updateDimensionLastUpdated("gender", post) - then: "The dimension row we updated has changed but the others have not" - dims.find { it.apiName == "user_country" }.lastUpdated == lastUpdated + then: "The dimension row we updated has changed" dims.find { it.apiName == "gender" }.lastUpdated == newLastUpdated r.getStatusInfo() == Status.OK + + and: "Others have not" + dims.find { it.apiName == "user_country" }.lastUpdated == lastUpdated } - def "Check updateDimensionLastUpdated for missing dimension fails"() { - + def "UpdateDimensionLastUpdated with unknown dimension gives a NOT FOUND response"() { setup: - DateTime newLastUpdated = new DateTime(10000) - - String post = """{"name": "unknown","lastUpdated":"$newLastUpdated"}""" - - def dims = dimensionCacheLoaderServlet.dimensionDictionary.findAll() - - when: - Response r = dimensionCacheLoaderServlet.updateDimensionLastUpdated("unknown", post) + String post = """{"name": "unknown","lastUpdated":"${new DateTime(10000)}"}""" - then: "Bad Request" - r.getStatusInfo() == Status.BAD_REQUEST + expect: + dimensionCacheLoaderServlet.updateDimensionLastUpdated("unknown", post).getStatusInfo() == Status.NOT_FOUND } def "Check addReplaceDimensionRows for dimension cache loader endpoint"() { - setup: String post = """{ "dimensionRows": [ @@ -192,8 +183,8 @@ class DimensionCacheLoaderServletSpec extends Specification { then: r.getStatusInfo() == Status.OK - expectedDimensionUserCountry.searchProvider.findAllDimensionRows(). - containsAll(dimensionUserCountry.searchProvider.findAllDimensionRows()) + expectedDimensionUserCountry.searchProvider.findAllDimensionRows() + .containsAll(dimensionUserCountry.searchProvider.findAllDimensionRows()) } def "Check addUpdateDimensionRows for dimension cache loader endpoint"() { @@ -251,16 +242,18 @@ class DimensionCacheLoaderServletSpec extends Specification { then: r.getStatusInfo() == Status.OK - expectedDimensionUserCountry.searchProvider.findAllDimensionRows(). - containsAll(dimensionUserCountry.searchProvider.findAllDimensionRows()) + expectedDimensionUserCountry.searchProvider.findAllDimensionRows() + .containsAll(dimensionUserCountry.searchProvider.findAllDimensionRows()) } - def "Check servet getDimensionLastUpdated"() { + def "Check servlet getDimensionLastUpdated"() { setup: String expected = """{"name":"gender","lastUpdated":"$lastUpdated"}""" - expect: + when: Response r = dimensionCacheLoaderServlet.getDimensionLastUpdated("gender") + + then: r.getStatusInfo() == Status.OK GroovyTestUtils.compareJson(expected, r.getEntity()) } @@ -269,14 +262,16 @@ class DimensionCacheLoaderServletSpec extends Specification { setup: String expected = """{"cacheStatus":"Active"}""" - expect: + when: Response r = dimensionCacheLoaderServlet.getMsg() + + then: r.getStatusInfo() == Status.OK GroovyTestUtils.compareJson(expected, r.getEntity()) } def "Delete data cache"() { - // mock new servlet that enforces one call to dataCache clear + // mock new servlet that validates one call to dataCache clear DataCache dataCache2 = Mock(DataCache) 1 * dataCache2.clear() def dimensionCacheLoaderServlet2 = new DimensionCacheLoaderServlet( @@ -285,28 +280,19 @@ class DimensionCacheLoaderServletSpec extends Specification { MAPPERS ) - when: - Response r = dimensionCacheLoaderServlet2.deleteDataCache() - - then: - r.getStatusInfo() == Status.OK + expect: + dimensionCacheLoaderServlet2.deleteDataCache().getStatusInfo() == Status.OK } def "Delete data cache endpoint"() { setup: JerseyTestBinder jtb = new JerseyTestBinder(DimensionCacheLoaderServlet.class) - when: "DELETE" - Response r = jtb.getHarness().target("cache/data").request().delete() - - then: "OK" - Status.OK.getStatusCode() == r.getStatus() - - when: "GET" - r = jtb.getHarness().target("cache/data").request().get() + expect: "DELETE is allowed" + jtb.getHarness().target("cache/data").request().delete().getStatusInfo() == Status.OK - then: "method not allowed" - Status.METHOD_NOT_ALLOWED.getStatusCode() == r.getStatus() + and: "GET is not" + jtb.getHarness().target("cache/data").request().get().getStatusInfo() == Status.METHOD_NOT_ALLOWED cleanup: jtb.tearDown() @@ -315,13 +301,7 @@ class DimensionCacheLoaderServletSpec extends Specification { def "POST to addReplaceDimensionRows accepts different charsets for application/json"() { setup: JerseyTestBinder jtb = new JerseyTestBinder(DimensionCacheLoaderServlet.class) - Response r; - - when: "POST color dimension rows with Content-Type: application/json" - r = jtb.getHarness().target("cache/dimensions/color/dimensionRows") - .request() - .header("Content-Type", "application/json") - .post(Entity.json(""" + def postBody = Entity.json(""" { "dimensionRows": [ { @@ -329,44 +309,25 @@ class DimensionCacheLoaderServletSpec extends Specification { "description": "red" } ] - }""")) + }""") - then: "OK" - Status.OK.getStatusCode() == r.getStatus() + expect: "can POST with Content-Type: application/json" + jtb.getHarness().target("cache/dimensions/color/dimensionRows") + .request() + .header("Content-Type", "application/json") + .post(postBody).getStatusInfo() == Status.OK - when: "POST color dimension rows with Content-Type: application/json; charset=utf-8" - r = jtb.getHarness().target("cache/dimensions/color/dimensionRows") + and: "can POST with Content-Type: application/json; charset=utf-8" + jtb.getHarness().target("cache/dimensions/color/dimensionRows") .request() .header("Content-Type", "application/json; charset=utf-8") - .post(Entity.json(""" - { - "dimensionRows": [ - { - "id": "2", - "description": "blue" - } - ] - }""")) - - then: "OK" - Status.OK.getStatusCode() == r.getStatus() + .post(postBody).getStatusInfo() == Status.OK - when: "POST color dimension rows with Content-Type: application/json; charset=utf-16" - r = jtb.getHarness().target("cache/dimensions/color/dimensionRows") + and: "can POST with Content-Type: application/json; charset=utf-16" + jtb.getHarness().target("cache/dimensions/color/dimensionRows") .request() .header("Content-Type", "application/json; charset=utf-16") - .post(Entity.json(""" - { - "dimensionRows": [ - { - "id": "3", - "description": "green" - } - ] - }""")) - - then: "OK" - Status.OK.getStatusCode() == r.getStatus() + .post(postBody).getStatusInfo() == Status.OK cleanup: jtb.tearDown() diff --git a/fili-core/src/test/java/com/yahoo/bard/webservice/util/ClassScanner.java b/fili-core/src/test/java/com/yahoo/bard/webservice/util/ClassScanner.java index 3cf9b70282..2321128dbc 100644 --- a/fili-core/src/test/java/com/yahoo/bard/webservice/util/ClassScanner.java +++ b/fili-core/src/test/java/com/yahoo/bard/webservice/util/ClassScanner.java @@ -92,7 +92,8 @@ public List> getClasses() throws IOException, ClassNotFoundException { URL resource = resources.nextElement(); dirs.add(new File(resource.getFile())); } - ArrayList> classes = new ArrayList<>(); + + List> classes = new ArrayList<>(); for (File directory : dirs) { classes.addAll(findClasses(directory, packageName)); } @@ -112,14 +113,16 @@ private List> findClasses(File directory, String packageName) throws Cl if (!directory.exists()) { return classes; } - TreeSet files = new TreeSet<>(Arrays.asList(directory.listFiles())); + Iterable files = new TreeSet<>(Arrays.asList(directory.listFiles())); for (File file : files) { String name = file.getName(); if (file.isDirectory()) { assert !name.contains("."); + // Extend the package and recurse classes.addAll(findClasses(file, packageName + "." + name)); } else if (name.endsWith(".class")) { + // Grab just the class name, stripping the .class extension name = packageName + '.' + name.substring(0, name.length() - 6); classes.add(Class.forName(name)); } @@ -174,7 +177,7 @@ private T constructObject(Class newClass, Args mode, Stack stack) } // saves context for the InstantiationException - IllegalArgumentException cause = null; + IllegalArgumentException cause; // Try a no arg constructor first try { @@ -277,7 +280,6 @@ private T constructArg(Class cls, Args mode, Stack stack) throws I arg = cachedValue; } else if (cls == Object[].class) { arg = (T) new Object[0]; - } else if (cls.isArray()) { Object arrayElement = constructArg(cls.getComponentType(), mode, stack); arg = (T) Array.newInstance(cls.getComponentType(), 1); @@ -287,7 +289,7 @@ private T constructArg(Class cls, Args mode, Stack stack) throws I } else { try { arg = cls.newInstance(); - } catch (Throwable e) { + } catch (Throwable ignored) { try { arg = constructObject(cls, mode, stack); } catch (Throwable e2) { @@ -389,8 +391,8 @@ private T getCachedValue(Class cls) { * * @param values The value being cached */ - public void putInArgumentValueCache(Collection values) { - values.stream().forEach(value -> putInArgumentValueCache(value.getClass(), value)); + public void putInArgumentValueCache(Collection values) { + values.forEach(value -> putInArgumentValueCache(value.getClass(), value)); } /** diff --git a/fili-system-config/src/main/java/com/yahoo/bard/webservice/config/ConfigResourceLoader.java b/fili-system-config/src/main/java/com/yahoo/bard/webservice/config/ConfigResourceLoader.java index bf7dcd420d..7668af99e3 100644 --- a/fili-system-config/src/main/java/com/yahoo/bard/webservice/config/ConfigResourceLoader.java +++ b/fili-system-config/src/main/java/com/yahoo/bard/webservice/config/ConfigResourceLoader.java @@ -93,8 +93,9 @@ public Configuration loadConfigFromResource(Resource resource) { result.load(resource.getInputStream()); return result; } catch (ConfigurationException | IOException e) { - LOG.error(CONFIGURATION_LOAD_ERROR.format(resource.getFilename()), e); - throw new SystemConfigException(CONFIGURATION_LOAD_ERROR.format(resource.getFilename()), e); + String message = CONFIGURATION_LOAD_ERROR.format(resource.getFilename()); + LOG.error(message, e); + throw new SystemConfigException(message, e); } } @@ -108,7 +109,7 @@ public Configuration loadConfigFromResource(Resource resource) { public boolean isResourceNotAJar(Resource resource) { try { return !resource.getURI().getScheme().equals("jar"); - } catch (IOException e) { + } catch (IOException ignored) { // If the resource doesn't parse cleanly as a URI, it's not a jar return true; }