From d8129b5a1260e33de52ab390d64722e4fdd9f9ef Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Sun, 20 Mar 2022 18:21:52 +0100 Subject: [PATCH 1/6] Create config abstraction for user configurations Signed-off-by: Christian Wolf --- lib/Helper/UserConfigHelper.php | 149 +++++++++++++++++++++ tests/Unit/Helper/UserConfigHelperTest.php | 141 +++++++++++++++++++ 2 files changed, 290 insertions(+) create mode 100644 lib/Helper/UserConfigHelper.php create mode 100644 tests/Unit/Helper/UserConfigHelperTest.php diff --git a/lib/Helper/UserConfigHelper.php b/lib/Helper/UserConfigHelper.php new file mode 100644 index 000000000..687b2fb36 --- /dev/null +++ b/lib/Helper/UserConfigHelper.php @@ -0,0 +1,149 @@ +userId = $UserId; + $this->config = $config; + $this->l = $l; + } + + /** + * Get a config value from the database + * + * @param string $key The key to get + * @return string The resulting value or '' if the key was not found + */ + private function getRawValue(string $key): string { + return $this->config->getUserValue($this->userId, 'cookbook', $key); + } + + /** + * Set a config value in the database + * + * @param string $key The key of the configuration + * @param string $value The value of the config entry + * @return void + */ + private function setRawValue(string $key, string $value): void { + $this->config->setUserValue($this->userId, 'cookbook', $key, $value); + } + + /** + * Get the timestamp of the last rescan of the library + * + * @return integer The timestamp of the last index rebuild + */ + public function getLastIndexUpdate(): int { + $rawValue = $this->getRawValue('last_index_update'); + if ($rawValue === '') { + return 0; + } + + return intval($rawValue); + } + + /** + * Set the timestamp of the last rescan of the library + * + * @param integer $value The timestamp of the last index rebuild + * @return void + */ + public function setLastIndexUpdate(int $value): void { + $this->setRawValue('last_index_update', strval($value)); + } + + /** + * Get the number of seconds between rescans of the library + * + * @return integer The number of seconds to wait before a new rescan is triggered + */ + public function getUpdateInterval(): int { + $rawValue = $this->getRawValue('update_interval'); + if ($rawValue === '') { + return 60 * 10; + } + + return intval($rawValue); + } + + /** + * Set the interval between the rescan events of the complete library + * + * @param integer $value The number of seconds to wait at least between rescans + * @return void + */ + public function setUpdateInterval(int $value): void { + $this->setRawValue('update_interval', $value); + } + + /** + * Check if the primary imgae should be printed or not + * + * @return boolean true, if the imgae shoudl be printed + */ + public function getPrintImage(): bool { + $rawValue = $this->getRawValue('print_image'); + if ($rawValue === '') { + return true; + } + return $rawValue === '1'; + } + + /** + * Set if the imgae should be printed + * + * @param boolean $value true if the image should be printed + * @return void + */ + public function setPrintImage(bool $value): void { + if ($value) { + $this->setRawValue('print_image', '1'); + } else { + $this->setRawValue('print_image', '0'); + } + } + + /** + * Get the name of the default cookbook. + * + * If no folder is stored in the config yet, a default setting will be generated and saved. + * + * @return string The name of the folder within the users files + */ + public function getFolderName(): string { + $rawValue = $this->getRawValue('folder'); + if ($rawValue === '') { + $path = '/' . $this->l->t('Recipes'); + $this->setFolderName($path); + return $path; + } + + return $rawValue; + } + + /** + * Set the folder for the user's cookbook. + * + * @param string $value The name of the folder withon the user's files + * @return void + */ + public function setFolderName(string $value): void { + $this->setRawValue('folder', $value); + } +} diff --git a/tests/Unit/Helper/UserConfigHelperTest.php b/tests/Unit/Helper/UserConfigHelperTest.php new file mode 100644 index 000000000..02d9ab38f --- /dev/null +++ b/tests/Unit/Helper/UserConfigHelperTest.php @@ -0,0 +1,141 @@ +userId = 'myuserid'; + + $this->config = $this->createMock(IConfig::class); + + $this->l = $this->createStub(IL10N::class); + $this->l->method('t')->willReturnArgument(0); + + $this->dut = new UserConfigHelper($this->userId, $this->config, $this->l); + } + + public function testLastIndexUpdate() { + $originalValue = 10; + $saveValue = 20; + + $this->config->expects($this->exactly(2))->method('getUserValue') + ->with($this->userId, 'cookbook', 'last_index_update') + ->willReturnOnConsecutiveCalls(strval($originalValue), strval($saveValue)); + + $this->config->expects($this->once())->method('setUserValue') + ->with($this->userId, 'cookbook', 'last_index_update', strval($saveValue)); + + $this->assertEquals($originalValue, $this->dut->getLastIndexUpdate()); + $this->dut->setLastIndexUpdate($saveValue); + $this->assertEquals($saveValue, $this->dut->getLastIndexUpdate()); + } + + public function testLastIndexUpdateUnset() { + $this->config->expects($this->once())->method('getUserValue') + ->with($this->userId, 'cookbook', 'last_index_update') + ->willReturn(''); + + $this->assertEquals(0, $this->dut->getLastIndexUpdate()); + } + + public function testUpdateInterval() { + $originalValue = 10; + $saveValue = 20; + + $this->config->expects($this->exactly(2))->method('getUserValue') + ->with($this->userId, 'cookbook', 'update_interval') + ->willReturnOnConsecutiveCalls(strval($originalValue), strval($saveValue)); + + $this->config->expects($this->once())->method('setUserValue') + ->with($this->userId, 'cookbook', 'update_interval', strval($saveValue)); + + $this->assertEquals($originalValue, $this->dut->getUpdateInterval()); + $this->dut->setUpdateInterval($saveValue); + $this->assertEquals($saveValue, $this->dut->getUpdateInterval()); + } + + public function testUpdateIntervalUnset() { + $this->config->expects($this->once())->method('getUserValue') + ->with($this->userId, 'cookbook', 'update_interval') + ->willReturn(''); + + $this->assertEquals(600, $this->dut->getUpdateInterval()); + } + + public function testPrintImage() { + $this->config->expects($this->exactly(3))->method('getUserValue') + ->with($this->userId, 'cookbook', 'print_image') + ->willReturnOnConsecutiveCalls('0', '1', '0'); + + $this->config->expects($this->exactly(2))->method('setUserValue') + ->withConsecutive( + [$this->userId, 'cookbook', 'print_image', '1'], + [$this->userId, 'cookbook', 'print_image', '0'] + ); + + $this->assertFalse($this->dut->getPrintImage()); + $this->dut->setPrintImage(true); + $this->assertTrue($this->dut->getPrintImage()); + $this->dut->setPrintImage(false); + $this->assertFalse($this->dut->getPrintImage()); + } + + public function testPrintImageUnset() { + $this->config->expects($this->once())->method('getUserValue') + ->with($this->userId, 'cookbook', 'print_image') + ->willReturn(''); + + $this->assertTrue($this->dut->getPrintImage()); + } + + public function testFolderName() { + $originalValue = '/Recipes'; + $saveValue = '/My Recipes'; + + $this->config->expects($this->exactly(2))->method('getUserValue') + ->with($this->userId, 'cookbook', 'folder') + ->willReturnOnConsecutiveCalls($originalValue, $saveValue); + + $this->config->expects($this->once())->method('setUserValue') + ->with($this->userId, 'cookbook', 'folder', $saveValue); + + $this->assertEquals($originalValue, $this->dut->getFolderName()); + $this->dut->setFolderName($saveValue); + $this->assertEquals($saveValue, $this->dut->getFolderName()); + } + + public function testFolderNameUnset() { + $this->config->expects($this->once())->method('getUserValue') + ->with($this->userId, 'cookbook', 'folder') + ->willReturn(''); + + $this->config->expects($this->once())->method('setUserValue') + ->with($this->userId, 'cookbook', 'folder', '/Recipes'); + + $this->assertEquals('/Recipes', $this->dut->getFolderName()); + } +} From a419af4b03c53eb794c322ea2f57eeaecb2f0be9 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Tue, 22 Mar 2022 17:29:58 +0100 Subject: [PATCH 2/6] Replaced user config by class access Signed-off-by: Christian Wolf --- lib/Helper/UserConfigHelper.php | 2 +- lib/Service/DbCacheService.php | 16 ++++++------ lib/Service/RecipeService.php | 29 ++++++++++------------ tests/Unit/Helper/UserConfigHelperTest.php | 2 +- 4 files changed, 23 insertions(+), 26 deletions(-) diff --git a/lib/Helper/UserConfigHelper.php b/lib/Helper/UserConfigHelper.php index 687b2fb36..a63e3ebbf 100644 --- a/lib/Helper/UserConfigHelper.php +++ b/lib/Helper/UserConfigHelper.php @@ -76,7 +76,7 @@ public function setLastIndexUpdate(int $value): void { public function getUpdateInterval(): int { $rawValue = $this->getRawValue('update_interval'); if ($rawValue === '') { - return 60 * 10; + return 5; } return intval($rawValue); diff --git a/lib/Service/DbCacheService.php b/lib/Service/DbCacheService.php index f81f43e5a..2ead509f4 100644 --- a/lib/Service/DbCacheService.php +++ b/lib/Service/DbCacheService.php @@ -5,8 +5,8 @@ use OCA\Cookbook\Db\RecipeDb; use OCP\Files\File; use OCP\AppFramework\Db\DoesNotExistException; -use OCP\IConfig; use OCA\Cookbook\Exception\InvalidJSONFileException; +use OCA\Cookbook\Helper\UserConfigHelper; class DbCacheService { private $userId; @@ -23,9 +23,9 @@ class DbCacheService { private $recipeService; /** - * @var IConfig + * @var UserConfigHelper */ - private $config; + private $userConfigHelper; private $jsonFiles; private $dbReceipeFiles; @@ -36,11 +36,11 @@ class DbCacheService { private $obsoleteRecipes; private $updatedRecipes; - public function __construct(?string $UserId, RecipeDb $db, RecipeService $recipeService, IConfig $config) { + public function __construct(?string $UserId, RecipeDb $db, RecipeService $recipeService, UserConfigHelper $userConfigHelper) { $this->userId = $UserId; $this->db = $db; $this->recipeService = $recipeService; - $this->config = $config; + $this->userConfigHelper = $userConfigHelper; } public function updateCache() { @@ -340,14 +340,14 @@ private function updateKeywords() { * Gets the last time the search index was updated */ public function getSearchIndexLastUpdateTime() { - return (int) $this->config->getUserValue($this->userId, 'cookbook', 'last_index_update'); + return $this->userConfigHelper->getLastIndexUpdate(); } /** * @return int */ public function getSearchIndexUpdateInterval(): int { - $interval = (int)$this->config->getUserValue($this->userId, 'cookbook', 'update_interval'); + $interval = $this->userConfigHelper->getUpdateInterval(); if ($interval < 1) { $interval = 5; @@ -373,7 +373,7 @@ private function checkSearchIndexUpdate() { $this->updateCache(); // Cache the last index update - $this->config->setUserValue($this->userId, 'cookbook', 'last_index_update', time()); + $this->userConfigHelper->setLastIndexUpdate(time()); // TODO Make triggers more general, need refactoring of *all* Services $this->recipeService->updateSearchIndex(); diff --git a/lib/Service/RecipeService.php b/lib/Service/RecipeService.php index d60b0c799..2a31733f3 100755 --- a/lib/Service/RecipeService.php +++ b/lib/Service/RecipeService.php @@ -6,7 +6,6 @@ use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; use OCP\Image; -use OCP\IConfig; use OCP\IL10N; use OCP\Files\IRootFolder; use OCP\Files\File; @@ -16,6 +15,7 @@ use Psr\Log\LoggerInterface; use OCA\Cookbook\Exception\UserFolderNotWritableException; use OCA\Cookbook\Exception\RecipeExistsException; +use OCA\Cookbook\Helper\UserConfigHelper; /** * Main service class for the cookbook app. @@ -26,17 +26,21 @@ class RecipeService { private $root; private $user_id; private $db; - private $config; private $il10n; private $logger; - public function __construct(?string $UserId, IRootFolder $root, RecipeDb $db, IConfig $config, IL10N $il10n, LoggerInterface $logger) { + /** + * @var UserConfigHelper + */ + private $userConfigHelper; + + public function __construct(?string $UserId, IRootFolder $root, RecipeDb $db, UserConfigHelper $userConfigHelper, IL10N $il10n, LoggerInterface $logger) { $this->user_id = $UserId; $this->root = $root; $this->db = $db; - $this->config = $config; $this->il10n = $il10n; $this->logger = $logger; + $this->userConfigHelper = $userConfigHelper; } /** @@ -1020,21 +1024,14 @@ public function findRecipesInSearchIndex($keywords_string): array { * @param string $path */ public function setUserFolderPath(string $path) { - $this->config->setUserValue($this->user_id, 'cookbook', 'folder', $path); + $this->userConfigHelper->setFolderName($path); } /** * @return string */ public function getUserFolderPath() { - $path = $this->config->getUserValue($this->user_id, 'cookbook', 'folder'); - - if (!$path) { - $path = '/' . $this->il10n->t('Recipes'); - $this->config->setUserValue($this->user_id, 'cookbook', 'folder', $path); - } - - return $path; + return $this->userConfigHelper->getFolderName(); } /** @@ -1042,7 +1039,7 @@ public function getUserFolderPath() { * @throws PreConditionNotMetException */ public function setSearchIndexUpdateInterval(int $interval) { - $this->config->setUserValue($this->user_id, 'cookbook', 'update_interval', $interval); + $this->userConfigHelper->setUpdateInterval($interval); } /** @@ -1060,7 +1057,7 @@ public function getFolderForUser() { * @throws PreConditionNotMetException */ public function setPrintImage(bool $printImage) { - $this->config->setUserValue($this->user_id, 'cookbook', 'print_image', (int) $printImage); + $this->userConfigHelper->setPrintImage($printImage); } /** @@ -1068,7 +1065,7 @@ public function setPrintImage(bool $printImage) { * @return bool */ public function getPrintImage() { - return (bool) $this->config->getUserValue($this->user_id, 'cookbook', 'print_image'); + return $this->userConfigHelper->getPrintImage(); } /** diff --git a/tests/Unit/Helper/UserConfigHelperTest.php b/tests/Unit/Helper/UserConfigHelperTest.php index 02d9ab38f..34e41f10a 100644 --- a/tests/Unit/Helper/UserConfigHelperTest.php +++ b/tests/Unit/Helper/UserConfigHelperTest.php @@ -83,7 +83,7 @@ public function testUpdateIntervalUnset() { ->with($this->userId, 'cookbook', 'update_interval') ->willReturn(''); - $this->assertEquals(600, $this->dut->getUpdateInterval()); + $this->assertEquals(5, $this->dut->getUpdateInterval()); } public function testPrintImage() { From d993f4457e2d52ae65a7e97cad087ab641cb0334 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Tue, 22 Mar 2022 17:32:37 +0100 Subject: [PATCH 3/6] Update changelog Signed-off-by: Christian Wolf --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2de58f3fd..e6edc3727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ [#914](https://github.com/nextcloud/cookbook/pull/914) @MarcelRobitaille - Replace multiple spaces with a single one when pasting [#924](https://github.com/nextcloud/cookbook/pull/924) @MarcelRobitaille +- Create abstraction class for access to user configuration + [#926](https://github.com/nextcloud/cookbook/pull/926) @christianlupus ### Documentation - Introduction about how to start coding From 00ecbd280767272efd54866c41a6165b46d23edf Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Tue, 22 Mar 2022 17:35:27 +0100 Subject: [PATCH 4/6] Fix type hint for PHP 7.3 Signed-off-by: Christian Wolf --- lib/Helper/UserConfigHelper.php | 17 ++++++++++++++--- tests/Unit/Helper/UserConfigHelperTest.php | 10 ++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/lib/Helper/UserConfigHelper.php b/lib/Helper/UserConfigHelper.php index a63e3ebbf..0e2685755 100644 --- a/lib/Helper/UserConfigHelper.php +++ b/lib/Helper/UserConfigHelper.php @@ -9,9 +9,20 @@ * This class allows access to the per-user configuration of the app */ class UserConfigHelper { - private string $userId; - private IConfig $config; - private IL10N $l; + /** + * @var string + */ + private $userId; + + /** + * @var IConfig + */ + private $config; + + /** + * @var IL10N + */ + private $l; public function __construct( string $UserId, diff --git a/tests/Unit/Helper/UserConfigHelperTest.php b/tests/Unit/Helper/UserConfigHelperTest.php index 34e41f10a..fe3ad81b3 100644 --- a/tests/Unit/Helper/UserConfigHelperTest.php +++ b/tests/Unit/Helper/UserConfigHelperTest.php @@ -13,9 +13,15 @@ * @covers \OCA\Cookbook\Helper\UserConfigHelper */ class UserConfigHelperTest extends TestCase { - private UserConfigHelper $dut; + /** + * @var UserConfigHelper + */ + private $dut; - private string $userId; + /** + * @var string + */ + private $userId; /** * @var MockObject From cf2194607c9f292ef34e6731a542e0ae24eeb668 Mon Sep 17 00:00:00 2001 From: Christian Date: Wed, 23 Mar 2022 07:24:48 +0100 Subject: [PATCH 5/6] Apply suggestions from code review Review by @seyfeb Co-authored-by: Sebastian Fey Signed-off-by: Christian Wolf --- lib/Helper/UserConfigHelper.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Helper/UserConfigHelper.php b/lib/Helper/UserConfigHelper.php index 0e2685755..a21f18ab5 100644 --- a/lib/Helper/UserConfigHelper.php +++ b/lib/Helper/UserConfigHelper.php @@ -106,7 +106,7 @@ public function setUpdateInterval(int $value): void { /** * Check if the primary imgae should be printed or not * - * @return boolean true, if the imgae shoudl be printed + * @return boolean true, if the image should be printed */ public function getPrintImage(): bool { $rawValue = $this->getRawValue('print_image'); @@ -117,7 +117,7 @@ public function getPrintImage(): bool { } /** - * Set if the imgae should be printed + * Set if the image should be printed * * @param boolean $value true if the image should be printed * @return void @@ -151,7 +151,7 @@ public function getFolderName(): string { /** * Set the folder for the user's cookbook. * - * @param string $value The name of the folder withon the user's files + * @param string $value The name of the folder within the user's files * @return void */ public function setFolderName(string $value): void { From 8a1242d645367fc5ffba6eb8155710a137536174 Mon Sep 17 00:00:00 2001 From: Christian Wolf Date: Wed, 23 Mar 2022 07:38:12 +0100 Subject: [PATCH 6/6] Extract magic values into constants Signed-off-by: Christian Wolf --- lib/Helper/UserConfigHelper.php | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/lib/Helper/UserConfigHelper.php b/lib/Helper/UserConfigHelper.php index a21f18ab5..76f015c36 100644 --- a/lib/Helper/UserConfigHelper.php +++ b/lib/Helper/UserConfigHelper.php @@ -2,6 +2,7 @@ namespace OCA\Cookbook\Helper; +use OCA\Cookbook\AppInfo\Application; use OCP\IConfig; use OCP\IL10N; @@ -34,6 +35,11 @@ public function __construct( $this->l = $l; } + protected const KEY_LAST_INDEX_UPDATE = 'last_index_update'; + protected const KEY_UPDATE_INTERVAL = 'update_interval'; + protected const KEY_PRINT_IMAGE = 'print_image'; + protected const KEY_FOLDER = 'folder'; + /** * Get a config value from the database * @@ -41,7 +47,7 @@ public function __construct( * @return string The resulting value or '' if the key was not found */ private function getRawValue(string $key): string { - return $this->config->getUserValue($this->userId, 'cookbook', $key); + return $this->config->getUserValue($this->userId, Application::APP_ID, $key); } /** @@ -52,7 +58,7 @@ private function getRawValue(string $key): string { * @return void */ private function setRawValue(string $key, string $value): void { - $this->config->setUserValue($this->userId, 'cookbook', $key, $value); + $this->config->setUserValue($this->userId, Application::APP_ID, $key, $value); } /** @@ -61,7 +67,7 @@ private function setRawValue(string $key, string $value): void { * @return integer The timestamp of the last index rebuild */ public function getLastIndexUpdate(): int { - $rawValue = $this->getRawValue('last_index_update'); + $rawValue = $this->getRawValue(self::KEY_LAST_INDEX_UPDATE); if ($rawValue === '') { return 0; } @@ -76,7 +82,7 @@ public function getLastIndexUpdate(): int { * @return void */ public function setLastIndexUpdate(int $value): void { - $this->setRawValue('last_index_update', strval($value)); + $this->setRawValue(self::KEY_LAST_INDEX_UPDATE, strval($value)); } /** @@ -85,7 +91,7 @@ public function setLastIndexUpdate(int $value): void { * @return integer The number of seconds to wait before a new rescan is triggered */ public function getUpdateInterval(): int { - $rawValue = $this->getRawValue('update_interval'); + $rawValue = $this->getRawValue(self::KEY_UPDATE_INTERVAL); if ($rawValue === '') { return 5; } @@ -100,7 +106,7 @@ public function getUpdateInterval(): int { * @return void */ public function setUpdateInterval(int $value): void { - $this->setRawValue('update_interval', $value); + $this->setRawValue(self::KEY_UPDATE_INTERVAL, $value); } /** @@ -109,7 +115,7 @@ public function setUpdateInterval(int $value): void { * @return boolean true, if the image should be printed */ public function getPrintImage(): bool { - $rawValue = $this->getRawValue('print_image'); + $rawValue = $this->getRawValue(self::KEY_PRINT_IMAGE); if ($rawValue === '') { return true; } @@ -124,9 +130,9 @@ public function getPrintImage(): bool { */ public function setPrintImage(bool $value): void { if ($value) { - $this->setRawValue('print_image', '1'); + $this->setRawValue(self::KEY_PRINT_IMAGE, '1'); } else { - $this->setRawValue('print_image', '0'); + $this->setRawValue(self::KEY_PRINT_IMAGE, '0'); } } @@ -138,7 +144,8 @@ public function setPrintImage(bool $value): void { * @return string The name of the folder within the users files */ public function getFolderName(): string { - $rawValue = $this->getRawValue('folder'); + $rawValue = $this->getRawValue(self::KEY_FOLDER); + if ($rawValue === '') { $path = '/' . $this->l->t('Recipes'); $this->setFolderName($path); @@ -155,6 +162,6 @@ public function getFolderName(): string { * @return void */ public function setFolderName(string $value): void { - $this->setRawValue('folder', $value); + $this->setRawValue(self::KEY_FOLDER, $value); } }