diff options
| -rw-r--r-- | includes/bootstrap.inc | 18 | ||||
| -rw-r--r-- | includes/common.inc | 2 | ||||
| -rw-r--r-- | includes/theme.inc | 42 | ||||
| -rw-r--r-- | modules/simpletest/tests/theme.test | 51 | ||||
| -rw-r--r-- | modules/simpletest/tests/theme_test.module | 14 | ||||
| -rw-r--r-- | modules/simpletest/tests/theme_test.template_test.tpl.php | 2 |
6 files changed, 102 insertions, 27 deletions
diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 425a74acb..4ef4f2b83 100644 --- a/includes/bootstrap.inc +++ b/includes/bootstrap.inc @@ -289,12 +289,12 @@ abstract class DrupalCacheArray implements ArrayAccess { /** * A cid to pass to cache_set() and cache_get(). */ - private $cid; + protected $cid; /** * A bin to pass to cache_set() and cache_get(). */ - private $bin; + protected $bin; /** * An array of keys to add to the cache at the end of the request. @@ -393,24 +393,20 @@ abstract class DrupalCacheArray implements ArrayAccess { /** * Writes a value to the persistent cache immediately. * - * @param $cid - * The cache ID. - * @param $bin - * The cache bin. * @param $data * The data to write to the persistent cache. * @param $lock * Whether to acquire a lock before writing to cache. */ - protected function set($cid, $data, $bin, $lock = TRUE) { + protected function set($data, $lock = TRUE) { // Lock cache writes to help avoid stampedes. // To implement locking for cache misses, override __construct(). - $lock_name = $cid . ':' . $bin; + $lock_name = $this->cid . ':' . $this->bin; if (!$lock || lock_acquire($lock_name)) { - if ($cached = cache_get($cid, $bin)) { + if ($cached = cache_get($this->cid, $this->bin)) { $data = $cached->data + $data; } - cache_set($cid, $data, $bin); + cache_set($this->cid, $data, $this->bin); if ($lock) { lock_release($lock_name); } @@ -428,7 +424,7 @@ abstract class DrupalCacheArray implements ArrayAccess { } } if (!empty($data)) { - $this->set($this->cid, $data, $this->bin); + $this->set($data); } } } diff --git a/includes/common.inc b/includes/common.inc index b07bf9669..5e90f78f1 100644 --- a/includes/common.inc +++ b/includes/common.inc @@ -2370,7 +2370,7 @@ function l($text, $path, array $options = array()) { // rendering. if (variable_get('theme_link', TRUE)) { drupal_theme_initialize(); - $registry = theme_get_registry(); + $registry = theme_get_registry(FALSE); // We don't want to duplicate functionality that's in theme(), so any // hint of a module or theme doing anything at all special with the 'link' // theme hook should simply result in theme() being called. This includes diff --git a/includes/theme.inc b/includes/theme.inc index 9a921decc..da4200e56 100644 --- a/includes/theme.inc +++ b/includes/theme.inc @@ -372,23 +372,31 @@ class ThemeRegistry Extends DrupalCacheArray { $data = $cached->data; } else { - $complete_registry = theme_get_registry(); + // If there is no runtime cache stored, fetch the full theme registry, + // but then initialize each value to NULL. This allows offsetExists() + // to function correctly on non-registered theme hooks without triggering + // a call to resolveCacheMiss(). + $data = $this->initializeRegistry(); if ($this->persistable) { - // If there is no runtime cache stored, fetch the full theme registry, - // but then initialize each value to NULL. This allows - // offsetExists() to function correctly on non-registered theme hooks - // without triggering a call to resolveCacheMiss(). - $data = array_fill_keys(array_keys($complete_registry), NULL); - $this->set($this->cid, $data, $this->bin); - $this->completeRegistry = $complete_registry; - } - else { - $data = $complete_registry; + $this->set($data); } } $this->storage = $data; } + /** + * Initializes the full theme registry. + * + * @return + * An array with the keys of the full theme registry, but the values + * initialized to NULL. + */ + function initializeRegistry() { + $this->completeRegistry = theme_get_registry(); + + return array_fill_keys(array_keys($this->completeRegistry), NULL); + } + public function offsetExists($offset) { // Since the theme registry allows for theme hooks to be requested that // are not registered, just check the existence of the key in the registry. @@ -420,15 +428,19 @@ class ThemeRegistry Extends DrupalCacheArray { return $this->storage[$offset]; } - public function set($cid, $data, $bin, $lock = TRUE) { - $lock_name = $cid . ':' . $bin; + public function set($data, $lock = TRUE) { + $lock_name = $this->cid . ':' . $this->bin; if (!$lock || lock_acquire($lock_name)) { - if ($cached = cache_get($cid, $this->bin)) { + if ($cached = cache_get($this->cid, $this->bin)) { // Use array merge instead of union so that filled in values in $data // overwrite empty values in the current cache. $data = array_merge($cached->data, $data); } - cache_set($cid, $data, $bin); + else { + $registry = $this->initializeRegistry(); + $data = array_merge($registry, $data); + } + cache_set($this->cid, $data, $this->bin); if ($lock) { lock_release($lock_name); } diff --git a/modules/simpletest/tests/theme.test b/modules/simpletest/tests/theme.test index d54885099..af1141124 100644 --- a/modules/simpletest/tests/theme.test +++ b/modules/simpletest/tests/theme.test @@ -382,3 +382,54 @@ class ThemeHtmlTag extends DrupalUnitTestCase { $this->assertEqual('<title>title test</title>'."\n", theme_html_tag($tag), t('Test title tag generation.')); } } + +/** + * Tests for the ThemeRegistry class. + */ +class ThemeRegistryTestCase extends DrupalWebTestCase { + public static function getInfo() { + return array( + 'name' => 'ThemeRegistry', + 'description' => 'Tests the behavior of the ThemeRegistry class', + 'group' => 'Theme', + ); + } + function setUp() { + parent::setUp('theme_test'); + } + + /** + * Tests the behavior of the theme registry class. + */ + function testRaceCondition() { + $_SERVER['REQUEST_METHOD'] = 'GET'; + $cid = 'test_theme_registry'; + + // Directly instantiate the theme registry, this will cause a base cache + // entry to be written in __construct(). + $registry = new ThemeRegistry($cid, 'cache'); + + $this->assertTrue(cache_get($cid), 'Cache entry was created.'); + + // Trigger a cache miss for an offset. + $this->assertTrue($registry['theme_test_template_test'], 'Offset was returned correctly from the theme registry.'); + // This will cause the ThemeRegistry class to write an updated version of + // the cache entry when it is destroyed, usually at the end of the request. + // Before that happens, manually delete the cache entry we created earlier + // so that the new entry is written from scratch. + cache_clear_all($cid, 'cache'); + + // Destroy the class so that it triggers a cache write for the offset. + unset($registry); + + $this->assertTrue(cache_get($cid), 'Cache entry was created.'); + + // Create a new instance of the class. Confirm that both the offset + // requested previously, and one that has not yet been requested are both + // available. + $registry = new ThemeRegistry($cid, 'cache'); + + $this->assertTrue($registry['theme_test_template_test'], 'Offset was returned correctly from the theme registry'); + $this->assertTrue($registry['theme_test_template_test_2'], 'Offset was returned correctly from the theme registry'); + } +} diff --git a/modules/simpletest/tests/theme_test.module b/modules/simpletest/tests/theme_test.module index 9cec5381d..48e2e83c6 100644 --- a/modules/simpletest/tests/theme_test.module +++ b/modules/simpletest/tests/theme_test.module @@ -1,6 +1,20 @@ <?php /** + * Implements hook_theme(). + */ +function theme_test_theme($existing, $type, $theme, $path) { + $items['theme_test_template_test'] = array( + 'template' => 'theme_test.template_test', + ); + $items['theme_test_template_test_2'] = array( + 'template' => 'theme_test.template_test', + ); + + return $items; +} + +/** * Implements hook_system_theme_info(). */ function theme_test_system_theme_info() { diff --git a/modules/simpletest/tests/theme_test.template_test.tpl.php b/modules/simpletest/tests/theme_test.template_test.tpl.php new file mode 100644 index 000000000..cde8faadd --- /dev/null +++ b/modules/simpletest/tests/theme_test.template_test.tpl.php @@ -0,0 +1,2 @@ +<!-- Output for Theme API test --> +Fail: Template not overridden. |