Changing Amqp message body type from AmqpValue to AmqpData

[binzywu]

Description

Changing Message.Body to ArraySegment and changing the amqp message body type to Data for the interoperate with old .Net client library

This resolves #138

This checklist is used to make sure that common guidelines for a pull request are followed.

• I have read the contribution guidelines.
• Title of the pull request is clear and informative.
• There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR.
• The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).
• If applicable, the public code is properly documented.
• Pull request includes test coverage for the included changes.
• The code builds without any errors.

[codecov-io]

Codecov Report

Merging #184 into dev will increase coverage by 1.29%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##              dev     #184      +/-   ##
==========================================
+ Coverage   55.86%   57.16%   +1.29%     
==========================================
  Files          69       69              
  Lines        5331     5334       +3     
  Branches      638      638              
==========================================
+ Hits         2978     3049      +71     
+ Misses       2053     1987      -66     
+ Partials      300      298       -2

Impacted Files	Coverage Δ
src/Microsoft.Azure.ServiceBus/Message.cs	70% <100%> (ø)	⬆️
...soft.Azure.ServiceBus/Amqp/AmqpMessageConverter.cs	60.64% <100%> (+1.15%)	⬆️
...ure.ServiceBus/Primitives/ConcurrentExpiringSet.cs	89.74% <0%> (-10.26%)	⬇️
...Microsoft.Azure.ServiceBus/Amqp/AmqpLinkCreator.cs	90.9% <0%> (-6.82%)	⬇️
...Microsoft.Azure.ServiceBus/MessagingEventSource.cs	19.91% <0%> (+0.27%)	⬆️
...osoft.Azure.ServiceBus/Primitives/TimeoutHelper.cs	34.51% <0%> (+2.65%)	⬆️
...c/Microsoft.Azure.ServiceBus/SessionReceivePump.cs	74.45% <0%> (+3.58%)	⬆️
...Microsoft.Azure.ServiceBus/Core/MessageReceiver.cs	77.74% <0%> (+4.34%)	⬆️
src/Microsoft.Azure.ServiceBus/MessageSession.cs	36.19% <0%> (+5.71%)	⬆️
... and 2 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ffe9d9...81e9be9. Read the comment docs.

[SeanFeldman]

This feels off. To create a new message with payload, ctor accepts byte[].
To assign a body later I need to use ArraySegment<byte>. Why can't it be byte[] all the way?

[binzywu]

so let's move byte[] ctor? 😃

[binzywu]

Yes, this is a breaking change. I am not saying we should do this. 😃 Comments are needed to see what's the right move here. ArraySegment can certainly bring many values and even we don't expose it as Body, internally we will still use ArraySegment as there are optimizations can be done. But public interface could remain as byte[]. We are very close to make this library GA for it's first version and want to get some critical decisions done before it goes 1.0.

[SeanFeldman]

so let's move byte[] ctor?

Not sure I follow. Did you mean "remove"?

Yes, this is a breaking change

Well, this is a pre-release 🙂

ArraySegment - not arguing. Probably internally makes a lot of sense. Externally perhaps should be byte[] as the first/default option.

[binzywu]

K. Thanks for the feedback. Yeah, I will have another discussion with team tomorrow and will update the pull request. But generally this change is mainly to change the message to use Amqp Data as we do some extra processing in both old client and service for AmqpValue and Data will be processed as is which is desired.