Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix broken KafkaEmitterConfig parsing #5201

Merged
merged 2 commits into from Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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