Merge pull request #2872 from nextcloud/admin-settings-split-db

split db logic from settings manager and test them separately

8 files changed, 459 insertions, 279 deletions

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 7a4cc6eaa84..84bd8c15e67 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -752,6 +752,7 @@ return array(
     'OC\\Settings\\Controller\\SecuritySettingsController' => $baseDir . '/settings/Controller/SecuritySettingsController.php',
     'OC\\Settings\\Controller\\UsersController' => $baseDir . '/settings/Controller/UsersController.php',
     'OC\\Settings\\Manager' => $baseDir . '/lib/private/Settings/Manager.php',
+    'OC\\Settings\\Mapper' => $baseDir . '/lib/private/Settings/Mapper.php',
     'OC\\Settings\\Middleware\\SubadminMiddleware' => $baseDir . '/settings/Middleware/SubadminMiddleware.php',
     'OC\\Settings\\RemoveOrphaned' => $baseDir . '/lib/private/Settings/RemoveOrphaned.php',
     'OC\\Settings\\Section' => $baseDir . '/lib/private/Settings/Section.php',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 6a063f0d8c7..1c7ef9b067b 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -782,6 +782,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Settings\\Controller\\SecuritySettingsController' => __DIR__ . '/../../..' . '/settings/Controller/SecuritySettingsController.php',
         'OC\\Settings\\Controller\\UsersController' => __DIR__ . '/../../..' . '/settings/Controller/UsersController.php',
         'OC\\Settings\\Manager' => __DIR__ . '/../../..' . '/lib/private/Settings/Manager.php',
+        'OC\\Settings\\Mapper' => __DIR__ . '/../../..' . '/lib/private/Settings/Mapper.php',
         'OC\\Settings\\Middleware\\SubadminMiddleware' => __DIR__ . '/../../..' . '/settings/Middleware/SubadminMiddleware.php',
         'OC\\Settings\\RemoveOrphaned' => __DIR__ . '/../../..' . '/lib/private/Settings/RemoveOrphaned.php',
         'OC\\Settings\\Section' => __DIR__ . '/../../..' . '/lib/private/Settings/Section.php',
diff --git a/lib/private/Server.php b/lib/private/Server.php
index 6f4d4f066e7..5bc72e3614f 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -791,7 +791,8 @@ class Server extends ServerContainer implements IServerContainer {
 				$c->getConfig(),
 				$c->getEncryptionManager(),
 				$c->getUserManager(),
-				$c->getLockingProvider()
+				$c->getLockingProvider(),
+				new \OC\Settings\Mapper($c->getDatabaseConnection())
 			);
 			return $manager;
 		});
diff --git a/lib/private/Settings/Admin/Encryption.php b/lib/private/Settings/Admin/Encryption.php
index 6e93407f1a3..63020c6bce7 100644
--- a/lib/private/Settings/Admin/Encryption.php
+++ b/lib/private/Settings/Admin/Encryption.php
@@ -23,23 +23,23 @@
 
 namespace OC\Settings\Admin;
 
-use OC\Encryption\Manager;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\Encryption\IManager;
 use OCP\IUserManager;
 use OCP\Settings\ISettings;
 
 class Encryption implements ISettings {
-	/** @var Manager */
+	/** @var IManager */
 	private $manager;
 
 	/** @var IUserManager */
 	private $userManager;
 
 	/**
-	 * @param Manager $manager
+	 * @param IManager $manager
 	 * @param IUserManager $userManager
 	 */
-	public function __construct(Manager $manager, IUserManager $userManager) {
+	public function __construct(IManager $manager, IUserManager $userManager) {
 		$this->manager = $manager;
 		$this->userManager = $userManager;
 	}
diff --git a/lib/private/Settings/Manager.php b/lib/private/Settings/Manager.php
index 990750848d3..949826aa246 100644
--- a/lib/private/Settings/Manager.php
+++ b/lib/private/Settings/Manager.php
@@ -43,6 +43,8 @@ class Manager implements IManager {
 	private $log;
 	/** @var IDBConnection */
 	private $dbc;
+	/** @var Mapper */
+	private $mapper;
 	/** @var IL10N */
 	private $l;
 	/** @var IConfig */
@@ -62,6 +64,8 @@ class Manager implements IManager {
 	 * @param EncryptionManager $encryptionManager
 	 * @param IUserManager $userManager
 	 * @param ILockingProvider $lockingProvider
+	 * @param Mapper $mapper
+	 * @internal param IDBConnection $dbc
 	 */
 	public function __construct(
 		ILogger $log,
@@ -70,10 +74,12 @@ class Manager implements IManager {
 		IConfig $config,
 		EncryptionManager $encryptionManager,
 		IUserManager $userManager,
-		ILockingProvider $lockingProvider
+		ILockingProvider $lockingProvider,
+		Mapper $mapper
 	) {
 		$this->log = $log;
 		$this->dbc = $dbc;
+		$this->mapper = $mapper;
 		$this->l = $l;
 		$this->config = $config;
 		$this->encryptionManager = $encryptionManager;
@@ -85,10 +91,10 @@ class Manager implements IManager {
 	 * @inheritdoc
 	 */
 	public function setupSettings(array $settings) {
-		if(isset($settings[IManager::KEY_ADMIN_SECTION])) {
+		if (isset($settings[IManager::KEY_ADMIN_SECTION])) {
 			$this->setupAdminSection($settings[IManager::KEY_ADMIN_SECTION]);
 		}
-		if(isset($settings[IManager::KEY_ADMIN_SETTINGS])) {
+		if (isset($settings[IManager::KEY_ADMIN_SETTINGS])) {
 			$this->setupAdminSettings($settings[IManager::KEY_ADMIN_SETTINGS]);
 		}
 	}
@@ -104,50 +110,33 @@ class Manager implements IManager {
 	public function onAppDisabled($appId) {
 		$appInfo = \OC_App::getAppInfo($appId); // hello static legacy
 
-		if(isset($appInfo['settings'][IManager::KEY_ADMIN_SECTION])) {
-			$this->remove(self::TABLE_ADMIN_SECTIONS, trim($appInfo['settings'][IManager::KEY_ADMIN_SECTION], '\\'));
+		if (isset($appInfo['settings'][IManager::KEY_ADMIN_SECTION])) {
+			$this->mapper->remove(self::TABLE_ADMIN_SECTIONS, trim($appInfo['settings'][IManager::KEY_ADMIN_SECTION], '\\'));
 		}
-		if(isset($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS])) {
-			$this->remove(self::TABLE_ADMIN_SETTINGS, trim($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS], '\\'));
+		if (isset($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS])) {
+			$this->mapper->remove(self::TABLE_ADMIN_SETTINGS, trim($appInfo['settings'][IManager::KEY_ADMIN_SETTINGS], '\\'));
 		}
 	}
 
 	public function checkForOrphanedClassNames() {
-		$tables = [ self::TABLE_ADMIN_SECTIONS, self::TABLE_ADMIN_SETTINGS ];
+		$tables = [self::TABLE_ADMIN_SECTIONS, self::TABLE_ADMIN_SETTINGS];
 		foreach ($tables as $table) {
-			$classes = $this->getClasses($table);
-			foreach($classes as $className) {
+			$classes = $this->mapper->getClasses($table);
+			foreach ($classes as $className) {
 				try {
 					\OC::$server->query($className);
 				} catch (QueryException $e) {
-					$this->remove($table, $className);
+					$this->mapper->remove($table, $className);
 				}
 			}
 		}
 	}
 
 	/**
-	 * returns the registerd classes in the given table
-	 *
-	 * @param $table
-	 * @return string[]
-	 */
-	private function getClasses($table) {
-		$q = $this->dbc->getQueryBuilder();
-		$resultStatement = $q->select('class')
-			->from($table)
-			->execute();
-		$data = $resultStatement->fetchAll();
-		$resultStatement->closeCursor();
-
-		return array_map(function($row) { return $row['class']; }, $data);
-	}
-
-	/**
 	 * @param string $sectionClassName
 	 */
 	private function setupAdminSection($sectionClassName) {
-		if(!class_exists($sectionClassName)) {
+		if (!class_exists($sectionClassName)) {
 			$this->log->debug('Could not find admin section class ' . $sectionClassName);
 			return;
 		}
@@ -158,14 +147,14 @@ class Manager implements IManager {
 			return;
 		}
 
-		if(!$section instanceof ISection) {
+		if (!$section instanceof ISection) {
 			$this->log->error(
 				'Admin section instance must implement \OCP\ISection. Invalid class: {class}',
 				['class' => $sectionClassName]
 			);
 			return;
 		}
-		if(!$this->hasAdminSection(get_class($section))) {
+		if (!$this->hasAdminSection(get_class($section))) {
 			$this->addAdminSection($section);
 		} else {
 			$this->updateAdminSection($section);
@@ -173,7 +162,7 @@ class Manager implements IManager {
 	}
 
 	private function addAdminSection(ISection $section) {
-		$this->add(self::TABLE_ADMIN_SECTIONS, [
+		$this->mapper->add(self::TABLE_ADMIN_SECTIONS, [
 			'id' => $section->getID(),
 			'class' => get_class($section),
 			'priority' => $section->getPriority(),
@@ -181,28 +170,15 @@ class Manager implements IManager {
 	}
 
 	private function addAdminSettings(ISettings $settings) {
-		$this->add(self::TABLE_ADMIN_SETTINGS, [
+		$this->mapper->add(self::TABLE_ADMIN_SETTINGS, [
 			'class' => get_class($settings),
 			'section' => $settings->getSection(),
 			'priority' => $settings->getPriority(),
 		]);
 	}
 
-	/**
-	 * @param string $table
-	 * @param array $values
-	 */
-	private function add($table, array $values) {
-		$query = $this->dbc->getQueryBuilder();
-		$values = array_map(function($value) use ($query) {
-			return $query->createNamedParameter($value);
-		}, $values);
-		$query->insert($table)->values($values);
-		$query->execute();
-	}
-
 	private function updateAdminSettings(ISettings $settings) {
-		$this->update(
+		$this->mapper->update(
 			self::TABLE_ADMIN_SETTINGS,
 			'class',
 			get_class($settings),
@@ -214,35 +190,23 @@ class Manager implements IManager {
 	}
 
 	private function updateAdminSection(ISection $section) {
-		$this->update(
+		$this->mapper->update(
 			self::TABLE_ADMIN_SECTIONS,
 			'class',
 			get_class($section),
 			[
-				'id'       => $section->getID(),
+				'id' => $section->getID(),
 				'priority' => $section->getPriority(),
 			]
 		);
 	}
 
-	private function update($table, $idCol, $id, $values) {
-		$query = $this->dbc->getQueryBuilder();
-		$query->update($table);
-		foreach($values as $key => $value) {
-			$query->set($key, $query->createNamedParameter($value));
-		}
-		$query
-			->where($query->expr()->eq($idCol, $query->createParameter($idCol)))
-			->setParameter($idCol, $id)
-			->execute();
-	}
-
 	/**
 	 * @param string $className
 	 * @return bool
 	 */
 	private function hasAdminSection($className) {
-		return $this->has(self::TABLE_ADMIN_SECTIONS, $className);
+		return $this->mapper->has(self::TABLE_ADMIN_SECTIONS, $className);
 	}
 
 	/**
@@ -250,44 +214,11 @@ class Manager implements IManager {
 	 * @return bool
 	 */
 	private function hasAdminSettings($className) {
-		return $this->has(self::TABLE_ADMIN_SETTINGS, $className);
-	}
-
-	/**
-	 * @param string $table
-	 * @param string $className
-	 * @return bool
-	 */
-	private function has($table, $className) {
-		$query = $this->dbc->getQueryBuilder();
-		$query->select('class')
-			->from($table)
-			->where($query->expr()->eq('class', $query->createNamedParameter($className)))
-			->setMaxResults(1);
-
-		$result = $query->execute();
-		$row = $result->fetch();
-		$result->closeCursor();
-
-		return (bool) $row;
-	}
-
-	/**
-	 * deletes an settings or admin entry from the given table
-	 *
-	 * @param $table
-	 * @param $className
-	 */
-	private function remove($table, $className) {
-		$query = $this->dbc->getQueryBuilder();
-		$query->delete($table)
-			->where($query->expr()->eq('class', $query->createNamedParameter($className)));
-
-		$query->execute();
+		return $this->mapper->has(self::TABLE_ADMIN_SETTINGS, $className);
 	}
 
 	private function setupAdminSettings($settingsClassName) {
-		if(!class_exists($settingsClassName)) {
+		if (!class_exists($settingsClassName)) {
 			$this->log->debug('Could not find admin section class ' . $settingsClassName);
 			return;
 		}
@@ -300,14 +231,14 @@ class Manager implements IManager {
 			return;
 		}
 
-		if(!$settings instanceof ISettings) {
+		if (!$settings instanceof ISettings) {
 			$this->log->error(
 				'Admin section instance must implement \OCP\Settings\ISection. Invalid class: {class}',
 				['class' => $settingsClassName]
 			);
 			return;
 		}
-		if(!$this->hasAdminSettings(get_class($settings))) {
+		if (!$this->hasAdminSettings(get_class($settings))) {
 			$this->addAdminSettings($settings);
 		} else {
 			$this->updateAdminSettings($settings);
@@ -329,24 +260,17 @@ class Manager implements IManager {
 	public function getAdminSections() {
 		// built-in sections
 		$sections = [
-			 0 => [new Section('server',        $this->l->t('Server settings'), 0)],
-			 5 => [new Section('sharing',       $this->l->t('Sharing'), 0)],
-			45 => [new Section('encryption',    $this->l->t('Encryption'), 0)],
-			98 => [new Section('additional',    $this->l->t('Additional settings'), 0)],
-			99 => [new Section('tips-tricks',   $this->l->t('Tips & tricks'), 0)],
+			0 => [new Section('server', $this->l->t('Server settings'), 0)],
+			5 => [new Section('sharing', $this->l->t('Sharing'), 0)],
+			45 => [new Section('encryption', $this->l->t('Encryption'), 0)],
+			98 => [new Section('additional', $this->l->t('Additional settings'), 0)],
+			99 => [new Section('tips-tricks', $this->l->t('Tips & tricks'), 0)],
 		];
 
-		$query = $this->dbc->getQueryBuilder();
-		$query->selectDistinct('s.class')
-			->addSelect('s.priority')
-			->from(self::TABLE_ADMIN_SECTIONS, 's')
-			->from(self::TABLE_ADMIN_SETTINGS, 'f')
-			->where($query->expr()->eq('s.id', 'f.section'))
-		;
-		$result = $query->execute();
+		$rows = $this->mapper->getAdminSectionsFromDB();
 
-		while($row = $result->fetch()) {
-			if(!isset($sections[$row['priority']])) {
+		foreach ($rows as $row) {
+			if (!isset($sections[$row['priority']])) {
 				$sections[$row['priority']] = [];
 			}
 			try {
@@ -355,38 +279,42 @@ class Manager implements IManager {
 				// skip
 			}
 		}
-		$result->closeCursor();
 
 		ksort($sections);
+
 		return $sections;
 	}
 
+	/**
+	 * @param string $section
+	 * @return ISection[]
+	 */
 	private function getBuiltInAdminSettings($section) {
 		$forms = [];
 		try {
-			if($section === 'server') {
+			if ($section === 'server') {
 				/** @var ISettings $form */
 				$form = new Admin\Server($this->dbc, $this->config, $this->lockingProvider, $this->l);
 				$forms[$form->getPriority()] = [$form];
 				$form = new Admin\ServerDevNotice();
 				$forms[$form->getPriority()] = [$form];
 			}
-			if($section === 'encryption') {
+			if ($section === 'encryption') {
 				/** @var ISettings $form */
 				$form = new Admin\Encryption($this->encryptionManager, $this->userManager);
 				$forms[$form->getPriority()] = [$form];
 			}
-			if($section === 'sharing') {
+			if ($section === 'sharing') {
 				/** @var ISettings $form */
 				$form = new Admin\Sharing($this->config);
 				$forms[$form->getPriority()] = [$form];
 			}
-			if($section === 'additional') {
+			if ($section === 'additional') {
 				/** @var ISettings $form */
 				$form = new Admin\Additional($this->config);
 				$forms[$form->getPriority()] = [$form];
 			}
-			if($section === 'tips-tricks') {
+			if ($section === 'tips-tricks') {
 				/** @var ISettings $form */
 				$form = new Admin\TipsTricks($this->config);
 				$forms[$form->getPriority()] = [$form];
@@ -397,16 +325,15 @@ class Manager implements IManager {
 		return $forms;
 	}
 
-	private function getAdminSettingsFromDB($section, &$settings) {
-		$query = $this->dbc->getQueryBuilder();
-		$query->select(['class', 'priority'])
-			->from(self::TABLE_ADMIN_SETTINGS)
-			->where($query->expr()->eq('section', $this->dbc->getQueryBuilder()->createParameter('section')))
-			->setParameter('section', $section);
+	/**
+	 * @inheritdoc
+	 */
+	public function getAdminSettings($section) {
+		$settings = $this->getBuiltInAdminSettings($section);
+		$dbRows = $this->mapper->getAdminSettingsFromDB($section);
 
-		$result = $query->execute();
-		while($row = $result->fetch()) {
-			if(!isset($settings[$row['priority']])) {
+		foreach ($dbRows as $row) {
+			if (!isset($settings[$row['priority']])) {
 				$settings[$row['priority']] = [];
 			}
 			try {
@@ -415,17 +342,8 @@ class Manager implements IManager {
 				// skip
 			}
 		}
-		$result->closeCursor();
 
 		ksort($settings);
-	}
-
-	/**
-	 * @inheritdoc
-	 */
-	public function getAdminSettings($section) {
-		$settings = $this->getBuiltInAdminSettings($section);
-		$this->getAdminSettingsFromDB($section, $settings);
 		return $settings;
 	}
 }
diff --git a/lib/private/Settings/Mapper.php b/lib/private/Settings/Mapper.php
new file mode 100644
index 00000000000..2525f2c9854
--- /dev/null
+++ b/lib/private/Settings/Mapper.php
@@ -0,0 +1,163 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016 Robin Appelman <robin@icewind.nl>
+ *
+ * @author Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OC\Settings;
+
+use OCP\IDBConnection;
+
+class Mapper {
+	const TABLE_ADMIN_SETTINGS = 'admin_settings';
+	const TABLE_ADMIN_SECTIONS = 'admin_sections';
+
+	/** @var IDBConnection */
+	private $dbc;
+
+	/**
+	 * @param IDBConnection $dbc
+	 */
+	public function __construct(IDBConnection $dbc) {
+		$this->dbc = $dbc;
+	}
+
+	/**
+	 * Get the configured admin settings from the database for the provided section
+	 *
+	 * @param string $section
+	 * @return array[] [['class' => string, 'priority' => int], ...]
+	 */
+	public function getAdminSettingsFromDB($section) {
+		$query = $this->dbc->getQueryBuilder();
+		$query->select(['class', 'priority'])
+			->from(self::TABLE_ADMIN_SETTINGS)
+			->where($query->expr()->eq('section', $this->dbc->getQueryBuilder()->createParameter('section')))
+			->setParameter('section', $section);
+
+		$result = $query->execute();
+		return $result->fetchAll();
+	}
+
+	/**
+	 * Get the configured admin sections from the database
+	 *
+	 * @return array[] [['class' => string, 'priority' => int], ...]
+	 */
+	public function getAdminSectionsFromDB() {
+		$query = $this->dbc->getQueryBuilder();
+		$query->selectDistinct('s.class')
+			->addSelect('s.priority')
+			->from(self::TABLE_ADMIN_SECTIONS, 's')
+			->from(self::TABLE_ADMIN_SETTINGS, 'f')
+			->where($query->expr()->eq('s.id', 'f.section'));
+		$result = $query->execute();
+		return array_map(function ($row) {
+			$row['priority'] = (int)$row['priority'];
+			return $row;
+		}, $result->fetchAll());
+	}
+
+	/**
+	 * @param string $table Mapper::TABLE_ADMIN_SECTIONS or Mapper::TABLE_ADMIN_SETTINGS
+	 * @param array $values
+	 */
+	public function add($table, array $values) {
+		$query = $this->dbc->getQueryBuilder();
+		$values = array_map(function ($value) use ($query) {
+			return $query->createNamedParameter($value);
+		}, $values);
+		$query->insert($table)->values($values);
+		$query->execute();
+	}
+
+	/**
+	 * returns the registered classes in the given table
+	 *
+	 * @param $table Mapper::TABLE_ADMIN_SECTIONS or Mapper::TABLE_ADMIN_SETTINGS
+	 * @return string[]
+	 */
+	public function getClasses($table) {
+		$q = $this->dbc->getQueryBuilder();
+		$resultStatement = $q->select('class')
+			->from($table)
+			->execute();
+		$data = $resultStatement->fetchAll();
+		$resultStatement->closeCursor();
+
+		return array_map(function ($row) {
+			return $row['class'];
+		}, $data);
+	}
+
+	/**
+	 * Check if a class is configured in the database
+	 *
+	 * @param string $table Mapper::TABLE_ADMIN_SECTIONS or Mapper::TABLE_ADMIN_SETTINGS
+	 * @param string $className
+	 * @return bool
+	 */
+	public function has($table, $className) {
+		$query = $this->dbc->getQueryBuilder();
+		$query->select('class')
+			->from($table)
+			->where($query->expr()->eq('class', $query->createNamedParameter($className)))
+			->setMaxResults(1);
+
+		$result = $query->execute();
+		$row = $result->fetch();
+		$result->closeCursor();
+
+		return (bool)$row;
+	}
+
+	/**
+	 * deletes an settings or admin entry from the given table
+	 *
+	 * @param $table Mapper::TABLE_ADMIN_SECTIONS or Mapper::TABLE_ADMIN_SETTINGS
+	 * @param $className
+	 */
+	public function remove($table, $className) {
+		$query = $this->dbc->getQueryBuilder();
+		$query->delete($table)
+			->where($query->expr()->eq('class', $query->createNamedParameter($className)));
+
+		$query->execute();
+	}
+
+	/**
+	 * @param $table Mapper::TABLE_ADMIN_SECTIONS or Mapper::TABLE_ADMIN_SETTINGS
+	 * @param $idCol
+	 * @param $id
+	 * @param $values
+	 */
+	public function update($table, $idCol, $id, $values) {
+		$query = $this->dbc->getQueryBuilder();
+		$query->update($table);
+		foreach ($values as $key => $value) {
+			$query->set($key, $query->createNamedParameter($value));
+		}
+		$query
+			->where($query->expr()->eq($idCol, $query->createParameter($idCol)))
+			->setParameter($idCol, $id)
+			->execute();
+	}
+
+}
diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php
index 150609499ad..b91331a1d30 100644
--- a/tests/lib/Settings/ManagerTest.php
+++ b/tests/lib/Settings/ManagerTest.php
@@ -25,6 +25,7 @@ namespace Tests\Settings;
 
 use OC\Settings\Admin\Sharing;
 use OC\Settings\Manager;
+use OC\Settings\Mapper;
 use OC\Settings\Section;
 use OCP\Encryption\IManager;
 use OCP\IConfig;
@@ -36,22 +37,24 @@ use OCP\Lock\ILockingProvider;
 use Test\TestCase;
 
 class ManagerTest extends TestCase {
-	/** @var Manager */
+	/** @var Manager|\PHPUnit_Framework_MockObject_MockObject */
 	private $manager;
-	/** @var ILogger */
+	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
 	private $logger;
-	/** @var IDBConnection */
+	/** @var IDBConnection|\PHPUnit_Framework_MockObject_MockObject */
 	private $dbConnection;
-	/** @var IL10N */
+	/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
 	private $l10n;
-	/** @var IConfig */
+	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
 	private $config;
-	/** @var IManager */
+	/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $encryptionManager;
-	/** @var IUserManager */
+	/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
 	private $userManager;
-	/** @var ILockingProvider */
+	/** @var ILockingProvider|\PHPUnit_Framework_MockObject_MockObject */
 	private $lockingProvider;
+	/** @var Mapper|\PHPUnit_Framework_MockObject_MockObject */
+	private $mapper;
 
 	public function setUp() {
 		parent::setUp();
@@ -63,6 +66,7 @@ class ManagerTest extends TestCase {
 		$this->encryptionManager = $this->getMockBuilder('\OCP\Encryption\IManager')->getMock();
 		$this->userManager = $this->getMockBuilder('\OCP\IUserManager')->getMock();
 		$this->lockingProvider = $this->getMockBuilder('\OCP\Lock\ILockingProvider')->getMock();
+		$this->mapper = $this->getMockBuilder(Mapper::class)->disableOriginalConstructor()->getMock();
 
 		$this->manager = new Manager(
 			$this->logger,
@@ -71,63 +75,49 @@ class ManagerTest extends TestCase {
 			$this->config,
 			$this->encryptionManager,
 			$this->userManager,
-			$this->lockingProvider
+			$this->lockingProvider,
+			$this->mapper
 		);
 	}
 
-	public function testSetupSettings() {
-		$qb = $this->getMockBuilder('\OCP\DB\QueryBuilder\IQueryBuilder')->getMock();
-		$qb
-			->expects($this->once())
-			->method('select')
-			->with('class')
-			->willReturn($qb);
-		$this->dbConnection
-			->expects($this->at(0))
-			->method('getQueryBuilder')
-			->willReturn($qb);
-		$qb
-			->expects($this->once())
-			->method('from')
-			->with('admin_settings')
-			->willReturn($qb);
-		$expressionBuilder = $this->getMockBuilder('\OCP\DB\QueryBuilder\IExpressionBuilder')->getMock();
-		$qb
-			->expects($this->once())
-			->method('expr')
-			->willReturn($expressionBuilder);
-		$param = $this->getMockBuilder('\OCP\DB\QueryBuilder\IParameter')->getMock();
-		$qb
-			->expects($this->once())
-			->method('createNamedParameter')
-			->with('OCA\Files\Settings\Admin')
-			->willReturn($param);
-		$expressionBuilder
-			->expects($this->once())
-			->method('eq')
-			->with('class', $param)
-			->willReturn('myString');
-		$qb
-			->expects($this->once())
-			->method('where')
-			->with('myString')
-			->willReturn($qb);
-		$stmt = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement')->getMock();
-		$qb
-			->expects($this->once())
-			->method('execute')
-			->willReturn($stmt);
-
-		$qb1 = $this->getMockBuilder('\OCP\DB\QueryBuilder\IQueryBuilder')->getMock();
-		$qb1
-			->expects($this->once())
-			->method('insert')
-			->with('admin_settings')
-			->willReturn($qb1);
-		$this->dbConnection
-			->expects($this->at(1))
-			->method('getQueryBuilder')
-			->willReturn($qb1);
+	public function testSetupSettingsUpdate() {
+		$this->mapper->expects($this->any())
+			->method('has')
+			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->will($this->returnValue(true));
+
+		$this->mapper->expects($this->once())
+			->method('update')
+			->with('admin_settings',
+				'class',
+				'OCA\Files\Settings\Admin', [
+					'section' => 'additional',
+					'priority' => 5
+				]);
+		$this->mapper->expects($this->never())
+			->method('add');
+
+		$this->manager->setupSettings([
+			'admin' => 'OCA\Files\Settings\Admin',
+		]);
+	}
+
+	public function testSetupSettingsAdd() {
+		$this->mapper->expects($this->any())
+			->method('has')
+			->with('admin_settings', 'OCA\Files\Settings\Admin')
+			->will($this->returnValue(false));
+
+		$this->mapper->expects($this->once())
+			->method('add')
+			->with('admin_settings', [
+				'class' => 'OCA\Files\Settings\Admin',
+				'section' => 'additional',
+				'priority' => 5
+			]);
+
+		$this->mapper->expects($this->never())
+			->method('update');
 
 		$this->manager->setupSettings([
 			'admin' => 'OCA\Files\Settings\Admin',
@@ -135,44 +125,49 @@ class ManagerTest extends TestCase {
 	}
 
 	public function testGetAdminSections() {
-		$qb = $this->getMockBuilder('\OCP\DB\QueryBuilder\IQueryBuilder')->getMock();
-		$expr = $this->getMockBuilder('OCP\DB\QueryBuilder\IExpressionBuilder')->getMock();
-		$qb
-			->expects($this->once())
-			->method('selectDistinct')
-			->with('s.class')
-			->willReturn($qb);
-		$qb
-			->expects($this->once())
-			->method('addSelect')
-			->with('s.priority')
-			->willReturn($qb);
-		$qb
-			->expects($this->exactly(2))
-			->method('from')
-			->willReturn($qb);
-		$qb
-			->expects($this->once())
-			->method('expr')
-			->willReturn($expr);
-		$qb
-			->expects($this->once())
-			->method('where')
-			->willReturn($qb);
-		$stmt = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement')->getMock();
-		$qb
-			->expects($this->once())
-			->method('execute')
-			->willReturn($stmt);
-		$this->dbConnection
-			->expects($this->once())
-			->method('getQueryBuilder')
-			->willReturn($qb);
 		$this->l10n
 			->expects($this->any())
 			->method('t')
 			->will($this->returnArgument(0));
 
+		$this->mapper->expects($this->once())
+			->method('getAdminSectionsFromDB')
+			->will($this->returnValue([
+				['class' => '\OCA\LogReader\Settings\Section', 'priority' => 90]
+			]));
+
+		$this->mapper->expects($this->once())
+			->method('getAdminSettingsCountFromDB')
+			->will($this->returnValue([
+				'logging' => 1
+			]));
+
+		$this->assertEquals([
+			0 => [new Section('server', 'Server settings', 0)],
+			5 => [new Section('sharing', 'Sharing', 0)],
+			45 => [new Section('encryption', 'Encryption', 0)],
+			90 => [new \OCA\LogReader\Settings\Section(\OC::$server->getL10N('logreader'))],
+			98 => [new Section('additional', 'Additional settings', 0)],
+			99 => [new Section('tips-tricks', 'Tips & tricks', 0)],
+		], $this->manager->getAdminSections());
+	}
+
+	public function testGetAdminSectionsEmptySection() {
+		$this->l10n
+			->expects($this->any())
+			->method('t')
+			->will($this->returnArgument(0));
+
+		$this->mapper->expects($this->once())
+			->method('getAdminSectionsFromDB')
+			->will($this->returnValue([
+				['class' => '\OCA\LogReader\Settings\Section', 'priority' => 90]
+			]));
+
+		$this->mapper->expects($this->once())
+			->method('getAdminSettingsCountFromDB')
+			->will($this->returnValue([]));
+
 		$this->assertEquals([
 			0 => [new Section('server', 'Server settings', 0)],
 			5 => [new Section('sharing', 'Sharing', 0)],
@@ -183,47 +178,9 @@ class ManagerTest extends TestCase {
 	}
 
 	public function testGetAdminSettings() {
-		$qb = $this->getMockBuilder('\OCP\DB\QueryBuilder\IQueryBuilder')->getMock();
-		$qb
-			->expects($this->once())
-			->method('select')
-			->with(['class', 'priority'])
-			->willReturn($qb);
-		$qb
-			->expects($this->once())
-			->method('from')
-			->with('admin_settings')
-			->willReturn($qb);
-		$expressionBuilder = $this->getMockBuilder('\OCP\DB\QueryBuilder\IExpressionBuilder')->getMock();
-		$qb
-			->expects($this->once())
-			->method('expr')
-			->willReturn($expressionBuilder);
-		$param = $this->getMockBuilder('\OCP\DB\QueryBuilder\IParameter')->getMock();
-		$qb
-			->expects($this->once())
-			->method('createParameter')
-			->with('section')
-			->willReturn($param);
-		$expressionBuilder
-			->expects($this->once())
-			->method('eq')
-			->with('section', $param)
-			->willReturn('myString');
-		$qb
-			->expects($this->once())
-			->method('where')
-			->with('myString')
-			->willReturn($qb);
-		$stmt = $this->getMockBuilder('\Doctrine\DBAL\Driver\Statement')->getMock();
-		$qb
-			->expects($this->once())
-			->method('execute')
-			->willReturn($stmt);
-		$this->dbConnection
-			->expects($this->exactly(2))
-			->method('getQueryBuilder')
-			->willReturn($qb);
+		$this->mapper->expects($this->any())
+			->method('getAdminSettingsFromDB')
+			->will($this->returnValue([]));
 
 		$this->assertEquals([
 			0 => [new Sharing($this->config)],
diff --git a/tests/lib/Settings/MapperTest.php b/tests/lib/Settings/MapperTest.php
new file mode 100644
index 00000000000..6a648acd5f7
--- /dev/null
+++ b/tests/lib/Settings/MapperTest.php
@@ -0,0 +1,139 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016 Robin Appelman <robin@icewind.nl>
+ *
+ * @author Robin Appelman <robin@icewind.nl>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace Tests\Settings;
+
+use OC\DB\QueryBuilder\Literal;
+use OC\Settings\Mapper;
+use Test\TestCase;
+
+/**
+ * @group DB
+ */
+class MapperTest extends TestCase {
+	const SECTION_PREFIX = 'test_section_';
+
+	/** @var Mapper */
+	private $mapper;
+
+	public function setUp() {
+		parent::setUp();
+		$this->mapper = new Mapper(\OC::$server->getDatabaseConnection());
+	}
+
+	public function tearDown() {
+		parent::tearDown();
+
+		$db = \OC::$server->getDatabaseConnection();
+		$builder = $db->getQueryBuilder();
+
+		$builder->delete(Mapper::TABLE_ADMIN_SECTIONS)
+			->where($builder->expr()->like('id', new Literal(self::SECTION_PREFIX . '%')));
+
+		$builder->delete(Mapper::TABLE_ADMIN_SETTINGS)
+			->where($builder->expr()->like('section', new Literal(self::SECTION_PREFIX . '%')));
+	}
+
+	public function testManipulateSettings() {
+		$this->assertEquals(false, $this->mapper->has(Mapper::TABLE_ADMIN_SETTINGS, '\OC\Dummy'));
+		$this->assertNotContains('\OC\Dummy', $this->mapper->getClasses(Mapper::TABLE_ADMIN_SETTINGS));
+
+		$this->mapper->add(Mapper::TABLE_ADMIN_SETTINGS, [
+			'class' => '\OC\Dummy',
+			'section' => self::SECTION_PREFIX . '1',
+			'priority' => 5
+		]);
+
+		$this->assertEquals(true, $this->mapper->has(Mapper::TABLE_ADMIN_SETTINGS, '\OC\Dummy'));
+
+		$this->assertContains('\OC\Dummy', $this->mapper->getClasses(Mapper::TABLE_ADMIN_SETTINGS));
+
+		$rows = $this->mapper->getAdminSettingsFromDB(self::SECTION_PREFIX . '1');
+		$this->assertEquals([
+			['class' => '\OC\Dummy', 'priority' => 5]
+		], $rows);
+
+		$this->mapper->update(Mapper::TABLE_ADMIN_SETTINGS, 'class', '\OC\Dummy', [
+			'section' => self::SECTION_PREFIX . '1', 'priority' => 15
+		]);
+
+		$rows = $this->mapper->getAdminSettingsFromDB(self::SECTION_PREFIX . '1');
+		$this->assertEquals([
+			['class' => '\OC\Dummy', 'priority' => 15]
+		], $rows);
+
+		$this->mapper->update(Mapper::TABLE_ADMIN_SETTINGS, 'class', '\OC\Dummy', [
+			'section' => self::SECTION_PREFIX . '2', 'priority' => 15
+		]);
+
+		$this->assertEquals([], $this->mapper->getAdminSettingsFromDB(self::SECTION_PREFIX . '1'));
+		$rows = $this->mapper->getAdminSettingsFromDB(self::SECTION_PREFIX . '2');
+		$this->assertEquals([
+			['class' => '\OC\Dummy', 'priority' => 15]
+		], $rows);
+
+		$this->mapper->remove(Mapper::TABLE_ADMIN_SETTINGS, '\OC\Dummy');
+
+		$this->assertEquals(false, $this->mapper->has(Mapper::TABLE_ADMIN_SETTINGS, '\OC\Dummy'));
+	}
+
+	public function testGetAdminSections() {
+		$this->assertFalse($this->mapper->has(Mapper::TABLE_ADMIN_SECTIONS, '\OC\Dummy'));
+
+		$this->mapper->add(Mapper::TABLE_ADMIN_SECTIONS, [
+			'id' => self::SECTION_PREFIX . '1',
+			'class' => '\OC\Dummy',
+			'priority' => 1,
+		]);
+
+		$this->assertTrue($this->mapper->has(Mapper::TABLE_ADMIN_SECTIONS, '\OC\Dummy'));
+
+		// until we add a setting for the section it's not returned
+		$this->assertNotContains([
+			'class' => '\OC\Dummy',
+			'priority' => 1,
+		], $this->mapper->getAdminSectionsFromDB());
+
+		$this->mapper->add(Mapper::TABLE_ADMIN_SETTINGS, [
+			'class' => '\OC\Dummy',
+			'section' => self::SECTION_PREFIX . '1',
+			'priority' => 5
+		]);
+
+		$this->assertContains([
+			'class' => '\OC\Dummy',
+			'priority' => 1,
+		], $this->mapper->getAdminSectionsFromDB());
+
+		$this->mapper->remove(Mapper::TABLE_ADMIN_SETTINGS, '\OC\Dummy');
+
+		$this->assertNotContains([
+			'class' => '\OC\Dummy',
+			'priority' => 1,
+		], $this->mapper->getAdminSectionsFromDB());
+
+		$this->mapper->remove(Mapper::TABLE_ADMIN_SECTIONS, '\OC\Dummy');
+
+		$this->assertFalse($this->mapper->has(Mapper::TABLE_ADMIN_SECTIONS, '\OC\Dummy'));
+	}
+}