- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 4.6k
handle moveFromStorage within the same storage even when storage wrap… #17264
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.
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
Conversation
…pers are applied to the source storage the target storage doesn't need additional handling for wrappers as the wrappers implementation of moveFromStorage already deals with that Any storage based on local storage isn't affected by this as local storage already has it's own way of handling with this Signed-off-by: Robin Appelman <robin@icewind.nl>
| * @var Jail $sourceStorage | ||
| */ | ||
| $sourceInternalPath = $sourceStorage->getUnjailedPath($sourceInternalPath); | ||
| $sourceStorage = $sourceStorage->getUnjailedStorage(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do this? It is not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is, in the next loop iteration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right it loops... 🤦♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same .... Pretty nice the while we loop the storage wrappers up thing.
| private function isSameStorage(IStorage $storage): bool { | ||
| while ($storage->instanceOfStorage(Wrapper::class)) { | ||
| /** | ||
| * @var Wrapper $sourceStorage | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * @var Wrapper $sourceStorage | |
| * @var Wrapper | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. And testing on objectstore with groupfolders didn't do 💥
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| @icewind1991 backport? | 
| /backport to stable17 | 
| /backport to stable16 | 
| backport to stable17 in #17277 | 
| backport to stable16 in #17278 | 
…pers are applied to the source storage
the target storage doesn't need additional handling for wrappers as the wrappers implementation of moveFromStorage already deals with that
Any storage based on local storage isn't affected by this as local storage already has it's own way of handling with this
You can test this by using groupfolders on a nextcloud instance using object storage, currently this will use the fallback "copy+delete" path in that case instead of a simple rename