Skip to content

Implement support for server chunked file upload V2 API - #5939

Merged
claucambra merged 17 commits into
masterfrom
feature/chunk-c2
Aug 31, 2023
Merged

Implement support for server chunked file upload V2 API#5939
claucambra merged 17 commits into
masterfrom
feature/chunk-c2

Conversation

@claucambra

@claucambra claucambra commented Jul 31, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Implement #5819

@claucambra claucambra self-assigned this Jul 31, 2023
github-actions[bot]
github-actions Bot previously requested changes Jul 31, 2023
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

Comment thread src/libsync/propagateupload.h Outdated
Comment thread src/libsync/propagateupload.h Outdated
Comment thread src/libsync/syncoptions.h Outdated
Comment thread src/libsync/syncoptions.h Outdated
@codecov

codecov Bot commented Jul 31, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5939 (2f9dd79) into master (eec4069) will decrease coverage by 0.03%.
Report is 12 commits behind head on master.
The diff coverage is 90.90%.

❗ Current head 2f9dd79 differs from pull request most recent head a0c528a. Consider uploading reports for the commit a0c528a to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5939      +/-   ##
==========================================
- Coverage   60.09%   60.06%   -0.03%     
==========================================
  Files         145      145              
  Lines       18810    18838      +28     
==========================================
+ Hits        11303    11315      +12     
- Misses       7507     7523      +16
Files Changed Coverage Δ
src/common/syncjournaldb.h 75.00% <ø> (ø)
src/libsync/propagateupload.h 30.35% <0.00%> (ø)
src/libsync/propagateuploadv1.cpp 69.44% <33.33%> (ø)
src/common/syncjournaldb.cpp 79.53% <66.66%> (ø)
src/libsync/bulkpropagatorjob.cpp 74.71% <100.00%> (ø)
src/libsync/configfile.cpp 27.71% <100.00%> (+0.62%) ⬆️
src/libsync/owncloudpropagator.cpp 84.63% <100.00%> (ø)
src/libsync/propagateuploadng.cpp 84.72% <100.00%> (+2.33%) ⬆️
src/libsync/syncoptions.cpp 74.46% <100.00%> (+8.75%) ⬆️
src/libsync/syncoptions.h 100.00% <100.00%> (ø)

... and 8 files with indirect coverage changes

@github-actions
github-actions Bot dismissed their stale review July 31, 2023 07:25

No clang-tidy warnings found so I assume my comments were addressed

@sonarqubecloud

sonarqubecloud Bot commented Jul 31, 2023

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@tobiasKaminsky tobiasKaminsky linked an issue Aug 2, 2023 that may be closed by this pull request
Use MultipartUpload for uploading chunks to s3 #5819
Closed
@claucambra claucambra linked an issue Aug 13, 2023 that may be closed by this pull request
Support s3-multipart feature #5554
Closed
mgallien
mgallien requested changes Aug 31, 2023
View reviewed changes

@mgallien mgallien left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not done yet
see my comments and please fix the build failure in the drone CI pipeline

Comment thread src/common/syncjournaldb.h Outdated
Comment thread src/libsync/bulkpropagatorjob.cpp Outdated
@claucambra
claucambra requested a review from mgallien August 31, 2023 11:35
@claucambra @mgallien
Const autofy startChunkUpload in propagatuploadng
795cd8c 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Refactor startChunkUpload, move upload finalisation to separate method
d99d35e 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Clean up declaration of propagateuploadng
12827d1 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Split getting chunk upload folder from chunkUrl as this was previousl…
614d48b 
…y misleading and unclear

Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Fix http error code uint warning
c29181e 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Conform to chunk sizing restrictions introduced in Chunking V2
cbbb4c8 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Start numbering chunks from 1 per chunking v2 api
fbb513a 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Sent destination header per chunking v2 api
a647ee1 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Assert that chunk num passed to chunkUrl is at least 1
a8a2a86 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Ensure chunkUploadFolder components are QStrings
7d90641 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Fix chunk numbering in chunking tests
189bc67 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Clarify use of ProgressInfo _chunk
7cb8640 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Enforce Chunk V2 API chunk size limits at the sync options level
2f5c164 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Add test for Chunk V2 restrictions in testchunkingng
eb01a57 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Use [[nodiscard]] for new syncoptions and propagateupload functions
eadb458 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Ensure default min and max chunk sizes are multiplied as long longs
f46d71f 
Signed-off-by: Claudio Cambra <[email protected]>
@claucambra @mgallien
Replace comments about relevancy of chunk to propagat v1 with variabl…
a0c528a 
…e name change

Signed-off-by: Claudio Cambra <[email protected]>
@mgallien
mgallien enabled auto-merge August 31, 2023 13:43
@nextcloud-desktop-bot

nextcloud-desktop-bot commented Aug 31, 2023

Copy link
Copy Markdown

AppImage file: nextcloud-PR-5939-a0c528a19044599eaad029b063f595697bb018aa-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.

@sonarqubecloud

sonarqubecloud Bot commented Aug 31, 2023

Copy link
Copy Markdown

SonarCloud Quality Gate failed.    Quality Gate failed

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

96.9% 96.9% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@claucambra
claucambra disabled auto-merge August 31, 2023 14:44
@claucambra
claucambra merged commit ec9b940 into master Aug 31, 2023
@claucambra
claucambra deleted the feature/chunk-c2 branch August 31, 2023 14:45
@mgallien mgallien added this to the 3.10.0 milestone Sep 7, 2023
@solracsf solracsf mentioned this pull request Oct 27, 2023
Closed
@joshtrichards joshtrichards mentioned this pull request Aug 17, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgallien mgallien mgallien approved these changes
@github-actions github-actions[bot] github-actions[bot] left review comments
@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner
@allexzander allexzander Awaiting requested review from allexzander

Assignees

@claucambra claucambra

Labels

None yet

Projects

None yet

Milestone

3.10.0

Development

Successfully merging this pull request may close these issues.

Use MultipartUpload for uploading chunks to s3 Support s3-multipart feature

3 participants

@claucambra @nextcloud-desktop-bot @mgallien