Skip to content

fix(S3): Use original folder size during copy - #52242

Merged
artonge merged 3 commits into
masterfrom
artonge/fix/copy_subfolders_s3
Jun 2, 2025
Merged

fix(S3): Use original folder size during copy#52242
artonge merged 3 commits into
masterfrom
artonge/fix/copy_subfolders_s3

Conversation

@artonge

@artongeartonge commented Apr 17, 2025

Copy link
Copy Markdown
Collaborator

This prevents having copied folders with a wrongly set size of 0KB.

Not sure about the handleCopiesAsOwned part, but I kept it from copyFile to preserve the same behaviour. It was introduced in #41565. Maybe @susnux you can judge whether it makes sense or not. Validated.

@artonge
artonge requested a review from a team as a code ownerApril 17, 2025 10:14
@artonge
artonge requested review from Altahrim, icewind1991 and provokateurin and removed request for a teamApril 17, 2025 10:14
@artongeartonge self-assigned this Apr 17, 2025
@artongeartonge added bug 3. to review Waiting for reviews feature: files php Pull requests that update Php code feature: object storage labels Apr 17, 2025
@artongeartonge added this to the Nextcloud 32 milestone Apr 17, 2025
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable31

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable30

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

/backport to stable29

@artonge
artonge requested a review from susnuxApril 17, 2025 10:16
Comment threadlib/private/Files/ObjectStore/ObjectStoreStorage.php Outdated
Comment threadlib/private/Files/ObjectStore/ObjectStoreStorage.php Outdated
@icewind1991

Copy link
Copy Markdown
Member

The least-invasive change would probably be to pass the needed metadata to mkdir

Alternatively you can first do the recursive cache copy, and then do the object copy recursively without changing the cache. (care has to be taken than to not have the db and object store desync if the object copy fails then)

@DaphneMuller

Copy link
Copy Markdown
Contributor

@artonge anything that we should be doing here to follow up on it?

@artonge

Copy link
Copy Markdown
CollaboratorAuthor

@artonge anything that we should be doing here to follow up on it?

No, just prioritizing :)

@artongeartonge added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 14, 2025
@codecov

codecovBot commented May 14, 2025

Copy link
Copy Markdown

❌ Unsupported file format

Upload processing failed due to unsupported file format. Please review the parser error message:

Error deserializing json
Caused by:
expected value at line 1 column 1

For more help, visit our troubleshooting guide.

@artonge
artongeforce-pushed the artonge/fix/copy_subfolders_s3 branch from 4fb8346 to 104b0c3CompareMay 14, 2025 16:54
@come-nc
come-nc removed their request for review May 14, 2025 17:59
@artonge
artongeforce-pushed the artonge/fix/copy_subfolders_s3 branch from 104b0c3 to 5305b24CompareMay 16, 2025 14:20
@artonge
artongeforce-pushed the artonge/fix/copy_subfolders_s3 branch 2 times, most recently from f0a41af to 45e7407CompareMay 19, 2025 09:48
@artonge

Copy link
Copy Markdown
CollaboratorAuthor

The least-invasive change would probably be to pass the needed metadata to mkdir

Alternatively you can first do the recursive cache copy, and then do the object copy recursively without changing the cache. (care has to be taken than to not have the db and object store desync if the object copy fails then)

@icewind1991 I ended up keeping the mkdir as is, and updating its size right after.

@artongeartonge changed the title fix(S3): Copy folder's cache info instead of creating a new onefix(S3): Use original folder size during copyMay 22, 2025
@artonge
artongeforce-pushed the artonge/fix/copy_subfolders_s3 branch from 45e7407 to fa2ed6bCompareMay 22, 2025 08:25
@DaphneMuller

Copy link
Copy Markdown
Contributor

@artonge@icewind1991 any chance this can be merged soon?

@icewind1991

Copy link
Copy Markdown
Member

@icewind1991 I ended up keeping the mkdir as is, and updating its size right after.

Pushed a change to do the size in the mkdir, there is no need to do 3 queries to the filecache when 1 can suffice.

Also pushed a test to ensure this doesn't regress.

artongeand others added 3 commits May 28, 2025 15:52
This prevent having copied folders with a wrongly set size of 0KB.
- Fix#51916
Signed-off-by: Louis Chemineau <louis@chmn.me>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@artonge
artongeforce-pushed the artonge/fix/copy_subfolders_s3 branch from d4378de to e21ce79CompareMay 28, 2025 13:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developingWork in progressbugfeature: filesfeature: object storagephpPull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Object storage (S3) folder on duplication has 0 KB folder size

6 participants

@artonge@icewind1991@DaphneMuller@susnux@come-nc@skjnldsv