Handle "not found" errors properly in user- and groupprovider

Return a proper NOT_FOUND error instead of INTERNAL when a user or group is not found.

changelog/unreleased/handle-user-group-not-found.md

@@ -0,0 +1,3 @@
+Enhancement: Return a proper NOT_FOUND error when a user or group is not found
+
+https://github.com/cs3org/reva/pull/2282

internal/grpc/services/groupprovider/groupprovider.go

@@ -103,10 +103,14 @@ func (s *service) Register(ss *grpc.Server) {
 func (s *service) GetGroup(ctx context.Context, req *grouppb.GetGroupRequest) (*grouppb.GetGroupResponse, error) {
 	group, err := s.groupmgr.GetGroup(ctx, req.GroupId)
 	if err != nil {
-		err = errors.Wrap(err, "groupprovidersvc: error getting group")
-		return &grouppb.GetGroupResponse{
-			Status: status.NewInternal(ctx, err, "error getting group"),
-		}, nil
+		res := &grouppb.GetGroupResponse{}
+		if _, ok := err.(errtypes.NotFound); ok {
+			res.Status = status.NewNotFound(ctx, fmt.Sprintf("group not found"))
+		} else {
+			err = errors.Wrap(err, "groupprovidersvc: error getting group")
+			res.Status = status.NewInternal(ctx, err, "error getting group")
+		}
+		return res, nil
 	}
 
 	return &grouppb.GetGroupResponse{
@@ -118,10 +122,14 @@ func (s *service) GetGroup(ctx context.Context, req *grouppb.GetGroupRequest) (*
 func (s *service) GetGroupByClaim(ctx context.Context, req *grouppb.GetGroupByClaimRequest) (*grouppb.GetGroupByClaimResponse, error) {
 	group, err := s.groupmgr.GetGroupByClaim(ctx, req.Claim, req.Value)
 	if err != nil {
-		err = errors.Wrap(err, "groupprovidersvc: error getting group by claim")
-		return &grouppb.GetGroupByClaimResponse{
-			Status: status.NewInternal(ctx, err, "error getting group by claim"),
-		}, nil
+		res := &grouppb.GetGroupByClaimResponse{}
+		if _, ok := err.(errtypes.NotFound); ok {
+			res.Status = status.NewNotFound(ctx, fmt.Sprintf("group not found %s %s", req.Claim, req.Value))
+		} else {
+			err = errors.Wrap(err, "groupprovidersvc: error getting group by claim")
+			res.Status = status.NewInternal(ctx, err, "error getting group by claim")
+		}
+		return res, nil
 	}
 
 	return &grouppb.GetGroupByClaimResponse{

internal/grpc/services/userprovider/userprovider.go

@@ -127,10 +127,12 @@ func (s *service) Register(ss *grpc.Server) {
 func (s *service) GetUser(ctx context.Context, req *userpb.GetUserRequest) (*userpb.GetUserResponse, error) {
 	user, err := s.usermgr.GetUser(ctx, req.UserId)
 	if err != nil {
-		// TODO(labkode): check for not found.
-		err = errors.Wrap(err, "userprovidersvc: error getting user")
-		res := &userpb.GetUserResponse{
-			Status: status.NewInternal(ctx, err, "error getting user"),
+		res := &userpb.GetUserResponse{}
+		if _, ok := err.(errtypes.NotFound); ok {
+			res.Status = status.NewNotFound(ctx, "user not found")
+		} else {
+			err = errors.Wrap(err, "userprovidersvc: error getting user")
+			res.Status = status.NewInternal(ctx, err, "error getting user")
 		}
 		return res, nil
 	}
@@ -145,10 +147,12 @@ func (s *service) GetUser(ctx context.Context, req *userpb.GetUserRequest) (*use
 func (s *service) GetUserByClaim(ctx context.Context, req *userpb.GetUserByClaimRequest) (*userpb.GetUserByClaimResponse, error) {
 	user, err := s.usermgr.GetUserByClaim(ctx, req.Claim, req.Value)
 	if err != nil {
-		// TODO(labkode): check for not found.
-		err = errors.Wrap(err, "userprovidersvc: error getting user by claim")
-		res := &userpb.GetUserByClaimResponse{
-			Status: status.NewInternal(ctx, err, "error getting user by claim"),
+		res := &userpb.GetUserByClaimResponse{}
+		if _, ok := err.(errtypes.NotFound); ok {
+			res.Status = status.NewNotFound(ctx, fmt.Sprintf("user not found %s %s", req.Claim, req.Value))
+		} else {
+			err = errors.Wrap(err, "userprovidersvc: error getting user by claim")
+			res.Status = status.NewInternal(ctx, err, "error getting user by claim")
 		}
 		return res, nil
 	}

tests/integration/grpc/userprovider_test.go

@@ -134,7 +134,7 @@ var _ = Describe("user providers", func() {
 					},
 					want: &userpb.GetUserResponse{
 						Status: &rpc.Status{
-							Code: 15,
+							Code: 6,
 						},
 					},
 				},
@@ -147,7 +147,7 @@ var _ = Describe("user providers", func() {
 					},
 					want: &userpb.GetUserResponse{
 						Status: &rpc.Status{
-							Code: 15,
+							Code: 6,
 						},
 					},
 				},
@@ -160,7 +160,7 @@ var _ = Describe("user providers", func() {
 					},
 					want: &userpb.GetUserResponse{
 						Status: &rpc.Status{
-							Code: 15,
+							Code: 6,
 						},
 					},
 				},