Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SB Track2] Message - support all types of body supported by track 1 #8384

Closed
hemanttanwar opened this issue Feb 21, 2020 · 5 comments
Closed
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Milestone

Comments

@hemanttanwar hemanttanwar added Service Bus Client This issue points to a problem in the data-plane of the library. ServiceBusTrack2 This label is to manage work item for track 2. labels Feb 21, 2020
@conniey conniey removed the ServiceBusTrack2 This label is to manage work item for track 2. label Mar 5, 2020
@conniey conniey added this to the [2020] March milestone Mar 5, 2020
@conniey conniey changed the title [SB Track 2] Message - support all types of body supported by track 1 [SB Track2] Message - support all types of body supported by track 1 Mar 21, 2020
@conniey conniey modified the milestones: [2020] May, [2020] July Jun 8, 2020
@hemanttanwar hemanttanwar self-assigned this Jul 16, 2020
@hemanttanwar
Copy link
Contributor Author

hemanttanwar commented Jul 20, 2020

AMQP data Types

There are three classes of Amqp data types. There is also the concept of described types, which means adding a descriptor to a type. Composite and Restricted can also be Described, while Primitive can not be.

Primitive - this is any of the common primitive types such as string, boolean, byte, int, double, array, list, map etc.

Composite - these are types containing multiple fields. It is actually represented as a described list where each element of the list can be a different type.

Restricted - this applies constraints to another type. e.g. an int that is restricted to a few values can be thought of as an enum. It is also used for reserved described types, such as all of the different body types.

Data is simply a described array of bytes:

Amqp Restricted types are as follows..

<type name="data" class="restricted" source="binary" provides="section">     
<descriptor name="amqp:data:binary" code="0x00000000:0x00000075"/>
</type>

AmqpSequence is a described list of any AMPQ type. Each element in the sequence can be of a different type.

<type name="amqp-sequence" class="restricted" source="list" provides="section">     
<descriptor name="amqp:amqp-sequence:list" code="0x00000000:0x00000076"/> 
</type>

AmqpValue is a described AMQP type.

<type name="amqp-value" class="restricted" source="*" provides="section">     
<descriptor name="amqp:amqp-value:*" code="0x00000000:0x00000077"/> 
</type>

Java Track 1, supports reading and writing the different body types, but for binary and sequence, it limits to a single section of each. This differs from the AMQP spec which allows for a list of each. The reason for this discrepancy is that the underlying AMQP library used by the Java SDK does not support including a list of AmqpSequence or Data.

The options are listed in priority order below.

Options 1 : API to send and receive various AMQP types .

API View : https://apiview.dev/Assemblies/Review/97f677511d9c46f9bc1e5a147e9342e5

ServiceBusMessage.java // To send message
public <T> ServiceBusMessage(T objectValue, MessageBodyType messageBodyType)

public MessageBodyType getBodyType()
public List<byte[]> getBinaryData()
public List<List<Object>> getSequenceData()
public Object getValueData()

ServiceBusReceivedMessage.java // To read these types
public MessageBodyType getBodyType()
public List<byte[]> getBinaryData()
public List<List<Object>> getSequenceData()
public Object getValueData()

Options 2 : API to send and receive various AMQP types using factory methods.

ServiceBusMessage.java // To send message
public static ServiceBusMessage fromBinaryData(List<byte[]> binaryData)
public static ServiceBusMessage fromValueData(Object value) 
public static ServiceBusMessage fromSequenceData(List<List<Object>> sequenceData)

ServiceBusReceivedMessage.java // To read these types

public List<byte[]> getBinaryData()
public List<List<Object>> getSequenceData() 
public Object getValueData()

Con :
If we exposed the raw AMQP types/Bytes, we are tying our API to a specific underlying protocol, as the API is an abstraction that should be able to evolve separately from the protocol.

The "serializer support in track2 SDK" discussion should be able to support the ask of support any data types. And these API if we provide now, will become obsilute.

Option 3: Use constructor overload to create ServiceBusMessage

ServiceBusMessage.java // To send message
public ServiceBusMessage(List<byte[]> binaryData) // 1
public <T> ServiceBusMessage (T value)  // 2
public <List<T>> ServiceBusMessage fromSequenceData(List<T> sequenceData) // 3

ServiceBusReceivedMessage.java // To read these types

public List<byte[]> getBinaryData()
public List<List<Object>> getSequenceData() 
public Object getValueData()

Con : This will not work since Constructor #1, #3 conflict.

Option 4 : Use serializer instead

The Amqp Data types except 'Data' is stored in a different location in AMQP message. And Serializer convert everything into Bytes and store in 'Data' as bute array.
It can not support Amqp Sequence , AmqpValue and other restricted types.

Read comment below from Clemens.

Comments from Clemens Vasters in favor of supporting AmqpType read/write

AMQP is our canonical standard protocol across all messaging services with MQTT complementing for device connectivity and that is a strategic commitment and has been standing for 8+ years. There should always be a hook to get and manipulate all elements of the AMQP message, but it does not need to be straight in your face for all details.

What is important to underline is that AMQP is an interop protocol and not all messages may originate in our services and not all clients may be under our control. If someone sets an AMQP message annotation when sending the message to Artemis and that message ends up being routed to Service Bus, you need to be able to get at it. That is already a problem today, so we need to improve on that rather than keep that abstracted.

Solutions increasingly span edge and cloud and there’s an a heterogeneous set of messaging assets that get integrated directly, for instance via the RabbitMQ shovel or Apache Qpid Dispatch or Apache Camel, etc. Being able to reach down into the AMQP details will matter.

I agree that there’s a 80% scenario where customers primarily care about the payload and maybe the “subject” to figure out what to do with it and we should make that straightforward without poking the customer in the eye with too many details.

But the sophisticated scenarios matter and they should not be too clunky to get to. AMQP is an application protocol that has particular settlement semantics and we should not hide those too far away just as we would not do that for HTTP (no longer – after massive pushback).

DOTNET Discussion thread : Azure/azure-sdk-for-net#11143

@JonathanGiles @conniey @srnagar

@hemanttanwar
Copy link
Contributor Author

After discussion with @JonathanGiles , we will consider Option 1 to start with .

**Key points **

  • It is important to provide support for sending and receiving AMQP types as messaging solution need to live in a large eco-system and these types are supported by many other libraries.
  • It is important for inter-operability

Java Specific : We still have to investigate why java proton j AMQP library only support List of 1 instead of List of List according to AMQP Spec.

@hemanttanwar
Copy link
Contributor Author

hemanttanwar commented Jul 28, 2020

Option 1.1 : Variation in Option 1

API View : https://apiview.dev/Assemblies/Review/6af850ed7eb04843a6897afbe13cdd28

Use key "Amqp" in API to indicate this API belongs to AMQP. V-Team lean towards this suggestion.

**Why **

In absence of any indication that this API is for "Amqp" only, user would expect it to work for other protocol if we implement in future.

Since we know that user who use this Api , are already deep down in Amqp protocol and they are specifically looking for these Amqp types.

Pro :
Avoid any confusion in user what is this API for.
Indicate to user that it will not work for any other protocol in future, since we know this is very specific to AMQP.
95-99 % of user who do not use this API, will be very clear that this API is for AMQP and will not attempt to use it.

Con :
May be con ? ~ The "Amqp", protocol specific is in there.

enum MessageBodyType{
AMQP_BINARY,
AMQP_SEQUENCE,
AMQP_VALUE
}

ServiceBusReceivedMessage.java // To read these types
public MessageBodyType getBodyType()
public List<byte[]> getAmqpBinaryBody()
public List<List<Object>> getAmqpSequenceBody()
public Object getAmqpValueBody()

Comments from Clemens email in this context

Sophisticated scenarios matter and they should not be too clunky to get to. AMQP is an application protocol that has particular settlement semantics and we should not hide those too far away just as we would not do that for HTTP (no longer – after massive pushback).

@hemanttanwar
Copy link
Contributor Author

Closing this in favor of #17614

@hemanttanwar
Copy link
Contributor Author

Closed in favor of : #17614

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Projects
None yet
Development

No branches or pull requests

3 participants