Skip to content

Commit

Permalink
Fix panic in ObjectSearch (#42)
Browse files Browse the repository at this point in the history 
* Enable all test relations and update tests

* Fix panic in ObjectSearch when model inversion fails
  • Loading branch information
@ronenh
ronenh authored Apr 10, 2024
1 parent 8cddd1e commit 4efa307
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 23 deletions.
5 changes: 3 additions & 2 deletions graph/objects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"github.com/aserto-dev/azm/model"
dsc "github.com/aserto-dev/go-directory/aserto/directory/common/v3"
dsr "github.com/aserto-dev/go-directory/aserto/directory/reader/v3"
"github.com/aserto-dev/go-directory/pkg/derr"
"github.com/samber/lo"
)

Expand All @@ -25,7 +26,7 @@ func NewObjectSearch(m *model.Model, req *dsr.GetGraphRequest, reader RelationRe
// uses mangled names that are not valid identifiers.
if err := im.Validate(model.SkipNameValidation, model.AllowPermissionInArrowBase); err != nil {
// TODO: we should persist the inverted model instead of computing it on the fly.
panic(err)
return nil, derr.ErrUnknown.Msg("internal error: unable to search for objects.")
}

iParams := invertGetGraphRequest(im, req)
Expand Down Expand Up @@ -81,7 +82,7 @@ func (s *ObjectSearch) Search() (*dsr.GetGraphResponse, error) {
resp.Results = res.Subjects()

if s.subjectSearch.explain {
resp.Explanation = res.Explain()
resp.Explanation, _ = res.Explain()
}

resp.Trace = memo.Trace()
Expand Down
6 changes: 2 additions & 4 deletions graph/objects_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ var searchObjectsTests = []searchTest{
{"doc:?#viewer@user:f1_viewer", []object{{"doc", "doc2"}}},
{"doc:?#viewer@group:d1_viewers#member", []object{{"doc", "doc1"}}},
{"doc:?#parent@folder:folder1", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}},

{"group:?#member@group:leaf#member", []object{{"group", "branch"}, {"group", "trunk"}, {"group", "root"}}},
{"doc:?#viewer@group:leaf#member", []object{{"doc", "doc_tree"}}},
{"group:?#member@group:yang#member", []object{{"group", "yin"}, {"group", "yang"}}},
Expand All @@ -98,7 +97,6 @@ var searchObjectsTests = []searchTest{
{"folder:?#is_owner@user:f1_owner", []object{{"folder", "folder1"}, {"folder", "folder2"}}},
{"folder:?#can_create_file@user:f1_owner", []object{{"folder", "folder1"}, {"folder", "folder2"}}},
{"folder:?#can_read@user:f1_owner", []object{{"folder", "folder1"}, {"folder", "folder2"}}},

{"folder:?#can_share@user:f1_owner", []object{{"folder", "folder1"}, {"folder", "folder2"}}},
{"doc:?#can_change_owner@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}},
{"doc:?#can_write@user:f1_owner", []object{{"doc", "doc1"}, {"doc", "doc2"}, {"doc", "doc3"}}},
Expand Down Expand Up @@ -192,8 +190,8 @@ func relations() RelationsReader {

"group:d1_viewers#member@group:d1_subviewers#member",
"group:d1_subviewers#member@user:user3",
// "group:f1_viewers#member@group:f1_subviewers#member",
// "group:d1_subviewers#member@user:user4",
"group:f1_viewers#member@group:f1_subviewers#member",
"group:d1_subviewers#member@user:user4",

// nested groups
"group:leaf#member@user:leaf_user",
Expand Down
9 changes: 2 additions & 7 deletions graph/search.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (r searchResults) Subjects() []*dsc.ObjectIdentifier {
})
}

func (r searchResults) Explain() *structpb.Struct {
func (r searchResults) Explain() (*structpb.Struct, error) {
explanation := lo.MapEntries(r, func(obj object, paths []searchPath) (string, any) {
key := fmt.Sprintf("%s:%s", obj.Type, obj.ID)

Expand All @@ -61,12 +61,7 @@ func (r searchResults) Explain() *structpb.Struct {
return key, val
})

res, err := structpb.NewStruct(explanation)
if err != nil {
panic(err)
}

return res
return structpb.NewStruct(explanation)
}

type searchStatus int
Expand Down
2 changes: 1 addition & 1 deletion graph/subjects.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (s *SubjectSearch) Search() (*dsr.GetGraphResponse, error) {
resp.Results = res.Subjects()

if s.explain {
resp.Explanation = res.Explain()
resp.Explanation, _ = res.Explain()
}

resp.Trace = s.memo.Trace()
Expand Down
21 changes: 12 additions & 9 deletions graph/subjects_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,40 +53,43 @@ var searchSubjectsTests = []searchTest{
{"folder:folder2#owner@user:?", []object{}},
{"folder:folder2#viewer@user:?", []object{}},
{"group:f1_viewers#member@user:?", []object{{"user", "f1_viewer"}}},
{"folder:folder1#viewer@group:?#member", []object{{"group", "f1_viewers"}}},
{"folder:folder1#viewer@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}}},
{"doc:doc1#parent@folder:?", []object{{"folder", "folder1"}}},
{"folder:folder2#viewer@group:?#member", []object{}},
{"doc:doc1#owner@user:?", []object{{"user", "d1_owner"}}},
{"doc:doc1#viewer@group:?#member", []object{{"group", "d1_viewers"}, {"group", "d1_subviewers"}}},
{"doc:doc1#viewer@user:?", []object{{"user", "user1"}, {"user", "user2"}, {"user", "user3"}, {"user", "f1_owner"}}},
{"doc:doc1#viewer@user:?", []object{{"user", "user1"}, {"user", "user2"}, {"user", "user3"}, {"user", "user4"}, {"user", "f1_owner"}}},
{"doc:doc2#viewer@user:?", []object{{"user", "*"}, {"user", "user2"}}},
{"group:d1_viewers#member@group:?#member", []object{{"group", "d1_subviewers"}}},
{"group:d1_viewers#member@user:?", []object{{"user", "user2"}, {"user", "user3"}}},
{"group:d1_viewers#member@user:?", []object{{"user", "user2"}, {"user", "user3"}, {"user", "user4"}}},
{"group:root#member@user:?", []object{{"user", "leaf_user"}}},
{"group:root#member@group:?#member", []object{{"group", "leaf"}, {"group", "branch"}, {"group", "trunk"}}},

// Permissions
{"folder:folder1#can_create_file@user:?", []object{{"user", "f1_owner"}}},
{"folder:folder1#can_read@user:?", []object{{"user", "f1_owner"}, {"user", "f1_viewer"}}},
{"folder:folder1#can_read@group:?#member", []object{{"group", "f1_viewers"}}},
{"folder:folder1#can_read@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}}},
{"folder:folder1#can_share@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc1#can_change_owner@user:?", []object{{"user", "d1_owner"}, {"user", "f1_owner"}}},
{"doc:doc1#can_write@user:?", []object{{"user", "d1_owner"}, {"user", "f1_owner"}}},
{"doc:doc1#can_read@user:?", []object{{"user", "d1_owner"}, {"user", "f1_owner"}, {"user", "user1"}, {"user", "user2"}, {"user", "user3"}, {"user", "f1_viewer"}}},
{"doc:doc1#can_read@group:?#member", []object{{"group", "f1_viewers"}, {"group", "d1_viewers"}, {"group", "d1_subviewers"}}},
{"doc:doc1#can_read@user:?", []object{
{"user", "d1_owner"}, {"user", "f1_owner"}, {"user", "f1_viewer"},
{"user", "user1"}, {"user", "user2"}, {"user", "user3"}, {"user", "user4"},
}},
{"doc:doc1#can_read@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}, {"group", "d1_viewers"}, {"group", "d1_subviewers"}}},
{"doc:doc1#can_share@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc1#can_invite@user:?", []object{{"user", "f1_viewer"}}},
{"doc:doc1#can_invite@group:?#member", []object{{"group", "f1_viewers"}}},
{"doc:doc1#can_invite@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}}},
{"doc:doc2#can_change_owner@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc2#can_write@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc2#can_read@user:?", []object{{"user", "*"}, {"user", "user2"}, {"user", "f1_owner"}, {"user", "f1_viewer"}}},
{"doc:doc2#can_share@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc2#can_invite@user:?", []object{{"user", "f1_owner"}, {"user", "f1_viewer"}}},
{"doc:doc2#can_read@group:?#member", []object{{"group", "f1_viewers"}}},
{"doc:doc2#can_read@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}}},
{"doc:doc3#can_change_owner@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc3#can_write@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc3#can_read@user:?", []object{{"user", "f1_owner"}, {"user", "f1_viewer"}}},
{"doc:doc3#can_share@user:?", []object{{"user", "f1_owner"}}},
{"doc:doc3#can_invite@user:?", []object{{"user", "f1_owner"}, {"user", "f1_viewer"}}},
{"doc:doc3#can_read@group:?#member", []object{{"group", "f1_viewers"}}},
{"doc:doc3#can_read@group:?#member", []object{{"group", "f1_viewers"}, {"group", "f1_subviewers"}}},
}

0 comments on commit 4efa307

Please sign in to comment.