From eda2a55adfdecbe6af0f9e8ad589f878b64c1912 Mon Sep 17 00:00:00 2001
From: Sujith H <sharidasan@owncloud.com>
Date: Thu, 19 Jul 2018 13:25:06 +0530
Subject: [PATCH] Correct the faulty behaviour of testPutWithModifyRun

The following corrections were made to testPutWithModifyRun:
a) Its updated to listen to event file.beforeCreate eventually
   updated the assert
b) Updated the method with acquire and release lock so that
   the put method is executed gracefully.
c) Remove the listener of the event once we have tested in
   testPutWithModifyRun
d) The event name to be listened in testChunkedPutFails is
   adjusted accordingly

Signed-off-by: Sujith H <sharidasan@owncloud.com>
---
 apps/dav/lib/Connector/Sabre/File.php         |  4 ++
 .../tests/unit/Connector/Sabre/FileTest.php   | 52 +++++--------------
 2 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/apps/dav/lib/Connector/Sabre/File.php b/apps/dav/lib/Connector/Sabre/File.php
index 486b2032cc2d..2a7398ac19cd 100644
--- a/apps/dav/lib/Connector/Sabre/File.php
+++ b/apps/dav/lib/Connector/Sabre/File.php
@@ -223,6 +223,10 @@ public function put($data) {
 			$view = \OC\Files\Filesystem::getView();
 			if ($view) {
 				$run = $this->emitPreHooks($exists);
+				if ($run === false) {
+					$view->unlockFile($this->path, ILockingProvider::LOCK_SHARED);
+					return null;
+				}
 			} else {
 				$run = true;
 			}
diff --git a/apps/dav/tests/unit/Connector/Sabre/FileTest.php b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
index a33d37d73e69..22c59313a080 100644
--- a/apps/dav/tests/unit/Connector/Sabre/FileTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/FileTest.php
@@ -293,14 +293,12 @@ function ($path) use ($storage) {
 
 	/**
 	 * Test the run value when put is called. And then try to modify the run
-	 * value in the listener. Once it is modified check the exception returned
-	 * from main function. The reason for exception is put from Sabre/File.php
-	 * throws exception
+	 * value in the listener.
 	 */
 	public function testPutWithModifyRun() {
 		$calledUploadAllowed = [];
-		\OC::$server->getEventDispatcher()->addListener('file.beforeUpdate', function (GenericEvent $event) use (&$calledUploadAllowed) {
-			$calledUploadAllowed[] = 'file.beforeUpdate';
+		\OC::$server->getEventDispatcher()->addListener('file.beforeCreate', function (GenericEvent $event) use (&$calledUploadAllowed) {
+			$calledUploadAllowed[] = 'file.beforeCreate';
 			//Now modify run
 			$event->setArgument('run', false);
 			$calledUploadAllowed[] = $event;
@@ -319,30 +317,17 @@ public function testPutWithModifyRun() {
 			->setConstructorArgs([])
 			->getMock();
 
-		$view->expects($this->atLeastOnce())
-			->method('resolvePath')
-			->will($this->returnCallback(
-				function ($path) use ($storage) {
-					return [$storage, $path];
-				}
-			));
-
-		$view->expects($this->any())
-			->method('getRelativePath')
-			->will($this->returnArgument(0));
-
-		$info = new FileInfo('/test.txt', $this->getMockStorage(), null, [
-			'permissions' => Constants::PERMISSION_ALL
-		], null);
-
-		$file = new File($view, $info);
-
-		$file->put($this->getStream('test data'));
-
+		$this->assertNull($this->doPut('/test1.txt'));
 		$this->assertInstanceOf(GenericEvent::class, $calledUploadAllowed[1]);
 		$this->assertArrayHasKey('run', $calledUploadAllowed[1]);
 		$this->assertFalse($calledUploadAllowed[1]->getArgument('run'));
-		$this->assertEquals('file.beforeUpdate', $calledUploadAllowed[0]);
+		$this->assertEquals('file.beforeCreate', $calledUploadAllowed[0]);
+
+		//Remove the listener for the event 'file.beforeCreate'
+		$eventListeners = \OC::$server->getEventDispatcher()->getListeners('file.beforeCreate');
+		foreach ($eventListeners as $eventListener) {
+			\OC::$server->getEventDispatcher()->removeListener('file.beforeCreate', $eventListener);
+		}
 	}
 
 	/**
@@ -406,7 +391,7 @@ function ($path) use ($storage) {
 				$calledCreateAllowed[] = 'file.beforeCreate';
 				$calledCreateAllowed[] = $event;
 			});
-			\OC::$server->getEventDispatcher()->addListener('file.beforeCreate', function (GenericEvent $event) use (&$calledWriteAllowed) {
+			\OC::$server->getEventDispatcher()->addListener('file.beforeWrite', function (GenericEvent $event) use (&$calledWriteAllowed) {
 				$calledWriteAllowed[] = 'file.beforeWrite';
 				$calledWriteAllowed[] = $event;
 			});
@@ -840,19 +825,8 @@ public function testPutSingleFileCancelPreHook() {
 		);
 
 		// action
-		$thrown = false;
-		try {
-			$this->doPut('/foo.txt');
-		} catch (Exception $e) {
-			$thrown = true;
-		}
+		$this->assertNull($this->doPut('/foo.txt'));
 
-		// objectstore does not use partfiles -> no move after upload -> no exception
-		if ($this->runsWithPrimaryObjectstorage()) {
-			$this->assertFalse($thrown);
-		} else {
-			$this->assertTrue($thrown);
-		}
 		$this->assertEmpty($this->listPartFiles(), 'No stray part files');
 	}