Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Allow for empty keys in hash map (#10869) 

* allow for empty keys in hash map

* fix serde test
This commit is contained in:
Abhishek Agarwal 2021-02-11 00:49:57 +05:30 committed by GitHub
parent 1ec3f0bd73
commit 8718155f8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 180 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
View File

@ -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();
}
}

157
core/src/test/java/org/apache/druid/guice/DruidSecondaryModuleTest.java
View File

@ -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

13
processing/src/test/java/org/apache/druid/query/extraction/MapLookupExtractionFnSerDeTest.java
View File

@ -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}}",