diff --git a/src/Core/Config/Config.php b/src/Core/Config/Config.php index 15f1c127d..e8d5a960f 100644 --- a/src/Core/Config/Config.php +++ b/src/Core/Config/Config.php @@ -4,6 +4,7 @@ namespace SilverStripe\Core\Config; use InvalidArgumentException; use SilverStripe\Config\Collections\ConfigCollectionInterface; +use SilverStripe\Config\Collections\DeltaConfigCollection; use SilverStripe\Config\Collections\MutableConfigCollectionInterface; abstract class Config @@ -25,6 +26,13 @@ abstract class Config */ const EXCLUDE_EXTRA_SOURCES = 4; + /** + * Disable all modifications to the config + * + * @const + */ + const NO_DELTAS = 8; + /** * Get the current active Config instance. * diff --git a/src/Core/Config/CoreConfigFactory.php b/src/Core/Config/CoreConfigFactory.php index 509f17a29..9ab016616 100644 --- a/src/Core/Config/CoreConfigFactory.php +++ b/src/Core/Config/CoreConfigFactory.php @@ -4,6 +4,7 @@ namespace SilverStripe\Core\Config; use Psr\SimpleCache\CacheInterface; use SilverStripe\Config\Collections\CachedConfigCollection; +use SilverStripe\Config\Collections\DeltaConfigCollection; use SilverStripe\Config\Collections\MemoryConfigCollection; use SilverStripe\Config\Transformer\PrivateStaticTransformer; use SilverStripe\Config\Transformer\YamlTransformer; @@ -47,6 +48,11 @@ class CoreConfigFactory { $instance = new CachedConfigCollection(); + // Override nested config to use delta collection + $instance->setNestFactory(function ($collection) { + return DeltaConfigCollection::createFromCollection($collection, Config::NO_DELTAS); + }); + // Create config cache if ($this->cacheFactory) { $cache = $this->cacheFactory->create(CacheInterface::class . '.configcache', [ diff --git a/src/Core/Config/Middleware/ExtensionMiddleware.php b/src/Core/Config/Middleware/ExtensionMiddleware.php index c640d63cf..ef64c2994 100644 --- a/src/Core/Config/Middleware/ExtensionMiddleware.php +++ b/src/Core/Config/Middleware/ExtensionMiddleware.php @@ -6,6 +6,7 @@ use Generator; use InvalidArgumentException; use SilverStripe\Config\MergeStrategy\Priority; use SilverStripe\Config\Middleware\Middleware; +use SilverStripe\Config\Middleware\MiddlewareCommon; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Extension; @@ -15,6 +16,11 @@ class ExtensionMiddleware implements Middleware { use MiddlewareCommon; + public function __construct($disableFlag = 0) + { + $this->setDisableFlag($disableFlag); + } + /** * Get config for a class * @@ -32,7 +38,7 @@ class ExtensionMiddleware implements Middleware return $config; } - foreach ($this->getExtraConfig($class, $config) as $extra) { + foreach ($this->getExtraConfig($class, $config, $excludeMiddleware) as $extra) { $config = Priority::mergeArray($extra, $config); } return $config; @@ -43,15 +49,19 @@ class ExtensionMiddleware implements Middleware * * @param string $class * @param array $classConfig + * @param int $excludeMiddleware * @return Generator */ - protected function getExtraConfig($class, $classConfig) + protected function getExtraConfig($class, $classConfig, $excludeMiddleware) { - if (empty($classConfig['extensions'])) { + // Note: 'extensions' config needs to come from it's own middleware call in case + // applied by delta middleware (e.g. Object::add_extension) + $extensionSourceConfig = Config::inst()->get($class, null, Config::UNINHERITED | $excludeMiddleware | $this->disableFlag); + if (empty($extensionSourceConfig['extensions'])) { return; } - $extensions = $classConfig['extensions']; + $extensions = $extensionSourceConfig['extensions']; foreach ($extensions as $extension) { list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension); if (!class_exists($extensionClass)) { @@ -72,7 +82,11 @@ class ExtensionMiddleware implements Middleware // continue } // Merge config from extension - $extensionConfig = Config::inst()->get($extensionClassParent, null, true); + $extensionConfig = Config::inst()->get( + $extensionClassParent, + null, + Config::EXCLUDE_EXTRA_SOURCES | Config::UNINHERITED + ); if ($extensionConfig) { yield $extensionConfig; } diff --git a/src/Core/Config/Middleware/InheritanceMiddleware.php b/src/Core/Config/Middleware/InheritanceMiddleware.php index 80654771b..ea394795c 100644 --- a/src/Core/Config/Middleware/InheritanceMiddleware.php +++ b/src/Core/Config/Middleware/InheritanceMiddleware.php @@ -4,12 +4,18 @@ namespace SilverStripe\Core\Config\Middleware; use SilverStripe\Config\MergeStrategy\Priority; use SilverStripe\Config\Middleware\Middleware; -use SilverStripe\Core\ClassInfo; +use SilverStripe\Config\Middleware\MiddlewareCommon; +use SilverStripe\Core\Config\Config; class InheritanceMiddleware implements Middleware { use MiddlewareCommon; + public function __construct($disableFlag = 0) + { + $this->setDisableFlag($disableFlag); + } + /** * Get config for a class * @@ -20,18 +26,20 @@ class InheritanceMiddleware implements Middleware */ public function getClassConfig($class, $excludeMiddleware, $next) { - // Check if enabled + // Skip if disabled + $config = $next($class, $excludeMiddleware); if (!$this->enabled($excludeMiddleware)) { - return $next($class, $excludeMiddleware); + return $config; } - // Merge hierarchy - $config = []; - foreach (ClassInfo::ancestry($class) as $nextClass) { - $nextConfig = $next($nextClass, $excludeMiddleware); - $config = Priority::mergeArray($nextConfig, $config); + // Skip if no parent class + $parent = get_parent_class($class); + if (!$parent) { + return $config; } - return $config; + // Merge with parent class + $parentConfig = Config::inst()->get($parent, null, $excludeMiddleware); + return Priority::mergeArray($config, $parentConfig); } } diff --git a/src/Core/Config/Middleware/MiddlewareCommon.php b/src/Core/Config/Middleware/MiddlewareCommon.php deleted file mode 100644 index 298346b78..000000000 --- a/src/Core/Config/Middleware/MiddlewareCommon.php +++ /dev/null @@ -1,48 +0,0 @@ -disableFlag = $disableFlag; - } - - /** - * Check if this middlware is enabled - * - * @param int|true $excludeMiddleware - * @return bool - */ - protected function enabled($excludeMiddleware) - { - if ($excludeMiddleware === true) { - return false; - } - if (!$this->disableFlag) { - return true; - } - return ($excludeMiddleware & $this->disableFlag) !== $this->disableFlag; - } - - public function serialize() - { - return json_encode([$this->disableFlag]); - } - - public function unserialize($serialized) - { - list($this->disableFlag) = json_decode($serialized, true); - } -} diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index b5f4c5efa..7eb30ab1b 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -247,7 +247,7 @@ trait Extensible // Build filtered extension list $found = false; - $config = Config::inst()->get($class, 'extensions', true) ?: []; + $config = Config::inst()->get($class, 'extensions', Config::EXCLUDE_EXTRA_SOURCES | Config::UNINHERITED) ?: []; foreach ($config as $key => $candidate) { // extensions with parameters will be stored in config as ExtensionName("Param"). if (strcasecmp($candidate, $extension) === 0 || @@ -332,7 +332,7 @@ trait Extensible $sources = null; // Get a list of extensions - $extensions = Config::inst()->get($class, 'extensions', true); + $extensions = Config::inst()->get($class, 'extensions', Config::EXCLUDE_EXTRA_SOURCES | Config::UNINHERITED); if (!$extensions) { return null; @@ -547,7 +547,7 @@ trait Extensible if (in_array($class, self::$unextendable_classes)) { continue; } - $extensions = Config::inst()->get($class, 'extensions', true); + $extensions = Config::inst()->get($class, 'extensions', Config::UNINHERITED | Config::EXCLUDE_EXTRA_SOURCES); if ($extensions) { foreach ($extensions as $extension) { diff --git a/tests/php/Core/Config/ConfigTest.php b/tests/php/Core/Config/ConfigTest.php index aa57435fe..38409ad52 100644 --- a/tests/php/Core/Config/ConfigTest.php +++ b/tests/php/Core/Config/ConfigTest.php @@ -33,6 +33,7 @@ class ConfigTest extends SapphireTest public function testUpdateStatic() { + // Test base state $this->assertEquals( ['test_1'], Config::inst()->get(ConfigTest\First::class, 'first') @@ -46,7 +47,7 @@ class ConfigTest extends SapphireTest ); $this->assertEquals( [ 'test_2' ], - Config::inst()->get(ConfigTest\Second::class, 'first', true) + Config::inst()->get(ConfigTest\Second::class, 'first', Config::UNINHERITED) ); $this->assertEquals( [ @@ -61,59 +62,108 @@ class ConfigTest extends SapphireTest Config::inst()->get(ConfigTest\Third::class, 'first', true) ); + // Modify first param Config::modify()->merge(ConfigTest\First::class, 'first', array('test_1_2')); Config::modify()->merge(ConfigTest\Third::class, 'first', array('test_3_2')); Config::modify()->merge(ConfigTest\Fourth::class, 'first', array('test_4')); + // Check base class $this->assertEquals( ['test_1', 'test_1_2'], Config::inst()->get(ConfigTest\First::class, 'first') ); $this->assertEquals( ['test_1', 'test_1_2'], - Config::inst()->get(ConfigTest\First::class, 'first', true) + Config::inst()->get(ConfigTest\First::class, 'first', Config::UNINHERITED) + ); + $this->assertEquals( + ['test_1'], + Config::inst()->get(ConfigTest\First::class, 'first', Config::NO_DELTAS) + ); + $this->assertEquals( + ['test_1'], + Config::inst()->get(ConfigTest\First::class, 'first', Config::NO_DELTAS | Config::UNINHERITED) ); + // Modify second param Config::modify()->merge(ConfigTest\Fourth::class, 'second', array('test_4')); Config::modify()->merge(ConfigTest\Third::class, 'second', array('test_3_2')); + // Check fourth class $this->assertEquals( ['test_1', 'test_3', 'test_3_2', 'test_4'], Config::inst()->get(ConfigTest\Fourth::class, 'second') ); $this->assertEquals( ['test_4'], - Config::inst()->get(ConfigTest\Fourth::class, 'second', true) + Config::inst()->get(ConfigTest\Fourth::class, 'second', Config::UNINHERITED) ); + $this->assertEquals( + ['test_1', 'test_3'], + Config::inst()->get(ConfigTest\Fourth::class, 'second', Config::NO_DELTAS) + ); + $this->assertEquals( + null, + Config::inst()->get(ConfigTest\Fourth::class, 'second', Config::NO_DELTAS | Config::UNINHERITED) + ); + + // Check third class $this->assertEquals( ['test_1', 'test_3', 'test_3_2'], Config::inst()->get(ConfigTest\Third::class, 'second') ); $this->assertEquals( ['test_3', 'test_3_2'], - Config::inst()->get(ConfigTest\Third::class, 'second', true) + Config::inst()->get(ConfigTest\Third::class, 'second', Config::UNINHERITED) + ); + $this->assertEquals( + ['test_1', 'test_3'], + Config::inst()->get(ConfigTest\Third::class, 'second', Config::NO_DELTAS) + ); + $this->assertEquals( + ['test_3'], + Config::inst()->get(ConfigTest\Third::class, 'second', Config::NO_DELTAS | Config::UNINHERITED) ); + // Test remove() Config::modify()->remove(ConfigTest\Third::class, 'second'); + + // Check third class ->get() $this->assertEquals( - ['test_1'], + null, Config::inst()->get(ConfigTest\Third::class, 'second') ); - $this->assertTrue( - Config::inst()->exists(ConfigTest\Third::class, 'second') + $this->assertEquals( + ['test_1', 'test_3'], + Config::inst()->get(ConfigTest\Third::class, 'second', Config::NO_DELTAS) ); $this->assertEquals( null, - Config::inst()->get(ConfigTest\Third::class, 'second', true) + Config::inst()->get(ConfigTest\Third::class, 'second', Config::UNINHERITED) + ); + + // Check ->exists() + $this->assertFalse( + Config::inst()->exists(ConfigTest\Third::class, 'second') ); $this->assertFalse( - Config::inst()->exists(ConfigTest\Third::class, 'second', true) + Config::inst()->exists(ConfigTest\Third::class, 'second', Config::UNINHERITED) ); + $this->assertTrue( + Config::inst()->exists(ConfigTest\Third::class, 'second', Config::NO_DELTAS) + ); + + // Test merge() Config::modify()->merge(ConfigTest\Third::class, 'second', ['test_3_2']); $this->assertEquals( - ['test_1', 'test_3_2'], + ['test_3_2'], Config::inst()->get(ConfigTest\Third::class, 'second') ); + // No-deltas omits both above ->remove() as well as ->merge() + $this->assertEquals( + ['test_1', 'test_3'], + Config::inst()->get(ConfigTest\Third::class, 'second', Config::NO_DELTAS) + ); } public function testUpdateWithFalsyValues()