From c02de748e56b559b7429f232d5d07606e90d4b44 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 23 Jan 2015 15:37:15 +0100 Subject: [PATCH 1/4] Cache some values from the extensions --- lib/private/activitymanager.php | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/private/activitymanager.php b/lib/private/activitymanager.php index 70bd227b417..b7999d0216a 100644 --- a/lib/private/activitymanager.php +++ b/lib/private/activitymanager.php @@ -25,6 +25,15 @@ class ActivityManager implements IManager { */ private $extensions = array(); + /** @var array list of filters "name" => "is valid" */ + protected $validFilters = array(); + + /** @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 @@ -173,16 +182,26 @@ 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; } @@ -191,16 +210,22 @@ class ActivityManager implements IManager { * @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 ''; } @@ -249,15 +274,21 @@ 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; } From be63e18b0aaf403dab86b3b42500e5e4b18183e4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 13 Feb 2015 11:50:55 +0100 Subject: [PATCH 2/4] Check whether filter is valid, before doing stuff for it --- lib/private/activitymanager.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/private/activitymanager.php b/lib/private/activitymanager.php index b7999d0216a..ee7059d492d 100644 --- a/lib/private/activitymanager.php +++ b/lib/private/activitymanager.php @@ -26,7 +26,11 @@ class ActivityManager implements IManager { private $extensions = array(); /** @var array list of filters "name" => "is valid" */ - protected $validFilters = array(); + protected $validFilters = array( + 'all' => true, + 'by' => true, + 'self' => true, + ); /** @var array list of type icons "type" => "css class" */ protected $typeIcons = array(); @@ -123,6 +127,10 @@ class ActivityManager implements IManager { * @return array */ function filterNotificationTypes($types, $filter) { + if (!$this->isFilterValid($filter)) { + return $types; + } + foreach($this->extensions as $extension) { $c = $extension(); if ($c instanceof IExtension) { @@ -297,6 +305,9 @@ class ActivityManager implements IManager { * @return array */ function getQueryForFilter($filter) { + if (!$this->isFilterValid($filter)) { + return [null, null]; + } $conditions = array(); $parameters = array(); From 6c349c00bb7ce3c43a44588de5cf158158d2990b Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 13 Feb 2015 11:51:55 +0100 Subject: [PATCH 3/4] Order methods to by grouped by their task --- lib/private/activitymanager.php | 92 +++++++++++++++--------------- lib/public/activity/iextension.php | 38 ++++++------ lib/public/activity/imanager.php | 26 ++++----- 3 files changed, 78 insertions(+), 78 deletions(-) diff --git a/lib/private/activitymanager.php b/lib/private/activitymanager.php index ee7059d492d..24acb5d5f69 100644 --- a/lib/private/activitymanager.php +++ b/lib/private/activitymanager.php @@ -121,28 +121,6 @@ class ActivityManager implements IManager { return $notificationTypes; } - /** - * @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 $method * @return array @@ -161,6 +139,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 @@ -213,30 +215,6 @@ class ActivityManager implements IManager { return false; } - /** - * @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 array $activity * @return integer|false @@ -300,6 +278,28 @@ class ActivityManager implements IManager { 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 diff --git a/lib/public/activity/iextension.php b/lib/public/activity/iextension.php index 1b405ad8d3d..0f6c3c5f8a5 100644 --- a/lib/public/activity/iextension.php +++ b/lib/public/activity/iextension.php @@ -46,16 +46,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. @@ -65,6 +55,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. @@ -92,15 +91,6 @@ interface IExtension { */ 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. @@ -127,6 +117,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. diff --git a/lib/public/activity/imanager.php b/lib/public/activity/imanager.php index a08670be4b0..4a07e6912b0 100644 --- a/lib/public/activity/imanager.php +++ b/lib/public/activity/imanager.php @@ -75,19 +75,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 @@ -106,12 +105,6 @@ interface IManager { */ function getSpecialParameterList($app, $text); - /** - * @param string $type - * @return string - */ - function getTypeIcon($type); - /** * @param array $activity * @return integer|false @@ -129,6 +122,13 @@ interface IManager { */ function isFilterValid($filterValue); + /** + * @param array $types + * @param string $filter + * @return array + */ + function filterNotificationTypes($types, $filter); + /** * @param string $filter * @return array From 12fc6258854eb2e9d72a8777131e2bca71075b41 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 13 Feb 2015 13:08:57 +0100 Subject: [PATCH 4/4] Fix test with the invalid filter check being added --- tests/lib/activitymanager.php | 88 ++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/tests/lib/activitymanager.php b/tests/lib/activitymanager.php index 6a5af7b259b..d227c05d827 100644 --- a/tests/lib/activitymanager.php +++ b/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'); - $this->assertFalse($result); + $result = $this->activityManager->getQueryForFilter('InvalidFilter'); + $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; }