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 2017/11/15 18:47:40 UTC

[2/2] qpid-proton git commit: PROTON-1685: [go] support for marshalling/unmarshaling AMQP arrays

PROTON-1685: [go] support for marshalling/unmarshaling AMQP arrays

Simplified marshalling code and lookup of "arrayable" types
More consistent treatment of slices and arrays


Project: http://git-wip-us.apache.org/repos/asf/qpid-proton/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-proton/commit/1659af1f
Tree: http://git-wip-us.apache.org/repos/asf/qpid-proton/tree/1659af1f
Diff: http://git-wip-us.apache.org/repos/asf/qpid-proton/diff/1659af1f

Branch: refs/heads/master
Commit: 1659af1f60205b6cd2da52a3a98668f3670c9ab1
Parents: 8e3fa19
Author: Alan Conway <ac...@redhat.com>
Authored: Tue Nov 14 16:00:40 2017 -0500
Committer: Alan Conway <ac...@redhat.com>
Committed: Wed Nov 15 13:45:56 2017 -0500

----------------------------------------------------------------------
 .../go/src/qpid.apache.org/amqp/marshal.go      | 232 ++++++++-----------
 .../go/src/qpid.apache.org/amqp/types_test.go   |   6 +-
 .../go/src/qpid.apache.org/amqp/unmarshal.go    |  12 +-
 3 files changed, 108 insertions(+), 142 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1659af1f/proton-c/bindings/go/src/qpid.apache.org/amqp/marshal.go
----------------------------------------------------------------------
diff --git a/proton-c/bindings/go/src/qpid.apache.org/amqp/marshal.go b/proton-c/bindings/go/src/qpid.apache.org/amqp/marshal.go
index bbb2450..1e4587c 100644
--- a/proton-c/bindings/go/src/qpid.apache.org/amqp/marshal.go
+++ b/proton-c/bindings/go/src/qpid.apache.org/amqp/marshal.go
@@ -162,198 +162,164 @@ func encodeGrow(buffer []byte, encode encodeFn) ([]byte, error) {
 	return buffer, err
 }
 
-const intIsLong bool = (unsafe.Sizeof(int(0)) == 8)
-
-// Marshal v to data if data != nil
-// Return the pn_type_t for v, even if data == nil
-func marshal(i interface{}, data *C.pn_data_t) C.pn_type_t {
-	if data != nil { // On exit, check for errors on the data object
-		defer func() {
-			if err := dataMarshalError(i, data); err != nil {
-				panic(err)
-			}
-		}()
-	}
+// Marshal v to data
+func marshal(i interface{}, data *C.pn_data_t) {
 	switch v := i.(type) {
 	case nil:
-		if data != nil {
-			C.pn_data_put_null(data)
-		}
-		return C.PN_NULL
+		C.pn_data_put_null(data)
 	case bool:
-		if data != nil {
-			C.pn_data_put_bool(data, C.bool(v))
-		}
-		return C.PN_BOOL
+		C.pn_data_put_bool(data, C.bool(v))
+
+	// Signed integers
 	case int8:
-		if data != nil {
-			C.pn_data_put_byte(data, C.int8_t(v))
-		}
-		return C.PN_BYTE
+		C.pn_data_put_byte(data, C.int8_t(v))
 	case int16:
-		if data != nil {
-			C.pn_data_put_short(data, C.int16_t(v))
-		}
-		return C.PN_SHORT
+		C.pn_data_put_short(data, C.int16_t(v))
 	case int32:
-		if data != nil {
-			C.pn_data_put_int(data, C.int32_t(v))
-		}
-		return C.PN_INT
+		C.pn_data_put_int(data, C.int32_t(v))
 	case int64:
-		if data != nil {
-			C.pn_data_put_long(data, C.int64_t(v))
-		}
-		return C.PN_LONG
+		C.pn_data_put_long(data, C.int64_t(v))
 	case int:
 		if intIsLong {
 			C.pn_data_put_long(data, C.int64_t(v))
-			return C.PN_LONG
 		} else {
 			C.pn_data_put_int(data, C.int32_t(v))
-			return C.PN_INT
 		}
+
+		// Unsigned integers
 	case uint8:
-		if data != nil {
-			C.pn_data_put_ubyte(data, C.uint8_t(v))
-		}
-		return C.PN_UBYTE
+		C.pn_data_put_ubyte(data, C.uint8_t(v))
 	case uint16:
-		if data != nil {
-			C.pn_data_put_ushort(data, C.uint16_t(v))
-		}
-		return C.PN_USHORT
+		C.pn_data_put_ushort(data, C.uint16_t(v))
 	case uint32:
-		if data != nil {
-			C.pn_data_put_uint(data, C.uint32_t(v))
-		}
-		return C.PN_UINT
+		C.pn_data_put_uint(data, C.uint32_t(v))
 	case uint64:
-		if data != nil {
-			C.pn_data_put_ulong(data, C.uint64_t(v))
-		}
-		return C.PN_ULONG
+		C.pn_data_put_ulong(data, C.uint64_t(v))
 	case uint:
 		if intIsLong {
 			C.pn_data_put_ulong(data, C.uint64_t(v))
-			return C.PN_ULONG
 		} else {
 			C.pn_data_put_uint(data, C.uint32_t(v))
-			return C.PN_UINT
 		}
+
+		// Floating point
 	case float32:
-		if data != nil {
-			C.pn_data_put_float(data, C.float(v))
-		}
-		return C.PN_FLOAT
+		C.pn_data_put_float(data, C.float(v))
 	case float64:
-		if data != nil {
-			C.pn_data_put_double(data, C.double(v))
-		}
-		return C.PN_DOUBLE
-	case string:
-		if data != nil {
-			C.pn_data_put_string(data, pnBytes([]byte(v)))
-		}
-		return C.PN_STRING
+		C.pn_data_put_double(data, C.double(v))
 
+		// String-like (string, binary, symbol)
+	case string:
+		C.pn_data_put_string(data, pnBytes([]byte(v)))
 	case []byte:
-		if data != nil {
-			C.pn_data_put_binary(data, pnBytes(v))
-		}
-		return C.PN_BINARY
-
+		C.pn_data_put_binary(data, pnBytes(v))
 	case Binary:
-		if data != nil {
-			C.pn_data_put_binary(data, pnBytes([]byte(v)))
-		}
-		return C.PN_BINARY
-
+		C.pn_data_put_binary(data, pnBytes([]byte(v)))
 	case Symbol:
-		if data != nil {
-			C.pn_data_put_symbol(data, pnBytes([]byte(v)))
-		}
-		return C.PN_SYMBOL
+		C.pn_data_put_symbol(data, pnBytes([]byte(v)))
 
+		// Other simple types
+	case time.Time:
+		C.pn_data_put_timestamp(data, C.pn_timestamp_t(v.UnixNano()/1000))
+	case UUID:
+		C.pn_data_put_uuid(data, *(*C.pn_uuid_t)(unsafe.Pointer(&v[0])))
+	case Char:
+		C.pn_data_put_char(data, (C.pn_char_t)(v))
+
+		// Described types
 	case Described:
 		C.pn_data_put_described(data)
 		C.pn_data_enter(data)
 		marshal(v.Descriptor, data)
 		marshal(v.Value, data)
 		C.pn_data_exit(data)
-		return C.PN_DESCRIBED
 
+		// Restricted type annotation-key, marshals as contained value
 	case AnnotationKey:
-		return marshal(v.Get(), data)
-
-	case time.Time:
-		if data != nil {
-			C.pn_data_put_timestamp(data, C.pn_timestamp_t(v.UnixNano()/1000))
-		}
-		return C.PN_TIMESTAMP
-
-	case UUID:
-		if data != nil {
-			C.pn_data_put_uuid(data, *(*C.pn_uuid_t)(unsafe.Pointer(&v[0])))
-		}
-		return C.PN_UUID
-
-	case Char:
-		if data != nil {
-			C.pn_data_put_char(data, (C.pn_char_t)(v))
-		}
-		return C.PN_CHAR
+		marshal(v.Get(), data)
 
 	default:
-		// Look at more complex types by reflected structure
-
+		// Examine complex types (Go map, slice, array) by reflected structure
 		switch reflect.TypeOf(i).Kind() {
 
 		case reflect.Map:
-			if data != nil {
-				m := reflect.ValueOf(v)
-				C.pn_data_put_map(data)
-				C.pn_data_enter(data)
+			m := reflect.ValueOf(v)
+			C.pn_data_put_map(data)
+			if C.pn_data_enter(data) {
 				defer C.pn_data_exit(data)
-				for _, key := range m.MapKeys() {
-					marshal(key.Interface(), data)
-					marshal(m.MapIndex(key).Interface(), data)
-				}
+			} else {
+				panic(dataMarshalError(i, data))
+			}
+			for _, key := range m.MapKeys() {
+				marshal(key.Interface(), data)
+				marshal(m.MapIndex(key).Interface(), data)
 			}
-			return C.PN_MAP
 
 		case reflect.Slice, reflect.Array:
 			// Note: Go array and slice are mapped the same way:
 			// if element type is an interface, map to AMQP list (mixed type)
 			// if element type is a non-interface type map to AMQP array (single type)
 			s := reflect.ValueOf(v)
-			var ret C.pn_type_t
-			t := reflect.TypeOf(i).Elem()
-			if t.Kind() == reflect.Interface {
-				if data != nil {
-					C.pn_data_put_list(data)
-				}
-				ret = C.PN_LIST
+			if pnType, ok := arrayTypeMap[s.Type().Elem()]; ok {
+				C.pn_data_put_array(data, false, pnType)
 			} else {
-				if data != nil {
-					pnType := marshal(reflect.Zero(t).Interface(), nil)
-					C.pn_data_put_array(data, false, pnType)
-				}
-				ret = C.PN_ARRAY
+				C.pn_data_put_list(data)
 			}
-			if data != nil {
-				C.pn_data_enter(data)
-				defer C.pn_data_exit(data)
-				for j := 0; j < s.Len(); j++ {
-					marshal(s.Index(j).Interface(), data)
-				}
+			C.pn_data_enter(data)
+			defer C.pn_data_exit(data)
+			for j := 0; j < s.Len(); j++ {
+				marshal(s.Index(j).Interface(), data)
 			}
-			return ret
 
 		default:
 			panic(newMarshalError(v, "no conversion"))
 		}
 	}
+	if err := dataMarshalError(i, data); err != nil {
+		panic(err)
+	}
+}
+
+// Mapping froo Go element type to AMQP array type for types that can go in an AMQP array
+// NOTE: this must be kept consistent with marshal() which does the actual marshalling.
+var arrayTypeMap = map[reflect.Type]C.pn_type_t{
+	nil:                  C.PN_NULL,
+	reflect.TypeOf(true): C.PN_BOOL,
+
+	reflect.TypeOf(int8(0)):  C.PN_BYTE,
+	reflect.TypeOf(int16(0)): C.PN_INT,
+	reflect.TypeOf(int32(0)): C.PN_SHORT,
+	reflect.TypeOf(int64(0)): C.PN_LONG,
+
+	reflect.TypeOf(uint8(0)):  C.PN_UBYTE,
+	reflect.TypeOf(uint16(0)): C.PN_UINT,
+	reflect.TypeOf(uint32(0)): C.PN_USHORT,
+	reflect.TypeOf(uint64(0)): C.PN_ULONG,
+
+	reflect.TypeOf(float32(0)): C.PN_FLOAT,
+	reflect.TypeOf(float64(0)): C.PN_DOUBLE,
+
+	reflect.TypeOf(""):                    C.PN_STRING,
+	reflect.TypeOf((*Symbol)(nil)).Elem(): C.PN_SYMBOL,
+	reflect.TypeOf((*Binary)(nil)).Elem(): C.PN_BINARY,
+	reflect.TypeOf([]byte{}):              C.PN_BINARY,
+
+	reflect.TypeOf((*time.Time)(nil)).Elem(): C.PN_TIMESTAMP,
+	reflect.TypeOf((*UUID)(nil)).Elem():      C.PN_UUID,
+	reflect.TypeOf((*Char)(nil)).Elem():      C.PN_CHAR,
+}
+
+const intIsLong = unsafe.Sizeof(int(0)) == 8
+
+// Compute mapping of int/uint at runtime as they depend on execution environment.
+func init() {
+	if intIsLong {
+		arrayTypeMap[reflect.TypeOf(int(0))] = C.PN_LONG
+		arrayTypeMap[reflect.TypeOf(uint(0))] = C.PN_ULONG
+	} else {
+		arrayTypeMap[reflect.TypeOf(int(0))] = C.PN_INT
+		arrayTypeMap[reflect.TypeOf(uint(0))] = C.PN_UINT
+	}
 }
 
 func clearMarshal(v interface{}, data *C.pn_data_t) {

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1659af1f/proton-c/bindings/go/src/qpid.apache.org/amqp/types_test.go
----------------------------------------------------------------------
diff --git a/proton-c/bindings/go/src/qpid.apache.org/amqp/types_test.go b/proton-c/bindings/go/src/qpid.apache.org/amqp/types_test.go
index 9d5a6b6..5dcee2e 100644
--- a/proton-c/bindings/go/src/qpid.apache.org/amqp/types_test.go
+++ b/proton-c/bindings/go/src/qpid.apache.org/amqp/types_test.go
@@ -71,11 +71,11 @@ var rtValues = []interface{}{
 	Char('\u2318'),
 	Map{"V": "X"},
 	List{"V", int32(1)},
-	[]string{"a", "b", "c"}, // to AMQP array
+	[]string{"a", "b", "c"},
 }
 
-// Go values that unmarshal as an equivalent value but a different default type
-// if unmarshalled to an interface{}
+// Go values that round-trip if unmarshalled back to the same type they were
+// marshalled from, but unmarshal to interface{} as a different default type.
 var oddValues = []interface{}{
 	int(-99),                  // int32|64 depending on platform
 	uint(99),                  // int32|64 depending on platform

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/1659af1f/proton-c/bindings/go/src/qpid.apache.org/amqp/unmarshal.go
----------------------------------------------------------------------
diff --git a/proton-c/bindings/go/src/qpid.apache.org/amqp/unmarshal.go b/proton-c/bindings/go/src/qpid.apache.org/amqp/unmarshal.go
index 04917a5..2fe5672 100644
--- a/proton-c/bindings/go/src/qpid.apache.org/amqp/unmarshal.go
+++ b/proton-c/bindings/go/src/qpid.apache.org/amqp/unmarshal.go
@@ -174,19 +174,19 @@ type as follows:
  |map[K]T                     |map, provided all keys and values can unmarshal   |
  |                            |to types K,T                                      |
  +----------------------------+--------------------------------------------------+
- |[]interface{}               |AMQP list or array                                |
+ |[]interface{}               |list or array                                     |
  +----------------------------+--------------------------------------------------+
- |[]T                         |AMQP list or array if elements can unmarshal as T |
+ |[]T                         |list or array if elements can unmarshal as T      |
  +----------------------------+------------------n-------------------------------+
  |interface{}                 |any AMQP type[2]                                  |
  +----------------------------+--------------------------------------------------+
 
-[1] An AMQP described value can also convert as if it were a plain value,
-discarding the descriptor. Unmarshalling into the special amqp.Described type
-preserves the descriptor.
+[1] An AMQP described value can also unmarshal to a plain value, discarding the
+descriptor. Unmarshalling into the special amqp.Described type preserves the
+descriptor.
 
 [2] Any AMQP value can be unmarshalled to an interface{}. The Go type is
-chosen based on the AMQP type as follows:
+determined by the AMQP type as follows:
 
  +----------------------------+--------------------------------------------------+
  |Source AMQP Type            |Go Type in target interface{}                     |


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org