Skip to content

Commit 9ab9886

Browse files
LarsJurgensenLarsJurgensen
committed
fix(decomposedfs): [OCISDEV-946]: refactor move event out of decomposedfs
1 parent 806e6cd commit 9ab9886

18 files changed

Lines changed: 196 additions & 174 deletions

File tree

‎internal/grpc/interceptors/eventsmiddleware/conversion.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -283,11 +283,14 @@ func ItemTrashed(r *provider.DeleteResponse, req *provider.DeleteRequest, spaceO
283283

284284
// ItemMoved converts the response to an event
285285
func ItemMoved(r *provider.MoveResponse, req *provider.MoveRequest, spaceOwner *user.UserId, executant *user.User) events.ItemMoved {
286+
var newRef, oldRef provider.Reference
287+
_ = utils.ReadJSONFromOpaque(r.Opaque, "newref", &newRef)
288+
_ = utils.ReadJSONFromOpaque(r.Opaque, "oldref", &oldRef)
286289
return events.ItemMoved{
287290
SpaceOwner: spaceOwner,
288291
Executant: executant.GetId(),
289-
Ref: req.Destination,
290-
OldReference: req.Source,
292+
Ref: &newRef,
293+
OldReference: &oldRef,
291294
Timestamp: utils.TSNow(),
292295
ImpersonatingUser: extractImpersonator(executant),
293296
}

‎internal/grpc/interceptors/eventsmiddleware/conversion_test.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package eventsmiddleware
2+
3+
import (
4+
"testing"
5+
6+
user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
7+
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
8+
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
9+
"github.com/owncloud/reva/v2/pkg/utils"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestItemMoved(t *testing.T) {
14+
spaceOwner := &user.UserId{OpaqueId: "owner-1"}
15+
executant := &user.User{Id: &user.UserId{OpaqueId: "user-1"}}
16+
newRef := &provider.Reference{
17+
ResourceId: &provider.ResourceId{StorageId: "storage-1", SpaceId: "space-1", OpaqueId: "node-1"},
18+
Path: "./new-name.txt",
19+
}
20+
oldRef := &provider.Reference{
21+
ResourceId: &provider.ResourceId{StorageId: "storage-1", SpaceId: "space-1", OpaqueId: "node-1"},
22+
Path: "./old-name.txt",
23+
}
24+
25+
res := &provider.MoveResponse{Status: &rpc.Status{Code: rpc.Code_CODE_OK}}
26+
res.Opaque = utils.AppendJSONToOpaque(res.Opaque, "newref", newRef)
27+
res.Opaque = utils.AppendJSONToOpaque(res.Opaque, "oldref", oldRef)
28+
req := &provider.MoveRequest{Source: oldRef, Destination: newRef}
29+
30+
ev := ItemMoved(res, req, spaceOwner, executant)
31+
32+
require.Equal(t, spaceOwner, ev.SpaceOwner)
33+
require.Equal(t, executant.GetId(), ev.Executant)
34+
require.Equal(t, newRef.GetResourceId().GetSpaceId(), ev.Ref.GetResourceId().GetSpaceId())
35+
require.Equal(t, newRef.GetResourceId().GetOpaqueId(), ev.Ref.GetResourceId().GetOpaqueId())
36+
require.Equal(t, newRef.Path, ev.Ref.Path)
37+
require.Equal(t, oldRef.Path, ev.OldReference.Path)
38+
require.NotNil(t, ev.Timestamp)
39+
}

‎internal/grpc/interceptors/eventsmiddleware/events.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,12 @@ func NewUnary(m map[string]interface{}) (grpc.UnaryServerInterceptor, int, error
7676

7777
executant, _ := revactx.ContextGetUser(ctx)
7878

79-
// The MoveResponse event is moved to the decomposedfs
8079
var ev interface{}
8180
switch v := res.(type) {
81+
case *provider.MoveResponse:
82+
if isSuccess(v) {
83+
ev = ItemMoved(v, req.(*provider.MoveRequest), ownerID, executant)
84+
}
8285
case *collaboration.CreateShareResponse:
8386
if isSuccess(v) {
8487
ev = ShareCreated(v, executant)

‎internal/grpc/services/storageprovider/storageprovider.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -761,11 +761,17 @@ func (s *Service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro
761761
func (s *Service) Move(ctx context.Context, req *provider.MoveRequest) (*provider.MoveResponse, error) {
762762
ctx = ctxpkg.ContextSetLockID(ctx, req.LockId)
763763

764-
err := s.Storage.Move(ctx, req.Source, req.Destination)
764+
result, err := s.Storage.Move(ctx, req.Source, req.Destination)
765765

766-
return &provider.MoveResponse{
766+
res := &provider.MoveResponse{
767767
Status: status.NewStatusFromErrType(ctx, "move", err),
768-
}, nil
768+
}
769+
if err == nil && result != nil {
770+
storagespace.ContextSendSpaceOwnerID(ctx, result.SpaceOwner)
771+
res.Opaque = utils.AppendJSONToOpaque(res.Opaque, "newref", result.NewReference)
772+
res.Opaque = utils.AppendJSONToOpaque(res.Opaque, "oldref", result.OldReference)
773+
}
774+
return res, nil
769775
}
770776

771777
func (s *Service) Stat(ctx context.Context, req *provider.StatRequest) (*provider.StatResponse, error) {

‎pkg/ocm/storage/received/ocm.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,14 +237,14 @@ func (d *driver) TouchFile(ctx context.Context, ref *provider.Reference, markpro
237237
return client.Write(rel, []byte{}, 0)
238238
}
239239

240-
func (d *driver) Move(ctx context.Context, oldRef, newRef *provider.Reference) error {
240+
func (d *driver) Move(ctx context.Context, oldRef, newRef *provider.Reference) (*storage.MoveResult, error) {
241241
client, _, relOld, err := d.webdavClient(ctx, nil, oldRef)
242242
if err != nil {
243-
return err
243+
return nil, err
244244
}
245245
_, relNew := shareInfoFromReference(newRef)
246246

247-
return client.Rename(relOld, relNew, false)
247+
return nil, client.Rename(relOld, relNew, false)
248248
}
249249

250250
func getPathFromShareIDAndRelPath(shareID *ocmpb.ShareId, relPath string) string {

‎pkg/storage/fs/cephfs/cephfs.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,15 @@ func (fs *cephfs) Delete(ctx context.Context, ref *provider.Reference) (err erro
195195
return getRevaError(err)
196196
}
197197

198-
func (fs *cephfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (err error) {
198+
func (fs *cephfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (*storage.MoveResult, error) {
199199
var oldPath, newPath string
200+
var err error
200201
user := fs.makeUser(ctx)
201202
if oldPath, err = user.resolveRef(oldRef); err != nil {
202-
return
203+
return nil, err
203204
}
204205
if newPath, err = user.resolveRef(newRef); err != nil {
205-
return
206+
return nil, err
206207
}
207208

208209
user.op(func(cv *cacheVal) {
@@ -215,10 +216,10 @@ func (fs *cephfs) Move(ctx context.Context, oldRef, newRef *provider.Reference)
215216

216217
// has already been moved by direct mount
217218
if err != nil && err.Error() == errNotFound {
218-
return nil
219+
return nil, nil
219220
}
220221

221-
return getRevaError(err)
222+
return nil, getRevaError(err)
222223
}
223224

224225
func (fs *cephfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (ri *provider.ResourceInfo, err error) {

‎pkg/storage/fs/hello/unimplemented.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ func (fs *hellofs) Delete(ctx context.Context, ref *provider.Reference) error {
7070
}
7171

7272
// Move changes the path of a resource
73-
func (fs *hellofs) Move(ctx context.Context, oldRef, newRef *provider.Reference) error {
74-
return errtypes.NotSupported("unimplemented")
73+
func (fs *hellofs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (*storage.MoveResult, error) {
74+
return nil, errtypes.NotSupported("unimplemented")
7575
}
7676

7777
// Upload creates or updates a resource of type file with a new revision

‎pkg/storage/fs/nextcloud/nextcloud.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ func (nc *StorageDriver) Delete(ctx context.Context, ref *provider.Reference) er
288288
}
289289

290290
// Move as defined in the storage.FS interface
291-
func (nc *StorageDriver) Move(ctx context.Context, oldRef, newRef *provider.Reference) error {
291+
func (nc *StorageDriver) Move(ctx context.Context, oldRef, newRef *provider.Reference) (*storage.MoveResult, error) {
292292
type paramsObj struct {
293293
OldRef *provider.Reference `json:"oldRef"`
294294
NewRef *provider.Reference `json:"newRef"`
@@ -302,7 +302,7 @@ func (nc *StorageDriver) Move(ctx context.Context, oldRef, newRef *provider.Refe
302302
log.Info().Msgf("Move %s", bodyStr)
303303

304304
_, _, err := nc.do(ctx, Action{"Move", string(bodyStr)})
305-
return err
305+
return nil, err
306306
}
307307

308308
// GetMD as defined in the storage.FS interface

‎pkg/storage/fs/nextcloud/nextcloud_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ var _ = Describe("Nextcloud", func() {
204204
},
205205
Path: "/some/new/path",
206206
}
207-
err := nc.Move(ctx, ref1, ref2)
207+
_, err := nc.Move(ctx, ref1, ref2)
208208
Expect(err).ToNot(HaveOccurred())
209209
checkCalled(called, `POST /apps/sciencemesh/~tester/api/storage/Move {"oldRef":{"resource_id":{"storage_id":"storage-id-1","opaque_id":"opaque-id-1"},"path":"/some/old/path"},"newRef":{"resource_id":{"storage_id":"storage-id-2","opaque_id":"opaque-id-2"},"path":"/some/new/path"}}`)
210210
})

‎pkg/storage/fs/owncloudsql/owncloudsql.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,51 +1295,51 @@ func (fs *owncloudsqlfs) trashVersions(ctx context.Context, ip string, origin st
12951295
return nil
12961296
}
12971297

1298-
func (fs *owncloudsqlfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (err error) {
1299-
var oldIP string
1300-
if oldIP, err = fs.resolve(ctx, oldRef); err != nil {
1301-
return errors.Wrap(err, "owncloudsql: error resolving reference")
1298+
func (fs *owncloudsqlfs) Move(ctx context.Context, oldRef, newRef *provider.Reference) (*storage.MoveResult, error) {
1299+
oldIP, err := fs.resolve(ctx, oldRef)
1300+
if err != nil {
1301+
return nil, errors.Wrap(err, "owncloudsql: error resolving reference")
13021302
}
13031303

13041304
// check permissions
13051305
if perm, err := fs.readPermissions(ctx, oldIP); err == nil {
13061306
if !perm.Move { // TODO add dedicated permission?
1307-
return errtypes.PermissionDenied("")
1307+
return nil, errtypes.PermissionDenied("")
13081308
}
13091309
} else {
13101310
if isNotFound(err) {
1311-
return errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(oldIP)))
1311+
return nil, errtypes.NotFound(fs.toStoragePath(ctx, filepath.Dir(oldIP)))
13121312
}
1313-
return errors.Wrap(err, "owncloudsql: error reading permissions")
1313+
return nil, errors.Wrap(err, "owncloudsql: error reading permissions")
13141314
}
13151315

1316-
var newIP string
1317-
if newIP, err = fs.resolve(ctx, newRef); err != nil {
1318-
return errors.Wrap(err, "owncloudsql: error resolving reference")
1316+
newIP, err := fs.resolve(ctx, newRef)
1317+
if err != nil {
1318+
return nil, errors.Wrap(err, "owncloudsql: error resolving reference")
13191319
}
13201320

13211321
// TODO check target permissions ... if it exists
1322-
storage, err := fs.getStorage(ctx, oldIP)
1322+
stor, err := fs.getStorage(ctx, oldIP)
13231323
if err != nil {
1324-
return err
1324+
return nil, err
13251325
}
1326-
err = fs.filecache.Move(ctx, storage, fs.toDatabasePath(oldIP), fs.toDatabasePath(newIP))
1326+
err = fs.filecache.Move(ctx, stor, fs.toDatabasePath(oldIP), fs.toDatabasePath(newIP))
13271327
if err != nil {
1328-
return err
1328+
return nil, err
13291329
}
13301330
if err = os.Rename(oldIP, newIP); err != nil {
1331-
return errors.Wrap(err, "owncloudsql: error moving "+oldIP+" to "+newIP)
1331+
return nil, errors.Wrap(err, "owncloudsql: error moving "+oldIP+" to "+newIP)
13321332
}
13331333

13341334
if err := fs.propagate(ctx, newIP); err != nil {
1335-
return err
1335+
return nil, err
13361336
}
13371337
if filepath.Dir(newIP) != filepath.Dir(oldIP) {
13381338
if err := fs.propagate(ctx, filepath.Dir(oldIP)); err != nil {
1339-
return err
1339+
return nil, err
13401340
}
13411341
}
1342-
return nil
1342+
return nil, nil
13431343
}
13441344

13451345
func (fs *owncloudsqlfs) GetMD(ctx context.Context, ref *provider.Reference, mdKeys []string, fieldMask []string) (*provider.ResourceInfo, error) {

0 commit comments

Comments
 (0)