BUGFIX #4255 sharvey: Fix application of extra db fields by DataObjectDecorators.

API CHANGE #4255 sharvey: DataObjectDecorator::extraStatics() can no longer refer to $this because it's called staticly


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

core/Object.php

@@ -388,9 +388,10 @@ abstract class Object {
 			unset(self::$classes_constructed[$subclass]);
 		}
 		
-		
 		// merge with existing static vars
 		self::add_static_var($class, 'extensions', array($extension));
+		
+		DataObjectDecorator::load_extra_statics($class, $extension);
 	}
 	
 	/**

core/model/DataObject.php

@@ -386,11 +386,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->setOwner($this);
+			DataObjectDecorator::load_extra_statics($this->class, $instance->class);
-			$instance->loadExtraStatics();
-			$instance->clearOwner();
 		}
 
 		// Set up accessors for joined items

core/model/DataObjectDecorator.php

@@ -34,65 +34,44 @@ abstract class DataObjectDecorator extends Extension {
 	private static $extra_statics_loaded = array();
 	
 	/**
-	 * Set the owner of this decorator.
+	 * Load the extra static definitions for the given extension
-	 * @param DataObject $owner
+	 * 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
 	 */
-	/*
+	public static function load_extra_statics($class, $extension) {
-	function setOwner($owner, $ownerBaseClass = null) {
+		if(!empty(self::$extra_statics_loaded[$class][$extension])) return;
-		if($owner && !($owner instanceof DataObject)) {
+		self::$extra_statics_loaded[$class][$extension] = true;
-			user_error(sprintf(
+		
-				"DataObjectDecorator->setOwner(): Trying to decorate an object of class '%s' with '%s', 
+		if(!method_exists($extension, 'extraStatics')) return;
-				only Dataobject subclasses are supported.",
-				get_class($owner), $this->class),
-				E_USER_ERROR
-			);
-			return false;
-		}
 		
-		parent::setOwner($owner, $ownerBaseClass);
-	}
-	*/
-	
-	/**
-	 * Load the extra database fields defined in extraStatics.
-	 */
-	function loadExtraStatics() {
-		if(!empty(self::$extra_statics_loaded[$this->ownerBaseClass][$this->class])) return;
-		self::$extra_statics_loaded[$this->ownerBaseClass][$this->class] = true;
-
 		// 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, 'extraStatics'));
-			foreach($fields as $relation => $newVal) {
+		if($statics) {
-				if(isset(self::$decoratable_statics[$relation])) {
+			foreach($statics as $name => $newVal) {
-					$origVal = Object::get_static($this->ownerBaseClass, $relation);
+				if(isset(self::$decoratable_statics[$name])) {
-
+					$origVal = self::get_static($class, $name);
+				
 					// Array to be merged 
-					if(self::$decoratable_statics[$relation]) {
+					if(self::$decoratable_statics[$name]) {
 						// Can't use add_static_var() here as it would merge the array rather than replacing
-						Object::set_static($this->ownerBaseClass, $relation, array_merge((array)$origVal, $newVal));
+						self::set_static($class, $name, array_merge((array)$origVal, $newVal));
-
+					
-					// Value to be overwritten
+						// Value to be overwritten
 					} else {
-						Object::set_static ($this->ownerBaseClass, $relation, $newVal);
+						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;
 		}
 	}
 	
-	/**
-	 * @deprecated 2.4 Use loadExtraStatics()
-	 */
-	function loadExtraDBFields() {
-		user_error('DataObjectDecorator::loadExtraDBFields() is deprecated. Please use loadExtraStatics() instead.', E_USER_NOTICE);
-		return $this->loadExtraStatics();
-	}
-
 	/**
 	 * Edit the given query object to support queries for this extension
 	 *
@@ -155,16 +134,9 @@ abstract class DataObjectDecorator extends Extension {
 	 *               the values are additional fields/relations to be defined.
 	 */
 	function extraStatics() {
-		return $this->extraDBFields();
-	}
-	
-	/**
-	 * @deprecated 2.4 Use extraStatics()
-	 */
-	function extraDBFields() {
 		return array();
 	}
-
+	
 	/**
 	 * This function is used to provide modifications to the form in the CMS
 	 * by the decorator. By default, no changes are made. {@link DataObject->getCMSFields()}.

core/model/Translatable.php

@@ -438,19 +438,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/model/TranslatableTest.php

@@ -33,21 +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->setOwner($newSiteTree);
-			$extInstance->loadExtraStatics();
-			$extInstance->clearOwner();
-		}
-		// @todo Hack to refresh statics on the newly decorated classes
-		$TranslatableTest_DataObject = new TranslatableTest_DataObject();
-		foreach($TranslatableTest_DataObject->getExtensionInstances() as $extInstance) {
-			$extInstance->setOwner($TranslatableTest_DataObject);
-			$extInstance->loadExtraStatics();
-			$extInstance->clearOwner();
-		}
-
 		// 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();