Issue #2757427 by alexpott, Xano: Array was modified by the user comparison function in ConfigDependencyManager::getDependentEntities()

core/lib/Drupal/Core/Config/Entity/ConfigDependencyManager.php

@@ -187,11 +187,32 @@ class ConfigDependencyManager {
    */
   public function sortAll() {
     $graph = $this->getGraph();
-    // Sort by reverse weight and alphabetically. The most dependent entities
+    // Sort by weight and alphabetically. The most dependent entities
     // are last and entities with the same weight are alphabetically ordered.
-    uasort($graph, array($this, 'sortGraph'));
+    uasort($graph, array($this, 'sortGraphByWeight'));
     // Use array_intersect_key() to exclude modules and themes from the list.
-    return array_reverse(array_keys(array_intersect_key($graph, $this->data)));
+    return array_keys(array_intersect_key($graph, $this->data));
+  }
+
+  /**
+   * Sorts the dependency graph by weight and alphabetically.
+   *
+   * @param array $a
+   *   First item for comparison. The compared items should be associative
+   *   arrays that include a 'weight' and a 'name' key.
+   * @param array $b
+   *   Second item for comparison.
+   *
+   * @return int
+   *   The comparison result for uasort().
+   */
+  protected function sortGraphByWeight(array $a, array $b) {
+    $weight_cmp = SortArray::sortByKeyInt($a, $b, 'weight');
+
+    if ($weight_cmp === 0) {
+      return SortArray::sortByKeyString($a, $b, 'name');
+    }
+    return $weight_cmp;
   }
 
   /**
@@ -199,7 +220,7 @@ class ConfigDependencyManager {
    *
    * @param array $a
    *   First item for comparison. The compared items should be associative
-   *   arrays that include a 'weight' and a 'component' key.
+   *   arrays that include a 'weight' and a 'name' key.
    * @param array $b
    *   Second item for comparison.
    *
@@ -210,7 +231,7 @@ class ConfigDependencyManager {
     $weight_cmp = SortArray::sortByKeyInt($a, $b, 'weight') * -1;
 
     if ($weight_cmp === 0) {
-      return SortArray::sortByKeyString($a, $b, 'component');
+      return SortArray::sortByKeyString($a, $b, 'name');
     }
     return $weight_cmp;
   }
@@ -254,14 +275,20 @@ class ConfigDependencyManager {
       foreach ($this->data as $entity) {
         $graph_key = $entity->getConfigDependencyName();
         if (!isset($graph[$graph_key])) {
-          $graph[$graph_key]['edges'] = [];
+          $graph[$graph_key] = [
+            'edges' => [],
+            'name' => $graph_key,
+          ];
         }
         // Include all dependencies in the graph so that topographical sorting
         // works.
         foreach (array_merge($entity->getDependencies('config'), $entity->getDependencies('module'), $entity->getDependencies('theme')) as $dependency) {
           $graph[$dependency]['edges'][$graph_key] = TRUE;
+          $graph[$dependency]['name'] = $dependency;
         }
       }
+      // Ensure that order of the graph is consistent.
+      krsort($graph);
       $graph_object = new Graph($graph);
       $this->graph = $graph_object->searchAndSort();
     }

core/tests/Drupal/KernelTests/Core/Config/ConfigImporterTest.php

@@ -348,8 +348,8 @@ class ConfigImporterTest extends KernelTestBase {
     $updates = $this->configImporter->reset()->getStorageComparer()->getChangelist('update');
     $expected = array(
       $name_deleter,
-      $name_other,
       $name_deletee,
+      $name_other,
     );
     $this->assertIdentical($expected, $updates);
 

core/tests/Drupal/Tests/Core/Config/ConfigDependencyManagerTest.php

@@ -0,0 +1,120 @@
+<?php
+
+namespace Drupal\Tests\Core\Config;
+
+use Drupal\Core\Config\Entity\ConfigDependencyManager;
+use Drupal\Tests\UnitTestCase;
+
+/**
+ * Tests the ConfigDependencyManager class.
+ *
+ * @group Config
+ *
+ * @coversDefaultClass \Drupal\Core\Config\Entity\ConfigDependencyManager
+ */
+class ConfigDependencyManagerTest extends UnitTestCase {
+
+  /**
+   * @dataProvider providerTestSortAll
+   */
+  public function testSortAll(array $data, array $expected_order) {
+    $dependency_manager = new ConfigDependencyManager();
+    $dependency_manager->setData($data);
+    $this->assertEquals($expected_order, $dependency_manager->sortAll());
+  }
+
+  public function providerTestSortAll() {
+    $datasets[] = [
+      [
+        'provider.entity_b' => [],
+        'provider.entity_a' => [],
+      ],
+      ['provider.entity_a', 'provider.entity_b'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_a' => [],
+        'provider.entity_b' => [],
+      ],
+      ['provider.entity_a', 'provider.entity_b'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_b' => ['dependencies' => ['config' => ['provider.entity_a']]],
+        'provider.entity_a' => [],
+      ],
+      ['provider.entity_a', 'provider.entity_b'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_a' => [],
+        'provider.entity_b' => ['dependencies' => ['config' => ['provider.entity_a']]],
+      ],
+      ['provider.entity_a', 'provider.entity_b'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_b' => [],
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_b']]],
+      ],
+      ['provider.entity_b', 'provider.entity_a'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_b']]],
+        'provider.entity_b' => [],
+      ],
+      ['provider.entity_b', 'provider.entity_a'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_b']]],
+        'provider.entity_b' => [],
+        'block.block.a' => [],
+        'block.block.b' => [],
+      ],
+      ['block.block.a', 'provider.entity_b', 'block.block.b', 'provider.entity_a'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_b' => [],
+        'block.block.b' => [],
+        'block.block.a' => [],
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_b']]],
+      ],
+      ['block.block.a', 'provider.entity_b', 'block.block.b', 'provider.entity_a'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_b' => [],
+        'block.block.b' => [],
+        'block.block.a' => [],
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_b']]],
+        'provider.entity_c' => ['dependencies' => ['config' => ['block.block.a']]],
+      ],
+      ['block.block.a', 'block.block.b', 'provider.entity_b', 'provider.entity_a', 'provider.entity_c'],
+    ];
+
+    $datasets[] = [
+      [
+        'provider.entity_b' => ['dependencies' => ['module' => ['system']]],
+        'block.block.b' => [],
+        'block.block.a' => ['dependencies' => ['module' => ['system']]],
+        'provider.entity_a' => ['dependencies' => ['config' => ['provider.entity_c']]],
+        'provider.entity_c' => ['dependencies' => ['config' => ['block.block.a']]],
+      ],
+      ['block.block.b', 'block.block.a', 'provider.entity_c', 'provider.entity_a', 'provider.entity_b'],
+    ];
+
+    return $datasets;
+  }
+
+}

core/tests/Drupal/Tests/Core/Config/StorageComparerTest.php

@@ -158,8 +158,8 @@ class StorageComparerTest extends UnitTestCase {
     $this->storageComparer->createChangelist();
     $expected = array(
       'field.storage.node.body',
-      'views.view.test_view',
       'field.field.node.article.body',
+      'views.view.test_view',
     );
     $this->assertEquals($expected, $this->storageComparer->getChangelist('create'));
     $this->assertEmpty($this->storageComparer->getChangelist('delete'));
@@ -196,8 +196,8 @@ class StorageComparerTest extends UnitTestCase {
 
     $this->storageComparer->createChangelist();
     $expected = array(
-      'field.field.node.article.body',
       'views.view.test_view',
+      'field.field.node.article.body',
       'field.storage.node.body',
     );
     $this->assertEquals($expected, $this->storageComparer->getChangelist('delete'));
@@ -236,8 +236,8 @@ class StorageComparerTest extends UnitTestCase {
     $this->storageComparer->createChangelist();
     $expected = array(
       'field.storage.node.body',
-      'system.site',
       'field.field.node.article.body',
+      'system.site',
     );
     $this->assertEquals($expected, $this->storageComparer->getChangelist('update'));
     $this->assertEmpty($this->storageComparer->getChangelist('create'));