From 10dece653f36d65a5649014464d40df85833c49f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Thu, 10 Sep 2015 15:46:23 +1200 Subject: [PATCH] API Consolidate DataObject db methods BUG Fix namespace and getField on composite fields --- dev/CsvBulkLoader.php | 1 + docs/en/04_Changelogs/4.0.0.md | 97 ++++ docs/en/changelogs/4.0.0.md | 53 -- forms/FormScaffolder.php | 14 +- logging/HTTPOutputHandler.php | 3 +- model/DataObject.php | 522 +++++++++--------- model/DataQuery.php | 12 +- model/Versioned.php | 4 +- model/connect/DBSchemaManager.php | 9 +- model/fieldtypes/CompositeDBField.php | 9 + model/fieldtypes/PrimaryKey.php | 29 +- security/Security.php | 2 +- tests/model/CompositeDBFieldTest.php | 13 +- .../model/DataObjectSchemaGenerationTest.php | 8 +- tests/model/DataObjectTest.php | 89 +-- tests/security/SecurityTest.php | 2 +- 16 files changed, 475 insertions(+), 392 deletions(-) create mode 100644 docs/en/04_Changelogs/4.0.0.md delete mode 100644 docs/en/changelogs/4.0.0.md diff --git a/dev/CsvBulkLoader.php b/dev/CsvBulkLoader.php index 5ec535608..dcbdd3b81 100644 --- a/dev/CsvBulkLoader.php +++ b/dev/CsvBulkLoader.php @@ -60,6 +60,7 @@ class CsvBulkLoader extends BulkLoader { * @return null|BulkLoader_Result */ protected function processAll($filepath, $preview = false) { + $filepath = Director::getAbsFile($filepath); $files = $this->splitFile($filepath); $result = null; diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md new file mode 100644 index 000000000..781be09e3 --- /dev/null +++ b/docs/en/04_Changelogs/4.0.0.md @@ -0,0 +1,97 @@ +# 4.0.0 (unreleased) + +## Overview + +### Framework + + * Deprecate `SQLQuery` in favour `SQLSelect` + * `DataList::filter` by null now internally generates "IS NULL" or "IS NOT NULL" conditions appropriately on queries + * `DataObject::database_fields` now returns all fields on that table. + * `DataObject::db` now returns composite fields. + * `DataObject::ClassName` field has been refactored into a `DBClassName` type field. + +## Upgrading + +### Upgrading code that uses composite db fields. + +`CompositeDBField` is now an abstract class, not an interface. In many cases, custom code that handled +saving of content into composite fields can be removed, as it is now handled by the base class. + +The below describes the minimum amount of effort required to implement a composite DB field. + + :::php + 'Varchar(200)', + 'Suburb' => 'Varchar(100)', + 'City' => 'Varchar(100)', + 'Country' => 'Varchar(100)' + ); + + public function scaffoldFormField($title = null) { + new AddressFormField($this->getName(), $title); + } + } + + +### Upgrading code that references `DataObject::database_fields` or `DataObject::db` + +These methods have been updated to include base fields (such as ID, ClassName, Created, and LastEdited), as +well as composite DB fields. + +`DataObject::database_fields` does not have a second parameter anymore, and can be called directly on an object +or class. E.g. `Member::database_fields()` + +If user code requires the list of fields excluding base fields, then use custom_database_fields instead, or +make sure to call `unset($fields['ID']);` if this field should be excluded. + +`DataObject:db()` will return all logical fields, including foreign key ids and composite DB Fields, alongside +any child fields of these composites. This method can now take a second parameter $includesTable, which +when set to true (with a field name as the first parameter), will also include the table prefix in +`Table.ClassName(args)` format. + + +### Update code that uses SQLQuery + +SQLQuery is still implemented, but now extends the new SQLSelect class and has some methods +deprecated. Previously this class was used for both selecting and deleting, but these +have been superceded by the specialised SQLSelect and SQLDelete classes. + +Take care for any code or functions which expect an object of type `SQLQuery`, as +these references should be replaced with `SQLSelect`. Legacy code which generates +`SQLQuery` can still communicate with new code that expects `SQLSelect` as it is a +subclass of `SQLSelect`, but the inverse is not true. + +### Update implementations of augmentSQL + +Since this method now takes a `SQLSelect` as a first parameter, existing code referencing the deprecated `SQLQuery` +type will raise a PHP error. + +E.g. + +Before: + + :::php + function augmentSQL(SQLQuery &$query, DataQuery &$dataQuery = null) { + $locale = Translatable::get_current_locale(); + if(!preg_match('/("|\'|`)Locale("|\'|`)/', implode(' ', $query->getWhere()))) { + $qry = sprintf('"Locale" = \'%s\'', Convert::raw2sql($locale)); + $query->addWhere($qry); + } + } + +After: + + :::php + function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null) { + $locale = Translatable::get_current_locale(); + if(!preg_match('/("|\'|`)Locale("|\'|`)/', implode(' ', $query->getWhereParameterised($parameters)))) { + $query->addWhere(array( + '"Locale"' => $locale + )); + } + } + + diff --git a/docs/en/changelogs/4.0.0.md b/docs/en/changelogs/4.0.0.md deleted file mode 100644 index b5d7a2702..000000000 --- a/docs/en/changelogs/4.0.0.md +++ /dev/null @@ -1,53 +0,0 @@ -# 4.0.0 (unreleased) - -## Overview - -### Framework - - * Deprecate `SQLQuery` in favour `SQLSelect` - * `DataList::filter` by null now internally generates "IS NULL" or "IS NOT NULL" conditions appropriately on queries - -## Upgrading - -### Update code that uses SQLQuery - -SQLQuery is still implemented, but now extends the new SQLSelect class and has some methods -deprecated. Previously this class was used for both selecting and deleting, but these -have been superceded by the specialised SQLSelect and SQLDelete classes. - -Take care for any code or functions which expect an object of type `SQLQuery`, as -these references should be replaced with `SQLSelect`. Legacy code which generates -`SQLQuery` can still communicate with new code that expects `SQLSelect` as it is a -subclass of `SQLSelect`, but the inverse is not true. - -### Update implementations of augmentSQL - -Since this method now takes a `SQLSelect` as a first parameter, existing code referencing the deprecated `SQLQuery` -type will raise a PHP error. - -E.g. - -Before: - - :::php - function augmentSQL(SQLQuery &$query, DataQuery &$dataQuery = null) { - $locale = Translatable::get_current_locale(); - if(!preg_match('/("|\'|`)Locale("|\'|`)/', implode(' ', $query->getWhere()))) { - $qry = sprintf('"Locale" = \'%s\'', Convert::raw2sql($locale)); - $query->addWhere($qry); - } - } - -After: - - :::php - function augmentSQL(SQLSelect $query, DataQuery $dataQuery = null) { - $locale = Translatable::get_current_locale(); - if(!preg_match('/("|\'|`)Locale("|\'|`)/', implode(' ', $query->getWhereParameterised($parameters)))) { - $query->addWhere(array( - '"Locale"' => $locale - )); - } - } - - diff --git a/forms/FormScaffolder.php b/forms/FormScaffolder.php index 1fa632315..078c73bb1 100644 --- a/forms/FormScaffolder.php +++ b/forms/FormScaffolder.php @@ -73,8 +73,9 @@ class FormScaffolder extends Object { $mainTab->setTitle(_t('SiteTree.TABMAIN', "Main")); } - // add database fields - foreach($this->obj->db() as $fieldName => $fieldType) { + // Add logical fields directly specified in db config + foreach($this->obj->config()->db as $fieldName => $fieldType) { + // Skip restricted fields if($this->restrictFields && !in_array($fieldName, $this->restrictFields)) continue; // @todo Pass localized title @@ -82,7 +83,14 @@ class FormScaffolder extends Object { $fieldClass = $this->fieldClasses[$fieldName]; $fieldObject = new $fieldClass($fieldName); } else { - $fieldObject = $this->obj->dbObject($fieldName)->scaffoldFormField(null, $this->getParamsArray()); + $fieldObject = $this + ->obj + ->dbObject($fieldName) + ->scaffoldFormField(null, $this->getParamsArray()); + } + // Allow fields to opt-out of scaffolding + if(!$fieldObject) { + continue; } $fieldObject->setTitle($this->obj->fieldLabel($fieldName)); if($this->tabbed) { diff --git a/logging/HTTPOutputHandler.php b/logging/HTTPOutputHandler.php index 6427ba451..87ccd0e2b 100644 --- a/logging/HTTPOutputHandler.php +++ b/logging/HTTPOutputHandler.php @@ -2,7 +2,6 @@ namespace SilverStripe\Framework\Logging; -use Monolog\Logger; use Monolog\Handler\AbstractProcessingHandler; /** @@ -53,7 +52,7 @@ class HTTPOutputHandler extends AbstractProcessingHandler if(\Controller::has_curr()) { $response = \Controller::curr()->getResponse(); } else { - $response = new SS_HTTPResponse(); + $response = new \SS_HTTPResponse(); } // If headers have been sent then these won't be used, and may throw errors that we wont' want to see. diff --git a/model/DataObject.php b/model/DataObject.php index b52b4f2fa..58c7a6c1b 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -175,16 +175,19 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Static caches used by relevant functions. */ - public static $cache_has_own_table = array(); - protected static $_cache_db = array(); + protected static $_cache_has_own_table = array(); protected static $_cache_get_one; protected static $_cache_get_class_ancestry; protected static $_cache_composite_fields = array(); - protected static $_cache_is_composite_field = array(); - protected static $_cache_custom_database_fields = array(); + protected static $_cache_database_fields = array(); protected static $_cache_field_labels = array(); - // base fields which are not defined in static $db + /** + * Base fields which are not defined in static $db + * + * @config + * @var array + */ private static $fixed_fields = array( 'ID' => 'PrimaryKey', 'ClassName' => 'DBClassName', @@ -234,29 +237,94 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * for any classes if a new class manifest is generated. */ public static function clear_classname_spec_cache() { + Deprecation::notice('4.0', 'Call DBClassName::clear_classname_cache() instead'); DBClassName::clear_classname_cache(); } /** - * Return the complete map of fields on this object, including "Created", "LastEdited" and "ClassName". - * See {@link custom_database_fields()} for a getter that excludes these "base fields". + * Return the complete map of fields to specification on this object, including fixed_fields. + * "ID" will be included on every table. * - * @param string $class - * @param boolean $queryDB Determine if the DB may be queried for additional information - * @return array + * Composite DB field specifications are returned by reference if necessary, but not in the return + * array. + * + * Can be called directly on an object. E.g. Member::database_fields() + * + * @param string $class Class name to query from + * @return array Map of fieldname to specification, similiar to {@link DataObject::$db}. */ - public static function database_fields($class, $queryDB = true) { - if(get_parent_class($class) == 'DataObject') { - // Merge fixed with ClassName spec and custom db fields - $fixed = self::$fixed_fields; - unset($fixed['ID']); - return array_merge( - $fixed, - self::custom_database_fields($class) - ); + public static function database_fields($class = null) { + if(empty($class)) { + $class = get_called_class(); } - return self::custom_database_fields($class); + // Refresh cache + self::cache_database_fields($class); + + // Return cached values + return self::$_cache_database_fields[$class]; + } + + /** + * Cache all database and composite fields for the given class. + * Will do nothing if already cached + * + * @param string $class Class name to cache + */ + protected static function cache_database_fields($class) { + // Skip if already cached + if( isset(self::$_cache_database_fields[$class]) + && isset(self::$_cache_composite_fields[$class]) + ) { + return; + } + + $compositeFields = array(); + $dbFields = array(); + + // Ensure fixed fields appear at the start + $fixedFields = self::config()->fixed_fields; + if(get_parent_class($class) === 'DataObject') { + // Merge fixed with ClassName spec and custom db fields + $dbFields = $fixedFields; + } else { + $dbFields['ID'] = $fixedFields['ID']; + } + + // Check each DB value as either a field or composite field + $db = Config::inst()->get($class, 'db', Config::UNINHERITED) ?: array(); + foreach($db as $fieldName => $fieldSpec) { + $fieldClass = strtok($fieldSpec, '('); + if(is_subclass_of($fieldClass, 'CompositeDBField')) { + $compositeFields[$fieldName] = $fieldSpec; + } else { + $dbFields[$fieldName] = $fieldSpec; + } + } + + // Add in all has_ones + $hasOne = Config::inst()->get($class, 'has_one', Config::UNINHERITED) ?: array(); + foreach($hasOne as $fieldName => $hasOneClass) { + if($hasOneClass === 'DataObject') { + $compositeFields[$fieldName] = 'PolymorphicForeignKey'; + } else { + $dbFields["{$fieldName}ID"] = 'ForeignKey'; + } + } + + // Merge composite fields into DB + foreach($compositeFields as $fieldName => $fieldSpec) { + $fieldObj = Object::create_from_string($fieldSpec, $fieldName); + $fieldObj->setTable($class); + $nestedFields = $fieldObj->compositeDatabaseFields(); + foreach($nestedFields as $nestedName => $nestedSpec) { + $dbFields["{$fieldName}{$nestedName}"] = $nestedSpec; + } + } + + // Return cached results + self::$_cache_database_fields[$class] = $dbFields; + self::$_cache_composite_fields[$class] = $compositeFields; } /** @@ -268,45 +336,24 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Does not include "base fields" like "ID", "ClassName", "Created", "LastEdited", * see {@link database_fields()}. * + * Can be called directly on an object. E.g. Member::custom_database_fields() + * * @uses CompositeDBField->compositeDatabaseFields() * - * @param string $class + * @param string $class Class name to query from * @return array Map of fieldname to specification, similiar to {@link DataObject::$db}. */ - public static function custom_database_fields($class) { - if(isset(self::$_cache_custom_database_fields[$class])) { - return self::$_cache_custom_database_fields[$class]; + public static function custom_database_fields($class = null) { + if(empty($class)) { + $class = get_called_class(); } + + // Get all fields + $fields = self::database_fields($class); - $fields = Config::inst()->get($class, 'db', Config::UNINHERITED); - - foreach(self::composite_fields($class, false) as $fieldName => $fieldClass) { - // Remove the original fieldname, it's not an actual database column - unset($fields[$fieldName]); - - // Add all composite columns, including polymorphic relationships - $fieldObj = Object::create_from_string($fieldClass, $fieldName); - $fieldObj->setTable($class); - $compositeFields = $fieldObj->compositeDatabaseFields(); - foreach($compositeFields as $compositeName => $spec) { - $fields["{$fieldName}{$compositeName}"] = $spec; - } - } - - // Add has_one relationships - $hasOne = Config::inst()->get($class, 'has_one', Config::UNINHERITED); - if($hasOne) foreach($hasOne as $field => $hasOneClass) { - // exclude polymorphic relationships - if($hasOneClass !== 'DataObject') { - $fields[$field . 'ID'] = 'ForeignKey'; - } - } - - $output = (array) $fields; - - self::$_cache_custom_database_fields[$class] = $output; - - return $output; + // Remove fixed fields. This assumes that NO fixed_fields are composite + $fields = array_diff_key($fields, self::config()->fixed_fields); + return $fields; } /** @@ -316,83 +363,49 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @param string $class Class to check * @param string $name Field to check * @param boolean $aggregated True if parent classes should be checked, or false to limit to this class - * @return string Class name of composite field if it exists + * @return string|false Class spec name of composite field if it exists, or false if not */ public static function is_composite_field($class, $name, $aggregated = true) { - $key = $class . '_' . $name . '_' . (string)$aggregated; - - if(!isset(DataObject::$_cache_is_composite_field[$key])) { - $isComposite = null; - - if(!isset(DataObject::$_cache_composite_fields[$class])) { - self::cache_composite_fields($class); - } - - if(isset(DataObject::$_cache_composite_fields[$class][$name])) { - $isComposite = DataObject::$_cache_composite_fields[$class][$name]; - } elseif($aggregated && $class != 'DataObject' && ($parentClass=get_parent_class($class)) != 'DataObject') { - $isComposite = self::is_composite_field($parentClass, $name); - } - - DataObject::$_cache_is_composite_field[$key] = ($isComposite) ? $isComposite : false; - } - - return DataObject::$_cache_is_composite_field[$key] ?: null; + $fields = self::composite_fields($class, $aggregated); + return isset($fields[$name]) ? $fields[$name] : false; } /** * Returns a list of all the composite if the given db field on the class is a composite field. * Will check all applicable ancestor classes and aggregate results. * + * Can be called directly on an object. E.g. Member::composite_fields(), or Member::composite_fields(null, true) + * to aggregate. + * * Includes composite has_one (Polymorphic) fields * * @param string $class Name of class to check * @param bool $aggregated Include fields in entire hierarchy, rather than just on this table - * @return array + * @return array List of composite fields and their class spec */ - public static function composite_fields($class, $aggregated = true) { - if(!isset(DataObject::$_cache_composite_fields[$class])) self::cache_composite_fields($class); - - $compositeFields = DataObject::$_cache_composite_fields[$class]; - - if($aggregated && $class != 'DataObject' && ($parentClass=get_parent_class($class)) != 'DataObject') { - $compositeFields = array_merge($compositeFields, - self::composite_fields($parentClass)); + public static function composite_fields($class = null, $aggregated = true) { + // Check $class + if(empty($class)) { + $class = get_called_class(); + } + if($class === 'DataObject') { + return array(); } - return $compositeFields; - } + // Refresh cache + self::cache_database_fields($class); - /** - * Internal cacher for the composite field information - */ - private static function cache_composite_fields($class) { - $compositeFields = array(); - - // Check db - $fields = Config::inst()->get($class, 'db', Config::UNINHERITED); - if($fields) foreach($fields as $fieldName => $fieldClass) { - if(!is_string($fieldClass)) continue; - - // Strip off any parameters - $bPos = strpos($fieldClass, '('); - if($bPos !== FALSE) $fieldClass = substr($fieldClass, 0, $bPos); - - // Test to see if it extends CompositeDBField - if(is_subclass_of($fieldClass, 'CompositeDBField')) { - $compositeFields[$fieldName] = $fieldClass; - } + // Get fields for this class + $compositeFields = self::$_cache_composite_fields[$class]; + if(!$aggregated) { + return $compositeFields; } - // check has_one PolymorphicForeignKey - $hasOne = Config::inst()->get($class, 'has_one', Config::UNINHERITED); - if($hasOne) foreach($hasOne as $fieldName => $hasOneClass) { - if($hasOneClass === 'DataObject') { - $compositeFields[$fieldName] = 'PolymorphicForeignKey'; - } - } - - DataObject::$_cache_composite_fields[$class] = $compositeFields; + // Recursively merge + return array_merge( + $compositeFields, + self::composite_fields(get_parent_class($class)) + ); } /** @@ -691,25 +704,26 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Returns TRUE if all values (other than "ID") are * considered empty (by weak boolean comparison). - * Only checks for fields listed in {@link custom_database_fields()} - * - * @todo Use DBField->hasValue() * * @return boolean */ - public function isEmpty(){ - $isEmpty = true; - $customFields = self::custom_database_fields(get_class($this)); - if($map = $this->toMap()){ - foreach($map as $k=>$v){ - // only look at custom fields - if(!array_key_exists($k, $customFields)) continue; + public function isEmpty() { + $fixed = $this->config()->fixed_fields; + foreach($this->toMap() as $field => $value){ + // only look at custom fields + if(isset($fixed[$field])) { + continue; + } - $dbObj = ($v instanceof DBField) ? $v : $this->dbObject($k); - $isEmpty = ($isEmpty && !$dbObj->exists()); + $dbObject = $this->dbObject($field); + if(!$dbObject) { + continue; + } + if($dbObject->exists()) { + return false; } } - return $isEmpty; + return true; } /** @@ -945,15 +959,29 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } // makes sure we don't merge data like ID or ClassName - $leftData = $leftObj->inheritedDatabaseFields(); - $rightData = $rightObj->inheritedDatabaseFields(); + $leftData = $leftObj->db(); + $rightData = $rightObj->db(); + + foreach($rightData as $key=>$rightSpec) { + // Don't merge ID + if($key === 'ID') { + continue; + } + + // Only merge relations if allowed + if($rightSpec === 'ForeignKey' && !$includeRelations) { + continue; + } - foreach($rightData as $key=>$rightVal) { // don't merge conflicting values if priority is 'left' - if($priority == 'left' && $leftObj->{$key} !== $rightObj->{$key}) continue; + if($priority == 'left' && $leftObj->{$key} !== $rightObj->{$key}) { + continue; + } // don't overwrite existing left values with empty right values (if $overwriteWithEmpty is set) - if($priority == 'right' && !$overwriteWithEmpty && empty($rightObj->{$key})) continue; + if($priority == 'right' && !$overwriteWithEmpty && empty($rightObj->{$key})) { + continue; + } // TODO remove redundant merge of has_one fields $leftObj->{$key} = $rightObj->{$key}; @@ -983,16 +1011,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } } - - if($hasOne = $this->hasOne()) { - foreach($hasOne as $relationship => $class) { - $leftComponent = $leftObj->getComponent($relationship); - $rightComponent = $rightObj->getComponent($relationship); - if($leftComponent->exists() && $rightComponent->exists() && $priority == 'right') { - $leftObj->{$relationship . 'ID'} = $rightObj->{$relationship . 'ID'}; - } - } - } } return true; @@ -1012,7 +1030,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // $this->record might not contain the blank values so we loop on $this->inheritedDatabaseFields() as well $fieldNames = array_unique(array_merge( array_keys($this->record), - array_keys($this->inheritedDatabaseFields()))); + array_keys($this->db()) + )); foreach($fieldNames as $fieldName) { if(!isset($this->changed[$fieldName])) $this->changed[$fieldName] = self::CHANGE_STRICT; @@ -1473,13 +1492,13 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return array Class ancestry */ public function getClassAncestry() { - if(!isset(DataObject::$_cache_get_class_ancestry[$this->class])) { - DataObject::$_cache_get_class_ancestry[$this->class] = array($this->class); - while(($class=get_parent_class(DataObject::$_cache_get_class_ancestry[$this->class][0])) != "DataObject") { - array_unshift(DataObject::$_cache_get_class_ancestry[$this->class], $class); + if(!isset(self::$_cache_get_class_ancestry[$this->class])) { + self::$_cache_get_class_ancestry[$this->class] = array($this->class); + while(($class=get_parent_class(self::$_cache_get_class_ancestry[$this->class][0])) != "DataObject") { + array_unshift(self::$_cache_get_class_ancestry[$this->class], $class); } } - return DataObject::$_cache_get_class_ancestry[$this->class]; + return self::$_cache_get_class_ancestry[$this->class]; } /** @@ -1801,16 +1820,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Return data for a specific has_one component. * @param string $component - * @param string $table Out parameter of the table this has_one field belongs to * @return string|null */ - public function hasOneComponent($component, &$table = null) { + public function hasOneComponent($component) { $classes = ClassInfo::ancestry($this, true); foreach(array_reverse($classes) as $class) { $hasOnes = Config::inst()->get($class, 'has_one', Config::UNINHERITED); if(isset($hasOnes[$component])) { - $table = $class; return $hasOnes[$component]; } } @@ -1879,14 +1896,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Return all of the database fields defined in self::$db and all the parent classes. - * Doesn't include any fields specified by self::$has_one. Use $this->hasOne() to get these fields + * Return all of the database fields in this object * * @param string $fieldName Limit the output to a specific field name - * @param string $table Out parameter of the table this db field is set to - * @return array The database fields + * @param string $includeTable If returning a single column, prefix the column with the table name + * in Table.Column(spec) format + * @return array|string|null The database fields, or if searching a single field, just this one field if found + * Field will be a string in ClassName(args) format, or Table.ClassName(args) format if $includeTable is true */ - public function db($fieldName = null, &$table = null) { + public function db($fieldName = null, $includeTable = false) { $classes = ClassInfo::ancestry($this, true); // If we're looking for a specific field, we want to hit subclasses first as they may override field types @@ -1894,26 +1912,36 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $classes = array_reverse($classes); } - $items = array(); + $db = array(); foreach($classes as $class) { - if(isset(self::$_cache_db[$class])) { - $dbItems = self::$_cache_db[$class]; - } else { - $dbItems = (array) Config::inst()->get($class, 'db', Config::UNINHERITED); - self::$_cache_db[$class] = $dbItems; - } + // Merge fields with new fields and composite fields + $fields = self::database_fields($class); + $compositeFields = self::composite_fields($class, false); + $db = array_merge($db, $fields, $compositeFields); - if($fieldName) { - if(isset($dbItems[$fieldName])) { - $table = $class; - return $dbItems[$fieldName]; + // Check for search field + if($fieldName && isset($db[$fieldName])) { + // Return found field + if(!$includeTable) { + return $db[$fieldName]; } - } else { - $items = isset($items) ? array_merge((array) $items, $dbItems) : $dbItems; + + // Set table for the given field + if(in_array($fieldName, $this->config()->fixed_fields)) { + $table = $this->baseTable(); + } else { + $table = $class; + } + return $table . "." . $db[$fieldName]; } } - return $items; + // At end of search complete + if($fieldName) { + return null; + } else { + return $db; + } } /** @@ -2255,6 +2283,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $field = $this->relObject($fieldName)->scaffoldSearchField(); } + // Allow fields to opt out of search + if(!$field) { + continue; + } + if (strstr($fieldName, '.')) { $field->setName(str_replace('.', '__', $fieldName)); } @@ -2396,10 +2429,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // In case of complex fields, return the DBField object if(self::is_composite_field($this->class, $field)) { - $helper = $this->db($field); - $obj = Object::create_from_string($helper, $field); - $obj->setValue(null, $this, false); - $this->record[$field] = $obj; + $this->record[$field] = $this->dbObject($field); } return isset($this->record[$field]) ? $this->record[$field] : null; @@ -2445,7 +2475,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Add SQL for fields, both simple & multi-value // TODO: This is copy & pasted from buildSQL(), it could be moved into a method - $databaseFields = self::database_fields($tableClass, false); + $databaseFields = self::database_fields($tableClass); if($databaseFields) foreach($databaseFields as $k => $v) { if(!isset($this->record[$k]) || $this->record[$k] === null) { $columns[] = $k; @@ -2518,12 +2548,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if(is_array($databaseFieldsOnly)) { $fields = array_intersect_key((array)$this->changed, array_flip($databaseFieldsOnly)); } elseif($databaseFieldsOnly) { - $databaseFields = $this->inheritedDatabaseFields(); - $databaseFields['ID'] = true; - $databaseFields['LastEdited'] = true; - $databaseFields['Created'] = true; - $databaseFields['ClassName'] = true; - $fields = array_intersect_key((array)$this->changed, $databaseFields); + $fields = array_intersect_key((array)$this->changed, $this->db()); } else { $fields = $this->changed; } @@ -2652,7 +2677,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity public function castingHelper($field) { // Allows db to act as implicit casting override - if($fieldSpec = $this->dbHelper($field)) { + if($fieldSpec = $this->db($field)) { return $fieldSpec; } return parent::castingHelper($field); @@ -2683,9 +2708,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return boolean */ public function hasDatabaseField($field) { - if(isset(self::$fixed_fields[$field])) return true; - - return array_key_exists($field, $this->inheritedDatabaseFields()); + return $this->db($field) + && ! self::is_composite_field(get_class($this), $field); } /** @@ -2708,10 +2732,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return string The field type of the given field */ public static function has_own_table_database_field($class, $field) { - // Since database_fields omits 'ID' - if($field == "ID") return "Int"; - - $fieldMap = self::database_fields($class, false); + $fieldMap = self::database_fields($class); // Remove string-based "constructor-arguments" from the DBField definition if(isset($fieldMap[$field])) { @@ -2732,16 +2753,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if(!is_subclass_of($dataClass,'DataObject')) return false; $dataClass = ClassInfo::class_name($dataClass); - if(!isset(DataObject::$cache_has_own_table[$dataClass])) { + if(!isset(self::$_cache_has_own_table[$dataClass])) { if(get_parent_class($dataClass) == 'DataObject') { - DataObject::$cache_has_own_table[$dataClass] = true; + self::$_cache_has_own_table[$dataClass] = true; } else { - DataObject::$cache_has_own_table[$dataClass] + self::$_cache_has_own_table[$dataClass] = Config::inst()->get($dataClass, 'db', Config::UNINHERITED) || Config::inst()->get($dataClass, 'has_one', Config::UNINHERITED); } } - return DataObject::$cache_has_own_table[$dataClass]; + return self::$_cache_has_own_table[$dataClass]; } /** @@ -2922,57 +2943,26 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return DBField The field as a DBField object */ public function dbObject($fieldName) { + $value = isset($this->record[$fieldName]) + ? $this->record[$fieldName] + : null; + // If we have a DBField object in $this->record, then return that - if(isset($this->record[$fieldName]) && is_object($this->record[$fieldName])) { - return $this->record[$fieldName]; + if(is_object($value)) { + return $value; } // Build and populate new field otherwise - $helper = $this->dbHelper($fieldName, $table); + $helper = $this->db($fieldName, true); if($helper) { - $obj = Object::create_from_string($helper, $fieldName); + list($table, $spec) = explode('.', $helper); + $obj = Object::create_from_string($spec, $fieldName); $obj->setTable($table); - $obj->setValue($this->$fieldName, $this, false); + $obj->setValue($value, $this, false); return $obj; } } - /** - * Get helper class spec for the given db field. - * - * Note that child fields of composite db fields will not be detectable via this method. - * These should be resolved indirectly by referencing 'CompositeField.Child' instead of 'CompositeFieldChild' - * in your templates - * - * @param string $fieldName - * @param string $table Out parameter of the table this has_one field belongs to - * @return DBField - */ - public function dbHelper($fieldName, &$table = null) { - // Fixed fields - if(array_key_exists($fieldName, self::$fixed_fields)) { - $table = $this->baseTable(); - return self::$fixed_fields[$fieldName]; - } - - // General casting information for items in $db - if($helper = $this->db($fieldName, $table)) { - return $helper; - } - - // Special case for has_one relationships - if(preg_match('/.+ID$/', $fieldName) && $this->hasOneComponent(substr($fieldName,0,-2), $table)) { - return 'ForeignKey'; - } - - // has_one for polymorphic relations do not end in ID - if(($type = $this->hasOneComponent($fieldName, $table)) && ($type === 'DataObject')) { - return 'PolymorphicForeignKey'; - } - - return null; - } - /** * Traverses to a DBField referenced by relationships between data objects. * @@ -3018,7 +3008,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Traverses to a field referenced by relationships between data objects, returning the value * The path to the related field is specified with dot separated syntax (eg: Parent.Child.Child.FieldName) * - * @param $fieldPath string + * @param $fieldName string * @return string | null - will return null on a missing value */ public function relField($fieldName) { @@ -3155,24 +3145,24 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $cacheKey = md5(var_export($cacheComponents, true)); // Flush destroyed items out of the cache - if($cache && isset(DataObject::$_cache_get_one[$callerClass][$cacheKey]) - && DataObject::$_cache_get_one[$callerClass][$cacheKey] instanceof DataObject - && DataObject::$_cache_get_one[$callerClass][$cacheKey]->destroyed) { + if($cache && isset(self::$_cache_get_one[$callerClass][$cacheKey]) + && self::$_cache_get_one[$callerClass][$cacheKey] instanceof DataObject + && self::$_cache_get_one[$callerClass][$cacheKey]->destroyed) { - DataObject::$_cache_get_one[$callerClass][$cacheKey] = false; + self::$_cache_get_one[$callerClass][$cacheKey] = false; } - if(!$cache || !isset(DataObject::$_cache_get_one[$callerClass][$cacheKey])) { + if(!$cache || !isset(self::$_cache_get_one[$callerClass][$cacheKey])) { $dl = DataObject::get($callerClass)->where($filter)->sort($orderby); $item = $dl->First(); if($cache) { - DataObject::$_cache_get_one[$callerClass][$cacheKey] = $item; - if(!DataObject::$_cache_get_one[$callerClass][$cacheKey]) { - DataObject::$_cache_get_one[$callerClass][$cacheKey] = false; + self::$_cache_get_one[$callerClass][$cacheKey] = $item; + if(!self::$_cache_get_one[$callerClass][$cacheKey]) { + self::$_cache_get_one[$callerClass][$cacheKey] = false; } } } - return $cache ? DataObject::$_cache_get_one[$callerClass][$cacheKey] : $item; + return $cache ? self::$_cache_get_one[$callerClass][$cacheKey] : $item; } /** @@ -3185,13 +3175,13 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function flushCache($persistent = true) { if($this->class == 'DataObject') { - DataObject::$_cache_get_one = array(); + self::$_cache_get_one = array(); return $this; } $classes = ClassInfo::ancestry($this->class); foreach($classes as $class) { - if(isset(DataObject::$_cache_get_one[$class])) unset(DataObject::$_cache_get_one[$class]); + if(isset(self::$_cache_get_one[$class])) unset(self::$_cache_get_one[$class]); } $this->extend('flushCache'); @@ -3204,27 +3194,25 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Flush the get_one global cache and destroy associated objects. */ public static function flush_and_destroy_cache() { - if(DataObject::$_cache_get_one) foreach(DataObject::$_cache_get_one as $class => $items) { + if(self::$_cache_get_one) foreach(self::$_cache_get_one as $class => $items) { if(is_array($items)) foreach($items as $item) { if($item) $item->destroy(); } } - DataObject::$_cache_get_one = array(); + self::$_cache_get_one = array(); } /** * Reset all global caches associated with DataObject. */ public static function reset() { - self::clear_classname_spec_cache(); - DataObject::$cache_has_own_table = array(); - DataObject::$_cache_db = array(); - DataObject::$_cache_get_one = array(); - DataObject::$_cache_composite_fields = array(); - DataObject::$_cache_is_composite_field = array(); - DataObject::$_cache_custom_database_fields = array(); - DataObject::$_cache_get_class_ancestry = array(); - DataObject::$_cache_field_labels = array(); + DBClassName::clear_classname_cache(); + self::$_cache_has_own_table = array(); + self::$_cache_get_one = array(); + self::$_cache_composite_fields = array(); + self::$_cache_database_fields = array(); + self::$_cache_get_class_ancestry = array(); + self::$_cache_field_labels = array(); } /** @@ -3450,25 +3438,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Returns fields bu traversing the class heirachy in a bottom-up direction. - * - * Needed to avoid getCMSFields being empty when customDatabaseFields overlooks - * the inheritance chain of the $db array, where a child data object has no $db array, - * but still needs to know the properties of its parent. This should be merged into databaseFields or - * customDatabaseFields. - * - * @todo review whether this is still needed after recent API changes + * @deprecated since version 4.0 */ public function inheritedDatabaseFields() { - $fields = array(); - $currentObj = $this->class; - - while($currentObj != 'DataObject') { - $fields = array_merge($fields, self::custom_database_fields($currentObj)); - $currentObj = get_parent_class($currentObj); - } - - return (array) $fields; + Deprecation::notice('4.0', 'Use db() instead'); + return $this->db(); } /** @@ -3740,10 +3714,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @var array */ private static $casting = array( - "ID" => "PrimaryKey", - "ClassName" => "DBClassName", - "LastEdited" => "SS_Datetime", - "Created" => "SS_Datetime", "Title" => 'Text', ); diff --git a/model/DataQuery.php b/model/DataQuery.php index ba4e61385..9ed6c62f2 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -207,7 +207,8 @@ class DataQuery { $selectColumns = null; if ($queriedColumns) { // Restrict queried columns to that on the selected table - $tableFields = DataObject::database_fields($tableClass, false); + $tableFields = DataObject::database_fields($tableClass); + unset($tableFields['ID']); $selectColumns = array_intersect($queriedColumns, array_keys($tableFields)); } @@ -445,9 +446,10 @@ class DataQuery { */ protected function selectColumnsFromTable(SQLSelect &$query, $tableClass, $columns = null) { // Add SQL for multi-value fields - $databaseFields = DataObject::database_fields($tableClass, false); + $databaseFields = DataObject::database_fields($tableClass); $compositeFields = DataObject::composite_fields($tableClass, false); - if($databaseFields) foreach($databaseFields as $k => $v) { + unset($databaseFields['ID']); + foreach($databaseFields as $k => $v) { if((is_null($columns) || in_array($k, $columns)) && !isset($compositeFields[$k])) { // Update $collidingFields if necessary if($expressionForField = $query->expressionForField($k)) { @@ -459,7 +461,7 @@ class DataQuery { } } } - if($compositeFields) foreach($compositeFields as $k => $v) { + foreach($compositeFields as $k => $v) { if((is_null($columns) || in_array($k, $columns)) && $v) { $dbO = Object::create_from_string($v, $k); $dbO->setTable($tableClass); @@ -762,7 +764,7 @@ class DataQuery { $query->setSelect(array()); $query->selectField($fieldExpression, $field); $this->ensureSelectContainsOrderbyColumns($query, $originalSelect); - + return $query->execute()->column($field); } diff --git a/model/Versioned.php b/model/Versioned.php index 4d90a7336..e6fba50e2 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -401,7 +401,9 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { if ($suffix) $table = "{$classTable}_$suffix"; else $table = $classTable; - if($fields = DataObject::database_fields($this->owner->class)) { + $fields = DataObject::database_fields($this->owner->class); + unset($fields['ID']); + if($fields) { $options = Config::inst()->get($this->owner->class, 'create_table_options', Config::FIRST_SET); $indexes = $this->owner->databaseIndexes(); if ($suffix && ($ext = $this->owner->getExtensionInstance($allSuffixes[$suffix]))) { diff --git a/model/connect/DBSchemaManager.php b/model/connect/DBSchemaManager.php index 6b1fffd1f..f7e8ffa2d 100644 --- a/model/connect/DBSchemaManager.php +++ b/model/connect/DBSchemaManager.php @@ -328,7 +328,9 @@ abstract class DBSchemaManager { } //DB ABSTRACTION: we need to convert this to a db-specific version: - $this->requireField($table, 'ID', $this->IdColumn(false, $hasAutoIncPK)); + if(!isset($fieldSchema['ID'])) { + $this->requireField($table, 'ID', $this->IdColumn(false, $hasAutoIncPK)); + } // Create custom fields if ($fieldSchema) { @@ -347,6 +349,11 @@ abstract class DBSchemaManager { $fieldObj->arrayValue = $arrayValue; $fieldObj->setTable($table); + + if($fieldObj instanceof PrimaryKey) { + $fieldObj->setAutoIncrement($hasAutoIncPK); + } + $fieldObj->requireField(); } } diff --git a/model/fieldtypes/CompositeDBField.php b/model/fieldtypes/CompositeDBField.php index 9fb616115..f26a908d5 100644 --- a/model/fieldtypes/CompositeDBField.php +++ b/model/fieldtypes/CompositeDBField.php @@ -257,4 +257,13 @@ abstract class CompositeDBField extends DBField { return $fieldObject; } + public function castingHelper($field) { + $fields = $this->compositeDatabaseFields(); + if(isset($fields[$field])) { + return $fields[$field]; + } + + parent::castingHelper($field); + } + } diff --git a/model/fieldtypes/PrimaryKey.php b/model/fieldtypes/PrimaryKey.php index 854cefcaf..8b900dda0 100644 --- a/model/fieldtypes/PrimaryKey.php +++ b/model/fieldtypes/PrimaryKey.php @@ -15,6 +15,25 @@ class PrimaryKey extends Int { private static $default_search_filter_class = 'ExactMatchFilter'; + /** + * @var bool + */ + protected $autoIncrement = true; + + public function setAutoIncrement($autoIncrement) { + $this->autoIncrement = $autoIncrement; + return $this; + } + + public function getAutoIncrement() { + return $this->autoIncrement; + } + + public function requireField() { + $spec = DB::get_schema()->IdColumn(false, $this->getAutoIncrement()); + DB::require_field($this->getTable(), $this->getName(), $spec); + } + /** * @param string $name * @param DataOject $object The object that this is primary key for (should have a relation with $name) @@ -25,11 +44,11 @@ class PrimaryKey extends Int { } public function scaffoldFormField($title = null, $params = null) { - $titleField = ($this->object->hasField('Title')) ? 'Title' : 'Name'; - $map = DataList::create(get_class($this->object))->map('ID', $titleField); - $field = new DropdownField($this->name, $title, $map); - $field->setEmptyString(' '); - return $field; + return null; + } + + public function scaffoldSearchField($title = null) { + parent::scaffoldFormField($title); } public function setValue($value, $record = null, $markChanged = true) { diff --git a/security/Security.php b/security/Security.php index a0e0db854..b6e35dd65 100644 --- a/security/Security.php +++ b/security/Security.php @@ -1012,7 +1012,7 @@ class Security extends Controller implements TemplateGlobalProvider { $dbFields = DB::field_list($table); if(!$dbFields) return false; - $objFields = DataObject::database_fields($table, false); + $objFields = DataObject::database_fields($table); $missingFields = array_diff_key($objFields, $dbFields); if($missingFields) return false; diff --git a/tests/model/CompositeDBFieldTest.php b/tests/model/CompositeDBFieldTest.php index db79954ad..65c03e941 100644 --- a/tests/model/CompositeDBFieldTest.php +++ b/tests/model/CompositeDBFieldTest.php @@ -21,6 +21,13 @@ class CompositeDBFieldTest extends SapphireTest { $this->assertTrue($obj->dbObject('MyMoney')->hasField('Amount')); $this->assertTrue($obj->dbObject('MyMoney')->hasField('Currency')); + // Test getField accessor + $this->assertTrue($obj->MyMoney instanceof Money); + $this->assertTrue($obj->MyMoney->hasField('Amount')); + $obj->MyMoney->Amount = 100.00; + $this->assertEquals(100.00, $obj->MyMoney->Amount); + $this->assertEquals(100.00, $obj->MyMoneyAmount); + // Not strictly correct $this->assertFalse($obj->dbObject('MyMoney')->hasField('MyMoneyAmount')); $this->assertFalse($obj->dbObject('MyMoney')->hasField('MyMoneyCurrency')); @@ -32,7 +39,7 @@ class CompositeDBFieldTest extends SapphireTest { */ public function testCompositeFieldMetaDataFunctions() { $this->assertEquals('Money', DataObject::is_composite_field('CompositeDBFieldTest_DataObject', 'MyMoney')); - $this->assertNull(DataObject::is_composite_field('CompositeDBFieldTest_DataObject', 'Title')); + $this->assertFalse(DataObject::is_composite_field('CompositeDBFieldTest_DataObject', 'Title')); $this->assertEquals( array( 'MyMoney' => 'Money', @@ -44,8 +51,8 @@ class CompositeDBFieldTest extends SapphireTest { $this->assertEquals('Money', DataObject::is_composite_field('SubclassedDBFieldObject', 'MyMoney')); $this->assertEquals('Money', DataObject::is_composite_field('SubclassedDBFieldObject', 'OtherMoney')); - $this->assertNull(DataObject::is_composite_field('SubclassedDBFieldObject', 'Title')); - $this->assertNull(DataObject::is_composite_field('SubclassedDBFieldObject', 'OtherField')); + $this->assertFalse(DataObject::is_composite_field('SubclassedDBFieldObject', 'Title')); + $this->assertFalse(DataObject::is_composite_field('SubclassedDBFieldObject', 'OtherField')); $this->assertEquals( array( 'MyMoney' => 'Money', diff --git a/tests/model/DataObjectSchemaGenerationTest.php b/tests/model/DataObjectSchemaGenerationTest.php index 519c4c273..775da0538 100644 --- a/tests/model/DataObjectSchemaGenerationTest.php +++ b/tests/model/DataObjectSchemaGenerationTest.php @@ -129,7 +129,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest { public function testClassNameSpecGeneration() { // Test with blank entries - DataObject::clear_classname_spec_cache(); + DBClassName::clear_classname_cache(); $do1 = new DataObjectSchemaGenerationTest_DO(); $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO'); $this->assertEquals("DBClassName", $fields['ClassName']); @@ -145,7 +145,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest { // Test with instance of subclass $item1 = new DataObjectSchemaGenerationTest_IndexDO(); $item1->write(); - DataObject::clear_classname_spec_cache(); + DBClassName::clear_classname_cache(); $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO'); $this->assertEquals("DBClassName", $fields['ClassName']); $this->assertEquals( @@ -160,7 +160,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest { // Test with instance of main class $item2 = new DataObjectSchemaGenerationTest_DO(); $item2->write(); - DataObject::clear_classname_spec_cache(); + DBClassName::clear_classname_cache(); $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO'); $this->assertEquals("DBClassName", $fields['ClassName']); $this->assertEquals( @@ -177,7 +177,7 @@ class DataObjectSchemaGenerationTest extends SapphireTest { $item1->write(); $item2 = new DataObjectSchemaGenerationTest_DO(); $item2->write(); - DataObject::clear_classname_spec_cache(); + DBClassName::clear_classname_cache(); $fields = DataObject::database_fields('DataObjectSchemaGenerationTest_DO'); $this->assertEquals("DBClassName", $fields['ClassName']); $this->assertEquals( diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 685a02b78..4f1c35e13 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -35,19 +35,29 @@ class DataObjectTest extends SapphireTest { // Assert fields are included $this->assertArrayHasKey('Name', $dbFields); - // Assert the base fields are excluded - $this->assertArrayNotHasKey('Created', $dbFields); - $this->assertArrayNotHasKey('LastEdited', $dbFields); - $this->assertArrayNotHasKey('ClassName', $dbFields); - $this->assertArrayNotHasKey('ID', $dbFields); + // Assert the base fields are included + $this->assertArrayHasKey('Created', $dbFields); + $this->assertArrayHasKey('LastEdited', $dbFields); + $this->assertArrayHasKey('ClassName', $dbFields); + $this->assertArrayHasKey('ID', $dbFields); // Assert that the correct field type is returned when passing a field $this->assertEquals('Varchar', $obj->db('Name')); $this->assertEquals('Text', $obj->db('Comment')); + // Test with table required + $this->assertEquals('DataObjectTest_TeamComment.Varchar', $obj->db('Name', true)); + $this->assertEquals('DataObjectTest_TeamComment.Text', $obj->db('Comment', true)); + $obj = new DataObjectTest_ExtendedTeamComment(); $dbFields = $obj->db(); + // fixed fields are still included in extended classes + $this->assertArrayHasKey('Created', $dbFields); + $this->assertArrayHasKey('LastEdited', $dbFields); + $this->assertArrayHasKey('ClassName', $dbFields); + $this->assertArrayHasKey('ID', $dbFields); + // Assert overloaded fields have correct data type $this->assertEquals('HTMLText', $obj->db('Comment')); $this->assertEquals('HTMLText', $dbFields['Comment'], @@ -55,10 +65,14 @@ class DataObjectTest extends SapphireTest { // assertEquals doesn't verify the order of array elements, so access keys manually to check order: // expected: array('Name' => 'Varchar', 'Comment' => 'HTMLText') - reset($dbFields); - $this->assertEquals('Name', key($dbFields), 'DataObject::db returns fields in correct order'); - next($dbFields); - $this->assertEquals('Comment', key($dbFields), 'DataObject::db returns fields in correct order'); + $this->assertEquals( + array( + 'Name', + 'Comment' + ), + array_slice(array_keys($dbFields), 4, 2), + 'DataObject::db returns fields in correct order' + ); } public function testConstructAcceptsValues() { @@ -807,26 +821,8 @@ class DataObjectTest extends SapphireTest { $subteamInstance = $this->objFromFixture('DataObjectTest_SubTeam', 'subteam1'); $this->assertEquals( - array_keys($teamInstance->inheritedDatabaseFields()), array( - //'ID', - //'ClassName', - //'Created', - //'LastEdited', - 'Title', - 'DatabaseField', - 'ExtendedDatabaseField', - 'CaptainID', - 'HasOneRelationshipID', - 'ExtendedHasOneRelationshipID' - ), - 'inheritedDatabaseFields() contains all fields defined on instance: base, extended and foreign keys' - ); - - $this->assertEquals( - array_keys(DataObject::database_fields('DataObjectTest_Team', false)), - array( - //'ID', + 'ID', 'ClassName', 'LastEdited', 'Created', @@ -837,34 +833,53 @@ class DataObjectTest extends SapphireTest { 'HasOneRelationshipID', 'ExtendedHasOneRelationshipID' ), - 'databaseFields() contains only fields defined on instance, including base, extended and foreign keys' + array_keys($teamInstance->db()), + 'db() contains all fields defined on instance: base, extended and foreign keys' ); $this->assertEquals( - array_keys($subteamInstance->inheritedDatabaseFields()), array( - //'ID', - //'ClassName', - //'Created', - //'LastEdited', - 'SubclassDatabaseField', - 'ParentTeamID', + 'ID', + 'ClassName', + 'LastEdited', + 'Created', + 'Title', + 'DatabaseField', + 'ExtendedDatabaseField', + 'CaptainID', + 'HasOneRelationshipID', + 'ExtendedHasOneRelationshipID' + ), + array_keys(DataObjectTest_Team::database_fields()), + 'database_fields() contains only fields defined on instance, including base, extended and foreign keys' + ); + + $this->assertEquals( + array( + 'ID', + 'ClassName', + 'LastEdited', + 'Created', 'Title', 'DatabaseField', 'ExtendedDatabaseField', 'CaptainID', 'HasOneRelationshipID', 'ExtendedHasOneRelationshipID', + 'SubclassDatabaseField', + 'ParentTeamID', ), + array_keys($subteamInstance->db()), 'inheritedDatabaseFields() on subclass contains all fields, including base, extended and foreign keys' ); $this->assertEquals( - array_keys(DataObject::database_fields('DataObjectTest_SubTeam', false)), array( + 'ID', 'SubclassDatabaseField', 'ParentTeamID', ), + array_keys(DataObject::database_fields('DataObjectTest_SubTeam')), 'databaseFields() on subclass contains only fields defined on instance' ); } diff --git a/tests/security/SecurityTest.php b/tests/security/SecurityTest.php index e824cb6f3..03eac3ed5 100644 --- a/tests/security/SecurityTest.php +++ b/tests/security/SecurityTest.php @@ -558,7 +558,7 @@ class SecurityTest extends FunctionalTest { $old = Security::$force_database_is_ready; Security::$force_database_is_ready = null; Security::$database_is_ready = false; - DataObject::clear_classname_spec_cache(); + DBClassName::clear_classname_cache(); // Assumption: The database has been built correctly by the test runner, // and has all columns present in the ORM