Merge pull request #352 from owncloud/oracle-problem-public-page

Correctly fall back to empty string when there is no user

appinfo/database.xml

@@ -32,13 +32,13 @@
 			<field>
 				<name>type</name>
 				<type>text</type>
-				<notnull>true</notnull>
+				<notnull>false</notnull>
 				<length>255</length>
 			</field>
 			<field>
 				<name>user</name>
 				<type>text</type>
-				<notnull>true</notnull>
+				<notnull>false</notnull>
 				<length>64</length>
 			</field>
 			<field>

appinfo/version

@@ -1 +1 @@
-2.1.0
+2.1.1

lib/data.php

@@ -25,6 +25,7 @@ namespace OCA\Activity;
 
 use OCP\Activity\IExtension;
 use OCP\DB;
+use OCP\IUser;
 use OCP\User;
 use OCP\Util;
 
@@ -81,9 +82,18 @@ class Data {
 	 */
 	public static function send($app, $subject, $subjectparams = array(), $message = '', $messageparams = array(), $file = '', $link = '', $affecteduser = '', $type = '', $prio = IExtension::PRIORITY_MEDIUM) {
 		$timestamp = time();
-		$user = User::getUser();
-		
-		if ($affecteduser === '') {
+
+		$user = \OC::$server->getUserSession()->getUser();
+		if ($user instanceof IUser) {
+			$user = $user->getUID();
+		} else {
+			// Public page or incognito mode
+			$user = '';
+		}
+
+		if ($affecteduser === '' && $user === '') {
+			return false;
+		} elseif ($affecteduser === '') {
 			$auser = $user;
 		} else {
 			$auser = $affecteduser;
@@ -107,7 +117,7 @@ class Data {
 	 * @param array  $subjectParams Array of parameters that are filled in the placeholders
 	 * @param string $affectedUser Name of the user we are sending the activity to
 	 * @param string $type Type of notification
-	 * @param int $latestSendTime Activity time() + batch setting of $affecteduser
+	 * @param int $latestSendTime Activity time() + batch setting of $affectedUser
 	 * @return bool
 	 */
 	public static function storeMail($app, $subject, array $subjectParams, $affectedUser, $type, $latestSendTime) {

tests/datatest.php

@@ -25,6 +25,7 @@ namespace OCA\Activity\Tests;
 use OC\ActivityManager;
 use OCA\Activity\Data;
 use OCA\Activity\Tests\Mock\Extension;
+use OCP\Activity\IExtension;
 
 class DataTest extends TestCase {
 	/** @var \OCA\Activity\Data */
@@ -89,4 +90,74 @@ class DataTest extends TestCase {
 	public function testValidateFilter($filter, $expected) {
 		$this->assertEquals($expected, $this->data->validateFilter($filter));
 	}
+
+	public function dataSend() {
+		return [
+			// Default case
+			['author', 'affectedUser', 'author', 'affectedUser', true],
+			// Public page / Incognito mode
+			['', 'affectedUser', '', 'affectedUser', true],
+			// No affected user, falling back to author
+			['author', '', 'author', 'author', true],
+			// No affected user and no author => no activity
+			['', '', '', '', false],
+		];
+	}
+
+	/**
+	 * @dataProvider dataSend
+	 *
+	 * @param string $actionUser
+	 * @param string $affectedUser
+	 */
+	public function testSend($actionUser, $affectedUser, $expectedAuthor, $expectedAffected, $expectedActivity) {
+		$mockSession = $this->getMockBuilder('\OC\User\Session')
+			->disableOriginalConstructor()
+			->getMock();
+
+		if ($actionUser !== '') {
+			$mockUser = $this->getMockBuilder('\OCP\IUser')
+				->disableOriginalConstructor()
+				->getMock();
+			$mockUser->expects($this->any())
+				->method('getUID')
+				->willReturn($actionUser);
+
+			$mockSession->expects($this->any())
+				->method('getUser')
+				->willReturn($mockUser);
+		} else {
+			$mockSession->expects($this->any())
+				->method('getUser')
+				->willReturn(null);
+		}
+
+		$this->overwriteService('UserSession', $mockSession);
+		$this->deleteTestActivities();
+
+		$this->assertSame($expectedActivity, Data::send('test', 'subject', [], '', [], '', '', $affectedUser, 'type', IExtension::PRIORITY_MEDIUM));
+
+		$connection = \OC::$server->getDatabaseConnection();
+		$query = $connection->prepare('SELECT `user`, `affecteduser` FROM `*PREFIX*activity` WHERE `app` = ? ORDER BY `activity_id` DESC');
+		$query->execute(['test']);
+		$row = $query->fetch();
+
+		if ($expectedActivity) {
+			$this->assertEquals(['user' => $expectedAuthor, 'affecteduser' => $expectedAffected], $row);
+		} else {
+			$this->assertFalse($row);
+		}
+
+		$this->deleteTestActivities();
+		$this->restoreService('UserSession');
+	}
+
+	/**
+	 * Delete all testing activities
+	 */
+	public function deleteTestActivities() {
+		$connection = \OC::$server->getDatabaseConnection();
+		$query = $connection->prepare('DELETE FROM `*PREFIX*activity` WHERE `app` = ?');
+		$query->execute(['test']);
+	}
 }

tests/testcase.php

@@ -23,4 +23,43 @@
 namespace OCA\Activity\Tests;
 
 abstract class TestCase extends \Test\TestCase {
+	/** @var array */
+	protected $services = [];
+
+	/**
+	 * @param string $name
+	 * @param mixed $newService
+	 * @return bool
+	 */
+	public function overwriteService($name, $newService) {
+		if (isset($this->services[$name])) {
+			return false;
+		}
+
+		$this->services[$name] = \OC::$server->query($name);
+		\OC::$server->registerService($name, function () use ($newService) {
+			return $newService;
+		});
+
+		return true;
+	}
+
+	/**
+	 * @param string $name
+	 * @return bool
+	 */
+	public function restoreService($name) {
+		if ($this->services[$name]) {
+			$oldService = $this->services[$name];
+			\OC::$server->registerService($name, function () use ($oldService) {
+				return $oldService;
+			});
+
+
+			unset($this->services[$name]);
+			return true;
+		}
+
+		return false;
+	}
 }