BUGFIX #4285: Fixed static application bug that appeared in 2.3.2

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/branches/2.3@81698 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/Object.php

@@ -41,10 +41,11 @@ abstract class Object {
 	 */
 	
 	private static
-		$statics          = array(),
+		$statics                 = array(),
-		$cached_statics   = array(),
+		$cached_statics          = array(),
-		$extra_statics    = array(),
+		$extra_statics           = array(),
-		$replaced_statics = array();
+		$replaced_statics        = array(),
+		$_cache_statics_prepared = array();
 	
 	private static
 		$classes_constructed = array(),
@@ -170,7 +171,12 @@ abstract class Object {
 	 * @return mixed
 	 */
 	public static function get_static($class, $name, $uncached = false) {
+		if(!isset(self::$_cache_statics_prepared[$class])) {
+			Object::prepare_statics($class);
+		}
+
 		if(!isset(self::$cached_statics[$class][$name]) || $uncached) {
+			//if($class == 'DataObjectDecoratorTest_MyObject') Debug::message("$class - $name");
 			$extra     = $builtIn = $break = $replacedAt = false;
 			$ancestry  = array_reverse(ClassInfo::ancestry($class));
 			
@@ -234,6 +240,10 @@ abstract class Object {
 	 * @param mixed $value
 	 */
 	public static function set_static($class, $name, $value) {
+		if(!isset(self::$_cache_statics_prepared[$class])) {
+			Object::prepare_statics($class);
+		}
+
 		self::$statics[$class][$name]        = $value;
 		self::$cached_statics[$class][$name] = true;
 	}
@@ -368,7 +378,7 @@ abstract class Object {
 	 *  as a string, e.g. "Versioned" or "Translatable('Param')"
 	 */
 	public static function add_extension($class, $extension) {
-		if(!preg_match('/([^(]*)/', $extension, $matches)) {
+		if(!preg_match('/^([^(]*)/', $extension, $matches)) {
 			return false;
 		}
 		$extensionClass = $matches[1];
@@ -388,10 +398,36 @@ abstract class Object {
 			unset(self::$classes_constructed[$subclass]);
 		}
 		
-		
 		// merge with existing static vars
 		self::add_static_var($class, 'extensions', array($extension));
+		
+		// load statics now for DataObject classes
+		if(is_subclass_of($class, 'DataObject')) {
+			DataObjectDecorator::load_extra_statics($class, $extensionClass);
+		}
 	}
+
+	/**
+	 * Prepare static variables before processing a {@link get_static} or {@link set_static}
+	 * call.
+	 */
+	private static function prepare_statics($class) {
+		// _cache_statics_prepared setting must come first to prevent infinite loops when we call
+		// get_static below
+		self::$_cache_statics_prepared[$class] = true;
+
+		// load statics now for DataObject classes
+		if(is_subclass_of($class, 'DataObject')) {
+			$extensions = Object::get_static($class, 'extensions');
+			if($extensions) foreach($extensions as $extension) {
+				if(preg_match('/^([^(]*)/', $extension, $matches)) {
+					$extensionClass = $matches[1];
+					DataObjectDecorator::load_extra_statics($class, $extensionClass);
+				}
+			}
+		}
+	}
+	
 	
 	/**
 	 * Remove an extension from a class.

core/model/DataObject.php

@@ -297,9 +297,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 
 		parent::defineMethods();
 
-		// Define the extra db fields
+		// Define the extra db fields - this is only necessary for extensions added in the
+		// class definition.  Object::add_extension() will call this at definition time for
+		// those objects, which is a better mechanism.  Perhaps extensions defined inside the
+		// class def can somehow be applied at definiton time also?
 		if($this->extension_instances) foreach($this->extension_instances as $i => $instance) {
-			$instance->loadExtraStatics();
+			DataObjectDecorator::load_extra_statics($this->class, $instance->class);
 		}
 
 		// Set up accessors for joined items

core/model/DataObjectDecorator.php

@@ -13,26 +13,27 @@ abstract class DataObjectDecorator extends Extension {
 	 * which can be decorated onto. This list is
 	 * limited for security and performance reasons.
 	 *
+	 * Keys are the static names, and the values are whether or not the value is an array that should
+	 * be merged.
+	 *
 	 * @var array
 	 */
 	protected static $decoratable_statics = array(
-		'db', 
+		'db' => true, 
-		'has_one', 
+		'has_one' => true, 
-		'indexes', 
+		'indexes' => true, 
-		'defaults', 
+		'defaults' => true, 
-		'has_many', 
+		'has_many' => true, 
-		'many_many', 
+		'many_many' => true, 
-		'belongs_many_many', 
+		'belongs_many_many' => true, 
-		'many_many_extraFields',
+		'many_many_extraFields' => true,
-		'searchable_fields',
+		'searchable_fields' => true,
+		'api_access' => false,
 	);
+
 	
 	private static $extra_statics_loaded = array();
 	
-	/**
-	 * Set the owner of this decorator.
-	 * @param DataObject $owner
-	 */
 	function setOwner(Object $owner, $ownerBaseClass = null) {
 		if(!($owner instanceof DataObject)) {
 			user_error(sprintf(
@@ -43,34 +44,55 @@ abstract class DataObjectDecorator extends Extension {
 			);
 			return false;
 		}
-		
 		parent::setOwner($owner, $ownerBaseClass);
 	}
-	
+
 	/**
-	 * Load the extra database fields defined in extraStatics.
+	 * Load the extra static definitions for the given extension
+	 * class name, called by {@link Object::add_extension()}
+	 * 
+	 * @param string $class Class name of the owner class (or owner base class)
+	 * @param string $extension Class name of the extension class
 	 */
-	function loadExtraStatics() {
+	public static function load_extra_statics($class, $extension) {
-		if(!empty(self::$extra_statics_loaded[$this->ownerBaseClass][$this->class])) return;
+		if(!empty(self::$extra_statics_loaded[$class][$extension])) return;
-		self::$extra_statics_loaded[$this->ownerBaseClass][$this->class] = true;
+		self::$extra_statics_loaded[$class][$extension] = true;
-
+		
+		// @deprecated 2.4 - use extraStatics() now, not extraDBFields()
+		if(method_exists($extension, 'extraDBFields')) {
+			$extraStaticsMethod = 'extraDBFields';
+		} else {
+			$extraStaticsMethod = 'extraStatics';
+		}
+		
 		// If the extension has been manually applied to a subclass, we should ignore that.
-		if(Object::has_extension(get_parent_class($this->owner), $this->class)) return;
+		if(Object::has_extension(get_parent_class($class), $extension)) return;
-
+		
-		if($fields = $this->extraStatics()) {
+		$statics = call_user_func(array($extension, $extraStaticsMethod));
-			foreach($fields as $relation => $fields) {
+		
-				if(in_array($relation, self::$decoratable_statics)) {
+		if($statics) {
-					// Can't use add_static_var() here as it would merge the array rather than replacing
+			foreach($statics as $name => $newVal) {
-					Object::set_static (
+				if(isset(self::$decoratable_statics[$name])) {
-						$this->ownerBaseClass,
+					$origVal = self::get_static($class, $name);
-						$relation,
+					if($class == 'VersionedTest_DataObject') {
-						array_merge((array) Object::get_static($this->ownerBaseClass, $relation), $fields)
+						Debug::message($name);
-					);
+						Debug::dump($newVal);
+					}
+				
+					// Array to be merged 
+					if(self::$decoratable_statics[$name]) {
+						// Can't use add_static_var() here as it would merge the array rather than replacing
+						self::set_static($class, $name, array_merge((array)$origVal, $newVal));
+					
+						// Value to be overwritten
+					} else {
+						Object::set_static($class, $name, $newVal);
+					}
 				}
 			}
 			
-			DataObject::$cache_has_own_table[$this->ownerBaseClass]       = null;
+			DataObject::$cache_has_own_table[$class]       = null;
-			DataObject::$cache_has_own_table_field[$this->ownerBaseClass] = null;
+			DataObject::$cache_has_own_table_field[$class] = null;
 		}
 	}
 	
@@ -149,13 +171,6 @@ abstract class DataObjectDecorator extends Extension {
 	 *               the values are additional fields/relations to be defined.
 	 */
 	function extraStatics() {
-		return $this->extraDBFields();
-	}
-	
-	/**
-	 * @deprecated 2.3 Use extraStatics()
-	 */
-	function extraDBFields() {
 		return array();
 	}
 

core/model/SiteTree.php

@@ -1935,4 +1935,5 @@ class SiteTree extends DataObject implements PermissionProvider,i18nEntityProvid
 	}
 
 }
-?>
+
+?>

core/model/Translatable.php

@@ -436,19 +436,15 @@ class Translatable extends DataObjectDecorator {
 	}
 	
 	function extraStatics() {
-		if(get_class($this->owner) == ClassInfo::baseDataClass(get_class($this->owner))) {
+		return array(
-			return array(
+			"db" => array(
-				"db" => array(
+				"Locale" => "DBLocale",
-						"Locale" => "DBLocale",
+				//"TranslationMasterID" => "Int" // optional relation to a "translation master"
-						//"TranslationMasterID" => "Int" // optional relation to a "translation master"
+			),
-				),
+			"defaults" => array(
-                "defaults" => array(
+				"Locale" => Translatable::default_locale() // as an overloaded getter as well: getLang()
-                    "Locale" => Translatable::default_locale() // as an overloaded getter as well: getLang()
+			)
-                )
+		);
-			);
-		} else {
-			return array();
-		}
 	}
 
 	/**

tests/DataObjectDecoratorTest.php

@@ -31,6 +31,13 @@ class DataObjectDecoratorTest extends SapphireTest {
 		$contact->delete();
 	}
 	
+	/**
+	 * Test that DataObject::$api_access can be set to true via a decorator
+	 */
+	function testApiAccessCanBeDecorated() {
+		$this->assertTrue(Object::get_static('DataObjectDecoratorTest_Member', 'api_access'));
+	}
+	
 	function testPermissionDecoration() {
 		// testing behaviour in isolation, too many sideeffects and other checks
 		// in SiteTree->can*() methods to test one single feature reliably with them
@@ -85,7 +92,8 @@ class DataObjectDecoratorTest_ContactRole extends DataObjectDecorator implements
 			),
 			'has_many' => array(
 				'RelatedObjects' => 'DataObjectDecoratorTest_RelatedObject'
-			)
+			),
+			'api_access' => true,
 		);
 	}
 	
@@ -167,6 +175,17 @@ class DataObjectDecoratorTest_Ext2 extends DataObjectDecorator implements TestOn
 	
 }
 
+class DataObjectDecoratorTest_Faves extends DataObjectDecorator implements TestOnly {
+	public function extraStatics() {
+		return array(
+			'many_many' => array(
+				'Faves' => 'Page'
+			)
+		);
+	}
+}
+
 DataObject::add_extension('DataObjectDecoratorTest_MyObject', 'DataObjectDecoratorTest_Ext1');
 DataObject::add_extension('DataObjectDecoratorTest_MyObject', 'DataObjectDecoratorTest_Ext2');
-?>
+DataObject::add_extension('DataObjectDecoratorTest_MyObject', 'DataObjectDecoratorTest_Faves');
+?>

tests/model/TranslatableTest.php

@@ -33,17 +33,6 @@ class TranslatableTest extends FunctionalTest {
 		global $_SINGLETONS;
 		$_SINGLETONS = array();
 
-		// @todo Hack to refresh statics on the newly decorated classes
-		$newSiteTree = new SiteTree();
-		foreach($newSiteTree->getExtensionInstances() as $extInstance) {
-			$extInstance->loadExtraStatics();
-		}
-		// @todo Hack to refresh statics on the newly decorated classes
-		$TranslatableTest_DataObject = new TranslatableTest_DataObject();
-		foreach($TranslatableTest_DataObject->getExtensionInstances() as $extInstance) {
-			$extInstance->loadExtraStatics();
-		}
-
 		// recreate database with new settings
 		$dbname = self::create_temp_db();
 		DB::set_alternative_database_name($dbname);

tests/search/TranslatableSearchFormTest.php

@@ -32,17 +32,6 @@ class TranslatableSearchFormTest extends FunctionalTest {
 		// clear singletons, they're caching old extension info which is used in DatabaseAdmin->doBuild()
 		global $_SINGLETONS;
 		$_SINGLETONS = array();
-		
-		// @todo Hack to refresh statics on the newly decorated classes
-		$newSiteTree = new SiteTree();
-		foreach($newSiteTree->getExtensionInstances() as $extInstance) {
-			$extInstance->loadExtraStatics();
-		}
-		// @todo Hack to refresh statics on the newly decorated classes
-		$TranslatableTest_DataObject = new TranslatableTest_DataObject();
-		foreach($TranslatableTest_DataObject->getExtensionInstances() as $extInstance) {
-			$extInstance->loadExtraStatics();
-		}
 
 		// recreate database with new settings
 		$dbname = self::create_temp_db();