Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

FIX ConfigManifest regenerating every request if variantKeySpec is an…

… empty array()
  • Loading branch information...
commit b44641336b3c27875259f2cffccf10d3cdf06bcb 1 parent e6011f3
@camspiers camspiers authored
View
21 core/manifest/ConfigManifest.php
@@ -23,7 +23,7 @@ class SS_ConfigManifest {
* the current environment.
* @var array
*/
- protected $variantKeySpec = array();
+ protected $variantKeySpec = false;
/**
* All the _config.php files. Need to be included every request & can't be cached. Not variant specific.
@@ -88,10 +88,7 @@ public function __construct($base, $includeTests = false, $forceRegen = false )
$this->includeTests = $includeTests;
// Get the Zend Cache to load/store cache into
- $this->cache = SS_Cache::factory('SS_Configuration', 'Core', array(
- 'automatic_serialization' => true,
- 'lifetime' => null
- ));
+ $this->cache = $this->getCache();
// Unless we're forcing regen, try loading from cache
if (!$forceRegen) {
@@ -102,7 +99,7 @@ public function __construct($base, $includeTests = false, $forceRegen = false )
}
// 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) {
+ if (false === $this->variantKeySpec) {
$this->regenerate($includeTests);
}
@@ -111,6 +108,18 @@ public function __construct($base, $includeTests = false, $forceRegen = false )
}
/**
+ * Provides a hook for mock unit tests despite no DI
+ * @return Zend_Cache_Frontend
+ */
+ protected function getCache()
+ {
+ return SS_Cache::factory('SS_Configuration', 'Core', array(
+ 'automatic_serialization' => true,
+ 'lifetime' => null
+ ));
+ }
+
+ /**
* 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
View
137 tests/core/manifest/ConfigManifestTest.php
@@ -8,6 +8,11 @@ public function relativeOrder($a, $b) {
class ConfigManifestTest extends SapphireTest {
+ /**
+ * This is a helper method for getting a new manifest
+ * @param $name
+ * @return any
+ */
protected function getConfigFixtureValue($name) {
$manifest = new SS_ConfigManifest(dirname(__FILE__).'/fixtures/configmanifest', true, true);
return $manifest->get('ConfigManifestTest', $name);
@@ -21,15 +26,139 @@ protected function getParsedAsMessage($path) {
}
/**
+ * A helper method to return a mock of the cache in order to test expectations and reduce dependency
+ * @return Zend_Cache_Core
+ */
+ protected function getCacheMock() {
+ return $this->getMock(
+ 'Zend_Cache_Core',
+ array('load', 'save'),
+ array(),
+ '',
+ false
+ );
+ }
+
+ /**
+ * A helper method to return a mock of the manifest in order to test expectations and reduce dependency
+ * @param $methods
+ * @return SS_ConfigManifest
+ */
+ protected function getManifestMock($methods) {
+ return $this->getMock(
+ 'SS_ConfigManifest',
+ $methods,
+ array(), // no constructor arguments
+ '', // default
+ false // don't call the constructor
+ );
+ }
+
+ /**
+ * Test the caching functionality when we are forcing regeneration
+ *
+ * 1. Test that regenerate is called in the default case and that cache->load isn't
+ * 2. Test that save is called correctly after the regeneration
+ */
+ public function testCachingForceRegeneration() {
+ // Test that regenerate is called correctly.
+ $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant'));
+
+ $manifest->expects($this->once()) // regenerate should be called once
+ ->method('regenerate')
+ ->with($this->equalTo(true)); // includeTests = true
+
+ // Set up a cache where we expect load to never be called
+ $cache = $this->getCacheMock();
+ $cache->expects($this->never())
+ ->method('load');
+
+ $manifest->expects($this->any())
+ ->method('getCache')
+ ->will($this->returnValue($cache));
+
+ $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', true, true);
+
+ // Test that save is called correctly
+ $manifest = $this->getManifestMock(array('getCache'));
+
+ $cache = $this->getCacheMock();
+ $cache->expects($this->atLeastOnce())
+ ->method('save');
+
+ $manifest->expects($this->any())
+ ->method('getCache')
+ ->will($this->returnValue($cache));
+
+ $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', true, true);
+ }
+
+ /**
+ * Test the caching functionality when we are not forcing regeneration
+ *
+ * 1. Test that load is called
+ * 2. Test the regenerate is called when the cache is unprimed
+ * 3. Test that when there is a value in the cache regenerate isn't called
+ */
+ public function testCachingNotForceRegeneration() {
+ // Test that load is called
+ $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant'));
+
+ // Load should be called twice
+ $cache = $this->getCacheMock();
+ $cache->expects($this->exactly(2))
+ ->method('load');
+
+ $manifest->expects($this->any())
+ ->method('getCache')
+ ->will($this->returnValue($cache));
+
+ $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', true, false);
+
+
+ // Now test that regenerate is called because the cache is unprimed
+ $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant'));
+
+ $cache = $this->getCacheMock();
+ $cache->expects($this->exactly(2))
+ ->method('load')
+ ->will($this->onConsecutiveCalls(false, false));
+
+ $manifest->expects($this->any())
+ ->method('getCache')
+ ->will($this->returnValue($cache));
+
+ $manifest->expects($this->once())
+ ->method('regenerate')
+ ->with($this->equalTo(false)); //includeTests = false
+
+ $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', false, false);
+
+ // Now test that when there is a value in the cache that regenerate isn't called
+ $manifest = $this->getManifestMock(array('getCache', 'regenerate', 'buildYamlConfigVariant'));
+
+ $cache = $this->getCacheMock();
+ $cache->expects($this->exactly(2))
+ ->method('load')
+ ->will($this->onConsecutiveCalls(array(), array()));
+
+ $manifest->expects($this->any())
+ ->method('getCache')
+ ->will($this->returnValue($cache));
+
+ $manifest->expects($this->never())
+ ->method('regenerate');
+
+ $manifest->__construct(dirname(__FILE__).'/fixtures/configmanifest', false, false);
+ }
+
+ /**
* This test checks the processing of before and after reference paths (module-name/filename#fragment)
* This method uses fixture/configmanifest/mysite/_config/addyamlconfigfile.yml as a fixture
*/
public function testAddYAMLConfigFileReferencePathParsing() {
// Use a mock to avoid testing unrelated functionality
- $manifest = $this->getMockBuilder('SS_ConfigManifest')
- ->disableOriginalConstructor()
- ->setMethods(array('addModule'))
- ->getMock();
+ $manifest = $this->getManifestMock(array('addModule'));
// This tests that the addModule method is called with the correct value
$manifest->expects($this->once())
Please sign in to comment.
Something went wrong with that request. Please try again.