Merge pull request #14195 from owncloud/activity-manager-performance-improvements

Activity manager performance improvements

lib/private/activitymanager.php

@@ -40,6 +40,19 @@ class ActivityManager implements IManager {
 	 */
 	private $extensions = array();
 
+	/** @var array list of filters "name" => "is valid" */
+	protected $validFilters = array(
+		'all'	=> true,
+		'by'	=> true,
+		'self'	=> true,
+	);
+
+	/** @var array list of type icons "type" => "css class" */
+	protected $typeIcons = array();
+
+	/** @var array list of special parameters "app" => ["text" => ["parameter" => "type"]] */
+	protected $specialParameters = array();
+
 	/**
 	 * @param $app
 	 * @param $subject
@@ -123,24 +136,6 @@ class ActivityManager implements IManager {
 		return $notificationTypes;
 	}
 
-	/**
-	 * @param array $types
-	 * @param string $filter
-	 * @return array
-	 */
-	function filterNotificationTypes($types, $filter) {
-		foreach($this->extensions as $extension) {
-			$c = $extension();
-			if ($c instanceof IExtension) {
-				$result = $c->filterNotificationTypes($types, $filter);
-				if (is_array($result)) {
-					$types = $result;
-				}
-			}
-		}
-		return $types;
-	}
-
 	/**
 	 * @param string $method
 	 * @return array
@@ -159,6 +154,30 @@ class ActivityManager implements IManager {
 		return $defaultTypes;
 	}
 
+	/**
+	 * @param string $type
+	 * @return string
+	 */
+	function getTypeIcon($type) {
+		if (isset($this->typeIcons[$type])) {
+			return $this->typeIcons[$type];
+		}
+
+		foreach($this->extensions as $extension) {
+			$c = $extension();
+			if ($c instanceof IExtension) {
+				$icon = $c->getTypeIcon($type);
+				if (is_string($icon)) {
+					$this->typeIcons[$type] = $icon;
+					return $icon;
+				}
+			}
+		}
+
+		$this->typeIcons[$type] = '';
+		return '';
+	}
+
 	/**
 	 * @param string $app
 	 * @param string $text
@@ -188,37 +207,29 @@ class ActivityManager implements IManager {
 	 * @return array|false
 	 */
 	function getSpecialParameterList($app, $text) {
+		if (isset($this->specialParameters[$app][$text])) {
+			return $this->specialParameters[$app][$text];
+		}
+
+		if (!isset($this->specialParameters[$app])) {
+			$this->specialParameters[$app] = array();
+		}
+
 		foreach($this->extensions as $extension) {
 			$c = $extension();
 			if ($c instanceof IExtension) {
 				$specialParameter = $c->getSpecialParameterList($app, $text);
 				if (is_array($specialParameter)) {
+					$this->specialParameters[$app][$text] = $specialParameter;
 					return $specialParameter;
 				}
 			}
 		}
 
+		$this->specialParameters[$app][$text] = false;
 		return false;
 	}
 
-	/**
-	 * @param string $type
-	 * @return string
-	 */
-	function getTypeIcon($type) {
-		foreach($this->extensions as $extension) {
-			$c = $extension();
-			if ($c instanceof IExtension) {
-				$icon = $c->getTypeIcon($type);
-				if (is_string($icon)) {
-					return $icon;
-				}
-			}
-		}
-
-		return '';
-	}
-
 	/**
 	 * @param array $activity
 	 * @return integer|false
@@ -264,23 +275,54 @@ class ActivityManager implements IManager {
 	 * @return boolean
 	 */
 	function isFilterValid($filterValue) {
+		if (isset($this->validFilters[$filterValue])) {
+			return $this->validFilters[$filterValue];
+		}
+
 		foreach($this->extensions as $extension) {
 			$c = $extension();
 			if ($c instanceof IExtension) {
 				if ($c->isFilterValid($filterValue) === true) {
+					$this->validFilters[$filterValue] = true;
 					return true;
 				}
 			}
 		}
 
+		$this->validFilters[$filterValue] = false;
 		return false;
 	}
 
+	/**
+	 * @param array $types
+	 * @param string $filter
+	 * @return array
+	 */
+	function filterNotificationTypes($types, $filter) {
+		if (!$this->isFilterValid($filter)) {
+			return $types;
+		}
+
+		foreach($this->extensions as $extension) {
+			$c = $extension();
+			if ($c instanceof IExtension) {
+				$result = $c->filterNotificationTypes($types, $filter);
+				if (is_array($result)) {
+					$types = $result;
+				}
+			}
+		}
+		return $types;
+	}
+
 	/**
 	 * @param string $filter
 	 * @return array
 	 */
 	function getQueryForFilter($filter) {
+		if (!$this->isFilterValid($filter)) {
+			return [null, null];
+		}
 
 		$conditions = array();
 		$parameters = array();

lib/public/activity/iextension.php

@@ -41,16 +41,6 @@ interface IExtension {
 	 */
 	public function getNotificationTypes($languageCode);
 
-	/**
-	 * The extension can filter the types based on the filter if required.
-	 * In case no filter is to be applied false is to be returned unchanged.
-	 *
-	 * @param array $types
-	 * @param string $filter
-	 * @return array|false
-	 */
-	public function filterNotificationTypes($types, $filter);
-
 	/**
 	 * For a given method additional types to be displayed in the settings can be returned.
 	 * In case no additional types are to be added false is to be returned.
@@ -60,6 +50,15 @@ interface IExtension {
 	 */
 	public function getDefaultTypes($method);
 
+	/**
+	 * A string naming the css class for the icon to be used can be returned.
+	 * If no icon is known for the given type false is to be returned.
+	 *
+	 * @param string $type
+	 * @return string|false
+	 */
+	public function getTypeIcon($type);
+
 	/**
 	 * The extension can translate a given message to the requested languages.
 	 * If no translation is available false is to be returned.
@@ -87,15 +86,6 @@ interface IExtension {
 	 */
 	public function getSpecialParameterList($app, $text);
 
-	/**
-	 * A string naming the css class for the icon to be used can be returned.
-	 * If no icon is known for the given type false is to be returned.
-	 *
-	 * @param string $type
-	 * @return string|false
-	 */
-	public function getTypeIcon($type);
-
 	/**
 	 * The extension can define the parameter grouping by returning the index as integer.
 	 * In case no grouping is required false is to be returned.
@@ -122,6 +112,16 @@ interface IExtension {
 	 */
 	public function isFilterValid($filterValue);
 
+	/**
+	 * The extension can filter the types based on the filter if required.
+	 * In case no filter is to be applied false is to be returned unchanged.
+	 *
+	 * @param array $types
+	 * @param string $filter
+	 * @return array|false
+	 */
+	public function filterNotificationTypes($types, $filter);
+
 	/**
 	 * For a given filter the extension can specify the sql query conditions including parameters for that query.
 	 * In case the extension does not know the filter false is to be returned.

lib/public/activity/imanager.php

@@ -71,19 +71,18 @@ interface IManager {
 	 */
 	function getNotificationTypes($languageCode);
 
-	/**
-	 * @param array $types
-	 * @param string $filter
-	 * @return array
-	 */
-	function filterNotificationTypes($types, $filter);
-
 	/**
 	 * @param string $method
 	 * @return array
 	 */
 	function getDefaultTypes($method);
 
+	/**
+	 * @param string $type
+	 * @return string
+	 */
+	function getTypeIcon($type);
+
 	/**
 	 * @param string $app
 	 * @param string $text
@@ -102,12 +101,6 @@ interface IManager {
 	 */
 	function getSpecialParameterList($app, $text);
 
-	/**
-	 * @param string $type
-	 * @return string
-	 */
-	function getTypeIcon($type);
-
 	/**
 	 * @param array $activity
 	 * @return integer|false
@@ -125,6 +118,13 @@ interface IManager {
 	 */
 	function isFilterValid($filterValue);
 
+	/**
+	 * @param array $types
+	 * @param string $filter
+	 * @return array
+	 */
+	function filterNotificationTypes($types, $filter);
+
 	/**
 	 * @param string $filter
 	 * @return array

tests/lib/activitymanager.php

@@ -31,16 +31,6 @@ class Test_ActivityManager extends \Test\TestCase {
 		$this->assertEquals(2, sizeof($result));
 	}
 
-	public function testFilterNotificationTypes() {
-		$result = $this->activityManager->filterNotificationTypes(array('NT0', 'NT1', 'NT2', 'NT3'), 'FILTER1');
-		$this->assertTrue(is_array($result));
-		$this->assertEquals(3, sizeof($result));
-
-		$result = $this->activityManager->filterNotificationTypes(array('NT0', 'NT1', 'NT2', 'NT3'), 'FILTER2');
-		$this->assertTrue(is_array($result));
-		$this->assertEquals(4, sizeof($result));
-	}
-
 	public function testDefaultTypes() {
 		$result = $this->activityManager->getDefaultTypes('stream');
 		$this->assertTrue(is_array($result));
@@ -51,6 +41,14 @@ class Test_ActivityManager extends \Test\TestCase {
 		$this->assertEquals(0, sizeof($result));
 	}
 
+	public function testTypeIcon() {
+		$result = $this->activityManager->getTypeIcon('NT1');
+		$this->assertEquals('icon-nt-one', $result);
+
+		$result = $this->activityManager->getTypeIcon('NT2');
+		$this->assertEquals('', $result);
+	}
+
 	public function testTranslate() {
 		$result = $this->activityManager->translate('APP0', '', '', array(), false, false, 'en');
 		$this->assertEquals('Stupid translation', $result);
@@ -67,14 +65,6 @@ class Test_ActivityManager extends \Test\TestCase {
 		$this->assertFalse($result);
 	}
 
-	public function testTypeIcon() {
-		$result = $this->activityManager->getTypeIcon('NT1');
-		$this->assertEquals('icon-nt-one', $result);
-
-		$result = $this->activityManager->getTypeIcon('NT2');
-		$this->assertEquals('', $result);
-	}
-
 	public function testGroupParameter() {
 		$result = $this->activityManager->getGroupParameter(array());
 		$this->assertEquals(5, $result);
@@ -90,15 +80,27 @@ class Test_ActivityManager extends \Test\TestCase {
 		$result = $this->activityManager->isFilterValid('fv01');
 		$this->assertTrue($result);
 
-		$result = $this->activityManager->isFilterValid('FV2');
+		$result = $this->activityManager->isFilterValid('InvalidFilter');
 		$this->assertFalse($result);
 	}
 
+	public function testFilterNotificationTypes() {
+		$result = $this->activityManager->filterNotificationTypes(array('NT0', 'NT1', 'NT2', 'NT3'), 'fv01');
+		$this->assertTrue(is_array($result));
+		$this->assertEquals(3, sizeof($result));
+
+		$result = $this->activityManager->filterNotificationTypes(array('NT0', 'NT1', 'NT2', 'NT3'), 'InvalidFilter');
+		$this->assertTrue(is_array($result));
+		$this->assertEquals(4, sizeof($result));
+	}
+
 	public function testQueryForFilter() {
+		// Register twice, to test the created sql part
 		$this->activityManager->registerExtension(function() {
 			return new SimpleExtension();
 		});
-		$result = $this->activityManager->getQueryForFilter('filter1');
+
+		$result = $this->activityManager->getQueryForFilter('fv01');
 		$this->assertEquals(
 			array(
 				' and ((`app` = ? and `message` like ?) or (`app` = ? and `message` like ?))',
@@ -106,8 +108,8 @@ class Test_ActivityManager extends \Test\TestCase {
 			), $result
 		);
 
-		$result = $this->activityManager->isFilterValid('filter2');
+		$result = $this->activityManager->getQueryForFilter('InvalidFilter');
-		$this->assertFalse($result);
+		$this->assertEquals(array(null, null), $result);
 	}
 }
 
@@ -117,13 +119,6 @@ class SimpleExtension implements \OCP\Activity\IExtension {
 		return array('NT1', 'NT2');
 	}
 
-	public function filterNotificationTypes($types, $filter) {
-		if ($filter === 'FILTER1') {
-			unset($types[0]);
-		}
-		return $types;
-	}
-
 	public function getDefaultTypes($method) {
 		if ($method === 'stream') {
 			return array('DT0');
@@ -132,6 +127,13 @@ class SimpleExtension implements \OCP\Activity\IExtension {
 		return array();
 	}
 
+	public function getTypeIcon($type) {
+		if ($type === 'NT1') {
+			return 'icon-nt-one';
+		}
+		return '';
+	}
+
 	public function translate($app, $text, $params, $stripPath, $highlightParams, $languageCode) {
 		if ($app === 'APP0') {
 			return "Stupid translation";
@@ -148,13 +150,6 @@ class SimpleExtension implements \OCP\Activity\IExtension {
 		return false;
 	}
 
-	public function getTypeIcon($type) {
-		if ($type === 'NT1') {
-			return 'icon-nt-one';
-		}
-		return '';
-	}
-
 	public function getGroupParameter($activity) {
 		return 5;
 	}
@@ -174,8 +169,15 @@ class SimpleExtension implements \OCP\Activity\IExtension {
 		return false;
 	}
 
+	public function filterNotificationTypes($types, $filter) {
+		if ($filter === 'fv01') {
+			unset($types[0]);
+		}
+		return $types;
+	}
+
 	public function getQueryForFilter($filter) {
-		if ($filter === 'filter1') {
+		if ($filter === 'fv01') {
 			return array('`app` = ? and `message` like ?', array('mail', 'ownCloud%'));
 		}
 
@@ -189,11 +191,11 @@ class NoOpExtension implements \OCP\Activity\IExtension {
 		return false;
 	}
 
-	public function filterNotificationTypes($types, $filter) {
+	public function getDefaultTypes($method) {
 		return false;
 	}
 
-	public function getDefaultTypes($method) {
+	public function getTypeIcon($type) {
 		return false;
 	}
 
@@ -205,10 +207,6 @@ class NoOpExtension implements \OCP\Activity\IExtension {
 		return false;
 	}
 
-	public function getTypeIcon($type) {
-		return false;
-	}
-
 	public function getGroupParameter($activity) {
 		return false;
 	}
@@ -221,6 +219,10 @@ class NoOpExtension implements \OCP\Activity\IExtension {
 		return false;
 	}
 
+	public function filterNotificationTypes($types, $filter) {
+		return false;
+	}
+
 	public function getQueryForFilter($filter) {
 		return false;
 	}