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.

Sign up for GitHub

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

Jump to bottom

improve uploading file in chunks #33692

Merged
merged 1 commit into from
Nov 29, 2018
Merged

improve uploading file in chunks #33692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions tests/TestHelpers/WebDavHelper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,4 +214,27 @@ public static function sanitizeUrl($url, $trailingSlash = false) {
$url = \preg_replace("/([^:]\/)\/+/", '$1', $url);
return $url;
}

/**
* decides if the proposed dav version and chunking version are
* a valid combination.
* If no chunkingVersion is specified, then any dav version is valid.
* If a chunkingVersion is specified, then it has to match the dav version.
* Note: in future the dav and chunking versions might or might not
* move together and/or be supported together. So a more complex
* matrix could be needed here.
*
* @param string|int $davPathVersion
* @param string|int|null $chunkingVersion
*
* @return boolean is this a valid combination
*/
public static function isValidDavChunkingCombination(
$davPathVersion, $chunkingVersion
) {
return (
($chunkingVersion === 'no' || $chunkingVersion === null) ||
($davPathVersion === $chunkingVersion)
);
}
}
25 changes: 14 additions & 11 deletions tests/acceptance/features/apiWebdavOperations/uploadFileUsingNewChunking.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,14 @@ Feature: upload file using new chunking
Scenario: Checking file id after a move overwrite using new chunking endpoint
Given user "user0" has copied file "/textfile0.txt" to "/existingFile.txt"
And user "user0" has stored id of file "/existingFile.txt"
When user "user0" uploads the following chunks to "/existingFile.txt" with new chunking and using the WebDAV API
| 1 | AAAAA |
| 2 | BBBBB |
| 3 | CCCCC |
When user "user0" uploads file "filesForUpload/textfile.txt" to "/existingFile.txt" in 3 chunks with new chunking and using the WebDAV API
Then user "user0" file "/existingFile.txt" should have the previously stored id
And the content of file "/existingFile.txt" for user "user0" should be "AAAAABBBBBCCCCC"
And the content of file "/existingFile.txt" for user "user0" should be:
"""
This is a testfile.

Cheers.
"""
And the log file should not contain any log-entries containing these attributes:
| app |
| dav |
Expand Down Expand Up @@ -104,13 +106,14 @@ Feature: upload file using new chunking

@smokeTest
Scenario Outline: Upload files with difficult names using new chunking
When user "user0" creates a new chunking upload with id "chunking-42" using the WebDAV API
And user "user0" uploads new chunk file "1" with "AAAAA" to id "chunking-42" using the WebDAV API
And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" using the WebDAV API
And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" using the WebDAV API
And user "user0" moves new chunk file with id "chunking-42" to "/<file-name>" using the WebDAV API
When user "user0" uploads file "filesForUpload/textfile.txt" to "/<file-name>" in 3 chunks with new chunking and using the WebDAV API
Then as "user0" file "/<file-name>" should exist
And the content of file "/<file-name>" for user "user0" should be "AAAAABBBBBCCCCC"
And the content of file "/<file-name>" for user "user0" should be:
"""
This is a testfile.

Cheers.
"""
And the log file should not contain any log-entries containing these attributes:
| app |
| dav |
Expand Down
34 changes: 28 additions & 6 deletions tests/acceptance/features/apiWebdavOperations/uploadFileUsingOldChunking.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Feature: upload file using old chunking
Given using OCS API version "1"
And using old DAV path
And user "user0" has been created with default attributes
And the owncloud log level has been set to debug
And the owncloud log has been cleared

Scenario: Upload chunked file asc
When user "user0" uploads the following "3" chunks to "/myChunkedFile.txt" with old chunking and using the WebDAV API
Expand All @@ -33,16 +35,36 @@ Feature: upload file using old chunking
Then as "user0" file "/myChunkedFile.txt" should exist
And the content of file "/myChunkedFile.txt" for user "user0" should be "AAAAABBBBBCCCCC"

Scenario: Checking file id after a move overwrite using old chunking endpoint
Given user "user0" has copied file "/textfile0.txt" to "/existingFile.txt"
And user "user0" has stored id of file "/existingFile.txt"
When user "user0" uploads file "filesForUpload/textfile.txt" to "/existingFile.txt" in 3 chunks with old chunking and using the WebDAV API
Then user "user0" file "/existingFile.txt" should have the previously stored id
And the content of file "/existingFile.txt" for user "user0" should be:
"""
This is a testfile.

Cheers.
"""
And the log file should not contain any log-entries containing these attributes:
| app |
| dav |

@smokeTest
Scenario Outline: Chunked upload files with difficult name
When user "user0" uploads the following "3" chunks to "/<file-name>" with old chunking and using the WebDAV API
| 1 | AAAAA |
| 2 | BBBBB |
| 3 | CCCCC |
When user "user0" uploads file "filesForUpload/textfile.txt" to "/<file-name>" in 3 chunks using the WebDAV API
Then as "user0" file "/<file-name>" should exist
And the content of file "/<file-name>" for user "user0" should be "AAAAABBBBBCCCCC"
And the content of file "/<file-name>" for user "user0" should be:
"""
This is a testfile.

Cheers.
"""
And the log file should not contain any log-entries containing these attributes:
| app |
| dav |
Examples:
| file-name |
| 0 |
| &#? |
| TIÄFÜ |
| 0 |
117 changes: 80 additions & 37 deletions tests/acceptance/features/bootstrap/WebDav.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
use GuzzleHttp\Message\ResponseInterface;
use GuzzleHttp\Ring\Exception\ConnectException;
use GuzzleHttp\Stream\StreamInterface;
use Guzzle\Http\Exception\BadResponseException;
use Sabre\DAV\Client as SClient;
use Sabre\DAV\Xml\Property\ResourceType;
use Sabre\Xml\LibXMLException;
use TestHelpers\OcsApiHelper;
use TestHelpers\SetupHelper;
use TestHelpers\UploadHelper;
use TestHelpers\WebDavHelper;
use TestHelpers\HttpRequestHelper;
use Sabre\DAV\Xml\Property\Complex;
Expand Down Expand Up @@ -87,6 +89,9 @@ trait WebDav {
private $responseXml = [];

private $httpRequestTimeout = 0;

private $chunkingToUse = null;

/**
* @Given /^using dav path "([^"]*)"$/
*
Expand Down Expand Up @@ -1797,65 +1802,103 @@ public function userOnUploadsAFileTo($user, $server, $source, $destination) {
$this->userUploadsAFileTo($user, $source, $destination);
$this->usingServer($previousServer);
}

/**
* @When /^user "([^"]*)" uploads file "([^"]*)" to "([^"]*)" in (\d+) chunks (?:with (new|old|v1|v2) chunking and)?\s?using the WebDAV API$/
* @When user :user uploads file :source to :destination with chunks using the WebDAV API
*
* @param string $user
* @param string $source
* @param string $destination
* @param string $chunkingVersion null for autodetect, "old" with old style, "new" for new style
* @param int $noOfChunks
* @param string $chunkingVersion old|v1|new|v2 null for autodetect
* @param bool $async use asynchronous move at the end or not
*
* @return void
*/
public function userUploadsAFileToWithChunks(
$user, $source, $destination, $chunkingVersion = null
$user, $source, $destination, $noOfChunks = 2, $chunkingVersion = null, $async = false
) {
$size = \filesize(
$this->acceptanceTestsDirLocation() . $source
PHPUnit_Framework_Assert::assertGreaterThan(
0, $noOfChunks, "What does it mean to have $noOfChunks chunks?"
Copy link
Contributor

@phil-davis phil-davis Nov 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

future data compression algorithm that can fit the whole file content into 0 chunks ;)
research project?

Copy link
Member Author

Choose a reason for hiding this comment

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

its all there https://github.com/philipl/pifs

πfs is a revolutionary new file system that, instead of wasting space storing your data on your hard drive, stores your data in π! You'll never run out of space again - π holds every file that could possibly exist! They said 100% compression was impossible? You're looking at it!

);
$contents = \file_get_contents(
$this->acceptanceTestsDirLocation() . $source
//use the chunking version that works with the set dav version
if ($chunkingVersion === null) {
if ($this->usingOldDavPath) {
$chunkingVersion = "v1";
} else {
$chunkingVersion = "v2";
}
}
$this->useSpecificChunking($chunkingVersion);
PHPUnit_Framework_Assert::assertTrue(
WebDavHelper::isValidDavChunkingCombination(
($this->usingOldDavPath) ? 1 : 2,
$this->chunkingToUse
),
"invalid chunking/webdav version combination"
);

// use two chunks for the sake of testing
$chunks = [];
$chunks[] = \substr($contents, 0, $size / 2);
$chunks[] = \substr($contents, $size / 2);

$this->uploadChunks($user, $chunks, $destination, $chunkingVersion);
$headers = [];
if ($async === true) {
$headers = ['OC-LazyOps' => 'true'];
}
try {
$this->responseXml = [];
$this->response = UploadHelper::upload(
$this->getBaseUrl(),
$this->getActualUsername($user),
$this->getUserPassword($user),
$this->acceptanceTestsDirLocation() . $source,
$destination,
$headers,
($this->usingOldDavPath) ? 1 : 2,
$this->chunkingToUse,
$noOfChunks
);
} catch (BadResponseException $e) {
// 4xx and 5xx responses cause an exception
$this->response = $e->getResponse();
}
}

/**
* @When /^user "([^"]*)" uploads file "([^"]*)" asynchronously to "([^"]*)" in (\d+) chunks (?:with (new|old|v1|v2) chunking and)?\s?using the WebDAV API$/
*
* @param string $user
* @param array $chunks
* @param string $source
* @param string $destination
* @param string|null $chunkingVersion null for autodetect, "old" with old style, "new" for new style
* @param int $noOfChunks
* @param string $chunkingVersion old|v1|new|v2 null for autodetect
*
* @return void
*/
public function uploadChunks(
$user, $chunks, $destination, $chunkingVersion = null
public function userUploadsAFileAsyncToWithChunks(
$user, $source, $destination, $noOfChunks = 2, $chunkingVersion = null
) {
if ($chunkingVersion === null) {
if ($this->usingOldDavPath) {
$chunkingVersion = 'old';
} else {
$chunkingVersion = 'new';
}
}
if ($chunkingVersion === 'old') {
foreach ($chunks as $index => $chunkContent) {
$this->userUploadsChunkedFile(
$user, $index + 1, \count($chunks), $chunkContent, $destination
);
}
$this->userUploadsAFileToWithChunks(
$user, $source, $destination, $noOfChunks, $chunkingVersion, true
);
}

/**
* sets the chunking version from human readable format
*
* @param string $version (no|v1|v2|new|old)
*
* @return void
*/
public function useSpecificChunking($version) {
if ($version === "v1" || $version === "old") {
$this->chunkingToUse = 1;
} elseif ($version === "v2" || $version === "new") {
$this->chunkingToUse = 2;
} elseif ($version === "no") {
$this->chunkingToUse = null;
} else {
$chunkDetails = [];
foreach ($chunks as $index => $chunkContent) {
$chunkDetails[] = [$index + 1, $chunkContent];
}
$this->userUploadsChunksUsingNewChunking($user, $destination, 'chunking-43', $chunkDetails);
throw new InvalidArgumentException(
"cannot set chunking version to $version"
);
}
}

Expand Down Expand Up @@ -1937,7 +1980,7 @@ public function uploadWithAllMechanisms(
}

$this->userUploadsAFileToWithChunks(
$user, $source, $destination . $suffix, 'old'
$user, $source, $destination . $suffix, 2, 'old'
);
$responses[] = $this->response;
}
Expand All @@ -1946,7 +1989,7 @@ public function uploadWithAllMechanisms(
$suffix = "-{$dav}dav-newchunking";
}
$this->userUploadsAFileToWithChunks(
$user, $source, $destination . $suffix, 'new'
$user, $source, $destination . $suffix, 2, 'new'
);
$responses[] = $this->response;
}
Expand Down