Move pre_shared and post_shared events to EventDispatcher

* Now listeners for those events get proper share objects. * Legacy hooks still fired * Updated tests Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
2017-08-03 15:14:29 +02:00 · 2017-08-03 15:14:29 +02:00 · 4633717f04
commit 4633717f04
parent 8fecc336ac
4 changed files with 298 additions and 120 deletions
--- a/lib/private/Share20/LegacyHooks.php
+++ b/lib/private/Share20/LegacyHooks.php
@ -22,6 +22,7 @@
 */
 namespace OC\Share20;

+use OCP\Files\File;
 use OCP\Share\IShare;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\GenericEvent;
@ -41,6 +42,8 @@ class LegacyHooks {
 		$this->eventDispatcher->addListener('OCP\Share::preUnshare', [$this, 'preUnshare']);
 		$this->eventDispatcher->addListener('OCP\Share::postUnshare', [$this, 'postUnshare']);
 		$this->eventDispatcher->addListener('OCP\Share::postUnshareFromSelf', [$this, 'postUnshareFromSelf']);
+		$this->eventDispatcher->addListener('OCP\Share::preShare', [$this, 'preShare']);
+		$this->eventDispatcher->addListener('OCP\Share::postShare', [$this, 'postShare']);
 	}

 	/**
@ -112,4 +115,58 @@ class LegacyHooks {
 		];
 		return $hookParams;
 	}
+
+	public function preShare(GenericEvent $e) {
+		/** @var IShare $share */
+		$share = $e->getSubject();
+
+		// Pre share hook
+		$run = true;
+		$error = '';
+		$preHookData = [
+			'itemType' => $share->getNode() instanceof File ? 'file' : 'folder',
+			'itemSource' => $share->getNode()->getId(),
+			'shareType' => $share->getShareType(),
+			'uidOwner' => $share->getSharedBy(),
+			'permissions' => $share->getPermissions(),
+			'fileSource' => $share->getNode()->getId(),
+			'expiration' => $share->getExpirationDate(),
+			'token' => $share->getToken(),
+			'itemTarget' => $share->getTarget(),
+			'shareWith' => $share->getSharedWith(),
+			'run' => &$run,
+			'error' => &$error,
+		];
+		\OC_Hook::emit('OCP\Share', 'pre_shared', $preHookData);
+
+		if ($run === false) {
+			$e->setArgument('error', $error);
+			$e->stopPropagation();
+		}
+
+		return $e;
+	}
+
+	public function postShare(GenericEvent $e) {
+		/** @var IShare $share */
+		$share = $e->getSubject();
+
+		$postHookData = [
+			'itemType' => $share->getNode() instanceof File ? 'file' : 'folder',
+			'itemSource' => $share->getNode()->getId(),
+			'shareType' => $share->getShareType(),
+			'uidOwner' => $share->getSharedBy(),
+			'permissions' => $share->getPermissions(),
+			'fileSource' => $share->getNode()->getId(),
+			'expiration' => $share->getExpirationDate(),
+			'token' => $share->getToken(),
+			'id' => $share->getId(),
+			'shareWith' => $share->getSharedWith(),
+			'itemTarget' => $share->getTarget(),
+			'fileTarget' => $share->getTarget(),
+		];
+
+		\OC_Hook::emit('OCP\Share', 'post_shared', $postHookData);
+
+	}
 }
--- a/lib/private/Share20/Manager.php
+++ b/lib/private/Share20/Manager.php
@ -636,27 +636,11 @@ class Manager implements IManager {
 		$target = \OC\Files\Filesystem::normalizePath($target);
 		$share->setTarget($target);

-		// Pre share hook
-		$run = true;
-		$error = '';
-		$preHookData = [
-			'itemType' => $share->getNode() instanceof \OCP\Files\File ? 'file' : 'folder',
-			'itemSource' => $share->getNode()->getId(),
-			'shareType' => $share->getShareType(),
-			'uidOwner' => $share->getSharedBy(),
-			'permissions' => $share->getPermissions(),
-			'fileSource' => $share->getNode()->getId(),
-			'expiration' => $share->getExpirationDate(),
-			'token' => $share->getToken(),
-			'itemTarget' => $share->getTarget(),
-			'shareWith' => $share->getSharedWith(),
-			'run' => &$run,
-			'error' => &$error,
-		];
-		\OC_Hook::emit('OCP\Share', 'pre_shared', $preHookData);
-
-		if ($run === false) {
-			throw new \Exception($error);
+		// Pre share event
+		$event = new GenericEvent($share);
+		$a = $this->eventDispatcher->dispatch('OCP\Share::preShare', $event);
+		if ($event->isPropagationStopped() && $event->hasArgument('error')) {
+			throw new \Exception($event->getArgument('error'));
 		}

 		$oldShare = $share;
@ -665,23 +649,9 @@ class Manager implements IManager {
 		//reuse the node we already have
 		$share->setNode($oldShare->getNode());

-		// Post share hook
-		$postHookData = [
-			'itemType' => $share->getNode() instanceof \OCP\Files\File ? 'file' : 'folder',
-			'itemSource' => $share->getNode()->getId(),
-			'shareType' => $share->getShareType(),
-			'uidOwner' => $share->getSharedBy(),
-			'permissions' => $share->getPermissions(),
-			'fileSource' => $share->getNode()->getId(),
-			'expiration' => $share->getExpirationDate(),
-			'token' => $share->getToken(),
-			'id' => $share->getId(),
-			'shareWith' => $share->getSharedWith(),
-			'itemTarget' => $share->getTarget(),
-			'fileTarget' => $share->getTarget(),
-		];
-
-		\OC_Hook::emit('OCP\Share', 'post_shared', $postHookData);
+		// Post share event
+		$event = new GenericEvent($share);
+		$this->eventDispatcher->dispatch('OCP\Share::postShare', $event);

 		if ($share->getShareType() === \OCP\Share::SHARE_TYPE_USER) {
 			$user = $this->userManager->get($share->getSharedWith());
--- a/tests/lib/Share20/LegacyHooksTest.php
+++ b/tests/lib/Share20/LegacyHooksTest.php
@ -24,6 +24,7 @@ namespace Test\Share20;

 use OC\Share20\LegacyHooks;
 use OC\Share20\Manager;
+use OCP\Constants;
 use OCP\Files\File;
 use Symfony\Component\EventDispatcher\EventDispatcher;
 use Symfony\Component\EventDispatcher\GenericEvent;
@ -187,4 +188,153 @@ class LegacyHooksTest extends TestCase {
 		$event = new GenericEvent($share);
 		$this->eventDispatcher->dispatch('OCP\Share::postUnshareFromSelf', $event);
 	}
+
+	public function testPreShare() {
+		$path = $this->createMock(File::class);
+		$path->method('getId')->willReturn(1);
+
+		$date = new \DateTime();
+
+		$share = $this->manager->newShare();
+		$share->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setSharedWith('awesomeUser')
+			->setSharedBy('sharedBy')
+			->setNode($path)
+			->setTarget('myTarget')
+			->setPermissions(Constants::PERMISSION_ALL)
+			->setExpirationDate($date)
+			->setPassword('password')
+			->setToken('token');
+
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['preShare'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'preShare');
+
+		$run = true;
+		$error = '';
+
+		$expected = [
+			'itemType' => 'file',
+			'itemSource' => 1,
+			'shareType' => \OCP\Share::SHARE_TYPE_LINK,
+			'shareWith' => 'awesomeUser',
+			'uidOwner' => 'sharedBy',
+			'fileSource' => 1,
+			'itemTarget' => 'myTarget',
+			'permissions' => Constants::PERMISSION_ALL,
+			'expiration' => $date,
+			'token' => 'token',
+			'run' => &$run,
+			'error' => &$error,
+		];
+
+		$hookListner
+			->expects($this->exactly(1))
+			->method('preShare')
+			->with($expected);
+
+		$event = new GenericEvent($share);
+		$this->eventDispatcher->dispatch('OCP\Share::preShare', $event);
+	}
+
+	public function testPreShareError() {
+		$path = $this->createMock(File::class);
+		$path->method('getId')->willReturn(1);
+
+		$date = new \DateTime();
+
+		$share = $this->manager->newShare();
+		$share->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setSharedWith('awesomeUser')
+			->setSharedBy('sharedBy')
+			->setNode($path)
+			->setTarget('myTarget')
+			->setPermissions(Constants::PERMISSION_ALL)
+			->setExpirationDate($date)
+			->setPassword('password')
+			->setToken('token');
+
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['preShare'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'preShare');
+
+		$run = true;
+		$error = '';
+
+		$expected = [
+			'itemType' => 'file',
+			'itemSource' => 1,
+			'shareType' => \OCP\Share::SHARE_TYPE_LINK,
+			'shareWith' => 'awesomeUser',
+			'uidOwner' => 'sharedBy',
+			'fileSource' => 1,
+			'itemTarget' => 'myTarget',
+			'permissions' => Constants::PERMISSION_ALL,
+			'expiration' => $date,
+			'token' => 'token',
+			'run' => &$run,
+			'error' => &$error,
+		];
+
+		$hookListner
+			->expects($this->exactly(1))
+			->method('preShare')
+			->with($expected)
+			->will($this->returnCallback(function ($data) {
+				$data['run'] = false;
+				$data['error'] = 'I error';
+			}));
+
+		$event = new GenericEvent($share);
+		$this->eventDispatcher->dispatch('OCP\Share::preShare', $event);
+
+		$this->assertTrue($event->isPropagationStopped());
+		$this->assertSame('I error', $event->getArgument('error'));
+	}
+
+	public function testPostShare() {
+		$path = $this->createMock(File::class);
+		$path->method('getId')->willReturn(1);
+
+		$date = new \DateTime();
+
+		$share = $this->manager->newShare();
+		$share->setId(42)
+			->setShareType(\OCP\Share::SHARE_TYPE_LINK)
+			->setSharedWith('awesomeUser')
+			->setSharedBy('sharedBy')
+			->setNode($path)
+			->setTarget('myTarget')
+			->setPermissions(Constants::PERMISSION_ALL)
+			->setExpirationDate($date)
+			->setPassword('password')
+			->setToken('token');
+
+
+		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['postShare'])->getMock();
+		\OCP\Util::connectHook('OCP\Share', 'post_shared', $hookListner, 'postShare');
+
+		$expected = [
+			'id' => 42,
+			'itemType' => 'file',
+			'itemSource' => 1,
+			'shareType' => \OCP\Share::SHARE_TYPE_LINK,
+			'shareWith' => 'awesomeUser',
+			'uidOwner' => 'sharedBy',
+			'fileSource' => 1,
+			'itemTarget' => 'myTarget',
+			'fileTarget' => 'myTarget',
+			'permissions' => Constants::PERMISSION_ALL,
+			'expiration' => $date,
+			'token' => 'token',
+		];
+
+		$hookListner
+			->expects($this->exactly(1))
+			->method('postShare')
+			->with($expected);
+
+		$event = new GenericEvent($share);
+		$this->eventDispatcher->dispatch('OCP\Share::postShare', $event);
+	}
 }
--- a/tests/lib/Share20/ManagerTest.php
+++ b/tests/lib/Share20/ManagerTest.php
@ -1705,46 +1705,45 @@ class ManagerTest extends \Test\TestCase {
 				return $share->setId(42);
 			}));

-		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre', 'post'])->getMock();
-		\OCP\Util::connectHook('OCP\Share', 'pre_shared',  $hookListner, 'pre');
-		\OCP\Util::connectHook('OCP\Share', 'post_shared', $hookListner, 'post');
+		// Pre share
+		$this->eventDispatcher->expects($this->at(0))
+			->method('dispatch')
+			->with(
+				$this->equalTo('OCP\Share::preShare'),
+				$this->callback(function(GenericEvent $e) use ($path, $date) {
+					/** @var IShare $share */
+					$share = $e->getSubject();

-		$hookListnerExpectsPre = [
-			'itemType' => 'file',
-			'itemSource' => 1,
-			'shareType' => \OCP\Share::SHARE_TYPE_LINK,
-			'uidOwner' => 'sharedBy',
-			'permissions' => 31,
-			'fileSource' => 1,
-			'expiration' => $date,
-			'token' => 'token',
-			'run' => true,
-			'error' => '',
-			'itemTarget' => '/target',
-			'shareWith' => null,
-		];
+					return $share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+						$share->getNode() === $path &&
+						$share->getSharedBy() === 'sharedBy' &&
+						$share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+						$share->getExpirationDate() === $date &&
+						$share->getPassword() === 'hashed' &&
+						$share->getToken() === 'token';
+				})
+			);

-		$hookListnerExpectsPost = [
-			'itemType' => 'file',
-			'itemSource' => 1,
-			'shareType' => \OCP\Share::SHARE_TYPE_LINK,
-			'uidOwner' => 'sharedBy',
-			'permissions' => 31,
-			'fileSource' => 1,
-			'expiration' => $date,
-			'token' => 'token',
-			'id' => 42,
-			'itemTarget' => '/target',
-			'fileTarget' => '/target',
-			'shareWith' => null,
-		];
+		// Post share
+		$this->eventDispatcher->expects($this->at(1))
+			->method('dispatch')
+			->with(
+				$this->equalTo('OCP\Share::postShare'),
+				$this->callback(function(GenericEvent $e) use ($path, $date) {
+					/** @var IShare $share */
+					$share = $e->getSubject();

-		$hookListner->expects($this->once())
-			->method('pre')
-			->with($this->equalTo($hookListnerExpectsPre));
-		$hookListner->expects($this->once())
-			->method('post')
-			->with($this->equalTo($hookListnerExpectsPost));
+					return $share->getShareType() === \OCP\Share::SHARE_TYPE_LINK &&
+						$share->getNode() === $path &&
+						$share->getSharedBy() === 'sharedBy' &&
+						$share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+						$share->getExpirationDate() === $date &&
+						$share->getPassword() === 'hashed' &&
+						$share->getToken() === 'token' &&
+						$share->getId() === '42' &&
+						$share->getTarget() === '/target';
+				})
+			);

 		/** @var IShare $share */
 		$share = $manager->createShare($share);
@ -1817,46 +1816,45 @@ class ManagerTest extends \Test\TestCase {
 				return $share->setId(42);
 			}));

-		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre', 'post'])->getMock();
-		\OCP\Util::connectHook('OCP\Share', 'pre_shared',  $hookListner, 'pre');
-		\OCP\Util::connectHook('OCP\Share', 'post_shared', $hookListner, 'post');
+		// Pre share
+		$this->eventDispatcher->expects($this->at(0))
+			->method('dispatch')
+			->with(
+				$this->equalTo('OCP\Share::preShare'),
+				$this->callback(function(GenericEvent $e) use ($path) {
+					/** @var IShare $share */
+					$share = $e->getSubject();

-		$hookListnerExpectsPre = [
-			'itemType' => 'file',
-			'itemSource' => 1,
-			'shareType' => \OCP\Share::SHARE_TYPE_EMAIL,
-			'uidOwner' => 'sharedBy',
-			'permissions' => 31,
-			'fileSource' => 1,
-			'expiration' => null,
-			'token' => 'token',
-			'run' => true,
-			'error' => '',
-			'itemTarget' => '/target',
-			'shareWith' => null,
-		];
+					return $share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL &&
+						$share->getNode() === $path &&
+						$share->getSharedBy() === 'sharedBy' &&
+						$share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+						$share->getExpirationDate() === null &&
+						$share->getPassword() === null &&
+						$share->getToken() === 'token';
+				})
+			);

-		$hookListnerExpectsPost = [
-			'itemType' => 'file',
-			'itemSource' => 1,
-			'shareType' => \OCP\Share::SHARE_TYPE_EMAIL,
-			'uidOwner' => 'sharedBy',
-			'permissions' => 31,
-			'fileSource' => 1,
-			'expiration' => null,
-			'token' => 'token',
-			'id' => 42,
-			'itemTarget' => '/target',
-			'fileTarget' => '/target',
-			'shareWith' => null,
-		];
+		// Post share
+		$this->eventDispatcher->expects($this->at(1))
+			->method('dispatch')
+			->with(
+				$this->equalTo('OCP\Share::postShare'),
+				$this->callback(function(GenericEvent $e) use ($path) {
+					/** @var IShare $share */
+					$share = $e->getSubject();

-		$hookListner->expects($this->once())
-			->method('pre')
-			->with($this->equalTo($hookListnerExpectsPre));
-		$hookListner->expects($this->once())
-			->method('post')
-			->with($this->equalTo($hookListnerExpectsPost));
+					return $share->getShareType() === \OCP\Share::SHARE_TYPE_EMAIL &&
+						$share->getNode() === $path &&
+						$share->getSharedBy() === 'sharedBy' &&
+						$share->getPermissions() === \OCP\Constants::PERMISSION_ALL &&
+						$share->getExpirationDate() === null &&
+						$share->getPassword() === null &&
+						$share->getToken() === 'token' &&
+						$share->getId() === '42' &&
+						$share->getTarget() === '/target';
+				})
+			);

 		/** @var IShare $share */
 		$share = $manager->createShare($share);
@ -1919,14 +1917,17 @@ class ManagerTest extends \Test\TestCase {
 			->method('setTarget')
 			->with('/target');

-		$hookListner = $this->getMockBuilder('Dummy')->setMethods(['pre'])->getMock();
-		\OCP\Util::connectHook('OCP\Share', 'pre_shared', $hookListner, 'pre');
-		$hookListner->expects($this->once())
-			->method('pre')
-			->will($this->returnCallback(function (array $data) {
-				$data['run'] = false;
-				$data['error'] = 'I won\'t let you share!';
-			}));
+		// Pre share
+		$this->eventDispatcher->expects($this->once())
+			->method('dispatch')
+			->with(
+				$this->equalTo('OCP\Share::preShare'),
+				$this->isInstanceOf(GenericEvent::class)
+			)->will($this->returnCallback(function($name, GenericEvent $e) {
+					$e->setArgument('error', 'I won\'t let you share!');
+					$e->stopPropagation();
+				})
+			);

 		$manager->createShare($share);
 	}