refactor(objectmapper): Fixup object mapper autowiring. (#105)

spinnaker · Oct 18, 2017 · 5a48ebe · 5a48ebe
1 parent fee8dae
commit 5a48ebe
Show file tree

Hide file tree

Showing 18 changed files with 99 additions and 134 deletions.
diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/config/AtlasSSEConverter.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/config/AtlasSSEConverter.java
@@ -18,8 +18,8 @@
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.kayenta.atlas.model.AtlasResults;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import lombok.extern.slf4j.Slf4j;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 import org.springframework.util.StringUtils;
 import retrofit.converter.ConversionException;
@@ -36,17 +36,16 @@
 import java.util.Objects;
 import java.util.stream.Collectors;
 
-import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
-
 @Component
 @Slf4j
 // Atlas returns one json stanza per line; we rely on that here.
 // We are not implementing full, proper SSE handling here.
 public class AtlasSSEConverter implements Converter {
 
-  private static final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
-  private static final List<String> expectedResultsTypeList = Arrays.asList("timeseries", "close");
+  private static final List<String> EXPECTED_RESULTS_TYPE_LIST = Arrays.asList("timeseries", "close");
+
+  @Autowired
+  ObjectMapper kayentaObjectMapper;
 
   @Override
   public List<AtlasResults> fromBody(TypedInput body, Type type) throws ConversionException {
@@ -67,7 +66,7 @@ public List<AtlasResults> fromBody(TypedInput body, Type type) throws Conversion
       List<AtlasResults> atlasResultsList =
         tokenizedLines
           .stream()
-          .map(AtlasSSEConverter::convertTokenizedLineToAtlasResults)
+          .map(this::convertTokenizedLineToAtlasResults)
           .filter(Objects::nonNull)
           .collect(Collectors.toList());
 
@@ -91,12 +90,12 @@ public List<AtlasResults> fromBody(TypedInput body, Type type) throws Conversion
     return null;
   }
 
-  private static AtlasResults convertTokenizedLineToAtlasResults(String[] tokenizedLine) {
+  private AtlasResults convertTokenizedLineToAtlasResults(String[] tokenizedLine) {
     try {
-      AtlasResults atlasResults = objectMapper.readValue(tokenizedLine[1], AtlasResults.class);
+      AtlasResults atlasResults = kayentaObjectMapper.readValue(tokenizedLine[1], AtlasResults.class);
       String atlasResultsType = atlasResults.getType();
 
-      if (StringUtils.isEmpty(atlasResultsType) || !expectedResultsTypeList.contains(atlasResultsType)) {
+      if (StringUtils.isEmpty(atlasResultsType) || !EXPECTED_RESULTS_TYPE_LIST.contains(atlasResultsType)) {
         log.info("Received results of type other than 'timeseries' or 'close' from Atlas: {}", atlasResults);
 
         // TODO: Retry on type 'error'?

diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/controllers/AtlasFetchController.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/controllers/AtlasFetchController.java
@@ -53,8 +53,8 @@ public String queryMetrics(@RequestParam(required = false) final String metricsA
                              @ApiParam(defaultValue = "cpu") @RequestParam String metricSetName,
                              @ApiParam(defaultValue = "cluster") @RequestParam String type,
                              @RequestParam String scope,
-                             @ApiParam(defaultValue = "2000-01-01T00:00:00Z") @RequestParam String start,
-                             @ApiParam(defaultValue = "2000-01-01T04:00:00Z") @RequestParam String end,
+                             @ApiParam(defaultValue = "2000-01-01T00:00:00Z") @RequestParam Instant start,
+                             @ApiParam(defaultValue = "2000-01-01T04:00:00Z") @RequestParam Instant end,
                              @ApiParam(defaultValue = "300") @RequestParam Long step) throws IOException {
     String resolvedMetricsAccountName = CredentialsHelper.resolveAccountByNameOrType(metricsAccountName,
                                                                                      AccountCredentials.Type.METRICS_STORE,
@@ -78,8 +78,8 @@ public String queryMetrics(@RequestParam(required = false) final String metricsA
     AtlasCanaryScope atlasCanaryScope = new AtlasCanaryScope();
     atlasCanaryScope.setType(type);
     atlasCanaryScope.setScope(scope);
-    atlasCanaryScope.setStart(Instant.parse(start));
-    atlasCanaryScope.setEnd(Instant.parse(end));
+    atlasCanaryScope.setStart(start);
+    atlasCanaryScope.setEnd(end);
     atlasCanaryScope.setStep(step);
 
     return synchronousQueryProcessor.processQuery(resolvedMetricsAccountName,

diff --git a/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/orca/AtlasFetchTask.java b/kayenta-atlas/src/main/java/com/netflix/kayenta/atlas/orca/AtlasFetchTask.java
@@ -27,7 +27,6 @@
 import com.netflix.kayenta.storage.ObjectType;
 import com.netflix.kayenta.storage.StorageService;
 import com.netflix.kayenta.storage.StorageServiceRepository;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import com.netflix.spinnaker.orca.ExecutionStatus;
 import com.netflix.spinnaker.orca.RetryableTask;
 import com.netflix.spinnaker.orca.TaskResult;
@@ -46,7 +45,8 @@
 @Slf4j
 public class AtlasFetchTask implements RetryableTask {
 
-  private final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
+  @Autowired
+  ObjectMapper kayentaObjectMapper;
 
   @Autowired
   AccountCredentialsRepository accountCredentialsRepository;
@@ -91,7 +91,7 @@ public TaskResult execute(Stage stage) {
     String scopeJson = (String)stage.getContext().get("atlasCanaryScope");
     AtlasCanaryScope atlasCanaryScope;
     try {
-      atlasCanaryScope = objectMapper.readValue(scopeJson, AtlasCanaryScope.class);
+      atlasCanaryScope = kayentaObjectMapper.readValue(scopeJson, AtlasCanaryScope.class);
     } catch (IOException e) {
       log.error("Unable to parse JSON scope: " + scopeJson, e);
       throw new RuntimeException(e);

diff --git a/kayenta-atlas/src/test/java/com/netflix/kayenta/atlas/IntegrationTest.java b/kayenta-atlas/src/test/java/com/netflix/kayenta/atlas/IntegrationTest.java
@@ -8,7 +8,6 @@
 import com.netflix.kayenta.canary.CanaryConfig;
 import com.netflix.kayenta.canary.CanaryMetricConfig;
 import com.netflix.kayenta.canary.providers.AtlasCanaryMetricSetQueryConfig;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import lombok.AllArgsConstructor;
 import lombok.Builder;
 import lombok.EqualsAndHashCode;
@@ -19,6 +18,7 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.core.io.ResourceLoader;
 import org.springframework.test.context.ContextConfiguration;
@@ -34,10 +34,10 @@
 import java.util.Map;
 import java.util.stream.Collectors;
 
-import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
-
 @Configuration
+@ComponentScan({
+  "com.netflix.kayenta.retrofit.config"
+})
 class TestConfig {}
 
 @Builder
@@ -58,10 +58,12 @@ class CanaryMetricConfigWithResults {
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(classes = {TestConfig.class})
 public class IntegrationTest {
+
     @Autowired
     private ResourceLoader resourceLoader;
 
-    private static final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
+    @Autowired
+    ObjectMapper kayentaObjectMapper;
 
     private String getFileContent(String filename) throws IOException {
         try (InputStream inputStream = resourceLoader.getResource("classpath:" + filename).getInputStream()) {
@@ -71,7 +73,7 @@ private String getFileContent(String filename) throws IOException {
 
     private CanaryConfig getConfig(String filename) throws IOException {
         String contents = getFileContent(filename);
-        return objectMapper.readValue(contents, CanaryConfig.class);
+        return kayentaObjectMapper.readValue(contents, CanaryConfig.class);
     }
 
     private CanaryMetricConfigWithResults queryMetric(CanaryMetricConfig metric, AtlasCanaryScope scope) {

diff --git a/...a-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientConfiguration.java b/...a-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientConfiguration.java
@@ -16,6 +16,9 @@
 
 package com.netflix.kayenta.retrofit.config;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
 import com.netflix.spinnaker.config.OkHttpClientConfiguration;
 import com.netflix.spinnaker.orca.retrofit.exceptions.RetrofitExceptionHandler;
 import com.squareup.okhttp.ConnectionPool;
@@ -24,10 +27,14 @@
 import org.springframework.beans.factory.config.ConfigurableBeanFactory;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Primary;
 import org.springframework.context.annotation.Scope;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
 
+import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
+import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
+
 @Configuration
 public class RetrofitClientConfiguration {
 
@@ -53,4 +60,18 @@ OkHttpClient okHttpClient(OkHttpClientConfiguration okHttpClientConfig) {
   RetrofitExceptionHandler retrofitExceptionHandler() {
     return new RetrofitExceptionHandler();
   }
+
+  @Bean
+  @Primary
+  ObjectMapper kayentaObjectMapper() {
+    ObjectMapper objectMapper = new ObjectMapper()
+      .setSerializationInclusion(NON_NULL)
+      .disable(FAIL_ON_UNKNOWN_PROPERTIES)
+      .configure(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS, false);
+
+    JavaTimeModule module = new JavaTimeModule();
+    objectMapper.registerModule(module);
+
+    return objectMapper;
+  }
 }
diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java b/kayenta-core/src/main/java/com/netflix/kayenta/retrofit/config/RetrofitClientFactory.java
@@ -17,7 +17,6 @@
 package com.netflix.kayenta.retrofit.config;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import com.netflix.spinnaker.retrofit.Slf4jRetrofitLogger;
 import com.squareup.okhttp.OkHttpClient;
 import org.springframework.beans.factory.annotation.Value;
@@ -29,8 +28,6 @@
 import retrofit.converter.Converter;
 import retrofit.converter.JacksonConverter;
 
-import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
 import static retrofit.Endpoints.newFixedEndpoint;
 
 @Component
@@ -40,10 +37,8 @@ public class RetrofitClientFactory {
   String retrofitLogLevel;
 
   @Bean
-  JacksonConverter jacksonConverterWithMapper() {
-    final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
-
-    return new JacksonConverter(objectMapper);
+  JacksonConverter jacksonConverterWithMapper(ObjectMapper kayentaObjectMapper) {
+    return new JacksonConverter(kayentaObjectMapper);
   }
 
   public <T> T createClient(Class<T> type, Converter converter, RemoteService remoteService, OkHttpClient okHttpClient) {

diff --git a/kayenta-core/src/main/java/com/netflix/kayenta/util/ObjectMapperFactory.java b/kayenta-core/src/main/java/com/netflix/kayenta/util/ObjectMapperFactory.java
diff --git a/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/CanaryScopeSpec.groovy b/kayenta-core/src/test/groovy/com/netflix/kayenta/metrics/CanaryScopeSpec.groovy
@@ -17,19 +17,14 @@
 package com.netflix.kayenta.metrics
 
 import com.fasterxml.jackson.databind.ObjectMapper
-import com.fasterxml.jackson.databind.SerializationFeature
-import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule
 import com.netflix.kayenta.canary.CanaryScope
-import com.netflix.kayenta.util.ObjectMapperFactory
+import com.netflix.kayenta.retrofit.config.RetrofitClientConfiguration
 import spock.lang.Shared
 import spock.lang.Specification
 import spock.lang.Unroll
 
 import java.time.Instant
 
-import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES
-
 class CanaryScopeSpec extends Specification {
 
   @Shared
@@ -59,7 +54,7 @@ class CanaryScopeSpec extends Specification {
   @Unroll
   void "should parse json"() {
     when:
-    ObjectMapper objectMapper = ObjectMapperFactory.getMapper()
+    ObjectMapper objectMapper = new RetrofitClientConfiguration().kayentaObjectMapper()
 
     CanaryScope scope = objectMapper.readValue(scope1Json, CanaryScope)
 
@@ -70,7 +65,7 @@ class CanaryScopeSpec extends Specification {
   @Unroll
   void "should render as json and come back again"() {
     when:
-    ObjectMapper objectMapper = ObjectMapperFactory.getMapper()
+    ObjectMapper objectMapper = new RetrofitClientConfiguration().kayentaObjectMapper()
 
     StringWriter jsonStream = new StringWriter()
     objectMapper.writeValue(jsonStream, scope1)

diff --git a/kayenta-gcs/src/main/java/com/netflix/kayenta/gcs/storage/GcsStorageService.java b/kayenta-gcs/src/main/java/com/netflix/kayenta/gcs/storage/GcsStorageService.java
@@ -28,7 +28,6 @@
 import com.netflix.kayenta.security.AccountCredentialsRepository;
 import com.netflix.kayenta.storage.ObjectType;
 import com.netflix.kayenta.storage.StorageService;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import lombok.Builder;
 import lombok.Getter;
 import lombok.Singular;
@@ -49,7 +48,8 @@
 @Slf4j
 public class GcsStorageService implements StorageService {
 
-  private static final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
+  @Autowired
+  ObjectMapper kayentaObjectMapper;
 
   @NotNull
   @Singular
@@ -138,7 +138,7 @@ private <T> T deserialize(Storage storage, StorageObject object, TypeReference t
     getter.executeMediaAndDownloadTo(output);
     String json = output.toString("UTF8");
 
-    return objectMapper.readValue(json, typeReference);
+    return kayentaObjectMapper.readValue(json, typeReference);
   }
 
   @Override
@@ -153,7 +153,7 @@ public <T> void storeObject(String accountName, ObjectType objectType, String ob
     ensureBucketExists(accountName);
 
     try {
-      byte[] bytes = objectMapper.writeValueAsBytes(obj);
+      byte[] bytes = kayentaObjectMapper.writeValueAsBytes(obj);
       StorageObject object = new StorageObject().setBucket(bucketName).setName(path);
       ByteArrayContent content = new ByteArrayContent("application/json", bytes);
 

diff --git a/...figbin/src/main/java/com/netflix/kayenta/configbin/config/ConfigBinResponseConverter.java b/...figbin/src/main/java/com/netflix/kayenta/configbin/config/ConfigBinResponseConverter.java
@@ -16,9 +16,6 @@
 
 package com.netflix.kayenta.configbin.config;
 
-import com.fasterxml.jackson.databind.JsonNode;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.netflix.kayenta.util.ObjectMapperFactory;
 import com.squareup.okhttp.RequestBody;
 import lombok.extern.slf4j.Slf4j;
 import okio.Buffer;
@@ -35,15 +32,10 @@
 import java.lang.reflect.Type;
 import java.nio.charset.StandardCharsets;
 
-import static com.fasterxml.jackson.annotation.JsonInclude.Include.NON_NULL;
-import static com.fasterxml.jackson.databind.DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES;
-
 @Component
 @Slf4j
 public class ConfigBinResponseConverter implements Converter {
 
-  private static final ObjectMapper objectMapper = ObjectMapperFactory.getMapper();
-
   @Override
   public String fromBody(TypedInput body, Type type) throws ConversionException {
     try {