mirror of https://github.com/apache/kafka.git
KAFKA-19213 Client ignores default properties (#20134)
https://issues.apache.org/jira/browse/KAFKA-19213 Fixes a bug where creating a producer/consumer using a `Properties` object created using the `Properties(Properties defaults)` constructor will ignore the default properties. Reviewers: Kirk True <kirk@kirktrue.pro>, TaiJuWu <tjwu1217@gmail.com>, Jhen-Yung Hsu <jhenyunghsu@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>
This commit is contained in:
parent
7738db9b2d
commit
1cef5325ad
|
@ -64,6 +64,7 @@ import java.util.Collection;
|
||||||
import java.util.Collections;
|
import java.util.Collections;
|
||||||
import java.util.Date;
|
import java.util.Date;
|
||||||
import java.util.EnumSet;
|
import java.util.EnumSet;
|
||||||
|
import java.util.Enumeration;
|
||||||
import java.util.HashMap;
|
import java.util.HashMap;
|
||||||
import java.util.HashSet;
|
import java.util.HashSet;
|
||||||
import java.util.Iterator;
|
import java.util.Iterator;
|
||||||
|
@ -1469,7 +1470,24 @@ public final class Utils {
|
||||||
* @return a map including all elements in properties
|
* @return a map including all elements in properties
|
||||||
*/
|
*/
|
||||||
public static Map<String, Object> propsToMap(Properties properties) {
|
public static Map<String, Object> propsToMap(Properties properties) {
|
||||||
return castToStringObjectMap(properties);
|
// This try catch block is to handle the case when the Properties object has non-String keys
|
||||||
|
// when calling the propertyNames() method. This is a workaround for the lack of a method that
|
||||||
|
// returns all properties including defaults and does not attempt to convert all keys to Strings.
|
||||||
|
Enumeration<?> enumeration;
|
||||||
|
try {
|
||||||
|
enumeration = properties.propertyNames();
|
||||||
|
} catch (ClassCastException e) {
|
||||||
|
throw new ConfigException("One or more keys is not a string.");
|
||||||
|
}
|
||||||
|
Map<String, Object> map = new HashMap<>();
|
||||||
|
while (enumeration.hasMoreElements()) {
|
||||||
|
String key = (String) enumeration.nextElement();
|
||||||
|
// properties.get(key) returns null for defaults, but properties.getProperty(key) returns null for
|
||||||
|
// non-string values. A combination of the two methods is used to cover all cases
|
||||||
|
Object value = (properties.get(key) != null) ? properties.get(key) : properties.getProperty(key);
|
||||||
|
map.put(key, value);
|
||||||
|
}
|
||||||
|
return map;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -1479,6 +1497,9 @@ public final class Utils {
|
||||||
* @throws ConfigException if any key is not a String
|
* @throws ConfigException if any key is not a String
|
||||||
*/
|
*/
|
||||||
public static Map<String, Object> castToStringObjectMap(Map<?, ?> inputMap) {
|
public static Map<String, Object> castToStringObjectMap(Map<?, ?> inputMap) {
|
||||||
|
if (inputMap instanceof Properties) {
|
||||||
|
return propsToMap((Properties) inputMap);
|
||||||
|
}
|
||||||
Map<String, Object> map = new HashMap<>(inputMap.size());
|
Map<String, Object> map = new HashMap<>(inputMap.size());
|
||||||
for (Map.Entry<?, ?> entry : inputMap.entrySet()) {
|
for (Map.Entry<?, ?> entry : inputMap.entrySet()) {
|
||||||
if (entry.getKey() instanceof String) {
|
if (entry.getKey() instanceof String) {
|
||||||
|
|
|
@ -579,7 +579,7 @@ public class KafkaProducerTest {
|
||||||
ConfigException ce = assertThrows(
|
ConfigException ce = assertThrows(
|
||||||
ConfigException.class,
|
ConfigException.class,
|
||||||
() -> new KafkaProducer<>(props, new StringSerializer(), new StringSerializer()));
|
() -> new KafkaProducer<>(props, new StringSerializer(), new StringSerializer()));
|
||||||
assertTrue(ce.getMessage().contains("not string key"), "Unexpected exception message: " + ce.getMessage());
|
assertTrue(ce.getMessage().contains("One or more keys is not a string."), "Unexpected exception message: " + ce.getMessage());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|
|
@ -896,12 +896,171 @@ public class UtilsTest {
|
||||||
assertValue(Collections.emptyMap());
|
assertValue(Collections.emptyMap());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPropsToMapNonStringKey() {
|
||||||
|
ConfigException ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.put(1, "value");
|
||||||
|
Utils.propsToMap(props);
|
||||||
|
});
|
||||||
|
assertTrue(ce.getMessage().contains("One or more keys is not a string."));
|
||||||
|
|
||||||
|
ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.put(true, "value");
|
||||||
|
props.put('a', "value");
|
||||||
|
Utils.propsToMap(props);
|
||||||
|
});
|
||||||
|
assertEquals("One or more keys is not a string.", ce.getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPropsToMapWithDefaults() {
|
||||||
|
Properties defaultProperties = new Properties();
|
||||||
|
defaultProperties.setProperty("DefaultKey1", "DefaultValue1");
|
||||||
|
defaultProperties.setProperty("DefaultKey2", "DefaultValue2");
|
||||||
|
|
||||||
|
Properties actualProperties = new Properties(defaultProperties);
|
||||||
|
actualProperties.setProperty("ActualKey1", "ActualValue1");
|
||||||
|
actualProperties.setProperty("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
final Map<String, Object> mapProperties = Utils.propsToMap(actualProperties);
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("DefaultKey1", "DefaultValue1");
|
||||||
|
expectedMap.put("DefaultKey2", "DefaultValue2");
|
||||||
|
expectedMap.put("ActualKey1", "ActualValue1");
|
||||||
|
expectedMap.put("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
assertEquals(expectedMap, mapProperties);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testPropsToMapWithDefaultsAndSameKey() {
|
||||||
|
Properties defaultProperties = new Properties();
|
||||||
|
defaultProperties.setProperty("DefaultKey1", "DefaultValue1");
|
||||||
|
defaultProperties.setProperty("DefaultKey2", "DefaultValue2");
|
||||||
|
|
||||||
|
Properties actualProperties = new Properties(defaultProperties);
|
||||||
|
actualProperties.setProperty("DefaultKey1", "ActualValue1");
|
||||||
|
actualProperties.setProperty("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
final Map<String, Object> mapProperties = Utils.propsToMap(actualProperties);
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("DefaultKey1", "ActualValue1");
|
||||||
|
expectedMap.put("DefaultKey2", "DefaultValue2");
|
||||||
|
expectedMap.put("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
assertEquals(expectedMap, mapProperties);
|
||||||
|
}
|
||||||
|
|
||||||
private static void assertValue(Object value) {
|
private static void assertValue(Object value) {
|
||||||
Properties props = new Properties();
|
Properties props = new Properties();
|
||||||
props.put("key", value);
|
props.put("key", value);
|
||||||
assertEquals(Utils.propsToMap(props).get("key"), value);
|
assertEquals(Utils.propsToMap(props).get("key"), value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMap() {
|
||||||
|
Map<Object, Object> map = new HashMap<>();
|
||||||
|
map.put("key1", "value1");
|
||||||
|
map.put("key2", 1);
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("key1", "value1");
|
||||||
|
expectedMap.put("key2", 1);
|
||||||
|
|
||||||
|
assertEquals(map, expectedMap);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMapNonStringKey() {
|
||||||
|
ConfigException ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Map<Object, Object> map = new HashMap<>();
|
||||||
|
map.put(1, "value");
|
||||||
|
Utils.castToStringObjectMap(map);
|
||||||
|
});
|
||||||
|
assertTrue(ce.getMessage().contains("Key must be a string."));
|
||||||
|
|
||||||
|
ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Map<Object, Object> map = new HashMap<>();
|
||||||
|
map.put(true, "value");
|
||||||
|
map.put('a', "value");
|
||||||
|
Utils.castToStringObjectMap(map);
|
||||||
|
});
|
||||||
|
assertTrue(ce.getMessage().contains("Key must be a string."));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMapPropertiesAsInput() {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.put("key1", "value1");
|
||||||
|
props.put("key2", "value2");
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("key1", "value1");
|
||||||
|
expectedMap.put("key2", "value2");
|
||||||
|
|
||||||
|
assertEquals(expectedMap, Utils.castToStringObjectMap(props));
|
||||||
|
assertEquals(Utils.propsToMap(props), Utils.castToStringObjectMap(props));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMapPropertiesNonStringKey() {
|
||||||
|
ConfigException ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.put(1, "value");
|
||||||
|
Utils.castToStringObjectMap(props);
|
||||||
|
});
|
||||||
|
assertEquals("One or more keys is not a string.", ce.getMessage());
|
||||||
|
|
||||||
|
ce = assertThrows(ConfigException.class, () -> {
|
||||||
|
Properties props = new Properties();
|
||||||
|
props.put(true, "value");
|
||||||
|
props.put('a', "value");
|
||||||
|
Utils.castToStringObjectMap(props);
|
||||||
|
});
|
||||||
|
assertEquals("One or more keys is not a string.", ce.getMessage());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMapPropertiesWithDefaults() {
|
||||||
|
Properties defaultProperties = new Properties();
|
||||||
|
defaultProperties.setProperty("DefaultKey1", "DefaultValue1");
|
||||||
|
defaultProperties.setProperty("DefaultKey2", "DefaultValue2");
|
||||||
|
|
||||||
|
Properties actualProperties = new Properties(defaultProperties);
|
||||||
|
actualProperties.setProperty("ActualKey1", "ActualValue1");
|
||||||
|
actualProperties.setProperty("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("DefaultKey1", "DefaultValue1");
|
||||||
|
expectedMap.put("DefaultKey2", "DefaultValue2");
|
||||||
|
expectedMap.put("ActualKey1", "ActualValue1");
|
||||||
|
expectedMap.put("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
assertEquals(expectedMap, Utils.castToStringObjectMap(actualProperties));
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void testCastToStringObjectMapPropertiesWithDefaultsAndSameKey() {
|
||||||
|
Properties defaultProperties = new Properties();
|
||||||
|
defaultProperties.setProperty("DefaultKey1", "DefaultValue1");
|
||||||
|
defaultProperties.setProperty("DefaultKey2", "DefaultValue2");
|
||||||
|
|
||||||
|
Properties actualProperties = new Properties(defaultProperties);
|
||||||
|
actualProperties.setProperty("DefaultKey1", "ActualValue1");
|
||||||
|
actualProperties.setProperty("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
Map<String, Object> expectedMap = new HashMap<>();
|
||||||
|
expectedMap.put("DefaultKey1", "ActualValue1");
|
||||||
|
expectedMap.put("DefaultKey2", "DefaultValue2");
|
||||||
|
expectedMap.put("ActualKey2", "ActualValue2");
|
||||||
|
|
||||||
|
assertEquals(expectedMap, Utils.castToStringObjectMap(actualProperties));
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
public void testCloseAllQuietly() {
|
public void testCloseAllQuietly() {
|
||||||
AtomicReference<Throwable> exception = new AtomicReference<>();
|
AtomicReference<Throwable> exception = new AtomicReference<>();
|
||||||
|
|
Loading…
Reference in New Issue