fix builtin not found err handling (#209)

aserto-dev · Dec 12, 2023 · e188fd6 · e188fd6
1 parent 1ff26b2
commit e188fd6
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 16 deletions.
diff --git a/builtins/edge/ds/identity.go b/builtins/edge/ds/identity.go
@@ -1,9 +1,10 @@
 package ds
 
 import (
-	"github.com/aserto-dev/go-authorizer/pkg/aerr"
 	"github.com/aserto-dev/topaz/directory"
 	"github.com/aserto-dev/topaz/resolvers"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 
 	"github.com/open-policy-agent/opa/ast"
 	"github.com/open-policy-agent/opa/rego"
@@ -61,10 +62,10 @@ func RegisterIdentity(logger *zerolog.Logger, fnName string, dr resolvers.Direct
 
 			user, err := directory.GetIdentityV2(bctx.Context, client, args.ID)
 			switch {
-			case errors.Is(err, aerr.ErrDirectoryObjectNotFound):
-				return nil, err
-			case err != nil:
+			case status.Code(err) == codes.NotFound:
 				traceError(&bctx, fnName, err)
+				return ast.NullTerm(), err
+			case err != nil:
 				return nil, err
 			default:
 				return ast.StringTerm(user.Id), nil

diff --git a/builtins/edge/ds/object.go b/builtins/edge/ds/object.go
@@ -87,11 +87,12 @@ func RegisterObject(logger *zerolog.Logger, fnName string, dr resolvers.Director
 			}
 
 			resp, err := client.GetObject(bctx.Context, req)
-			if err != nil {
+			switch {
+			case status.Code(err) == codes.NotFound:
 				traceError(&bctx, fnName, err)
-				if status.Code(err) != codes.NotFound {
-					return nil, err
-				}
+				return ast.NullTerm(), err
+			case err != nil:
+				return nil, err
 			}
 
 			buf := new(bytes.Buffer)

diff --git a/builtins/edge/ds/relation.go b/builtins/edge/ds/relation.go
@@ -108,11 +108,12 @@ func RegisterRelation(logger *zerolog.Logger, fnName string, dr resolvers.Direct
 			}
 
 			resp, err := client.GetRelation(bctx.Context, &args)
-			if err != nil {
+			switch {
+			case status.Code(err) == codes.NotFound:
 				traceError(&bctx, fnName, err)
-				if status.Code(err) != codes.NotFound {
-					return nil, err
-				}
+				return ast.NullTerm(), err
+			case err != nil:
+				return nil, err
 			}
 
 			buf := new(bytes.Buffer)

diff --git a/builtins/edge/ds/user.go b/builtins/edge/ds/user.go
@@ -73,11 +73,12 @@ func RegisterUser(logger *zerolog.Logger, fnName string, dr resolvers.DirectoryR
 				ObjectId:      args.ID,
 				WithRelations: false,
 			})
-			if err != nil {
+			switch {
+			case status.Code(err) == codes.NotFound:
 				traceError(&bctx, fnName, err)
-				if status.Code(err) != codes.NotFound {
-					return nil, err
-				}
+				return ast.NullTerm(), err
+			case err != nil:
+				return nil, err
 			}
 
 			buf := new(bytes.Buffer)

diff --git a/pkg/app/tests/builtin/builtin_test.go b/pkg/app/tests/builtin/builtin_test.go
@@ -164,3 +164,71 @@ func TestBuiltinsHelp(t *testing.T) {
 		t.Run(tc.name, f)
 	}
 }
+
+var builtinNotFoundErrTests = []struct {
+	name     string
+	query    string
+	expected map[string]interface{}
+}{
+	{
+		name:  "ds.identity",
+		query: `x = ds.identity({"id": "no_existing_identifier"})`,
+	},
+	{
+		name:  "ds.user",
+		query: `x = ds.user({"id": "none_existing_user_object_id"})`,
+	},
+	{
+		name:  "ds.object",
+		query: `x = ds.object({"object_type": "none_existing_type", "object_id": "none_existing_id"})`,
+	},
+	{
+		name: "ds.relation",
+		query: `x = ds.relation({
+			"object_type": "none_existing_object_type", 
+			"object_id": "none_existing_object_id",
+			"relation": "none_existing_relation",
+			"subject_type": "none_existing_subject_type",
+			"subject_id": "none_existing_subject_id",
+			})`,
+	},
+	{
+		name: "ds.relation.with.subject_relation",
+		query: `x = ds.relation({
+			"object_type": "none_existing_object_type", 
+			"object_id": "none_existing_object_id",
+			"relation": "none_existing_relation",
+			"subject_type": "none_existing_subject_type",
+			"subject_id": "none_existing_subject_id",
+			"subject_relation": "none_existing_subject_relation",
+			})`,
+	},
+}
+
+func TestBuiltinsNotFoundErr(t *testing.T) {
+	harness := atesting.SetupOnline(t, func(cfg *config.Config) {
+		cfg.Edge.DBPath = atesting.AssetAcmeDBFilePath()
+	})
+	defer harness.Cleanup()
+
+	client := harness.CreateGRPCClient()
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	for _, tc := range builtinNotFoundErrTests {
+		f := func(t *testing.T) {
+			resp, err := client.Query(ctx, &authz2.QueryRequest{
+				Query: tc.query,
+			})
+			require.NoError(t, err)
+			require.NotNil(t, resp)
+			require.NotNil(t, resp.Response)
+
+			r := resp.Response.AsMap()
+			require.NotNil(t, r)
+		}
+
+		t.Run(tc.name, f)
+	}
+}