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.

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
Show file tree
Hide file tree
Changes from all 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

Large diffs are not rendered by default.

59 changes: 24 additions & 35 deletions internal/eval/evalers.go
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/mapset"
"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 mapset.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 mapset.Container[types.EntityUID]) bool {
if parents.Contains(entity) {
return true
}
var known map[types.EntityUID]struct{}
var known mapset.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.Intersects(parents) {
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 := mapset.Make[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
22 changes: 13 additions & 9 deletions internal/eval/evalers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1833,9 +1833,9 @@ func TestEntityIn(t *testing.T) {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rhs := map[types.EntityUID]struct{}{}
var rhs []types.EntityUID
for _, v := range tt.rhs {
rhs[strEnt(v)] = struct{}{}
rhs = append(rhs, strEnt(v))
}
entityMap := types.Entities{}
for k, p := range tt.parents {
Expand All @@ -1846,10 +1846,10 @@ func TestEntityIn(t *testing.T) {
uid := strEnt(k)
entityMap[uid] = &types.Entity{
UID: uid,
Parents: ps,
Parents: types.NewEntityUIDSet(ps...),
}
}
res := entityInSet(&Env{Entities: entityMap}, strEnt(tt.lhs), rhs)
res := entityInSet(&Env{Entities: entityMap}, strEnt(tt.lhs), types.NewEntityUIDSet(rhs...))
testutil.Equals(t, res, tt.result)
})
}
Expand All @@ -1860,10 +1860,10 @@ func TestEntityIn(t *testing.T) {

entityMap := types.Entities{}
for i := 0; i < 100; i++ {
p := []types.EntityUID{
p := types.NewEntityUIDSet(
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 +1876,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"),
types.NewEntityUIDSet(types.NewEntityUID("0", "3")),
)
testutil.Equals(t, res, false)
})
}
Expand Down Expand Up @@ -2012,7 +2016,7 @@ func TestInNode(t *testing.T) {
uid := strEnt(k)
entityMap[uid] = &types.Entity{
UID: uid,
Parents: ps,
Parents: types.NewEntityUIDSet(ps...),
}
}
ec := InitEnv(&Env{Entities: entityMap})
Expand Down Expand Up @@ -2152,7 +2156,7 @@ func TestIsInNode(t *testing.T) {
uid := strEnt(k)
entityMap[uid] = &types.Entity{
UID: uid,
Parents: ps,
Parents: types.NewEntityUIDSet(ps...),
}
}
ec := InitEnv(&Env{Entities: entityMap})
Expand Down
6 changes: 2 additions & 4 deletions internal/eval/partial.go
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/mapset"
"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 := mapset.Immutable(t.Entities...)
return true, entityInSet(env, e, set)
case ast.ScopeTypeIs:
return true, e.Type == t.Type
Expand Down
58 changes: 58 additions & 0 deletions internal/mapset/immutable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package mapset

import (
"encoding/json"
)

type ImmutableMapSet[T comparable] MapSet[T]

func Immutable[T comparable](args ...T) ImmutableMapSet[T] {
return ImmutableMapSet[T](*FromItems(args...))
}

// Contains returns whether the item exists in the set
func (h ImmutableMapSet[T]) Contains(item T) bool {
return MapSet[T](h).Contains(item)
}

// Intersects returns whether any items in this set exist in o
func (h ImmutableMapSet[T]) Intersects(o Container[T]) bool {
return MapSet[T](h).Intersects(o)
}

// Iterate the items in the set, calling callback for each item. If the callback returns false, iteration is halted.
// Iteration order is undefined.
func (h ImmutableMapSet[T]) Iterate(callback func(item T) bool) {
MapSet[T](h).Iterate(callback)
}

func (h ImmutableMapSet[T]) Slice() []T {
return MapSet[T](h).Slice()
}

// Len returns the size of the set
func (h ImmutableMapSet[T]) Len() int {
return MapSet[T](h).Len()
}

// Equal returns whether the same items exist in both h and o
func (h ImmutableMapSet[T]) Equal(o ImmutableMapSet[T]) bool {
om := MapSet[T](o)
return MapSet[T](h).Equal(&om)
}

// MarshalJSON serializes a MapSet as a JSON array. Elements are ordered lexicographically by their marshaled value.
func (h ImmutableMapSet[T]) MarshalJSON() ([]byte, error) {
return MapSet[T](h).MarshalJSON()
}

// UnmarshalJSON deserializes an ImmutableMapSet from a JSON array.
func (h *ImmutableMapSet[T]) UnmarshalJSON(b []byte) error {
var s MapSet[T]
if err := json.Unmarshal(b, &s); err != nil {
return err
}

*h = ImmutableMapSet[T](s)
return nil
}
Loading
Loading