From cc355a4f18c222918798b4936e3bf5131a2d105a Mon Sep 17 00:00:00 2001 From: Sam Minnee Date: Mon, 4 Oct 2010 04:46:41 +0000 Subject: [PATCH] API CHANGE: Replaced eval based creation of extension and field objects with Object::create_from_string(). API CHANGE: Introduced new function Object::create_from_string() to instantiate an object from a string like 'Int(50)' (from r101093) git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@111585 467b73ca-7a2a-4603-9d3b-597d59a354a9 --- api/RSSFeed.php | 2 +- core/Object.php | 101 +++++++++++++++++++++++++++++++++++--- core/ViewableData.php | 51 +++++++------------ core/model/DataObject.php | 17 +++---- core/model/Database.php | 2 +- tests/ObjectTest.php | 16 +++++- 6 files changed, 137 insertions(+), 52 deletions(-) diff --git a/api/RSSFeed.php b/api/RSSFeed.php index 30551eba9..2b119e96a 100755 --- a/api/RSSFeed.php +++ b/api/RSSFeed.php @@ -285,7 +285,7 @@ class RSSFeed_Entry extends ViewableData { */ function rssField($fieldName, $defaultClass = 'Varchar') { if($fieldName) { - if($this->failover->castingHelperPair($fieldName)) { + if($this->failover->castingHelper($fieldName)) { $value = $this->failover->$fieldName; $obj = $this->failover->obj($fieldName); $obj->setValue($value); diff --git a/core/Object.php b/core/Object.php index de263ccb2..970bbeea3 100755 --- a/core/Object.php +++ b/core/Object.php @@ -96,6 +96,100 @@ abstract class Object { } } + private static $_cache_inst_args = array(); + + /** + * Create an object from a string representation. It treats it as a PHP constructor without the + * 'new' keyword. It also manages to construct the object without the use of eval(). + * + * Construction itself is done with Object::create(), so that Object::useCustomClass() calls + * are respected. + * + * `Object::create_from_string("Versioned('Stage','Live')")` will return the result of + * `Object::create('Versioned', 'Stage', 'Live);` + * + * It is designed for simple, clonable objects. The first time this method is called for a given + * string it is cached, and clones of that object are returned. + * + * If you pass the $firstArg argument, this will be prepended to the constructor arguments. It's + * impossible to pass null as the firstArg argument. + * + * `Object::create_from_string("Varchar(50)", "MyField")` will return the result of + * `Object::create('Vachar', 'MyField', '50');` + * + * Arguments are always strings, although this is a quirk of the current implementation rather + * than something that can be relied upon. + */ + static function create_from_string($classSpec, $firstArg = null) { + if(!isset(self::$_cache_inst_args[$classSpec.$firstArg])) { + // an $extension value can contain parameters as a string, + // e.g. "Versioned('Stage','Live')" + if(strpos($classSpec,'(') === false) { + if($firstArg === null) self::$_cache_inst_args[$classSpec.$firstArg] = Object::create($classSpec); + else self::$_cache_inst_args[$classSpec.$firstArg] = Object::create($classSpec, $firstArg); + + } else { + list($class, $args) = self::parse_class_spec($classSpec); + + if($firstArg !== null) array_unshift($args, $firstArg); + array_unshift($args, $class); + + self::$_cache_inst_args[$classSpec.$firstArg] = call_user_func_array('Object::create', $args); + } + } + + return clone self::$_cache_inst_args[$classSpec.$firstArg]; + } + + /** + * Parses a class-spec, such as "Versioned('Stage','Live')", as passed to create_from_string(). + * Returns a 2-elemnent array, with classname and arguments + */ + static function parse_class_spec($classSpec) { + $tokens = token_get_all("setOwner(null, $class); $this->extension_instances[$instance->class] = $instance; } @@ -552,7 +642,6 @@ abstract class Object { } } - /** * Attemps to locate and call a method dynamically added to a class at runtime if a default cannot be located * diff --git a/core/ViewableData.php b/core/ViewableData.php index ab78619df..5adbd6788 100755 --- a/core/ViewableData.php +++ b/core/ViewableData.php @@ -74,11 +74,7 @@ class ViewableData extends Object implements IteratorAggregate { * @return string */ public static function castingObjectCreator($fieldSchema) { - if(strpos($fieldSchema, '(') === false) { - return "return Object::create('{$fieldSchema}', \$fieldName);"; - } else { - return 'return Object::create(' . preg_replace('/^([^(]+)\(/', '\'$1\', $fieldName, ', $fieldSchema) . ';'; - } + user_error("Deprecated in a breaking way, use Object::create_from_string()", E_USER_WARNING); } /** @@ -89,21 +85,7 @@ class ViewableData extends Object implements IteratorAggregate { * @return array */ public static function castingObjectCreatorPair($fieldSchema) { - if(strpos($fieldSchema, '(') === false) { - return array ( - 'className' => $fieldSchema, - 'castingHelper' => self::castingObjectCreator($fieldSchema) - ); - } - - if(preg_match('/^([^(]+)\(/', $fieldSchema, $parts)) { - return array ( - 'className' => $parts[1], - 'castingHelper' => self::castingObjectCreator($fieldSchema) - ); - } - - throw new InvalidArgumentException("ViewableData::castingObjectCreatorPair(): bad field schema '$fieldSchema'"); + user_error("Deprecated in a breaking way, use Object::create_from_string()", E_USER_WARNING); } // FIELD GETTERS & SETTERS ----------------------------------------------------------------------------------------- @@ -251,15 +233,8 @@ class ViewableData extends Object implements IteratorAggregate { * @return array */ public function castingHelperPair($field) { - if(!isset(self::$casting_cache[$this->class])) { - if($this->failover) { - $this->failover->buildCastingCache(self::$casting_cache[$this->class]); - } - - $this->buildCastingCache(self::$casting_cache[$this->class]); - } - - if(isset(self::$casting_cache[$this->class][$field])) return self::$casting_cache[$this->class][$field]; + user_error("castingHelperPair() Deprecated, use castingHelper() instead", E_USER_NOTICE); + return $this->castingHelper($field); } /** @@ -270,7 +245,12 @@ class ViewableData extends Object implements IteratorAggregate { * @return string */ public function castingHelper($field) { - if($pair = $this->castingHelperPair($field)) return $pair['castingHelper']; + if($this instanceof DataObject && ($fieldSpec = $this->db($field))) { + return $fieldSpec; + } + + $specs = Object::combined_static(get_class($this), 'casting'); + if(isset($specs[$field])) return $specs[$field]; } /** @@ -280,7 +260,12 @@ class ViewableData extends Object implements IteratorAggregate { * @return string */ public function castingClass($field) { - if($pair = $this->castingHelperPair($field)) return $pair['className']; + $spec = $this->castingHelper($field); + if(!$spec) return null; + + $bPos = strpos($spec,'('); + if($bPos === false) return $spec; + else return substr($spec, 0, $bPos-1); } /** @@ -386,10 +371,10 @@ class ViewableData extends Object implements IteratorAggregate { if(!is_object($value) && ($this->castingClass($fieldName) || $forceReturnedObject)) { if(!$castConstructor = $this->castingHelper($fieldName)) { - $castConstructor = self::castingObjectCreator($this->stat('default_cast')); + $castConstructor = $this->stat('default_cast'); } - $valueObject = eval($castConstructor); + $valueObject = Object::create_from_string($castConstructor, $fieldName); $valueObject->setValue($value, ($this->hasMethod('getAllFields') ? $this->getAllFields() : null)); $value = $valueObject; diff --git a/core/model/DataObject.php b/core/model/DataObject.php index 11ef1b5d3..871ef21c2 100755 --- a/core/model/DataObject.php +++ b/core/model/DataObject.php @@ -2007,10 +2007,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Otherwise, we need to determine if this is a complex field if(self::is_composite_field($this->class, $field)) { - $helperPair = $this->castingHelperPair($field); - $constructor = $helperPair['castingHelper']; - $fieldName = $field; - $fieldObj = eval($constructor); + $helper = $this->castingHelper($field); + $fieldObj = Object::create_from_string($helper, $field); // write value only if either the field value exists, // or a valid record has been loaded from the database @@ -2156,7 +2154,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } $castingHelper = $this->castingHelper($fieldName); if($castingHelper) { - $fieldObj = eval($castingHelper); + $fieldObj = Object::create_from_string($castingHelper, $fieldName); $fieldObj->setValue($val); $fieldObj->saveInto($this); } else { @@ -2409,9 +2407,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return new PrimaryKey($fieldName, $this); // General casting information for items in $db or $casting - } else if($helperPair = $this->castingHelperPair($fieldName)) { - $constructor = $helperPair['castingHelper']; - $obj = eval($constructor); + } else if($helper = $this->castingHelper($fieldName)) { + $obj = Object::create_from_string($helper, $fieldName); $obj->setValue($this->$fieldName, $this->record, false); return $obj; @@ -2445,8 +2442,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $component = singleton($rel); } elseif ($rel = $component->many_many($relation)) { $component = singleton($rel[1]); - } elseif($info = $this->castingHelperPair($relation)) { - $component = singleton($info['className']); + } elseif($className = $this->castingClass($relation)) { + $component = $className; } } diff --git a/core/model/Database.php b/core/model/Database.php index 51b99889c..25850989c 100755 --- a/core/model/Database.php +++ b/core/model/Database.php @@ -313,7 +313,7 @@ abstract class SS_Database { $fieldSpec=substr($fieldSpec, 0, $pos); } - $fieldObj = eval(ViewableData::castingObjectCreator($fieldSpec)); + $fieldObj = Object::create_from_string($fieldSpec, $fieldName); $fieldObj->arrayValue=$arrayValue; $fieldObj->setTable($table); diff --git a/tests/ObjectTest.php b/tests/ObjectTest.php index b7927f586..bace066db 100755 --- a/tests/ObjectTest.php +++ b/tests/ObjectTest.php @@ -335,7 +335,21 @@ class ObjectTest extends SapphireTest { array('ExtendTest(test)', 'ExtendTest2(modified)') ); } - + + public function testParseClassSpec() { + $this->assertEquals( + array('Versioned',array('Stage', 'Live')), + Object::parse_class_spec("Versioned('Stage','Live')") + ); + $this->assertEquals( + array('Versioned',array('Stage,Live', 'Stage')), + Object::parse_class_spec("Versioned('Stage,Live','Stage')") + ); + $this->assertEquals( + array('Versioned',array('Stage\'Stage,Live\'Live', 'Live')), + Object::parse_class_spec("Versioned('Stage\'Stage,Live\'Live','Live')") + ); + } } /**#@+