fixup! feat: introduce UIntNode interface, used within DAG-CBOR codec

ipld · Jun 8, 2022 · adcc36d · adcc36d
1 parent 633915c
commit adcc36d
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 64 deletions.
diff --git a/codec/dagcbor/roundtrip_test.go b/codec/dagcbor/roundtrip_test.go
@@ -120,62 +120,63 @@ func TestRoundtripLinksAndBytes(t *testing.T) {
 }
 
 func TestInts(t *testing.T) {
-	t.Run("max uint64", func(t *testing.T) {
-		buf, err := hex.DecodeString("1bffffffffffffffff") // max uint64
-		qt.Assert(t, err, qt.IsNil)
-		nb := basicnode.Prototype.Any.NewBuilder()
-		err = Decode(nb, bytes.NewReader(buf))
-		qt.Assert(t, err, qt.IsNil)
-		n := nb.Build()
-
-		// the overflowed AsInt() int64 cast
-		_, err = n.AsInt()
-		qt.Assert(t, err.Error(), qt.Equals, "unsigned integer out of range of int64 type")
-
-		// get real, underlying value
-		uin, ok := n.(datamodel.UintNode)
-		qt.Assert(t, ok, qt.IsTrue)
-		val, err := uin.AsUint()
-		qt.Assert(t, err, qt.IsNil)
-		qt.Assert(t, val, qt.Equals, uint64(math.MaxUint64))
-
-		var byts bytes.Buffer
-		err = Encode(n, &byts)
-		qt.Assert(t, err, qt.IsNil)
-		qt.Assert(t, hex.EncodeToString(byts.Bytes()), qt.Equals, "1bffffffffffffffff")
-	})
-
-	t.Run("max int64", func(t *testing.T) {
-		buf, err := hex.DecodeString("1b7fffffffffffffff") // max int64
-		qt.Assert(t, err, qt.IsNil)
-		nb := basicnode.Prototype.Any.NewBuilder()
-		err = Decode(nb, bytes.NewReader(buf))
-		qt.Assert(t, err, qt.IsNil)
-		n := nb.Build()
-
-		ii, err := n.AsInt()
-		qt.Assert(t, err, qt.IsNil)
-		qt.Assert(t, ii, qt.Equals, int64(math.MaxInt64))
-
-		// doesn't need to be a uint
-		_, ok := n.(datamodel.UintNode)
-		qt.Assert(t, ok, qt.IsFalse)
-	})
-
-	t.Run("min int64", func(t *testing.T) {
-		buf, err := hex.DecodeString("3b7fffffffffffffff") // min int64
-		qt.Assert(t, err, qt.IsNil)
-		nb := basicnode.Prototype.Any.NewBuilder()
-		err = Decode(nb, bytes.NewReader(buf))
-		qt.Assert(t, err, qt.IsNil)
-		n := nb.Build()
-
-		ii, err := n.AsInt()
-		qt.Assert(t, err, qt.IsNil)
-		qt.Assert(t, ii, qt.Equals, int64(math.MinInt64))
-
-		// doesn't need to be a uint
-		_, ok := n.(datamodel.UintNode)
-		qt.Assert(t, ok, qt.IsFalse)
-	})
+	data := []struct {
+		name      string
+		hex       string
+		value     uint64
+		intValue  int64
+		intErr    string
+		decodeErr string
+	}{
+		{"max uint64", "1bffffffffffffffff", math.MaxUint64, 0, "unsigned integer out of range of int64 type", ""},
+		{"max int64", "1b7fffffffffffffff", math.MaxInt64, math.MaxInt64, "", ""},
+		{"1", "01", 1, 1, "", ""},
+		{"0", "00", 0, 0, "", ""},
+		{"-1", "20", 0, -1, "", ""},
+		{"min int64", "3b7fffffffffffffff", 0, math.MinInt64, "", ""},
+		{"~min uint64", "3bfffffffffffffffe", 0, 0, "", "cbor: negative integer out of rage of int64 type"},
+		// TODO: 3bffffffffffffffff isn't properly handled by refmt, it's coerced to zero
+		// MaxUint64 gets overflowed here: https://github.com/polydawn/refmt/blob/30ac6d18308e584ca6a2e74ba81475559db94c5f/cbor/cborDecoderTerminals.go#L75
+	}
+
+	for _, td := range data {
+		t.Run(td.name, func(t *testing.T) {
+			buf, err := hex.DecodeString(td.hex) // max uint64
+			qt.Assert(t, err, qt.IsNil)
+			nb := basicnode.Prototype.Any.NewBuilder()
+			err = Decode(nb, bytes.NewReader(buf))
+			if td.decodeErr != "" {
+				qt.Assert(t, err, qt.IsNotNil)
+				qt.Assert(t, err.Error(), qt.Equals, td.decodeErr)
+				return
+			}
+			qt.Assert(t, err, qt.IsNil)
+			n := nb.Build()
+
+			ii, err := n.AsInt()
+			if td.intErr != "" {
+				qt.Assert(t, err.Error(), qt.Equals, td.intErr)
+			} else {
+				qt.Assert(t, err, qt.IsNil)
+				qt.Assert(t, ii, qt.Equals, int64(td.intValue))
+			}
+
+			// if it's a positive number, we should be able to access it as a UintNode
+			// and be able to access the full int64 range
+			uin, ok := n.(datamodel.UintNode)
+			if td.intValue < 0 {
+				qt.Assert(t, ok, qt.IsFalse)
+			} else {
+				qt.Assert(t, ok, qt.IsTrue)
+				val, err := uin.AsUint()
+				qt.Assert(t, err, qt.IsNil)
+				qt.Assert(t, val, qt.Equals, uint64(td.value))
+			}
+
+			var byts bytes.Buffer
+			err = Encode(n, &byts)
+			qt.Assert(t, err, qt.IsNil)
+			qt.Assert(t, hex.EncodeToString(byts.Bytes()), qt.Equals, td.hex)
+		})
+	}
 }
diff --git a/codec/dagcbor/unmarshal.go b/codec/dagcbor/unmarshal.go
@@ -276,10 +276,7 @@ func unmarshal2(na datamodel.NodeAssembler, tokSrc shared.TokenSource, tk *tok.T
 		if *gas < 0 {
 			return ErrAllocationBudgetExceeded
 		}
-		if tk.Uint > uint64(math.MaxInt64) {
-			return na.AssignNode(basicnode.NewUInt(tk.Uint))
-		}
-		return na.AssignInt(int64(tk.Uint))
+		return na.AssignNode(basicnode.NewUint(tk.Uint))
 	case tok.TFloat64:
 		*gas -= 1
 		if *gas < 0 {

diff --git a/node/basicnode/int.go b/node/basicnode/int.go
@@ -21,13 +21,13 @@ func NewInt(value int64) datamodel.Node {
 	return plainInt(value)
 }
 
-// NewUInt creates a new uint64-backed Node which will behave as a plain Int
+// NewUint creates a new uint64-backed Node which will behave as a plain Int
 // node but also conforms to the datamodel.UintNode interface which can access
 // the full uint64 range.
 //
 // EXPERIMENTAL: this API is experimental and may be changed or removed in a
 // future release.
-func NewUInt(value uint64) datamodel.Node {
+func NewUint(value uint64) datamodel.Node {
 	return plainUint(value)
 }