Merge pull request #30930 from owncloud/add-symfony-events-for-delete…

…share Add symfony events for delete and create share
owncloud · Apr 6, 2018 · bedfb50 · bedfb50
2 parents 953a706 + 294bc9e
commit bedfb50
Show file tree

Hide file tree

Showing 6 changed files with 180 additions and 93 deletions.
diff --git a/apps/files_sharing/lib/API/Share20OCS.php b/apps/files_sharing/lib/API/Share20OCS.php
@@ -469,13 +469,6 @@ public function createShare() {
 		$share->setShareType($shareType);
 		$share->setSharedBy($this->currentUser->getUID());
 
-
-
-
-		$beforeEvent->setArgument('share', $this->formatShare($share));
-
-		$this->eventDispatcher->dispatch('share.beforeCreate', $beforeEvent);
-
 		try {
 			$share = $this->shareManager->createShare($share);
 		} catch (GenericShareException $e) {
@@ -492,10 +485,6 @@ public function createShare() {
 		$share->getNode()->unlock(\OCP\Lock\ILockingProvider::LOCK_SHARED);
 
 		$formattedShareAfterCreate = $this->formatShare($share);
-		$afterEvent = new GenericEvent(null, []);
-		$afterEvent->setArgument('share', $formattedShareAfterCreate);
-		$afterEvent->setArgument('result', 'success');
-		$this->eventDispatcher->dispatch('share.afterCreate', $afterEvent);
 		return new \OC\OCS\Result($formattedShareAfterCreate);
 	}
 

diff --git a/apps/files_sharing/tests/API/Share20OCSTest.php b/apps/files_sharing/tests/API/Share20OCSTest.php
@@ -24,7 +24,6 @@
  */
 namespace OCA\Files_Sharing\Tests\API;
 
-use JMS\Serializer\EventDispatcher\EventDispatcher;
 use OCA\Files_Sharing\API\Share20OCS;
 use OCP\Files\IRootFolder;
 use OCP\Files\NotFoundException;
@@ -38,6 +37,7 @@
 use OCP\Lock\LockedException;
 use OCP\Share;
 use Symfony\Component\EventDispatcher\GenericEvent;
+use Symfony\Component\EventDispatcher\EventDispatcher;
 use Test\TestCase;
 
 /**
@@ -108,7 +108,7 @@ protected function setUp() {
 				['core', 'shareapi_default_permissions', \OCP\Constants::PERMISSION_ALL, \OCP\Constants::PERMISSION_READ | \OCP\Constants::PERMISSION_CREATE]
 			]));
 
-		$this->eventDispatcher = \OC::$server->getEventDispatcher();
+		$this->eventDispatcher = new EventDispatcher();
 
 		$this->ocs = new Share20OCS(
 			$this->shareManager,
@@ -831,20 +831,11 @@ public function testCreateShareUser() {
 			}))
 			->will($this->returnArgument(0));
 
-		$calledAfterCreate = [];
-		\OC::$server->getEventDispatcher()->addListener('share.afterCreate', function (GenericEvent $event) use (&$calledAfterCreate) {
-			$calledAfterCreate[] = 'share.afterCreate';
-			$calledAfterCreate[] = $event;
-		});
 		$expected = new \OC\OCS\Result();
 		$result = $ocs->createShare();
 
 		$this->assertEquals($expected->getMeta(), $result->getMeta());
 		$this->assertEquals($expected->getData(), $result->getData());
-		$this->assertEquals('share.afterCreate', $calledAfterCreate[0]);
-		$this->assertInstanceOf(GenericEvent::class, $calledAfterCreate[1]);
-		$this->assertEquals('success', $calledAfterCreate[1]->getArgument('result'));
-		$this->assertArrayHasKey('share', $calledAfterCreate[1]);
 	}
 
 	public function testCreateShareGroupNoValidShareWith() {
@@ -2851,68 +2842,5 @@ public function testGetShareAdditionalInfo($configValue, $expectedInfo) {
 
 		$this->assertEquals($expectedInfo, $result['share_with_additional_info']);
 	}
-
-	public function testBeforeAndAfterCreateHooksAreCalled() {
-		$ocs = $this->mockFormatShare();
-		$share = $this->newShare();
-		$this->shareManager->method('newShare')->willReturn($share);
-
-		$storage = $this->createMock('OCP\Files\Storage');
-		$storage->method('instanceOfStorage')
-			->with('OCA\Files_Sharing\External\Storage')
-			->willReturn(false);
-		$path = $this->createMock('\OCP\Files\File');
-		$path->method('getStorage')
-			->willReturn($storage);
-
-		$userFolder = $this->createMock('\OCP\Files\Folder');
-		$node = $this->createMock('\OCP\Files\Node');
-		$node->method('getStorage')
-			->willReturn($storage);
-		$userFolder->method('get')->willReturn($node);
-		$this->request
-			->method('getParam')
-			->will($this->returnValueMap([
-				['path', null, 'valid-path'],
-				['permissions', null, \OCP\Constants::PERMISSION_ALL],
-				['shareType', $this->any(), Share::SHARE_TYPE_USER],
-				['shareWith', null, 'validUser'],
-			]));
-
-		$this->rootFolder
-			->method('getUserFolder')
-			->with('currentUser')
-			->willReturn($userFolder);
-		$this->userManager->method('userExists')
-			->withAnyParameters()
-			->willReturn(true);
-		$this->shareManager
-			->method('createShare')
-			->willReturn($share);
-
-		$shareBeforeCreateCalled = false;
-
-		\OC::$server->getEventDispatcher()->addListener('share.beforeCreate', function (GenericEvent $event) use (&$shareBeforeCreateCalled) {
-			$shareBeforeCreateCalled = true;
-			$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
-			$args = $event->getArguments();
-			$this->assertArrayHasKey('share', $args);
-			$this->assertArrayHasKey('run', $args);
-		});
-
-		$shareAfterCreateCalled = false;
-
-		\OC::$server->getEventDispatcher()->addListener('share.afterCreate', function (GenericEvent $event) use (&$shareAfterCreateCalled) {
-			$shareAfterCreateCalled = true;
-			$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
-			$args = $event->getArguments();
-			$this->assertArrayHasKey('share', $args);
-		});
-
-		$ocs->createShare();
-
-		$this->assertTrue($shareBeforeCreateCalled, 'share.beforeCreate not called');
-		$this->assertTrue($shareAfterCreateCalled, 'share.afterCreate not called');
-	}
 }
 
diff --git a/apps/files_sharing/tests/ApiTest.php b/apps/files_sharing/tests/ApiTest.php
@@ -150,7 +150,7 @@ function testCreateShareUserFile() {
 
 		$this->assertEquals('share.beforeCreate', $calledBeforeCreate[0]);
 		$this->assertInstanceOf(GenericEvent::class, $calledBeforeCreate[1]);
-		$this->assertTrue($calledBeforeCreate[1]->getArgument('run'));
+		$this->assertTrue($calledBeforeCreate[1]->getArgument('shareData')['run']);
 		$this->assertTrue($result->succeeded());
 		$data = $result->getData();
 		$this->assertEquals(19, $data['permissions']);

diff --git a/lib/private/Server.php b/lib/private/Server.php
@@ -865,7 +865,8 @@ public function __construct($webRoot, \OC\Config $config) {
 				$c->getL10N('core'),
 				$factory,
 				$c->getUserManager(),
-				$c->getLazyRootFolder()
+				$c->getLazyRootFolder(),
+				$c->getEventDispatcher()
 			);
 
 			return $manager;

diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php
@@ -44,6 +44,7 @@
 use OCP\Share\Exceptions\ShareNotFound;
 use OCP\Share\IManager;
 use OCP\Share\IProviderFactory;
+use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 /**
@@ -73,6 +74,8 @@ class Manager implements IManager {
 	private $rootFolder;
 	/** @var CappedMemoryCache */
 	private $sharingDisabledForUsersCache;
+	/** @var EventDispatcher  */
+	private $eventDispatcher;
 
 
 	/**
@@ -99,7 +102,8 @@ public function __construct(
 			IL10N $l,
 			IProviderFactory $factory,
 			IUserManager $userManager,
-			IRootFolder $rootFolder
+			IRootFolder $rootFolder,
+			EventDispatcher $eventDispatcher
 	) {
 		$this->logger = $logger;
 		$this->config = $config;
@@ -112,6 +116,7 @@ public function __construct(
 		$this->userManager = $userManager;
 		$this->rootFolder = $rootFolder;
 		$this->sharingDisabledForUsersCache = new CappedMemoryCache();
+		$this->eventDispatcher = $eventDispatcher;
 	}
 
 	/**
@@ -635,6 +640,9 @@ public function createShare(\OCP\Share\IShare $share) {
 		];
 		\OC_Hook::emit('OCP\Share', 'pre_shared', $preHookData);
 
+		$beforeEvent = new GenericEvent(null, ['shareData' => $preHookData]);
+		$this->eventDispatcher->dispatch('share.beforeCreate', $beforeEvent);
+
 		if ($run === false) {
 			throw new \Exception($error);
 		}
@@ -661,6 +669,9 @@ public function createShare(\OCP\Share\IShare $share) {
 
 		\OC_Hook::emit('OCP\Share', 'post_shared', $postHookData);
 
+		$afterEvent = new GenericEvent(null, ['shareData' => $postHookData, 'shareObject' => $share, 'result' => 'success']);
+		$this->eventDispatcher->dispatch('share.afterCreate', $afterEvent);
+
 		return $share;
 	}
 
@@ -846,6 +857,8 @@ public function deleteShare(\OCP\Share\IShare $share) {
 		// Emit pre-hook
 		\OC_Hook::emit('OCP\Share', 'pre_unshare', $hookParams);
 
+		$beforeEvent = new GenericEvent(null, ['share' => $hookParams, 'shareObject' => $share]);
+		$this->eventDispatcher->dispatch('share.beforeDelete', $beforeEvent);
 		// Get all children and delete them as well
 		$deletedShares = $this->deleteChildren($share);
 
@@ -863,6 +876,8 @@ public function deleteShare(\OCP\Share\IShare $share) {
 
 		// Emit post hook
 		\OC_Hook::emit('OCP\Share', 'post_unshare', $hookParams);
+		$afterEvent = new GenericEvent(null, ['share' => $hookParams['deletedShares']]);
+		$this->eventDispatcher->dispatch('share.afterDelete', $afterEvent);
 	}