mbk-lab
/
nextcloud
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Reduce settings manager complexity by loading sections via DI 

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
This commit is contained in:
Christoph Wurst 2018-09-26 16:46:35 +02:00
parent cd05670517
commit 0259792614
No known key found for this signature in database
GPG Key ID: CC42AC2A7F0E56D8
3 changed files with 63 additions and 140 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
lib/private/Server.php
View File

@ -1083,16 +1083,8 @@ class Server extends ServerContainer implements IServerContainer {
$c->getLogger(),
$c->getDatabaseConnection(),
$c->getL10N('lib'),
$c->getConfig(),
$c->getEncryptionManager(),
$c->getUserManager(),
$c->getLockingProvider(),
$c->getRequest(),
$c->getURLGenerator(),
$c->query(AccountManager::class),
$c->getGroupManager(),
$c->getL10NFactory(),
$c->getAppManager()
$c
);
return $manager;
});

112
lib/private/Settings/Manager.php
View File

@ -29,96 +29,45 @@
namespace OC\Settings;
use OC\Accounts\AccountManager;
use OCP\App\IAppManager;
use OCP\AppFramework\QueryException;
use OCP\Encryption\IManager as EncryptionManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\L10N\IFactory;
use OCP\Lock\ILockingProvider;
use OCP\Settings\ISettings;
use OCP\Settings\IManager;
use OCP\Settings\ISection;
use OCP\Util;
class Manager implements IManager {
/** @var ILogger */
private $log;
/** @var IDBConnection */
private $dbc;
/** @var IL10N */
private $l;
/** @var IConfig */
private $config;
/** @var EncryptionManager */
private $encryptionManager;
/** @var IUserManager */
private $userManager;
/** @var ILockingProvider */
private $lockingProvider;
/** @var IRequest */
private $request;
/** @var IURLGenerator */
private $url;
/** @var AccountManager */
private $accountManager;
/** @var IGroupManager */
private $groupManager;
/** @var IFactory */
private $l10nFactory;
/** @var IAppManager */
private $appManager;
/**
* @param ILogger $log
* @param IDBConnection $dbc
* @param IL10N $l
* @param IConfig $config
* @param EncryptionManager $encryptionManager
* @param IUserManager $userManager
* @param ILockingProvider $lockingProvider
* @param IRequest $request
* @param IURLGenerator $url
* @param AccountManager $accountManager
* @param IGroupManager $groupManager
* @param IFactory $l10nFactory
* @param IAppManager $appManager
*/
/** @var IServerContainer */
private $container;
public function __construct(
ILogger $log,
IDBConnection $dbc,
IL10N $l,
IConfig $config,
EncryptionManager $encryptionManager,
IUserManager $userManager,
ILockingProvider $lockingProvider,
IRequest $request,
IL10N $l10n,
IURLGenerator $url,
AccountManager $accountManager,
IGroupManager $groupManager,
IFactory $l10nFactory,
IAppManager $appManager
IServerContainer $container
) {
$this->log = $log;
$this->dbc = $dbc;
$this->l = $l;
$this->config = $config;
$this->encryptionManager = $encryptionManager;
$this->userManager = $userManager;
$this->lockingProvider = $lockingProvider;
$this->request = $request;
$this->l = $l10n;
$this->url = $url;
$this->accountManager = $accountManager;
$this->groupManager = $groupManager;
$this->l10nFactory = $l10nFactory;
$this->appManager = $appManager;
$this->container = $container;
}
/** @var array */
@ -130,6 +79,7 @@ class Manager implements IManager {
/**
* @param string $type 'admin' or 'personal'
* @param string $section Class must implement OCP\Settings\ISection
*
* @return void
*/
public function registerSection(string $type, string $section) {
@ -142,6 +92,7 @@ class Manager implements IManager {
/**
* @param string $type 'admin' or 'personal'
*
* @return ISection[]
*/
protected function getSections(string $type): array {
@ -191,6 +142,7 @@ class Manager implements IManager {
/**
* @param string $type 'admin' or 'personal'
* @param string $setting Class must implement OCP\Settings\ISetting
*
* @return void
*/
public function registerSetting(string $type, string $setting) {
@ -200,6 +152,7 @@ class Manager implements IManager {
/**
* @param string $type 'admin' or 'personal'
* @param string $section
*
* @return ISettings[]
*/
protected function getSettings(string $type, string $section): array {
@ -267,6 +220,7 @@ class Manager implements IManager {
/**
* @param string $section
*
* @return ISection[]
*/
private function getBuiltInAdminSettings($section): array {
@ -274,24 +228,24 @@ class Manager implements IManager {
if ($section === 'overview') {
/** @var ISettings $form */
$form = new Admin\Overview($this->config);
$form = $this->container->query(Admin\Overview::class);
$forms[$form->getPriority()] = [$form];
}
if ($section === 'server') {
/** @var ISettings $form */
$form = new Admin\Server($this->dbc, $this->request, $this->config, $this->lockingProvider, $this->l);
$form = $this->container->query(Admin\Server::class);
$forms[$form->getPriority()] = [$form];
$form = new Admin\Mail($this->config);
$form = $this->container->query(Admin\Mail::class);
$forms[$form->getPriority()] = [$form];
}
if ($section === 'security') {
/** @var ISettings $form */
$form = new Admin\Encryption($this->encryptionManager, $this->userManager);
$form = $this->container->query(Admin\Encryption::class);
$forms[$form->getPriority()] = [$form];
}
if ($section === 'sharing') {
/** @var ISettings $form */
$form = new Admin\Sharing($this->config, $this->l);
$form = $this->container->query(Admin\Sharing::class);
$forms[$form->getPriority()] = [$form];
}
@ -300,6 +254,7 @@ class Manager implements IManager {
/**
* @param string $section
*
* @return ISection[]
*/
private function getBuiltInPersonalSettings($section): array {
@ -307,27 +262,19 @@ class Manager implements IManager {
if ($section === 'personal-info') {
/** @var ISettings $form */
$form = new Personal\PersonalInfo(
$this->config,
$this->userManager,
$this->groupManager,
$this->accountManager,
$this->appManager,
$this->l10nFactory,
$this->l
);
$form = $this->container->query(Personal\PersonalInfo::class);
$forms[$form->getPriority()] = [$form];
$form = new Personal\ServerDevNotice();
$forms[$form->getPriority()] = [$form];
}
if($section === 'security') {
if ($section === 'security') {
/** @var ISettings $form */
$form = new Personal\Security($this->userManager);
$form = $this->container->query(Personal\Security::class);
$forms[$form->getPriority()] = [$form];
}
if ($section === 'additional') {
/** @var ISettings $form */
$form = new Personal\Additional();
$form = $this->container->query(Personal\Additional::class);
$forms[$form->getPriority()] = [$form];
}
@ -363,7 +310,7 @@ class Manager implements IManager {
];
$legacyForms = \OC_App::getForms('personal');
if(!empty($legacyForms) && $this->hasLegacyPersonalSettingsToRender($legacyForms)) {
if (!empty($legacyForms) && $this->hasLegacyPersonalSettingsToRender($legacyForms)) {
$sections[98] = [new Section('additional', $this->l->t('Additional settings'), 0, $this->url->imagePath('core', 'actions/settings-dark.svg'))];
}
@ -385,11 +332,12 @@ class Manager implements IManager {
/**
* @param string[] $forms
*
* @return bool
*/
private function hasLegacyPersonalSettingsToRender(array $forms): bool {
foreach ($forms as $form) {
if(trim($form) !== '') {
if (trim($form) !== '') {
return true;
}
}

81
tests/lib/Settings/ManagerTest.php
View File

@ -23,27 +23,20 @@
namespace Tests\Settings;
use OC\Accounts\AccountManager;
use OC\Settings\Admin\Sharing;
use OC\Settings\Manager;
use OC\Settings\Mapper;
use OC\Settings\Personal\Security;
use OC\Settings\Section;
use OCP\App\IAppManager;
use OCP\Encryption\IManager;
use OCP\IConfig;
use OCP\IDBConnection;
use OCP\IGroupManager;
use OCP\IL10N;
use OCP\ILogger;
use OCP\IRequest;
use OCP\IServerContainer;
use OCP\IURLGenerator;
use OCP\IUserManager;
use OCP\L10N\IFactory;
use OCP\Lock\ILockingProvider;
use Test\TestCase;
class ManagerTest extends TestCase {
/** @var Manager|\PHPUnit_Framework_MockObject_MockObject */
private $manager;
/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
@ -52,26 +45,10 @@ class ManagerTest extends TestCase {
private $dbConnection;
/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
private $l10n;
/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
private $config;
/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
private $encryptionManager;
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
private $userManager;
/** @var ILockingProvider|\PHPUnit_Framework_MockObject_MockObject */
private $lockingProvider;
/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
private $request;
/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
private $url;
/** @var AccountManager|\PHPUnit_Framework_MockObject_MockObject */
private $accountManager;
/** @var IGroupManager|\PHPUnit_Framework_MockObject_MockObject */
private $groupManager;
/** @var IFactory|\PHPUnit_Framework_MockObject_MockObject */
private $l10nFactory;
/** @var IAppManager */
private $appManager;
/** @var IServerContainer|\PHPUnit_Framework_MockObject_MockObject */
private $container;
public function setUp() {
parent::setUp();
@ -79,31 +56,15 @@ class ManagerTest extends TestCase {
$this->logger = $this->createMock(ILogger::class);
$this->dbConnection = $this->createMock(IDBConnection::class);
$this->l10n = $this->createMock(IL10N::class);
$this->config = $this->createMock(IConfig::class);
$this->encryptionManager = $this->createMock(IManager::class);
$this->userManager = $this->createMock(IUserManager::class);
$this->lockingProvider = $this->createMock(ILockingProvider::class);
$this->request = $this->createMock(IRequest::class);
$this->url = $this->createMock(IURLGenerator::class);
$this->accountManager = $this->createMock(AccountManager::class);
$this->groupManager = $this->createMock(IGroupManager::class);
$this->l10nFactory = $this->createMock(IFactory::class);
$this->appManager = $this->createMock(IAppManager::class);
$this->container = $this->createMock(IServerContainer::class);
$this->manager = new Manager(
$this->logger,
$this->dbConnection,
$this->l10n,
$this->config,
$this->encryptionManager,
$this->userManager,
$this->lockingProvider,
$this->request,
$this->url,
$this->accountManager,
$this->groupManager,
$this->l10nFactory,
$this->appManager
$this->container
);
}
@ -210,15 +171,37 @@ class ManagerTest extends TestCase {
}
public function testGetAdminSettings() {
$section = $this->createMock(Sharing::class);
$section->expects($this->once())
->method('getPriority')
->willReturn(13);
$this->container->expects($this->once())
->method('query')
->with(Sharing::class)
->willReturn($section);
$settings = $this->manager->getAdminSettings('sharing');
$this->assertEquals([
0 => [new Sharing($this->config, $this->l10n)],
], $this->manager->getAdminSettings('sharing'));
13 => [$section]
], $settings);
}
public function testGetPersonalSettings() {
$section = $this->createMock(Security::class);
$section->expects($this->once())
->method('getPriority')
->willReturn(16);
$this->container->expects($this->once())
->method('query')
->with(Security::class)
->willReturn($section);
$settings = $this->manager->getPersonalSettings('security');
$this->assertEquals([
10 => [new Security($this->userManager)],
], $this->manager->getPersonalSettings('security'));
16 => [$section]
], $settings);
}
public function testSameSectionAsPersonalAndAdmin() {