From fe42db98ac5b6638329e9c0c0a45ac37979246da Mon Sep 17 00:00:00 2001
From: Gian Merlino <gianmerlino@gmail.com>
Date: Thu, 25 May 2017 09:41:04 +0900
Subject: [PATCH] URIExtractionNamespace: Avoid problems due to
 canonicalization of lookup fields. (#4307)

Disables canonicalization for simpleJson, where expect field names to be unique
anyway. Keeps canonicalization enabled for customJson, but avoids sharing the
table with the global ObjectMapper.
---
 .../namespace/URIExtractionNamespace.java      | 18 ++++++++++++------
 .../NamespaceLookupExtractorFactoryTest.java   |  7 +++++--
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/extensions-core/lookups-cached-global/src/main/java/io/druid/query/lookup/namespace/URIExtractionNamespace.java b/extensions-core/lookups-cached-global/src/main/java/io/druid/query/lookup/namespace/URIExtractionNamespace.java
index 1c6bfd0447e..0b80c364592 100644
--- a/extensions-core/lookups-cached-global/src/main/java/io/druid/query/lookup/namespace/URIExtractionNamespace.java
+++ b/extensions-core/lookups-cached-global/src/main/java/io/druid/query/lookup/namespace/URIExtractionNamespace.java
@@ -25,6 +25,7 @@ import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.annotation.JsonSubTypes;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 import com.fasterxml.jackson.annotation.JsonTypeName;
+import com.fasterxml.jackson.core.JsonFactory;
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.annotations.VisibleForTesting;
@@ -530,8 +531,10 @@ public class URIExtractionNamespace implements ExtractionNamespace
       Preconditions.checkArgument(!Strings.isNullOrEmpty(valueFieldName), "[valueFieldName] cannot be empty");
       this.keyFieldName = keyFieldName;
       this.valueFieldName = valueFieldName;
+
+      // Copy jsonMapper; don't want to share canonicalization tables, etc., with the global ObjectMapper.
       this.parser = new DelegateParser(
-          new JSONParser(jsonMapper, ImmutableList.of(keyFieldName, valueFieldName)),
+          new JSONParser(jsonMapper.copy(), ImmutableList.of(keyFieldName, valueFieldName)),
           keyFieldName,
           valueFieldName
       );
@@ -588,6 +591,9 @@ public class URIExtractionNamespace implements ExtractionNamespace
   @JsonTypeName("simpleJson")
   public static class ObjectMapperFlatDataParser implements FlatDataParser
   {
+    private static final TypeReference<Map<String, String>> MAP_STRING_STRING = new TypeReference<Map<String, String>>()
+    {
+    };
 
     private final Parser<String, String> parser;
 
@@ -596,17 +602,17 @@ public class URIExtractionNamespace implements ExtractionNamespace
         final @JacksonInject @Json ObjectMapper jsonMapper
     )
     {
+      // There's no point canonicalizing field names, we expect them to all be unique.
+      final JsonFactory jsonFactory = jsonMapper.getFactory().copy();
+      jsonFactory.configure(JsonFactory.Feature.CANONICALIZE_FIELD_NAMES, false);
+
       parser = new Parser<String, String>()
       {
         @Override
         public Map<String, String> parse(String input)
         {
           try {
-            return jsonMapper.readValue(
-                input, new TypeReference<Map<String, String>>()
-                {
-                }
-            );
+            return jsonFactory.createParser(input).readValueAs(MAP_STRING_STRING);
           }
           catch (IOException e) {
             throw Throwables.propagate(e);
diff --git a/extensions-core/lookups-cached-global/src/test/java/io/druid/query/lookup/NamespaceLookupExtractorFactoryTest.java b/extensions-core/lookups-cached-global/src/test/java/io/druid/query/lookup/NamespaceLookupExtractorFactoryTest.java
index 3858e488d42..cea63afab76 100644
--- a/extensions-core/lookups-cached-global/src/test/java/io/druid/query/lookup/NamespaceLookupExtractorFactoryTest.java
+++ b/extensions-core/lookups-cached-global/src/test/java/io/druid/query/lookup/NamespaceLookupExtractorFactoryTest.java
@@ -94,10 +94,13 @@ public class NamespaceLookupExtractorFactoryTest
               Object valueId, DeserializationContext ctxt, BeanProperty forProperty, Object beanInstance
           )
           {
-            if ("io.druid.server.lookup.namespace.cache.CacheScheduler".equals(valueId)) {
+            if (CacheScheduler.class.getName().equals(valueId)) {
               return scheduler;
+            } else if (ObjectMapper.class.getName().equals(valueId)) {
+              return mapper;
+            } else {
+              return null;
             }
-            return null;
           }
         }
     );