Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create MapSet[T] and ImmutableMapSet[T] types and change Entity.Parents to be a ImmutableMapSet[EntityUID] #40

Merged
merged 17 commits into from
Sep 26, 2024
Merged

Create MapSet[T] and ImmutableMapSet[T] types and change Entity.Parents to be a ImmutableMapSet[EntityUID] #40

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
fd1c22a
cedar-go/internal: add a MapSet type, which is basically just some co…
patjakdev Sep 19, 2024
4fc3a74
cedar-go/types: convert Entity.Parents to a MapSet[EntityUID]
patjakdev Sep 19, 2024
c2ef430
cedar-go/internal/parser: use a MapSet[string] instead of a map[strin…
patjakdev Sep 20, 2024
58729be
internal/parser: use a MapSet[string] instead of a map[string]struct{…
patjakdev Sep 20, 2024
32380ba
x/exp/batch: use a MapSet[types.String] instead of map[types.String]{…
patjakdev Sep 20, 2024
2e445e2
internal/mapset: move mapset to its own package, rename functions app…
patjakdev Sep 24, 2024
d479334
internal/mapset: create new immutable mapset type
patjakdev Sep 24, 2024
a81d260
types: make EntityUIDSet an ImmutableHashSet
patjakdev Sep 24, 2024
68386de
internal/mapset: rename mapset.New to mapset.Make
patjakdev Sep 24, 2024
7b923a3
internal/mapset: remove some unused methods on MapSet
patjakdev Sep 24, 2024
6b1e959
internal/mapset: rename FromSlice to FromItems and use variadic args
patjakdev Sep 24, 2024
efe617a
internal/mapset: reduce the functionality of Intersection to just a b…
patjakdev Sep 24, 2024
05c78eb
internal/mapset: add some documentation around the zero value of MapSet
patjakdev Sep 24, 2024
c0c3981
types: fix some tiny typos
patjakdev Sep 24, 2024
b7a52e1
internal/mapset: give MarshalJSON a deterministic output
patjakdev Sep 26, 2024
b71df7e
Fix various nits
patjakdev Sep 26, 2024
e796ce2
types: ensure Entity marshals to JSON with a consistent ordering
patjakdev Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
462 changes: 231 additions & 231 deletions authorize_test.go
View file
Open in desktop

Large diffs are not rendered by default.

59 changes: 24 additions & 35 deletions internal/eval/evalers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/cedar-policy/cedar-go/internal/consts"
"github.com/cedar-policy/cedar-go/internal/extensions"
"github.com/cedar-policy/cedar-go/internal/sets"
"github.com/cedar-policy/cedar-go/types"
)

Expand Down Expand Up @@ -973,11 +974,6 @@ func newInEval(lhs, rhs Evaler) Evaler {
return &inEval{lhs: lhs, rhs: rhs}
}

func hasKnown(known map[types.EntityUID]struct{}, k types.EntityUID) bool {
_, ok := known[k]
return ok
}

func entityInOne(env *Env, entity types.EntityUID, parent types.EntityUID) bool {
key := inKey{a: entity, b: parent}
patjakdev marked this conversation as resolved.
Show resolved Hide resolved
if cached, ok := env.inCache[key]; ok {
Expand All @@ -987,31 +983,28 @@ func entityInOne(env *Env, entity types.EntityUID, parent types.EntityUID) bool
env.inCache[key] = result
return result
}

func entityInOneWork(env *Env, entity types.EntityUID, parent types.EntityUID) bool {
if entity == parent {
return true
}
var known map[types.EntityUID]struct{}
var known sets.MapSet[types.EntityUID]
var todo []types.EntityUID
var candidate = entity
for {
if fe, ok := env.Entities[candidate]; ok {
for _, k := range fe.Parents {
if k == parent {
return true
}
if fe.Parents.Contains(parent) {
return true
}
for _, k := range fe.Parents {
fe.Parents.Iterate(func(k types.EntityUID) bool {
p, ok := env.Entities[k]
if !ok || len(p.Parents) == 0 || k == entity || hasKnown(known, k) {
continue
if !ok || p.Parents.Len() == 0 || k == entity || known.Contains(k) {
return true
}
todo = append(todo, k)
if known == nil {
known = map[types.EntityUID]struct{}{}
}
known[k] = struct{}{}
}
known.Add(k)
patjakdev marked this conversation as resolved.
Show resolved Hide resolved
return true
})
}
if len(todo) == 0 {
return false
Expand All @@ -1020,31 +1013,27 @@ func entityInOneWork(env *Env, entity types.EntityUID, parent types.EntityUID) b
}
}

func entityInSet(env *Env, entity types.EntityUID, parents map[types.EntityUID]struct{}) bool {
if _, ok := parents[entity]; ok {
func entityInSet(env *Env, entity types.EntityUID, parents types.EntityUIDSet) bool {
if parents.Contains(entity) {
return true
}
var known map[types.EntityUID]struct{}
var known sets.MapSet[types.EntityUID]
var todo []types.EntityUID
var candidate = entity
for {
if fe, ok := env.Entities[candidate]; ok {
for _, k := range fe.Parents {
if _, ok := parents[k]; ok {
return true
}
if fe.Parents.Intersection(parents).Len() > 0 {
return true
}
for _, k := range fe.Parents {
fe.Parents.Iterate(func(k types.EntityUID) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but as a point of interest, I wonder if using a closure causes an allocation.

p, ok := env.Entities[k]
if !ok || len(p.Parents) == 0 || k == entity || hasKnown(known, k) {
continue
if !ok || p.Parents.Len() == 0 || k == entity || known.Contains(k) {
return true
}
todo = append(todo, k)
if known == nil {
known = map[types.EntityUID]struct{}{}
}
known[k] = struct{}{}
}
known.Add(k)
return true
})
}
if len(todo) == 0 {
return false
Expand Down Expand Up @@ -1072,14 +1061,14 @@ func doInEval(env *Env, lhs types.EntityUID, rhs types.Value) (types.Value, erro
case types.EntityUID:
return types.Boolean(entityInOne(env, lhs, rhsv)), nil
case types.Set:
query := make(map[types.EntityUID]struct{}, rhsv.Len())
query := sets.NewMapSet[types.EntityUID](rhsv.Len())
var err error
rhsv.Iterate(func(rhv types.Value) bool {
var e types.EntityUID
if e, err = ValueToEntity(rhv); err != nil {
return false
}
query[e] = struct{}{}
query.Add(e)
return true
})
if err != nil {
Expand Down
27 changes: 16 additions & 11 deletions internal/eval/evalers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/cedar-policy/cedar-go/internal/consts"
"github.com/cedar-policy/cedar-go/internal/parser"
"github.com/cedar-policy/cedar-go/internal/sets"
"github.com/cedar-policy/cedar-go/internal/testutil"
"github.com/cedar-policy/cedar-go/types"
)
Expand Down Expand Up @@ -1833,15 +1834,15 @@ func TestEntityIn(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rhs := map[types.EntityUID]struct{}{}
var rhs sets.MapSet[types.EntityUID]
for _, v := range tt.rhs {
rhs[strEnt(v)] = struct{}{}
rhs.Add(strEnt(v))
}
entityMap := types.Entities{}
for k, p := range tt.parents {
var ps []types.EntityUID
var ps sets.MapSet[types.EntityUID]
for _, pp := range p {
ps = append(ps, strEnt(pp))
ps.Add(strEnt(pp))
}
uid := strEnt(k)
entityMap[uid] = &types.Entity{
Expand All @@ -1860,10 +1861,10 @@ func TestEntityIn(t *testing.T) {

entityMap := types.Entities{}
for i := 0; i < 100; i++ {
p := []types.EntityUID{
p := sets.NewMapSetFromSlice([]types.EntityUID{
patjakdev marked this conversation as resolved.
Show resolved Hide resolved
types.NewEntityUID(types.EntityType(fmt.Sprint(i+1)), "1"),
types.NewEntityUID(types.EntityType(fmt.Sprint(i+1)), "2"),
}
})
uid1 := types.NewEntityUID(types.EntityType(fmt.Sprint(i)), "1")
entityMap[uid1] = &types.Entity{
UID: uid1,
Expand All @@ -1876,7 +1877,11 @@ func TestEntityIn(t *testing.T) {
}

}
res := entityInSet(&Env{Entities: entityMap}, types.NewEntityUID("0", "1"), map[types.EntityUID]struct{}{types.NewEntityUID("0", "3"): {}})
res := entityInSet(
&Env{Entities: entityMap},
types.NewEntityUID("0", "1"),
sets.NewMapSetFromSlice([]types.EntityUID{types.NewEntityUID("0", "3")}),
)
testutil.Equals(t, res, false)
})
}
Expand Down Expand Up @@ -2005,9 +2010,9 @@ func TestInNode(t *testing.T) {
n := newInEval(tt.lhs, tt.rhs)
entityMap := types.Entities{}
for k, p := range tt.parents {
var ps []types.EntityUID
var ps sets.MapSet[types.EntityUID]
for _, pp := range p {
ps = append(ps, strEnt(pp))
ps.Add(strEnt(pp))
}
uid := strEnt(k)
entityMap[uid] = &types.Entity{
Expand Down Expand Up @@ -2145,9 +2150,9 @@ func TestIsInNode(t *testing.T) {
n := newIsInEval(tt.lhs, tt.is, tt.rhs)
entityMap := types.Entities{}
for k, p := range tt.parents {
var ps []types.EntityUID
var ps sets.MapSet[types.EntityUID]
for _, pp := range p {
ps = append(ps, strEnt(pp))
ps.Add(strEnt(pp))
}
uid := strEnt(k)
entityMap[uid] = &types.Entity{
Expand Down
6 changes: 2 additions & 4 deletions internal/eval/partial.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"slices"

"github.com/cedar-policy/cedar-go/internal/ast"
"github.com/cedar-policy/cedar-go/internal/sets"
"github.com/cedar-policy/cedar-go/types"
)

Expand Down Expand Up @@ -140,10 +141,7 @@ func partialScopeEval(env *Env, ent types.Value, in ast.IsScopeNode) (evaled boo
case ast.ScopeTypeIn:
return true, entityInOne(env, e, t.Entity)
case ast.ScopeTypeInSet:
set := make(map[types.EntityUID]struct{}, len(t.Entities))
for _, e := range t.Entities {
set[e] = struct{}{}
}
set := sets.NewMapSetFromSlice(t.Entities)
patjakdev marked this conversation as resolved.
Show resolved Hide resolved
return true, entityInSet(env, e, set)
case ast.ScopeTypeIs:
return true, e.Type == t.Type
Expand Down
21 changes: 11 additions & 10 deletions internal/parser/cedar_unmarshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/cedar-policy/cedar-go/internal/ast"
"github.com/cedar-policy/cedar-go/internal/consts"
"github.com/cedar-policy/cedar-go/internal/extensions"
"github.com/cedar-policy/cedar-go/internal/sets"
"github.com/cedar-policy/cedar-go/types"
)

Expand Down Expand Up @@ -127,10 +128,10 @@ func (p *parser) errorf(s string, args ...interface{}) error {

func (p *parser) annotations() (ast.Annotations, error) {
var res ast.Annotations
known := map[string]struct{}{}
var known sets.MapSet[string]
for p.peek().Text == "@" {
p.advance()
err := p.annotation(&res, known)
err := p.annotation(&res, &known)
if err != nil {
return res, err
}
Expand All @@ -139,11 +140,11 @@ func (p *parser) annotations() (ast.Annotations, error) {

}

func (p *parser) annotation(a *ast.Annotations, known map[string]struct{}) error {
func (p *parser) annotation(a *ast.Annotations, known *sets.MapSet[string]) error {
patjakdev marked this conversation as resolved.
Show resolved Hide resolved
var err error
t := p.advance()
// As of 2024-09-13, the ability to use reserved keywords is not documented in the Cedar schema. The ability to use
// reserved keywords was added in this commit:
// As of 2024-09-13, the ability to use reserved keywords for annotation keys is not documented in the Cedar schema.
// This ability was added to the Rust implementation in this commit:
// https://github.com/cedar-policy/cedar/commit/5f62c6df06b59abc5634d6668198a826839c6fb7
if !(t.isIdent() || t.isReservedKeyword()) {
return p.errorf("expected ident or reserved keyword")
Expand All @@ -152,10 +153,10 @@ func (p *parser) annotation(a *ast.Annotations, known map[string]struct{}) error
if err = p.exact("("); err != nil {
return err
}
if _, ok := known[name]; ok {
if known.Contains(name) {
return p.errorf("duplicate annotation: @%s", name)
}
known[name] = struct{}{}
known.Add(name)
t = p.advance()
if !t.isString() {
return p.errorf("expected string")
Expand Down Expand Up @@ -815,7 +816,7 @@ func (p *parser) expressions(endOfListMarker string) ([]ast.Node, error) {
func (p *parser) record() (ast.Node, error) {
var res ast.Node
var elements ast.Pairs
known := map[string]struct{}{}
var known sets.MapSet[string]
for {
t := p.peek()
if t.Text == "}" {
Expand All @@ -832,10 +833,10 @@ func (p *parser) record() (ast.Node, error) {
return res, err
}

if _, ok := known[k]; ok {
if known.Contains(k) {
return res, p.errorf("duplicate key: %v", k)
}
known[k] = struct{}{}
known.Add(k)
elements = append(elements, ast.Pair{Key: types.String(k), Value: v})
}
}
Expand Down
Loading
Loading