Merge pull request #7971 from dhensby/pulls/4.0/fix-extension-config

FIX Config from extension should not override

docs/en/02_Developer_Guides/04_Configuration/00_Configuration.md

@@ -153,12 +153,12 @@ the result will be the higher priority false-ish value.
 
 The locations that configuration values are taken from in highest -> lowest priority order are:
 
-- Any values set via a call to Config#merge / Config#set
+- Runtime modifications, ie: any values set via a call to `Config::inst()->update()`
 - The configuration values taken from the YAML files in `_config/` directories (internally sorted in before / after 
 order, where the item that is latest is highest priority)
-- Any static set on an "additional static source" class (such as an extension) named the same as the name of the property
 - Any static set on the class named the same as the name of the property
 - The composite configuration value of the parent class of this class
+- Any static set on an "additional static source" class (such as an extension) named the same as the name of the property
 
 <div class="notice">
 It is an error to have mixed types of the same named property in different locations. An error will not necessarily

docs/en/04_Changelogs/4.0.5.md

@@ -0,0 +1,12 @@
+# 4.0.5
+
+## Notable changes
+
+Fix [#7971](https://github.com/silverstripe/silverstripe-framework/pull/7971) introduces a subtle change of behaviour
+to how `Config` settings are prioritised. In SilverStripe 4 there was a change where `Extension` objects took the
+highest importance when determining Config values; this was deemed to be unexpected and unintuitive as well as making it
+cumbersome and difficult for developers to override module defaults defined in `Extension`s. This change reverts
+behaviour to that of SilverStripe 3 where `Extension` instances are of lowest importance and are used only to set a
+default value. If you rely on your `Extension` or module providing an overriding config value, please move this to yaml.
+
+<!--- Changes below this line will be automatically regenerated -->

src/Core/Config/Middleware/ExtensionMiddleware.php

@@ -39,7 +39,7 @@ class ExtensionMiddleware implements Middleware
         }
 
         foreach ($this->getExtraConfig($class, $config, $excludeMiddleware) as $extra) {
-            $config = Priority::mergeArray($extra, $config);
+            $config = Priority::mergeArray($config, $extra);
         }
         return $config;
     }

tests/php/ORM/DataObjectTest.php

@@ -1062,8 +1062,7 @@ class DataObjectTest extends SapphireTest
 
         // Test logical fields (including composite)
         $teamSpecifications = $schema->fieldSpecs(DataObjectTest\Team::class);
-        $this->assertEquals(
-            array(
+        $expected = array(
             'ID',
             'ClassName',
             'LastEdited',
@@ -1075,14 +1074,18 @@ class DataObjectTest extends SapphireTest
             'FounderID',
             'HasOneRelationshipID',
             'ExtendedHasOneRelationshipID'
-            ),
-            array_keys($teamSpecifications),
+        );
+        $actual = array_keys($teamSpecifications);
+        sort($expected);
+        sort($actual);
+        $this->assertEquals(
+            $expected,
+            $actual,
             'fieldSpecifications() contains all fields defined on instance: base, extended and foreign keys'
         );
 
         $teamFields = $schema->databaseFields(DataObjectTest\Team::class, false);
-        $this->assertEquals(
-            array(
+        $expected = array(
             'ID',
             'ClassName',
             'LastEdited',
@@ -1094,14 +1097,18 @@ class DataObjectTest extends SapphireTest
             'FounderID',
             'HasOneRelationshipID',
             'ExtendedHasOneRelationshipID'
-            ),
-            array_keys($teamFields),
+        );
+        $actual = array_keys($teamFields);
+        sort($expected);
+        sort($actual);
+        $this->assertEquals(
+            $expected,
+            $actual,
             'databaseFields() contains only fields defined on instance, including base, extended and foreign keys'
         );
 
         $subteamSpecifications = $schema->fieldSpecs(DataObjectTest\SubTeam::class);
-        $this->assertEquals(
-            array(
+        $expected = array(
             'ID',
             'ClassName',
             'LastEdited',
@@ -1115,19 +1122,28 @@ class DataObjectTest extends SapphireTest
             'ExtendedHasOneRelationshipID',
             'SubclassDatabaseField',
             'ParentTeamID',
-            ),
-            array_keys($subteamSpecifications),
+        );
+        $actual = array_keys($subteamSpecifications);
+        sort($expected);
+        sort($actual);
+        $this->assertEquals(
+            $expected,
+            $actual,
             'fieldSpecifications() on subclass contains all fields, including base, extended  and foreign keys'
         );
 
         $subteamFields = $schema->databaseFields(DataObjectTest\SubTeam::class, false);
-        $this->assertEquals(
-            array(
+        $expected = array(
             'ID',
             'SubclassDatabaseField',
             'ParentTeamID',
-            ),
-            array_keys($subteamFields),
+        );
+        $actual = array_keys($subteamFields);
+        sort($expected);
+        sort($actual);
+        $this->assertEquals(
+            $expected,
+            $actual,
             'databaseFields() on subclass contains only fields defined on instance'
         );
     }