feat(DeclarativeSettings): Allow to implement value getter and setter…

… directly in Form Instead of implementing the form class, a setter event listener and a getter event listener, this allows to simply write a basic class that provides `getSchema`, `setValue` and `getValue` functions. Signed-off-by: Ferdinand Thiessen <[email protected]>
nextcloud · skjnldsv · Nov 6, 2024 · Oct 15, 2024 · Oct 15, 2024 · Oct 15, 2024
commit 9a7e1bb2276f4a4635cd330fbc2a385ebaeed5ee
@@ -698,6 +698,7 @@
     'OCP\\Settings\\Events\\DeclarativeSettingsSetValueEvent' => $baseDir . '/lib/public/Settings/Events/DeclarativeSettingsSetValueEvent.php',
     'OCP\\Settings\\IDeclarativeManager' => $baseDir . '/lib/public/Settings/IDeclarativeManager.php',
     'OCP\\Settings\\IDeclarativeSettingsForm' => $baseDir . '/lib/public/Settings/IDeclarativeSettingsForm.php',
+    'OCP\\Settings\\IDeclarativeSettingsFormWithHandlers' => $baseDir . '/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php',
     'OCP\\Settings\\IDelegatedSettings' => $baseDir . '/lib/public/Settings/IDelegatedSettings.php',
     'OCP\\Settings\\IIconSection' => $baseDir . '/lib/public/Settings/IIconSection.php',
     'OCP\\Settings\\IManager' => $baseDir . '/lib/public/Settings/IManager.php',

@@ -731,6 +731,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OCP\\Settings\\Events\\DeclarativeSettingsSetValueEvent' => __DIR__ . '/../../..' . '/lib/public/Settings/Events/DeclarativeSettingsSetValueEvent.php',
         'OCP\\Settings\\IDeclarativeManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeManager.php',
         'OCP\\Settings\\IDeclarativeSettingsForm' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeSettingsForm.php',
+        'OCP\\Settings\\IDeclarativeSettingsFormWithHandlers' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php',
         'OCP\\Settings\\IDelegatedSettings' => __DIR__ . '/../../..' . '/lib/public/Settings/IDelegatedSettings.php',
         'OCP\\Settings\\IIconSection' => __DIR__ . '/../../..' . '/lib/public/Settings/IIconSection.php',
         'OCP\\Settings\\IManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IManager.php',

@@ -22,6 +22,7 @@
 use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
 use OCP\Settings\IDeclarativeManager;
 use OCP\Settings\IDeclarativeSettingsForm;
+use OCP\Settings\IDeclarativeSettingsFormWithHandlers;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -32,6 +33,15 @@
  * @psalm-import-type DeclarativeSettingsFormSchemaWithoutValues from IDeclarativeSettingsForm
  */
 class DeclarativeManager implements IDeclarativeManager {
+
+	/** @var array<string, list<IDeclarativeSettingsForm>> */
+	private array $declarativeForms = [];
+
+	/**
+	 * @var array<string, list<DeclarativeSettingsFormSchemaWithoutValues>>
+	 */
+	private array $appSchemas = [];
+
 	public function __construct(
 		private IEventDispatcher $eventDispatcher,
 		private IGroupManager    $groupManager,
@@ -42,11 +52,6 @@ public function __construct(
 	) {
 	}
 
-	/**
-	 * @var array<string, list<DeclarativeSettingsFormSchemaWithoutValues>>
-	 */
-	private array $appSchemas = [];
-
 	/**
 	 * @inheritdoc
 	 */
@@ -77,11 +82,15 @@ public function registerSchema(string $app, array $schema): void {
 	 * @inheritdoc
 	 */
 	public function loadSchemas(): void {
-		$declarativeSettings = $this->coordinator->getRegistrationContext()->getDeclarativeSettings();
-		foreach ($declarativeSettings as $declarativeSetting) {
-			/** @var IDeclarativeSettingsForm $declarativeSettingObject */
-			$declarativeSettingObject = Server::get($declarativeSetting->getService());
-			$this->registerSchema($declarativeSetting->getAppId(), $declarativeSettingObject->getSchema());
+		if (empty($this->declarativeForms)) {
+			$declarativeSettings = $this->coordinator->getRegistrationContext()->getDeclarativeSettings();
+			foreach ($declarativeSettings as $declarativeSetting) {
+				$app = $declarativeSetting->getAppId();
+				/** @var IDeclarativeSettingsForm $declarativeForm */
+				$declarativeForm = Server::get($declarativeSetting->getService());
+				$this->registerSchema($app, $declarativeForm->getSchema());
+				$this->declarativeForms[$app][] = $declarativeForm;
+			}
 		}
 
 		$this->eventDispatcher->dispatchTyped(new DeclarativeSettingsRegisterFormEvent($this));
@@ -224,6 +233,10 @@ private function getValue(IUser $user, string $app, string $formId, string $fiel
 		$storageType = $this->getStorageType($app, $fieldId);
 		switch ($storageType) {
 			case DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL:
+				$form = $this->getForm($app, $formId);
+				if ($form !== null && $form instanceof IDeclarativeSettingsFormWithHandlers) {
+					return $form->getValue($fieldId, $user);
+				}
 				$event = new DeclarativeSettingsGetValueEvent($user, $app, $formId, $fieldId);
 				$this->eventDispatcher->dispatchTyped($event);
 				return $event->getValue();
@@ -244,6 +257,12 @@ public function setValue(IUser $user, string $app, string $formId, string $field
 		$storageType = $this->getStorageType($app, $fieldId);
 		switch ($storageType) {
 			case DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL:
+				$form = $this->getForm($app, $formId);
+				if ($form !== null && $form instanceof IDeclarativeSettingsFormWithHandlers) {
+					$form->setValue($fieldId, $value, $user);
+					break;
+				}
+				// fall back to event handling
 				$this->eventDispatcher->dispatchTyped(new DeclarativeSettingsSetValueEvent($user, $app, $formId, $fieldId, $value));
 				break;
 			case DeclarativeSettingsTypes::STORAGE_TYPE_INTERNAL:
@@ -254,6 +273,20 @@ public function setValue(IUser $user, string $app, string $formId, string $field
 		}
 	}
 
+	/**
+	 * If a declarative setting was registered as a form and not just a schema
+	 * then this will yield the registering form.
+	 */
+	private function getForm(string $app, string $formId): ?IDeclarativeSettingsForm {
+		$allForms = $this->declarativeForms[$app] ?? [];
+		foreach ($allForms as $form) {
+			if ($form->getSchema()['id'] === $formId) {
+				return $form;
+			}
+		}
+		return null;
+	}
+
 	private function getInternalValue(IUser $user, string $app, string $formId, string $fieldId): mixed {
 		$sectionType = $this->getSectionType($app, $fieldId);
 		$defaultValue = $this->getDefaultValue($app, $formId, $fieldId);

@@ -32,8 +32,9 @@ final class DeclarativeSettingsTypes {
 	/**
 	 * IDeclarativeSettingsForm storage_type which is determines where and how the config value is stored
 	 *
-	 *
-	 * For `external` storage_type the app implementing \OCP\Settings\SetDeclarativeSettingsValueEvent and \OCP\Settings\GetDeclarativeSettingsValueEvent events is responsible for storing and retrieving the config value.
+	 * For `external` storage_type the app needs to either implement event listeners for \OCP\Settings\SetDeclarativeSettingsValueEvent
+	 * and \OCP\Settings\GetDeclarativeSettingsValueEvent or the IDeclarativeSettingsForm also needs to implement
+	 * IDeclarativeSettingsFormWithHandlers for storing and retrieving the config value.
 	 *
 	 * @since 29.0.0
 	 */
@@ -43,7 +44,6 @@ final class DeclarativeSettingsTypes {
 	 * IDeclarativeSettingsForm storage_type which is determines where and how the config value is stored
 	 *
 	 * For `internal` storage_type the config value is stored in default `appconfig` and `preferences` tables.
-	 * For `external` storage_type the app implementing \OCP\Settings\SetDeclarativeSettingsValueEvent and \OCP\Settings\GetDeclarativeSettingsValueEvent events is responsible for storing and retrieving the config value.
 	 *
 	 * @since 29.0.0
 	 */

@@ -0,0 +1,31 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCP\Settings;
+
+use OCP\IUser;
+
+/**
+ * @since 31.0.0
+ */
+interface IDeclarativeSettingsFormWithHandlers extends IDeclarativeSettingsForm {
+
+	/**
+	 * This function is called to get the current value of a specific forms field.
+	 * @since 31.0.0
+	 */
+	public function getValue(string $fieldId, IUser $user): mixed;
+
+	/**
+	 * This function is called when a user updated a form field to persist the setting.
+	 * @since 31.0.0
+	 */
+	public function setValue(string $fieldId, mixed $value, IUser $user): void;
+
+}
@@ -10,6 +10,8 @@
 namespace Test\Settings;
 
 use OC\AppFramework\Bootstrap\Coordinator;
+use OC\AppFramework\Bootstrap\RegistrationContext;
+use OC\AppFramework\Bootstrap\ServiceRegistration;
 use OC\Settings\DeclarativeManager;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAppConfig;
@@ -19,6 +21,8 @@
 use OCP\Settings\DeclarativeSettingsTypes;
 use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
 use OCP\Settings\IDeclarativeManager;
+use OCP\Settings\IDeclarativeSettingsForm;
+use OCP\Settings\IDeclarativeSettingsFormWithHandlers;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 use Test\TestCase;
@@ -52,6 +56,8 @@ class DeclarativeManagerTest extends TestCase {
 	/** @var IUser|MockObject */
 	private $adminUser;
 
+	private IDeclarativeSettingsForm&MockObject $closureForm;
+
 	public const validSchemaAllFields = [
 		'id' => 'test_form_1',
 		'priority' => 10,
@@ -518,4 +524,74 @@ public function testAdminFormUserUnauthorized(): void {
 		$this->expectException(\Exception::class);
 		$this->declarativeManager->getFormsWithValues($this->user, $schema['section_type'], $schema['section_id']);
 	}
+
+	/**
+	 * Ensure that the `setValue` method is called if the form implements the handler interface.
+	 */
+	public function testSetValueWithHandler(): void {
+		$schema = self::validSchemaAllFields;
+		$schema['storage_type'] = DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL;
+
+		$form = $this->createMock(IDeclarativeSettingsFormWithHandlers::class);
+		$form->expects(self::atLeastOnce())
+			->method('getSchema')
+			->willReturn($schema);
+		// The setter should be called once!
+		$form->expects(self::once())
+			->method('setValue')
+			->with('test_field_2', 'some password', $this->adminUser);
+
+		\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
+
+		$context = $this->createMock(RegistrationContext::class);
+		$context->expects(self::atLeastOnce())
+			->method('getDeclarativeSettings')
+			->willReturn([new ServiceRegistration('testing', 'OCA\\Testing\\Settings\\DeclarativeForm')]);
+
+		$this->coordinator->expects(self::atLeastOnce())
+			->method('getRegistrationContext')
+			->willReturn($context);
+
+		$this->declarativeManager->loadSchemas();
+
+		$this->eventDispatcher->expects(self::never())
+			->method('dispatchTyped');
+
+		$this->declarativeManager->setValue($this->adminUser, 'testing', 'test_form_1', 'test_field_2', 'some password');
+	}
+
+	public function testGetValueWithHandler(): void {
+		$schema = self::validSchemaAllFields;
+		$schema['storage_type'] = DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL;
+
+		$form = $this->createMock(IDeclarativeSettingsFormWithHandlers::class);
+		$form->expects(self::atLeastOnce())
+			->method('getSchema')
+			->willReturn($schema);
+		// The setter should be called once!
+		$form->expects(self::once())
+			->method('getValue')
+			->with('test_field_2', $this->adminUser)
+			->willReturn('very secret password');
+
+		\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
+
+		$context = $this->createMock(RegistrationContext::class);
+		$context->expects(self::atLeastOnce())
+			->method('getDeclarativeSettings')
+			->willReturn([new ServiceRegistration('testing', 'OCA\\Testing\\Settings\\DeclarativeForm')]);
+
+		$this->coordinator->expects(self::atLeastOnce())
+			->method('getRegistrationContext')
+			->willReturn($context);
+
+		$this->declarativeManager->loadSchemas();
+
+		$this->eventDispatcher->expects(self::never())
+			->method('dispatchTyped');
+
+		$password = $this->invokePrivate($this->declarativeManager, 'getValue', [$this->adminUser, 'testing', 'test_form_1', 'test_field_2']);
+		self::assertEquals('very secret password', $password);
+	}
+
 }