Skip to content

Commit

Permalink
fix: ensure that both value and dynamic type are nil
Browse files Browse the repository at this point in the history
  • Loading branch information
fschade committed Apr 11, 2024
1 parent d742a2c commit 28607a8
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 16 deletions.
9 changes: 8 additions & 1 deletion services/graph/pkg/errorcode/cs3.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,14 @@ func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) er
err.errorCode = ItemIsLocked
}

return err
switch err {
// make sure value and dynamic type are both nil, [nil, *Error] != [nil, nil], [nil, nil] == [nil, nil],
// a named export would solve this too but is less explicit
case nil: // [nil, *Error]
return nil
default:
return err
}
}

// FromStat transforms a *provider.StatResponse object and an error into an *Error.
Expand Down
14 changes: 7 additions & 7 deletions services/graph/pkg/errorcode/cs3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ import (

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"))},
Expand Down Expand Up @@ -45,8 +45,8 @@ func TestFromCS3Status(t *testing.T) {
}

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)
}
}
}
Expand All @@ -55,7 +55,7 @@ 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"))},
{&provider.StatResponse{Status: &cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}}, nil, nil},
Expand Down
13 changes: 7 additions & 6 deletions services/graph/pkg/service/v0/api_driveitem_permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ 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/conversions"
"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"
Expand All @@ -35,9 +40,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() {
Expand Down Expand Up @@ -205,7 +207,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
statResponse.Status = status.NewNotFound(context.Background(), "not found")
permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(errorcode.New(errorcode.ItemNotFound, "not found")))
Expect(err).To(MatchError(conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "not found"))))
Expect(permission).To(BeZero())
})
})
Expand Down Expand Up @@ -649,7 +651,6 @@ var _ = Describe("DriveItemPermissionsService", func() {
},
},
}

})
It("fails when no share is found", func() {
getShareMockResponse.Share = nil
Expand Down Expand Up @@ -947,7 +948,7 @@ var _ = Describe("DriveItemPermissionsService", func() {

driveItemPermission.SetExpirationDateTime(expiration)
res, err := driveItemPermissionsService.UpdatePermission(context.Background(), driveItemId, "permissionid", driveItemPermission)
Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "expiration date is in the past")))
Expect(err).To(MatchError(conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "expiration date is in the past"))))
Expect(res).To(BeZero())
})
})
Expand Down
5 changes: 3 additions & 2 deletions services/graph/pkg/service/v0/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -535,7 +536,7 @@ func (g BaseGraphService) getCS3UserShareByID(ctx context.Context, permissionID

func (g BaseGraphService) getPermissionByID(ctx context.Context, permissionID string, itemID *storageprovider.ResourceId) (*libregraph.Permission, *storageprovider.ResourceId, error) {
publicShare, err := g.getCS3PublicShareByID(ctx, permissionID)
var errcode errorcode.Error
var errcode *errorcode.Error
switch {
case err == nil:
// the id is referencing a public share
Expand Down

0 comments on commit 28607a8

Please sign in to comment.