Merge pull request #12230 from nextcloud/bugfix/do_not_logerror_on_filelock

Do not log FileLock as exception

3 files changed, 13 insertions, 20 deletions

diff --git a/apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php b/apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php
index ecfd0e5692d..d134a0efaff 100644
--- a/apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php
+++ b/apps/dav/lib/Connector/Sabre/ExceptionLoggerPlugin.php
@@ -26,6 +26,7 @@
 
 namespace OCA\DAV\Connector\Sabre;
 
+use OCA\DAV\Connector\Sabre\Exception\FileLocked;
 use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
 use OCP\Files\StorageNotAvailableException;
 use OCP\ILogger;
@@ -69,6 +70,8 @@ class ExceptionLoggerPlugin extends \Sabre\DAV\ServerPlugin {
 		// happens when a certain method is not allowed to be called
 		// for example creating a folder that already exists
 		MethodNotAllowed::class => true,
+		// A locked file is perfectly valid and can happen in various cases
+		FileLocked::class => true,
 	];
 
 	/** @var string */
diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/DownloadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/DownloadTest.php
index 2cb08420f8d..127d48bd186 100644
--- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/DownloadTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/DownloadTest.php
@@ -46,9 +46,6 @@ class DownloadTest extends RequestTestCase {
 		$this->assertEquals(stream_get_contents($response->getBody()), 'bar');
 	}
 
-	/**
-	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\FileLocked
-	 */
 	public function testDownloadWriteLocked() {
 		$user = $this->getUniqueID();
 		$view = $this->setupUser($user, 'pass');
@@ -57,7 +54,8 @@ class DownloadTest extends RequestTestCase {
 
 		$view->lockFile('/foo.txt', ILockingProvider::LOCK_EXCLUSIVE);
 
-		$this->request($view, $user, 'pass', 'GET', '/foo.txt', 'asd');
+		$result = $this->request($view, $user, 'pass', 'GET', '/foo.txt', 'asd');
+		$this->assertEquals(Http::STATUS_LOCKED, $result->getStatus());
 	}
 
 	public function testDownloadReadLocked() {
diff --git a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php
index 5376241c61d..6ac3d66bad3 100644
--- a/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/RequestTest/UploadTest.php
@@ -68,9 +68,6 @@ class UploadTest extends RequestTestCase {
 		$this->assertEquals(3, $info->getSize());
 	}
 
-	/**
-	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\FileLocked
-	 */
 	public function testUploadOverWriteReadLocked() {
 		$user = $this->getUniqueID();
 		$view = $this->setupUser($user, 'pass');
@@ -79,12 +76,10 @@ class UploadTest extends RequestTestCase {
 
 		$view->lockFile('/foo.txt', ILockingProvider::LOCK_SHARED);
 
-		$this->request($view, $user, 'pass', 'PUT', '/foo.txt', 'asd');
+		$result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt', 'asd');
+		$this->assertEquals(Http::STATUS_LOCKED, $result->getStatus());
 	}
 
-	/**
-	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\FileLocked
-	 */
 	public function testUploadOverWriteWriteLocked() {
 		$user = $this->getUniqueID();
 		$view = $this->setupUser($user, 'pass');
@@ -94,7 +89,8 @@ class UploadTest extends RequestTestCase {
 
 		$view->lockFile('/foo.txt', ILockingProvider::LOCK_EXCLUSIVE);
 
-		$this->request($view, $user, 'pass', 'PUT', '/foo.txt', 'asd');
+		$result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt', 'asd');
+		$this->assertEquals(Http::STATUS_LOCKED, $result->getStatus());
 	}
 
 	public function testChunkedUpload() {
@@ -162,9 +158,6 @@ class UploadTest extends RequestTestCase {
 		$this->assertEquals(6, $info->getSize());
 	}
 
-	/**
-	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\FileLocked
-	 */
 	public function testChunkedUploadOutOfOrderReadLocked() {
 		$user = $this->getUniqueID();
 		$view = $this->setupUser($user, 'pass');
@@ -184,12 +177,10 @@ class UploadTest extends RequestTestCase {
 		$this->assertFalse($view->file_exists('foo.txt'));
 
 		// last chunk should trigger the locked error since it tries to assemble
-		$this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']);
+		$result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']);
+		$this->assertEquals(Http::STATUS_LOCKED, $result->getStatus());
 	}
 
-	/**
-	 * @expectedException \OCA\DAV\Connector\Sabre\Exception\FileLocked
-	 */
 	public function testChunkedUploadOutOfOrderWriteLocked() {
 		$user = $this->getUniqueID();
 		$view = $this->setupUser($user, 'pass');
@@ -209,6 +200,7 @@ class UploadTest extends RequestTestCase {
 		$this->assertFalse($view->file_exists('foo.txt'));
 
 		// last chunk should trigger the locked error since it tries to assemble
-		$this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']);
+		$result = $this->request($view, $user, 'pass', 'PUT', '/foo.txt-chunking-123-2-0', 'asd', ['OC-Chunked' => '1']);
+		$this->assertEquals(Http::STATUS_LOCKED, $result->getStatus());
 	}
 }