Issue #3439713 by phenaproxima, alexpott, sonfd, thejimbirch, larowlan,...

Issue #3439713 by phenaproxima, alexpott, sonfd, thejimbirch, larowlan, berdir: Deprecate using the simpleConfigUpdate action on config entities, and introduce a new setProperties action to replace the invalid use cases

core/lib/Drupal/Core/Config/Action/Plugin/ConfigAction/SetProperties.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Core\Config\Action\Plugin\ConfigAction;
+
+use Drupal\Component\Utility\NestedArray;
+use Drupal\Core\Config\Action\Attribute\ConfigAction;
+use Drupal\Core\Config\Action\ConfigActionException;
+use Drupal\Core\Config\Action\ConfigActionPluginInterface;
+use Drupal\Core\Config\ConfigManagerInterface;
+use Drupal\Core\Config\Entity\ConfigEntityInterface;
+use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+
+/**
+ * @internal
+ *   This API is experimental.
+ */
+#[ConfigAction(
+  id: 'setProperties',
+  admin_label: new TranslatableMarkup('Set property of a config entity'),
+  entity_types: ['*'],
+)]
+final class SetProperties implements ConfigActionPluginInterface, ContainerFactoryPluginInterface {
+
+  public function __construct(
+    private readonly ConfigManagerInterface $configManager,
+  ) {}
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition) {
+    return new static(
+      $container->get(ConfigManagerInterface::class),
+    );
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function apply(string $configName, mixed $values): void {
+    $entity = $this->configManager->loadConfigEntityByName($configName);
+    assert($entity instanceof ConfigEntityInterface);
+
+    assert(is_array($values));
+    assert(!array_is_list($values));
+
+    // Don't allow the ID or UUID to be changed.
+    $entity_keys = $entity->getEntityType()->getKeys();
+    $forbidden_keys = array_filter([
+      $entity_keys['id'],
+      $entity_keys['uuid'],
+    ]);
+
+    foreach ($values as $property_name => $value) {
+      if (in_array($property_name, $forbidden_keys, TRUE)) {
+        throw new ConfigActionException("Entity key '$property_name' cannot be changed by the setProperties config action.");
+      }
+      $parts = explode('.', $property_name);
+
+      $property_value = $entity->get($parts[0]);
+      if (count($parts) > 1) {
+        if (isset($property_value) && !is_array($property_value)) {
+          throw new ConfigActionException('The setProperties config action can only set nested values on arrays.');
+        }
+        $property_value ??= [];
+        NestedArray::setValue($property_value, array_slice($parts, 1), $value);
+      }
+      else {
+        $property_value = $value;
+      }
+      $entity->set($parts[0], $property_value);
+    }
+    $entity->save();
+  }
+
+}

core/lib/Drupal/Core/Config/Action/Plugin/ConfigAction/SimpleConfigUpdate.php

@@ -8,6 +8,7 @@
 use Drupal\Core\Config\Action\ConfigActionException;
 use Drupal\Core\Config\Action\ConfigActionPluginInterface;
 use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Config\ConfigManagerInterface;
 use Drupal\Core\Plugin\ContainerFactoryPluginInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -22,31 +23,31 @@
 )]
 final class SimpleConfigUpdate implements ConfigActionPluginInterface, ContainerFactoryPluginInterface {
 
-  /**
-   * Constructs a SimpleConfigUpdate object.
-   *
-   * @param \Drupal\Core\Config\ConfigFactoryInterface $configFactory
-   *   The config factory.
-   */
   public function __construct(
-    protected readonly ConfigFactoryInterface $configFactory,
-  ) {
-  }
+    private readonly ConfigFactoryInterface $configFactory,
+    private readonly ConfigManagerInterface $configManager,
+  ) {}
 
   /**
    * {@inheritdoc}
    */
   public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): static {
-    return new static($container->get('config.factory'));
+    return new static(
+      $container->get(ConfigFactoryInterface::class),
+      $container->get(ConfigManagerInterface::class),
+    );
   }
 
   /**
    * {@inheritdoc}
    */
   public function apply(string $configName, mixed $value): void {
+    if ($this->configManager->getEntityTypeIdByName($configName)) {
+      // @todo Make this an exception in https://www.drupal.org/node/3515544.
+      @trigger_error('Using the simpleConfigUpdate config action on config entities is deprecated in drupal:11.2.0 and throws an exception in drupal:12.0.0. Use the setProperties action instead. See https://www.drupal.org/node/3515543', E_USER_DEPRECATED);
+    }
+
     $config = $this->configFactory->getEditable($configName);
-    // @todo https://www.drupal.org/i/3439713 Should we error if this is a
-    //   config entity?
     if ($config->isNew()) {
       throw new ConfigActionException(sprintf('Config %s does not exist so can not be updated', $configName));
     }

core/tests/Drupal/KernelTests/Core/Recipe/ConfigActionValidationTest.php

@@ -77,7 +77,7 @@ public function testConfigActionsAreValidated(string $entity_type_id): void {
 config:
   actions:
     $config_name:
-      simpleConfigUpdate:
+      setProperties:
         $label_key: ''
 YAML;
 

core/tests/Drupal/KernelTests/Core/Recipe/EntityMethodConfigActionsTest.php

@@ -4,21 +4,27 @@
 
 namespace Drupal\KernelTests\Core\Recipe;
 
+use Drupal\block\Entity\Block;
+use Drupal\Core\Config\Action\ConfigActionException;
 use Drupal\Core\Config\Action\ConfigActionManager;
 use Drupal\Core\Entity\EntityDisplayRepositoryInterface;
 use Drupal\Core\Entity\EntityTypeManagerInterface;
+use Drupal\Core\Extension\ThemeInstallerInterface;
 use Drupal\entity_test\Entity\EntityTestBundle;
 use Drupal\KernelTests\KernelTestBase;
+use Drupal\Tests\block\Traits\BlockCreationTrait;
 
 /**
  * @group Recipe
  */
 class EntityMethodConfigActionsTest extends KernelTestBase {
 
+  use BlockCreationTrait;
+
   /**
    * {@inheritdoc}
    */
-  protected static $modules = ['config_test', 'entity_test', 'system'];
+  protected static $modules = ['block', 'config_test', 'entity_test', 'system'];
 
   /**
    * The configuration action manager.
@@ -172,4 +178,90 @@ public function testRemoveComponentFromDisplay(string $action_name): void {
     $this->assertFalse($this->configActionManager->hasDefinition($plugin_id));
   }
 
+  /**
+   * Test setting a nested property on a config entity.
+   */
+  public function testSetNestedProperty(): void {
+    $this->container->get(ThemeInstallerInterface::class)
+      ->install(['claro']);
+    $block = $this->placeBlock('local_tasks_block', ['theme' => 'claro']);
+
+    $this->configActionManager->applyAction(
+      'setProperties',
+      $block->getConfigDependencyName(),
+      ['settings.label' => 'Magic!'],
+    );
+    $settings = Block::load($block->id())->get('settings');
+    $this->assertSame('Magic!', $settings['label']);
+
+    // If the property is not nested, it should still work.
+    $settings['label'] = 'Mundane';
+    $this->configActionManager->applyAction(
+      'setProperties',
+      $block->getConfigDependencyName(),
+      ['settings' => $settings],
+    );
+    $settings = Block::load($block->id())->get('settings');
+    $this->assertSame('Mundane', $settings['label']);
+
+    // We can use this to set a scalar property normally.
+    $this->configActionManager->applyAction(
+      'setProperties',
+      $block->getConfigDependencyName(),
+      ['region' => 'highlighted'],
+    );
+    $this->assertSame('highlighted', Block::load($block->id())->getRegion());
+
+    // We should get an exception if we try to set a nested value on a property
+    // that isn't an array.
+    $this->expectException(ConfigActionException::class);
+    $this->expectExceptionMessage('The setProperties config action can only set nested values on arrays.');
+    $this->configActionManager->applyAction(
+      'setProperties',
+      $block->getConfigDependencyName(),
+      ['theme.name' => 'stark'],
+    );
+  }
+
+  /**
+   * Tests that the setProperties action refuses to modify entity IDs or UUIDs.
+   *
+   * @testWith ["id"]
+   *   ["uuid"]
+   */
+  public function testSetPropertiesWillNotChangeEntityKeys(string $key): void {
+    $view_display = $this->container->get(EntityDisplayRepositoryInterface::class)
+      ->getViewDisplay('entity_test_with_bundle', 'test');
+    $this->assertFalse($view_display->isNew());
+
+    $property_name = $view_display->getEntityType()->getKey($key);
+    $this->assertNotEmpty($property_name);
+
+    $this->expectException(ConfigActionException::class);
+    $this->expectExceptionMessage("Entity key '$property_name' cannot be changed by the setProperties config action.");
+    $this->configActionManager->applyAction(
+      'setProperties',
+      $view_display->getConfigDependencyName(),
+      [$property_name => '12345'],
+    );
+  }
+
+  /**
+   * Tests that the simpleConfigUpdate action cannot be used on entities.
+   *
+   * @group legacy
+   */
+  public function testSimpleConfigUpdateFailsOnEntities(): void {
+    $view_display = $this->container->get(EntityDisplayRepositoryInterface::class)
+      ->getViewDisplay('entity_test_with_bundle', 'test');
+    $view_display->save();
+
+    $this->expectDeprecation('Using the simpleConfigUpdate config action on config entities is deprecated in drupal:11.2.0 and throws an exception in drupal:12.0.0. Use the setProperties action instead. See https://www.drupal.org/node/3515543');
+    $this->configActionManager->applyAction(
+      'simpleConfigUpdate',
+      $view_display->getConfigDependencyName(),
+      ['hidden.uid' => TRUE],
+    );
+  }
+
 }