lianetm commented on code in PR #19467:
URL: https://github.com/apache/kafka/pull/19467#discussion_r2045298346
##########
clients/src/main/java/org/apache/kafka/common/serialization/Deserializer.java:
##########
@@ -25,39 +25,61 @@
/**
* An interface for converting bytes to objects.
- *
* A class that implements this interface is expected to have a constructor
with no parameters.
- * <p>
- * Implement {@link org.apache.kafka.common.ClusterResourceListener} to
receive cluster metadata once it's available. Please see the class
documentation for ClusterResourceListener for more information.
- * Implement {@link org.apache.kafka.common.metrics.Monitorable} to enable the
deserializer to register metrics. The following tags are automatically added to
- * all metrics registered: <code>config</code> set to either
<code>key.deserializer</code> or <code>value.deserializer</code>, and
<code>class</code> set to the Deserializer class name.
+ *
+ * <p>This interface can be combined with {@link
org.apache.kafka.common.ClusterResourceListener ClusterResourceListener}
+ * to receive cluster metadata once it's available, as well as {@link
org.apache.kafka.common.metrics.Monitorable Monitorable}
+ * to enable the deserializer to register metrics. For the later case, the
following tags are automatically added to
Review Comment:
latter (and we could just remove 'case')
##########
clients/src/main/java/org/apache/kafka/common/serialization/Serializer.java:
##########
@@ -45,28 +50,43 @@ default void configure(Map<String, ?> configs, boolean
isKey) {
/**
* Convert {@code data} into a byte array.
*
- * @param topic topic associated with data
- * @param data typed data
- * @return serialized bytes
+ * <p>It is recommended to serialize {@code null} data to the {@code null}
byte array.
+ *
+ * @param topic
+ * topic associated with data
+ * @param data
+ * typed data; may be {@code null}
+ *
+ * @return serialized bytes; may be {@code null}
*/
byte[] serialize(String topic, T data);
/**
* Convert {@code data} into a byte array.
*
- * @param topic topic associated with data
- * @param headers headers associated with the record
- * @param data typed data
- * @return serialized bytes
+ * <p>It is recommended to serialize {@code null} data to the {@code null}
byte array.
+ *
+ * <p>Note that the passed in {@link Headers} may be empty, but never
{@code null}.
+ * The implementation is allowed to modify the passed in headers, as a
side effect of serialization.
+ * It is considered best practise to not delete or modify existing
headers, but rather only add new ones.
Review Comment:
```suggestion
* It is considered best practice to not delete or modify existing
headers, but rather only add new ones.
```
##########
clients/src/main/java/org/apache/kafka/common/serialization/Deserializer.java:
##########
@@ -25,39 +25,61 @@
/**
* An interface for converting bytes to objects.
- *
* A class that implements this interface is expected to have a constructor
with no parameters.
- * <p>
- * Implement {@link org.apache.kafka.common.ClusterResourceListener} to
receive cluster metadata once it's available. Please see the class
documentation for ClusterResourceListener for more information.
- * Implement {@link org.apache.kafka.common.metrics.Monitorable} to enable the
deserializer to register metrics. The following tags are automatically added to
- * all metrics registered: <code>config</code> set to either
<code>key.deserializer</code> or <code>value.deserializer</code>, and
<code>class</code> set to the Deserializer class name.
+ *
+ * <p>This interface can be combined with {@link
org.apache.kafka.common.ClusterResourceListener ClusterResourceListener}
+ * to receive cluster metadata once it's available, as well as {@link
org.apache.kafka.common.metrics.Monitorable Monitorable}
+ * to enable the deserializer to register metrics. For the later case, the
following tags are automatically added to
+ * all metrics registered: {@code config} set to either {@code
key.deserializer} or {@code value.deserializer},
+ * and {@code class} set to the deserializer class name.
+ *
* @param <T> Type to be deserialized into.
*/
public interface Deserializer<T> extends Closeable {
/**
* Configure this class.
- * @param configs configs in key/value pairs
- * @param isKey whether is for key or value
+ *
+ * @param configs
+ * configs in key/value pairs
+ * @param isKey
+ * whether is for key or value
*/
default void configure(Map<String, ?> configs, boolean isKey) {
// intentionally left blank
}
/**
* Deserialize a record value from a byte array into a value or object.
- * @param topic topic associated with the data
- * @param data serialized bytes; may be null; implementations are
recommended to handle null by returning a value or null rather than throwing an
exception.
- * @return deserialized typed data; may be null
+ *
+ * <p>It is recommended to deserialize a {@code null} byte array to a
{@code null} object.
+ *
+ * @param topic
+ * topic associated with the data
+ * @param data
+ * serialized bytes; may be {@code null}
+ *
+ * @return deserialized typed data; may be {@code null}
*/
T deserialize(String topic, byte[] data);
/**
* Deserialize a record value from a byte array into a value or object.
- * @param topic topic associated with the data
- * @param headers headers associated with the record; may be empty.
- * @param data serialized bytes; may be null; implementations are
recommended to handle null by returning a value or null rather than throwing an
exception.
- * @return deserialized typed data; may be null
+ *
+ * <p>It is recommended to deserialize a {@code null} byte array to a
{@code null} object.
+ *
+ * <p>Note that the passed in {@link Headers} may be empty, but never
{@code null}.
+ * The implementation is allowed to modify the passed in headers, as a
side effect of deserialization.
+ * It is considered best practise to not delete or modify existing
headers, but rather only add new ones.
Review Comment:
```suggestion
* It is considered best practice to not delete or modify existing
headers, but rather only add new ones.
```
##########
clients/src/main/java/org/apache/kafka/common/serialization/Deserializer.java:
##########
@@ -73,19 +95,29 @@ default T deserialize(String topic, Headers headers, byte[]
data) {
* <p>Similarly, if this method is overridden, the implementation cannot
make any assumptions about the
* passed in {@link ByteBuffer} either.
*
- * @param topic topic associated with the data
- * @param headers headers associated with the record; may be empty.
- * @param data serialized ByteBuffer; may be null; implementations are
recommended to handle null by returning a value or null rather than throwing an
exception.
- * @return deserialized typed data; may be null
+ * <p>It is recommended to deserialize a {@code null} {@link ByteBuffer}
to a {@code null} object.
+ *
+ * <p>Note that the passed in {@link Headers} may be empty, but never
{@code null}.
+ * The implementation is allowed to modify the passed in headers, as a
side effect of deserialization.
+ * It is considered best practise to not delete or modify existing
headers, but rather only add new ones.
Review Comment:
```suggestion
* It is considered best practice to not delete or modify existing
headers, but rather only add new ones.
```
--
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
