Fix recycle to different location #1541
Conversation
aduffeck
force-pushed
the
fix-recycle-to-different-location
branch
11 times, most recently
from
3c8006e
to
6f79f53
Compare
aduffeck
force-pushed
the
fix-recycle-to-different-location
branch
6 times, most recently
from
9075f6c
to
ab76e87
Compare
|
@labkode @ishank011 I implemented the fix for the Note: Integration tests are currently still failing because of #1576 |
|
@aduffeck sure, I'll take it up this week. |
|
@ishank011 awesome, thank you 👍 |
aduffeck
force-pushed
the
fix-recycle-to-different-location
branch
from
ab76e87
to
fff301e
Compare
This is specified in the CS3 APIs as the "restore_path" field of the "RestoreRecycleItemRequest" resource but wasn't implemented in the storage provider until now.
aduffeck
force-pushed
the
fix-recycle-to-different-location
branch
from
fff301e
to
a1ecd7f
Compare
| if restorePath == "" { | ||
| v, err := xattr.Get(src, trashOriginPrefix) | ||
| if err != nil { | ||
| log.Error().Err(err).Str("key", key).Str("path", src).Msg("could not read origin") |
There was a problem hiding this comment.
Shouldn't we return here? Because if err != nil, restorePath will be the base of src, restoring to which might create conflicts
There was a problem hiding this comment.
It also restored to / before, I didn't want to change behaviour like that with this PR.
Having said that I think the origin xattr will only be missing in very bad exceptions anyway. In those cases it might be desirable to have the files restored to / (if possible), no?
| @@ -63,7 +63,7 @@ type Tree interface { | |||
| // CreateReference(ctx context.Context, node *node.Node, targetURI *url.URL) error | |||
| Move(ctx context.Context, oldNode *node.Node, newNode *node.Node) (err error) | |||
| Delete(ctx context.Context, node *node.Node) (err error) | |||
| RestoreRecycleItemFunc(ctx context.Context, key string) (*node.Node, func() error, error) | |||
| RestoreRecycleItemFunc(ctx context.Context, key, targetPath string) (*node.Node, func() error, error) | |||
There was a problem hiding this comment.
Can you change it to restorePath to maintain consistency?
There was a problem hiding this comment.
@ishank011 👍, done
Restoring to a different location than the original one is specified in the CS3 APIs as the
restore_pathfield of theRestoreRecycleItemRequestresource but wasn't implemented in the storage provider until now.