-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Fixed a problem with file names when moving from external storage to nextcloud (with ObjectStore S3 enabled) #43660
Fixed a problem with file names when moving from external storage to nextcloud (with ObjectStore S3 enabled) #43660
Conversation
…nextcloud (with ObjectStore S3 enabled) Signed-off-by: hopleus <hopleus@gmail.com>
/backport stable28 |
/backport stable27 |
/backport stable26 |
@joshtrichards Hi. Could you run the tests again? P.S. Last time cypress crashed with 503 errors and I didn't understand why |
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 don't think this is the correct approach to solve the issue, it ends up creating a cache entry for the .part
file that isn't used, it also impacts copies, where it will write to the wrong urn. And the method of passing the fileid to writeStream
is very fragile to other changes.
The best I can think of is having ObjectStoreStorage
overwrite moveFromStorage
where it can then write the files to the correct urn without making new cache entries for them and instead updating the existing source cache entries to move them to the new storage.
Great job with figuring out the root cause of the issue though! |
Signed-off-by: hopleus <hopleus@gmail.com>
I've changed the logic, check when the opportunity arises |
I can confirm that this solves the problem but the extra copy added will cause issues. I've created an alternative approach that forgoes the default Please verify that it resolves the issue. |
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 think the alternative from @icewind1991 looks a bit safer, could you double check if that also resolves your issue?
Thank you very much @hopleus 🙏 The alternative approach was just merged. Thanks again for bringing it to our attention and your thorough analysis of the root cause. |
Summary
Fixed a problem with file names when moving from external storage to nextcloud (with ObjectStore S3 enabled)
Use case
The file is copied from the external storage to the local one (extDisk: test.docx => localDisk: test.docx.part)
An entry is created about a new file (test.docx.part) in the database, the oc_filecache table. The new file contains a file id equal to the one that is saved as the filename in S3
Renames the file in the local storage from test.docx.part to test.docx
The information in the database is updated (new path, new storage ID) to record the source file (the one that we move from the external storage)
, the record in the database about the new (temporary) file (test.docx.part) is deleted.
Result: The old file id remains in the database, but with new information (path, name, storage). And S3 stores a file with a temporary ID that has not been changed after all the manipulations.
Checklist