Merge pull request #22602 from owncloud/fix_slow_chunkcheck

Do not check all chunks of a chunked upload if we do not need to

build/integration/features/bootstrap/WebDav.php

@@ -262,5 +262,16 @@ trait WebDav {
 		}
 	}
 
+	/**
+	 * @Given user :user uploads chunk file :num of :total with :data to :destination
+	 */
+	public function userUploadsChunkFileOfWithToWithChecksum($user, $num, $total, $data, $destination)
+	{
+		$num -= 1;
+		$data = \GuzzleHttp\Stream\Stream::factory($data);
+		$file = $destination . '-chunking-42-'.$total.'-'.$num;
+		$this->makeDavRequest($user, 'PUT', $file, ['OC-Chunked' => '1'], $data);
+	}
+
 }
 

build/integration/features/webdav-related.feature

@@ -141,3 +141,30 @@ Feature: webdav-related
 		Given Logging in using web as "admin"
 		When Sending a "PROPFIND" to "/remote.php/webdav/welcome.txt" with requesttoken
 		Then the HTTP status code should be "207"
+
+	Scenario: Upload chunked file asc
+		Given user "user0" exists
+		And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt"
+		When As an "user0"
+		And Downloading file "/myChunkedFile.txt"
+		Then Downloaded content should be "AAAAABBBBBCCCCC"
+
+	Scenario: Upload chunked file desc
+		Given user "user0" exists
+		And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt"
+		When As an "user0"
+		And Downloading file "/myChunkedFile.txt"
+		Then Downloaded content should be "AAAAABBBBBCCCCC"
+
+	Scenario: Upload chunked file random
+		Given user "user0" exists
+		And user "user0" uploads chunk file "2" of "3" with "BBBBB" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "3" of "3" with "CCCCC" to "/myChunkedFile.txt"
+		And user "user0" uploads chunk file "1" of "3" with "AAAAA" to "/myChunkedFile.txt"
+		When As an "user0"
+		And Downloading file "/myChunkedFile.txt"
+		Then Downloaded content should be "AAAAABBBBBCCCCC"

lib/private/filechunking.php

@@ -74,14 +74,16 @@ class OC_FileChunking {
 
 	public function isComplete() {
 		$prefix = $this->getPrefix();
-		$parts = 0;
 		$cache = $this->getCache();
-		for($i=0; $i < $this->info['chunkcount']; $i++) {
-			if ($cache->hasKey($prefix.$i)) {
-				$parts ++;
+		$chunkcount = (int)$this->info['chunkcount'];
+
+		for($i=($chunkcount-1); $i >= 0; $i--) {
+			if (!$cache->hasKey($prefix.$i)) {
+				return false;
 			}
 		}
-		return $parts == $this->info['chunkcount'];
+
+		return true;
 	}
 
 	/**

tests/lib/filechunking.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * @author Roeland Jago Douma <rullzer@owncloud.com>
+ *
+ * @copyright Copyright (c) 2016, ownCloud, Inc.
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+namespace Test;
+
+class FileChunking extends \Test\TestCase {
+
+	public function dataIsComplete() {
+		return [
+			[1, [], false],
+			[1, [0], true],
+			[2, [], false],
+			[2, [0], false],
+			[2, [1], false],
+			[2, [0,1], true],
+			[10, [], false],
+			[10, [0,1,2,3,4,5,6,7,8], false],
+			[10, [1,2,3,4,5,6,7,8,9], false],
+			[10, [0,1,2,3,5,6,7,8,9], false],
+			[10, [0,1,2,3,4,5,6,7,8,9], true],
+		];
+	}
+
+	/**
+	 * @dataProvider dataIsComplete
+	 * @param $total
+	 * @param array $present
+	 * @param $expected
+	 */
+	public function testIsComplete($total, array $present, $expected) {
+		$fileChunking = $this->getMockBuilder('\OC_FileChunking')
+			->setMethods(['getCache'])
+			->setConstructorArgs([[
+				'name' => 'file',
+				'transferid' => '42',
+				'chunkcount' => $total,
+			]])
+			->getMock();
+
+		$cache = $this->getMock('\OCP\ICache');
+
+		$cache->expects($this->atLeastOnce())
+			->method('hasKey')
+			->will($this->returnCallback(function ($key) use ($present) {
+				$data = explode('-', $key);
+				return in_array($data[3], $present);
+			}));
+
+		$fileChunking->method('getCache')->willReturn($cache);
+
+		$this->assertEquals($expected, $fileChunking->isComplete());
+	}
+}