Skip to content

Commit

Permalink
make recycle methods require a key and a path explicitly
Browse files Browse the repository at this point in the history
  • Loading branch information
David Christofas committed Jul 9, 2021
1 parent e465080 commit 34426fe
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 90 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/support-recycle-subpaths.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Support trashbin sub paths in the recycle API

The recycle API could only act on the root items of the trashbin. Meaning if you delete a deep tree, you couldn't restore just one file from that tree but you had to restore the whole tree. Now listing, restoring and purging work also for sub paths in the trashbin.

https://github.com/cs3org/reva/pull/1827

1 change: 0 additions & 1 deletion internal/grpc/services/gateway/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1954,7 +1954,6 @@ func (s *svc) RestoreRecycleItem(ctx context.Context, req *provider.RestoreRecyc
}

func (s *svc) PurgeRecycle(ctx context.Context, req *gateway.PurgeRecycleRequest) (*provider.PurgeRecycleResponse, error) {
// lookup storage by treating the key as a path. It has been prefixed with the storage path in ListRecycle
c, err := s.find(ctx, req.Ref)
if err != nil {
return &provider.PurgeRecycleResponse{
Expand Down
18 changes: 11 additions & 7 deletions internal/grpc/services/storageprovider/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/cs3org/reva/pkg/mime"
"github.com/cs3org/reva/pkg/rgrpc"
"github.com/cs3org/reva/pkg/rgrpc/status"
"github.com/cs3org/reva/pkg/rhttp/router"
"github.com/cs3org/reva/pkg/storage"
"github.com/cs3org/reva/pkg/storage/fs/registry"
"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -793,7 +794,12 @@ func (s *service) ListRecycleStream(req *provider.ListRecycleStreamRequest, ss p
ctx := ss.Context()
log := appctx.GetLogger(ctx)

items, err := s.storage.ListRecycle(ctx, nil)
ref, err := s.unwrap(ctx, req.Ref)
if err != nil {
return err
}

items, err := s.storage.ListRecycle(ctx, ref.ResourceId.OpaqueId, ref.Path)
if err != nil {
var st *rpc.Status
switch err.(type) {
Expand Down Expand Up @@ -833,7 +839,8 @@ func (s *service) ListRecycle(ctx context.Context, req *provider.ListRecycleRequ
if err != nil {
return nil, err
}
items, err := s.storage.ListRecycle(ctx, ref)
key, itemPath := router.ShiftPath(ref.Path)
items, err := s.storage.ListRecycle(ctx, key, itemPath)
// TODO(labkode): CRITICAL: fill recycle info with storage provider.
if err != nil {
var st *rpc.Status
Expand Down Expand Up @@ -863,10 +870,7 @@ func (s *service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreR
if err != nil {
return nil, err
}
ref.ResourceId = &provider.ResourceId{
OpaqueId: req.Key,
}
if err := s.storage.RestoreRecycleItem(ctx, ref, req.RestoreRef); err != nil {
if err := s.storage.RestoreRecycleItem(ctx, req.Key, ref.Path, req.RestoreRef); err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.IsNotFound:
Expand All @@ -890,7 +894,7 @@ func (s *service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreR
func (s *service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRequest) (*provider.PurgeRecycleResponse, error) {
// if a key was sent as opaque id purge only that item
if req.GetRef().GetResourceId() != nil && req.GetRef().GetResourceId().OpaqueId != "" {
if err := s.storage.PurgeRecycleItem(ctx, req.GetRef()); err != nil {
if err := s.storage.PurgeRecycleItem(ctx, req.GetRef().GetResourceId().OpaqueId, req.GetRef().Path); err != nil {
var st *rpc.Status
switch err.(type) {
case errtypes.IsNotFound:
Expand Down
22 changes: 10 additions & 12 deletions internal/http/services/owncloud/ocdav/trashbin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"fmt"
"net/http"
"path"
"path/filepath"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -108,7 +107,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
//}

if r.Method == "PROPFIND" {
h.listTrashbin(w, r, s, u, path.Join(key, r.URL.Path))
h.listTrashbin(w, r, s, u, key, r.URL.Path)
return
}
if key != "" && r.Method == "MOVE" {
Expand Down Expand Up @@ -142,7 +141,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
})
}

func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key string) {
func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key, itemPath string) {
ctx := r.Context()
ctx, span := trace.StartSpan(ctx, "listTrashbin")
defer span.End()
Expand Down Expand Up @@ -190,8 +189,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
}

// ask gateway for recycle items
// TODO(labkode): add Reference to ListRecycleRequest
getRecycleRes, err := gc.ListRecycle(ctx, &gateway.ListRecycleRequest{Ref: &provider.Reference{Path: filepath.Join(getHomeRes.Path, key)}})
getRecycleRes, err := gc.ListRecycle(ctx, &gateway.ListRecycleRequest{Ref: &provider.Reference{Path: path.Join(getHomeRes.Path, key, itemPath)}})

if err != nil {
sublog.Error().Err(err).Msg("error calling ListRecycle")
Expand Down Expand Up @@ -331,7 +329,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, u *use
Prop: []*propertyXML{},
})
// yes this is redundant, can be derived from oc:trashbin-original-location which contains the full path, clients should not fetch it
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:trashbin-original-filename", filepath.Base(item.Ref.Path)))
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:trashbin-original-filename", path.Base(item.Ref.Path)))
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:trashbin-original-location", strings.TrimPrefix(item.Ref.Path, "/")))
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:trashbin-delete-timestamp", strconv.FormatUint(item.DeletionTime.Seconds, 10)))
response.Propstat[0].Prop = append(response.Propstat[0].Prop, s.newProp("oc:trashbin-delete-datetime", dTime))
Expand Down Expand Up @@ -368,7 +366,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, u *use
}
case "trashbin-original-filename":
// yes this is redundant, can be derived from oc:trashbin-original-location which contains the full path, clients should not fetch it
propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:trashbin-original-filename", filepath.Base(item.Ref.Path)))
propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:trashbin-original-filename", path.Base(item.Ref.Path)))
case "trashbin-original-location":
// TODO (jfd) double check and clarify the cs3 spec what the Key is about and if Path is only the folder that contains the file or if it includes the filename
propstatOK.Prop = append(propstatOK.Prop, s.newProp("oc:trashbin-original-location", strings.TrimPrefix(item.Ref.Path, "/")))
Expand Down Expand Up @@ -415,7 +413,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, u *use
}

// restore has a destination and a key
func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, dst, key, resourcePath string) {
func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, dst, key, itemPath string) {
ctx := r.Context()
ctx, span := trace.StartSpan(ctx, "restore")
defer span.End()
Expand Down Expand Up @@ -453,7 +451,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
}

dstRef := &provider.Reference{
Path: filepath.Join(getHomeRes.Path, dst),
Path: path.Join(getHomeRes.Path, dst),
}

dstStatReq := &provider.StatRequest{
Expand Down Expand Up @@ -516,7 +514,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
// use the key which is prefixed with the StoragePath to lookup the correct storage ...
// TODO currently limited to the home storage
Ref: &provider.Reference{
Path: path.Join(getHomeRes.Path, resourcePath),
Path: path.Join(getHomeRes.Path, itemPath),
},
Key: key,
RestoreRef: &provider.Reference{Path: dst},
Expand Down Expand Up @@ -555,7 +553,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
}

// delete has only a key
func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key, path string) {
func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key, itemPath string) {
ctx := r.Context()
ctx, span := trace.StartSpan(ctx, "erase")
defer span.End()
Expand Down Expand Up @@ -599,7 +597,7 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
StorageId: sRes.Info.Id.StorageId,
OpaqueId: key,
},
Path: utils.MakeRelativePath(path),
Path: utils.MakeRelativePath(itemPath),
},
}

Expand Down
22 changes: 11 additions & 11 deletions pkg/storage/fs/owncloud/owncloud.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2060,12 +2060,12 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re
return fs.propagate(ctx, ip)
}

func (fs *ocfs) PurgeRecycleItem(ctx context.Context, ref *provider.Reference) error {
func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key, path string) error {
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return errors.Wrap(err, "ocfs: error resolving recycle path")
}
ip := filepath.Join(rp, filepath.Clean(ref.ResourceId.OpaqueId))
ip := filepath.Join(rp, filepath.Clean(key))
// TODO check permission?

// check permissions
Expand All @@ -2086,7 +2086,7 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, ref *provider.Reference) e
if err != nil {
return errors.Wrap(err, "ocfs: error deleting recycle item")
}
err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions", filepath.Clean(ref.ResourceId.OpaqueId)))
err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions", filepath.Clean(key)))
if err != nil {
return errors.Wrap(err, "ocfs: error deleting recycle item versions")
}
Expand Down Expand Up @@ -2153,15 +2153,15 @@ func (fs *ocfs) convertToRecycleItem(ctx context.Context, rp string, md os.FileI
}
}

func (fs *ocfs) ListRecycle(ctx context.Context, ref *provider.Reference) ([]*provider.RecycleItem, error) {
func (fs *ocfs) ListRecycle(ctx context.Context, key, path string) ([]*provider.RecycleItem, error) {
// TODO check permission? on what? user must be the owner?
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return nil, errors.Wrap(err, "ocfs: error resolving recycle path")
}

// list files folder
mds, err := ioutil.ReadDir(filepath.Join(rp, ref.Path))
mds, err := ioutil.ReadDir(filepath.Join(rp, key))
if err != nil {
log := appctx.GetLogger(ctx)
log.Debug().Err(err).Str("path", rp).Msg("trash not readable")
Expand All @@ -2180,18 +2180,18 @@ func (fs *ocfs) ListRecycle(ctx context.Context, ref *provider.Reference) ([]*pr
return items, nil
}

func (fs *ocfs) RestoreRecycleItem(ctx context.Context, trashRef *provider.Reference, restoreRef *provider.Reference) error {
func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key, path string, restoreRef *provider.Reference) error {
// TODO check permission? on what? user must be the owner?
log := appctx.GetLogger(ctx)
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return errors.Wrap(err, "ocfs: error resolving recycle path")
}
src := filepath.Join(rp, filepath.Clean(trashRef.ResourceId.OpaqueId))
src := filepath.Join(rp, filepath.Clean(key))

suffix := filepath.Ext(src)
if len(suffix) == 0 || !strings.HasPrefix(suffix, ".d") {
log.Error().Str("key", trashRef.ResourceId.OpaqueId).Str("path", src).Msg("invalid trash item suffix")
log.Error().Str("key", key).Str("path", src).Msg("invalid trash item suffix")
return nil
}

Expand All @@ -2201,20 +2201,20 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, trashRef *provider.Refer
if restoreRef.Path == "" {
v, err := xattr.Get(src, trashOriginPrefix)
if err != nil {
log.Error().Err(err).Str("key", trashRef.ResourceId.OpaqueId).Str("path", src).Msg("could not read origin")
log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin")
}
restoreRef.Path = filepath.Join("/", filepath.Clean(string(v)), strings.TrimSuffix(filepath.Base(src), suffix))
}
tgt := fs.toInternalPath(ctx, restoreRef.Path)
// move back to original location
if err := os.Rename(src, tgt); err != nil {
log.Error().Err(err).Str("key", trashRef.ResourceId.OpaqueId).Str("restorePath", restoreRef.Path).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
log.Error().Err(err).Str("key", key).Str("restorePath", restoreRef.Path).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
return errors.Wrap(err, "ocfs: could not restore item")
}
// unset trash origin location in metadata
if err := xattr.Remove(tgt, trashOriginPrefix); err != nil {
// just a warning, will be overwritten next time it is deleted
log.Warn().Err(err).Str("key", trashRef.ResourceId.OpaqueId).Str("tgt", tgt).Msg("could not unset origin")
log.Warn().Err(err).Str("key", key).Str("tgt", tgt).Msg("could not unset origin")
}
// TODO(jfd) restore versions

Expand Down
20 changes: 10 additions & 10 deletions pkg/storage/fs/owncloudsql/owncloudsql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1864,12 +1864,12 @@ func (fs *ocfs) RestoreRevision(ctx context.Context, ref *provider.Reference, re
return fs.propagate(ctx, ip)
}

func (fs *ocfs) PurgeRecycleItem(ctx context.Context, ref *provider.Reference) error {
func (fs *ocfs) PurgeRecycleItem(ctx context.Context, key, path string) error {
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return errors.Wrap(err, "owncloudsql: error resolving recycle path")
}
ip := filepath.Join(rp, filepath.Clean(ref.ResourceId.OpaqueId))
ip := filepath.Join(rp, filepath.Clean(key))
// TODO check permission?

// check permissions
Expand All @@ -1890,12 +1890,12 @@ func (fs *ocfs) PurgeRecycleItem(ctx context.Context, ref *provider.Reference) e
if err != nil {
return errors.Wrap(err, "owncloudsql: error deleting recycle item")
}
err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions", filepath.Clean(ref.ResourceId.OpaqueId)))
err = os.RemoveAll(filepath.Join(filepath.Dir(rp), "versions", filepath.Clean(key)))
if err != nil {
return errors.Wrap(err, "owncloudsql: error deleting recycle item versions")
}

base, ttime, err := splitTrashKey(ref.ResourceId.OpaqueId)
base, ttime, err := splitTrashKey(key)
if err != nil {
return err
}
Expand Down Expand Up @@ -1972,7 +1972,7 @@ func (fs *ocfs) convertToRecycleItem(ctx context.Context, md os.FileInfo) *provi
}
}

func (fs *ocfs) ListRecycle(ctx context.Context, ref *provider.Reference) ([]*provider.RecycleItem, error) {
func (fs *ocfs) ListRecycle(ctx context.Context, key, path string) ([]*provider.RecycleItem, error) {
// TODO check permission? on what? user must be the owner?
rp, err := fs.getRecyclePath(ctx)
if err != nil {
Expand All @@ -1999,32 +1999,32 @@ func (fs *ocfs) ListRecycle(ctx context.Context, ref *provider.Reference) ([]*pr
return items, nil
}

func (fs *ocfs) RestoreRecycleItem(ctx context.Context, trashRef *provider.Reference, restoreRef *provider.Reference) error {
func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key, path string, restoreRef *provider.Reference) error {
// TODO check permission? on what? user must be the owner?
log := appctx.GetLogger(ctx)
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return errors.Wrap(err, "owncloudsql: error resolving recycle path")
}
src := filepath.Join(rp, filepath.Clean(trashRef.ResourceId.OpaqueId))
src := filepath.Join(rp, filepath.Clean(key))

suffix := filepath.Ext(src)
if len(suffix) == 0 || !strings.HasPrefix(suffix, ".d") {
log.Error().Str("key", trashRef.ResourceId.OpaqueId).Str("path", src).Msg("invalid trash item suffix")
log.Error().Str("key", key).Str("path", src).Msg("invalid trash item suffix")
return nil
}

if restoreRef.Path == "" {
v, err := xattr.Get(src, trashOriginPrefix)
if err != nil {
log.Error().Err(err).Str("key", trashRef.ResourceId.OpaqueId).Str("path", src).Msg("could not read origin")
log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin")
}
restoreRef.Path = filepath.Join("/", filepath.Clean(string(v)), strings.TrimSuffix(filepath.Base(src), suffix))
}
tgt := fs.toInternalPath(ctx, restoreRef.Path)
// move back to original location
if err := os.Rename(src, tgt); err != nil {
log.Error().Err(err).Str("key", trashRef.ResourceId.OpaqueId).Str("restorePath", restoreRef.Path).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
log.Error().Err(err).Str("key", key).Str("restorePath", restoreRef.Path).Str("src", src).Str("tgt", tgt).Msg("could not restore item")
return errors.Wrap(err, "owncloudsql: could not restore item")
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/fs/s3/s3.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,19 +646,19 @@ func (fs *s3FS) RestoreRevision(ctx context.Context, ref *provider.Reference, re
return errtypes.NotSupported("restore revision")
}

func (fs *s3FS) PurgeRecycleItem(ctx context.Context, ref *provider.Reference) error {
func (fs *s3FS) PurgeRecycleItem(ctx context.Context, key, path string) error {
return errtypes.NotSupported("purge recycle item")
}

func (fs *s3FS) EmptyRecycle(ctx context.Context) error {
return errtypes.NotSupported("empty recycle")
}

func (fs *s3FS) ListRecycle(ctx context.Context, ref *provider.Reference) ([]*provider.RecycleItem, error) {
func (fs *s3FS) ListRecycle(ctx context.Context, key, path string) ([]*provider.RecycleItem, error) {
return nil, errtypes.NotSupported("list recycle")
}

func (fs *s3FS) RestoreRecycleItem(ctx context.Context, trashRef *provider.Reference, restoreRef *provider.Reference) error {
func (fs *s3FS) RestoreRecycleItem(ctx context.Context, key, path string, restoreRef *provider.Reference) error {
return errtypes.NotSupported("restore recycle")
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ type FS interface {
ListRevisions(ctx context.Context, ref *provider.Reference) ([]*provider.FileVersion, error)
DownloadRevision(ctx context.Context, ref *provider.Reference, key string) (io.ReadCloser, error)
RestoreRevision(ctx context.Context, ref *provider.Reference, key string) error
ListRecycle(ctx context.Context, ref *provider.Reference) ([]*provider.RecycleItem, error)
RestoreRecycleItem(ctx context.Context, trashRef *provider.Reference, restoreRef *provider.Reference) error
PurgeRecycleItem(ctx context.Context, ref *provider.Reference) error
ListRecycle(ctx context.Context, key, path string) ([]*provider.RecycleItem, error)
RestoreRecycleItem(ctx context.Context, key, path string, restoreRef *provider.Reference) error
PurgeRecycleItem(ctx context.Context, key, path string) error
EmptyRecycle(ctx context.Context) error
GetPathByID(ctx context.Context, id *provider.ResourceId) (string, error)
AddGrant(ctx context.Context, ref *provider.Reference, g *provider.Grant) error
Expand Down
Loading

0 comments on commit 34426fe

Please sign in to comment.