PROTON-1940: [c] normalize encoding of multiple="true" fields Append src to data after normalizing for "multiple" field encoding.
AMQP composite field definitions can be declared "multiple", see: - http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-types-v1.0-os.html#doc-idp115568 - http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-types-v1.0-os.html#section-composite-type-representation Multiple fields allow redundant encoding of two cases: 1. empty: null or an empty array. 2. single-value: direct encoding of value, or array with one element For encoding compactness and inter-operability, normalize multiple field values to always use null for empty, and direct encoding for single value. Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/5960f15d Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/5960f15d Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/5960f15d Branch: refs/heads/go1 Commit: 5960f15df324daf04387066af55dbb2f79c68f71 Parents: b114344 Author: Alan Conway <[email protected]> Authored: Mon Sep 24 11:41:03 2018 -0400 Committer: Alan Conway <[email protected]> Committed: Fri Sep 28 09:15:53 2018 -0400 ---------------------------------------------------------------------- c/src/core/codec.c | 106 +++++++++++++++++++++++++++---- c/src/core/transport.c | 9 ++- c/tests/data.c | 57 ++++++++++++++++- c/tests/test_tools.h | 7 ++ python/tests/proton_tests/engine.py | 2 +- 5 files changed, 165 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5960f15d/c/src/core/codec.c ---------------------------------------------------------------------- diff --git a/c/src/core/codec.c b/c/src/core/codec.c index 595a4e6..37165f5 100644 --- a/c/src/core/codec.c +++ b/c/src/core/codec.c @@ -481,6 +481,84 @@ static int pni_data_intern_node(pn_data_t *data, pni_node_t *node) return 0; } + +/* + Append src to data after normalizing for "multiple" field encoding. + + AMQP composite field definitions can be declared "multiple", see: + + - http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-types-v1.0-os.html#doc-idp115568 + - http://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-types-v1.0-os.html#section-composite-type-representation + + Multiple fields allow redundant encoding of two cases: + + 1. empty: null or an empty array. + 2. single-value: direct encoding of value, or array with one element + + For encoding compactness and inter-operability, normalize multiple + field values to always use null for empty, and direct encoding for + single value. +*/ +static int pni_normalize_multiple(pn_data_t *data, pn_data_t *src) { + int err = 0; + pn_handle_t point = pn_data_point(src); + pn_data_rewind(src); + pn_data_next(src); + if (pn_data_type(src) == PN_ARRAY) { + switch (pn_data_get_array(src)) { + case 0: /* Empty array => null */ + err = pn_data_put_null(data); + break; + case 1: /* Single-element array => encode the element */ + pn_data_enter(src); + pn_data_narrow(src); + err = pn_data_appendn(data, src, 1); + pn_data_widen(src); + break; + default: /* Multi-element array, encode unchanged */ + err = pn_data_appendn(data, src, 1); + break; + } + } else { + err = pn_data_appendn(data, src, 1); /* Non-array, append the value */ + } + pn_data_restore(src, point); + return err; +} + + +/* Format codes: + code: AMQP-type (arguments) + n: null () + o: bool (int) + B: ubyte (unsigned int) + b: byte (int) + H: ushort (unsigned int) + h: short (int) + I: uint (uint32_t) + i: int (int32_t) + L: ulong (ulong32_t) + l: long (long32_t) + t: timestamp (pn_timestamp_t) + f: float (float) + d: double (double) + Z: binary (size_t, char*) - must not be NULL + z: binary (size_t, char*) - encode as AMQP null if NULL + S: symbol (char*) + s: string (char*) + D: described - next two codes are [descriptor, body] + @: enter array. If followed by D, a described array. Following codes to matching ']' are elements. + T: type (pn_type_t) - set array type while in array + [: enter list. Following codes up to matching ']' are elements + {: enter map. Following codes up to matching '}' are key, value pairs + ]: exit list or array + }: exit map + ?: TODO document + *: TODO document + C: single value (pn_data_t*) - append the pn_data_t unmodified + M: multiple value (pn_data_t*) - normalize and append multiple field value, + see pni_normalize_multiple() + */ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) { int err = 0; @@ -529,7 +607,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) case 'd': err = pn_data_put_double(data, va_arg(ap, double)); break; - case 'Z': + case 'Z': /* encode binary, must not be NULL */ { // For maximum portability, caller must pass these as two separate args, not a single struct size_t size = va_arg(ap, size_t); @@ -537,7 +615,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) err = pn_data_put_binary(data, pn_bytes(size, start)); } break; - case 'z': + case 'z': /* encode binary or null if pointer is NULL */ { // For maximum portability, caller must pass these as two separate args, not a single struct size_t size = va_arg(ap, size_t); @@ -549,8 +627,8 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) } } break; - case 'S': - case 's': + case 'S': /* encode symbol or null if NULL */ + case 's': /* encode string or null if NULL */ { char *start = va_arg(ap, char *); size_t size; @@ -571,7 +649,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) err = pn_data_put_described(data); pn_data_enter(data); break; - case 'T': + case 'T': /* Set type of open array */ { pni_node_t *parent = pn_data_node(data, data->parent); if (parent->atom.type == PN_ARRAY) { @@ -581,7 +659,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) } } break; - case '@': + case '@': /* begin array */ { bool described; if (*(fmt + 1) == 'D') { @@ -594,14 +672,14 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) pn_data_enter(data); } break; - case '[': + case '[': /* begin list */ if (fmt < (begin + 2) || *(fmt - 2) != 'T') { err = pn_data_put_list(data); if (err) return err; pn_data_enter(data); } break; - case '{': + case '{': /* begin map */ err = pn_data_put_map(data); if (err) return err; pn_data_enter(data); @@ -612,7 +690,6 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) return pn_error_format(data->error, PN_ERR, "exit failed"); break; case '?': - /* Consumes 2 args: bool, value. Insert null if bool is false else value */ if (!va_arg(ap, int)) { err = pn_data_put_null(data); if (err) return err; @@ -645,7 +722,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) } } break; - case 'C': + case 'C': /* Append an existing pn_data_t * */ { pn_data_t *src = va_arg(ap, pn_data_t *); if (src && pn_data_size(src) > 0) { @@ -657,7 +734,14 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list ap) } } break; - default: + case 'M': + { + pn_data_t *src = va_arg(ap, pn_data_t *); + err = (src && pn_data_size(src) > 0) ? + pni_normalize_multiple(data, src) : pn_data_put_null(data); + break; + } + default: pn_logf("unrecognized fill code: 0x%.2X '%c'", code, code); return PN_ARG_ERR; } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5960f15d/c/src/core/transport.c ---------------------------------------------------------------------- diff --git a/c/src/core/transport.c b/c/src/core/transport.c index 7dee571..5dce530 100644 --- a/c/src/core/transport.c +++ b/c/src/core/transport.c @@ -1858,13 +1858,13 @@ static int pni_process_conn_setup(pn_transport_t *transport, pn_endpoint_t *endp pn_connection_t *connection = (pn_connection_t *) endpoint; const char *cid = pn_string_get(connection->container); pni_calculate_channel_max(transport); - int err = pn_post_frame(transport, AMQP_FRAME_TYPE, 0, "DL[SS?I?H?InnCCC]", OPEN, + int err = pn_post_frame(transport, AMQP_FRAME_TYPE, 0, "DL[SS?I?H?InnMMC]", OPEN, cid ? cid : "", pn_string_get(connection->hostname), // TODO: This is messy, because we also have to allow local_max_frame_ to be 0 to mean unlimited // otherwise flow control goes wrong transport->local_max_frame!=0 && transport->local_max_frame!=OPEN_MAX_FRAME_SIZE_DEFAULT, - transport->local_max_frame, + transport->local_max_frame, transport->channel_max!=OPEN_CHANNEL_MAX_DEFAULT, transport->channel_max, (bool)idle_timeout, idle_timeout, connection->offered_capabilities, @@ -2024,12 +2024,13 @@ static int pni_process_link_setup(pn_transport_t *transport, pn_endpoint_t *endp if (err) return err; } else { int err = pn_post_frame(transport, AMQP_FRAME_TYPE, ssn_state->local_channel, - "DL[SIoBB?DL[SIsIoC?sCnCC]?DL[SIsIoCC]nnIL]", ATTACH, + "DL[SIoBB?DL[SIsIoC?sCnMM]?DL[SIsIoCM]nnIL]", ATTACH, pn_string_get(link->name), state->local_handle, endpoint->type == RECEIVER, link->snd_settle_mode, link->rcv_settle_mode, + (bool) link->source.type, SOURCE, pn_string_get(link->source.address), link->source.durability, @@ -2041,6 +2042,7 @@ static int pni_process_link_setup(pn_transport_t *transport, pn_endpoint_t *endp link->source.filter, link->source.outcomes, link->source.capabilities, + (bool) link->target.type, TARGET, pn_string_get(link->target.address), link->target.durability, @@ -2049,6 +2051,7 @@ static int pni_process_link_setup(pn_transport_t *transport, pn_endpoint_t *endp link->target.dynamic, link->target.properties, link->target.capabilities, + 0, link->max_message_size); if (err) return err; } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5960f15d/c/tests/data.c ---------------------------------------------------------------------- diff --git a/c/tests/data.c b/c/tests/data.c index 10e7039..8f8030f 100644 --- a/c/tests/data.c +++ b/c/tests/data.c @@ -21,8 +21,10 @@ #undef NDEBUG /* Make sure that assert() is enabled even in a release build. */ -#include <proton/codec.h> +#include "test_tools.h" #include "core/data.h" + +#include <proton/codec.h> #include <assert.h> #include <stdio.h> @@ -44,6 +46,59 @@ static void test_grow(void) pn_data_free(data); } +static void test_multiple(test_t *t) { + pn_data_t *data = pn_data(1); + pn_data_t *src = pn_data(1); + + /* NULL data pointer */ + pn_data_fill(data, "M", NULL); + TEST_INSPECT(t, "null", data); + + /* Empty data object */ + pn_data_clear(data); + pn_data_fill(data, "M", src); + TEST_INSPECT(t, "null", data); + + /* Empty array */ + pn_data_clear(data); + pn_data_clear(src); + pn_data_put_array(src, false, PN_SYMBOL); + pn_data_fill(data, "M", src); + TEST_INSPECT(t, "null", data); + + /* Single-element array */ + pn_data_clear(data); + pn_data_clear(src); + pn_data_put_array(src, false, PN_SYMBOL); + pn_data_enter(src); + pn_data_put_symbol(src, PN_BYTES_LITERAL(foo)); + pn_data_fill(data, "M", src); + TEST_INSPECT(t, ":foo", data); + + /* Multi-element array */ + pn_data_clear(data); + pn_data_clear(src); + pn_data_put_array(src, false, PN_SYMBOL); + pn_data_enter(src); + pn_data_put_symbol(src, PN_BYTES_LITERAL(foo)); + pn_data_put_symbol(src, PN_BYTES_LITERAL(bar)); + pn_data_fill(data, "M", src); + TEST_INSPECT(t, "@PN_SYMBOL[:foo, :bar]", data); + + /* Non-array */ + pn_data_clear(data); + pn_data_clear(src); + pn_data_put_symbol(src, PN_BYTES_LITERAL(baz)); + pn_data_fill(data, "M", src); + TEST_INSPECT(t, ":baz", data); + + pn_data_free(data); + pn_data_free(src); +} + int main(int argc, char **argv) { + int failed = 0; test_grow(); + RUN_ARGV_TEST(failed, t, test_multiple(&t)); + return failed; } http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5960f15d/c/tests/test_tools.h ---------------------------------------------------------------------- diff --git a/c/tests/test_tools.h b/c/tests/test_tools.h index 6910450..164968e 100644 --- a/c/tests/test_tools.h +++ b/c/tests/test_tools.h @@ -160,6 +160,13 @@ bool test_str_equal_(test_t *t, const char* want, const char* got, const char *f } #define TEST_STR_EQUAL(TEST, WANT, GOT) test_str_equal_((TEST), (WANT), (GOT), __FILE__, __LINE__) +#define TEST_INSPECT(TEST, WANT, GOT) do { \ + pn_string_t *s = pn_string(NULL); \ + TEST_ASSERT(0 == pn_inspect(GOT, s)); \ + TEST_STR_EQUAL((TEST), (WANT), pn_string_get(s)); \ + pn_free(s); \ + } while (0) + #define TEST_STR_IN(TEST, WANT, GOT) \ test_check_((TEST), strstr((GOT), (WANT)), NULL, __FILE__, __LINE__, "'%s' not in '%s'", (WANT), (GOT)) http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/5960f15d/python/tests/proton_tests/engine.py ---------------------------------------------------------------------- diff --git a/python/tests/proton_tests/engine.py b/python/tests/proton_tests/engine.py index df9e6a1..41857c2 100644 --- a/python/tests/proton_tests/engine.py +++ b/python/tests/proton_tests/engine.py @@ -690,7 +690,7 @@ class LinkTest(Test): capabilities=["one", "two", "three"]), TerminusConfig(address="source", timeout=7, - capabilities=[])) + capabilities=None)) def test_distribution_mode(self): self._test_source_target(TerminusConfig(address="source", dist_mode=Terminus.DIST_MODE_COPY), --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
