Skip to content

Commit

Permalink
Use field mask to update received shares
Browse files Browse the repository at this point in the history
The next step for the shares storahge provider is to introduce field
masks so the shares can update state and mount point. This PR is based
on the full changeset, but removes everything but the API change.

Add a sharesstorageprovider

The provider exposes all received shares. It can be mounted to /home/Shares
to make a lot of special cases for shares handling in the gateway and
storage drivers superfluous.

Add missing mock file

Implement methods for setting/unsetting arbitrary metadata

Fix tests

Adapt to changes from rebase

Fix creating references with embedded mounts

Fix corner cases when stating shares

Allow moves between shares on the same storage

Make the storage rules known to the gateway as well

Do not choke on non-existent shares

Reject a share when it is being deleted

Fix rebase artifacts

WIP: Refactor statting shares. Merge shares permissions.

update after rebase, fix tests

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

list all shares

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

work on api change

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

Fix build

Persist mountpoints in the share managers followin the new cs3 api

Add support for renaming shares in the SharesStorageprovider

Adapt commands for updating received shares

Regenerate the share manager mock

Fix linter warning

Do not raise an internal error when trying to access non-existent shares

Make hound happy

Fix wrong column name in query

Fix typo

Do not confuse user and group names

Add test for listing received group shares

Do not list parent group shares if there is a child share for it already

Make hound happy

Hide the fact that accepted groups shares can be child shares in the db

list shares using the shares manager + hide group shares when the same resource has a user and group share

refactor all the ocs error writing from the new code

Only collide with mountpoints of shares pointing do different resources

Also return shares being shared with one of the user's groups

Add sharesstorageprovider service file for local acceptance tests

Adapt nextcloud share manager to new method signature

Also remove the test for UpdateReceivedShare which can not be tested
anymore with the new signature. The ReceivedShare never held the display
name that's being tested so the test only passed on the data from the
update field, but since the method only takes the actual received share
now this is no longer possible.

WIP: use go-cs3apis fork until it has been merged

Add placeholder changelog to make CI run

Tweak documentation on how to run the acceptance tests

Add missing storage registry rule for the sharesstorageprovider

Fix revad config for local acceptance tests

reduce changes to share api change

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>

JSON encode the FieldMask parameter as-is

paramsObj.Ref -> paramsObj.ReceivedShare

Restore deleted test

update code comment

remove go mod replace

Signed-off-by: Jörn Friedrich Dreyer <jfd@butonic.de>
  • Loading branch information
aduffeck authored and butonic committed Oct 4, 2021
1 parent 4b473fd commit d7e2e9f
Show file tree
Hide file tree
Showing 26 changed files with 560 additions and 174 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/sharemanager-api-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Change: Sharemanager API change

This PR updates reva to reflect the share manager CS3 API changes.

https://github.com/cs3org/reva/pull/2121
26 changes: 17 additions & 9 deletions cmd/reva/ocm-share-update-received.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1"
"github.com/pkg/errors"
"google.golang.org/protobuf/types/known/fieldmaskpb"
)

func ocmShareUpdateReceivedCommand() *command {
Expand Down Expand Up @@ -57,28 +58,35 @@ func ocmShareUpdateReceivedCommand() *command {

shareState := getOCMShareState(*state)

shareRequest := &ocm.UpdateReceivedOCMShareRequest{
shareRes, err := shareClient.GetReceivedOCMShare(ctx, &ocm.GetReceivedOCMShareRequest{
Ref: &ocm.ShareReference{
Spec: &ocm.ShareReference_Id{
Id: &ocm.ShareId{
OpaqueId: id,
},
},
},
Field: &ocm.UpdateReceivedOCMShareRequest_UpdateField{
Field: &ocm.UpdateReceivedOCMShareRequest_UpdateField_State{
State: shareState,
},
},
})
if err != nil {
return err
}
if shareRes.Status.Code != rpc.Code_CODE_OK {
return formatError(shareRes.Status)
}
shareRes.Share.State = shareState

shareRequest := &ocm.UpdateReceivedOCMShareRequest{
Share: shareRes.Share,
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state"}},
}

shareRes, err := shareClient.UpdateReceivedOCMShare(ctx, shareRequest)
updateRes, err := shareClient.UpdateReceivedOCMShare(ctx, shareRequest)
if err != nil {
return err
}

if shareRes.Status.Code != rpc.Code_CODE_OK {
return formatError(shareRes.Status)
if updateRes.Status.Code != rpc.Code_CODE_OK {
return formatError(updateRes.Status)
}

fmt.Println("OK")
Expand Down
26 changes: 17 additions & 9 deletions cmd/reva/share-update-received.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
"github.com/pkg/errors"
"google.golang.org/protobuf/types/known/fieldmaskpb"
)

func shareUpdateReceivedCommand() *command {
Expand Down Expand Up @@ -57,28 +58,35 @@ func shareUpdateReceivedCommand() *command {

shareState := getShareState(*state)

shareRequest := &collaboration.UpdateReceivedShareRequest{
shareRes, err := shareClient.GetReceivedShare(ctx, &collaboration.GetReceivedShareRequest{
Ref: &collaboration.ShareReference{
Spec: &collaboration.ShareReference_Id{
Id: &collaboration.ShareId{
OpaqueId: id,
},
},
},
Field: &collaboration.UpdateReceivedShareRequest_UpdateField{
Field: &collaboration.UpdateReceivedShareRequest_UpdateField_State{
State: shareState,
},
},
})
if err != nil {
return err
}
if shareRes.Status.Code != rpc.Code_CODE_OK {
return formatError(shareRes.Status)
}
shareRes.Share.State = shareState

shareRequest := &collaboration.UpdateReceivedShareRequest{
Share: shareRes.Share,
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state"}},
}

shareRes, err := shareClient.UpdateReceivedShare(ctx, shareRequest)
updateRes, err := shareClient.UpdateReceivedShare(ctx, shareRequest)
if err != nil {
return err
}

if shareRes.Status.Code != rpc.Code_CODE_OK {
return formatError(shareRes.Status)
if updateRes.Status.Code != rpc.Code_CODE_OK {
return formatError(updateRes.Status)
}

fmt.Println("OK")
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ require (
github.com/cheggaaa/pb v1.0.29
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
github.com/cs3org/go-cs3apis v0.0.0-20210922150613-cb9e3c99f8de
github.com/cs3org/go-cs3apis v0.0.0-20211004093007-d29741980082
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8
github.com/eventials/go-tus v0.0.0-20200718001131-45c7ec8f5d59
github.com/gdexlab/go-render v1.0.1
Expand Down Expand Up @@ -70,6 +70,7 @@ require (
golang.org/x/oauth2 v0.0.0-20210819190943-2bc19b11175f
golang.org/x/sys v0.0.0-20210921065528-437939a70204
golang.org/x/term v0.0.0-20210916214954-140adaaadfaf
google.golang.org/genproto v0.0.0-20200825200019-8632dd797987
google.golang.org/grpc v1.41.0
google.golang.org/protobuf v1.27.1
gopkg.in/square/go-jose.v2 v2.6.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,12 @@ github.com/coreos/go-systemd/v22 v22.3.2/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSV
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
github.com/cs3org/go-cs3apis v0.0.0-20210325133324-32b03d75a535 h1:555D8A3ddKqb4OyK9v5mdphw2zDLWKGXOkcnf1RQwTA=
github.com/cs3org/go-cs3apis v0.0.0-20210325133324-32b03d75a535/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/go-cs3apis v0.0.0-20210922150613-cb9e3c99f8de h1:N+AI8wz7yhDDqHDuq9EGaqQoFhAOi9XW37xt0ormflw=
github.com/cs3org/go-cs3apis v0.0.0-20210922150613-cb9e3c99f8de/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/go-cs3apis v0.0.0-20211004093007-d29741980082 h1:ErxzuD05JkSlTQrqc8YSca7R1BPFuCesufg8gxzTB2g=
github.com/cs3org/go-cs3apis v0.0.0-20211004093007-d29741980082/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
2 changes: 1 addition & 1 deletion internal/grpc/interceptors/auth/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func extractShareRef(req interface{}) (*collaboration.ShareReference, bool) {
case *collaboration.GetReceivedShareRequest:
return v.GetRef(), true
case *collaboration.UpdateReceivedShareRequest:
return v.GetRef(), true
return &collaboration.ShareReference{Spec: &collaboration.ShareReference_Id{Id: v.GetShare().GetShare().GetId()}}, true
}
return nil, false
}
94 changes: 52 additions & 42 deletions internal/grpc/services/gateway/ocmshareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,54 +224,64 @@ func (s *svc) UpdateReceivedOCMShare(ctx context.Context, req *ocm.UpdateReceive
return res, nil
}

// we don't commit to storage invalid update fields or empty display names.
if req.Field.GetState() == ocm.ShareState_SHARE_STATE_INVALID && req.Field.GetDisplayName() == "" {
log.Error().Msg("the update field is invalid, aborting reference manipulation")
return res, nil

}

// TODO(labkode): if update field is displayName we need to do a rename on the storage to align
// share display name and storage filename.
if req.Field.GetState() != ocm.ShareState_SHARE_STATE_INVALID {
if req.Field.GetState() == ocm.ShareState_SHARE_STATE_ACCEPTED {
getShareReq := &ocm.GetReceivedOCMShareRequest{Ref: req.Ref}
getShareRes, err := s.GetReceivedOCMShare(ctx, getShareReq)
if err != nil {
log.Err(err).Msg("gateway: error calling GetReceivedShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
// properties are updated in the order they appear in the field mask
// when an error occurs the request ends and no further fields are updated
for i := range req.UpdateMask.Paths {
switch req.UpdateMask.Paths[i] {
case "state":
switch req.GetShare().GetState() {
case ocm.ShareState_SHARE_STATE_ACCEPTED:
getShareReq := &ocm.GetReceivedOCMShareRequest{
Ref: &ocm.ShareReference{
Spec: &ocm.ShareReference_Id{
Id: req.Share.Share.Id,
},
},
}, nil
}

if getShareRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Msg("gateway: error calling GetReceivedShare")
}
getShareRes, err := s.GetReceivedOCMShare(ctx, getShareReq)
if err != nil {
log.Err(err).Msg("gateway: error calling GetReceivedShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
},
}, nil
}

if getShareRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Msg("gateway: error calling GetReceivedShare")
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
},
}, nil
}

share := getShareRes.Share
if share == nil {
panic("gateway: error updating a received share: the share is nil")
}

createRefStatus, err := s.createOCMReference(ctx, share.Share)
return &ocm.UpdateReceivedOCMShareResponse{
Status: &rpc.Status{
Code: rpc.Code_CODE_INTERNAL,
},
}, nil
}

share := getShareRes.Share
if share == nil {
panic("gateway: error updating a received share: the share is nil")
Status: createRefStatus,
}, err
case ocm.ShareState_SHARE_STATE_REJECTED:
s.removeReference(ctx, req.GetShare().GetShare().ResourceId) // error is logged inside removeReference
// FIXME we are ignoring an error from removeReference here
return res, nil
}

createRefStatus, err := s.createOCMReference(ctx, share.Share)
case "mount_point":
// TODO(labkode): implementing updating mount point
err = errtypes.NotSupported("gateway: update of mount point is not yet implemented")
return &ocm.UpdateReceivedOCMShareResponse{
Status: createRefStatus,
}, err
Status: status.NewUnimplemented(ctx, err, "error updating received share"),
}, nil
default:
return nil, errtypes.NotSupported("updating " + req.UpdateMask.Paths[i] + " is not supported")
}
}

// TODO(labkode): implementing updating display name
err = errtypes.NotSupported("gateway: update of display name is not yet implemented")
return &ocm.UpdateReceivedOCMShareResponse{
Status: status.NewUnimplemented(ctx, err, "error updating received share"),
}, nil
return res, nil
}

func (s *svc) GetReceivedOCMShare(ctx context.Context, req *ocm.GetReceivedOCMShareRequest) (*ocm.GetReceivedOCMShareResponse, error) {
Expand Down
21 changes: 7 additions & 14 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"time"

rtrace "github.com/cs3org/reva/pkg/trace"
"google.golang.org/protobuf/types/known/fieldmaskpb"

collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
Expand Down Expand Up @@ -888,7 +889,7 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide
// the following will check that:
// - the resource to delete is a share the current user received
// - signal the storage the delete must not land in the trashbin
// - delete the resource and update the share status to pending
// - delete the resource and update the share status to "rejected"
for _, share := range sRes.Shares {
if statRes != nil && (share.Share.ResourceId.OpaqueId == statRes.Info.Id.OpaqueId) && (share.Share.ResourceId.StorageId == statRes.Info.Id.StorageId) {
// this opaque needs explanation. It signals the storage the resource we're about to delete does not
Expand All @@ -903,21 +904,13 @@ func (s *svc) Delete(ctx context.Context, req *provider.DeleteRequest) (*provide
},
}

// the following block takes care of updating the state of the share to "pending". This will ensure the user
// the following block takes care of updating the state of the share to "rejected". This will ensure the user
// can "Accept" the share once again.
// TODO should this be pending? If so, update the two comments above as well. If not, get rid of this comment.
share.State = collaboration.ShareState_SHARE_STATE_REJECTED
r := &collaboration.UpdateReceivedShareRequest{
Ref: &collaboration.ShareReference{
Spec: &collaboration.ShareReference_Id{
Id: &collaboration.ShareId{
OpaqueId: share.Share.Id.OpaqueId,
},
},
},
Field: &collaboration.UpdateReceivedShareRequest_UpdateField{
Field: &collaboration.UpdateReceivedShareRequest_UpdateField_State{
State: collaboration.ShareState_SHARE_STATE_REJECTED,
},
},
Share: share,
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state"}},
}

_, err := s.UpdateReceivedShare(ctx, r)
Expand Down
Loading

0 comments on commit d7e2e9f

Please sign in to comment.