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>
owncloud · Jul 24, 2018 · eda2a55 · eda2a55
1 parent 1d4e24d
commit eda2a55
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 39 deletions.
diff --git 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
@@ -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');
 	}