From f09e6123615920ad3977d2ad67fd65e1ec1b1b7e Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 24 Aug 2017 17:14:17 +0200 Subject: [PATCH 1/3] Store new dav chunks directly, no hooks Store new dav chunks directly instead of going through File::put() which would run all hooks. Hooks are now only triggered at the end of the transfer when the future file is moved to the final file, in which case the assemble data goes through File::put() and triggers the hooks. --- apps/dav/lib/Connector/Sabre/Node.php | 4 ++++ apps/dav/lib/Upload/UploadFolder.php | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 4f393116cbe6..75fbfe5853e7 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -394,4 +394,8 @@ protected function sanitizeMtime ($mtimeFromRequest) { return $mtime; } + public function getView() { + return $this->fileView; + } + } diff --git a/apps/dav/lib/Upload/UploadFolder.php b/apps/dav/lib/Upload/UploadFolder.php index 0597e742e3de..09bc0cbe6d35 100644 --- a/apps/dav/lib/Upload/UploadFolder.php +++ b/apps/dav/lib/Upload/UploadFolder.php @@ -34,8 +34,8 @@ function __construct(Directory $node) { } function createFile($name, $data = null) { - // TODO: verify name - should be a simple number - $this->node->createFile($name, $data); + $view = $this->node->getView(); + $view->file_put_contents($this->node->getPath() . '/' . $name, $data); } function createDirectory($name) { From eef29c1b3e9a9768a641a148bbd193e874861bb6 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Fri, 25 Aug 2017 10:17:16 +0200 Subject: [PATCH 2/3] Adjust checksum tests for new dav chunking Don't send checksums for each chunk but for the final move. Added test for the final move. Removed test code for chunk checksum. --- .../integration/features/bootstrap/WebDav.php | 59 ++++++++----------- tests/integration/features/checksums.feature | 14 ++++- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/tests/integration/features/bootstrap/WebDav.php b/tests/integration/features/bootstrap/WebDav.php index f8277f8ff9a6..03d60e2069cb 100644 --- a/tests/integration/features/bootstrap/WebDav.php +++ b/tests/integration/features/bootstrap/WebDav.php @@ -859,55 +859,42 @@ public function userUploadsNewChunkFileOfWithToId($user, $num, $data, $id) } /** - * @Given user :user uploads new chunk file :num with :data to id :id with checksum :checksum + * @Given user :user moves new chunk file with id :id to :dest */ - public function userUploadsNewChunkFileOfWithToIdWithChecksum($user, $num, $data, $id, $checksum) - { - try { - $data = \GuzzleHttp\Stream\Stream::factory($data); - $destination = '/uploads/' . $user . '/' . $id . '/' . $num; - $this->makeDavRequest( - $user, - 'PUT', - $destination, - ['OC-Checksum' => $checksum], - $data, - "uploads" - ); - } catch (\GuzzleHttp\Exception\BadResponseException $ex) { - $this->response = $ex->getResponse(); - } + public function userMovesNewChunkFileWithIdToMychunkedfile($user, $id, $dest) { + $this->moveNewDavChunkToFinalFile($user, $id, $dest, []); } /** - * @Given user :user moves new chunk file with id :id to :dest + * @Then user :user moves new chunk file with id :id to :dest with size :size */ - public function userMovesNewChunkFileWithIdToMychunkedfile($user, $id, $dest) - { - try { - $source = '/uploads/' . $user . '/' . $id . '/.file'; - $destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest; - $this->makeDavRequest($user, 'MOVE', $source, [ - 'Destination' => $destination - ], null, "uploads"); - } catch (\GuzzleHttp\Exception\RequestException $ex) { - $this->response = $ex->getResponse(); - } + public function userMovesNewChunkFileWithIdToMychunkedfileWithSize($user, $id, $dest, $size) { + $this->moveNewDavChunkToFinalFile($user, $id, $dest, ['OC-Total-Length' => $size]); } /** - * @Then user :user moves new chunk file with id :id to :dest with size :size + * @Then user :user moves new chunk file with id :id to :dest with checksum :checksum */ - public function userMovesNewChunkFileWithIdToMychunkedfileWithSize($user, $id, $dest, $size) - { + public function userMovesNewChunkFileWithIdToMychunkedfileWithChecksum($user, $id, $dest, $checksum) { + $this->moveNewDavChunkToFinalFile($user, $id, $dest, ['OC-Checksum' => $checksum]); + } + + /** + * Move chunked new dav file to final file + * + * @param string $user user + * @param string $id upload id + * @param string $dest destination path + * @param array $headers extra headers + */ + private function moveNewDavChunkToFinalFile($user, $id, $dest, $headers) { $source = '/uploads/' . $user . '/' . $id . '/.file'; $destination = $this->baseUrlWithoutOCSAppendix() . $this->getDavFilesPath($user) . $dest; + $headers['Destination'] = $destination; + try { - $this->response = $this->makeDavRequest($user, 'MOVE', $source, [ - 'Destination' => $destination, - 'OC-Total-Length' => $size - ], null, "uploads"); + $this->response = $this->makeDavRequest($user, 'MOVE', $source, $headers, null, "uploads"); } catch(\GuzzleHttp\Exception\BadResponseException $ex) { $this->response = $ex->getResponse(); } diff --git a/tests/integration/features/checksums.feature b/tests/integration/features/checksums.feature index 50618cd0b166..54087651453e 100644 --- a/tests/integration/features/checksums.feature +++ b/tests/integration/features/checksums.feature @@ -124,12 +124,22 @@ Feature: checksums When user "user0" downloads the file "/local_storage/prueba_cksum.txt" Then The header checksum should match "SHA1:a35b7605c8f586d735435535c337adc066c2ccb6" - Scenario: Upload chunked file where checksum does not match + Scenario: Upload new dav chunked file where checksum matches Given using new dav path And user "user0" exists And user "user0" creates a new chunking upload with id "chunking-42" And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" - And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" with checksum "SHA1:f005ba11" + And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" + And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" with checksum "SHA1:5d84d61b03fdacf813640f5242d309721e0629b1" + Then the HTTP status code should be "201" + + Scenario: Upload new dav chunked file where checksum does not match + Given using new dav path + And user "user0" exists + And user "user0" creates a new chunking upload with id "chunking-42" + And user "user0" uploads new chunk file "2" with "BBBBB" to id "chunking-42" + And user "user0" uploads new chunk file "3" with "CCCCC" to id "chunking-42" + And user "user0" moves new chunk file with id "chunking-42" to "/myChunkedFile.txt" with checksum "SHA1:f005ba11" Then the HTTP status code should be "400" Scenario: Upload a file where checksum does not match From 44e72ac9b149d67d5c0c402049cfc676c3823350 Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Mon, 28 Aug 2017 12:35:14 +0200 Subject: [PATCH 3/3] Directory::createFileDirectly --- apps/dav/lib/Connector/Sabre/Directory.php | 10 ++++++++++ apps/dav/lib/Connector/Sabre/Node.php | 5 ----- apps/dav/lib/Upload/UploadFolder.php | 4 ++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/Directory.php b/apps/dav/lib/Connector/Sabre/Directory.php index 15bf6ab76161..16a68d1fa966 100644 --- a/apps/dav/lib/Connector/Sabre/Directory.php +++ b/apps/dav/lib/Connector/Sabre/Directory.php @@ -461,4 +461,14 @@ public function moveInto($targetName, $fullSourcePath, INode $sourceNode) { return true; } + + /** + * Create a file directly, bypassing the hooks + * + * @param string $name name + * @param resource $data data + */ + public function createFileDirectly($name, $data) { + $this->fileView->file_put_contents($this->getPath() . '/' . $name, $data); + } } diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php index 75fbfe5853e7..60f3816bd1fc 100644 --- a/apps/dav/lib/Connector/Sabre/Node.php +++ b/apps/dav/lib/Connector/Sabre/Node.php @@ -393,9 +393,4 @@ protected function sanitizeMtime ($mtimeFromRequest) { } return $mtime; } - - public function getView() { - return $this->fileView; - } - } diff --git a/apps/dav/lib/Upload/UploadFolder.php b/apps/dav/lib/Upload/UploadFolder.php index 09bc0cbe6d35..19e511f260d6 100644 --- a/apps/dav/lib/Upload/UploadFolder.php +++ b/apps/dav/lib/Upload/UploadFolder.php @@ -34,8 +34,8 @@ function __construct(Directory $node) { } function createFile($name, $data = null) { - $view = $this->node->getView(); - $view->file_put_contents($this->node->getPath() . '/' . $name, $data); + // need to bypass hooks for individual chunks + $this->node->createFileDirectly($name, $data); } function createDirectory($name) {