types: reduce duplicative testing of JSON unmarshaling

Signed-off-by: Patrick Jakubowski <[email protected]>
cedar-policy · Nov 12, 2024 · 3b282ac · 3b282ac
1 parent c95593a
commit 3b282ac
Show file tree

Hide file tree

Showing 6 changed files with 165 additions and 446 deletions.
diff --git a/types/datetime_test.go b/types/datetime_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"testing"
 	"time"
@@ -197,8 +198,25 @@ func TestDatetime(t *testing.T) {
 
 	t.Run("MarshalJSON", func(t *testing.T) {
 		t.Parallel()
-		bs, err := types.NewDatetime(time.UnixMilli(42)).MarshalJSON()
+		expected := `{
+			"__extn": {
+				"fn": "datetime",
+				"arg": "1970-01-01T00:00:00.042Z"
+			}
+		}`
+		dt1 := types.NewDatetime(time.UnixMilli(42))
+		testutil.JSONMarshalsTo(t, dt1, expected)
+
+		var dt2 types.Datetime
+		err := json.Unmarshal([]byte(expected), &dt2)
 		testutil.OK(t, err)
-		testutil.Equals(t, string(bs), `{"__extn":{"fn":"datetime","arg":"1970-01-01T00:00:00.042Z"}}`)
+		testutil.Equals(t, dt1, dt2)
+	})
+
+	t.Run("UnmarshalJSON/error", func(t *testing.T) {
+		t.Parallel()
+		var dt2 types.Datetime
+		err := json.Unmarshal([]byte("{}"), &dt2)
+		testutil.Error(t, err)
 	})
 }
diff --git a/types/decimal_test.go b/types/decimal_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"testing"
 
@@ -287,4 +288,28 @@ func TestDecimal(t *testing.T) {
 			string(testutil.Must(types.NewDecimal(42, 0)).MarshalCedar()),
 			`decimal("42.0")`)
 	})
+
+	t.Run("MarshalJSON", func(t *testing.T) {
+		t.Parallel()
+		expected := `{
+			"__extn": {
+				"fn": "decimal",
+				"arg": "1234.5678"
+			}
+		}`
+		d1 := testutil.Must(types.NewDecimal(12345678, -4))
+		testutil.JSONMarshalsTo(t, d1, expected)
+
+		var d2 types.Decimal
+		err := json.Unmarshal([]byte(expected), &d2)
+		testutil.OK(t, err)
+		testutil.Equals(t, d1, d2)
+	})
+
+	t.Run("UnmarshalJSON/error", func(t *testing.T) {
+		t.Parallel()
+		var dt2 types.Decimal
+		err := json.Unmarshal([]byte("{}"), &dt2)
+		testutil.Error(t, err)
+	})
 }
diff --git a/types/duration_test.go b/types/duration_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"testing"
 	"time"
@@ -176,8 +177,25 @@ func TestDuration(t *testing.T) {
 
 	t.Run("MarshalJSON", func(t *testing.T) {
 		t.Parallel()
-		bs, err := types.NewDuration(42 * time.Millisecond).MarshalJSON()
+		expected := `{
+			"__extn": {
+				"fn": "duration",
+				"arg": "42ms"
+			}
+		}`
+		d1 := types.NewDuration(42 * time.Millisecond)
+		testutil.JSONMarshalsTo(t, d1, expected)
+
+		var d2 types.Duration
+		err := json.Unmarshal([]byte(expected), &d2)
 		testutil.OK(t, err)
-		testutil.Equals(t, string(bs), `{"__extn":{"fn":"duration","arg":"42ms"}}`)
+		testutil.Equals(t, d1, d2)
+	})
+
+	t.Run("UnmarshalJSON/error", func(t *testing.T) {
+		t.Parallel()
+		var dt2 types.Duration
+		err := json.Unmarshal([]byte("{}"), &dt2)
+		testutil.Error(t, err)
 	})
 }
diff --git a/types/ipaddr_test.go b/types/ipaddr_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"testing"
 
@@ -277,4 +278,27 @@ func TestIP(t *testing.T) {
 			`ip("10.0.0.42")`)
 	})
 
+	t.Run("MarshalJSON", func(t *testing.T) {
+		t.Parallel()
+		expected := `{
+			"__extn": {
+				"fn": "ip",
+				"arg": "12.34.56.78"
+			}
+		}`
+		i1 := testutil.Must(types.ParseIPAddr("12.34.56.78"))
+		testutil.JSONMarshalsTo(t, i1, expected)
+
+		var i2 types.IPAddr
+		err := json.Unmarshal([]byte(expected), &i2)
+		testutil.OK(t, err)
+		testutil.Equals(t, i1, i2)
+	})
+
+	t.Run("UnmarshalJSON/error", func(t *testing.T) {
+		t.Parallel()
+		var dt2 types.IPAddr
+		err := json.Unmarshal([]byte("{}"), &dt2)
+		testutil.Error(t, err)
+	})
 }
diff --git a/types/json.go b/types/json.go
@@ -143,10 +143,9 @@ func unmarshalExtensionValue[T any](b []byte, extName string, parse func(string)
 			// If we didn't find an Extn, maybe it's just an extn.
 			var res2 extn
 
-			// N.B. we already know that this JSON unmarshals successfully as a struct, so it's
-			// not actually possible to get an error here. Hence, it's safe to ignore the return
-			// value.
-			_ = json.Unmarshal(b, &res2)
+			if err := json.Unmarshal(b, &res2); err != nil {
+				return zeroT, errors.Join(errJSONDecode, err)
+			}
 
 			// We've tried Ext.Fn and Fn, so no good.
 			if res2.Fn == "" {