Add an in-memory config storage

Looks good. Just some nit picks :)

1. +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,182 @@
   +    if (is_null($this->config)) {
   +      $this->config = new \ArrayObject();
   

   Nitpick but why are you setting the config first and then checking if that is NULL? You could do in one op, like:

   if (is_null($config)) {
     $config = new \ArrayObject();
   }
   

2. +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,182 @@
   +    else {
   

   No need for an else here. You can just return the array_keys if the prefix is empty. And then just start looping if you do have a prefix.

3. +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,182 @@
   +    else {
   

   Same here. You can just return, no need for an else.

4. +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,182 @@
   +     // Exclude the default collection and empty collections
   

   Missing period.

5. +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,182 @@
   +     if ($name != StorageInterface::DEFAULT_COLLECTION && !empty($data)) {
   

   !==

6. +++ b/core/tests/Drupal/KernelTests/Core/Config/Storage/MemoryStorageTest.php
   @@ -0,0 +1,60 @@
   +    // Create a directory.
   

   Left over comment?

Looks good, thanks for the fixes.

Apologies for the pedantry. Still need to get my head around the issue this blocks before being able to do a proper review here.

Fixing reference to use case memory storage.

+++ b/core/tests/Drupal/KernelTests/Core/Config/Storage/MemoryStorageTest.php
@@ -0,0 +1,59 @@
+  public function testInvalidStorage() {
+    // No-op as this test does not make sense.
+  }

The only reason this is not marked as a risky test is because \Drupal\KernelTests\KernelTestBase::assertPostConditions makes an assertion. Should do $this->markTestSkipped('MemoryStorage can not be invalid');

Here you go.

Back to rtbc, @alexpott's remark was fixed in #10.

+++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
@@ -0,0 +1,179 @@
+   * @param \ArrayAccess $config
+   *   (optional) The configuration in the storage.
+   */
+  public function __construct($collection = StorageInterface::DEFAULT_COLLECTION, \ArrayAccess $config = NULL) {
+    if (is_null($config)) {
+      $config = new \ArrayObject();
+    }
+    $this->collection = $collection;
+    $this->config = $config;
+    if (!isset($this->config[$collection])) {
+      $this->config[$collection] = [];
+    }
+  }
...
+  /**
+   * {@inheritdoc}
+   */
+  public function createCollection($collection) {
+    return new static(
+      $collection,
+      $this->config
+    );
+  }

I'm not sure about injecting $config into constructor. If this is only for colections then we could do

  /**
   * {@inheritdoc}
   */
  public function createCollection($collection) {
    if (!isset($this->config[$collection])) {
      $this->config[$collection] = [];
    }
    $collection = new static($collection);
    $collection->config = $this->config;
    return $collection;
  }

instead since you can access protected properties in that case.

If it is not then we should add additional test coverage in MemoryStorageTest of pre-populating MemoryStorage from something that implements ArrayAccess.

@bircher for testing purposes you don't need to have something unnecessary on the constructor. You can use reflection is you want. Personally I prefer #13

@bircher ok but then if we instantiate like this, it means that $this->config will no longer keep track of multiple collections but we will have a new instance for another collection (which duplicates whatever already exists inside). This also means that things like $this->config[$this->collection][$name] don't make sense anymore across the methods.

This is a big change from the initial implementation in which the storage kept track internally of its collections.

So this wont allow pre-populating the memory storage with multiple collections that are different (for example mimic'ing translations).

Aah, nevermind. Long day :)

Yupp, looks good to me!

Had failed for some reason. Back to RTBC

1. Extremely minor but this comment...

   +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,183 @@
   +    if (isset($this->config[$this->collection][$name])) {
   +      unset($this->config[$this->collection][$name]);
   +      // Unset the whole collection if it was the last config to be deleted.
   +      if (empty($this->config[$this->collection])) {
   +        $this->config->offsetUnset($this->collection);
   +      }
   +      return TRUE;
   

   ..

2. ... is much clearer/better worded than this one:

   +++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
   @@ -0,0 +1,183 @@
   +      if (strpos($name, $prefix) === 0) {
   +        unset($this->config[$this->collection][$name]);
   +      }
   +    }
   +    // Unset also the collection if it is empty now.
   +    if (empty($this->config[$this->collection])) {
   +      $this->config->offsetUnset($this->collection);
   +    }
   +
   

   So I think we should replace the second comment with exactly what's in the first. Leaving RTBC because that could be done on commit.

Some improvements though using array functions over foreach and addressed #23.

Also I've discovered an inconsistency in how CRUD operations occur on empty collections. This inconsistency extends to \Drupal\Core\Config\FileStorage and \Drupal\Core\Config\DatabaseStorage so it does not block this issue. Opened #3020964: Empty collection config CRUD operations are inconsistent across the backend.

Hmmm... as a result of working on #3020964: Empty collection config CRUD operations are inconsistent across the backend. I've got a few more fixes. We might want to land that first.

Ah patch attached now passes the latest test on #3020964: Empty collection config CRUD operations are inconsistent across the backend.

Rerolled now that #3020964: Empty collection config CRUD operations are inconsistent across the backend. has landed.

Reviewed #28 and looks good here.

+++ b/core/lib/Drupal/Core/Config/MemoryStorage.php
@@ -135,12 +132,12 @@ public function deleteAll($prefix = '') {
+    foreach ($this->config[$this->collection] as $name => $data) {

Doesn't this result in $data being an unused variable? I personally don't mind this vs. an array_keys() call but I think we had a load of patches cleaning this up at one point.

@catch not according to PHPStorm - it's when you don't use $name that you'd get an unused var.

At some point I guess this can turn into an array_filter() but not until we require at least PHP 5.6.

Let's make the change - these variables to think about is always a good thing.

Committed 243e979 and pushed to 8.7.x. Thanks!