refactor: extract admin settings to its own service

lib/Controller/AdminController.php

@@ -13,14 +13,13 @@ use OCA\Forum\Db\PostMapper;
 use OCA\Forum\Db\ThreadMapper;
 use OCA\Forum\Db\UserRoleMapper;
 use OCA\Forum\Db\UserStatsMapper;
+use OCA\Forum\Service\AdminSettingsService;
 use OCA\Forum\Service\UserService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
-use OCP\AppFramework\Services\IAppConfig;
-use OCP\IL10N;
 use OCP\IRequest;
 use OCP\IUserManager;
 use OCP\IUserSession;
@@ -39,9 +38,8 @@ class AdminController extends OCSController {
 		private UserRoleMapper $userRoleMapper,
 		private IUserManager $userManager,
 		private IUserSession $userSession,
-		private IAppConfig $config,
+		private AdminSettingsService $settingsService,
 		private LoggerInterface $logger,
-		private IL10N $l10n,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -168,12 +166,7 @@ class AdminController extends OCSController {
 	#[ApiRoute(verb: 'GET', url: '/api/admin/settings')]
 	public function getSettings(): DataResponse {
 		try {
-			$settings = [
-				'title' => $this->config->getAppValueString('title', $this->l10n->t('Forum'), true),
-				'subtitle' => $this->config->getAppValueString('subtitle', $this->l10n->t('Welcome to the forum!'), true),
-				'allow_guest_access' => $this->config->getAppValueBool('allow_guest_access', false, true),
-			];
-
+			$settings = $this->settingsService->getAllSettings();
 			return new DataResponse($settings);
 		} catch (\Exception $e) {
 			$this->logger->error('Error fetching settings: ' . $e->getMessage());
@@ -196,25 +189,20 @@ class AdminController extends OCSController {
 	#[ApiRoute(verb: 'PUT', url: '/api/admin/settings')]
 	public function updateSettings(?string $title = null, ?string $subtitle = null, ?bool $allow_guest_access = null): DataResponse {
 		try {
+			// Build settings array with only non-null values
+			$settingsToUpdate = [];
 			if ($title !== null) {
-				$this->config->setAppValueString('title', $title, true);
+				$settingsToUpdate[AdminSettingsService::SETTING_TITLE] = $title;
 			}
-
 			if ($subtitle !== null) {
-				$this->config->setAppValueString('subtitle', $subtitle, true);
+				$settingsToUpdate[AdminSettingsService::SETTING_SUBTITLE] = $subtitle;
 			}
-
 			if ($allow_guest_access !== null) {
-				$this->config->setAppValueBool('allow_guest_access', $allow_guest_access, true);
+				$settingsToUpdate[AdminSettingsService::SETTING_ALLOW_GUEST_ACCESS] = $allow_guest_access;
 			}
 
-			// Return updated settings
-			$settings = [
-				'title' => $this->config->getAppValueString('title', $this->l10n->t('Forum'), true),
-				'subtitle' => $this->config->getAppValueString('subtitle', $this->l10n->t('Welcome to the forum!'), true),
-				'allow_guest_access' => $this->config->getAppValueBool('allow_guest_access', false, true),
-			];
-
+			// Update settings and return all settings
+			$settings = $this->settingsService->updateSettings($settingsToUpdate);
 			return new DataResponse($settings);
 		} catch (\Exception $e) {
 			$this->logger->error('Error updating settings: ' . $e->getMessage());

lib/Controller/SettingsController.php

@@ -7,14 +7,13 @@ declare(strict_types=1);
 
 namespace OCA\Forum\Controller;
 
+use OCA\Forum\Service\AdminSettingsService;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\ApiRoute;
 use OCP\AppFramework\Http\Attribute\NoAdminRequired;
 use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\OCSController;
-use OCP\AppFramework\Services\IAppConfig;
-use OCP\IL10N;
 use OCP\IRequest;
 use Psr\Log\LoggerInterface;
 
@@ -23,9 +22,8 @@ class SettingsController extends OCSController {
 	public function __construct(
 		string $appName,
 		IRequest $request,
-		private IAppConfig $config,
+		private AdminSettingsService $settingsService,
 		private LoggerInterface $logger,
-		private IL10N $l10n,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -45,12 +43,7 @@ class SettingsController extends OCSController {
 	#[ApiRoute(verb: 'GET', url: '/api/settings')]
 	public function getPublicSettings(): DataResponse {
 		try {
-			$settings = [
-				'title' => $this->config->getAppValueString('title', $this->l10n->t('Forum'), true),
-				'subtitle' => $this->config->getAppValueString('subtitle', $this->l10n->t('Welcome to the forum!'), true),
-				'allow_guest_access' => $this->config->getAppValueBool('allow_guest_access', false, true),
-			];
-
+			$settings = $this->settingsService->getAllSettings();
 			return new DataResponse($settings);
 		} catch (\Exception $e) {
 			$this->logger->error('Error fetching public settings: ' . $e->getMessage());

lib/Service/AdminSettingsService.php

@@ -0,0 +1,130 @@
+<?php
+
+declare(strict_types=1);
+
+// SPDX-FileCopyrightText: Chen Asraf <contact@casraf.dev>
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+namespace OCA\Forum\Service;
+
+use OCP\AppFramework\Services\IAppConfig;
+use OCP\IL10N;
+use Psr\Log\LoggerInterface;
+
+class AdminSettingsService {
+	/** Setting key for forum title */
+	public const SETTING_TITLE = 'title';
+
+	/** Setting key for forum subtitle */
+	public const SETTING_SUBTITLE = 'subtitle';
+
+	/** Setting key for guest access */
+	public const SETTING_ALLOW_GUEST_ACCESS = 'allow_guest_access';
+
+	/** @var array<string> List of valid setting keys */
+	private const VALID_KEYS = [
+		self::SETTING_TITLE,
+		self::SETTING_SUBTITLE,
+		self::SETTING_ALLOW_GUEST_ACCESS,
+	];
+
+	public function __construct(
+		private IAppConfig $config,
+		private IL10N $l10n,
+		private LoggerInterface $logger,
+	) {
+	}
+
+	/**
+	 * Get default value for a setting
+	 *
+	 * @param string $key The setting key
+	 * @return mixed The default value
+	 */
+	private function getDefault(string $key): mixed {
+		return match ($key) {
+			self::SETTING_TITLE => $this->l10n->t('Forum'),
+			self::SETTING_SUBTITLE => $this->l10n->t('Welcome to the forum!'),
+			self::SETTING_ALLOW_GUEST_ACCESS => false,
+			default => null,
+		};
+	}
+
+	/**
+	 * Get all settings
+	 *
+	 * @return array<string, mixed> All settings
+	 */
+	public function getAllSettings(): array {
+		$settings = [];
+
+		foreach (self::VALID_KEYS as $key) {
+			$settings[$key] = $this->getSetting($key);
+		}
+
+		return $settings;
+	}
+
+	/**
+	 * Get a single setting
+	 *
+	 * @param string $key The setting key
+	 * @return mixed The setting value
+	 * @throws \InvalidArgumentException If the setting key is invalid
+	 */
+	public function getSetting(string $key): mixed {
+		if (!in_array($key, self::VALID_KEYS, true)) {
+			throw new \InvalidArgumentException("Invalid setting key: $key");
+		}
+
+		$default = $this->getDefault($key);
+
+		return match ($key) {
+			self::SETTING_ALLOW_GUEST_ACCESS => $this->config->getAppValueBool($key, $default, true),
+			default => $this->config->getAppValueString($key, $default, true),
+		};
+	}
+
+	/**
+	 * Update multiple settings
+	 *
+	 * @param array<string, mixed> $settings Key-value pairs of settings to update
+	 * @return array<string, mixed> All settings after update
+	 * @throws \InvalidArgumentException If any setting key is invalid
+	 */
+	public function updateSettings(array $settings): array {
+		// Validate all keys before updating
+		foreach ($settings as $key => $value) {
+			if (!in_array($key, self::VALID_KEYS, true)) {
+				throw new \InvalidArgumentException("Invalid setting key: $key");
+			}
+		}
+
+		// Update each setting
+		foreach ($settings as $key => $value) {
+			$this->setSetting($key, $value);
+		}
+
+		// Return all settings after update
+		return $this->getAllSettings();
+	}
+
+	/**
+	 * Set a single setting
+	 *
+	 * @param string $key The setting key
+	 * @param mixed $value The setting value
+	 * @throws \InvalidArgumentException If the setting key is invalid
+	 */
+	public function setSetting(string $key, mixed $value): void {
+		if (!in_array($key, self::VALID_KEYS, true)) {
+			throw new \InvalidArgumentException("Invalid setting key: $key");
+		}
+
+		if ($key === self::SETTING_ALLOW_GUEST_ACCESS) {
+			$this->config->setAppValueBool($key, (bool)$value, true);
+		} else {
+			$this->config->setAppValueString($key, (string)$value, true);
+		}
+	}
+}