FIX changing environment in config.php changes matched yaml rules

core/Config.php

@@ -295,8 +295,12 @@ class Config {
 	 * instead, the last manifest to be added always wins
 	 */
 	public function pushConfigYamlManifest(SS_ConfigManifest $manifest) {
-		array_unshift($this->manifests, $manifest->yamlConfig);
+		array_unshift($this->manifests, $manifest);
+
+		// Now that we've got another yaml config manifest we need to clean the cache
 		$this->cache->clean();
+		// We also need to clean the cache if the manifest's calculated config values change
+		$manifest->registerChangeCallback(array($this->cache, 'clean'));
 
 		// @todo: Do anything with these. They're for caching after config.php has executed
 		$this->collectConfigPHPSettings = true;
@@ -479,10 +483,13 @@ class Config {
 			}
 		}
 
+		$value = $nothing = null;
+
 		// Then the manifest values
 		foreach($this->manifests as $manifest) {
-			if (isset($manifest[$class][$name])) {
-				self::merge_low_into_high($result, $manifest[$class][$name], $suppress);
+			$value = $manifest->get($class, $name, $nothing);
+			if ($value !== $nothing) {
+				self::merge_low_into_high($result, $value, $suppress);
 				if ($result !== null && !is_array($result)) return $result;
 			}
 		}

core/manifest/ConfigManifest.php

@@ -9,7 +9,16 @@
  */
 class SS_ConfigManifest {
 
-	/** 
+	/** @var string - The base path used when building the manifest */
+	protected $base;
+
+	/** @var string - A string to prepend to all cache keys to ensure all keys are unique to just this $base */
+	protected $key;
+
+	/** @var bool - Whether `test` directories should be searched when searching for configuration */
+	protected $includeTests;
+
+	/**
 	  * All the values needed to be collected to determine the correct combination of fragements for
 	  * the current environment.
 	  * @var array
@@ -34,6 +43,17 @@ class SS_ConfigManifest {
 	 */
 	public $yamlConfig = array();
 
+	/**
+	 * The variant key state as when yamlConfig was loaded
+	 * @var string
+	 */
+	protected $yamlConfigVariantKey = null;
+
+	/**
+	 * @var [callback] A list of callbacks to be called whenever the content of yamlConfig changes
+	 */
+	protected $configChangeCallbacks = array();
+
 	/**
 	 * A side-effect of collecting the _config fragments is the calculation of all module directories, since
 	 * the definition of a module is "a directory that contains either a _config.php file or a _config directory
@@ -65,6 +85,7 @@ class SS_ConfigManifest {
 	public function __construct($base, $includeTests = false, $forceRegen = false ) {
 		$this->base = $base;
 		$this->key = sha1($base).'_';
+		$this->includeTests = $includeTests;
 
 		// Get the Zend Cache to load/store cache into
 		$this->cache = SS_Cache::factory('SS_Configuration', 'Core', array(
@@ -78,21 +99,28 @@ class SS_ConfigManifest {
 			$this->phpConfigSources = $this->cache->load($this->key.'php_config_sources');
 			// Get the variant key spec
 			$this->variantKeySpec = $this->cache->load($this->key.'variant_key_spec');
-			// Try getting the pre-filtered & merged config for this variant
-			if (!($this->yamlConfig = $this->cache->load($this->key.'yaml_config_'.$this->variantKey()))) {
-				// Otherwise, if we do have the yaml config fragments (and we should since we have a variant key spec)
-				// work out the config for this variant
-				if ($this->yamlConfigFragments = $this->cache->load($this->key.'yaml_config_fragments')) {
-					$this->buildYamlConfigVariant();
-				}
-			}
 		}
 
-		// If we don't have a config yet, we need to do a full regen to get it
-		if (!$this->yamlConfig) {
+		// If we don't have a variantKeySpec (because we're forcing regen, or it just wasn't in the cache), generate it
+		if (!$this->variantKeySpec) {
 			$this->regenerate($includeTests);
-			$this->buildYamlConfigVariant();
 		}
+
+		// At this point $this->variantKeySpec will always contain something valid, so we can build the variant
+		$this->buildYamlConfigVariant();
+	}
+
+	/**
+	 * Register a callback to be called whenever the calculated merged config changes
+	 *
+	 * In some situations the merged config can change - for instance, code in _config.php can cause which Only
+	 * and Except fragments match. Registering a callback with this function allows code to be called when
+	 * this happens.
+	 *
+	 * @param callback $callback
+	 */
+	public function registerChangeCallback($callback) {
+		$this->configChangeCallbacks[] = $callback;
 	}
 
 	/**
@@ -103,6 +131,22 @@ class SS_ConfigManifest {
 		foreach ($this->phpConfigSources as $config) {
 			require_once $config;
 		}
+
+		if ($this->variantKey() != $this->yamlConfigVariantKey) $this->buildYamlConfigVariant();
+	}
+
+	/**
+	 * Gets the (merged) config value for the given class and config property name
+	 *
+	 * @param string $class - The class to get the config property value for
+	 * @param string $name - The config property to get the value for
+	 * @param any $default - What to return if no value was contained in any YAML file for the passed $class and $name
+	 * @return any - The merged set of all values contained in all the YAML configuration files for the passed
+	 * $class and $name, or $default if there are none
+	 */
+	public function get($class, $name, $default=null) {
+		if (isset($this->yamlConfig[$class][$name])) return $this->yamlConfig[$class][$name];
+		else return $default;
 	}
 
 	/**
@@ -493,9 +537,32 @@ class SS_ConfigManifest {
 	/**
 	 * Calculates which yaml config fragments are applicable in this variant, and merge those all together into
 	 * the $this->yamlConfig propperty
+	 *
+	 * Checks cache and takes care of loading yamlConfigFragments if they aren't already present, but expects
+	 * $variantKeySpec to already be set
 	 */
 	public function buildYamlConfigVariant($cache = true) {
+		// Only try loading from cache if we don't have the fragments already loaded, as there's no way to know if a
+		// given variant is stale compared to the complete set of fragments
+		if (!$this->yamlConfigFragments) {
+			// First try and just load the exact variant
+			if ($this->yamlConfig = $this->cache->load($this->key.'yaml_config_'.$this->variantKey())) {
+				$this->yamlConfigVariantKey = $this->variantKey();
+				return;
+			}
+			// Otherwise try and load the fragments so we can build the variant
+			else {
+				$this->yamlConfigFragments = $this->cache->load($this->key.'yaml_config_fragments');
+			}
+		}
+
+		// If we still don't have any fragments we have to build them
+		if (!$this->yamlConfigFragments) {
+			$this->regenerate($this->includeTests, $cache);
+		}
+
 		$this->yamlConfig = array();
+		$this->yamlConfigVariantKey = $this->variantKey();
 
 		foreach ($this->yamlConfigFragments as $i => $fragment) {
 			$matches = true;
@@ -514,6 +581,9 @@ class SS_ConfigManifest {
 		if ($cache) {
 			$this->cache->save($this->yamlConfig, $this->key.'yaml_config_'.$this->variantKey());
 		}
+
+		// Since yamlConfig has changed, call any callbacks that are interested
+		foreach ($this->configChangeCallbacks as $callback) call_user_func($callback);
 	}
 
 	/**

tests/core/manifest/ConfigManifestTest.php

@@ -8,65 +8,65 @@ class ConfigManifestTest_ConfigManifestAccess extends SS_ConfigManifest {
 
 class ConfigManifestTest extends SapphireTest {
 
-	protected function getConfigFixture() {
+	protected function getConfigFixtureValue($name) {
 		$manifest = new SS_ConfigManifest(dirname(__FILE__).'/fixtures/configmanifest', true, true);
-		return $manifest->yamlConfig;
+		return $manifest->get('ConfigManifestTest', $name);
 	}
 
 	public function testClassRules() {
-		$config = $this->getConfigFixture();
+		$config = $this->getConfigFixtureValue('Class');
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['Class']['DirectorExists'],
+			'Yes', @$config['DirectorExists'],
 			'Only rule correctly detects existing class'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['Class']['NoSuchClassExists'],
+			'No', @$config['NoSuchClassExists'],
 			'Except rule correctly detects missing class'
 		);
 	}
 
 	public function testModuleRules() {
-		$config = $this->getConfigFixture();
+		$config = $this->getConfigFixtureValue('Module');
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['Module']['MysiteExists'],
+			'Yes', @$config['MysiteExists'],
 			'Only rule correctly detects existing module'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['Module']['NoSuchModuleExists'],
+			'No', @$config['NoSuchModuleExists'],
 			'Except rule correctly detects missing module'
 		);
 	}
 
 	public function testEnvVarSetRules() {
 		$_ENV['EnvVarSet_Foo'] = 1;
-		$config = $this->getConfigFixture();
+		$config = $this->getConfigFixtureValue('EnvVarSet');
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['EnvVarSet']['FooSet'],
+			'Yes', @$config['FooSet'],
 			'Only rule correctly detects set environment variable'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['EnvVarSet']['BarSet'],
+			'No', @$config['BarSet'],
 			'Except rule correctly detects unset environment variable'
 		);
 	}
 
 	public function testConstantDefinedRules() {
 		define('ConstantDefined_Foo', 1);
-		$config = $this->getConfigFixture();
+		$config = $this->getConfigFixtureValue('ConstantDefined');
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['ConstantDefined']['FooDefined'],
+			'Yes', @$config['FooDefined'],
 			'Only rule correctly detects defined constant'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['ConstantDefined']['BarDefined'],
+			'No', @$config['BarDefined'],
 			'Except rule correctly detects undefined constant'
 		);
 	}
@@ -74,34 +74,73 @@ class ConfigManifestTest extends SapphireTest {
 	public function testEnvOrConstantMatchesValueRules() {
 		$_ENV['EnvOrConstantMatchesValue_Foo'] = 'Foo';
 		define('EnvOrConstantMatchesValue_Bar', 'Bar');
-		$config = $this->getConfigFixture();
+		$config = $this->getConfigFixtureValue('EnvOrConstantMatchesValue');
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['EnvOrConstantMatchesValue']['FooIsFoo'],
+			'Yes', @$config['FooIsFoo'],
 			'Only rule correctly detects environment variable matches specified value'
 		);
 
 		$this->assertEquals(
-			'Yes', @$config['ConfigManifestTest']['EnvOrConstantMatchesValue']['BarIsBar'],
+			'Yes', @$config['BarIsBar'],
 			'Only rule correctly detects constant matches specified value'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['EnvOrConstantMatchesValue']['FooIsQux'],
+			'No', @$config['FooIsQux'],
 			'Except rule correctly detects environment variable that doesn\'t match specified value'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['EnvOrConstantMatchesValue']['BarIsQux'],
+			'No', @$config['BarIsQux'],
 			'Except rule correctly detects environment variable that doesn\'t match specified value'
 		);
 
 		$this->assertEquals(
-			'No', @$config['ConfigManifestTest']['EnvOrConstantMatchesValue']['BazIsBaz'],
+			'No', @$config['BazIsBaz'],
 			'Except rule correctly detects undefined variable'
 		);
 	}
 
+	public function testEnvironmentRules() {
+		foreach (array('dev', 'test', 'live') as $env) {
+			Config::inst()->nest();
+
+			Config::inst()->update('Director', 'environment_type', $env);
+			$config = $this->getConfigFixtureValue('Environment');
+
+			foreach (array('dev', 'test', 'live') as $check) {
+				$this->assertEquals(
+					$env == $check ? $check : 'not'.$check, @$config[ucfirst($check).'Environment'],
+					'Only & except rules correctly detect environment'
+				);
+			}
+
+			Config::inst()->unnest();
+		}
+	}
+
+	public function testDynamicEnvironmentRules() {
+		Config::inst()->nest();
+
+		// First, make sure environment_type is live
+		Config::inst()->update('Director', 'environment_type', 'live');
+		$this->assertEquals('live', Config::inst()->get('Director', 'environment_type'));
+
+		// Then, load in a new manifest, which includes a _config.php that sets environment_type to dev
+		$manifest = new SS_ConfigManifest(dirname(__FILE__).'/fixtures/configmanifest_dynamicenv', true, true);
+		Config::inst()->pushConfigYamlManifest($manifest);
+
+		// Make sure that stuck
+		$this->assertEquals('dev', Config::inst()->get('Director', 'environment_type'));
+
+		// And that the dynamic rule was calculated correctly
+		$this->assertEquals('dev', Config::inst()->get('ConfigManifestTest', 'DynamicEnvironment'));
+
+		Config::inst()->unnest();
+	}
+
+
 	public function testRelativeOrder() {
 		$accessor = new ConfigManifestTest_ConfigManifestAccess(BASE_PATH, true, false);
 

tests/core/manifest/fixtures/configmanifest/mysite/_config/envrules.yml

@@ -0,0 +1,42 @@
+---
+Only:
+  Environment: live
+---
+ConfigManifestTest:
+  Environment:
+    LiveEnvironment: live
+---
+Only:
+  Environment: dev
+---
+ConfigManifestTest:
+  Environment:
+    DevEnvironment: dev
+---
+Only:
+  Environment: test
+---
+ConfigManifestTest:
+  Environment:
+    TestEnvironment: test
+---
+Except:
+  Environment: live
+---
+ConfigManifestTest:
+  Environment:
+    LiveEnvironment: notlive
+---
+Except:
+  Environment: dev
+---
+ConfigManifestTest:
+  Environment:
+    DevEnvironment: notdev
+---
+Except:
+  Environment: test
+---
+ConfigManifestTest:
+  Environment:
+    TestEnvironment: nottest

tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config.php

@@ -0,0 +1,4 @@
+<?php
+
+// Dynamically change environment
+Config::inst()->update('Director', 'environment_type', 'dev');

tests/core/manifest/fixtures/configmanifest_dynamicenv/mysite/_config/environment.yml

@@ -0,0 +1,18 @@
+---
+Only:
+  Environment: live
+---
+ConfigManifestTest:
+  DynamicEnvironment: live
+---
+Only:
+  Environment: dev
+---
+ConfigManifestTest:
+  DynamicEnvironment: dev
+---
+Only:
+  Environment: test
+---
+ConfigManifestTest:
+  DynamicEnvironment: test