From 9a7e1bb2276f4a4635cd330fbc2a385ebaeed5ee Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 15 Oct 2024 20:22:33 +0200 Subject: [PATCH] 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 --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Settings/DeclarativeManager.php | 53 ++++++++++--- .../Settings/DeclarativeSettingsTypes.php | 6 +- .../IDeclarativeSettingsFormWithHandlers.php | 31 ++++++++ tests/lib/Settings/DeclarativeManagerTest.php | 76 +++++++++++++++++++ 6 files changed, 155 insertions(+), 13 deletions(-) create mode 100644 lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 1219c0f5aa4..545c33f2572 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -698,6 +698,7 @@ return array( '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', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 0e93ee8addd..fcb2a0cd8c9 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.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', diff --git a/lib/private/Settings/DeclarativeManager.php b/lib/private/Settings/DeclarativeManager.php index 5c2b868f417..ee253b86449 100644 --- a/lib/private/Settings/DeclarativeManager.php +++ b/lib/private/Settings/DeclarativeManager.php @@ -22,6 +22,7 @@ use OCP\Settings\Events\DeclarativeSettingsRegisterFormEvent; 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 @@ use Psr\Log\LoggerInterface; * @psalm-import-type DeclarativeSettingsFormSchemaWithoutValues from IDeclarativeSettingsForm */ class DeclarativeManager implements IDeclarativeManager { + + /** @var array> */ + private array $declarativeForms = []; + + /** + * @var array> + */ + private array $appSchemas = []; + public function __construct( private IEventDispatcher $eventDispatcher, private IGroupManager $groupManager, @@ -42,11 +52,6 @@ class DeclarativeManager implements IDeclarativeManager { ) { } - /** - * @var array> - */ - private array $appSchemas = []; - /** * @inheritdoc */ @@ -77,11 +82,15 @@ class DeclarativeManager implements IDeclarativeManager { * @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 @@ class DeclarativeManager implements IDeclarativeManager { $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 @@ class DeclarativeManager implements IDeclarativeManager { $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 @@ class DeclarativeManager implements IDeclarativeManager { } } + /** + * 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); diff --git a/lib/public/Settings/DeclarativeSettingsTypes.php b/lib/public/Settings/DeclarativeSettingsTypes.php index 82444af9b82..7edf0cbdd6e 100644 --- a/lib/public/Settings/DeclarativeSettingsTypes.php +++ b/lib/public/Settings/DeclarativeSettingsTypes.php @@ -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 */ diff --git a/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php b/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php new file mode 100644 index 00000000000..180df73d995 --- /dev/null +++ b/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php @@ -0,0 +1,31 @@ + 'test_form_1', 'priority' => 10, @@ -518,4 +524,74 @@ class DeclarativeManagerTest extends TestCase { $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); + } + }