This is an automated email from the ASF dual-hosted git repository.
cwylie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 8718155 Allow for empty keys in hash map (#10869)
8718155 is described below
commit 8718155f8ffa8c9d34877cbdb7f8eb86df383e29
Author: Abhishek Agarwal <[email protected]>
AuthorDate: Thu Feb 11 00:49:57 2021 +0530
Allow for empty keys in hash map (#10869)
* allow for empty keys in hash map
* fix serde test
---
.../druid/guice/GuiceAnnotationIntrospector.java | 19 ++-
.../druid/guice/DruidSecondaryModuleTest.java | 157 ++++++++++++++++++++-
.../extraction/MapLookupExtractionFnSerDeTest.java | 13 +-
3 files changed, 180 insertions(+), 9 deletions(-)
diff --git
a/core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
b/core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
index 29fac1a..ce49253 100644
--- a/core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
+++ b/core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
@@ -23,6 +23,7 @@ import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.introspect.Annotated;
+import com.fasterxml.jackson.databind.introspect.AnnotatedClass;
import com.fasterxml.jackson.databind.introspect.AnnotatedMember;
import com.fasterxml.jackson.databind.introspect.AnnotatedMethod;
import com.fasterxml.jackson.databind.introspect.AnnotatedParameter;
@@ -32,6 +33,7 @@ import com.google.inject.Key;
import org.apache.druid.java.util.common.IAE;
import java.lang.annotation.Annotation;
+import java.util.Map;
/**
*/
@@ -101,6 +103,7 @@ public class GuiceAnnotationIntrospector extends
NopAnnotationIntrospector
// delegate creators. I'm not 100% sure why it's not called, but guess
it's because the argument
// is some Java type that Jackson already knows how to deserialize. Since
there is only one argument,
// Jackson perhaps is able to just deserialize it without introspection.
+
if (ac instanceof AnnotatedParameter) {
final AnnotatedParameter ap = (AnnotatedParameter) ac;
if (ap.hasAnnotation(JsonProperty.class)) {
@@ -108,6 +111,20 @@ public class GuiceAnnotationIntrospector extends
NopAnnotationIntrospector
}
}
- return JsonIgnoreProperties.Value.forIgnoredProperties("");
+ // A map can have empty string keys e.g.
https://github.com/apache/druid/issues/10859. By returning empty ignored
+ // list for map, we can allow for empty string keys in a map. A nested
class within map
+ // can still be annotated with JacksonInject and still be
non-deserializable from user input
+ // Refer to {@link
com.fasterxml.jackson.databind.deser.BasicDeserializerFactory.createMapDeserializer}
for details
+ // on how the ignored list is passed to map deserializer
+ if (ac instanceof AnnotatedClass) {
+ final AnnotatedClass aClass = (AnnotatedClass) ac;
+ if (Map.class.isAssignableFrom(aClass.getAnnotated())) {
+ return JsonIgnoreProperties.Value.empty();
+ }
+ }
+
+ // We will allow serialization on empty properties. Properties marked with
{@code @JacksonInject} are still
+ // not serialized if there is no getter marked with {@code @JsonProperty}
+ return
JsonIgnoreProperties.Value.forIgnoredProperties("").withAllowGetters();
}
}
diff --git
a/core/src/test/java/org/apache/druid/guice/DruidSecondaryModuleTest.java
b/core/src/test/java/org/apache/druid/guice/DruidSecondaryModuleTest.java
index 12ab2dd..ddda5a5 100644
--- a/core/src/test/java/org/apache/druid/guice/DruidSecondaryModuleTest.java
+++ b/core/src/test/java/org/apache/druid/guice/DruidSecondaryModuleTest.java
@@ -26,6 +26,7 @@ import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.inject.Guice;
import com.google.inject.Injector;
import com.google.inject.Module;
@@ -38,6 +39,8 @@ import javax.annotation.Nullable;
import javax.validation.Validation;
import javax.validation.Validator;
import java.util.List;
+import java.util.Map;
+import java.util.Objects;
import java.util.Properties;
@RunWith(Enclosed.class)
@@ -90,6 +93,89 @@ public class DruidSecondaryModuleTest
Assert.assertEquals(PROPERTY_VALUE, object.injected.val);
}
+ @Test
+ public void testInjectNormalWithEmptyKeysInMap() throws
JsonProcessingException
+ {
+ final Properties props = new Properties();
+ props.setProperty(PROPERTY_NAME, PROPERTY_VALUE);
+ final Injector injector = makeInjectorWithProperties(props);
+ final ObjectMapper mapper = makeObjectMapper(injector);
+ final String json = "{\n"
+ + " \"map1\" : {\n"
+ + " \"foo\" : \"bar\",\n"
+ + " \"\" : \"empty\"\n"
+ + " },\n"
+ + " \"map2\" : {\n"
+ + " \"foo\" : {\n"
+ + " \"test\" : \"value1\"\n"
+ + " },\n"
+ + " \"\" : {\n"
+ + " \"test\" : \"value2\"\n"
+ + " }\n"
+ + " }\n"
+ + "}";
+ final ClassWithMapAndJacksonInject object = new
ClassWithMapAndJacksonInject(
+ ImmutableMap.of("foo", "bar", "", "empty"),
+ ImmutableMap.of("foo", new ClassWithJacksonInject("value1",
injector.getInstance(InjectedParameter.class)),
+ "", new ClassWithJacksonInject("value2",
injector.getInstance(InjectedParameter.class)))
+ );
+ final String jsonWritten =
mapper.writerWithDefaultPrettyPrinter().writeValueAsString(object);
+ Assert.assertEquals(json, jsonWritten);
+ final ClassWithMapAndJacksonInject objectRead = mapper.readValue(json,
ClassWithMapAndJacksonInject.class);
+ Assert.assertEquals(object, objectRead);
+ Assert.assertEquals("empty", objectRead.getStringStringMap().get(""));
+ }
+
+ @Test
+ public void testInjectNormalWithEmptyKeysAndInjectClass() throws
JsonProcessingException
+ {
+ final Properties props = new Properties();
+ props.setProperty(PROPERTY_NAME, PROPERTY_VALUE);
+ final Injector injector = makeInjectorWithProperties(props);
+ final ObjectMapper mapper = makeObjectMapper(injector);
+ final String json = "{\n"
+ + " \"map1\" : {\n"
+ + " \"foo\" : \"bar\",\n"
+ + " \"\" : \"empty\"\n"
+ + " },\n"
+ + " \"map2\" : {\n"
+ + " \"foo\" : {\n"
+ + " \"test\" : \"value1\",\n"
+ + " \"\" : \"nice try\"\n"
+ + " },\n"
+ + " \"\" : {\n"
+ + " \"test\" : \"value2\",\n"
+ + " \"\" : \"nice try\"\n"
+ + " }\n"
+ + " }\n"
+ + "}";
+ final String expectedSerializedJson
+ = "{\n"
+ + " \"map1\" : {\n"
+ + " \"foo\" : \"bar\",\n"
+ + " \"\" : \"empty\"\n"
+ + " },\n"
+ + " \"map2\" : {\n"
+ + " \"foo\" : {\n"
+ + " \"test\" : \"value1\"\n"
+ + " },\n"
+ + " \"\" : {\n"
+ + " \"test\" : \"value2\"\n"
+ + " }\n"
+ + " }\n"
+ + "}";
+ final ClassWithMapAndJacksonInject object = new
ClassWithMapAndJacksonInject(
+ ImmutableMap.of("foo", "bar", "", "empty"),
+ ImmutableMap.of("foo", new ClassWithJacksonInject("value1",
injector.getInstance(InjectedParameter.class)),
+ "", new ClassWithJacksonInject("value2",
injector.getInstance(InjectedParameter.class)))
+ );
+ final String jsonWritten =
mapper.writerWithDefaultPrettyPrinter().writeValueAsString(object);
+ Assert.assertEquals(expectedSerializedJson, jsonWritten);
+ final ClassWithMapAndJacksonInject objectRead = mapper.readValue(json,
ClassWithMapAndJacksonInject.class);
+ Assert.assertEquals(object, objectRead);
+ Assert.assertEquals("empty", objectRead.getStringStringMap().get(""));
+ }
+
private static class ClassWithJacksonInject
{
private final String test;
@@ -106,11 +192,30 @@ public class DruidSecondaryModuleTest
this.injected = injected;
}
- @JsonProperty
+ @JsonProperty("test")
public String getTest()
{
return test;
}
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ ClassWithJacksonInject that = (ClassWithJacksonInject) o;
+ return Objects.equals(test, that.test) && Objects.equals(injected.val,
that.injected.val);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(test, injected.val);
+ }
}
private static class ClassWithEmptyProperty
@@ -135,6 +240,56 @@ public class DruidSecondaryModuleTest
return test;
}
}
+
+ private static class ClassWithMapAndJacksonInject
+ {
+ private final Map<String, String> stringStringMap;
+ private final Map<String, ClassWithJacksonInject> stringJacksonInjectMap;
+
+ @JsonCreator
+ public ClassWithMapAndJacksonInject(
+ @JsonProperty("map1") Map<String, String> stringStringMap,
+ @JsonProperty("map2") Map<String, ClassWithJacksonInject>
stringJacksonInjectMap
+ )
+ {
+ this.stringStringMap = stringStringMap;
+ this.stringJacksonInjectMap = stringJacksonInjectMap;
+ }
+
+ @JsonProperty("map1")
+ public Map<String, String> getStringStringMap()
+ {
+ return stringStringMap;
+ }
+
+ @JsonProperty("map2")
+ public Map<String, ClassWithJacksonInject> getStringJacksonInjectMap()
+ {
+ return stringJacksonInjectMap;
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ ClassWithMapAndJacksonInject that = (ClassWithMapAndJacksonInject) o;
+ return Objects.equals(stringStringMap, that.stringStringMap) &&
Objects.equals(
+ stringJacksonInjectMap,
+ that.stringJacksonInjectMap
+ );
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(stringStringMap, stringJacksonInjectMap);
+ }
+ }
}
public static class ConstructorWithoutJacksonInjectTest
diff --git
a/processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
b/processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
index 92e5ef7..39144ff 100644
---
a/processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
+++
b/processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
@@ -21,11 +21,9 @@ package org.apache.druid.query.extraction;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableMap;
-import com.google.inject.Injector;
-import com.google.inject.Key;
-import org.apache.druid.guice.GuiceInjectors;
-import org.apache.druid.guice.annotations.Json;
+import org.apache.druid.common.config.NullHandling;
import org.apache.druid.java.util.common.StringUtils;
+import org.apache.druid.segment.TestHelper;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -42,19 +40,20 @@ public class MapLookupExtractionFnSerDeTest
private static ObjectMapper mapper;
private static final Map<String, String> RENAMES = ImmutableMap.of(
"foo", "bar",
- "bar", "baz"
+ "bar", "baz",
+ "", "empty"
);
@BeforeClass
public static void setup()
{
- Injector defaultInjector = GuiceInjectors.makeStartupInjector();
- mapper = defaultInjector.getInstance(Key.get(ObjectMapper.class,
Json.class));
+ mapper = TestHelper.makeJsonMapper();
}
@Test
public void testDeserialization() throws IOException
{
+ NullHandling.initializeForTests();
final DimExtractionFn fn =
mapper.readerFor(DimExtractionFn.class).readValue(
StringUtils.format(
"{\"type\":\"lookup\",\"lookup\":{\"type\":\"map\", \"map\":%s}}",
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
