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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
