Skip to content

Commit b9c308c

Browse files
philhasseypatjakdev
philhassey
authored and
patjakdev
committed
x/exp/schema: address many nits
Signed-off-by: philhassey <phil@strongdm.com>
1 parent 36c9ace commit b9c308c

File tree

14 files changed

+1079
-1144
lines changed

14 files changed

+1079
-1144
lines changed

‎policy_set.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import (
1111

1212
internaljson "github.com/cedar-policy/cedar-go/internal/json"
1313
"github.com/cedar-policy/cedar-go/types"
14-
internalast "github.com/cedar-policy/cedar-go/x/exp/ast"
14+
"github.com/cedar-policy/cedar-go/x/exp/ast"
1515
)
1616

1717
//revive:disable-next-line:exported
@@ -131,7 +131,7 @@ func (p *PolicySet) UnmarshalJSON(b []byte) error {
131131
policies: make(PolicyMap, len(jsonPolicySet.StaticPolicies)),
132132
}
133133
for k, v := range jsonPolicySet.StaticPolicies {
134-
p.policies[PolicyID(k)] = newPolicy((*internalast.Policy)(v))
134+
p.policies[PolicyID(k)] = newPolicy((*ast.Policy)(v))
135135
}
136136
return nil
137137
}

‎x/exp/schema/ast/ast.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ type CommonTypes map[types.Ident]CommonType
2424
type Namespaces map[types.Path]Namespace
2525

2626
// Schema is the top-level Cedar schema AST.
27+
// The Entities, Enums, Actions, and CommonTypes are for the top-level namespace.
2728
type Schema struct {
2829
Entities Entities
2930
Enums Enums
@@ -51,7 +52,7 @@ type CommonType struct {
5152
type Entity struct {
5253
Annotations Annotations
5354
ParentTypes []EntityTypeRef
54-
Shape *RecordType
55+
Shape RecordType
5556
Tags IsType
5657
}
5758

@@ -83,17 +84,18 @@ type ParentRef struct {
8384
}
8485

8586
// ParentRefFromID creates a ParentRef with only an ID.
86-
// Type is inferred as Action during resolution.
87+
// Type is inferred as Action and namespaced during resolution.
8788
func ParentRefFromID(id types.String) ParentRef {
8889
return ParentRef{
8990
ID: id,
9091
}
9192
}
9293

9394
// NewParentRef creates a ParentRef with type and ID.
94-
func NewParentRef(typ types.EntityType, id types.String) ParentRef {
95+
// Type will be namespaced during resolution.
96+
func NewParentRef(typ EntityTypeRef, id types.String) ParentRef {
9597
return ParentRef{
96-
Type: EntityTypeRef(typ),
98+
Type: typ,
9799
ID: id,
98100
}
99101
}

‎x/exp/schema/ast/ast_test.go‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,3 @@ func TestNewParentRef(t *testing.T) {
3232
testutil.Equals(t, ref.ID, types.String("view"))
3333
testutil.Equals(t, ref.Type, ast.EntityTypeRef("NS::Action"))
3434
}
35-
36-
func TestIsTypeInterface(t *testing.T) {
37-
// Verify all types satisfy IsType by assigning to the interface.
38-
// The isType() marker methods have { _ = 0 } bodies;
39-
// calling through the interface exercises them at runtime.
40-
var types []ast.IsType
41-
types = append(types, ast.StringType{})
42-
types = append(types, ast.LongType{})
43-
types = append(types, ast.BoolType{})
44-
types = append(types, ast.ExtensionType("ipaddr"))
45-
types = append(types, ast.SetType{Element: ast.StringType{}})
46-
types = append(types, ast.RecordType{})
47-
types = append(types, ast.EntityTypeRef("User"))
48-
types = append(types, ast.TypeRef("Foo"))
49-
testutil.Equals(t, len(types), 8)
50-
}

‎x/exp/schema/internal/json/json.go‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ func marshalNamespace(name types.Path, ns ast.Namespace) (jsonNamespace, error)
169169
sort.Strings(jet.MemberOfTypes)
170170
}
171171
if entity.Shape != nil {
172-
jt, err := marshalRecordType(*entity.Shape)
172+
jt, err := marshalRecordType(entity.Shape)
173173
if err != nil {
174174
return jsonNamespace{}, err
175175
}
@@ -215,15 +215,9 @@ func marshalNamespace(name types.Path, ns ast.Namespace) (jsonNamespace, error)
215215
for _, p := range action.AppliesTo.Principals {
216216
jat.PrincipalTypes = append(jat.PrincipalTypes, string(p))
217217
}
218-
if jat.PrincipalTypes == nil {
219-
jat.PrincipalTypes = []string{}
220-
}
221218
for _, r := range action.AppliesTo.Resources {
222219
jat.ResourceTypes = append(jat.ResourceTypes, string(r))
223220
}
224-
if jat.ResourceTypes == nil {
225-
jat.ResourceTypes = []string{}
226-
}
227221
if action.AppliesTo.Context != nil {
228222
jt, err := marshalIsType(action.AppliesTo.Context)
229223
if err != nil {
@@ -345,7 +339,7 @@ func unmarshalNamespace(name types.Path, jns jsonNamespace) (ast.Namespace, erro
345339
if err != nil {
346340
return ast.Namespace{}, fmt.Errorf("entity %q shape: %w", etName, err)
347341
}
348-
entity.Shape = &rec
342+
entity.Shape = rec
349343
}
350344
if jet.Tags != nil {
351345
t, err := unmarshalType(jet.Tags)
@@ -370,23 +364,17 @@ func unmarshalNamespace(name types.Path, jns jsonNamespace) (ast.Namespace, erro
370364
if parent.Type == "" {
371365
action.Parents = append(action.Parents, ast.ParentRefFromID(types.String(parent.ID)))
372366
} else {
373-
action.Parents = append(action.Parents, ast.NewParentRef(types.EntityType(parent.Type), types.String(parent.ID)))
367+
action.Parents = append(action.Parents, ast.NewParentRef(ast.EntityTypeRef(parent.Type), types.String(parent.ID)))
374368
}
375369
}
376370
if ja.AppliesTo != nil {
377371
at := &ast.AppliesTo{}
378372
for _, p := range ja.AppliesTo.PrincipalTypes {
379373
at.Principals = append(at.Principals, ast.EntityTypeRef(p))
380374
}
381-
if at.Principals == nil {
382-
at.Principals = []ast.EntityTypeRef{}
383-
}
384375
for _, r := range ja.AppliesTo.ResourceTypes {
385376
at.Resources = append(at.Resources, ast.EntityTypeRef(r))
386377
}
387-
if at.Resources == nil {
388-
at.Resources = []ast.EntityTypeRef{}
389-
}
390378
if ja.AppliesTo.Context != nil {
391379
t, err := unmarshalType(ja.AppliesTo.Context)
392380
if err != nil {

‎x/exp/schema/internal/json/json_internal_test.go‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55

66
"github.com/cedar-policy/cedar-go/internal/testutil"
77
"github.com/cedar-policy/cedar-go/types"
8-
ast2 "github.com/cedar-policy/cedar-go/x/exp/schema/ast"
8+
"github.com/cedar-policy/cedar-go/x/exp/schema/ast"
99
)
1010

1111
func TestMarshalIsTypeUnknown(t *testing.T) {
@@ -14,32 +14,32 @@ func TestMarshalIsTypeUnknown(t *testing.T) {
1414
}
1515

1616
func TestMarshalIsTypeSetError(t *testing.T) {
17-
_, err := marshalIsType(ast2.SetType{Element: nil})
17+
_, err := marshalIsType(ast.SetType{Element: nil})
1818
testutil.Error(t, err)
1919
}
2020

2121
func TestMarshalRecordTypeError(t *testing.T) {
22-
_, err := marshalRecordType(ast2.RecordType{
23-
"bad": ast2.Attribute{Type: nil},
22+
_, err := marshalRecordType(ast.RecordType{
23+
"bad": ast.Attribute{Type: nil},
2424
})
2525
testutil.Error(t, err)
2626
}
2727

2828
func TestMarshalNamespaceCommonTypeError(t *testing.T) {
29-
_, err := marshalNamespace("", ast2.Namespace{
30-
CommonTypes: ast2.CommonTypes{
31-
"Bad": ast2.CommonType{Type: nil},
29+
_, err := marshalNamespace("", ast.Namespace{
30+
CommonTypes: ast.CommonTypes{
31+
"Bad": ast.CommonType{Type: nil},
3232
},
3333
})
3434
testutil.Error(t, err)
3535
}
3636

3737
func TestMarshalNamespaceEntityShapeError(t *testing.T) {
38-
_, err := marshalNamespace("", ast2.Namespace{
39-
Entities: ast2.Entities{
40-
"Foo": ast2.Entity{
41-
Shape: &ast2.RecordType{
42-
"bad": ast2.Attribute{Type: nil},
38+
_, err := marshalNamespace("", ast.Namespace{
39+
Entities: ast.Entities{
40+
"Foo": ast.Entity{
41+
Shape: ast.RecordType{
42+
"bad": ast.Attribute{Type: nil},
4343
},
4444
},
4545
},
@@ -50,28 +50,28 @@ func TestMarshalNamespaceEntityShapeError(t *testing.T) {
5050
func TestMarshalNamespaceEntityTagsError(t *testing.T) {
5151
// Tags is nil, but the code checks `entity.Tags != nil` first
5252
// So we need a non-nil tags that fails. Use SetType{Element: nil}.
53-
_, err := marshalNamespace("", ast2.Namespace{
54-
Entities: ast2.Entities{
55-
"Foo": ast2.Entity{Tags: nil},
53+
_, err := marshalNamespace("", ast.Namespace{
54+
Entities: ast.Entities{
55+
"Foo": ast.Entity{Tags: nil},
5656
},
5757
})
5858
testutil.OK(t, err)
5959
}
6060

6161
func TestMarshalNamespaceEntityTagsError2(t *testing.T) {
62-
_, err := marshalNamespace("", ast2.Namespace{
63-
Entities: ast2.Entities{
64-
"Foo": ast2.Entity{Tags: ast2.SetType{Element: nil}},
62+
_, err := marshalNamespace("", ast.Namespace{
63+
Entities: ast.Entities{
64+
"Foo": ast.Entity{Tags: ast.SetType{Element: nil}},
6565
},
6666
})
6767
testutil.Error(t, err)
6868
}
6969

7070
func TestMarshalNamespaceActionAnnotations(t *testing.T) {
71-
ns, err := marshalNamespace("", ast2.Namespace{
72-
Actions: ast2.Actions{
73-
"view": ast2.Action{
74-
Annotations: ast2.Annotations{"doc": "test"},
71+
ns, err := marshalNamespace("", ast.Namespace{
72+
Actions: ast.Actions{
73+
"view": ast.Action{
74+
Annotations: ast.Annotations{"doc": "test"},
7575
},
7676
},
7777
})
@@ -80,11 +80,11 @@ func TestMarshalNamespaceActionAnnotations(t *testing.T) {
8080
}
8181

8282
func TestMarshalNamespaceContextError(t *testing.T) {
83-
_, err := marshalNamespace("", ast2.Namespace{
84-
Actions: ast2.Actions{
85-
"view": ast2.Action{
86-
AppliesTo: &ast2.AppliesTo{
87-
Context: ast2.SetType{Element: nil},
83+
_, err := marshalNamespace("", ast.Namespace{
84+
Actions: ast.Actions{
85+
"view": ast.Action{
86+
AppliesTo: &ast.AppliesTo{
87+
Context: ast.SetType{Element: nil},
8888
},
8989
},
9090
},
@@ -94,8 +94,8 @@ func TestMarshalNamespaceContextError(t *testing.T) {
9494

9595
func TestMarshalBareNamespaceError(t *testing.T) {
9696
s := &Schema{
97-
Entities: ast2.Entities{
98-
"Foo": ast2.Entity{Tags: ast2.SetType{Element: nil}},
97+
Entities: ast.Entities{
98+
"Foo": ast.Entity{Tags: ast.SetType{Element: nil}},
9999
},
100100
}
101101
_, err := s.MarshalJSON()
@@ -104,10 +104,10 @@ func TestMarshalBareNamespaceError(t *testing.T) {
104104

105105
func TestMarshalNamespacedError(t *testing.T) {
106106
s := &Schema{
107-
Namespaces: ast2.Namespaces{
108-
"NS": ast2.Namespace{
109-
Entities: ast2.Entities{
110-
"Foo": ast2.Entity{Tags: ast2.SetType{Element: nil}},
107+
Namespaces: ast.Namespaces{
108+
"NS": ast.Namespace{
109+
Entities: ast.Entities{
110+
"Foo": ast.Entity{Tags: ast.SetType{Element: nil}},
111111
},
112112
},
113113
},

0 commit comments

Comments
 (0)