From b9f3136f3a7564b983aef6564f9c7488d5b2b25b Mon Sep 17 00:00:00 2001 From: Bernhard Posselt Date: Sun, 20 Apr 2014 12:14:42 +0200 Subject: get rid of unneeded settings core class and inject it from the core container --- app/news.php | 27 +++---- controller/apicontroller.php | 7 +- controller/feedcontroller.php | 12 ++-- controller/itemcontroller.php | 18 +++-- controller/pagecontroller.php | 21 +++--- core/settings.php | 101 --------------------------- db/mapperfactory.php | 5 +- tests/unit/controller/ApiControllerTest.php | 5 +- tests/unit/controller/FeedControllerTest.php | 18 +++-- tests/unit/controller/ItemControllerTest.php | 30 +++++--- tests/unit/controller/PageControllerTest.php | 30 +++++--- tests/unit/db/MapperFactoryTest.php | 2 +- 12 files changed, 107 insertions(+), 169 deletions(-) delete mode 100644 core/settings.php diff --git a/app/news.php b/app/news.php index de3652e6f..1214c1918 100644 --- a/app/news.php +++ b/app/news.php @@ -18,7 +18,6 @@ use \OCP\AppFramework\App; use \OCA\News\Core\Logger; use \OCA\News\Core\Db; -use \OCA\News\Core\Settings; use \OCA\News\Controller\PageController; use \OCA\News\Controller\FolderController; @@ -80,8 +79,9 @@ class News extends App { return new PageController( $c->query('AppName'), $c->query('Request'), - $c->query('Settings'), - $c->query('L10N') + $c->query('CoreConfig'), + $c->query('L10N'), + $c->query('UserId') ); }); @@ -103,8 +103,8 @@ class News extends App { $c->query('FolderBusinessLayer'), $c->query('FeedBusinessLayer'), $c->query('ItemBusinessLayer'), - $c->query('UserId'), - $c->query('Settings') + $c->query('CoreConfig'), + $c->query('UserId') ); }); @@ -114,8 +114,8 @@ class News extends App { $c->query('Request'), $c->query('FeedBusinessLayer'), $c->query('ItemBusinessLayer'), - $c->query('UserId'), - $c->query('Settings') + $c->query('CoreConfig'), + $c->query('UserId') ); }); @@ -136,7 +136,8 @@ class News extends App { $c->query('AppName'), $c->query('Request'), $c->query('Updater'), - $c->query('Settings') + $c->query('CoreConfig'), + $c->query('UserId') ); }); @@ -213,7 +214,7 @@ class News extends App { */ $container->registerService('MapperFactory', function($c) { return new MapperFactory( - $c->query('Settings'), $c->query('Db') + $c->query('CoreConfig'), $c->query('Db') ); }); @@ -254,8 +255,8 @@ class News extends App { return new Db(); }); - $container->registerService('Settings', function($c) { - return new Settings($c['AppName'], $c['UserId']); + $container->registerService('CoreConfig', function($c) { + return $c->query('ServerContainer')->getConfig(); }); @@ -278,7 +279,7 @@ class News extends App { }); $container->registerService('simplePieCacheDirectory', function($c) { - $directory = $c->query('Settings')->getSystemValue('datadirectory') . + $directory = $c->query('CoreConfig')->getSystemValue('datadirectory') . '/news/cache/simplepie'; if(!is_dir($directory)) { @@ -288,7 +289,7 @@ class News extends App { }); $container->registerService('HTMLPurifier', function($c) { - $directory = $c->query('Settings')->getSystemValue('datadirectory') . + $directory = $c->query('CoreConfig')->getSystemValue('datadirectory') . '/news/cache/purifier'; if(!is_dir($directory)) { diff --git a/controller/apicontroller.php b/controller/apicontroller.php index 7af08d56a..44c888972 100644 --- a/controller/apicontroller.php +++ b/controller/apicontroller.php @@ -14,13 +14,13 @@ namespace OCA\News\Controller; use \OCP\IRequest; +use \OCP\IConfig; use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCP\AppFramework\Http\JSONResponse; use \OCP\AppFramework\Http\Response; use \OCA\News\Utility\Updater; -use \OCA\News\Core\Settings; class ApiController extends Controller { @@ -28,7 +28,7 @@ class ApiController extends Controller { private $settings; public function __construct($appName, IRequest $request, Updater $updater, - Settings $settings){ + IConfig $settings){ parent::__construct($appName, $request); $this->updater = $updater; $this->settings = $settings; @@ -41,7 +41,8 @@ class ApiController extends Controller { * @API */ public function version() { - $version = $this->settings->getAppValue('installed_version'); + $version = $this->settings->getAppValue($this->appName, + 'installed_version'); $response = new JSONResponse(array('version' => $version)); return $response; } diff --git a/controller/feedcontroller.php b/controller/feedcontroller.php index e9be7e811..afc3de742 100644 --- a/controller/feedcontroller.php +++ b/controller/feedcontroller.php @@ -14,11 +14,11 @@ namespace OCA\News\Controller; use \OCP\IRequest; +use \OCP\IConfig; use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCP\AppFramework\Http\JSONResponse; -use \OCA\News\Core\Settings; use \OCA\News\BusinessLayer\ItemBusinessLayer; use \OCA\News\BusinessLayer\FeedBusinessLayer; use \OCA\News\BusinessLayer\FolderBusinessLayer; @@ -40,8 +40,8 @@ class FeedController extends Controller { FolderBusinessLayer $folderBusinessLayer, FeedBusinessLayer $feedBusinessLayer, ItemBusinessLayer $itemBusinessLayer, - $userId, - Settings $settings){ + IConfig $settings, + $userId){ parent::__construct($appName, $request); $this->feedBusinessLayer = $feedBusinessLayer; $this->folderBusinessLayer = $folderBusinessLayer; @@ -77,8 +77,10 @@ class FeedController extends Controller { * @NoAdminRequired */ public function active(){ - $feedId = (int) $this->settings->getUserValue('lastViewedFeedId'); - $feedType = $this->settings->getUserValue('lastViewedFeedType'); + $feedId = (int) $this->settings->getUserValue($this->userId, + $this->appName,'lastViewedFeedId'); + $feedType = $this->settings->getUserValue($this->userId, $this->appName, + 'lastViewedFeedType'); // cast from null to int is 0 if($feedType !== null){ diff --git a/controller/itemcontroller.php b/controller/itemcontroller.php index ec316aabb..99f308be7 100644 --- a/controller/itemcontroller.php +++ b/controller/itemcontroller.php @@ -14,11 +14,11 @@ namespace OCA\News\Controller; use \OCP\IRequest; +use \OCP\IConfig; use \OCP\AppFramework\Controller; use \OCP\AppFramework\Http; use \OCP\AppFramework\Http\JSONResponse; -use \OCA\News\Core\Settings; use \OCA\News\BusinessLayer\BusinessLayerException; use \OCA\News\BusinessLayer\ItemBusinessLayer; use \OCA\News\BusinessLayer\FeedBusinessLayer; @@ -35,8 +35,8 @@ class ItemController extends Controller { IRequest $request, FeedBusinessLayer $feedBusinessLayer, ItemBusinessLayer $itemBusinessLayer, - $userId, - Settings $settings){ + IConfig $settings, + $userId){ parent::__construct($appName, $request); $this->itemBusinessLayer = $itemBusinessLayer; $this->feedBusinessLayer = $feedBusinessLayer; @@ -49,15 +49,18 @@ class ItemController extends Controller { * @NoAdminRequired */ public function index(){ - $showAll = $this->settings->getUserValue('showAll') === '1'; + $showAll = $this->settings->getUserValue($this->userId, $this->appName, + 'showAll') === '1'; $limit = $this->params('limit'); $type = (int) $this->params('type'); $id = (int) $this->params('id'); $offset = (int) $this->params('offset', 0); - $this->settings->setUserValue('lastViewedFeedId', $id); - $this->settings->setUserValue('lastViewedFeedType', $type); + $this->settings->setUserValue($this->userId, $this->appName, + 'lastViewedFeedId', $id); + $this->settings->setUserValue($this->userId, $this->appName, + 'lastViewedFeedType', $type); $params = array(); @@ -87,7 +90,8 @@ class ItemController extends Controller { * @NoAdminRequired */ public function newItems() { - $showAll = $this->settings->getUserValue('showAll') === '1'; + $showAll = $this->settings->getUserValue($this->userId, $this->appName, + 'showAll') === '1'; $type = (int) $this->params('type'); $id = (int) $this->params('id'); diff --git a/controller/pagecontroller.php b/controller/pagecontroller.php index 5b7cf7ea1..fe6fe967a 100644 --- a/controller/pagecontroller.php +++ b/controller/pagecontroller.php @@ -14,21 +14,22 @@ namespace OCA\News\Controller; use \OCP\IRequest; +use \OCP\IConfig; use \OCP\AppFramework\Http\JSONResponse; use \OCP\AppFramework\Controller; -use \OCA\News\Core\Settings; - class PageController extends Controller { private $settings; private $l10n; + private $userId; - public function __construct($appName, IRequest $request, Settings $settings, - $l10n){ + public function __construct($appName, IRequest $request, IConfig $settings, + $l10n, $userId){ parent::__construct($appName, $request); $this->settings = $settings; $this->l10n = $l10n; + $this->userId = $userId; } @@ -45,8 +46,10 @@ class PageController extends Controller { * @NoAdminRequired */ public function settings() { - $showAll = $this->settings->getUserValue('showAll'); - $compact = $this->settings->getUserValue('compact'); + $showAll = $this->settings->getUserValue($this->userId, $this->appName, + 'showAll'); + $compact = $this->settings->getUserValue($this->userId, $this->appName, + 'compact'); $language = $this->l10n->findLanguage(); $settings = array( @@ -67,11 +70,13 @@ class PageController extends Controller { $isCompact = $this->params('compact', null); if($isShowAll !== null) { - $this->settings->setUserValue('showAll', $isShowAll); + $this->settings->setUserValue($this->userId, $this->appName, + 'showAll', $isShowAll); } if($isCompact !== null) { - $this->settings->setUserValue('compact', $isCompact); + $this->settings->setUserValue($this->userId, $this->appName, + 'compact', $isCompact); } return new JSONResponse(); diff --git a/core/settings.php b/core/settings.php deleted file mode 100644 index 3d3c2dfdb..000000000 --- a/core/settings.php +++ /dev/null @@ -1,101 +0,0 @@ - - * @author Bernhard Posselt - * @copyright Alessandro Cosentino 2012 - * @copyright Bernhard Posselt 2012, 2014 - */ - -namespace OCA\News\Core; - -class Settings { - - protected $appName; - protected $userId; - - public function __construct($appName, $userId) { - $this->appName = $appName; - $this->userId = $userId; - } - - - /** - * Looks up a systemwide defined value - * @param string $key the key of the value, under which it was saved - * @return string the saved value - */ - public function getSystemValue($key){ - return \OCP\Config::getSystemValue($key, ''); - } - - - /** - * Sets a new systemwide value - * @param string $key the key of the value, under which will be saved - * @param string $value the value that should be stored - */ - public function setSystemValue($key, $value){ - return \OCP\Config::setSystemValue($key, $value); - } - - - /** - * Looks up an appwide defined value - * @param string $key the key of the value, under which it was saved - * @return string the saved value - */ - public function getAppValue($key, $appName=null){ - if($appName === null){ - $appName = $this->appName; - } - return \OCP\Config::getAppValue($appName, $key, ''); - } - - - /** - * Writes a new appwide value - * @param string $key the key of the value, under which will be saved - * @param string $value the value that should be stored - */ - public function setAppValue($key, $value, $appName=null){ - if($appName === null){ - $appName = $this->appName; - } - return \OCP\Config::setAppValue($appName, $key, $value); - } - - - - /** - * Shortcut for setting a user defined value - * @param string $key the key under which the value is being stored - * @param string $value the value that you want to store - * @param string $userId the userId of the user that we want to store the value under, defaults to the current one - */ - public function setUserValue($key, $value, $userId=null){ - if($userId === null){ - $userId = $this->userId; - } - \OCP\Config::setUserValue($userId, $this->appName, $key, $value); - } - - - /** - * Shortcut for getting a user defined value - * @param string $key the key under which the value is being stored - * @param string $userId the userId of the user that we want to store the value under, defaults to the current one - */ - public function getUserValue($key, $userId=null){ - if($userId === null){ - $userId = $this->userId; - } - return \OCP\Config::getUserValue($userId, $this->appName, $key); - } - - -} \ No newline at end of file diff --git a/db/mapperfactory.php b/db/mapperfactory.php index 6a1038af0..af2640a33 100644 --- a/db/mapperfactory.php +++ b/db/mapperfactory.php @@ -13,7 +13,8 @@ namespace OCA\News\Db; -use \OCA\News\Core\Settings; +use \OCP\IConfig; + use \OCA\News\Core\Db; @@ -21,7 +22,7 @@ class MapperFactory { private $settings; - public function __construct(Settings $settings, Db $db) { + public function __construct(IConfig $settings, Db $db) { $this->settings = $settings; $this->db = $db; } diff --git a/tests/unit/controller/ApiControllerTest.php b/tests/unit/controller/ApiControllerTest.php index 79a7d02e9..8975819ea 100644 --- a/tests/unit/controller/ApiControllerTest.php +++ b/tests/unit/controller/ApiControllerTest.php @@ -33,7 +33,7 @@ class ApiControllerTest extends ControllerTestUtility { protected function setUp() { $this->appName = 'news'; $this->settings = $this->getMockBuilder( - '\OCA\News\Core\Settings') + '\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); $this->request = $this->getMockBuilder( @@ -71,7 +71,8 @@ class ApiControllerTest extends ControllerTestUtility { public function testGetVersion(){ $this->settings->expects($this->once()) ->method('getAppValue') - ->with($this->equalTo('installed_version')) + ->with($this->equalTo($this->appName), + $this->equalTo('installed_version')) ->will($this->returnValue('1.0')); $response = $this->newsAPI->version(); diff --git a/tests/unit/controller/FeedControllerTest.php b/tests/unit/controller/FeedControllerTest.php index f25d4beed..a8309b41c 100644 --- a/tests/unit/controller/FeedControllerTest.php +++ b/tests/unit/controller/FeedControllerTest.php @@ -44,7 +44,7 @@ class FeedControllerTest extends ControllerTestUtility { $this->appName = 'news'; $this->user = 'jack'; $this->settings = $this->getMockBuilder( - '\OCA\News\Core\Settings') + '\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); $this->itemBusinessLayer = $this->getMockBuilder('\OCA\News\BusinessLayer\ItemBusinessLayer') @@ -61,8 +61,8 @@ class FeedControllerTest extends ControllerTestUtility { $this->folderBusinessLayer, $this->feedBusinessLayer, $this->itemBusinessLayer, - $this->user, - $this->settings); + $this->settings, + $this->user); } private function assertFeedControllerAnnotations($methodName){ @@ -82,8 +82,8 @@ class FeedControllerTest extends ControllerTestUtility { $this->folderBusinessLayer, $this->feedBusinessLayer, $this->itemBusinessLayer, - $this->user, - $this->settings); + $this->settings, + $this->user); } @@ -189,11 +189,15 @@ class FeedControllerTest extends ControllerTestUtility { private function activeInitMocks($id, $type){ $this->settings->expects($this->at(0)) ->method('getUserValue') - ->with($this->equalTo('lastViewedFeedId')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('lastViewedFeedId')) ->will($this->returnValue($id)); $this->settings->expects($this->at(1)) ->method('getUserValue') - ->with($this->equalTo('lastViewedFeedType')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('lastViewedFeedType')) ->will($this->returnValue($type)); } diff --git a/tests/unit/controller/ItemControllerTest.php b/tests/unit/controller/ItemControllerTest.php index c28eac9fa..6bc5595d8 100644 --- a/tests/unit/controller/ItemControllerTest.php +++ b/tests/unit/controller/ItemControllerTest.php @@ -44,7 +44,7 @@ class ItemControllerTest extends ControllerTestUtility { $this->appName = 'news'; $this->user = 'jackob'; $this->settings = $this->getMockBuilder( - '\OCA\News\Core\Settings') + '\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); $this->itemBusinessLayer = @@ -57,8 +57,8 @@ class ItemControllerTest extends ControllerTestUtility { ->getMock(); $this->request = $this->getRequest(); $this->controller = new ItemController($this->appName, $this->request, - $this->feedBusinessLayer, $this->itemBusinessLayer, $this->user, - $this->settings); + $this->feedBusinessLayer, $this->itemBusinessLayer, $this->settings, + $this->user); $this->newestItemId = 12312; } @@ -70,8 +70,8 @@ class ItemControllerTest extends ControllerTestUtility { $request = $this->getRequest($post); return new ItemController($this->appName, $request, - $this->feedBusinessLayer, $this->itemBusinessLayer, $this->user, - $this->settings); + $this->feedBusinessLayer, $this->itemBusinessLayer, $this->settings, + $this->user); } @@ -297,15 +297,21 @@ class ItemControllerTest extends ControllerTestUtility { private function itemsApiExpects($id, $type){ $this->settings->expects($this->once()) ->method('getUserValue') - ->with($this->equalTo('showAll')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll')) ->will($this->returnValue('1')); $this->settings->expects($this->at(1)) ->method('setUserValue') - ->with($this->equalTo('lastViewedFeedId'), + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('lastViewedFeedId'), $this->equalTo($id)); $this->settings->expects($this->at(2)) ->method('setUserValue') - ->with($this->equalTo('lastViewedFeedType'), + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('lastViewedFeedType'), $this->equalTo($type)); } @@ -434,7 +440,9 @@ class ItemControllerTest extends ControllerTestUtility { $this->settings->expects($this->once()) ->method('getUserValue') - ->with($this->equalTo('showAll')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll')) ->will($this->returnValue('1')); $this->feedBusinessLayer->expects($this->once()) @@ -479,7 +487,9 @@ class ItemControllerTest extends ControllerTestUtility { $this->settings->expects($this->once()) ->method('getUserValue') - ->with($this->equalTo('showAll')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll')) ->will($this->returnValue('1')); $this->itemBusinessLayer->expects($this->once()) diff --git a/tests/unit/controller/PageControllerTest.php b/tests/unit/controller/PageControllerTest.php index 898226361..49e155bd0 100644 --- a/tests/unit/controller/PageControllerTest.php +++ b/tests/unit/controller/PageControllerTest.php @@ -37,15 +37,15 @@ class PageControllerTest extends ControllerTestUtility { */ public function setUp(){ $this->appName = 'news'; + $this->user = 'becka'; $this->l10n = $this->getMock('L10N', array('findLanguage')); $this->settings = $this->getMockBuilder( - '\OCA\News\Core\Settings') + '\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); $this->request = $this->getRequest(); $this->controller = new PageController($this->appName, $this->request, - $this->settings, $this->l10n); - $this->user = 'becka'; + $this->settings, $this->l10n, $this->user); } @@ -83,11 +83,15 @@ class PageControllerTest extends ControllerTestUtility { ->will($this->returnValue('de')); $this->settings->expects($this->at(0)) ->method('getUserValue') - ->with($this->equalTo('showAll')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll')) ->will($this->returnValue('1')); $this->settings->expects($this->at(1)) ->method('getUserValue') - ->with($this->equalTo('compact')) + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('compact')) ->will($this->returnValue('1')); $response = $this->controller->settings(); @@ -102,15 +106,19 @@ class PageControllerTest extends ControllerTestUtility { 'compact' => true ))); $this->controller = new PageController($this->appName, $request, - $this->settings, $this->l10n); + $this->settings, $this->l10n, $this->user); $this->settings->expects($this->at(0)) ->method('setUserValue') - ->with($this->equalTo('showAll'), + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll'), $this->equalTo(true)); $this->settings->expects($this->at(1)) ->method('setUserValue') - ->with($this->equalTo('compact'), + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('compact'), $this->equalTo(true)); $response = $this->controller->updateSettings(); @@ -123,11 +131,13 @@ class PageControllerTest extends ControllerTestUtility { 'showAll' => true ))); $this->controller = new PageController($this->appName, $request, - $this->settings, $this->l10n); + $this->settings, $this->l10n, $this->user); $this->settings->expects($this->once()) ->method('setUserValue') - ->with($this->equalTo('showAll'), + ->with($this->equalTo($this->user), + $this->equalTo($this->appName), + $this->equalTo('showAll'), $this->equalTo(true)); $response = $this->controller->updateSettings(); diff --git a/tests/unit/db/MapperFactoryTest.php b/tests/unit/db/MapperFactoryTest.php index 3e82c1555..a83eca8ec 100644 --- a/tests/unit/db/MapperFactoryTest.php +++ b/tests/unit/db/MapperFactoryTest.php @@ -23,7 +23,7 @@ class MapperFactoryTest extends \PHPUnit_Framework_TestCase { private $settings; public function setUp() { - $this->settings = $this->getMockBuilder('\OCA\News\Core\Settings') + $this->settings = $this->getMockBuilder('\OCP\IConfig') ->disableOriginalConstructor() ->getMock(); $this->db = $this->getMockBuilder('\OCA\News\Core\Db') -- cgit v1.2.3