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.

Files: 
CommentFileSizeAuthor
#8 1739900-fix-code-comment.patch578 bytesalexpott
PASSED: [[SimpleTest]]: [MySQL] 40,359 pass(es).
[ View ]
#2 1-2-interdiff.txt1.17 KBalexpott
#2 1739900-2.drupal8.config.rename.patch6.34 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 40,255 pass(es).
[ View ]
#1 1739900.drupal8.config.rename.patch5.44 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 40,256 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new5.44 KB
PASSED: [[SimpleTest]]: [MySQL] 40,256 pass(es).
[ View ]

Patch

StatusFileSize
new6.34 KB
PASSED: [[SimpleTest]]: [MySQL] 40,255 pass(es).
[ View ]
new1.17 KB

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

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!

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

Status:Reviewed & tested by the community» Fixed

Looks good. Committed/pushed to 8.x.

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?

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

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new578 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,359 pass(es).
[ View ]

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

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.

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;
  }

Status:Reviewed & tested by the community» Fixed

Committed/pushed the follow-up.

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

Issue summary:View changes

Fixing tag