add missing pieces to Settings Manager and fix and extend its unit tests

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

lib/private/Settings/Manager.php

@@ -464,6 +464,22 @@ class Manager implements IManager {
 			15 => [new Section('sync-clients', $this->l->t('Sync clients'), 0, $this->url->imagePath('settings', 'change.svg'))],
 			98 => [new Section('additional', $this->l->t('Additional settings'), 0, $this->url->imagePath('core', 'actions/settings-dark.svg'))],
 		];
+
+		$rows = $this->mapper->getPersonalSectionsFromDB();
+
+		foreach ($rows as $row) {
+			if (!isset($sections[$row['priority']])) {
+				$sections[$row['priority']] = [];
+			}
+			try {
+				$sections[$row['priority']][] = $this->query($row['class']);
+			} catch (QueryException $e) {
+				// skip
+			}
+		}
+
+		ksort($sections);
+
 		return $sections;
 	}
 
@@ -472,6 +488,20 @@ class Manager implements IManager {
 	 */
 	public function getPersonalSettings($section) {
 		$settings = $this->getBuiltInPersonalSettings($section);
+		$dbRows = $this->mapper->getPersonalSettingsFromDB($section);
+
+		foreach ($dbRows as $row) {
+			if (!isset($settings[$row['priority']])) {
+				$settings[$row['priority']] = [];
+			}
+			try {
+				$settings[$row['priority']][] = $this->query($row['class']);
+			} catch (QueryException $e) {
+				// skip
+			}
+		}
+
+		ksort($settings);
 		return $settings;
 	}
 }

lib/private/Settings/Mapper.php

@@ -85,11 +85,39 @@ class Mapper {
 	 * @return array[] [['class' => string, 'priority' => int], ...]
 	 */
 	public function getAdminSectionsFromDB() {
+		return $this->getSectionsFromDB('admin');
+	}
+
+	/**
+	 * Get the configured admin sections from the database
+	 *
+	 * @return array[] [['class' => string, 'priority' => int], ...]
+	 */
+	public function getPersonalSectionsFromDB() {
+		return $this->getSectionsFromDB('personal');
+	}
+
+	/**
+	 * Get the configured sections from the database by table
+	 *
+	 * @param string $type either 'personal' or 'admin'
+	 * @return array[] [['class' => string, 'priority' => int], ...]
+	 */
+	public function getSectionsFromDB($type) {
+		if($type === 'personal') {
+			$sectionsTable = self::TABLE_ADMIN_SECTIONS;
+			$settingsTable = self::TABLE_ADMIN_SETTINGS;
+		} else if($type === 'admin') {
+			$sectionsTable = self::TABLE_PERSONAL_SECTIONS;
+			$settingsTable = self::TABLE_PERSONAL_SETTINGS;
+		} else {
+			throw new \InvalidArgumentException('"admin" or "personal" expected');
+		}
 		$query = $this->dbc->getQueryBuilder();
 		$query->selectDistinct('s.class')
 			->addSelect('s.priority')
-			->from(self::TABLE_ADMIN_SECTIONS, 's')
-			->from(self::TABLE_ADMIN_SETTINGS, 'f')
+			->from($sectionsTable, 's')
+			->from($settingsTable, 'f')
 			->where($query->expr()->eq('s.id', 'f.section'));
 		$result = $query->execute();
 		return array_map(function ($row) {

tests/lib/Settings/ManagerTest.php

@@ -23,18 +23,22 @@
 
 namespace Tests\Settings;
 
+use OC\Accounts\AccountManager;
 use OC\Settings\Admin\Sharing;
 use OC\Settings\Manager;
 use OC\Settings\Mapper;
+use OC\Settings\Personal\AppPasswords;
 use OC\Settings\Section;
 use OCP\Encryption\IManager;
 use OCP\IConfig;
 use OCP\IDBConnection;
+use OCP\IGroupManager;
 use OCP\IL10N;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IURLGenerator;
 use OCP\IUserManager;
+use OCP\L10N\IFactory;
 use OCP\Lock\ILockingProvider;
 use Test\TestCase;
 
@@ -61,6 +65,14 @@ class ManagerTest extends TestCase {
 	private $mapper;
 	/** @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 \OC_Defaults|\PHPUnit_Framework_MockObject_MockObject */
+	private $defaults;
 
 	public function setUp() {
 		parent::setUp();
@@ -75,6 +87,10 @@ class ManagerTest extends TestCase {
 		$this->request = $this->createMock(IRequest::class);
 		$this->mapper = $this->createMock(Mapper::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->defaults = $this->createMock(\OC_Defaults::class);
 
 		$this->manager = new Manager(
 			$this->logger,
@@ -86,21 +102,39 @@ class ManagerTest extends TestCase {
 			$this->lockingProvider,
 			$this->request,
 			$this->mapper,
-			$this->url
+			$this->url,
+			$this->accountManager,
+			$this->groupManager,
+			$this->l10nFactory,
+			$this->defaults
 		);
 	}
 
-	public function testSetupSettingsUpdate() {
+	public function settingsTypeProvider() {
+		return [
+			['admin', 'admin_settings'],
+			['personal', 'personal_settings'],
+		];
+	}
+
+	/**
+	 * @dataProvider settingsTypeProvider
+	 * @param string $type
+	 * @param string $table
+	 */
+	public function testSetupSettingsUpdate($type, $table) {
+		$className = 'OCA\Files\Settings\Admin';
+
 		$this->mapper->expects($this->any())
 			->method('has')
-			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->with($table, $className)
 			->will($this->returnValue(true));
 
 		$this->mapper->expects($this->once())
 			->method('update')
-			->with('admin_settings',
+			->with($table,
 				'class',
-				'OCA\Files\Settings\Admin', [
+				$className, [
 					'section' => 'additional',
 					'priority' => 5
 				]);
@@ -108,19 +142,24 @@ class ManagerTest extends TestCase {
 			->method('add');
 
 		$this->manager->setupSettings([
-			'admin' => 'OCA\Files\Settings\Admin',
+			$type => $className,
 		]);
 	}
 
-	public function testSetupSettingsAdd() {
+	/**
+	 * @dataProvider settingsTypeProvider
+	 * @param string $type
+	 * @param string $table
+	 */
+	public function testSetupSettingsAdd($type, $table) {
 		$this->mapper->expects($this->any())
 			->method('has')
-			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->with($table, 'OCA\Files\Settings\Admin')
 			->will($this->returnValue(false));
 
 		$this->mapper->expects($this->once())
 			->method('add')
-			->with('admin_settings', [
+			->with($table, [
 				'class' => 'OCA\Files\Settings\Admin',
 				'section' => 'additional',
 				'priority' => 5
@@ -130,7 +169,7 @@ class ManagerTest extends TestCase {
 			->method('update');
 
 		$this->manager->setupSettings([
-			'admin' => 'OCA\Files\Settings\Admin',
+			$type => 'OCA\Files\Settings\Admin',
 		]);
 	}
 
@@ -167,6 +206,38 @@ class ManagerTest extends TestCase {
 		], $this->manager->getAdminSections());
 	}
 
+	public function testGetPersonalSections() {
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnArgument(0));
+
+		$this->mapper->expects($this->once())
+			->method('getPersonalSectionsFromDB')
+			->will($this->returnValue([
+				['class' => \OCA\WorkflowEngine\Settings\Section::class, 'priority' => 90]
+			]));
+
+		$this->url->expects($this->exactly(5))
+			->method('imagePath')
+			->willReturnMap([
+				['core', 'actions/info.svg', '1'],
+				['settings', 'admin.svg', '2'],
+				['settings', 'password.svg', '3'],
+				['settings', 'change.svg', '4'],
+				['core', 'actions/settings-dark.svg', '5'],
+			]);
+
+		$this->assertEquals([
+			0 => [new Section('personal-info', 'Personal info', 0, '1')],
+			5 => [new Section('sessions', 'Sessions', 0, '2')],
+			10 => [new Section('app-passwords', 'App passwords', 0, '3')],
+			15 => [new Section('sync-clients', 'Sync clients', 0, '4')],
+			90 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
+			98 => [new Section('additional', 'Additional settings', 0, '5')],
+		], $this->manager->getPersonalSections());
+	}
+
 	public function testGetAdminSectionsEmptySection() {
 		$this->l10n
 			->expects($this->any())
@@ -198,6 +269,35 @@ class ManagerTest extends TestCase {
 		], $this->manager->getAdminSections());
 	}
 
+	public function testGetPersonalSectionsEmptySection() {
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnArgument(0));
+
+		$this->mapper->expects($this->once())
+			->method('getPersonalSectionsFromDB')
+			->will($this->returnValue([]));
+
+		$this->url->expects($this->exactly(5))
+			->method('imagePath')
+			->willReturnMap([
+				['core', 'actions/info.svg', '1'],
+				['settings', 'admin.svg', '2'],
+				['settings', 'password.svg', '3'],
+				['settings', 'change.svg', '4'],
+				['core', 'actions/settings-dark.svg', '5'],
+			]);
+
+		$this->assertEquals([
+			0 => [new Section('personal-info', 'Personal info', 0, '1')],
+			5 => [new Section('sessions', 'Sessions', 0, '2')],
+			10 => [new Section('app-passwords', 'App passwords', 0, '3')],
+			15 => [new Section('sync-clients', 'Sync clients', 0, '4')],
+			98 => [new Section('additional', 'Additional settings', 0, '5')],
+		], $this->manager->getPersonalSections());
+	}
+
 	public function testGetAdminSettings() {
 		$this->mapper->expects($this->any())
 			->method('getAdminSettingsFromDB')
@@ -207,4 +307,14 @@ class ManagerTest extends TestCase {
 			0 => [new Sharing($this->config)],
 		], $this->manager->getAdminSettings('sharing'));
 	}
+
+	public function testGetPersonalSettings() {
+		$this->mapper->expects($this->any())
+			->method('getPersonalSettingsFromDB')
+			->will($this->returnValue([]));
+
+		$this->assertEquals([
+			5 => [new AppPasswords()],
+		], $this->manager->getPersonalSettings('app-passwords'));
+	}
 }