Skip to content

Commit

Permalink
Fix broken KafkaEmitterConfig parsing (#5201)
Browse files Browse the repository at this point in the history
* Fix broken KafkaEmitterConfig parsing

This was a regression introduced in
#4722

KafkaEmitterConfig property names have dot(.) in the name of properties
and JsonConfigurator behavior was changed to not support that.
Added a test and fixed parsing of properties that have dot(.) in
property names

* Fix test failure
  • Loading branch information
nishantmonu51 authored and jon-wei committed Jan 3, 2018
1 parent 0f773af commit 59af4d3
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 6 deletions.
6 changes: 4 additions & 2 deletions api/src/main/java/io/druid/guice/JsonConfigurator.java
Expand Up @@ -93,7 +93,6 @@ public <T> T configurate(Properties props, String propertyPrefix, Class<T> clazz
log.info(e, "Unable to parse [%s]=[%s] as a json object, using as is.", prop, propValue);
value = propValue;
}

hieraricalPutValue(propertyPrefix, prop, prop.substring(propertyBase.length()), value, jsonMap);
}
}
Expand Down Expand Up @@ -175,8 +174,11 @@ private static void hieraricalPutValue(
)
{
int dotIndex = property.indexOf('.');
// Always put property with name even if it is of form a.b. This will make sure the property is available for classes
// where JsonProperty names are of the form a.b
// Note:- this will cause more than required properties to be present in the jsonMap.
targetMap.put(property, value);
if (dotIndex < 0) {
targetMap.put(property, value);
return;
}
if (dotIndex == 0) {
Expand Down
34 changes: 30 additions & 4 deletions api/src/test/java/io/druid/guice/JsonConfiguratorTest.java
Expand Up @@ -94,10 +94,13 @@ public ExecutableValidator forExecutables()
public void testTest()
{
Assert.assertEquals(
new MappableObject("p1", ImmutableList.<String>of("p2")),
new MappableObject("p1", ImmutableList.<String>of("p2"))
new MappableObject("p1", ImmutableList.<String>of("p2"), "p2"),
new MappableObject("p1", ImmutableList.<String>of("p2"), "p2")
);
Assert.assertEquals(
new MappableObject("p1", null, null),
new MappableObject("p1", ImmutableList.<String>of(), null)
);
Assert.assertEquals(new MappableObject("p1", null), new MappableObject("p1", ImmutableList.<String>of()));
}

@Test
Expand Down Expand Up @@ -140,6 +143,19 @@ public void testQuotedConfig()
Assert.assertEquals("testing \"prop1\"", obj.prop1);
Assert.assertEquals(ImmutableList.of(), obj.prop1List);
}

@Test
public void testPropertyWithDot()
{
final JsonConfigurator configurator = new JsonConfigurator(mapper, validator);
properties.setProperty(PROP_PREFIX + "prop2.prop.2", "testing");
properties.setProperty(PROP_PREFIX + "prop1", "prop1");
final MappableObject obj = configurator.configurate(properties, PROP_PREFIX, MappableObject.class);
Assert.assertEquals("testing", obj.prop2);
Assert.assertEquals(ImmutableList.of(), obj.prop1List);
Assert.assertEquals("prop1", obj.prop1);

}
}

class MappableObject
Expand All @@ -148,15 +164,19 @@ class MappableObject
final String prop1;
@JsonProperty("prop1List")
final List<String> prop1List;
@JsonProperty("prop2.prop.2")
final String prop2;

@JsonCreator
protected MappableObject(
@JsonProperty("prop1") final String prop1,
@JsonProperty("prop1List") final List<String> prop1List
@JsonProperty("prop1List") final List<String> prop1List,
@JsonProperty("prop2.prop.2") final String prop2
)
{
this.prop1 = prop1;
this.prop1List = prop1List == null ? ImmutableList.<String>of() : prop1List;
this.prop2 = prop2;
}


Expand All @@ -172,6 +192,12 @@ public String getProp1()
return prop1;
}

@JsonProperty
public String getProp2()
{
return prop2;
}

@Override
public boolean equals(Object o)
{
Expand Down
Expand Up @@ -31,6 +31,7 @@
import io.druid.guice.LazySingleton;
import io.druid.guice.LifecycleModule;
import io.druid.guice.ServerModule;
import io.druid.jackson.JacksonModule;
import org.junit.Assert;
import org.junit.Test;

Expand Down Expand Up @@ -67,6 +68,7 @@ private Injector makeInjectorWithProperties(final Properties props)
new DruidGuiceExtensions(),
new LifecycleModule(),
new ServerModule(),
new JacksonModule(),
new Module()
{
@Override
Expand Down

0 comments on commit 59af4d3

Please sign in to comment.