Chris Egerton created KAFKA-8299:
------------------------------------
Summary: Add type-safe instantiation of generic classes to
AbstractConfig
Key: KAFKA-8299
URL: https://issues.apache.org/jira/browse/KAFKA-8299
Project: Kafka
Issue Type: Improvement
Components: config
Reporter: Chris Egerton
Assignee: Chris Egerton
{{AbstractConfig.getConfiguredInstance(String key, Class<T> klass)}} and other
similar methods isn't type-safe for generic types. For example, the following
code compiles but generates a runtime exception when the created {{Consumer}}
is invoked:
{code:java}
public class KafkaIssueSnippet {
public static class PrintInt implements Consumer<Integer> {
@Override
public void accept(Integer i) {
System.out.println(i);
}
}
public static void main(String[] args) {
final String stringConsumerProp = "string.consumer.class";
AbstractConfig config = new AbstractConfig(
new ConfigDef().define(
stringConsumerProp,
ConfigDef.Type.CLASS,
ConfigDef.Importance.HIGH,
"A class that implements Consumer<String>"
),
Collections.singletonMap(
stringConsumerProp,
PrintInt.class.getName()
)
);
Consumer<String> stringConsumer = config.getConfiguredInstance(
stringConsumerProp,
Consumer.class
);
stringConsumer.accept("Oops! ClassCastException");
}
}{code}
The compiler (rightfully so) generates a warning about the unchecked cast from
{{Consumer}} to {{Consumer<String>}} to indicate that exactly this sort of
thing may happen, but it would be nice if we didn't have to worry about this in
the first place and instead had the same guarantees for generic types that we
do for non-generic types: that either the {{getConfiguredInstance(...)}} method
returns an object to us that we know for sure is an instance of the requested
type, or an exception is thrown.
Apache Commons contains a useful reflection library that could possibly be used
to bridge this gap; specifically, its
[TypeUtils|https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/reflect/TypeUtils.html]
and
[TypeLiteral|https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/reflect/TypeLiteral.html]
classes could be used to add new {{getConfiguredInstance}} and
{{getConfiguredInstances}} methods to the {{AbstractConfig}} class that accept
instances of {{TypeLiteral}} instead of {{Class}} and then perform type
checking to ensure that the requested class actually implements/extends from
the requested type.
Since this affects public API it's possible a KIP will be required, but the
changes are pretty lightweight (four new methods that heavily resemble existing
ones). If a contributor or committer, especially one familiar with this section
of the codebase, has an opinion on the necessity of a KIP their input would be
appreciated.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
