From b8958ee9379be1876ed982fdef88be6934ba39da Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 10 Nov 2016 15:33:00 +0100 Subject: [PATCH] Fix activity manager tests Signed-off-by: Joas Schilling --- lib/private/Activity/Manager.php | 30 ++++++++----- lib/private/Server.php | 7 ++- lib/public/Activity/IManager.php | 2 - tests/lib/Activity/ManagerTest.php | 70 ++++++++++++++---------------- 4 files changed, 56 insertions(+), 53 deletions(-) diff --git a/lib/private/Activity/Manager.php b/lib/private/Activity/Manager.php index 04a8dd96a45..c4f3d348dbe 100644 --- a/lib/private/Activity/Manager.php +++ b/lib/private/Activity/Manager.php @@ -24,7 +24,6 @@ namespace OC\Activity; -use OC\RichObjectStrings\Validator; use OCP\Activity\IConsumer; use OCP\Activity\IEvent; use OCP\Activity\IExtension; @@ -36,6 +35,7 @@ use OCP\IConfig; use OCP\IRequest; use OCP\IUser; use OCP\IUserSession; +use OCP\RichObjectStrings\IValidator; class Manager implements IManager { /** @var IRequest */ @@ -47,6 +47,9 @@ class Manager implements IManager { /** @var IConfig */ protected $config; + /** @var IValidator */ + protected $validator; + /** @var string */ protected $formattingObjectType; @@ -62,13 +65,16 @@ class Manager implements IManager { * @param IRequest $request * @param IUserSession $session * @param IConfig $config + * @param IValidator $validator */ public function __construct(IRequest $request, IUserSession $session, - IConfig $config) { + IConfig $config, + IValidator $validator) { $this->request = $request; $this->session = $session; $this->config = $config; + $this->validator = $validator; } /** @var \Closure[] */ @@ -151,9 +157,7 @@ class Manager implements IManager { * @return IEvent */ public function generateEvent() { - return new Event( - new Validator() - ); + return new Event($this->validator); } /** @@ -166,12 +170,18 @@ class Manager implements IManager { * - setSubject() * * @param IEvent $event - * @return null * @throws \BadMethodCallException if required values have not been set */ public function publish(IEvent $event) { + $this->publishToConsumers($event, false); + } - if ($event->getAuthor() === null) { + /** + * @param IEvent $event + * @param bool $legacyActivity + */ + protected function publishToConsumers(IEvent $event, $legacyActivity) { + if ($event->getAuthor() === '') { if ($this->session->getUser() instanceof IUser) { $event->setAuthor($this->session->getUser()->getUID()); } @@ -181,7 +191,7 @@ class Manager implements IManager { $event->setTimestamp(time()); } - if (!$event->isValid()) { + if (!$legacyActivity && !$event->isValid()) { throw new \BadMethodCallException('The given event is invalid'); } @@ -201,7 +211,6 @@ class Manager implements IManager { * @param string $affectedUser Recipient of the activity * @param string $type Type of the notification * @param int $priority Priority of the notification - * @return null */ public function publishActivity($app, $subject, $subjectParams, $message, $messageParams, $file, $link, $affectedUser, $type, $priority) { $event = $this->generateEvent(); @@ -213,7 +222,7 @@ class Manager implements IManager { ->setObject('', 0, $file) ->setLink($link); - $this->publish($event); + $this->publishToConsumers($event, true); } /** @@ -236,7 +245,6 @@ class Manager implements IManager { * $callable has to return an instance of OCA\Activity\IExtension * * @param \Closure $callable - * @return void */ public function registerExtension(\Closure $callable) { array_push($this->extensionsClosures, $callable); diff --git a/lib/private/Server.php b/lib/private/Server.php index 8e41ba212ad..abedf8230ed 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -90,6 +90,7 @@ use OC\Tagging\TagMapper; use OCA\Theming\ThemingDefaults; use OCP\IL10N; use OCP\IServerContainer; +use OCP\RichObjectStrings\IValidator; use OCP\Security\IContentSecurityPolicyManager; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -394,9 +395,11 @@ class Server extends ServerContainer implements IServerContainer { return new \OC\Activity\Manager( $c->getRequest(), $c->getUserSession(), - $c->getConfig() + $c->getConfig(), + $c->query(IValidator::class) ); }); + $this->registerAlias(IValidator::class, Validator::class); $this->registerService('AvatarManager', function (Server $c) { return new AvatarManager( $c->getUserManager(), @@ -662,7 +665,7 @@ class Server extends ServerContainer implements IServerContainer { }); $this->registerService('NotificationManager', function (Server $c) { return new Manager( - $c->query(Validator::class) + $c->query(IValidator::class) ); }); $this->registerService('CapabilitiesManager', function (Server $c) { diff --git a/lib/public/Activity/IManager.php b/lib/public/Activity/IManager.php index 48071729ed1..abad12f15f0 100644 --- a/lib/public/Activity/IManager.php +++ b/lib/public/Activity/IManager.php @@ -64,7 +64,6 @@ interface IManager { * - setSubject() * * @param IEvent $event - * @return null * @since 8.2.0 */ public function publish(IEvent $event); @@ -80,7 +79,6 @@ interface IManager { * @param string $affectedUser Recipient of the activity * @param string $type Type of the notification * @param int $priority Priority of the notification - * @return null * @since 6.0.0 * @deprecated 8.2.0 Grab an IEvent from generateEvent() instead and use the publish() method */ diff --git a/tests/lib/Activity/ManagerTest.php b/tests/lib/Activity/ManagerTest.php index cf855dd2813..13932f389f8 100644 --- a/tests/lib/Activity/ManagerTest.php +++ b/tests/lib/Activity/ManagerTest.php @@ -10,6 +10,10 @@ namespace Test\Activity; +use OCP\IConfig; +use OCP\IRequest; +use OCP\IUserSession; +use OCP\RichObjectStrings\IValidator; use Test\TestCase; class ManagerTest extends TestCase { @@ -17,32 +21,28 @@ class ManagerTest extends TestCase { /** @var \OC\Activity\Manager */ private $activityManager; - /** @var \OCP\IRequest|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ protected $request; - - /** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IUserSession|\PHPUnit_Framework_MockObject_MockObject */ protected $session; - - /** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */ + /** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */ protected $config; + /** @var IValidator|\PHPUnit_Framework_MockObject_MockObject */ + protected $validator; protected function setUp() { parent::setUp(); - $this->request = $this->getMockBuilder('OCP\IRequest') - ->disableOriginalConstructor() - ->getMock(); - $this->session = $this->getMockBuilder('OCP\IUserSession') - ->disableOriginalConstructor() - ->getMock(); - $this->config = $this->getMockBuilder('OCP\IConfig') - ->disableOriginalConstructor() - ->getMock(); + $this->request = $this->createMock(IRequest::class); + $this->session = $this->createMock(IUserSession::class); + $this->config = $this->createMock(IConfig::class); + $this->validator = $this->createMock(IValidator::class); $this->activityManager = new \OC\Activity\Manager( $this->request, $this->session, - $this->config + $this->config, + $this->validator ); $this->assertSame([], $this->invokePrivate($this->activityManager, 'getConsumers')); @@ -264,32 +264,26 @@ class ManagerTest extends TestCase { /** * @expectedException \BadMethodCallException - * @expectedExceptionMessage App not set - * @expectedExceptionCode 10 */ public function testPublishExceptionNoApp() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $this->activityManager->publish($event); } /** * @expectedException \BadMethodCallException - * @expectedExceptionMessage Type not set - * @expectedExceptionCode 11 */ public function testPublishExceptionNoType() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test'); $this->activityManager->publish($event); } /** * @expectedException \BadMethodCallException - * @expectedExceptionMessage Affected user not set - * @expectedExceptionCode 12 */ public function testPublishExceptionNoAffectedUser() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test') ->setType('test_type'); $this->activityManager->publish($event); @@ -297,11 +291,9 @@ class ManagerTest extends TestCase { /** * @expectedException \BadMethodCallException - * @expectedExceptionMessage Subject not set - * @expectedExceptionCode 13 */ public function testPublishExceptionNoSubject() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test') ->setType('test_type') ->setAffectedUser('test_affected'); @@ -310,16 +302,17 @@ class ManagerTest extends TestCase { public function dataPublish() { return [ - [null], - ['test_author'], + [null, ''], + ['test_author', 'test_author'], ]; } /** * @dataProvider dataPublish - * @param string $author + * @param string|null $author + * @param string $expected */ - public function testPublish($author) { + public function testPublish($author, $expected) { if ($author !== null) { $authorObject = $this->getMockBuilder('OCP\IUser') ->disableOriginalConstructor() @@ -332,11 +325,12 @@ class ManagerTest extends TestCase { ->willReturn($authorObject); } - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test') ->setType('test_type') ->setSubject('test_subject', []) - ->setAffectedUser('test_affected'); + ->setAffectedUser('test_affected') + ->setObject('file', 123); $consumer = $this->getMockBuilder('OCP\Activity\IConsumer') ->disableOriginalConstructor() @@ -344,10 +338,10 @@ class ManagerTest extends TestCase { $consumer->expects($this->once()) ->method('receive') ->with($event) - ->willReturnCallback(function(\OCP\Activity\IEvent $event) use ($author) { + ->willReturnCallback(function(\OCP\Activity\IEvent $event) use ($expected) { $this->assertLessThanOrEqual(time() + 2, $event->getTimestamp(), 'Timestamp not set correctly'); $this->assertGreaterThanOrEqual(time() - 2, $event->getTimestamp(), 'Timestamp not set correctly'); - $this->assertSame($author, $event->getAuthor(), 'Author name not set correctly'); + $this->assertSame($expected, $event->getAuthor(), 'Author name not set correctly'); }); $this->activityManager->registerConsumer(function () use ($consumer) { return $consumer; @@ -357,7 +351,7 @@ class ManagerTest extends TestCase { } public function testPublishAllManually() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test_app') ->setType('test_type') ->setAffectedUser('test_affected') @@ -397,7 +391,7 @@ class ManagerTest extends TestCase { } public function testDeprecatedPublishActivity() { - $event = new \OC\Activity\Event(); + $event = $this->activityManager->generateEvent(); $event->setApp('test_app') ->setType('test_type') ->setAffectedUser('test_affected') @@ -428,7 +422,7 @@ class ManagerTest extends TestCase { // The following values can not be used via publishActivity() $this->assertLessThanOrEqual(time() + 2, $event->getTimestamp(), 'Timestamp not set correctly'); $this->assertGreaterThanOrEqual(time() - 2, $event->getTimestamp(), 'Timestamp not set correctly'); - $this->assertSame(null, $event->getAuthor(), 'Author not set correctly'); + $this->assertSame('', $event->getAuthor(), 'Author not set correctly'); $this->assertSame('', $event->getObjectType(), 'Object type should not be set'); $this->assertSame(0, $event->getObjectId(), 'Object ID should not be set'); }); -- 2.39.5