Merge pull request #8096 from nextcloud/feature/6382/do-not-cache-settings-in-db-anymore

Do not cache the settings/sections in the database anymore

2 files changed, 5 insertions, 240 deletions

diff --git a/tests/lib/Settings/ManagerTest.php b/tests/lib/Settings/ManagerTest.php
index be5aad2e414..577abc7915c 100644
--- a/tests/lib/Settings/ManagerTest.php
+++ b/tests/lib/Settings/ManagerTest.php
@@ -62,8 +62,6 @@ class ManagerTest extends TestCase {
 	private $lockingProvider;
 	/** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */
 	private $request;
-	/** @var Mapper|\PHPUnit_Framework_MockObject_MockObject */
-	private $mapper;
 	/** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */
 	private $url;
 	/** @var AccountManager|\PHPUnit_Framework_MockObject_MockObject */
@@ -72,8 +70,6 @@ class ManagerTest extends TestCase {
 	private $groupManager;
 	/** @var  IFactory|\PHPUnit_Framework_MockObject_MockObject */
 	private $l10nFactory;
-	/** @var \OC_Defaults|\PHPUnit_Framework_MockObject_MockObject */
-	private $defaults;
 	/** @var IAppManager */
 	private $appManager;
 
@@ -88,12 +84,10 @@ class ManagerTest extends TestCase {
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->lockingProvider = $this->createMock(ILockingProvider::class);
 		$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->appManager = $this->createMock(IAppManager::class);
 
 		$this->manager = new Manager(
@@ -105,90 +99,21 @@ class ManagerTest extends TestCase {
 			$this->userManager,
 			$this->lockingProvider,
 			$this->request,
-			$this->mapper,
 			$this->url,
 			$this->accountManager,
 			$this->groupManager,
 			$this->l10nFactory,
-			$this->defaults,
 			$this->appManager
 		);
 	}
 
-	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($table, $className)
-			->will($this->returnValue(true));
-
-		$this->mapper->expects($this->once())
-			->method('update')
-			->with($table,
-				'class',
-				$className, [
-					'section' => 'additional',
-					'priority' => 5
-				]);
-		$this->mapper->expects($this->never())
-			->method('add');
-
-		$this->manager->setupSettings([
-			$type => [$className],
-		]);
-	}
-
-	/**
-	 * @dataProvider settingsTypeProvider
-	 * @param string $type
-	 * @param string $table
-	 */
-	public function testSetupSettingsAdd($type, $table) {
-		$this->mapper->expects($this->any())
-			->method('has')
-			->with($table, 'OCA\Files\Settings\Admin')
-			->will($this->returnValue(false));
-
-		$this->mapper->expects($this->once())
-			->method('add')
-			->with($table, [
-				'class' => 'OCA\Files\Settings\Admin',
-				'section' => 'additional',
-				'priority' => 5
-			]);
-
-		$this->mapper->expects($this->never())
-			->method('update');
-
-		$this->manager->setupSettings([
-			$type => ['OCA\Files\Settings\Admin'],
-		]);
-	}
-
 	public function testGetAdminSections() {
 		$this->l10n
 			->expects($this->any())
 			->method('t')
 			->will($this->returnArgument(0));
 
-		$this->mapper->expects($this->once())
-			->method('getAdminSectionsFromDB')
-			->will($this->returnValue([
-				['class' => \OCA\WorkflowEngine\Settings\Section::class, 'priority' => 90]
-			]));
+		$this->manager->registerSection('admin', \OCA\WorkflowEngine\Settings\Section::class);
 
 		$this->url->expects($this->exactly(6))
 			->method('imagePath')
@@ -205,7 +130,7 @@ class ManagerTest extends TestCase {
 			5 => [new Section('sharing', 'Sharing', 0, '2')],
 			10 => [new Section('security', 'Security', 0, '3')],
 			45 => [new Section('encryption', 'Encryption', 0, '3')],
-			90 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
+			55 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
 			98 => [new Section('additional', 'Additional settings', 0, '4')],
 			99 => [new Section('tips-tricks', 'Tips & tricks', 0, '5')],
 		], $this->manager->getAdminSections());
@@ -217,11 +142,7 @@ class ManagerTest extends TestCase {
 			->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->manager->registerSection('personal', \OCA\WorkflowEngine\Settings\Section::class);
 
 		$this->url->expects($this->exactly(3))
 			->method('imagePath')
@@ -231,11 +152,11 @@ class ManagerTest extends TestCase {
 				['settings', 'change.svg', '3'],
 			]);
 
-		$this->assertArraySubset([
+		$this->assertEquals([
 			0 => [new Section('personal-info', 'Personal info', 0, '1')],
 			5 => [new Section('security', 'Security', 0, '2')],
 			15 => [new Section('sync-clients', 'Sync clients', 0, '3')],
-			90 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
+			55 => [\OC::$server->query(\OCA\WorkflowEngine\Settings\Section::class)],
 		], $this->manager->getPersonalSections());
 	}
 
@@ -245,11 +166,6 @@ class ManagerTest extends TestCase {
 			->method('t')
 			->will($this->returnArgument(0));
 
-		$this->mapper->expects($this->once())
-			->method('getAdminSectionsFromDB')
-			->will($this->returnValue([
-			]));
-
 		$this->url->expects($this->exactly(6))
 			->method('imagePath')
 			->willReturnMap([
@@ -276,10 +192,6 @@ class ManagerTest extends TestCase {
 			->method('t')
 			->will($this->returnArgument(0));
 
-		$this->mapper->expects($this->once())
-			->method('getPersonalSectionsFromDB')
-			->will($this->returnValue([]));
-
 		$this->url->expects($this->exactly(3))
 			->method('imagePath')
 			->willReturnMap([
@@ -296,20 +208,12 @@ class ManagerTest extends TestCase {
 	}
 
 	public function testGetAdminSettings() {
-		$this->mapper->expects($this->any())
-			->method('getAdminSettingsFromDB')
-			->will($this->returnValue([]));
-
 		$this->assertEquals([
 			0 => [new Sharing($this->config)],
 		], $this->manager->getAdminSettings('sharing'));
 	}
 
 	public function testGetPersonalSettings() {
-		$this->mapper->expects($this->any())
-			->method('getPersonalSettingsFromDB')
-			->will($this->returnValue([]));
-
 		$this->assertEquals([
 			10 => [new Security()],
 		], $this->manager->getPersonalSettings('security'));
diff --git a/tests/lib/Settings/MapperTest.php b/tests/lib/Settings/MapperTest.php
deleted file mode 100644
index 6a648acd5f7..00000000000
--- a/tests/lib/Settings/MapperTest.php
+++ /dev/null
@@ -1,139 +0,0 @@
-<?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'));
-	}
-}