artemlivshits commented on code in PR #19163:
URL: https://github.com/apache/kafka/pull/19163#discussion_r1985871102


##########
clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java:
##########
@@ -122,8 +124,8 @@ public final ByteBuffer serializeWithHeader(RequestHeader 
header) {
     }
 
     // Visible for testing
-    public final ByteBuffer serialize() {
-        return MessageUtil.toByteBuffer(data(), version);
+    public final Readable serialize() {

Review Comment:
   Maybe just return `ByteAccessor`?



##########
clients/src/main/java/org/apache/kafka/common/protocol/MessageUtil.java:
##########
@@ -204,13 +204,17 @@ public static boolean 
compareRawTaggedFields(List<RawTaggedField> first,
         }
     }
 
-    public static ByteBuffer toByteBuffer(final Message message, final short 
version) {
+    public static Readable toReadable(final Message message, final short 
version) {
         ObjectSerializationCache cache = new ObjectSerializationCache();
         int messageSize = message.size(cache, version);
         ByteBufferAccessor bytes = new 
ByteBufferAccessor(ByteBuffer.allocate(messageSize));
         message.write(bytes, cache, version);
         bytes.flip();
-        return bytes.buffer();
+        return bytes;
+    }
+
+    public static ByteBuffer toByteBuffer(final Message message, final short 
version) {
+        return ((ByteBufferAccessor) toReadable(message, version)).buffer();

Review Comment:
   I agree, we could probably just have  `ByteAccessor toByteAcessor(final 
Message message, final short version)` either as a single method to be used for 
both variants or as a private method with 2 public wrappers `toReadable` and 
`toByteBuffer`.



-- 
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

Reply via email to