Revise Map encoding for attributes in AMQP

[clemensv]

Looking at the AMQP bindings, it's not clear that "Map" values can be represented as requested in AMQP, because the application-properties section does not allow values of the map type.

The keys of this map are restricted to be of type string (which excludes the possibility of a null key) and the values are restricted to be of simple types only, that is, excluding map, list, and array types.

Originally posted by @evankanderson in #456 (comment)

That constraint is one that the AMQP TC now regrets, but it's difficult to change now. The AMQP-native way to do this would be to store the Map as a binary-encoded AMQP map, which is a little weird, but would be close to the wire model you'd have if maps were allowed. The more straightforward way for apps would be to store it as a JSON-encoded string.

[evankanderson]

I think either format would be fine, but unless the AMQP TC wants to revise the standard, we should probably suggest one of the workarounds -- I'm not sure which to recommend, but I suspect that the binary-encoded AMQP map would need some sort of indication for bridging into other formats so that it's clear that the value is an encoded Map and not an arbitrary Binary string.

[duglin]

@clemensv do you want to work on a PR for this one?

[clemensv]

@duglin Yes, I will work on this.

Thread on the AMQP TC mailing list: https://lists.oasis-open.org/archives/amqp/201906/msg00013.html

[clemensv]

@duglin @evankanderson

As the linked thread shows, @rgodfrey reminded me that the key reason for the type limitation is that message/event selectors and filters generally don't allow for traversing complex structures, also because that can become rather expensive and drives complexity (suddenly people will want expressions that search inside a collection).

It would be possible to create a capability override in AMQP to allow for complex types (lists and maps) in application-properties even if the core spec disallows it, but the capability override might not be consistently supported on a multi-hop route. In reality, we're not sure whether implementations have a hard enforcement of the rule that the values must not be complex, but filters will certainly not be able to do anything with that content.

Therefore, and informed by the filter consideration, I would prefer if we would explicitly discourage usage of complex types in attributes for CloudEvents altogether, and the AMQP binding would use JSON encoding as an escape hatch similar to how HTTP handles it now.

[evankanderson]

Do we have a concrete use case for Map values at the moment? Map values are one of the more complex areas of the spec, and the only one which does not seem to be used in the core or extensions. Furthermore, it sounds like several other ecosystems (HTTP and AMQP) don't support them, and the AMQP decision was guided by careful consideration. Would it be simpler to remove Map from the v1 spec altogether?

…

On Mon, Jun 24, 2019, 12:46 AM Clemens Vasters ***@***.***> wrote: @duglin <https://github.com/duglin> @evankanderson <https://github.com/evankanderson> As the linked thread shows, @rgodfrey <https://github.com/rgodfrey> reminded me that the key reason for the type limitation is that message/event selectors and filters generally don't allow for traversing complex structures, also because that can become rather expensive and drives complexity (suddenly people will want expressions that search inside a collection). It would be possible to create a capability override in AMQP to allow for complex types (lists and maps) in application-properties even if the core spec disallows it, but the capability override might not be consistently supported on a multi-hop route. In reality, we're not sure whether implementations have a hard enforcement of the rule that the values must not be complex, but filters will certainly not be able to do anything with that content. Therefore, and informed by the filter consideration, I would prefer if we would explicitly discourage usage of complex types in attributes for CloudEvents altogether, and the AMQP binding would use JSON encoding as an escape hatch similar to how HTTP handles it now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#460?email_source=notifications&email_token=AB4XEN7X4J4R4LMSER75Q5LP4B3T7A5CNFSM4HY6AVI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYMCEXY#issuecomment-504898143>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB4XEN2CQADKFNCUSAH4753P4B3T7ANCNFSM4HY6AVIQ> .

[evankanderson]

Reviewing the spec, it turns out that Map is an important part of Any, which is needed for the data attribute. But we could remove Map and Any from consideration in Context Attributes, which would remove a number of tricky cases from the encoding, and also remove the problematic HTTP header text.

Suggested #467 to remove Any and Map types from Context Attributes.

[clemensv]

@evankanderson I second #467 to resolve this issue