Copilot commented on code in PR #48: URL: https://github.com/apache/activemq-nms-amqp/pull/48#discussion_r2180420311
########## src/NMS.AMQP/Provider/Amqp/Message/AmqpNmsBytesMessageFacade.cs: ########## @@ -45,58 +46,60 @@ public BinaryReader GetDataReader() if (byteIn == null) { - Data body = GetBinaryFromBody(); - Stream dataStream = new MemoryStream(body.Binary, false); + byte[] body = Content; + Stream dataStream = new MemoryStream(body, false); byteIn = new EndianBinaryReader(dataStream); } return byteIn; } - private Data GetBinaryFromBody() + public byte[] Content { - RestrictedDescribed body = Message.BodySection; - Data result = EMPTY_DATA; - if (body == null) + get { - return result; - } - else if (body is Data) - { - byte[] binary = (body as Data).Binary; - if (binary != null && binary.Length != 0) + RestrictedDescribed body = Message.BodySection; + byte[] result = EMPTY_BINARY; + if (body == null) { - return body as Data; + return result; } - } - else if (body is AmqpValue) - { - object value = (body as AmqpValue).Value; - if (value == null) + else if (body is Data) { - return result; + byte[] binary = (body as Data).Binary; + if (binary != null && binary.Length != 0) + { + return binary; + } } - - if (value is byte[]) + else if (body is AmqpValue) { - byte[] dataValue = value as byte[]; - if (dataValue.Length > 0) + object value = (body as AmqpValue).Value; + if (value == null) { - result = new Data(); - result.Binary = dataValue; + return result; + } + if (value is byte[]) + { + byte[] dataValue = value as byte[]; + if (dataValue.Length > 0) + { + return dataValue; + } + } + else + { + throw new IllegalStateException("Unexpected Amqp value content-type: " + value.GetType().FullName); } } else { - throw new IllegalStateException("Unexpected Amqp value content-type: " + value.GetType().FullName); + throw new IllegalStateException("Unexpected body content-type: " + body.GetType().FullName); } - } - else - { - throw new IllegalStateException("Unexpected body content-type: " + body.GetType().FullName); - } - return result; + return result; + } + set => Message.BodySection = new Data { Binary = value }; Review Comment: After updating the body section, reset any cached readers/writers (`byteIn`, `byteOut`) to avoid stale stream state when consuming or writing content again. ```suggestion set { Message.BodySection = new Data { Binary = value }; byteIn = null; byteOut = null; } ``` ########## src/NMS.AMQP/Provider/Amqp/Message/AmqpNmsBytesMessageFacade.cs: ########## @@ -31,10 +31,11 @@ public class AmqpNmsBytesMessageFacade : AmqpNmsMessageFacade, INmsBytesMessageF private EndianBinaryReader byteIn = null; private EndianBinaryWriter byteOut = null; - private static readonly Data EMPTY_DATA = new Data { Binary = new byte[0] }; + private static readonly byte[] EMPTY_BINARY = new byte[0]; + private static readonly Data EMPTY_DATA = new Data { Binary = EMPTY_BINARY }; Review Comment: The EMPTY_DATA constant is no longer referenced after the refactoring. Consider removing it to clean up unused code. ```suggestion ``` ########## src/NMS.AMQP/Message/NmsBytesMessage.cs: ########## @@ -292,15 +295,19 @@ public int ReadBytes(byte[] value) public int ReadBytes(byte[] value, int length) { InitializeReading(); + return ReadBytes(dataIn, value, length); + } + private int ReadBytes(BinaryReader binaryReader, byte[] value, int length) Review Comment: [nitpick] This private overload shares the same name as the public `ReadBytes` method, which may cause confusion. Consider renaming it to something like `ReadBytesInternal`. ```suggestion return ReadBytesInternal(dataIn, value, length); } private int ReadBytesInternal(BinaryReader binaryReader, byte[] value, int length) ``` -- 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact