You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ac...@apache.org on 2018/09/28 13:31:04 UTC
qpid-proton git commit: PROTON-1940: [c] normalize encoding of
multiple="true" fields
Repository: qpid-proton
Updated Branches:
refs/heads/master b114344ff -> 5960f15df
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/master
Commit: 5960f15df324daf04387066af55dbb2f79c68f71
Parents: b114344
Author: Alan Conway <ac...@redhat.com>
Authored: Mon Sep 24 11:41:03 2018 -0400
Committer: Alan Conway <ac...@redhat.com>
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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org