fix(files): preserve mtimes when copying and moving - #62429
Conversation
Signed-off-by: Madison Steiner <8176115+mh0pe@users.noreply.github.com>
Signed-off-by: Madison Steiner <8176115+mh0pe@users.noreply.github.com>
|
Friendly maintainer ping @Altahrim @salmart-dev @leftybournes @come-nc — this is ready for review. It preserves mtimes across same-storage and cross-storage copy/move paths, including recursive directories, with focused regression coverage. Please let me know if you’d like any additional validation or changes to the scope. |
come-nc
left a comment
There was a problem hiding this comment.
Looks close to what I did in #43689
If I remember correctly @icewind1991 told me at the time that this was not always wanted (for performance reasons I think).
Still, some of what’s in there looks quite straightforward and shoud be merged I think. But maybe not all, especially the recursion on all copied files in a folder.
When comparing with my version it looks like you missed encryption wrapper.
| } | ||
| } | ||
|
|
||
| private function preserveCopyMtime(Storage $sourceStorage, Storage $targetStorage, string $sourceInternalPath, string $targetInternalPath): void { |
There was a problem hiding this comment.
This is recursively checking all files in a copied folder, was that tested for performance? Pretty sure it will add a big overhead.
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Summary
Preserve file and directory modification times consistently when copying and moving files, including same-storage operations, cross-storage fallbacks, and recursive directory copies.
What changed
View::copy(..., false)as an explicit opt-out.Why
Nextcloud already preserves the logical
mtimein the file cache while tracking the backing storage timestamp separately asstorage_mtime. The storage operations did not apply that model consistently:View::copy()ignored its preservation flag, recursive storage copies dropped the flag, the local fallback dropped it, and the known-mtime wrapper read timestamps after paths had moved.The copied or moved node now retains its content chronology while the destination parent's mtime and etag still record the operation. Directory copies restore mtimes bottom-up in a post-copy traversal, avoiding a public
IStoragesignature change for third-party storage implementations.Validation
git diff --check: passing.TODO
Checklist
master3. to review, feature component) — maintainer action requiredstable32) — maintainer action requiredAI (if applicable)