[tests-only] Tus tests for uploading to shared files with checksums #38484

swoichha · 2021-03-09T11:26:15Z

Description

This PR adds tests for:

Uploading to shared files

Related Issue

How Has This Been Tested?

🤖

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Database schema changes (next release will require increase of minor version instead of patch)
Breaking change (fix or feature that would cause existing functionality to change)
Technical debt
Tests only (no source changes)

Checklist:

Talank

LGTM

Talank · 2021-03-09T11:32:52Z

tests/acceptance/features/apiWebdavUploadTUS/uploadToShare.feature

+    When user "Brian" creates a new TUS resource on the WebDAV API with these headers:
+      | Tus-Resumable   | 1.0.0                                         |
+      | Upload-Length   | 16                         |
+      | Upload-Metadata | filename L1NoYXJlcy9GT0xERVIvdGV4dGZpbGUudHh0 |


it is hard to understand the encrypted name and would have been better if there is a comment somewhere that says what is the actual name of this file.
But, that's just a suggestion. Everything looks good to me anyway. 👍

The encrypted file name can be seen here https://github.com/owncloud/core/pull/38484/files#diff-0613ee3f838a6348f280cfa6593798e61953aea15b0fb94b85d92335095700d9R91.

So far for all the tests we haven't mentioned the encrypted file name(which is sent as metadata to created the file) in any comments so I don't if we should or shouldn't do it. @individual-it what do you think about this idea of mentioning encrypted filename in a comment?

I think it would be good to mention. maybe let's have an other PR doing that for all places

individual-it

please also add tests where

the sharer upload a file in a shared folder and the receiver checks the checksum with propfind and in the header
upload with other checksum types
upload with chucks to a share (from both sides)
upload wrong checkssums (from both sides)

if you like some or all of those things could go in a separate PR

individual-it · 2021-03-10T06:18:22Z

tests/acceptance/features/apiWebdavUploadTUS/optionsRequest.feature

+      | Tus-Resumable          | 1.0.0                                  |
+      | Tus-Version            | 1.0.0                                  |
+      | Tus-Extension          | creation,creation-with-upload,checksum |
+      | Tus-Checksum-Algorithm | md5,sha1,adler32                       |


these tests are good, but IMO don't belong into this PR as it claims to be about shares

individual-it · 2021-03-10T06:21:36Z

tests/acceptance/features/apiWebdavUploadTUS/uploadToShare.feature

+      | Upload-Metadata | filename L1NoYXJlcy9GT0xERVIvdGV4dGZpbGUudHh0 |
+    And user "Brian" uploads file with checksum "MD5 827ccb0eea8a706c4c34a16891f84e7b" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API
+    Then as "Alice" file "/FOLDER/textfile.txt" should exist
+    And the content of file "/FOLDER/textfile.txt" for user "Alice" should be "uploaded content"


checking the checksum in header and by webdav would be useful

dpakach

apart from everything mentioned already looks good

swoichha · 2021-03-10T08:46:24Z

please also add tests where

the sharer upload a file in a shared folder and the receiver checks the checksum with propfind and in the header

upload with other checksum types

upload with chucks to a share (from both sides)

upload wrong checkssums (from both sides)

if you like some or all of those things could go in a separate PR

@individual-it I have added all these tests and removed changes done in tests for OPTIONS request, I will add them back later on new PR as this PR claims to be about shares.

Talank · 2021-03-10T09:03:03Z

tests/acceptance/features/apiWebdavUploadTUS/uploadToShare.feature

+    And user "Alice" has created a new TUS resource on the WebDAV API with these headers:
+      | Upload-Length   | 5                                     |
+      | Upload-Metadata | filename L0ZPTERFUi90ZXh0RmlsZS50eHQ= |
+    And user "Alice" uploads file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513954" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API


Suggested change

And user "Alice" uploads file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513954" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API

When user "Alice" uploads file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513954" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API

I think this is supposed to be a When step

Talank

LGTM 👍

sonarcloud · 2021-03-11T04:35:21Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

swoichha self-assigned this Mar 9, 2021

swoichha added 3 - To Review QA:team labels Mar 9, 2021

swoichha requested review from Talank, kiranparajuli589, jasson99, dpakach and haribhandari07 March 9, 2021 11:27

swoichha mentioned this pull request Mar 9, 2021

Ensure file consistency using checksums owncloud/ocis#1754

Closed

4 tasks

Talank reviewed Mar 9, 2021

View reviewed changes

swoichha requested a review from individual-it March 10, 2021 05:57

individual-it requested changes Mar 10, 2021

View reviewed changes

dpakach reviewed Mar 10, 2021

View reviewed changes

swoichha force-pushed the tusTests-checksum-shared-file branch from 7fe3b9b to 397386d Compare March 10, 2021 08:43

swoichha requested a review from Talank March 10, 2021 08:43

swoichha requested a review from individual-it March 10, 2021 08:46

swoichha mentioned this pull request Mar 10, 2021

Add TUS checksumming tests owncloud/ocis#1599

Closed

Talank reviewed Mar 10, 2021

View reviewed changes

swoichha requested a review from Talank March 10, 2021 09:23

swoichha force-pushed the tusTests-checksum-shared-file branch from 397386d to bd4b4fb Compare March 10, 2021 09:23

Talank reviewed Mar 10, 2021

View reviewed changes

individual-it approved these changes Mar 10, 2021

View reviewed changes

swoichha force-pushed the tusTests-checksum-shared-file branch from bd4b4fb to 0abc926 Compare March 10, 2021 10:29

swoichha added 2 commits March 11, 2021 09:18

add tests for uploading to shared files

3dc17bf

added extra line after each test

51fafc8

swoichha force-pushed the tusTests-checksum-shared-file branch from 7d0ebfa to 51fafc8 Compare March 11, 2021 03:33

swoichha merged commit d90b702 into master Mar 11, 2021

delete-merged-branch bot deleted the tusTests-checksum-shared-file branch March 11, 2021 05:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests-only] Tus tests for uploading to shared files with checksums #38484

[tests-only] Tus tests for uploading to shared files with checksums #38484

swoichha commented Mar 9, 2021 •

edited

Loading

Talank left a comment

Talank Mar 9, 2021

swoichha Mar 10, 2021

individual-it Mar 10, 2021

individual-it left a comment •

edited by swoichha

Loading

individual-it Mar 10, 2021

individual-it Mar 10, 2021

dpakach left a comment

swoichha commented Mar 10, 2021

Talank Mar 10, 2021

Talank left a comment

sonarcloud bot commented Mar 11, 2021

	And user "Alice" uploads file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513954" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API
	When user "Alice" uploads file with checksum "SHA1 8cb2237d0679ca88db6464eac60da96345513954" to the last created TUS Location with offset "0" and content "uploaded content" using the TUS protocol on the WebDAV API

[tests-only] Tus tests for uploading to shared files with checksums #38484

[tests-only] Tus tests for uploading to shared files with checksums #38484

Conversation

swoichha commented Mar 9, 2021 • edited Loading

Description

Related Issue

How Has This Been Tested?

Types of changes

Checklist:

Talank left a comment

Choose a reason for hiding this comment

Talank Mar 9, 2021

Choose a reason for hiding this comment

swoichha Mar 10, 2021

Choose a reason for hiding this comment

individual-it Mar 10, 2021

Choose a reason for hiding this comment

individual-it left a comment • edited by swoichha Loading

Choose a reason for hiding this comment

individual-it Mar 10, 2021

Choose a reason for hiding this comment

individual-it Mar 10, 2021

Choose a reason for hiding this comment

dpakach left a comment

Choose a reason for hiding this comment

swoichha commented Mar 10, 2021

Talank Mar 10, 2021

Choose a reason for hiding this comment

Talank left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Mar 11, 2021

swoichha commented Mar 9, 2021 •

edited

Loading

individual-it left a comment •

edited by swoichha

Loading