diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index 12bd7b5c8b3..6b3f71be317 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -17,12 +17,12 @@ import ( // // This function is particularly useful when dealing with CS3 responses, // and a unified error handling within the application is necessary. -func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *Error { +func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) error { if inerr != nil { - return &Error{msg: inerr.Error(), errorCode: GeneralException} + return Error{msg: inerr.Error(), errorCode: GeneralException} } - err := &Error{errorCode: GeneralException, msg: "unspecified error has occurred"} + err := Error{errorCode: GeneralException, msg: "unspecified error has occurred"} if status != nil { err.msg = status.GetMessage() @@ -33,7 +33,7 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *E case slices.Contains(ignore, status.GetCode()): fallthrough case code == cs3rpc.Code_CODE_OK: - err = nil + return nil case code == cs3rpc.Code_CODE_NOT_FOUND: err.errorCode = ItemNotFound case code == cs3rpc.Code_CODE_PERMISSION_DENIED: @@ -61,11 +61,11 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *E return err } -// FromStat transforms a *provider.StatResponse object and an error into an *Error. +// FromStat transforms a *provider.StatResponse object and an error into an Error. // // It takes a stat of type *provider.StatResponse, an error, and a variadic parameter of type cs3rpc.Code. // It invokes the FromCS3Status function with the StatResponse Status and the ignore codes. -func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) *Error { +func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) error { // TODO: look into ResourceInfo to get the postprocessing state and map that to 425 status? return FromCS3Status(stat.GetStatus(), err, ignore...) } diff --git a/services/graph/pkg/errorcode/cs3_test.go b/services/graph/pkg/errorcode/cs3_test.go index f4a1f60a198..5b160e7ad78 100644 --- a/services/graph/pkg/errorcode/cs3_test.go +++ b/services/graph/pkg/errorcode/cs3_test.go @@ -8,45 +8,44 @@ import ( cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/owncloud/ocis/v2/ocis-pkg/conversions" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" ) func TestFromCS3Status(t *testing.T) { var tests = []struct { - status *cs3rpc.Status - err error - ignore []cs3rpc.Code - result *errorcode.Error + status *cs3rpc.Status + err error + ignore []cs3rpc.Code + expected error }{ - {nil, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "unspecified error has occurred"))}, - {nil, errors.New("test error"), nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "test error"))}, + {nil, nil, nil, errorcode.New(errorcode.GeneralException, "unspecified error has occurred")}, + {nil, errors.New("test error"), nil, errorcode.New(errorcode.GeneralException, "test error")}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}, nil, nil, nil}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND}, nil}, {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED}, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRange, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ServiceNotAvailable, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.QuotaLimitReached, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemIsLocked, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, nil, errorcode.New(errorcode.ItemNotFound, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, nil, errorcode.New(errorcode.AccessDenied, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, errorcode.New(errorcode.Unauthenticated, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRequest, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, errorcode.New(errorcode.NameAlreadyExists, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRequest, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.NotSupported, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, nil, errorcode.New(errorcode.InvalidRange, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, nil, errorcode.New(errorcode.ServiceNotAvailable, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, nil, errorcode.New(errorcode.GeneralException, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, nil, errorcode.New(errorcode.QuotaLimitReached, "msg")}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, nil, errorcode.New(errorcode.ItemIsLocked, "msg")}, } for _, test := range tests { - if output := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(output, test.result) { - t.Error("Test Failed: {} expected, received: {}", test.result, output) + if got := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(got, test.expected) { + t.Error("Test Failed: {} expected, received: {}", test.expected, got) } } } @@ -55,9 +54,9 @@ func TestFromStat(t *testing.T) { var tests = []struct { stat *provider.StatResponse err error - result *errorcode.Error + result error }{ - {nil, errors.New("some error"), conversions.ToPointer(errorcode.New(errorcode.GeneralException, "some error"))}, + {nil, errors.New("some error"), errorcode.New(errorcode.GeneralException, "some error")}, {&provider.StatResponse{Status: &cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}}, nil, nil}, } diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index 88bb421f24b..f4a9bda4102 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -166,7 +166,8 @@ func RenderError(w http.ResponseWriter, r *http.Request, err error) { var errcode Error if errors.As(err, &errcode) { errcode.Render(w, r) - } else { - GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) + return } + + GeneralException.Render(w, r, http.StatusInternalServerError, err.Error()) } diff --git a/services/graph/pkg/errorcode/errorcode_test.go b/services/graph/pkg/errorcode/errorcode_test.go new file mode 100644 index 00000000000..003bdfc8f96 --- /dev/null +++ b/services/graph/pkg/errorcode/errorcode_test.go @@ -0,0 +1,45 @@ +package errorcode_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" +) + +type customErr struct{} + +func (customErr) Error() string { + return "some error" +} + +func TestRenderError(t *testing.T) { + t.Parallel() + + t.Run("errorcode.Error value error", func(t *testing.T) { + r := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + err := errorcode.New(errorcode.ItemNotFound, "test error") + errorcode.RenderError(w, r, err) + require.Equal(t, http.StatusNotFound, w.Code) + }) + + t.Run("errorcode.Error zero value error", func(t *testing.T) { + r := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + var err errorcode.Error + errorcode.RenderError(w, r, err) + require.Equal(t, http.StatusForbidden, w.Code) + }) + + t.Run("custom error", func(t *testing.T) { + r := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + var err customErr + errorcode.RenderError(w, r, err) + require.Equal(t, http.StatusInternalServerError, w.Code) + }) +} diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions.go b/services/graph/pkg/service/v0/api_driveitem_permissions.go index 1fd6caa56b1..a3525f1d422 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions.go @@ -83,9 +83,9 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId stor } statResponse, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{Ref: &storageprovider.Reference{ResourceId: &resourceId}}) - if errCode := errorcode.FromStat(statResponse, err); errCode != nil { - s.logger.Warn().Err(errCode).Interface("stat.res", statResponse).Msg("stat failed") - return libregraph.Permission{}, *errCode + if err := errorcode.FromStat(statResponse, err); err != nil { + s.logger.Warn().Err(err).Interface("stat.res", statResponse).Msg("stat failed") + return libregraph.Permission{}, err } resourceInfo := statResponse.GetInfo() @@ -178,9 +178,9 @@ func (s DriveItemPermissionsService) Invite(ctx context.Context, resourceId stor } createShareResponse, err := gatewayClient.CreateShare(ctx, createShareRequest) - if errCode := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); errCode != nil { + if err := errorcode.FromCS3Status(createShareResponse.GetStatus(), err); err != nil { s.logger.Debug().Err(err).Msg("share creation failed") - return libregraph.Permission{}, *errCode + return libregraph.Permission{}, err } if id := createShareResponse.GetShare().GetId().GetOpaqueId(); id != "" { @@ -227,8 +227,8 @@ func (s DriveItemPermissionsService) ListPermissions(ctx context.Context, itemID } statResponse, err := gatewayClient.Stat(ctx, &storageprovider.StatRequest{Ref: &storageprovider.Reference{ResourceId: &itemID}}) - if errCode := errorcode.FromStat(statResponse, err); errCode != nil { - s.logger.Warn().Err(errCode).Interface("stat.res", statResponse).Msg("stat failed") + if err := errorcode.FromStat(statResponse, err); err != nil { + s.logger.Warn().Err(err).Interface("stat.res", statResponse).Msg("stat failed") return collectionOfPermissions, err } diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_links.go b/services/graph/pkg/service/v0/api_driveitem_permissions_links.go index c1410d880c8..3de834a331d 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_links.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_links.go @@ -290,8 +290,8 @@ func (s DriveItemPermissionsService) updatePublicLinkPermission(ctx context.Cont }, }) - if errCode := errorcode.FromCS3Status(statResp.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(statResp.GetStatus(), err); err != nil { + return nil, err } if newPermission.HasExpirationDateTime() { @@ -369,8 +369,8 @@ func (s DriveItemPermissionsService) updatePublicLinkPassword(ctx context.Contex }, }, }) - if errCode := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); err != nil { + return nil, err } permission, err := s.libreGraphPermissionFromCS3PublicShare(changeLinkRes.GetShare()) if err != nil { @@ -396,8 +396,8 @@ func (s DriveItemPermissionsService) updatePublicLink(ctx context.Context, permi }, }) - if errCode := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(changeLinkRes.GetStatus(), err); err != nil { + return nil, err } permission, err := s.libreGraphPermissionFromCS3PublicShare(changeLinkRes.GetShare()) diff --git a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go index 3c2d25bb829..5e63dec64ed 100644 --- a/services/graph/pkg/service/v0/api_driveitem_permissions_test.go +++ b/services/graph/pkg/service/v0/api_driveitem_permissions_test.go @@ -27,6 +27,10 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/stretchr/testify/mock" + "github.com/tidwall/gjson" + "google.golang.org/grpc" + "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/mocks" "github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults" @@ -35,9 +39,6 @@ import ( "github.com/owncloud/ocis/v2/services/graph/pkg/linktype" svc "github.com/owncloud/ocis/v2/services/graph/pkg/service/v0" "github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole" - "github.com/stretchr/testify/mock" - "github.com/tidwall/gjson" - "google.golang.org/grpc" ) var _ = Describe("DriveItemPermissionsService", func() { @@ -649,7 +650,6 @@ var _ = Describe("DriveItemPermissionsService", func() { }, }, } - }) It("fails when no share is found", func() { getShareMockResponse.Share = nil diff --git a/services/graph/pkg/service/v0/api_drives_drive_item.go b/services/graph/pkg/service/v0/api_drives_drive_item.go index 2b874d5eb31..882132fa0d1 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item.go @@ -71,11 +71,11 @@ func (s DrivesDriveItemService) UnmountShare(ctx context.Context, resourceID sto }, }, ) - if errCode := errorcode.FromCS3Status(getReceivedShareResponse.GetStatus(), err); errCode != nil { - s.logger.Debug().Err(errCode). + if err := errorcode.FromCS3Status(getReceivedShareResponse.GetStatus(), err); err != nil { + s.logger.Debug().Err(err). Str("shareid", shareId). Msg("failed to read share") - return errCode + return err } // Find all accepted shares for this resource diff --git a/services/graph/pkg/service/v0/api_drives_drive_item_test.go b/services/graph/pkg/service/v0/api_drives_drive_item_test.go index 960d58d7259..6c7f4684bea 100644 --- a/services/graph/pkg/service/v0/api_drives_drive_item_test.go +++ b/services/graph/pkg/service/v0/api_drives_drive_item_test.go @@ -24,6 +24,7 @@ import ( "github.com/cs3org/reva/v2/pkg/rgrpc/status" "github.com/cs3org/reva/v2/pkg/storagespace" cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks" + "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/mocks" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" @@ -377,7 +378,7 @@ var _ = Describe("DrivesDriveItemService", func() { Return(&collaborationv1beta1.GetReceivedShareResponse{}, errors.New("listing shares failed")) err := drivesDriveItemService.UnmountShare(context.Background(), storageprovider.ResourceId{}) - Expect(err).To(MatchError(&expectedError)) + Expect(err).To(MatchError(expectedError)) }) It("uses the correct filters to get the shares", func() { diff --git a/services/graph/pkg/service/v0/base.go b/services/graph/pkg/service/v0/base.go index e1bec910931..9df32420fbe 100644 --- a/services/graph/pkg/service/v0/base.go +++ b/services/graph/pkg/service/v0/base.go @@ -21,13 +21,14 @@ import ( "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" libregraph "github.com/owncloud/libre-graph-api-go" + "google.golang.org/protobuf/types/known/fieldmaskpb" + "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/pkg/config" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" "github.com/owncloud/ocis/v2/services/graph/pkg/identity" "github.com/owncloud/ocis/v2/services/graph/pkg/linktype" "github.com/owncloud/ocis/v2/services/graph/pkg/unifiedrole" - "google.golang.org/protobuf/types/known/fieldmaskpb" ) // BaseGraphService implements a couple of helper functions that are @@ -418,8 +419,8 @@ func (g BaseGraphService) getCS3PublicShareByID(ctx context.Context, permissionI }, }, ) - if errCode := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); err != nil { + return nil, err } return getPublicShareResp.GetShare(), nil } @@ -441,8 +442,8 @@ func (g BaseGraphService) removePublicShare(ctx context.Context, permissionID st }, }, }) - if errcode := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); errcode != nil { - return *errcode + if err := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); err != nil { + return err } // We need to return an untyped nil here otherwise the error==nil check won't work return nil @@ -466,8 +467,8 @@ func (g BaseGraphService) removeUserShare(ctx context.Context, permissionID stri }, }) - if errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { - return *errCode + if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { + return err } // We need to return an untyped nil here otherwise the error==nil check won't work return nil @@ -495,8 +496,8 @@ func (g BaseGraphService) removeSpacePermission(ctx context.Context, permissionI }, }) - if errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { - return *errCode + if err := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); err != nil { + return err } // We need to return an untyped nil here otherwise the error==nil check won't work return nil @@ -527,8 +528,8 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID }, }, }) - if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(getShareResp.GetStatus(), err); err != nil { + return nil, err } return getShareResp.GetShare(), nil } @@ -667,8 +668,8 @@ func (g BaseGraphService) updateUserShare(ctx context.Context, permissionID stri } updateUserShareResp, err := gatewayClient.UpdateShare(ctx, &cs3UpdateShareReq) - if errCode := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); errCode != nil { - return nil, *errCode + if err := errorcode.FromCS3Status(updateUserShareResp.GetStatus(), err); err != nil { + return nil, err } permission, err := g.cs3UserShareToPermission(ctx, updateUserShareResp.GetShare(), IsSpaceRoot(itemID)) diff --git a/services/graph/pkg/service/v0/drives.go b/services/graph/pkg/service/v0/drives.go index b8416210605..60527674520 100644 --- a/services/graph/pkg/service/v0/drives.go +++ b/services/graph/pkg/service/v0/drives.go @@ -27,7 +27,7 @@ import ( revactx "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/storagespace" "github.com/cs3org/reva/v2/pkg/utils" - "github.com/owncloud/ocis/v2/ocis-pkg/conversions" + "github.com/owncloud/ocis/v2/ocis-pkg/l10n" "github.com/owncloud/ocis/v2/ocis-pkg/service/grpc" v0 "github.com/owncloud/ocis/v2/protogen/gen/ocis/messages/settings/v0" @@ -78,7 +78,7 @@ func (g Graph) GetDrives(version APIVersion) http.HandlerFunc { func (g Graph) GetDrivesV1(w http.ResponseWriter, r *http.Request) { spaces, errCode := g.getDrives(r, false, APIVersion_1) if errCode != nil { - errCode.Render(w, r) + errorcode.RenderError(w, r, errCode) return } @@ -99,7 +99,7 @@ func (g Graph) GetDrivesV1(w http.ResponseWriter, r *http.Request) { func (g Graph) GetDrivesV1Beta1(w http.ResponseWriter, r *http.Request) { spaces, errCode := g.getDrives(r, false, APIVersion_1_Beta_1) if errCode != nil { - errCode.Render(w, r) + errorcode.RenderError(w, r, errCode) return } @@ -133,7 +133,7 @@ func (g Graph) GetAllDrives(version APIVersion) http.HandlerFunc { func (g Graph) GetAllDrivesV1(w http.ResponseWriter, r *http.Request) { spaces, errCode := g.getDrives(r, true, APIVersion_1) if errCode != nil { - errCode.Render(w, r) + errorcode.RenderError(w, r, errCode) return } @@ -153,7 +153,7 @@ func (g Graph) GetAllDrivesV1(w http.ResponseWriter, r *http.Request) { func (g Graph) GetAllDrivesV1Beta1(w http.ResponseWriter, r *http.Request) { drives, errCode := g.getDrives(r, true, APIVersion_1_Beta_1) if errCode != nil { - errCode.Render(w, r) + errorcode.RenderError(w, r, errCode) return } @@ -168,7 +168,7 @@ func (g Graph) GetAllDrivesV1Beta1(w http.ResponseWriter, r *http.Request) { } // getDrives implements the Service interface. -func (g Graph) getDrives(r *http.Request, unrestricted bool, apiVersion APIVersion) ([]*libregraph.Drive, *errorcode.Error) { +func (g Graph) getDrives(r *http.Request, unrestricted bool, apiVersion APIVersion) ([]*libregraph.Drive, error) { logger := g.logger.SubloggerWithRequestID(r.Context()) logger.Info(). Interface("query", r.URL.Query()). @@ -179,20 +179,20 @@ func (g Graph) getDrives(r *http.Request, unrestricted bool, apiVersion APIVersi odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query()) if err != nil { logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get drives: query error") - return nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, err.Error())) + return nil, errorcode.New(errorcode.InvalidRequest, err.Error()) } ctx := r.Context() filters, err := generateCs3Filters(odataReq) if err != nil { logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get drives: error parsing filters") - return nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, err.Error())) + return nil, errorcode.New(errorcode.NotSupported, err.Error()) } if !unrestricted { user, ok := revactx.ContextGetUser(r.Context()) if !ok { logger.Debug().Msg("could not create drive: invalid user") - return nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "invalid user")) + return nil, errorcode.New(errorcode.AccessDenied, "invalid user") } filters = append(filters, &storageprovider.ListStorageSpacesRequest_Filter{ Type: storageprovider.ListStorageSpacesRequest_Filter_TYPE_USER, @@ -210,32 +210,32 @@ func (g Graph) getDrives(r *http.Request, unrestricted bool, apiVersion APIVersi switch { case err != nil: logger.Error().Err(err).Msg("could not get drives: transport error") - return nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, err.Error())) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) case res.Status.Code != cs3rpc.Code_CODE_OK: if res.Status.Code == cs3rpc.Code_CODE_NOT_FOUND { // ok, empty return return nil, nil } logger.Debug().Str("message", res.GetStatus().GetMessage()).Msg("could not get drives: grpc error") - return nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, res.Status.Message)) + return nil, errorcode.New(errorcode.GeneralException, res.Status.Message) } webDavBaseURL, err := g.getWebDavBaseURL() if err != nil { logger.Error().Err(err).Str("url", webDavBaseURL.String()).Msg("could not get drives: error parsing url") - return nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, err.Error())) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } spaces, err := g.formatDrives(ctx, webDavBaseURL, res.StorageSpaces, apiVersion) if err != nil { logger.Debug().Err(err).Msg("could not get drives: error parsing grpc response") - return nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, err.Error())) + return nil, errorcode.New(errorcode.GeneralException, err.Error()) } spaces, err = sortSpaces(odataReq, spaces) if err != nil { logger.Debug().Err(err).Msg("could not get drives: error sorting the spaces list according to query") - return nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, err.Error())) + return nil, errorcode.New(errorcode.InvalidRequest, err.Error()) } return spaces, nil diff --git a/services/graph/pkg/service/v0/sharedwithme.go b/services/graph/pkg/service/v0/sharedwithme.go index 42d02d76c55..8d88206d272 100644 --- a/services/graph/pkg/service/v0/sharedwithme.go +++ b/services/graph/pkg/service/v0/sharedwithme.go @@ -34,9 +34,9 @@ func (g Graph) listSharedWithMe(ctx context.Context) ([]libregraph.DriveItem, er } listReceivedSharesResponse, err := gatewayClient.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{}) - if errCode := errorcode.FromCS3Status(listReceivedSharesResponse.GetStatus(), err); errCode != nil { + if err := errorcode.FromCS3Status(listReceivedSharesResponse.GetStatus(), err); err != nil { g.logger.Error().Err(err).Msg("listing shares failed") - return nil, *errCode + return nil, err } return cs3ReceivedSharesToDriveItems(ctx, g.logger, gatewayClient, g.identityCache, listReceivedSharesResponse.GetShares()) diff --git a/services/graph/pkg/service/v0/utils.go b/services/graph/pkg/service/v0/utils.go index 55311185b67..24ef3da4e2f 100644 --- a/services/graph/pkg/service/v0/utils.go +++ b/services/graph/pkg/service/v0/utils.go @@ -3,6 +3,7 @@ package svc import ( "context" "encoding/json" + "errors" "io" "net/http" "reflect" @@ -16,6 +17,7 @@ import ( "golang.org/x/sync/errgroup" libregraph "github.com/owncloud/libre-graph-api-go" + "github.com/owncloud/ocis/v2/ocis-pkg/log" "github.com/owncloud/ocis/v2/services/graph/pkg/errorcode" "github.com/owncloud/ocis/v2/services/graph/pkg/identity" @@ -166,12 +168,15 @@ func cs3ReceivedSharesToDriveItems(ctx context.Context, }, }) - switch errCode := errorcode.FromCS3Status(shareStat.GetStatus(), err); { - case errCode == nil: - break + var errCode errorcode.Error + errors.As(errorcode.FromCS3Status(shareStat.GetStatus(), err), &errCode) + + switch { // skip ItemNotFound shares, they might have been deleted in the meantime or orphans. case errCode.GetCode() == errorcode.ItemNotFound: return nil + case err == nil: + break default: logger.Error().Err(errCode).Msg("could not stat") return errCode