Skip to content

Commit

Permalink
server: differentiate between missing and undefined doc in default de…
Browse files Browse the repository at this point in the history
…cision (#5420)

Since "document missing or undefined" caused confusion, we can be
more helpful and report whether the default decision document is
missing, or whether it's there but undefined. I would probably have
preferred for undefined to just result in an empty/undefined result,
but that'd be quite a breaking change at this point in time, so
hopefully this can at least make things a little less convoluted.

Fixes #5344

Signed-off-by: Anders Eknert <anders@eknert.com>
  • Loading branch information
anderseknert committed Nov 29, 2022
1 parent d0026f4 commit 518507b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
8 changes: 7 additions & 1 deletion server/server.go
Expand Up @@ -1014,7 +1014,13 @@ func (s *Server) v0QueryPath(w http.ResponseWriter, r *http.Request, urlPath str
}

if len(rs) == 0 {
err := types.NewErrorV1(types.CodeUndefinedDocument, fmt.Sprintf("%v: %v", types.MsgUndefinedError, stringPathToDataRef(urlPath)))
ref := stringPathToDataRef(urlPath)

var messageType = types.MsgMissingError
if len(s.getCompiler().GetRulesForVirtualDocument(ref)) > 0 {
messageType = types.MsgFoundUndefinedError
}
err := types.NewErrorV1(types.CodeUndefinedDocument, fmt.Sprintf("%v: %v", messageType, ref))
if logErr := logger.Log(ctx, txn, decisionID, r.RemoteAddr, urlPath, "", goInput, input, nil, ndbCache, err, m); logErr != nil {
writer.ErrorAuto(w, logErr)
return
Expand Down
41 changes: 40 additions & 1 deletion server/server_test.go
Expand Up @@ -3198,7 +3198,7 @@ func TestDecisionLogging(t *testing.T) {
code: 404,
response: `{
"code": "undefined_document",
"message": "document missing or undefined: data.test"
"message": "document missing: data.test"
}`,
},
}
Expand Down Expand Up @@ -3406,6 +3406,15 @@ func TestUnversionedPost(t *testing.T) {
t.Fatalf("Expected not found before policy added but got %v", f.recorder)
}

expectedBody := `{
"code": "undefined_document",
"message": "document missing: data.system.main"
}
`
if f.recorder.Body.String() != expectedBody {
t.Errorf("Expected %s got %s", expectedBody, f.recorder.Body.String())
}

module := `
package system.main
Expand All @@ -3425,6 +3434,36 @@ func TestUnversionedPost(t *testing.T) {
if f.recorder.Code != 200 || f.recorder.Body.String() != expected {
t.Fatalf(`Expected HTTP 200 / %v but got: %v`, expected, f.recorder)
}

module = `
package system
main {
input.foo == "bar"
}
`

if err := f.v1("PUT", "/policies/test", module, 200, ""); err != nil {
t.Fatal(err)
}

f.reset()
f.server.Handler.ServeHTTP(f.recorder, func() *http.Request {
return newReqUnversioned(http.MethodPost, "/", `{"input": {"foo": "bar"}}`)
}())

if f.recorder.Code != 404 {
t.Fatalf("Expected not found before policy added but got %v", f.recorder)
}

expectedBody = `{
"code": "undefined_document",
"message": "document undefined: data.system.main"
}
`
if f.recorder.Body.String() != expectedBody {
t.Errorf("Expected %s got %s", expectedBody, f.recorder.Body.String())
}
}

func TestQueryV1Explain(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions server/types/types.go
Expand Up @@ -80,6 +80,8 @@ const (
MsgUnauthorizedUndefinedError = "authorization policy missing or undefined"
MsgUnauthorizedError = "request rejected by administrative policy"
MsgUndefinedError = "document missing or undefined"
MsgMissingError = "document missing"
MsgFoundUndefinedError = "document undefined"
MsgPluginConfigError = "error(s) occurred while configuring plugin(s)"
)

Expand Down

0 comments on commit 518507b

Please sign in to comment.