Skip to content

Commit

Permalink
topdown: Base document dereference with composite
Browse files Browse the repository at this point in the history
Previously this would raise an error when attempting to build the
storage path for some ref like `data.foo[["bar"]]`, but there isn't
really any reason why it should halt evaluation. It should just
be undefined.

This changes to look for this type of ref _before_ handing it off to
the store and will just return undefined now instead.

Fixes: #1057
Signed-off-by: Patrick East <east.patrick@gmail.com>
  • Loading branch information
patrick-east committed Sep 26, 2019
1 parent e5f7ef6 commit 9baaf47
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
13 changes: 13 additions & 0 deletions topdown/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,10 @@ func (e *eval) Resolve(ref ast.Ref) (ast.Value, error) {
}

func (e *eval) resolveReadFromStorage(ref ast.Ref, a ast.Value) (ast.Value, error) {
if refContainsNonScalar(ref) {
return a, nil
}

path, err := storage.NewPathForRef(ref)
if err != nil {
if !storage.IsNotFound(err) {
Expand Down Expand Up @@ -2304,3 +2308,12 @@ func refSliceContainsPrefix(sl []ast.Ref, prefix ast.Ref) bool {
}
return false
}

func refContainsNonScalar(ref ast.Ref) bool {
for _, term := range ref[1:] {
if !ast.IsScalar(term.Value) {
return true
}
}
return false
}
50 changes: 50 additions & 0 deletions topdown/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,53 @@ func TestMergeError(t *testing.T) {
t.Fatal("Expected error")
}
}

func TestRefContainsNonScalar(t *testing.T) {
cases := []struct {
note string
ref ast.Ref
expected bool
}{
{
note: "empty ref",
ref: ast.MustParseRef("data"),
expected: false,
},
{
note: "string ref",
ref: ast.MustParseRef(`data.foo["bar"]`),
expected: false,
},
{
note: "number ref",
ref: ast.MustParseRef("data.foo[1]"),
expected: false,
},
{
note: "set ref",
ref: ast.MustParseRef("data.foo[{0}]"),
expected: true,
},
{
note: "array ref",
ref: ast.MustParseRef(`data.foo[["bar"]]`),
expected: true,
},
{
note: "object ref",
ref: ast.MustParseRef(`data.foo[{"bar": 1}]`),
expected: true,
},
}

for _, tc := range cases {
t.Run(tc.note, func(t *testing.T) {
actual := refContainsNonScalar(tc.ref)

if actual != tc.expected {
t.Errorf("Expected %t for %s", tc.expected, tc.ref)
}
})
}

}
19 changes: 19 additions & 0 deletions topdown/topdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2672,6 +2672,25 @@ p[x] { data.a[i] = x }`,
}
}

func TestTopDownCompositeBaseDereference(t *testing.T) {
tests := []struct {
note string
rule string
expected interface{}
}{
// Expect that each of these will evaluate without any errors raised
{"array", `p { not data.a[[0]] }`, "true"},
{"object", `p { not data.a[{"b": "c"}] }`, "true"},
{"set", `p { not data.a[["b"]] }`, "true"},
}

data := loadSmallTestData()

for _, tc := range tests {
runTopDownTestCase(t, data, tc.note, []string{tc.rule}, tc.expected)
}
}

func compileModules(input []string) *ast.Compiler {

mods := map[string]*ast.Module{}
Expand Down

0 comments on commit 9baaf47

Please sign in to comment.