Skip to content
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

fix bulk upload of empty files #5871

Merged
merged 2 commits into from Jul 10, 2023
Merged

fix bulk upload of empty files #5871

merged 2 commits into from Jul 10, 2023

Conversation

mgallien
Copy link
Collaborator

@mgallien mgallien commented Jul 7, 2023

force an empty body when we bulk upload empty files

force a "valid" checksum to be computed for empty files as bulk upload server side expects a checksum even for empty files

Close #5824

@mgallien
Copy link
Collaborator Author

mgallien commented Jul 7, 2023

require also a fix from server nextcloud/server#39221

@mgallien mgallien force-pushed the bugfix/bulkUploadEmptyFiles branch from 02beb67 to 1fb6fb7 Compare July 7, 2023 09:29
@mgallien
Copy link
Collaborator Author

mgallien commented Jul 7, 2023

/backport to stable-3.9

@allexzander
Copy link
Collaborator

@mgallien Looks nice, but I am still getting an error when syncing an empty file:

[WARNING] Could not complete propagation of "New Text Document (5).txt" by OCC::BulkPropagatorJob(0x2b23bd9f650) with status OCC::SyncFileItem::NormalError and error: "Network error: 302"
modify final status NormalError OCC::SyncFileItem::NormalError OCC::SyncFileItem::NormalError
[WARNING] Item "New Text Document (5).txt" retrieved resulted in "Network error: 302"
[WARNING] Item "New Text Document (5).txt" retrieved resulted in error "Network error: 302"

@claucambra claucambra self-requested a review July 8, 2023 15:31
@mgallien
Copy link
Collaborator Author

@mgallien Looks nice, but I am still getting an error when syncing an empty file:

[WARNING] Could not complete propagation of "New Text Document (5).txt" by OCC::BulkPropagatorJob(0x2b23bd9f650) with status OCC::SyncFileItem::NormalError and error: "Network error: 302" modify final status NormalError OCC::SyncFileItem::NormalError OCC::SyncFileItem::NormalError [WARNING] Item "New Text Document (5).txt" retrieved resulted in "Network error: 302" [WARNING] Item "New Text Document (5).txt" retrieved resulted in error "Network error: 302"

there are server side changes needed for a full fix
see nextcloud/server#39221

force an empty body when we bulk upload empty files

force a "valid" checksum to be computed for empty files as bulk upload
server side expects a checksum even for empty files

Close #5824

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@mgallien mgallien force-pushed the bugfix/bulkUploadEmptyFiles branch from 1fb6fb7 to 8cd1a30 Compare July 10, 2023 08:38
@allexzander
Copy link
Collaborator

@mgallien Looks nice, but I am still getting an error when syncing an empty file:
[WARNING] Could not complete propagation of "New Text Document (5).txt" by OCC::BulkPropagatorJob(0x2b23bd9f650) with status OCC::SyncFileItem::NormalError and error: "Network error: 302" modify final status NormalError OCC::SyncFileItem::NormalError OCC::SyncFileItem::NormalError [WARNING] Item "New Text Document (5).txt" retrieved resulted in "Network error: 302" [WARNING] Item "New Text Document (5).txt" retrieved resulted in error "Network error: 302"

there are server side changes needed for a full fix see nextcloud/server#39221

got it, approved, but could not test of course

Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #5871 (5540f7f) into master (4cc1395) will increase coverage by 0.00%.
The diff coverage is 77.77%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5871   +/-   ##
=======================================
  Coverage   60.08%   60.08%           
=======================================
  Files         145      145           
  Lines       18715    18711    -4     
=======================================
- Hits        11244    11242    -2     
+ Misses       7471     7469    -2     
Impacted Files Coverage Δ
src/libsync/putmultifilejob.cpp 88.46% <50.00%> (-3.38%) ⬇️
src/common/checksumcalculator.cpp 86.58% <100.00%> (-1.06%) ⬇️

... and 11 files with indirect coverage changes

@sonarcloud
Copy link

sonarcloud bot commented Jul 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5871-5540f7f0f4dde32c874ddd6e20508ba911497c1b-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@mgallien mgallien merged commit e25c59e into master Jul 10, 2023
13 checks passed
@mgallien mgallien deleted the bugfix/bulkUploadEmptyFiles branch July 10, 2023 15:56
@mgallien mgallien added this to the 3.10.0 milestone Jul 11, 2023
@AndyXheli
Copy link

@mgallien do this also fix ? #5758 (comment) looks like the same issue to me ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Error when creating a new file on Windows
5 participants