Issue #1371484 by catch, langworthy, makara, ArtistConk: Fixed Private properties in abstract class DrupalCacheArray.

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);
     }
   }
 }

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

includes/theme.inc

@@ -372,23 +372,31 @@ class ThemeRegistry Extends DrupalCacheArray {
       $data = $cached->data;
     }
     else {
-      $complete_registry = theme_get_registry();
-      if ($this->persistable) {
       // If there is no runtime cache stored, fetch the full theme registry,
-        // but then initialize each value to NULL. This allows
+      // but then initialize each value to NULL. This allows offsetExists()
-        // offsetExists() to function correctly on non-registered theme hooks
+      // to function correctly on non-registered theme hooks without triggering
-        // without triggering a call to resolveCacheMiss().
+      // a call to resolveCacheMiss().
-        $data = array_fill_keys(array_keys($complete_registry), NULL);
+      $data = $this->initializeRegistry();
-        $this->set($this->cid, $data, $this->bin);
+      if ($this->persistable) {
-        $this->completeRegistry = $complete_registry;
+        $this->set($data);
-      }
-      else {
-        $data = $complete_registry;
       }
     }
     $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) {
+  public function set($data, $lock = TRUE) {
-    $lock_name = $cid . ':' . $bin;
+    $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);
       }

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');
+  }
+}

modules/simpletest/tests/theme_test.module

@@ -1,5 +1,19 @@
 <?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().
  */

modules/simpletest/tests/theme_test.template_test.tpl.php

@@ -0,0 +1,2 @@
+<!-- Output for Theme API test -->
+Fail: Template not overridden.