Uploaded image for project: 'Qpid Proton'
  1. Qpid Proton
  2. PROTON-1450

Add Filter LinkOption to Electron API

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • proton-c-0.18.0
    • go-binding
    • None

    Description

      There doesn't appear to be a way to set filters with the current Go bindings. Microsoft's Event Hubs use filters to indicate the starting offset of messages (http://azure.github.io/amqpnetlite/articles/azure_eventhubs.html#filter). Without this option it would be difficult to properly use the bindings with Event Hubs.

      There is a wrapper which patches the bindings to provide the functionality here: https://github.com/recobe182/gohub. Though I am not familiar enough with Qpid to determine if the patch is reasonable.

      Thank you for considering this request and creating/maintaining this project!

      Attachments

        Issue Links

          Activity

            aconway Alan Conway added a comment -

            Many thanks for this! I haven't had a chance to look at your patch yet but I definitely want to include this feature. Will get back to you ASAP.

            aconway Alan Conway added a comment - Many thanks for this! I haven't had a chance to look at your patch yet but I definitely want to include this feature. Will get back to you ASAP.
            aconway Alan Conway added a comment -

            Thanks again, I am looking at https://github.com/recobe182/gohub/blob/master/filter_anno.patch, let me know if that is not the right patch.

            I think all your ideas are good, and you've pointed out some other flaws I will address regarding detailed AMQP protocol support. Here's my thinking so far, please let me know if this makes sense and suits your needs or if you have other ideas:

            1. Filter goes TerminusSettings, not directly on LinkSettings. A link's Source and Target can both have filters.

            2. `type Filter map[Symbol]interface{}` rather than `map[string]string` to follow AMQP spec.

            `Symbol` is just `type Symbol string` so converts to or from string but is encoded as AMQP symbol. The AMQP spec allows other types (not just string) to be used as values in filter maps: you can still use `string` for your application and the encoding will be the same, but interface{} allows other types in future.

            3. Annotations: your patch points out a bug here - AMQP annotation keys can be symbol OR ulong, so map[string] is not accurate. Your patch fixes the encoding correctly, but the API is still wrong since you can't set or get ulong keys. I propose to leave the existing Annotation method (with your encoding fix) for backwards compatibility and add new methods like:

            func (Message) MessageAnnotations() map[AnnotationKey]interface{}

            My proposal for the AnnotationKey type is at: https://groups.google.com/forum/#!topic/golang-nuts/SsjSEqVcVqQ

            So code that uses the old Annotation methods will still work (with the correct encoding) but the new method will be recommended for the future.

            As an added bonus, the new methods are more consistent with C++ and other bindings.

            Thoughts?

            Side note: The AMQP spec says filter values should be "described types" - if Azure is happy with plain strings then don't change anything, but it might come up in future. The go library doesn't have described type support yet: https://issues.apache.org/jira/browse/PROTON-1456

            aconway Alan Conway added a comment - Thanks again, I am looking at https://github.com/recobe182/gohub/blob/master/filter_anno.patch , let me know if that is not the right patch. I think all your ideas are good, and you've pointed out some other flaws I will address regarding detailed AMQP protocol support. Here's my thinking so far, please let me know if this makes sense and suits your needs or if you have other ideas: 1. Filter goes TerminusSettings, not directly on LinkSettings. A link's Source and Target can both have filters. 2. `type Filter map [Symbol] interface{}` rather than `map [string] string` to follow AMQP spec. `Symbol` is just `type Symbol string` so converts to or from string but is encoded as AMQP symbol. The AMQP spec allows other types (not just string) to be used as values in filter maps: you can still use `string` for your application and the encoding will be the same, but interface{} allows other types in future. 3. Annotations: your patch points out a bug here - AMQP annotation keys can be symbol OR ulong, so map [string] is not accurate. Your patch fixes the encoding correctly, but the API is still wrong since you can't set or get ulong keys. I propose to leave the existing Annotation method (with your encoding fix) for backwards compatibility and add new methods like: func (Message) MessageAnnotations() map [AnnotationKey] interface{} My proposal for the AnnotationKey type is at: https://groups.google.com/forum/#!topic/golang-nuts/SsjSEqVcVqQ So code that uses the old Annotation methods will still work (with the correct encoding) but the new method will be recommended for the future. As an added bonus, the new methods are more consistent with C++ and other bindings. Thoughts? Side note: The AMQP spec says filter values should be "described types" - if Azure is happy with plain strings then don't change anything, but it might come up in future. The go library doesn't have described type support yet: https://issues.apache.org/jira/browse/PROTON-1456

            I'm not the author of that patch, my apologies if my phrasing implied otherwise. I found it while researching AMQP 1.0 support for Go. Since it's MIT licensed I figured it might be a useful reference for this issue.

            I am working on a pure Go AMQP 1.0 client implementation (hobby project, pre-alpha). Currently I'm representing filters as `map[Symbol]interface{}`, as you suggested. I can only find a reference to filters on the source (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-source), not the target (http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-target), but perhaps I've misunderstood something.

            Your AnnotationKey proposal looks to be a good solution. I was considering using `map[interface{}]interface{}` but only allowing clients of the library to set annotations via type safe function arguments. Something similar to `func AnnotationSymbol(key string, value interface{}) OptionFunc`/`func AnnotationUlong(key uint64, value interface{}) OptionFunc`. Although, since ulong keys are reserved it may be nice to have specific function arguments for each of the reserved annotations and a single `func Annotation(key string, value interface{}) OptionFunc` for "x-" annotations.

            kalebksp Kale Blankenship added a comment - I'm not the author of that patch, my apologies if my phrasing implied otherwise. I found it while researching AMQP 1.0 support for Go. Since it's MIT licensed I figured it might be a useful reference for this issue. I am working on a pure Go AMQP 1.0 client implementation (hobby project, pre-alpha). Currently I'm representing filters as `map [Symbol] interface{}`, as you suggested. I can only find a reference to filters on the source ( http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-source ), not the target ( http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-target ), but perhaps I've misunderstood something. Your AnnotationKey proposal looks to be a good solution. I was considering using `map [interface{}] interface{}` but only allowing clients of the library to set annotations via type safe function arguments. Something similar to `func AnnotationSymbol(key string, value interface{}) OptionFunc`/`func AnnotationUlong(key uint64, value interface{}) OptionFunc`. Although, since ulong keys are reserved it may be nice to have specific function arguments for each of the reserved annotations and a single `func Annotation(key string, value interface{}) OptionFunc` for "x-" annotations.
            aconway Alan Conway added a comment -

            Apologies for long delay: you are correct that filters apply only to the source. The proton C libary abstracts source and target as "terminus" but that includes everything from source and target, so I think the Go electorn package should follow the spec rather than the C library.

            I prefer an AnnotationKey type because the same restricted type is used in a few places in the spec. Doing it as a type allows all the encode/decode rules to be handled in one place, and lets you store/pass AnnotationKey's around in your application without "forgetting" the restrictions. I'll take a stab at coding it up.

            aconway Alan Conway added a comment - Apologies for long delay: you are correct that filters apply only to the source. The proton C libary abstracts source and target as "terminus" but that includes everything from source and target, so I think the Go electorn package should follow the spec rather than the C library. I prefer an AnnotationKey type because the same restricted type is used in a few places in the spec. Doing it as a type allows all the encode/decode rules to be handled in one place, and lets you store/pass AnnotationKey's around in your application without "forgetting" the restrictions. I'll take a stab at coding it up.

            Commit 110f851add21f4f79de08a0af55d93226593c26c in qpid-proton's branch refs/heads/master from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=110f851 ]

            PROTON-1450: go binding amqp.Key and message map types.

            Add amqp.Key type for message maps with keys that can be ulong or symbol.

            Add type-safe methods to deal with amqp.Message maps, old methods remain for
            compatibility but are deprecated.

            jira-bot ASF subversion and git services added a comment - Commit 110f851add21f4f79de08a0af55d93226593c26c in qpid-proton's branch refs/heads/master from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=110f851 ] PROTON-1450 : go binding amqp.Key and message map types. Add amqp.Key type for message maps with keys that can be ulong or symbol. Add type-safe methods to deal with amqp.Message maps, old methods remain for compatibility but are deprecated.
            aconway Alan Conway added a comment -

            The above commit adds the type safe amqp.Key type and type safe methods to correctly manipulate and encode amqp.Message maps.

            I have also fixed PROTON-1456 (described type support). The AMQP spec says filters should use described types, but the filter support in electron will not enforce that: if your AMQP service accepts non-described types in a filter map that will still be supported.

            I will add the actual filter support shortly to complete this issue, using map[Symbol]interface{} as the filter type, so you would do something like
            filter[Symbol("foo")] = "filter-data"
            or
            filter[Symbol("foo")] = Described

            {Symbol("filter-descriptor"), "filter-data"}

            etc.

            aconway Alan Conway added a comment - The above commit adds the type safe amqp.Key type and type safe methods to correctly manipulate and encode amqp.Message maps. I have also fixed PROTON-1456 (described type support). The AMQP spec says filters should use described types, but the filter support in electron will not enforce that: if your AMQP service accepts non-described types in a filter map that will still be supported. I will add the actual filter support shortly to complete this issue, using map [Symbol] interface{} as the filter type, so you would do something like filter [Symbol("foo")] = "filter-data" or filter [Symbol("foo")] = Described {Symbol("filter-descriptor"), "filter-data"} etc.

            Commit 110f851add21f4f79de08a0af55d93226593c26c in qpid-proton's branch refs/heads/PROTON-1488 from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=110f851 ]

            PROTON-1450: go binding amqp.Key and message map types.

            Add amqp.Key type for message maps with keys that can be ulong or symbol.

            Add type-safe methods to deal with amqp.Message maps, old methods remain for
            compatibility but are deprecated.

            jira-bot ASF subversion and git services added a comment - Commit 110f851add21f4f79de08a0af55d93226593c26c in qpid-proton's branch refs/heads/ PROTON-1488 from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=110f851 ] PROTON-1450 : go binding amqp.Key and message map types. Add amqp.Key type for message maps with keys that can be ulong or symbol. Add type-safe methods to deal with amqp.Message maps, old methods remain for compatibility but are deprecated.

            Commit 4cf899be7058fb47148a1d384aeb2ee46c1641e0 in qpid-proton's branch refs/heads/master from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=4cf899b ]

            PROTON-1450: Rename AnnotationKey, correct ApplicationProperties

            Renamed Key->AnnotationKey and constructors XXXKey() to AnnotationKeyXXX()
            ApplicationProperties map keyed by string in accordance with AMQP spec.

            jira-bot ASF subversion and git services added a comment - Commit 4cf899be7058fb47148a1d384aeb2ee46c1641e0 in qpid-proton's branch refs/heads/master from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=4cf899b ] PROTON-1450 : Rename AnnotationKey, correct ApplicationProperties Renamed Key->AnnotationKey and constructors XXXKey() to AnnotationKeyXXX() ApplicationProperties map keyed by string in accordance with AMQP spec.

            Commit c31e2ecf08fa3e5b46dc5c00a81049ec0d555196 in qpid-proton's branch refs/heads/master from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=c31e2ec ]

            PROTON-1450: go electron filter support

            electron.Filter() creates LinkOption to set filter, LinkSettings.Filter() gets filter.
            proton.Data.Marshal()/Unmarshal() allow setting/getting proton.Data content.

            jira-bot ASF subversion and git services added a comment - Commit c31e2ecf08fa3e5b46dc5c00a81049ec0d555196 in qpid-proton's branch refs/heads/master from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=c31e2ec ] PROTON-1450 : go electron filter support electron.Filter() creates LinkOption to set filter, LinkSettings.Filter() gets filter. proton.Data.Marshal()/Unmarshal() allow setting/getting proton.Data content.

            Commit 4cf899be7058fb47148a1d384aeb2ee46c1641e0 in qpid-proton's branch refs/heads/go1 from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=4cf899b ]

            PROTON-1450: Rename AnnotationKey, correct ApplicationProperties

            Renamed Key->AnnotationKey and constructors XXXKey() to AnnotationKeyXXX()
            ApplicationProperties map keyed by string in accordance with AMQP spec.

            jira-bot ASF subversion and git services added a comment - Commit 4cf899be7058fb47148a1d384aeb2ee46c1641e0 in qpid-proton's branch refs/heads/go1 from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=4cf899b ] PROTON-1450 : Rename AnnotationKey, correct ApplicationProperties Renamed Key->AnnotationKey and constructors XXXKey() to AnnotationKeyXXX() ApplicationProperties map keyed by string in accordance with AMQP spec.

            Commit c31e2ecf08fa3e5b46dc5c00a81049ec0d555196 in qpid-proton's branch refs/heads/go1 from aconway
            [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=c31e2ec ]

            PROTON-1450: go electron filter support

            electron.Filter() creates LinkOption to set filter, LinkSettings.Filter() gets filter.
            proton.Data.Marshal()/Unmarshal() allow setting/getting proton.Data content.

            jira-bot ASF subversion and git services added a comment - Commit c31e2ecf08fa3e5b46dc5c00a81049ec0d555196 in qpid-proton's branch refs/heads/go1 from aconway [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=c31e2ec ] PROTON-1450 : go electron filter support electron.Filter() creates LinkOption to set filter, LinkSettings.Filter() gets filter. proton.Data.Marshal()/Unmarshal() allow setting/getting proton.Data content.

            People

              aconway Alan Conway
              kalebksp Kale Blankenship
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: