frankvicky commented on code in PR #21408:
URL: https://github.com/apache/kafka/pull/21408#discussion_r2770938049
##########
streams/src/main/java/org/apache/kafka/streams/state/internals/ValueTimestampHeadersDeserializer.java:
##########
@@ -113,30 +127,21 @@ static byte[] rawValue(final byte[]
rawValueTimestampHeaders) {
}
final ByteBuffer buffer = ByteBuffer.wrap(rawValueTimestampHeaders);
- final int headerSize = ByteUtils.readVarint(buffer);
- buffer.position(buffer.position() + headerSize + Long.BYTES);
-
- final byte[] rawValue = new byte[buffer.remaining()];
- buffer.get(rawValue);
+ final int headersSize = ByteUtils.readVarint(buffer);
+ buffer.position(buffer.position() + headersSize + Long.BYTES);
- return rawValue;
+ return readBytes(buffer, buffer.remaining());
}
/**
* Extract timestamp from serialized ValueTimestampHeaders.
*/
static long timestamp(final byte[] rawValueTimestampHeaders) {
- if (rawValueTimestampHeaders == null) {
Review Comment:
It's quite tricky here. Since `timestamp()` returns a primitive `long` (not
`Long`), returning null isn't an option.
I considered throwing an `IllegalArgumentException` for `null` input to be
consistent with the validation approach used in readBytes(), but that would be
inconsistent with how the analogous method in `ValueAndTimestampDeserializer`
handles this case.
Looking at `ValueAndTimestampDeserializer.timestamp()`, which also returns
primitive long, the design choice there is to let `ByteBuffer.wrap()` throw
`NullPointerException` for null input. This makes the caller responsible for
null checks before invocation. I followed the same pattern for consistency with
the existing codebase.
Make sense?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
