Fix activity manager tests

Signed-off-by: Joas Schilling <coding@schilljs.com>

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(
+		return new Event($this->validator);
-			new 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);

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) {

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
 	 */

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 IUserSession|\PHPUnit_Framework_MockObject_MockObject */
-	/** @var \OCP\IUserSession|\PHPUnit_Framework_MockObject_MockObject */
 	protected $session;
-
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
-	/** @var \OCP\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')
+		$this->request = $this->createMock(IRequest::class);
-			->disableOriginalConstructor()
+		$this->session = $this->createMock(IUserSession::class);
-			->getMock();
+		$this->config = $this->createMock(IConfig::class);
-		$this->session = $this->getMockBuilder('OCP\IUserSession')
+		$this->validator = $this->createMock(IValidator::class);
-			->disableOriginalConstructor()
-			->getMock();
-		$this->config = $this->getMockBuilder('OCP\IConfig')
-			->disableOriginalConstructor()
-			->getMock();
 
 		$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],
+			[null, ''],
-			['test_author'],
+			['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');
 			});