Skip to content

Commit

Permalink
allow listing directory trash items by key
Browse files Browse the repository at this point in the history 
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
  • Loading branch information
@butonic
butonic committed Aug 22, 2024
1 parent 774910f commit d6492dd
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 89 deletions.
9 changes: 9 additions & 0 deletions changelog/unreleased/trash-listing-bykey-fixes.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Bugfix: Allow listing directory trash items by key

The storageprovider now passes on the key without inventing a `/` as the relative path when it was not present at the end of the key. This allows differentiating requests that want to get the trash item of a folder itself (where the relative path is empty) or listing the children of a folder in the trash (where the relative path at least starts with a `/`).

We also fixed the `/dav/spaces` endpoint to not invent a `/` at the end of URLs to allow clients to actually make these different requests.

As a byproduct we now return the size of trashed items.

https://github.com/cs3org/reva/pull/4818
31 changes: 23 additions & 8 deletions internal/grpc/services/storageprovider/storageprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"path"
"sort"
"strconv"
"strings"
"time"

rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
Expand Down Expand Up @@ -880,8 +881,11 @@ func (s *Service) ListRecycleStream(req *provider.ListRecycleStreamRequest, ss p
ctx := ss.Context()
log := appctx.GetLogger(ctx)

key, itemPath := router.ShiftPath(req.Key)
items, err := s.Storage.ListRecycle(ctx, req.Ref, key, itemPath)
// if no slash is present in the key, do not pass a relative path to the storage
// when a path is passed to the storage, it will list the contents of the directory
key, relativePath := splitKeyAndPath(req.GetKey())
items, err := s.Storage.ListRecycle(ctx, req.Ref, key, relativePath)

if err != nil {
var st *rpc.Status
switch err.(type) {
Expand Down Expand Up @@ -924,8 +928,10 @@ func (s *Service) ListRecycleStream(req *provider.ListRecycleStreamRequest, ss p
}

func (s *Service) ListRecycle(ctx context.Context, req *provider.ListRecycleRequest) (*provider.ListRecycleResponse, error) {
key, itemPath := router.ShiftPath(req.Key)
items, err := s.Storage.ListRecycle(ctx, req.Ref, key, itemPath)
// if no slash is present in the key, do not pass a relative path to the storage
// when a path is passed to the storage, it will list the contents of the directory
key, relativePath := splitKeyAndPath(req.GetKey())
items, err := s.Storage.ListRecycle(ctx, req.Ref, key, relativePath)
if err != nil {
var st *rpc.Status
switch err.(type) {
Expand Down Expand Up @@ -962,8 +968,8 @@ func (s *Service) RestoreRecycleItem(ctx context.Context, req *provider.RestoreR
ctx = ctxpkg.ContextSetLockID(ctx, req.LockId)

// TODO(labkode): CRITICAL: fill recycle info with storage provider.
key, itemPath := router.ShiftPath(req.Key)
err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, itemPath, req.RestoreRef)
key, relativePath := splitKeyAndPath(req.GetKey())
err := s.Storage.RestoreRecycleItem(ctx, req.Ref, key, relativePath, req.RestoreRef)

res := &provider.RestoreRecycleItemResponse{
Status: status.NewStatusFromErrType(ctx, "restore recycle item", err),
Expand All @@ -980,9 +986,9 @@ func (s *Service) PurgeRecycle(ctx context.Context, req *provider.PurgeRecycleRe
}

// if a key was sent as opaque id purge only that item
key, itemPath := router.ShiftPath(req.Key)
key, relativePath := splitKeyAndPath(req.GetKey())
if key != "" {
if err := s.Storage.PurgeRecycleItem(ctx, req.Ref, key, itemPath); err != nil {
if err := s.Storage.PurgeRecycleItem(ctx, req.Ref, key, relativePath); err != nil {
st := status.NewStatusFromErrType(ctx, "error purging recycle item", err)
appctx.GetLogger(ctx).
Error().
Expand Down Expand Up @@ -1313,3 +1319,12 @@ func canLockPublicShare(ctx context.Context) bool {
psr := utils.ReadPlainFromOpaque(u.Opaque, "public-share-role")
return psr == "" || psr == conversions.RoleEditor
}

// splitKeyAndPath splits a key into a root and a relative path
func splitKeyAndPath(key string) (string, string) {
root, relativePath := router.ShiftPath(key)
if relativePath == "/" && !strings.HasSuffix(key, "/") {
relativePath = ""
}
return root, relativePath
}
25 changes: 16 additions & 9 deletions internal/http/services/owncloud/ocdav/spaces.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package ocdav
import (
"net/http"
"path"
"strings"

provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/config"
Expand Down Expand Up @@ -132,8 +133,7 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
ctx := r.Context()
log := appctx.GetLogger(ctx)

var spaceID string
spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
spaceID, key := splitSpaceAndKey(r.URL.Path)
if spaceID == "" {
// listing is disabled, no auth will change that
w.WriteHeader(http.StatusMethodNotAllowed)
Expand All @@ -146,12 +146,9 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
return
}

var key string
key, r.URL.Path = router.ShiftPath(r.URL.Path)

switch r.Method {
case MethodPropfind:
trashbinHandler.listTrashbin(w, r, s, &ref, path.Join(_trashbinPath, spaceID), key, r.URL.Path)
trashbinHandler.listTrashbin(w, r, s, &ref, path.Join(_trashbinPath, spaceID), key)
case MethodMove:
if key == "" {
http.Error(w, "501 Not implemented", http.StatusNotImplemented)
Expand All @@ -167,15 +164,25 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
w.WriteHeader(http.StatusBadRequest)
return
}
log.Debug().Str("key", key).Str("path", r.URL.Path).Str("dst", dst).Msg("spaces restore")
log.Debug().Str("key", key).Str("dst", dst).Msg("spaces restore")

dstRef := proto.Clone(&ref).(*provider.Reference)
dstRef.Path = utils.MakeRelativePath(dst)

trashbinHandler.restore(w, r, s, &ref, dstRef, key, r.URL.Path)
trashbinHandler.restore(w, r, s, &ref, dstRef, key)
case http.MethodDelete:
trashbinHandler.delete(w, r, s, &ref, key, r.URL.Path)
trashbinHandler.delete(w, r, s, &ref, key)
default:
http.Error(w, "501 Not implemented", http.StatusNotImplemented)
}
}

func splitSpaceAndKey(p string) (space, key string) {
p = strings.TrimPrefix(p, "/")
parts := strings.SplitN(p, "/", 2)
space = parts[0]
if len(parts) > 1 {
key = parts[1]
}
return
}
32 changes: 15 additions & 17 deletions internal/http/services/owncloud/ocdav/trashbin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,12 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
}
ref := spacelookup.MakeRelativeReference(space, ".", false)

// key will be a base64 encoded cs3 path, it uniquely identifies a trash item & storage
var key string
key, r.URL.Path = router.ShiftPath(r.URL.Path)
// key will be a base64 encoded cs3 path, it uniquely identifies a trash item with an opaque id and an optional path
key := r.URL.Path

switch r.Method {
case MethodPropfind:
h.listTrashbin(w, r, s, ref, user.Username, key, r.URL.Path)
h.listTrashbin(w, r, s, ref, user.Username, key)
case MethodMove:
if key == "" {
http.Error(w, "501 Not implemented", http.StatusNotImplemented)
Expand Down Expand Up @@ -172,16 +171,16 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
dstRef := spacelookup.MakeRelativeReference(space, p, false)

log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
h.restore(w, r, s, ref, dstRef, key, r.URL.Path)
h.restore(w, r, s, ref, dstRef, key)
case http.MethodDelete:
h.delete(w, r, s, ref, key, r.URL.Path)
h.delete(w, r, s, ref, key)
default:
http.Error(w, "501 Not implemented", http.StatusNotImplemented)
}
})
}

func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, refBase, key, itemPath string) {
func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, refBase, key string) {
ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "list_trashbin")
defer span.End()

Expand Down Expand Up @@ -214,7 +213,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
}

if depth == net.DepthZero {
rootHref := path.Join(refBase, key, itemPath)
rootHref := path.Join(refBase, key)
propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, nil, nil)
if err != nil {
sublog.Error().Err(err).Msg("error formatting propfind")
Expand Down Expand Up @@ -246,7 +245,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
return
}
// ask gateway for recycle items
getRecycleRes, err := client.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: path.Join(key, itemPath)})
getRecycleRes, err := client.ListRecycle(ctx, &provider.ListRecycleRequest{Ref: ref, Key: key})
if err != nil {
sublog.Error().Err(err).Msg("error calling ListRecycle")
w.WriteHeader(http.StatusInternalServerError)
Expand Down Expand Up @@ -304,7 +303,7 @@ func (h *TrashbinHandler) listTrashbin(w http.ResponseWriter, r *http.Request, s
}
}

rootHref := path.Join(refBase, key, itemPath)
rootHref := path.Join(refBase, key)
propRes, err := h.formatTrashPropfind(ctx, s, ref.ResourceId.SpaceId, refBase, rootHref, &pf, items)
if err != nil {
sublog.Error().Err(err).Msg("error formatting propfind")
Expand Down Expand Up @@ -480,7 +479,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, spaceI
return &response, nil
}

func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref, dst *provider.Reference, key, itemPath string) {
func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref, dst *provider.Reference, key string) {
ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "restore")
defer span.End()

Expand Down Expand Up @@ -566,7 +565,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc

req := &provider.RestoreRecycleItemRequest{
Ref: ref,
Key: path.Join(key, itemPath),
Key: key,
RestoreRef: dst,
}

Expand Down Expand Up @@ -608,16 +607,15 @@ 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, ref *provider.Reference, key, itemPath string) {
func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, key string) {
ctx, span := appctx.GetTracerProvider(r.Context()).Tracer(tracerName).Start(r.Context(), "erase")
defer span.End()

sublog := appctx.GetLogger(ctx).With().Interface("reference", ref).Str("key", key).Str("item_path", itemPath).Logger()
sublog := appctx.GetLogger(ctx).With().Interface("reference", ref).Str("key", key).Logger()

trashPath := path.Join(key, itemPath)
req := &provider.PurgeRecycleRequest{
Ref: ref,
Key: trashPath,
Key: key,
}

client, err := s.gatewaySelector.Next()
Expand All @@ -638,7 +636,7 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
case rpc.Code_CODE_NOT_FOUND:
sublog.Debug().Interface("status", res.Status).Msg("resource not found")
w.WriteHeader(http.StatusConflict)
m := fmt.Sprintf("path %s not found", trashPath)
m := fmt.Sprintf("key %s not found", key)
b, err := errors.Marshal(http.StatusConflict, m, "", "")
errors.HandleWebdavError(&sublog, w, b, err)
case rpc.Code_CODE_PERMISSION_DENIED:
Expand Down
51 changes: 28 additions & 23 deletions pkg/storage/utils/decomposedfs/recycle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
iofs "io/fs"
"os"
"path/filepath"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -55,6 +54,9 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
if ref == nil || ref.ResourceId == nil || ref.ResourceId.OpaqueId == "" {
return nil, errtypes.BadRequest("spaceid required")
}
if key == "" && relativePath != "" {
return nil, errtypes.BadRequest("key is required when navigating with a path")
}
spaceID := ref.ResourceId.OpaqueId

sublog := appctx.GetLogger(ctx).With().Str("spaceid", spaceID).Str("key", key).Str("relative_path", relativePath).Logger()
Expand All @@ -75,7 +77,7 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
return nil, errtypes.NotFound(key)
}

if key == "" && relativePath == "/" {
if key == "" && relativePath == "" {
return fs.listTrashRoot(ctx, spaceID)
}

Expand Down Expand Up @@ -113,16 +115,25 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
sublog.Error().Err(err).Msg("could not parse time format, ignoring")
}

nodeType := fs.lu.TypeFromPath(ctx, originalPath)
if nodeType != provider.ResourceType_RESOURCE_TYPE_CONTAINER {
var size int64
if relativePath == "" {
// this is the case when we want to directly list a file in the trashbin
blobsize, err := strconv.ParseInt(string(attrs[prefixes.BlobsizeAttr]), 10, 64)
if err != nil {
return items, err
nodeType := fs.lu.TypeFromPath(ctx, originalPath)
switch nodeType {
case provider.ResourceType_RESOURCE_TYPE_FILE:
size, err = fs.lu.ReadBlobSizeAttr(ctx, originalPath)
if err != nil {
return items, err
}
case provider.ResourceType_RESOURCE_TYPE_CONTAINER:
size, err = fs.lu.MetadataBackend().GetInt64(ctx, originalPath, prefixes.TreesizeAttr)
if err != nil {
return items, err
}
}
item := &provider.RecycleItem{
Type: nodeType,
Size: uint64(blobsize),
Type: fs.lu.TypeFromPath(ctx, originalPath),
Size: uint64(size),
Key: filepath.Join(key, relativePath),
DeletionTime: deletionTime,
Ref: &provider.Reference{
Expand All @@ -134,9 +145,6 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
}

// we have to read the names and stat the path to follow the symlinks
if err != nil {
return nil, err
}
childrenPath := filepath.Join(originalPath, relativePath)
childrenDir, err := os.Open(childrenPath)
if err != nil {
Expand All @@ -154,9 +162,10 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
continue
}

size := int64(0)
// reset size
size = 0

nodeType = fs.lu.TypeFromPath(ctx, resolvedChildPath)
nodeType := fs.lu.TypeFromPath(ctx, resolvedChildPath)
switch nodeType {
case provider.ResourceType_RESOURCE_TYPE_FILE:
size, err = fs.lu.ReadBlobSizeAttr(ctx, resolvedChildPath)
Expand All @@ -165,12 +174,7 @@ func (fs *Decomposedfs) ListRecycle(ctx context.Context, ref *provider.Reference
continue
}
case provider.ResourceType_RESOURCE_TYPE_CONTAINER:
attr, err := fs.lu.MetadataBackend().Get(ctx, resolvedChildPath, prefixes.TreesizeAttr)
if err != nil {
sublog.Error().Err(err).Str("name", name).Msg("invalid tree size, skipping")
continue
}
size, err = strconv.ParseInt(string(attr), 10, 64)
size, err = fs.lu.MetadataBackend().GetInt64(ctx, resolvedChildPath, prefixes.TreesizeAttr)
if err != nil {
sublog.Error().Err(err).Str("name", name).Msg("invalid tree size, skipping")
continue
Expand Down Expand Up @@ -217,7 +221,7 @@ func readTrashLink(path string) (string, string, string, error) {
func (fs *Decomposedfs) listTrashRoot(ctx context.Context, spaceID string) ([]*provider.RecycleItem, error) {
log := appctx.GetLogger(ctx)
trashRoot := fs.getRecycleRoot(spaceID)

items := []*provider.RecycleItem{}
subTrees, err := filepath.Glob(trashRoot + "/*")
if err != nil {
return nil, err
Expand Down Expand Up @@ -256,6 +260,7 @@ func (fs *Decomposedfs) listTrashRoot(ctx context.Context, spaceID string) ([]*p
}

for _, itemPath := range matches {
// TODO can we encode this in the path instead of reading the link?
nodePath, nodeID, timeSuffix, err := readTrashLink(itemPath)
if err != nil {
log.Error().Err(err).Str("trashRoot", trashRoot).Str("item", itemPath).Msg("error reading trash link, skipping")
Expand Down Expand Up @@ -300,6 +305,7 @@ func (fs *Decomposedfs) listTrashRoot(ctx context.Context, spaceID string) ([]*p
} else {
log.Error().Str("trashRoot", trashRoot).Str("item", itemPath).Str("spaceid", spaceID).Str("nodeid", nodeID).Str("dtime", timeSuffix).Msg("could not read origin path")
}

select {
case results <- item:
case <-ctx.Done():
Expand All @@ -318,7 +324,6 @@ func (fs *Decomposedfs) listTrashRoot(ctx context.Context, spaceID string) ([]*p
}()

// Collect results
items := []*provider.RecycleItem{}
for ri := range results {
items = append(items, ri)
}
Expand Down Expand Up @@ -414,7 +419,7 @@ func (fs *Decomposedfs) EmptyRecycle(ctx context.Context, ref *provider.Referenc
return errtypes.BadRequest("spaceid must be set")
}

items, err := fs.ListRecycle(ctx, ref, "", "/")
items, err := fs.ListRecycle(ctx, ref, "", "")
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit d6492dd

Please sign in to comment.