cerner/common-kafka

Make package-private ProcessingKafkaConsumer constructor public

Closed this issue · 4 comments

Is there a particular reason why the ProcessingKafkaConsumer constructor that takes a Consumer argument is package-private instead of public?

I have a custom Deserializer for deserializing JSON directly back into a corresponding POJO using Jackson that I would like to use with the ProcessingKafkaConsumer class.

The constructor for my deserializer takes in an ObjectMapper and a Class<T> so that it knows which POJO to deserialize into (in reality it's a large polymorphic hierarchy of event classes, but using the root class allow Jackson to deserialize them all).

public KafkaJacksonJsonDeserializer(ObjectMapper objectMapper, Class<T> clazz)

Since I need to pass the class information to the constructor I cannot use the reflection based properties for the deserializer class name. The Kafka Consumer class itself provides a constructor that allows passing in direct instances of the deserializers, but there isn't a corresponding constructor on ProcessingKafkaConsumer.

If the package-private constructor on ProcessingKafkaConsumer were public I could create the Consumer instance as I need it and then pass it to be wrapped by ProcessingKafkaConsumer.

Alternatively, if there were another public constructor that accepted the deserializer instances directly, similar to the constructor on Consumer, I could use that.

The constructor likely exists to help with testing. We likely didn't open it up because theres some assumptions we making about the ProcessingConfig and the Consumer's configuration. The only concern I have would be ensuring that the ProcessingConfig and the configuration the Consumer is using match. The ProcessingKafkaConsumer uses the ProcessingConfig to know how to behave given how the Consumer is setup. Additionally anyone using it would need to ensure that auto committing was disabled

Would the alternative approach of adding a second public constructor that takes in the two serializer instances directly be preferable?

That seems fine to me

I agree, the alternative approach sounds good