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

split db logic from settings manager and test them separately

2 files changed, 232 insertions, 136 deletions

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'));
+	}
+}