Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix restoring and deleting trash items via ocs #1103

Merged
merged 1 commit into from
Aug 19, 2020
Merged

fix restoring and deleting trash items via ocs #1103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/unreleased/fix-trash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: restore and delete trash items via ocs

The OCS api was not passing the correct key and references to the CS3 API. Furthermore, the owncloud storage driver was constructing the wrong target path when restoring.

https://github.com/cs3org/reva/pull/1103
2 changes: 1 addition & 1 deletion internal/grpc/services/gateway/storageprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ func (s *svc) ListRecycleStream(_ *gateway.ListRecycleStreamRequest, _ gateway.G
return errors.New("Unimplemented")
}

// TODO use the ListRecycleRequest.Ref to only list the trish of a specific storage
// TODO use the ListRecycleRequest.Ref to only list the trash of a specific storage
func (s *svc) ListRecycle(ctx context.Context, req *gateway.ListRecycleRequest) (*provider.ListRecycleResponse, error) {
c, err := s.find(ctx, req.GetRef())
if err != nil {
Expand Down
89 changes: 57 additions & 32 deletions internal/http/services/owncloud/ocdav/trashbin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
if key != "" && r.Method == "MOVE" {
dstHeader := r.Header.Get("Destination")

log.Info().Str("key", key).Str("dst", dstHeader).Msg("restore")
log.Debug().Str("key", key).Str("dst", dstHeader).Msg("restore")

if dstHeader == "" {
w.WriteHeader(http.StatusBadRequest)
Expand All @@ -119,7 +119,7 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
ctx = context.WithValue(ctx, ctxKeyBaseURI, baseURI)
r = r.WithContext(ctx)

log.Info().Str("url_path", urlPath).Str("base_uri", baseURI).Msg("move urls")
log.Debug().Str("url_path", urlPath).Str("base_uri", baseURI).Msg("move urls")
// TODO make request.php optional in destination header
i := strings.Index(urlPath, baseURI)
if i == -1 {
Expand Down Expand Up @@ -214,13 +214,13 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
responses = append(responses, &responseXML{
Href: (&url.URL{Path: ctx.Value(ctxKeyBaseURI).(string) + "/"}).EscapedPath(), // url encode response.Href TODO (jfd) really? /should be ok ... we may actually only need to escape the username
Propstat: []propstatXML{
propstatXML{
{
Status: "HTTP/1.1 200 OK",
Prop: []*propertyXML{
s.newProp("d:resourcetype", "<d:collection/>"),
},
},
propstatXML{
{
Status: "HTTP/1.1 404 Not Found",
Prop: []*propertyXML{
s.newProp("oc:trashbin-original-filename", ""),
Expand All @@ -231,28 +231,7 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
},
},
})
/*
for i := range items {
vi := &provider.ResourceInfo{
// TODO(jfd) we cannot access version content, this will be a problem when trying to fetch version thumbnails
//Opaque
Type: provider.ResourceType_RESOURCE_TYPE_FILE,
Id: &provider.ResourceId{
StorageId: "trashbin", // this is a virtual storage
OpaqueId: path.Join("trash-bin", u.Username, items[i].GetKey()),
},
//Checksum
//Etag: v.ETag,
//MimeType
Mtime: items[i].DeletionTime,
Path: items[i].Key,
//PermissionSet
Size: items[i].Size,
Owner: u.Id,
}
infos = append(infos, vi)
}
*/

for i := range items {
res, err := h.itemToPropResponse(ctx, s, pf, items[i])
if err != nil {
Expand All @@ -271,6 +250,9 @@ func (h *TrashbinHandler) formatTrashPropfind(ctx context.Context, s *svc, u *us
return msg, nil
}

// itemToPropResponse needs to create a listing that contains a key and destination
// the key is the name of an entry in the trash listing
// for now we need to limit trash to the users home, so we can expect all trash keys to have the home storage as the opaque id
func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, pf *propfindXML, item *provider.RecycleItem) (*responseXML, error) {

baseURI := ctx.Value(ctxKeyBaseURI).(string)
Expand Down Expand Up @@ -377,6 +359,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, pf *pr
return &response, nil
}

// restore has a destination and a key
func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, dst string, key string) {
ctx := r.Context()
log := appctx.GetLogger(ctx)
Expand All @@ -388,18 +371,28 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
return
}

rid := unwrap(key)
getHomeRes, err := client.GetHome(ctx, &provider.GetHomeRequest{})
if err != nil {
log.Error().Err(err).Msg("error calling GetHomeProvider")
w.WriteHeader(http.StatusInternalServerError)
return
}
if getHomeRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Int32("code", int32(getHomeRes.Status.Code)).Str("trace", getHomeRes.Status.Trace).Msg(getHomeRes.Status.Message)
w.WriteHeader(http.StatusInternalServerError)
}

req := &provider.RestoreRecycleItemRequest{
// use the target path to find the storage provider
// this means we can only undelete on the same storage, not to a different folder
// use the key which is prefixed with the StoragePath to lookup the correct storage ...
// TODO currently limited to the home storage
Ref: &provider.Reference{
Spec: &provider.Reference_Id{
Id: rid,
Spec: &provider.Reference_Path{
Path: getHomeRes.Path,
},
},
Key: rid.GetOpaqueId(),
Key: key,
RestorePath: dst,
}

Expand All @@ -420,6 +413,7 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
w.WriteHeader(http.StatusNoContent)
}

// delete has only a key
func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, key string) {

ctx := r.Context()
Expand All @@ -432,12 +426,43 @@ func (h *TrashbinHandler) delete(w http.ResponseWriter, r *http.Request, s *svc,
return
}

rid := unwrap(key)
getHomeRes, err := client.GetHome(ctx, &provider.GetHomeRequest{})
if err != nil {
log.Error().Err(err).Msg("error calling GetHomeProvider")
w.WriteHeader(http.StatusInternalServerError)
return
}
if getHomeRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Int32("code", int32(getHomeRes.Status.Code)).Str("trace", getHomeRes.Status.Trace).Msg(getHomeRes.Status.Message)
w.WriteHeader(http.StatusInternalServerError)
}
sRes, err := client.Stat(ctx, &provider.StatRequest{
Ref: &provider.Reference{
Spec: &provider.Reference_Path{
Path: getHomeRes.Path,
},
},
})
if err != nil {
log.Error().Err(err).Msg("error calling Stat")
w.WriteHeader(http.StatusInternalServerError)
return
}
if sRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Int32("code", int32(sRes.Status.Code)).Str("trace", sRes.Status.Trace).Msg(sRes.Status.Message)
w.WriteHeader(http.StatusInternalServerError)
}

// set key as opaque id, the storageprovider will use it as the key for the
// storage drives PurgeRecycleItem key call

req := &gateway.PurgeRecycleRequest{
Ref: &provider.Reference{
Spec: &provider.Reference_Id{
Id: rid,
Id: &provider.ResourceId{
OpaqueId: key,
StorageId: sRes.Info.Id.StorageId,
},
},
},
}
Expand Down
15 changes: 5 additions & 10 deletions pkg/storage/fs/owncloud/owncloud.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1931,10 +1931,6 @@ func (fs *ocfs) ListRecycle(ctx context.Context) ([]*provider.RecycleItem, error

func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error {
log := appctx.GetLogger(ctx)
u, ok := user.ContextGetUser(ctx)
if !ok {
return errors.Wrap(errtypes.UserRequired("userrequired"), "error getting user from ctx")
}
rp, err := fs.getRecyclePath(ctx)
if err != nil {
return errors.Wrap(err, "ocfs: error resolving recycle path")
Expand All @@ -1943,27 +1939,26 @@ func (fs *ocfs) RestoreRecycleItem(ctx context.Context, key string) error {

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

origin := "/"
if v, err := xattr.Get(src, trashOriginPrefix); err != nil {
log.Error().Err(err).Str("path", src).Msg("could not read origin")
log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin")
} else {
origin = path.Clean(string(v))
}
layout := templates.WithUser(u, fs.c.UserLayout)
tgt := path.Join(fs.wrap(ctx, path.Join("/", layout, origin)), strings.TrimSuffix(path.Base(src), suffix))
tgt := fs.wrap(ctx, path.Join("/", origin, strings.TrimSuffix(path.Base(src), suffix)))
// move back to original location
if err := os.Rename(src, tgt); err != nil {
log.Error().Err(err).Str("path", src).Msg("could not restore item")
log.Error().Err(err).Str("key", key).Str("origin", origin).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("path", 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