Skip to content
Snippets Groups Projects
Commit 09b18e5e authored by catch's avatar catch
Browse files

Issue #3399295 by alexpott, Wim Leers, bircher, andypost, borisson_: Allow all... 

Issue #3399295 by alexpott, Wim Leers, bircher, andypost, borisson_: Allow all callables in ConfigTarget
parent 0940ba00
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 185 additions and 53 deletions
core/lib/Drupal/Core/Form/ConfigFormBase.php
+ 14
− 12
View file @ 09b18e5e
......@@ -99,7 +99,7 @@ public function loadDefaultValuesFromConfig(array $element): array {
$value = $this->config($target->configName)->get($target->propertyPath);
if ($target->fromConfig) {
$value = call_user_func($target->fromConfig, $value);
$value = ($target->fromConfig)($value);
}
$element['#default_value'] = $value;
}
......@@ -134,11 +134,10 @@ public function storeConfigKeyToFormElementMap(array $element, FormStateInterfac
$map = $form_state->get(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP) ?? [];
$target = $element['#config_target'];
if (is_string($target)) {
$target = ConfigTarget::fromString($target);
if ($target instanceof ConfigTarget) {
$target = $target->configName . ':' . $target->propertyPath;
}
$target->elementParents = $element['#parents'];
$map[$target->configName . ':' . $target->propertyPath] = $target;
$map[$target] = $element['#array_parents'];
$form_state->set(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP, $map);
}
foreach (Element::children($element) as $key) {
......@@ -158,7 +157,7 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
foreach ($this->getEditableConfigNames() as $config_name) {
$config = $this->config($config_name);
try {
static::copyFormValuesToConfig($config, $form_state);
static::copyFormValuesToConfig($config, $form_state, $form);
}
catch (\BadMethodCallException $e) {
// Nothing to do: this config form does not yet use validation
......@@ -193,7 +192,8 @@ public function validateForm(array &$form, FormStateInterface $form_state) {
}
if (isset($map["$config_name:$property_path"])) {
$form_element_name = implode('][', $map["$config_name:$property_path"]->elementParents);
$config_target = ConfigTarget::fromForm($map["$config_name:$property_path"], $form);
$form_element_name = implode('][', $config_target->elementParents);
}
else {
// We cannot determine where to place the violation. The only option
......@@ -264,7 +264,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
foreach ($this->getEditableConfigNames() as $config_name) {
$config = $this->config($config_name);
try {
static::copyFormValuesToConfig($config, $form_state);
static::copyFormValuesToConfig($config, $form_state, $form);
$config->save();
}
catch (\BadMethodCallException $e) {
......@@ -287,10 +287,12 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
* The configuration being edited.
* @param \Drupal\Core\Form\FormStateInterface $form_state
* The current state of the form.
* @param array $form
* The form array.
*
* @see \Drupal\Core\Entity\EntityForm::copyFormValuesToEntity()
*/
private static function copyFormValuesToConfig(Config $config, FormStateInterface $form_state): void {
private static function copyFormValuesToConfig(Config $config, FormStateInterface $form_state, array $form): void {
$map = $form_state->get(static::CONFIG_KEY_TO_FORM_ELEMENT_MAP);
// If there's no map of config keys to form elements, this form does not
// yet support config validation.
......@@ -299,12 +301,12 @@ private static function copyFormValuesToConfig(Config $config, FormStateInterfac
throw new \BadMethodCallException();
}
/** @var \Drupal\Core\Form\ConfigTarget $target */
foreach ($map as $target) {
foreach ($map as $element_parents) {
$target = ConfigTarget::fromForm($element_parents, $form);
if ($target->configName === $config->getName()) {
$value = $form_state->getValue($target->elementParents);
if ($target->toConfig) {
$value = call_user_func($target->toConfig, $value);
$value = ($target->toConfig)($value);
}
$config->set($target->propertyPath, $value);
}
......
core/lib/Drupal/Core/Form/ConfigTarget.php
+ 51
− 17
View file @ 09b18e5e
......@@ -4,6 +4,8 @@
namespace Drupal\Core\Form;
use Drupal\Component\Utility\NestedArray;
/**
* Represents the mapping of a config property to a form element.
*/
......@@ -21,6 +23,20 @@ final class ConfigTarget {
*/
public array $elementParents;
/**
* Transforms a value loaded from config before it gets displayed by the form.
*
* @var \Closure|null
*/
public readonly ?\Closure $fromConfig;
/**
* Transforms a value submitted by the form before it is set in the config.
*
* @var \Closure|null
*/
public readonly ?\Closure $toConfig;
/**
* Constructs a ConfigTarget object.
*
......@@ -29,11 +45,11 @@ final class ConfigTarget {
* `system.site`.
* @param string $propertyPath
* The property path being read or written, e.g., `page.front`.
* @param string|null $fromConfig
* @param callable|null $fromConfig
* (optional) A callback which should transform the value loaded from
* config before it gets displayed by the form. If NULL, no transformation
* will be done. Defaults to NULL.
* @param string|null $toConfig
* @param callable|null $toConfig
* (optional) A callback which should transform the value submitted by the
* form before it is set in the config object. If NULL, no transformation
* will be done. Defaults to NULL.
......@@ -41,22 +57,11 @@ final class ConfigTarget {
public function __construct(
public readonly string $configName,
public readonly string $propertyPath,
public readonly ?string $fromConfig = NULL,
public readonly ?string $toConfig = NULL,
?callable $fromConfig = NULL,
?callable $toConfig = NULL,
) {
// If they're passed at all, $fromConfig and $toConfig need to be string
// callables in order to guarantee that this object can be serialized as
// part of a larger form array. If these could be arrays, then they could be
// in the form of [$object, 'method'], which would break serialization if
// $object was not serializable. This is also why we don't type hint these
// parameters as ?callable, since that would allow closures (which can't
// be serialized).
if ($fromConfig) {
assert(is_callable($fromConfig));
}
if ($toConfig) {
assert(is_callable($toConfig));
}
$this->fromConfig = $fromConfig ? $fromConfig(...) : NULL;
$this->toConfig = $toConfig ? $toConfig(...) : NULL;
}
/**
......@@ -83,4 +88,33 @@ public static function fromString(string $target, ?string $fromConfig = NULL, ?s
return new self($configName, $propertyPath, $fromConfig, $toConfig);
}
/**
* Gets the config target object for an element from a form array.
*
* @param array $array_parents
* The array to locate the element in the form.
* @param array $form
* The form array.
*
* @return self
* A ConfigTarget instance.
*/
public static function fromForm(array $array_parents, array $form): self {
$element = NestedArray::getValue($form, $array_parents);
if (!isset($element['#config_target'])) {
throw new \LogicException('The form element [' . implode('][', $array_parents) . '] does not have the #config_target property set');
}
$target = $element['#config_target'];
if (is_string($target)) {
$target = ConfigTarget::fromString($target);
}
if (!$target instanceof ConfigTarget) {
throw new \LogicException('The form element [' . implode('][', $array_parents) . '] #config_target property is not a string or a ConfigTarget object');
}
// Add the element information to the config target object.
$target->elementParents = $element['#parents'];
return $target;
}
}
core/modules/jsonapi/src/Form/JsonApiSettingsForm.php
+ 5
− 6
View file @ 09b18e5e
......@@ -35,17 +35,16 @@ public function buildForm(array $form, FormStateInterface $form_state) {
'#type' => 'radios',
'#title' => $this->t('Allowed operations'),
'#options' => [
1 => $this->t('Accept only JSON:API read operations.'),
0 => $this->t('Accept all JSON:API create, read, update, and delete operations.'),
'r' => $this->t('Accept only JSON:API read operations.'),
'rw' => $this->t('Accept all JSON:API create, read, update, and delete operations.'),
],
'#config_target' => new ConfigTarget(
'jsonapi.settings',
'read_only',
// Convert the value to an integer when displaying the config value in
// the form.
'intval',
// Convert the bool config value to an expected string.
fn($value) => $value ? 'r' : 'rw',
// Convert the submitted value to a boolean before storing it in config.
'boolval',
fn($value) => $value === 'r',
),
'#description' => $this->t('Warning: Only enable all operations if the site requires it. <a href=":docs">Learn more about securing your site with JSON:API.</a>', [':docs' => 'https://www.drupal.org/docs/8/modules/jsonapi/security-considerations']),
];
......
core/modules/jsonapi/tests/src/Functional/SettingsFormTest.php
+ 4
− 4
View file @ 09b18e5e
......@@ -29,15 +29,15 @@ public function testSettingsForm(): void {
$this->drupalGet('/admin/config/services/jsonapi');
$page = $this->getSession()->getPage();
$page->selectFieldOption('read_only', 0);
$page->selectFieldOption('read_only', 'rw');
$page->pressButton('Save configuration');
$assert_session = $this->assertSession();
$assert_session->pageTextContains('The configuration options have been saved.');
$assert_session->fieldValueEquals('read_only', 0);
$assert_session->fieldValueEquals('read_only', 'rw');
$page->selectFieldOption('read_only', 1);
$page->selectFieldOption('read_only', 'r');
$page->pressButton('Save configuration');
$assert_session->fieldValueEquals('read_only', '1');
$assert_session->fieldValueEquals('read_only', 'r');
$assert_session->pageTextContains('The configuration options have been saved.');
}
......
core/modules/media/src/Form/MediaSettingsForm.php
+ 1
− 14
View file @ 09b18e5e
......@@ -104,7 +104,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
'#title' => $this->t('iFrame domain'),
'#size' => 40,
'#maxlength' => 255,
'#config_target' => new ConfigTarget('media.settings', 'iframe_domain', toConfig: static::class . '::nullIfEmptyString'),
'#config_target' => new ConfigTarget('media.settings', 'iframe_domain', toConfig: fn(?string $value) => $value ?: NULL),
'#description' => $this->t('Enter a different domain from which to serve oEmbed content, including the <em>http://</em> or <em>https://</em> prefix. This domain needs to point back to this site, or existing oEmbed content may not display correctly, or at all.'),
];
......@@ -118,17 +118,4 @@ public function buildForm(array $form, FormStateInterface $form_state) {
return parent::buildForm($form, $form_state);
}
/**
* Converts an empty string to NULL.
*
* @param string|null $value
* The value to transform.
*
* @return string|null
* The given string, or NULL if it was empty.
*/
public static function nullIfEmptyString(?string $value): ?string {
return $value ?: NULL;
}
}
core/tests/Drupal/Tests/Core/Form/ConfigTargetTest.php 0 → 100644
+ 110
− 0
View file @ 09b18e5e
<?php
namespace Drupal\Tests\Core\Form;
use Drupal\Core\Form\ConfigTarget;
use Drupal\Tests\UnitTestCase;
/**
* @coversDefaultClass \Drupal\Core\Form\ConfigTarget
* @group Form
*/
class ConfigTargetTest extends UnitTestCase {
/**
* @covers ::fromForm
* @covers ::fromString
*/
public function testFromFormString() {
$form = [
'group' => [
'#type' => 'details',
'test' => [
'#type' => 'text',
'#default_value' => 'A test',
'#config_target' => 'system.site:name',
'#name' => 'test',
'#parents' => ['test'],
],
],
];
$config_target = ConfigTarget::fromForm(['group', 'test'], $form);
$this->assertSame('system.site', $config_target->configName);
$this->assertSame('name', $config_target->propertyPath);
$this->assertSame(['test'], $config_target->elementParents);
}
/**
* @covers ::fromForm
*/
public function testFromFormConfigTarget() {
$form = [
'test' => [
'#type' => 'text',
'#default_value' => 'A test',
'#config_target' => new ConfigTarget('system.site', 'admin_compact_mode', 'intval', 'boolval'),
'#name' => 'test',
'#parents' => ['test'],
],
];
$config_target = ConfigTarget::fromForm(['test'], $form);
$this->assertSame('system.site', $config_target->configName);
$this->assertSame('admin_compact_mode', $config_target->propertyPath);
$this->assertSame(['test'], $config_target->elementParents);
$this->assertSame(1, ($config_target->fromConfig)(TRUE));
$this->assertFalse(($config_target->toConfig)('0'));
}
/**
* @covers ::fromForm
* @dataProvider providerTestFromFormException
*/
public function testFromFormException(array $form, array $array_parents, string $exception_message) {
$this->expectException(\LogicException::class);
$this->expectExceptionMessage($exception_message);
ConfigTarget::fromForm($array_parents, $form);
}
public function providerTestFromFormException(): array {
return [
'No #config_target' => [
[
'test' => [
'#type' => 'text',
'#default_value' => 'A test',
],
],
['test'],
'The form element [test] does not have the #config_target property set',
],
'No #config_target nested' => [
[
'group' => [
'#type' => 'details',
'test' => [
'#type' => 'text',
'#default_value' => 'A test',
],
],
],
['group', 'test'],
'The form element [group][test] does not have the #config_target property set',
],
'Boolean #config_target nested' => [
[
'group' => [
'#type' => 'details',
'test' => [
'#type' => 'text',
'#config_target' => FALSE,
'#default_value' => 'A test',
],
],
],
['group', 'test'],
'The form element [group][test] #config_target property is not a string or a ConfigTarget object',
],
];
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment