mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-06-30 00:19:25 +02:00
Merge pull request #2207 from camspiers/config-caching-fix
FIX ConfigManifest regenerating every request if variantKeySpec is an empty array()
This commit is contained in:
Hamish Friedlander
commit
ca2b81c6c2
|
|
@ -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 @@ class SS_ConfigManifest {
|
|||
$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 @@ class SS_ConfigManifest {
|
|||
}
|
||||
|
||||
// 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);
|
||||
}
|
||||
|
||||
|
|
@ -110,6 +107,18 @@ class SS_ConfigManifest {
|
|||
$this->buildYamlConfigVariant();
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
*
|
||||
|
|
|
|||
|
|
@ -8,6 +8,11 @@ class ConfigManifestTest_ConfigManifestAccess extends SS_ConfigManifest {
|
|||
|
||||
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);
|
||||
|
|
@ -20,16 +25,140 @@ class ConfigManifestTest extends SapphireTest {
|
|||
return sprintf('Reference path "%s" failed to parse correctly', $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())
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user