As part of converting field api to CMI we delete and then save a new configuration object - see comment #4 on #1738284: Field API CMI conversion.

This should be a single atomic rename operation instead as both of our config storage class support this - update for the database and rename for the file.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

Status: Active » Needs review
FileSize
5.44 KB

Patch

alexpott’s picture

Additional tests to confirm errors throw when renaming a config object that (a) doesn't exist and (b) to one that already exists.

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Solid, did a quick test with the rename issue that came up in the field cmi conversion and works fine!

sun’s picture

I reviewed this as well, looks good to go :)

catch’s picture

Status: Reviewed & tested by the community » Fixed

Looks good. Committed/pushed to 8.x.

tstoeckler’s picture

Status: Fixed » Needs work
+++ b/core/lib/Drupal/Core/Config/FileStorage.php
@@ -108,6 +108,17 @@ class FileStorage implements StorageInterface {
   /**
+   * Implements Drupal\Core\Config\StorageInterface::delete().
+   */
+  public function rename($name, $new_name) {

That's a copy paste error for the function documentation.

Marking needs work for that.

Also, why does the NullStorage not allow renaming? It seems that wouldn't hurt anything, right?

alexpott’s picture

@tstoeckler Good catch... will post a follow up... NullStorage has to implement the rename() method because it implements StorageInterface

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
578 bytes

Done... since this is just a a simple code comment fix marking as RTBC

tstoeckler’s picture

Right, I get that. But NullStorage::rename returns FALSE, so that in Config::rename $this->name does not get updated. What I was trying to say was that NullStorage::rename might as well return TRUE. I'll open a new issue for that, though, if it bothers me enough. :-)

Agree with RTBC.

alexpott’s picture

It returns false because it is a null object and therefore does not exist so you can't rename it... that's why delete does the same thing :)

  /**
   * Implements Drupal\Core\Config\StorageInterface::delete().
   */
  public function delete($name) {
    return FALSE;
  }
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed the follow-up.

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Fixing tag