Skip to content

Commit

Permalink
types: Invert the special casing of JSON marshaling for Value
Browse files Browse the repository at this point in the history
Entities and extension functions have both an "implict" and "explict" form of JSON serialization, where the "implicit" form is only valid if the serialized data can be unambiguously determined to be of a given type. This can happen positionally (e.g. EntityUIDs in the serialization of the Entities object) or via knowledge gleaned from a schema (which we don't yet support).

It's _always_ safe to encode an entity or extension function via the "explicit" form, so let's make that the default. This also alleviates the types that don't have implicit and explicit forms from having to implement ExplicitMarshalJSON.

With this approach, callers who know that it's safe to use the implicit form can do so and everyone else will get the safe explicit form. The two existing callers that use the implicit form are the JSON serialization of Entities and the scope operators which always take EntityUIDs. I've updated these to use the implicit form, which matches the behavior of the Rust SDK.

At this time, I don't really see any reason to ever encode extension functions via the implicit form. Even if we had schema support, it just seems to save a few bytes and makes the JSON encoding mildly more legible, which shouldn't really be a goal of the JSON. FWIW, the Rust SDK doesn't seem to ever encode extension functions via the implicit form.

Signed-off-by: Patrick Jakubowski <patrick.jakubowski@strongdm.com>
  • Loading branch information
patjakdev committed Sep 21, 2024
1 parent 981a06d commit d230845
Show file tree
Hide file tree
Showing 20 changed files with 129 additions and 142 deletions.
19 changes: 12 additions & 7 deletions internal/json/json.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package json

import (
"encoding/json"

"github.com/cedar-policy/cedar-go/types"
)

Expand All @@ -13,16 +15,18 @@ type policyJSON struct {
Conditions []conditionJSON `json:"conditions,omitempty"`
}

// scopeInJSON uses the implicit form of EntityUID JSON serialization to match the Rust SDK
type scopeInJSON struct {
Entity types.EntityUID `json:"entity"`
Entity types.ImplicitlyMarshaledEntityUID `json:"entity"`
}

// scopeJSON uses the implicit form of EntityUID JSON serialization to match the Rust SDK
type scopeJSON struct {
Op string `json:"op"`
Entity *types.EntityUID `json:"entity,omitempty"`
Entities []types.EntityUID `json:"entities,omitempty"`
EntityType string `json:"entity_type,omitempty"`
In *scopeInJSON `json:"in,omitempty"`
Op string `json:"op"`
Entity *types.ImplicitlyMarshaledEntityUID `json:"entity,omitempty"`
Entities []types.ImplicitlyMarshaledEntityUID `json:"entities,omitempty"`
EntityType string `json:"entity_type,omitempty"`
In *scopeInJSON `json:"in,omitempty"`
}

type conditionJSON struct {
Expand Down Expand Up @@ -72,8 +76,9 @@ type valueJSON struct {
}

func (e *valueJSON) MarshalJSON() ([]byte, error) {
return e.v.ExplicitMarshalJSON()
return json.Marshal(e.v)
}

func (e *valueJSON) UnmarshalJSON(b []byte) error {
return types.UnmarshalJSON(b, &e.v)
}
Expand Down
12 changes: 8 additions & 4 deletions internal/json/json_marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@ func (s *scopeJSON) FromNode(src ast.IsScopeNode) {
return
case ast.ScopeTypeEq:
s.Op = "=="
e := t.Entity
e := types.ImplicitlyMarshaledEntityUID(t.Entity)
s.Entity = &e
return
case ast.ScopeTypeIn:
s.Op = "in"
e := t.Entity
e := types.ImplicitlyMarshaledEntityUID(t.Entity)
s.Entity = &e
return
case ast.ScopeTypeInSet:
s.Op = "in"
s.Entities = t.Entities
es := make([]types.ImplicitlyMarshaledEntityUID, len(t.Entities))
for i, e := range t.Entities {
es[i] = types.ImplicitlyMarshaledEntityUID(e)
}
s.Entities = es
return
case ast.ScopeTypeIs:
s.Op = "is"
Expand All @@ -35,7 +39,7 @@ func (s *scopeJSON) FromNode(src ast.IsScopeNode) {
s.Op = "is"
s.EntityType = string(t.Type)
s.In = &scopeInJSON{
Entity: t.Entity,
Entity: types.ImplicitlyMarshaledEntityUID(t.Entity),
}
return
default:
Expand Down
16 changes: 10 additions & 6 deletions internal/json/json_unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ func (s *scopeJSON) ToPrincipalResourceNode() (isPrincipalResourceScopeNode, err
if s.Entity == nil {
return nil, fmt.Errorf("missing entity")
}
return ast.Scope{}.Eq(*s.Entity), nil
return ast.Scope{}.Eq(types.EntityUID(*s.Entity)), nil
case "in":
if s.Entity == nil {
return nil, fmt.Errorf("missing entity")
}
return ast.Scope{}.In(*s.Entity), nil
return ast.Scope{}.In(types.EntityUID(*s.Entity)), nil
case "is":
if s.In == nil {
return ast.Scope{}.Is(types.EntityType(s.EntityType)), nil
}
return ast.Scope{}.IsIn(types.EntityType(s.EntityType), s.In.Entity), nil
return ast.Scope{}.IsIn(types.EntityType(s.EntityType), types.EntityUID(s.In.Entity)), nil
}
return nil, fmt.Errorf("unknown op: %v", s.Op)
}
Expand All @@ -48,12 +48,16 @@ func (s *scopeJSON) ToActionNode() (ast.IsActionScopeNode, error) {
if s.Entity == nil {
return nil, fmt.Errorf("missing entity")
}
return ast.Scope{}.Eq(*s.Entity), nil
return ast.Scope{}.Eq(types.EntityUID(*s.Entity)), nil
case "in":
if s.Entity != nil {
return ast.Scope{}.In(*s.Entity), nil
return ast.Scope{}.In(types.EntityUID(*s.Entity)), nil
}
return ast.Scope{}.InSet(s.Entities), nil
es := make([]types.EntityUID, len(s.Entities))
for i, e := range s.Entities {
es[i] = types.EntityUID(e)
}
return ast.Scope{}.InSet(es), nil
}
return nil, fmt.Errorf("unknown op: %v", s.Op)
}
Expand Down
7 changes: 0 additions & 7 deletions types/boolean.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package types

import (
"encoding/json"
)

// A Boolean is a value that is either true or false.
type Boolean bool

Expand All @@ -28,9 +24,6 @@ func (v Boolean) MarshalCedar() []byte {
return []byte("false")
}

// ExplicitMarshalJSON marshals the Boolean into JSON.
func (v Boolean) ExplicitMarshalJSON() ([]byte, error) { return json.Marshal(v) }

func (v Boolean) hash() uint64 {
if v {
return 1
Expand Down
10 changes: 1 addition & 9 deletions types/datetime.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,16 +286,8 @@ func (a *Datetime) UnmarshalJSON(b []byte) error {
return nil
}

// MarshalJSON implements the encoding/json.Marshaler interface
//
// It produces the direct representation of a Cedar Datetime.
// MarshalJSON Marshal's a Cedar Datetime with the explicit representation
func (a Datetime) MarshalJSON() ([]byte, error) {
return []byte(`datetime("` + a.String() + `")`), nil
}

// ExplicitMarshalJSON Marshal's a Cedar Datetime with the explicit
// representation
func (a Datetime) ExplicitMarshalJSON() ([]byte, error) {
return json.Marshal(extValueJSON{
Extn: &extn{
Fn: "datetime",
Expand Down
2 changes: 1 addition & 1 deletion types/datetime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,6 @@ func TestDatetime(t *testing.T) {
t.Parallel()
bs, err := types.FromStdTime(time.UnixMilli(42)).MarshalJSON()
testutil.OK(t, err)
testutil.Equals(t, string(bs), `datetime("1970-01-01T00:00:00.042Z")`)
testutil.Equals(t, string(bs), `{"__extn":{"fn":"datetime","arg":"1970-01-01T00:00:00.042Z"}}`)
})
}
7 changes: 2 additions & 5 deletions types/decimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,11 +176,8 @@ func (v *Decimal) UnmarshalJSON(b []byte) error {
return nil
}

// ExplicitMarshalJSON marshals the Decimal into JSON using the implicit form.
func (v Decimal) MarshalJSON() ([]byte, error) { return []byte(`"` + v.String() + `"`), nil }

// ExplicitMarshalJSON marshals the Decimal into JSON using the explicit form.
func (v Decimal) ExplicitMarshalJSON() ([]byte, error) {
// MarshalJSON marshals the Decimal into JSON using the explicit form.
func (v Decimal) MarshalJSON() ([]byte, error) {
return json.Marshal(extValueJSON{
Extn: &extn{
Fn: "decimal",
Expand Down
7 changes: 2 additions & 5 deletions types/duration.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,11 +255,8 @@ func (v *Duration) UnmarshalJSON(b []byte) error {
return nil
}

// MarshalJSON marshals the Duration into JSON using the implicit form.
func (v Duration) MarshalJSON() ([]byte, error) { return []byte(`"` + v.String() + `"`), nil }

// ExplicitMarshalJSON marshals the Decimal into JSON using the explicit form.
func (v Duration) ExplicitMarshalJSON() ([]byte, error) {
// MarshalJSON marshals the Duration into JSON using the explicit form.
func (v Duration) MarshalJSON() ([]byte, error) {
return json.Marshal(extValueJSON{
Extn: &extn{
Fn: "duration",
Expand Down
3 changes: 1 addition & 2 deletions types/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ func TestDuration(t *testing.T) {
t.Parallel()
bs, err := types.FromStdDuration(42 * time.Millisecond).MarshalJSON()
testutil.OK(t, err)
testutil.Equals(t, string(bs), `"42ms"`)
testutil.Equals(t, string(bs), `{"__extn":{"fn":"duration","arg":"42ms"}}`)
})

}
22 changes: 21 additions & 1 deletion types/entities.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,30 @@ type Entities map[EntityUID]*Entity
// An Entity defines the parents and attributes for an EntityUID.
type Entity struct {
UID EntityUID `json:"uid"`
Parents []EntityUID `json:"parents,omitempty"`
Parents []EntityUID `json:"parents"`
Attributes Record `json:"attrs"`
}

// MarshalJSON serializes Entity as a JSON object, using the implicit form of EntityUID encoding to match the Rust
// SDK's behavior.
func (e Entity) MarshalJSON() ([]byte, error) {
parents := make([]ImplicitlyMarshaledEntityUID, len(e.Parents))
for i, p := range e.Parents {
parents[i] = ImplicitlyMarshaledEntityUID(p)
}

m := struct {
UID ImplicitlyMarshaledEntityUID `json:"uid"`
Parents []ImplicitlyMarshaledEntityUID `json:"parents"`
Attributes Record `json:"attrs"`
}{
ImplicitlyMarshaledEntityUID(e.UID),
parents,
e.Attributes,
}
return json.Marshal(m)
}

func (e Entities) MarshalJSON() ([]byte, error) {
s := maps.Values(e)
slices.SortFunc(s, func(a, b *Entity) int {
Expand Down
2 changes: 1 addition & 1 deletion types/entities_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestEntitiesJSON(t *testing.T) {
e[ent2.UID] = ent2
b, err := e.MarshalJSON()
testutil.OK(t, err)
testutil.Equals(t, string(b), `[{"uid":{"type":"Type","id":"id"},"attrs":{"key":42}},{"uid":{"type":"Type","id":"id2"},"attrs":{"key":42}}]`)
testutil.Equals(t, string(b), `[{"uid":{"type":"Type","id":"id"},"parents":[],"attrs":{"key":42}},{"uid":{"type":"Type","id":"id2"},"parents":[],"attrs":{"key":42}}]`)
})

t.Run("Unmarshal", func(t *testing.T) {
Expand Down
23 changes: 14 additions & 9 deletions types/entity_uid.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,8 @@ func (v *EntityUID) UnmarshalJSON(b []byte) error {
return errJSONEntityNotFound
}

// ExplicitMarshalJSON marshals the EntityUID into JSON using the implicit form.
// MarshalJSON marshals the EntityUID into JSON using the explicit form.
func (v EntityUID) MarshalJSON() ([]byte, error) {
return json.Marshal(entityValueJSON{
Type: (*string)(&v.Type),
ID: (*string)(&v.ID),
})
}

// ExplicitMarshalJSON marshals the EntityUID into JSON using the explicit form.
func (v EntityUID) ExplicitMarshalJSON() ([]byte, error) {
return json.Marshal(entityValueJSON{
Entity: &extEntity{
Type: string(v.Type),
Expand All @@ -86,3 +78,16 @@ func (v EntityUID) hash() uint64 {
_, _ = h.Write([]byte(v.ID))
return h.Sum64()
}

// ImplicitlyMarshaledEntityUID exists to allow the marshaling of the EntityUID into JSON using the implicit form. Users
// can opt in to this form if they know that this EntityUID will be serialized to a place where its type will be
// unambiguous.
type ImplicitlyMarshaledEntityUID EntityUID

func (i ImplicitlyMarshaledEntityUID) MarshalJSON() ([]byte, error) {
s := struct {
Type EntityType `json:"type"`
ID String `json:"id"`
}{i.Type, i.ID}
return json.Marshal(s)
}
7 changes: 2 additions & 5 deletions types/ipaddr.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,8 @@ func (v *IPAddr) UnmarshalJSON(b []byte) error {
return nil
}

// ExplicitMarshalJSON marshals the IPAddr into JSON using the implicit form.
func (v IPAddr) MarshalJSON() ([]byte, error) { return []byte(`"` + v.String() + `"`), nil }

// ExplicitMarshalJSON marshals the IPAddr into JSON using the explicit form.
func (v IPAddr) ExplicitMarshalJSON() ([]byte, error) {
// MarshalJSON marshals the IPAddr into JSON using the explicit form.
func (v IPAddr) MarshalJSON() ([]byte, error) {
if v.Prefix().Bits() == v.Prefix().Addr().BitLen() {
return json.Marshal(extValueJSON{
Extn: &extn{
Expand Down
Loading

0 comments on commit d230845

Please sign in to comment.