abhishekagarwal87 commented on a change in pull request #10869:
URL: https://github.com/apache/druid/pull/10869#discussion_r573529209
##########
File path:
core/src/main/java/org/apache/druid/guice/GuiceAnnotationIntrospector.java
##########
@@ -101,13 +103,28 @@ public Object findInjectableValueId(AnnotatedMember m)
// 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)) {
return JsonIgnoreProperties.Value.empty();
}
}
- 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();
Review comment:
This extra step is required because for serialization, Jackson tries to
find `propertyIgnorals` on the getter method. Here is that annotated method
from debugging a test
`[method
org.apache.druid.guice.DruidSecondaryModuleTest$ConstructorWithJacksonInjectTest$ClassWithMapAndJacksonInject#getStringStringMap(0
params)]`
Our special check for `Map` won't cover this method and that will result in
empty properties ignored during serialization.
We can either check if `ac` passed in `findPropertyIgnorals` is
`AnnotatedMethod` and return type is `Map` and then return `Empty` or we can
just allow serialization for empty properties. Latter works too and is less
likely to cause bugs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
