kirktrue commented on code in PR #20134:
URL: https://github.com/apache/kafka/pull/20134#discussion_r2206026360
##########
clients/src/test/java/org/apache/kafka/clients/producer/KafkaProducerTest.java:
##########
@@ -579,7 +579,7 @@ public void testConstructorWithNotStringKey() {
ConfigException ce = assertThrows(
ConfigException.class,
() -> new KafkaProducer<>(props, new StringSerializer(), new
StringSerializer()));
- assertTrue(ce.getMessage().contains("not string key"), "Unexpected
exception message: " + ce.getMessage());
+ assertTrue(ce.getMessage().contains("not a string"), "Unexpected
exception message: " + ce.getMessage());
Review Comment:
You could match on the complete error message (`"One or more keys is not a
string."`), right?
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -1469,7 +1470,25 @@ public static <K, V> Map<K, V> filterMap(final Map<K, V>
map, final Predicate<En
* @return a map including all elements in 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> props = 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);
+ System.out.printf("%s %s%n", key, value);
Review Comment:
Oops. Please remove debug 😄
##########
clients/src/main/java/org/apache/kafka/common/utils/Utils.java:
##########
@@ -1469,7 +1470,25 @@ public static <K, V> Map<K, V> filterMap(final Map<K, V>
map, final Predicate<En
* @return a map including all elements in 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> props = new HashMap<>();
Review Comment:
Pretty nit-picky, but I would name the map something other than `props`.
`castToStringObjectMap()` uses `map`, FWIW.
--
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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
