Skip to content

Commit 9a7e1bb

Browse files
committed
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]>
1 parent cd3dc17 commit 9a7e1bb

File tree

6 files changed

+155
-13
lines changed

6 files changed

+155
-13
lines changed

lib/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,7 @@
698698
'OCP\\Settings\\Events\\DeclarativeSettingsSetValueEvent' => $baseDir . '/lib/public/Settings/Events/DeclarativeSettingsSetValueEvent.php',
699699
'OCP\\Settings\\IDeclarativeManager' => $baseDir . '/lib/public/Settings/IDeclarativeManager.php',
700700
'OCP\\Settings\\IDeclarativeSettingsForm' => $baseDir . '/lib/public/Settings/IDeclarativeSettingsForm.php',
701+
'OCP\\Settings\\IDeclarativeSettingsFormWithHandlers' => $baseDir . '/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php',
701702
'OCP\\Settings\\IDelegatedSettings' => $baseDir . '/lib/public/Settings/IDelegatedSettings.php',
702703
'OCP\\Settings\\IIconSection' => $baseDir . '/lib/public/Settings/IIconSection.php',
703704
'OCP\\Settings\\IManager' => $baseDir . '/lib/public/Settings/IManager.php',

lib/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
731731
'OCP\\Settings\\Events\\DeclarativeSettingsSetValueEvent' => __DIR__ . '/../../..' . '/lib/public/Settings/Events/DeclarativeSettingsSetValueEvent.php',
732732
'OCP\\Settings\\IDeclarativeManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeManager.php',
733733
'OCP\\Settings\\IDeclarativeSettingsForm' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeSettingsForm.php',
734+
'OCP\\Settings\\IDeclarativeSettingsFormWithHandlers' => __DIR__ . '/../../..' . '/lib/public/Settings/IDeclarativeSettingsFormWithHandlers.php',
734735
'OCP\\Settings\\IDelegatedSettings' => __DIR__ . '/../../..' . '/lib/public/Settings/IDelegatedSettings.php',
735736
'OCP\\Settings\\IIconSection' => __DIR__ . '/../../..' . '/lib/public/Settings/IIconSection.php',
736737
'OCP\\Settings\\IManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IManager.php',

lib/private/Settings/DeclarativeManager.php

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
2323
use OCP\Settings\IDeclarativeManager;
2424
use OCP\Settings\IDeclarativeSettingsForm;
25+
use OCP\Settings\IDeclarativeSettingsFormWithHandlers;
2526
use Psr\Log\LoggerInterface;
2627

2728
/**
@@ -32,6 +33,15 @@
3233
* @psalm-import-type DeclarativeSettingsFormSchemaWithoutValues from IDeclarativeSettingsForm
3334
*/
3435
class DeclarativeManager implements IDeclarativeManager {
36+
37+
/** @var array<string, list<IDeclarativeSettingsForm>> */
38+
private array $declarativeForms = [];
39+
40+
/**
41+
* @var array<string, list<DeclarativeSettingsFormSchemaWithoutValues>>
42+
*/
43+
private array $appSchemas = [];
44+
3545
public function __construct(
3646
private IEventDispatcher $eventDispatcher,
3747
private IGroupManager $groupManager,
@@ -42,11 +52,6 @@ public function __construct(
4252
) {
4353
}
4454

45-
/**
46-
* @var array<string, list<DeclarativeSettingsFormSchemaWithoutValues>>
47-
*/
48-
private array $appSchemas = [];
49-
5055
/**
5156
* @inheritdoc
5257
*/
@@ -77,11 +82,15 @@ public function registerSchema(string $app, array $schema): void {
7782
* @inheritdoc
7883
*/
7984
public function loadSchemas(): void {
80-
$declarativeSettings = $this->coordinator->getRegistrationContext()->getDeclarativeSettings();
81-
foreach ($declarativeSettings as $declarativeSetting) {
82-
/** @var IDeclarativeSettingsForm $declarativeSettingObject */
83-
$declarativeSettingObject = Server::get($declarativeSetting->getService());
84-
$this->registerSchema($declarativeSetting->getAppId(), $declarativeSettingObject->getSchema());
85+
if (empty($this->declarativeForms)) {
86+
$declarativeSettings = $this->coordinator->getRegistrationContext()->getDeclarativeSettings();
87+
foreach ($declarativeSettings as $declarativeSetting) {
88+
$app = $declarativeSetting->getAppId();
89+
/** @var IDeclarativeSettingsForm $declarativeForm */
90+
$declarativeForm = Server::get($declarativeSetting->getService());
91+
$this->registerSchema($app, $declarativeForm->getSchema());
92+
$this->declarativeForms[$app][] = $declarativeForm;
93+
}
8594
}
8695

8796
$this->eventDispatcher->dispatchTyped(new DeclarativeSettingsRegisterFormEvent($this));
@@ -224,6 +233,10 @@ private function getValue(IUser $user, string $app, string $formId, string $fiel
224233
$storageType = $this->getStorageType($app, $fieldId);
225234
switch ($storageType) {
226235
case DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL:
236+
$form = $this->getForm($app, $formId);
237+
if ($form !== null && $form instanceof IDeclarativeSettingsFormWithHandlers) {
238+
return $form->getValue($fieldId, $user);
239+
}
227240
$event = new DeclarativeSettingsGetValueEvent($user, $app, $formId, $fieldId);
228241
$this->eventDispatcher->dispatchTyped($event);
229242
return $event->getValue();
@@ -244,6 +257,12 @@ public function setValue(IUser $user, string $app, string $formId, string $field
244257
$storageType = $this->getStorageType($app, $fieldId);
245258
switch ($storageType) {
246259
case DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL:
260+
$form = $this->getForm($app, $formId);
261+
if ($form !== null && $form instanceof IDeclarativeSettingsFormWithHandlers) {
262+
$form->setValue($fieldId, $value, $user);
263+
break;
264+
}
265+
// fall back to event handling
247266
$this->eventDispatcher->dispatchTyped(new DeclarativeSettingsSetValueEvent($user, $app, $formId, $fieldId, $value));
248267
break;
249268
case DeclarativeSettingsTypes::STORAGE_TYPE_INTERNAL:
@@ -254,6 +273,20 @@ public function setValue(IUser $user, string $app, string $formId, string $field
254273
}
255274
}
256275

276+
/**
277+
* If a declarative setting was registered as a form and not just a schema
278+
* then this will yield the registering form.
279+
*/
280+
private function getForm(string $app, string $formId): ?IDeclarativeSettingsForm {
281+
$allForms = $this->declarativeForms[$app] ?? [];
282+
foreach ($allForms as $form) {
283+
if ($form->getSchema()['id'] === $formId) {
284+
return $form;
285+
}
286+
}
287+
return null;
288+
}
289+
257290
private function getInternalValue(IUser $user, string $app, string $formId, string $fieldId): mixed {
258291
$sectionType = $this->getSectionType($app, $fieldId);
259292
$defaultValue = $this->getDefaultValue($app, $formId, $fieldId);

lib/public/Settings/DeclarativeSettingsTypes.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,9 @@ final class DeclarativeSettingsTypes {
3232
/**
3333
* IDeclarativeSettingsForm storage_type which is determines where and how the config value is stored
3434
*
35-
*
36-
* For `external` storage_type the app implementing \OCP\Settings\SetDeclarativeSettingsValueEvent and \OCP\Settings\GetDeclarativeSettingsValueEvent events is responsible for storing and retrieving the config value.
35+
* For `external` storage_type the app needs to either implement event listeners for \OCP\Settings\SetDeclarativeSettingsValueEvent
36+
* and \OCP\Settings\GetDeclarativeSettingsValueEvent or the IDeclarativeSettingsForm also needs to implement
37+
* IDeclarativeSettingsFormWithHandlers for storing and retrieving the config value.
3738
*
3839
* @since 29.0.0
3940
*/
@@ -43,7 +44,6 @@ final class DeclarativeSettingsTypes {
4344
* IDeclarativeSettingsForm storage_type which is determines where and how the config value is stored
4445
*
4546
* For `internal` storage_type the config value is stored in default `appconfig` and `preferences` tables.
46-
* For `external` storage_type the app implementing \OCP\Settings\SetDeclarativeSettingsValueEvent and \OCP\Settings\GetDeclarativeSettingsValueEvent events is responsible for storing and retrieving the config value.
4747
*
4848
* @since 29.0.0
4949
*/
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCP\Settings;
11+
12+
use OCP\IUser;
13+
14+
/**
15+
* @since 31.0.0
16+
*/
17+
interface IDeclarativeSettingsFormWithHandlers extends IDeclarativeSettingsForm {
18+
19+
/**
20+
* This function is called to get the current value of a specific forms field.
21+
* @since 31.0.0
22+
*/
23+
public function getValue(string $fieldId, IUser $user): mixed;
24+
25+
/**
26+
* This function is called when a user updated a form field to persist the setting.
27+
* @since 31.0.0
28+
*/
29+
public function setValue(string $fieldId, mixed $value, IUser $user): void;
30+
31+
}

tests/lib/Settings/DeclarativeManagerTest.php

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
namespace Test\Settings;
1111

1212
use OC\AppFramework\Bootstrap\Coordinator;
13+
use OC\AppFramework\Bootstrap\RegistrationContext;
14+
use OC\AppFramework\Bootstrap\ServiceRegistration;
1315
use OC\Settings\DeclarativeManager;
1416
use OCP\EventDispatcher\IEventDispatcher;
1517
use OCP\IAppConfig;
@@ -19,6 +21,8 @@
1921
use OCP\Settings\DeclarativeSettingsTypes;
2022
use OCP\Settings\Events\DeclarativeSettingsSetValueEvent;
2123
use OCP\Settings\IDeclarativeManager;
24+
use OCP\Settings\IDeclarativeSettingsForm;
25+
use OCP\Settings\IDeclarativeSettingsFormWithHandlers;
2226
use PHPUnit\Framework\MockObject\MockObject;
2327
use Psr\Log\LoggerInterface;
2428
use Test\TestCase;
@@ -52,6 +56,8 @@ class DeclarativeManagerTest extends TestCase {
5256
/** @var IUser|MockObject */
5357
private $adminUser;
5458

59+
private IDeclarativeSettingsForm&MockObject $closureForm;
60+
5561
public const validSchemaAllFields = [
5662
'id' => 'test_form_1',
5763
'priority' => 10,
@@ -518,4 +524,74 @@ public function testAdminFormUserUnauthorized(): void {
518524
$this->expectException(\Exception::class);
519525
$this->declarativeManager->getFormsWithValues($this->user, $schema['section_type'], $schema['section_id']);
520526
}
527+
528+
/**
529+
* Ensure that the `setValue` method is called if the form implements the handler interface.
530+
*/
531+
public function testSetValueWithHandler(): void {
532+
$schema = self::validSchemaAllFields;
533+
$schema['storage_type'] = DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL;
534+
535+
$form = $this->createMock(IDeclarativeSettingsFormWithHandlers::class);
536+
$form->expects(self::atLeastOnce())
537+
->method('getSchema')
538+
->willReturn($schema);
539+
// The setter should be called once!
540+
$form->expects(self::once())
541+
->method('setValue')
542+
->with('test_field_2', 'some password', $this->adminUser);
543+
544+
\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
545+
546+
$context = $this->createMock(RegistrationContext::class);
547+
$context->expects(self::atLeastOnce())
548+
->method('getDeclarativeSettings')
549+
->willReturn([new ServiceRegistration('testing', 'OCA\\Testing\\Settings\\DeclarativeForm')]);
550+
551+
$this->coordinator->expects(self::atLeastOnce())
552+
->method('getRegistrationContext')
553+
->willReturn($context);
554+
555+
$this->declarativeManager->loadSchemas();
556+
557+
$this->eventDispatcher->expects(self::never())
558+
->method('dispatchTyped');
559+
560+
$this->declarativeManager->setValue($this->adminUser, 'testing', 'test_form_1', 'test_field_2', 'some password');
561+
}
562+
563+
public function testGetValueWithHandler(): void {
564+
$schema = self::validSchemaAllFields;
565+
$schema['storage_type'] = DeclarativeSettingsTypes::STORAGE_TYPE_EXTERNAL;
566+
567+
$form = $this->createMock(IDeclarativeSettingsFormWithHandlers::class);
568+
$form->expects(self::atLeastOnce())
569+
->method('getSchema')
570+
->willReturn($schema);
571+
// The setter should be called once!
572+
$form->expects(self::once())
573+
->method('getValue')
574+
->with('test_field_2', $this->adminUser)
575+
->willReturn('very secret password');
576+
577+
\OC::$server->registerService('OCA\\Testing\\Settings\\DeclarativeForm', fn () => $form, false);
578+
579+
$context = $this->createMock(RegistrationContext::class);
580+
$context->expects(self::atLeastOnce())
581+
->method('getDeclarativeSettings')
582+
->willReturn([new ServiceRegistration('testing', 'OCA\\Testing\\Settings\\DeclarativeForm')]);
583+
584+
$this->coordinator->expects(self::atLeastOnce())
585+
->method('getRegistrationContext')
586+
->willReturn($context);
587+
588+
$this->declarativeManager->loadSchemas();
589+
590+
$this->eventDispatcher->expects(self::never())
591+
->method('dispatchTyped');
592+
593+
$password = $this->invokePrivate($this->declarativeManager, 'getValue', [$this->adminUser, 'testing', 'test_form_1', 'test_field_2']);
594+
self::assertEquals('very secret password', $password);
595+
}
596+
521597
}

0 commit comments

Comments
 (0)