Browse Source

codec: Add support for decoding byte slices, even if encoded as a regular array.

This requires us checking currentEncodedType in the stream first, before
deciding how to decode the value when decoding into a slice.

There's a slight performance hit, but correctness is more important.

Also, put in flag to "conditionally" enable encoding uint as positive fixint,
and update tests so they work across binc and msgpack and regardless of flag.
(This means ensuring that all unsigned int values in test are > 128).
(I probably will not enable this flag by default, but it's there ... for placeholding and reminder).

Fixes issue #26 .

Updates issue #24 .
Ugorji Nwoke 12 years ago
parent
commit
10b6078349
4 changed files with 68 additions and 36 deletions
  1. 20 5
      codec/codecs_test.go
  2. 39 28
      codec/decode.go
  3. 2 0
      codec/helper.go
  4. 7 3
      codec/msgpack.go

+ 20 - 5
codec/codecs_test.go

@@ -19,6 +19,8 @@ package codec
 // Some hints:
 // Some hints:
 // - python msgpack encodes positive numbers as uints, so use uints below
 // - python msgpack encodes positive numbers as uints, so use uints below
 //   for positive numbers.
 //   for positive numbers.
+// - flag mpEncodeUintAsFixnum allows uints be encoded as fixnums.
+//   To accomodate it being true or false, ensure all unsigned values below are > 128.
 
 
 import (
 import (
 	"bytes"
 	"bytes"
@@ -350,8 +352,8 @@ func testInit() {
 		},
 		},
 		map[interface{}]interface{}{
 		map[interface{}]interface{}{
 			true:     "true",
 			true:     "true",
-			uint8(8): false,
-			"false":  uint8(0),
+			uint8(138): false,
+			"false":  uint8(200),
 		},
 		},
 		newTestStruc(0, false),
 		newTestStruc(0, false),
 	}
 	}
@@ -439,12 +441,12 @@ func newTestStruc(depth int, bench bool) (ts *TestStruc) {
 		Sslice:    []string{"one", "two", "three"},
 		Sslice:    []string{"one", "two", "three"},
 		I64slice:  []int64{1, 2, 3},
 		I64slice:  []int64{1, 2, 3},
 		I16slice:  []int16{4, 5, 6},
 		I16slice:  []int16{4, 5, 6},
-		Ui64slice: []uint64{7, 8, 9},
-		Ui8slice:  []uint8{10, 11, 12},
+		Ui64slice: []uint64{137, 138, 139},
+		Ui8slice:  []uint8{210, 211, 212},
 		Bslice:    []bool{true, false, true, false},
 		Bslice:    []bool{true, false, true, false},
 		Byslice:   []byte{13, 14, 15},
 		Byslice:   []byte{13, 14, 15},
 
 
-		Islice: []interface{}{"true", true, "no", false, uint64(88), float64(0.4)},
+		Islice: []interface{}{"true", true, "no", false, uint64(288), float64(0.4)},
 
 
 		Ms: map[string]interface{}{
 		Ms: map[string]interface{}{
 			"true":     "true",
 			"true":     "true",
@@ -723,6 +725,19 @@ func testCodecMiscOne(t *testing.T, h Handle) {
 		}
 		}
 		// fmt.Printf(">>>> err: %v. tarr1: %v, tarr2: %v\n", err, tarr0, tarr2)
 		// fmt.Printf(">>>> err: %v. tarr1: %v, tarr2: %v\n", err, tarr0, tarr2)
 	}
 	}
+
+	// test byte array, even if empty (msgpack only)
+	if h == testMsgpackH {
+		type ystruct struct {
+			Anarray []byte
+		}
+		var ya = ystruct{}
+		err = testUnmarshal(&ya, []byte{0x91, 0x90}, h)
+		if err != nil {
+			logT(t, "Error unmarshalling into ystruct: %v, Err: %v", ya, err)
+			t.FailNow()
+		}
+	}
 }
 }
 
 
 func testCodecEmbeddedPointer(t *testing.T, h Handle) {
 func testCodecEmbeddedPointer(t *testing.T, h Handle) {

+ 39 - 28
codec/decode.go

@@ -414,42 +414,47 @@ func (f *decFnInfo) kStruct(rv reflect.Value) {
 
 
 func (f *decFnInfo) kSlice(rv reflect.Value) {
 func (f *decFnInfo) kSlice(rv reflect.Value) {
 	// A slice can be set from a map or array in stream.
 	// A slice can be set from a map or array in stream.
-
+	currEncodedType := f.dd.currentEncodedType()
+	
+	switch currEncodedType {
+	case valueTypeBytes, valueTypeString:
+		if f.ti.rtid == uint8SliceTypId || f.ti.rt.Elem().Kind() == reflect.Uint8 { 
+			if bs2, changed2 := f.dd.decodeBytes(rv.Bytes()); changed2 {
+				rv.SetBytes(bs2)
+			}
+			return
+		}
+	}
+	
 	if shortCircuitReflectToFastPath && rv.CanAddr() {
 	if shortCircuitReflectToFastPath && rv.CanAddr() {
 		switch f.ti.rtid {
 		switch f.ti.rtid {
 		case intfSliceTypId:
 		case intfSliceTypId:
-			f.d.decSliceIntf(rv.Addr().Interface().(*[]interface{}), f.array)
+			f.d.decSliceIntf(rv.Addr().Interface().(*[]interface{}), currEncodedType, f.array)
 			return
 			return
 		case uint64SliceTypId:
 		case uint64SliceTypId:
-			f.d.decSliceUint64(rv.Addr().Interface().(*[]uint64), f.array)
+			f.d.decSliceUint64(rv.Addr().Interface().(*[]uint64), currEncodedType, f.array)
 			return
 			return
 		case int64SliceTypId:
 		case int64SliceTypId:
-			f.d.decSliceInt64(rv.Addr().Interface().(*[]int64), f.array)
+			f.d.decSliceInt64(rv.Addr().Interface().(*[]int64), currEncodedType, f.array)
 			return
 			return
 		case strSliceTypId:
 		case strSliceTypId:
-			f.d.decSliceStr(rv.Addr().Interface().(*[]string), f.array)
+			f.d.decSliceStr(rv.Addr().Interface().(*[]string), currEncodedType, f.array)
 			return
 			return
 		}
 		}
 	}
 	}
 
 
-	if f.ti.rtid == uint8SliceTypId || f.ti.rt.Elem().Kind() == reflect.Uint8 { 
-		if bs2, changed2 := f.dd.decodeBytes(rv.Bytes()); changed2 {
-			rv.SetBytes(bs2)
-		}
-		return
-	}
-
-	containerLen, containerLenS := decContLens(f.dd)
+	containerLen, containerLenS := decContLens(f.dd, currEncodedType)
 
 
 	// an array can never return a nil slice. so no need to check f.array here.
 	// an array can never return a nil slice. so no need to check f.array here.
 
 
 	if rv.IsNil() {
 	if rv.IsNil() {
 		rv.Set(reflect.MakeSlice(f.ti.rt, containerLenS, containerLenS))
 		rv.Set(reflect.MakeSlice(f.ti.rt, containerLenS, containerLenS))
 	}
 	}
+	
 	if containerLen == 0 {
 	if containerLen == 0 {
 		return
 		return
 	}
 	}
-
+	
 	if rvcap, rvlen := rv.Len(), rv.Cap(); containerLenS > rvcap {
 	if rvcap, rvlen := rv.Len(), rv.Cap(); containerLenS > rvcap {
 		if f.array { // !rv.CanSet()
 		if f.array { // !rv.CanSet()
 			decErr(msgDecCannotExpandArr, rvcap, containerLenS)
 			decErr(msgDecCannotExpandArr, rvcap, containerLenS)
@@ -664,13 +669,13 @@ func (d *Decoder) decode(iv interface{}) {
 		*v, _ = d.d.decodeBytes(v2)
 		*v, _ = d.d.decodeBytes(v2)
 
 
 	case *[]interface{}:
 	case *[]interface{}:
-		d.decSliceIntf(v, false)
+		d.decSliceIntf(v, valueTypeInvalid, false)
 	case *[]uint64:
 	case *[]uint64:
-		d.decSliceUint64(v, false)
+		d.decSliceUint64(v, valueTypeInvalid, false)
 	case *[]int64:
 	case *[]int64:
-		d.decSliceInt64(v, false)
+		d.decSliceInt64(v, valueTypeInvalid, false)
 	case *[]string:
 	case *[]string:
-		d.decSliceStr(v, false)
+		d.decSliceStr(v, valueTypeInvalid, false)
 	case *map[string]interface{}:
 	case *map[string]interface{}:
 		d.decMapStrIntf(v)
 		d.decMapStrIntf(v)
 	case *map[interface{}]interface{}:
 	case *map[interface{}]interface{}:
@@ -853,8 +858,8 @@ func (d *Decoder) decEmbeddedField(rv reflect.Value, index []int) {
 
 
 // short circuit functions for common maps and slices
 // short circuit functions for common maps and slices
 
 
-func (d *Decoder) decSliceIntf(v *[]interface{}, doNotReset bool) {
-	_, containerLenS := decContLens(d.d)
+func (d *Decoder) decSliceIntf(v *[]interface{}, currEncodedType valueType, doNotReset bool) {
+	_, containerLenS := decContLens(d.d, currEncodedType)
 	s := *v
 	s := *v
 	if s == nil {
 	if s == nil {
 		s = make([]interface{}, containerLenS, containerLenS)
 		s = make([]interface{}, containerLenS, containerLenS)
@@ -873,8 +878,8 @@ func (d *Decoder) decSliceIntf(v *[]interface{}, doNotReset bool) {
 	*v = s
 	*v = s
 }
 }
 
 
-func (d *Decoder) decSliceInt64(v *[]int64, doNotReset bool) {
-	_, containerLenS := decContLens(d.d)
+func (d *Decoder) decSliceInt64(v *[]int64, currEncodedType valueType, doNotReset bool) {
+	_, containerLenS := decContLens(d.d, currEncodedType)
 	s := *v
 	s := *v
 	if s == nil {
 	if s == nil {
 		s = make([]int64, containerLenS, containerLenS)
 		s = make([]int64, containerLenS, containerLenS)
@@ -895,8 +900,8 @@ func (d *Decoder) decSliceInt64(v *[]int64, doNotReset bool) {
 	*v = s
 	*v = s
 }
 }
 
 
-func (d *Decoder) decSliceUint64(v *[]uint64, doNotReset bool) {
-	_, containerLenS := decContLens(d.d)
+func (d *Decoder) decSliceUint64(v *[]uint64, currEncodedType valueType, doNotReset bool) {
+	_, containerLenS := decContLens(d.d, currEncodedType)
 	s := *v
 	s := *v
 	if s == nil {
 	if s == nil {
 		s = make([]uint64, containerLenS, containerLenS)
 		s = make([]uint64, containerLenS, containerLenS)
@@ -917,8 +922,8 @@ func (d *Decoder) decSliceUint64(v *[]uint64, doNotReset bool) {
 	*v = s
 	*v = s
 }
 }
 
 
-func (d *Decoder) decSliceStr(v *[]string, doNotReset bool) {
-	_, containerLenS := decContLens(d.d)
+func (d *Decoder) decSliceStr(v *[]string, currEncodedType valueType, doNotReset bool) {
+	_, containerLenS := decContLens(d.d, currEncodedType)
 	s := *v
 	s := *v
 	if s == nil {
 	if s == nil {
 		s = make([]string, containerLenS, containerLenS)
 		s = make([]string, containerLenS, containerLenS)
@@ -1009,8 +1014,11 @@ func (d *Decoder) decMapStrIntf(v *map[string]interface{}) {
 
 
 // ----------------------------------------
 // ----------------------------------------
 
 
-func decContLens(dd decDriver) (containerLen, containerLenS int) {
-	switch currEncodedType := dd.currentEncodedType(); currEncodedType {
+func decContLens(dd decDriver, currEncodedType valueType) (containerLen, containerLenS int) {
+	if currEncodedType == valueTypeInvalid {
+		currEncodedType = dd.currentEncodedType()
+	}
+	switch currEncodedType {
 	case valueTypeArray:
 	case valueTypeArray:
 		containerLen = dd.readArrayLen()
 		containerLen = dd.readArrayLen()
 		containerLenS = containerLen
 		containerLenS = containerLen
@@ -1027,3 +1035,6 @@ func decContLens(dd decDriver) (containerLen, containerLenS int) {
 func decErr(format string, params ...interface{}) {
 func decErr(format string, params ...interface{}) {
 	doPanic(msgTagDec, format, params...)
 	doPanic(msgTagDec, format, params...)
 }
 }
+
+
+

+ 2 - 0
codec/helper.go

@@ -74,6 +74,8 @@ const (
 	valueTypeArray
 	valueTypeArray
 	valueTypeTimestamp
 	valueTypeTimestamp
 	valueTypeExt
 	valueTypeExt
+	
+	valueTypeInvalid = 0xff
 )
 )
 
 
 var (
 var (

+ 7 - 3
codec/msgpack.go

@@ -61,6 +61,11 @@ const (
 	mpNegFixNumMax = 0xff
 	mpNegFixNumMax = 0xff
 )
 )
 
 
+// TODO: Should I enable this, which causes small uints be encoded as fixnums?
+// uints are not fixnums. fixnums are always signed.
+// conditionally support them (but keep flag off for compatibility).
+const mpEncodeUintAsFixnum = false 
+
 // MsgpackSpecRpcMultiArgs is a special type which signifies to the MsgpackSpecRpcCodec
 // MsgpackSpecRpcMultiArgs is a special type which signifies to the MsgpackSpecRpcCodec
 // that the backend RPC service takes multiple arguments, which have been arranged
 // that the backend RPC service takes multiple arguments, which have been arranged
 // in sequence in the slice.
 // in sequence in the slice.
@@ -122,10 +127,9 @@ func (e *msgpackEncDriver) encodeInt(i int64) {
 }
 }
 
 
 func (e *msgpackEncDriver) encodeUint(i uint64) {
 func (e *msgpackEncDriver) encodeUint(i uint64) {
-	// uints are not fixnums. fixnums are always signed.
-	// case i <= math.MaxInt8:
-	// 	e.w.writen1(byte(i))
 	switch {
 	switch {
+	case mpEncodeUintAsFixnum && i <= math.MaxInt8:
+		e.w.writen1(byte(i))
 	case i <= math.MaxUint8:
 	case i <= math.MaxUint8:
 		e.w.writen2(mpUint8, byte(i))
 		e.w.writen2(mpUint8, byte(i))
 	case i <= math.MaxUint16:
 	case i <= math.MaxUint16: