diff --git a/admin/code/CampaignAdmin.php b/admin/code/CampaignAdmin.php index d02e511ae..d66b49159 100644 --- a/admin/code/CampaignAdmin.php +++ b/admin/code/CampaignAdmin.php @@ -271,7 +271,7 @@ JSON; * @return array */ protected function getChangeSetItemResource(ChangeSetItem $changeSetItem) { - $baseClass = ClassInfo::baseDataClass($changeSetItem->ObjectClass); + $baseClass = DataObject::getSchema()->baseDataClass($changeSetItem->ObjectClass); $baseSingleton = DataObject::singleton($baseClass); $thumbnailWidth = (int)$this->config()->thumbnail_width; $thumbnailHeight = (int)$this->config()->thumbnail_height; diff --git a/control/Director.php b/control/Director.php index a3ef4eb74..ed8a29558 100644 --- a/control/Director.php +++ b/control/Director.php @@ -268,7 +268,7 @@ class Director implements TemplateGlobalProvider { // These are needed so that calling Director::test() does not muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics. - $oldStage = Versioned::get_stage(); + $oldReadingMode = Versioned::get_reading_mode(); $getVars = array(); if (!$httpMethod) $httpMethod = ($postVars || is_array($postVars)) ? "POST" : "GET"; @@ -294,7 +294,7 @@ class Director implements TemplateGlobalProvider { // Set callback to invoke prior to return $onCleanup = function() use( $existingRequestVars, $existingGetVars, $existingPostVars, $existingSessionVars, - $existingCookies, $existingServer, $existingRequirementsBackend, $oldStage + $existingCookies, $existingServer, $existingRequirementsBackend, $oldReadingMode ) { // Restore the super globals $_REQUEST = $existingRequestVars; @@ -308,7 +308,7 @@ class Director implements TemplateGlobalProvider { // These are needed so that calling Director::test() does not muck with whoever is calling it. // Really, it's some inappropriate coupling and should be resolved by making less use of statics - Versioned::set_stage($oldStage); + Versioned::set_reading_mode($oldReadingMode); Injector::unnest(); // Restore old CookieJar, etc Config::unnest(); diff --git a/core/ClassInfo.php b/core/ClassInfo.php index 57523ba46..e04417e72 100644 --- a/core/ClassInfo.php +++ b/core/ClassInfo.php @@ -78,8 +78,9 @@ class ClassInfo { * Returns an array of the current class and all its ancestors and children * which require a DB table. * + * @todo Move this into {@see DataObjectSchema} + * * @param string|object $class - * @todo Move this into data object * @return array */ public static function dataClassesFor($class) { @@ -104,28 +105,11 @@ class ClassInfo { } /** - * Returns the root class (the first to extend from DataObject) for the - * passed class. - * - * @param string|object $class - * @return string + * @deprecated 4.0..5.0 */ public static function baseDataClass($class) { - if(is_string($class) && !class_exists($class)) return null; - - $class = self::class_name($class); - - if (!is_subclass_of($class, 'DataObject')) { - throw new InvalidArgumentException("$class is not a subclass of DataObject"); - } - - while ($next = get_parent_class($class)) { - if ($next == 'DataObject') { - return $class; - } - - $class = $next; - } + Deprecation::notice('5.0', 'Use DataObject::getSchema()->baseDataClass()'); + return DataObject::getSchema()->baseDataClass($class); } /** @@ -174,8 +158,6 @@ class ClassInfo { public static function class_name($nameOrObject) { if (is_object($nameOrObject)) { return get_class($nameOrObject); - } elseif (!self::exists($nameOrObject)) { - throw new InvalidArgumentException("Class {$nameOrObject} doesn't exist"); } $reflection = new ReflectionClass($nameOrObject); return $reflection->getName(); @@ -291,53 +273,12 @@ class ClassInfo { return strtolower(self::$method_from_cache[$lClass][$lMethod]) == $lCompclass; } - /** - * Returns the table name in the class hierarchy which contains a given - * field column for a {@link DataObject}. If the field does not exist, this - * will return null. - * - * @param string $candidateClass - * @param string $fieldName - * - * @return string + * @deprecated 4.0..5.0 */ public static function table_for_object_field($candidateClass, $fieldName) { - if(!$candidateClass - || !$fieldName - || !class_exists($candidateClass) - || !is_subclass_of($candidateClass, 'DataObject') - ) { - return null; - } - - //normalise class name - $candidateClass = self::class_name($candidateClass); - $exists = self::exists($candidateClass); - - // Short circuit for fixed fields - $fixed = DataObject::config()->fixed_fields; - if($exists && isset($fixed[$fieldName])) { - return self::baseDataClass($candidateClass); - } - - // Find regular field - while($candidateClass && $candidateClass != 'DataObject' && $exists) { - if( DataObject::has_own_table($candidateClass) - && DataObject::has_own_table_database_field($candidateClass, $fieldName) - ) { - break; - } - - $candidateClass = get_parent_class($candidateClass); - $exists = $candidateClass && self::exists($candidateClass); - } - - if(!$candidateClass || !$exists) { - return null; - } - - return $candidateClass; + Deprecation::notice('5.0', 'Use DataObject::getSchema()->tableForField()'); + return DataObject::getSchema()->tableForField($candidateClass, $fieldName); } } diff --git a/core/Convert.php b/core/Convert.php index be4dde8d8..30fefc0f2 100644 --- a/core/Convert.php +++ b/core/Convert.php @@ -176,19 +176,13 @@ class Convert { * table, or column name. Supports encoding of multi identfiers separated by * a delimiter (e.g. ".") * - * @param string|array $identifier The identifier to escape. E.g. 'SiteTree.Title' + * @param string|array $identifier The identifier to escape. E.g. 'SiteTree.Title' or list of identifiers + * to be joined via the separator. * @param string $separator The string that delimits subsequent identifiers - * @return string|array The escaped identifier. E.g. '"SiteTree"."Title"' + * @return string The escaped identifier. E.g. '"SiteTree"."Title"' */ public static function symbol2sql($identifier, $separator = '.') { - if(is_array($identifier)) { - foreach($identifier as $k => $v) { - $identifier[$k] = self::symbol2sql($v, $separator); - } - return $identifier; - } else { - return DB::get_conn()->escapeIdentifier($identifier, $separator); - } + return DB::get_conn()->escapeIdentifier($identifier, $separator); } /** diff --git a/dev/FixtureBlueprint.php b/dev/FixtureBlueprint.php index 1dae42533..28cd35794 100644 --- a/dev/FixtureBlueprint.php +++ b/dev/FixtureBlueprint.php @@ -59,13 +59,14 @@ class FixtureBlueprint { } /** - * @param String $identifier Unique identifier for this fixture type - * @param Array $data Map of property names to their values. - * @param Array $fixtures Map of fixture names to an associative array of their in-memory + * @param string $identifier Unique identifier for this fixture type + * @param array $data Map of property names to their values. + * @param array $fixtures Map of fixture names to an associative array of their in-memory * identifiers mapped to their database IDs. Used to look up * existing fixtures which might be referenced in the $data attribute * via the => notation. * @return DataObject + * @throws Exception */ public function createObject($identifier, $data = null, $fixtures = null) { // We have to disable validation while we import the fixtures, as the order in @@ -89,12 +90,13 @@ class FixtureBlueprint { // The database needs to allow inserting values into the foreign key column (ID in our case) $conn = DB::get_conn(); + $baseTable = DataObject::getSchema()->baseDataTable($class); if(method_exists($conn, 'allowPrimaryKeyEditing')) { - $conn->allowPrimaryKeyEditing(ClassInfo::baseDataClass($class), true); + $conn->allowPrimaryKeyEditing($baseTable, true); } $obj->write(false, true); if(method_exists($conn, 'allowPrimaryKeyEditing')) { - $conn->allowPrimaryKeyEditing(ClassInfo::baseDataClass($class), false); + $conn->allowPrimaryKeyEditing($baseTable, false); } } @@ -206,7 +208,8 @@ class FixtureBlueprint { } /** - * @param Array $defaults + * @param array $defaults + * @return $this */ public function setDefaults($defaults) { $this->defaults = $defaults; @@ -214,14 +217,14 @@ class FixtureBlueprint { } /** - * @return Array + * @return array */ public function getDefaults() { return $this->defaults; } /** - * @return String + * @return string */ public function getClass() { return $this->class; @@ -230,8 +233,9 @@ class FixtureBlueprint { /** * See class documentation. * - * @param String $type + * @param string $type * @param callable $callback + * @return $this */ public function addCallback($type, $callback) { if(!array_key_exists($type, $this->callbacks)) { @@ -243,12 +247,15 @@ class FixtureBlueprint { } /** - * @param String $type + * @param string $type * @param callable $callback + * @return $this */ public function removeCallback($type, $callback) { $pos = array_search($callback, $this->callbacks[$type]); - if($pos !== false) unset($this->callbacks[$type][$pos]); + if($pos !== false) { + unset($this->callbacks[$type][$pos]); + } return $this; } @@ -263,7 +270,7 @@ class FixtureBlueprint { * Parse a value from a fixture file. If it starts with => * it will get an ID from the fixture dictionary * - * @param string $fieldVal + * @param string $value * @param array $fixtures See {@link createObject()} * @param string $class If the value parsed is a class relation, this parameter * will be given the value of that class's name @@ -293,13 +300,16 @@ class FixtureBlueprint { } protected function overrideField($obj, $fieldName, $value, $fixtures = null) { - $table = ClassInfo::table_for_object_field(get_class($obj), $fieldName); + $class = get_class($obj); + $table = DataObject::getSchema()->tableForField($class, $fieldName); $value = $this->parseValue($value, $fixtures); DB::manipulate(array( $table => array( - "command" => "update", "id" => $obj->ID, - "fields" => array($fieldName => $value) + "command" => "update", + "id" => $obj->ID, + "class" => $class, + "fields" => array($fieldName => $value), ) )); $obj->$fieldName = $value; diff --git a/docs/en/02_Developer_Guides/00_Model/01_Data_Model_and_ORM.md b/docs/en/02_Developer_Guides/00_Model/01_Data_Model_and_ORM.md index 17104026c..93bd0baa5 100644 --- a/docs/en/02_Developer_Guides/00_Model/01_Data_Model_and_ORM.md +++ b/docs/en/02_Developer_Guides/00_Model/01_Data_Model_and_ORM.md @@ -489,6 +489,59 @@ offset, if not provided as an argument, will default to 0. Note that the `limit` argument order is different from a MySQL LIMIT clause. +### Mapping classes to tables with DataObjectSchema + +Note that in most cases, the underlying database table for any DataObject instance will be the same as the class name. +However in cases where dealing with namespaced classes, especially when using DB schema which don't support +slashes in table names, it is necessary to provide an alternate mapping. + +For instance, the below model will be stored in the table name `BannerImage` + + + :::php + namespace SilverStripe\BannerManager; + class BannerImage extends \DataObject { + private static $table_name = 'BannerImage'; + } + + +Note that any model class which does not explicitly declare a `table_name` config option will have a name +automatically generated for them. In the above case, the table name would have been +`SilverStripe\BannerManager\BannerImage` + +When creating raw SQL queries that contain table names, it is necessary to ensure your queries have the correct +table. This functionality can be provided by the [api:DataObjectSchema] service, which can be accessed via +`DataObject::getSchema()`. This service provides the following methods, most of which have a table and class +equivalent version. + +Methods which return class names: + + * `tableClass($table)` Finds the class name for a given table. This also handles suffixed tables such as `Table_Live`. + * `baseDataClass($class)` Returns the base data class for the given class. + * `classForField($class, $field)` Finds the specific class that directly holds the given field + +Methods which return table names: + + * `tableName($class)` Returns the table name for a given class or object. + * `baseDataTable($class)` Returns the base data class for the given class. + * `tableForField($class, $field)` Finds the specific class that directly holds the given field and returns the table. + +Note that in cases where the class name is required, an instance of the object may be substituted. + +For example, if running a query against a particular model, you will need to ensure you use the correct +table and column. + + + :::php + public function countDuplicates($model, $fieldToCheck) { + $table = DataObject::getSchema()->tableForField($model, $field); + $query = new SQLSelect(); + $query->setFrom("\"{$table}\""); + $query->setWhere(["\"{$table}\".\"{$field}\"" => $model->$fieldToCheck]); + return $query->count(); + } + + ### Raw SQL Occasionally, the system described above won't let you do exactly what you need to do. In these situations, we have @@ -620,3 +673,4 @@ To retrieve a news article, SilverStripe joins the [api:SiteTree], [api:Page] an * [api:DataObject] * [api:DataList] * [api:DataQuery] +* [api:DataObjectSchema] diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index 4f4d148dd..3bf1a0cb2 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -83,7 +83,7 @@ * Versioned constructor now only allows a single string to declare whether staging is enabled or not. The number of names of stages are no longer able to be specified. See below for upgrading notes for models with custom stages. - * `reading_stage` is now `set_stage` + * `reading_stage` is now `set_stage` and throws an error if setting an invalid stage. * `current_stage` is now `get_stage` * `getVersionedStages` is gone. * `get_live_stage` is removed. Use the `Versioned::LIVE` constant instead. @@ -91,8 +91,13 @@ * `$versionableExtensions` is now `private static` instead of `protected static` * `hasStages` is addded to check if an object has a given stage. * `stageTable` is added to get the table for a given class and stage. + * Any extension declared via `versionableExtensions` config on Versioned dataobject must now + `VersionableExtension` interface at a minimum. `Translatable` has been removed from default + `versionableExtensions` * `ChangeSet` and `ChangeSetItem` have been added for batch publishing of versioned dataobjects. * `FormAction::setValidationExempt` can be used to turn on or off form validation for individual actions + * `DataObject.table_name` config can now be used to customise the database table for any record. + * `DataObjectSchema` class added to assist with mapping between classes and tables. ### Front-end build tooling for CMS interface @@ -162,6 +167,10 @@ admin/font/ => admin/client/dist/font/ * History.js * `debugmethods` querystring argument has been removed from debugging. + + * The following ClassInfo methods are now deprecated: + * `ClassInfo::baseDataClass` - Use `DataObject::getSchema()->baseDataClass()` instead. + * `ClassInfo::table_for_object_field` - Use `DataObject::getSchema()->tableForField()` instead ### ORM @@ -563,6 +572,36 @@ these references should be replaced with `SQLSelect`. Legacy code which generate `SQLQuery` can still communicate with new code that expects `SQLSelect` as it is a subclass of `SQLSelect`, but the inverse is not true. +### Update code that references table names + +A major change in 4.0.0 is that now tables and class names can differ from model to model. In order to +fix a table name, to prevent it being changed (for instance, when applying a namespace to a model) +the `table_name` config can be applied to any DataObject class. + + + :::php + namespace SilverStripe\BannerManager; + class BannerImage extends \DataObject { + private static $table_name = 'BannerImage'; + } + + +In order to ensure you are using the correct table for any class a new [api:DataObjectSchema] service +is available to manage these mappings. + + + :::php + public function countDuplicates($model, $fieldToCheck) { + $table = DataObject::getSchema()->tableForField($model, $field); + $query = new SQLSelect(); + $query->setFrom("\"{$table}\""); + $query->setWhere(["\"{$table}\".\"{$field}\"" => $model->$fieldToCheck]); + return $query->count(); + } + + +See [versioned documentation](/developer_guides/model/data_model_and_orm) for more information. + ### Update implementations of augmentSQL Since this method now takes a `SQLSelect` as a first parameter, existing code referencing the deprecated `SQLQuery` diff --git a/filesystem/AssetControlExtension.php b/filesystem/AssetControlExtension.php index 7dcd070a4..85c1884e2 100644 --- a/filesystem/AssetControlExtension.php +++ b/filesystem/AssetControlExtension.php @@ -155,7 +155,7 @@ class AssetControlExtension extends \DataExtension } // Unauthenticated member to use for checking visibility - $baseClass = \ClassInfo::baseDataClass($this->owner); + $baseClass = $this->owner->baseClass(); $filter = array("\"{$baseClass}\".\"ID\"" => $this->owner->ID); $stages = $this->owner->getVersionedStages(); // {@see Versioned::getVersionedStages} foreach ($stages as $stage) { diff --git a/model/DataList.php b/model/DataList.php index 066d67eb3..af966839c 100644 --- a/model/DataList.php +++ b/model/DataList.php @@ -511,12 +511,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab $relationModelName = $query->applyRelation($relations, $linearOnly); // Find the db field the relation belongs to - $className = ClassInfo::table_for_object_field($relationModelName, $fieldName); - if(empty($className)) { - $className = $relationModelName; - } - - $columnName = '"'.$className.'"."'.$fieldName.'"'; + $columnName = DataObject::getSchema()->sqlColumnForField($relationModelName, $fieldName); } ); } diff --git a/model/DataObject.php b/model/DataObject.php index 82e6c3bcd..9b4c944d0 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -193,8 +193,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity 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_database_fields = array(); protected static $_cache_field_labels = array(); /** @@ -220,6 +218,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity 'AssetControl' => '\\SilverStripe\\Filesystem\\AssetControlExtension' ); + /** + * Override table name for this class. If ignored will default to FQN of class. + * This option is not inheritable, and must be set on each class. + * If left blank naming will default to the legacy (3.x) behaviour. + * + * @var string + */ + private static $table_name = null; + /** * Non-static relationship cache, indexed by component name. */ @@ -230,6 +237,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ protected $unsavedRelations; + /** + * Get schema object + * + * @return DataObjectSchema + */ + public static function getSchema() { + return Injector::inst()->get('DataObjectSchema'); + } + /** * Return the complete map of fields to specification on this object, including fixed_fields. * "ID" will be included on every table. @@ -246,74 +262,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if(empty($class)) { $class = get_called_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(singleton($fieldClass) instanceof DBComposite) { - $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; + return static::getSchema()->databaseFields($class); } /** @@ -337,10 +286,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $class = get_called_class(); } - // Get all fields - $fields = self::database_fields($class); - // Remove fixed fields. This assumes that NO fixed_fields are composite + $fields = static::getSchema()->databaseFields($class); $fields = array_diff_key($fields, self::config()->fixed_fields); return $fields; } @@ -377,24 +324,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if(empty($class)) { $class = get_called_class(); } - if($class === 'DataObject') { - return array(); - } - - // Refresh cache - self::cache_database_fields($class); - - // Get fields for this class - $compositeFields = self::$_cache_composite_fields[$class]; - if(!$aggregated) { - return $compositeFields; - } - - // Recursively merge - return array_merge( - $compositeFields, - self::composite_fields(get_parent_class($class)) - ); + return static::getSchema()->compositeFields($class, $aggregated); } /** @@ -1250,10 +1180,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @param string $now Timestamp to use for the current time * @param bool $isNewRecord Whether this should be treated as a new record write * @param array $manipulation Manipulation to write to - * @param string $class Table and Class to select and write to + * @param string $class Class of table to manipulate */ protected function prepareManipulationTable($baseTable, $now, $isNewRecord, &$manipulation, $class) { - $manipulation[$class] = array(); + $table = $this->getSchema()->tableName($class); + $manipulation[$table] = array(); // Extract records for this table foreach($this->record as $fieldName => $fieldValue) { @@ -1261,7 +1192,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Check if this record pertains to this table, and // we're not attempting to reset the BaseTable->ID if( empty($this->changed[$fieldName]) - || ($class === $baseTable && $fieldName === 'ID') + || ($table === $baseTable && $fieldName === 'ID') || (!self::has_own_table_database_field($class, $fieldName) && !self::is_composite_field($class, $fieldName, false)) ) { @@ -1276,25 +1207,26 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } // Write to manipulation - $fieldObj->writeToManipulation($manipulation[$class]); + $fieldObj->writeToManipulation($manipulation[$table]); } // Ensure update of Created and LastEdited columns - if($baseTable === $class) { - $manipulation[$class]['fields']['LastEdited'] = $now; + if($baseTable === $table) { + $manipulation[$table]['fields']['LastEdited'] = $now; if($isNewRecord) { - $manipulation[$class]['fields']['Created'] + $manipulation[$table]['fields']['Created'] = empty($this->record['Created']) ? $now : $this->record['Created']; - $manipulation[$class]['fields']['ClassName'] = $this->class; + $manipulation[$table]['fields']['ClassName'] = $this->class; } } // Inserts done one the base table are performed in another step, so the manipulation should instead // attempt an update, as though it were a normal update. - $manipulation[$class]['command'] = $isNewRecord ? 'insert' : 'update'; - $manipulation[$class]['id'] = $this->record['ID']; + $manipulation[$table]['command'] = $isNewRecord ? 'insert' : 'update'; + $manipulation[$table]['id'] = $this->record['ID']; + $manipulation[$table]['class'] = $class; } /** @@ -1379,7 +1311,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if($hasChanges || $forceWrite || $isNewRecord) { // New records have their insert into the base data table done first, so that they can pass the // generated primary key on to the rest of the manipulation - $baseTable = ClassInfo::baseDataClass($this->class); + $baseTable = $this->baseTable(); $this->writeBaseRecord($baseTable, $now); // Write the DB manipulation for all changed fields @@ -1730,7 +1662,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity case 'has_one': { // Mock has_many $joinField = "{$remoteRelation}ID"; - $componentClass = ClassInfo::table_for_object_field($remoteClass, $joinField); + $componentClass = static::getSchema()->classForField($remoteClass, $joinField); $result = HasManyList::create($componentClass, $joinField); if ($this->model) { $result->setDataModel($this->model); @@ -1907,7 +1839,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $result = $result->alterDataQuery(function($query) use ($extraFields) { $query->setQueryParam('Component.ExtraFields', $extraFields); }); - + if($this->model) { $result->setDataModel($this->model); } @@ -1998,12 +1930,13 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Return all of the database fields in this object * * @param string $fieldName Limit the output to a specific field name - * @param bool $includeTable If returning a single column, prefix the column with the table name + * @param bool $includeClass If returning a single column, prefix the column with the class 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 + * @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 FieldClass(args) + * format, or RecordClass.FieldClass(args) format if $includeClass is true */ - public function db($fieldName = null, $includeTable = false) { + public function db($fieldName = null, $includeClass = 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 @@ -2021,17 +1954,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Check for search field if($fieldName && isset($db[$fieldName])) { // Return found field - if(!$includeTable) { + if(!$includeClass) { return $db[$fieldName]; } - - // 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 $class . "." . $db[$fieldName]; } } @@ -2179,54 +2105,60 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity foreach($classes as $class) { $manyMany = Config::inst()->get($class, 'many_many', Config::UNINHERITED); // Check if the component is defined in many_many on this class - $candidate = (isset($manyMany[$component])) ? $manyMany[$component] : null; - if($candidate) { - $parentField = $class . "ID"; - $childField = ($class == $candidate) ? "ChildID" : $candidate . "ID"; - return array($class, $candidate, $parentField, $childField, "{$class}_$component"); + if(isset($manyMany[$component])) { + $candidate = $manyMany[$component]; + $classTable = static::getSchema()->tableName($class); + $candidateTable = static::getSchema()->tableName($candidate); + $parentField = "{$classTable}ID"; + $childField = $class === $candidate ? "ChildID" : "{$candidateTable}ID"; + $joinTable = "{$classTable}_{$component}"; + return array($class, $candidate, $parentField, $childField, $joinTable); } // Check if the component is defined in belongs_many_many on this class $belongsManyMany = Config::inst()->get($class, 'belongs_many_many', Config::UNINHERITED); - $candidate = (isset($belongsManyMany[$component])) ? $belongsManyMany[$component] : null; - if($candidate) { - // Extract class and relation name from dot-notation - if(strpos($candidate, '.') !== false) { - list($candidate, $relationName) = explode('.', $candidate, 2); - } + if(!isset($belongsManyMany[$component])) { + continue; + } - $childField = $candidate . "ID"; + // Extract class and relation name from dot-notation + $candidate = $belongsManyMany[$component]; + $relationName = null; + if(strpos($candidate, '.') !== false) { + list($candidate, $relationName) = explode('.', $candidate, 2); + } + $candidateTable = static::getSchema()->tableName($candidate); + $childField = $candidateTable . "ID"; - // We need to find the inverse component name - $otherManyMany = Config::inst()->get($candidate, 'many_many', Config::UNINHERITED); - if(!$otherManyMany) { - throw new LogicException("Inverse component of $candidate not found ({$this->class})"); - } - - // If we've got a relation name (extracted from dot-notation), we can already work out - // the join table and candidate class name... - if(isset($relationName) && isset($otherManyMany[$relationName])) { - $candidateClass = $otherManyMany[$relationName]; - $joinTable = "{$candidate}_{$relationName}"; - } else { - // ... otherwise, we need to loop over the many_manys and find a relation that - // matches up to this class - foreach($otherManyMany as $inverseComponentName => $candidateClass) { - if($candidateClass == $class || is_subclass_of($class, $candidateClass)) { - $joinTable = "{$candidate}_{$inverseComponentName}"; - break; - } + // We need to find the inverse component name, if not explicitly given + $otherManyMany = Config::inst()->get($candidate, 'many_many', Config::UNINHERITED); + if(!$relationName && $otherManyMany) { + foreach($otherManyMany as $inverseComponentName => $childClass) { + if($childClass === $class || is_subclass_of($class, $childClass)) { + $relationName = $inverseComponentName; + break; } } - - // If we could work out the join table, we've got all the info we need - if(isset($joinTable)) { - $parentField = ($class == $candidate) ? "ChildID" : $candidateClass . "ID"; - return array($class, $candidate, $parentField, $childField, $joinTable); - } - - throw new LogicException("Orphaned \$belongs_many_many value for $this->class.$component"); } + + // Check valid relation found + if(!$relationName || !$otherManyMany || !isset($otherManyMany[$relationName])) { + throw new LogicException("Inverse component of $candidate not found ({$this->class})"); + } + + // If we've got a relation name (extracted from dot-notation), we can already work out + // the join table and candidate class name... + $childClass = $otherManyMany[$relationName]; + $joinTable = "{$candidateTable}_{$relationName}"; + + // If we could work out the join table, we've got all the info we need + if ($childClass === $candidate) { + $parentField = "ChildID"; + } else { + $childTable = static::getSchema()->tableName($childClass); + $parentField = "{$childTable}ID"; + } + return array($class, $candidate, $parentField, $childField, $joinTable); } } @@ -2470,16 +2402,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Loads all the stub fields that an initial lazy load didn't load fully. * - * @param string $tableClass Base table to load the values from. Others are joined as required. - * Not specifying a tableClass will load all lazy fields from all tables. + * @param string $class Class to load the values from. Others are joined as required. + * Not specifying a tableClass will load all lazy fields from all tables. * @return bool Flag if lazy loading succeeded */ - protected function loadLazyFields($tableClass = null) { + protected function loadLazyFields($class = null) { if(!$this->isInDB() || !is_numeric($this->ID)) { return false; } - if (!$tableClass) { + if (!$class) { $loaded = array(); foreach ($this->record as $key => $value) { @@ -2492,7 +2424,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return false; } - $dataQuery = new DataQuery($tableClass); + $dataQuery = new DataQuery($class); // Reset query parameter context to that of this DataObject if($params = $this->getSourceQueryParams()) { @@ -2503,16 +2435,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Limit query to the current record, unless it has the Versioned extension, // in which case it requires special handling through augmentLoadLazyFields() - $baseTable = ClassInfo::baseDataClass($this); + $baseIDColumn = static::getSchema()->sqlColumnForField($this, 'ID'); $dataQuery->where([ - "\"{$baseTable}\".\"ID\"" => $this->record['ID'] + $baseIDColumn => $this->record['ID'] ])->limit(1); $columns = array(); // 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); + $databaseFields = self::database_fields($class); if($databaseFields) foreach($databaseFields as $k => $v) { if(!isset($this->record[$k]) || $this->record[$k] === null) { $columns[] = $k; @@ -2805,19 +2737,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * @return bool */ public static function has_own_table($dataClass) { - if(!is_subclass_of($dataClass,'DataObject')) return false; - - $dataClass = ClassInfo::class_name($dataClass); - if(!isset(self::$_cache_has_own_table[$dataClass])) { - if(get_parent_class($dataClass) == 'DataObject') { - self::$_cache_has_own_table[$dataClass] = true; - } else { - self::$_cache_has_own_table[$dataClass] - = Config::inst()->get($dataClass, 'db', Config::UNINHERITED) - || Config::inst()->get($dataClass, 'has_one', Config::UNINHERITED); - } + if(!is_subclass_of($dataClass, 'DataObject')) { + return false; } - return self::$_cache_has_own_table[$dataClass]; + $fields = static::database_fields($dataClass); + return !empty($fields); } /** @@ -3263,12 +3187,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Reset all global caches associated with DataObject. */ public static function reset() { + // @todo Decouple these DBClassName::clear_classname_cache(); + ClassInfo::reset_db_cache(); + static::getSchema()->reset(); 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(); } @@ -3286,25 +3210,27 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity user_error("DataObject::get_by_id passed a non-numeric ID #$id", E_USER_WARNING); } - // Check filter column - if(is_subclass_of($callerClass, 'DataObject')) { - $baseClass = ClassInfo::baseDataClass($callerClass); - $column = "\"$baseClass\".\"ID\""; - } else{ - // This simpler code will be used by non-DataObject classes that implement DataObjectInterface - $column = '"ID"'; - } - - // Relegate to get_one + // Pass to get_one + $column = static::getSchema()->sqlColumnForField($callerClass, 'ID'); return DataObject::get_one($callerClass, array($column => $id), $cache); } /** * Get the name of the base table for this object + * + * @return string */ public function baseTable() { - $tableClasses = ClassInfo::dataClassesFor($this->class); - return array_shift($tableClasses); + return static::getSchema()->baseDataTable($this); + } + + /** + * Get the base class for this object + * + * @return string + */ + public function baseClass() { + return static::getSchema()->baseDataClass($this); } /** @@ -3403,19 +3329,20 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity public function requireTable() { // Only build the table if we've actually got fields $fields = self::database_fields($this->class); + $table = static::getSchema()->tableName($this->class); $extensions = self::database_extensions($this->class); $indexes = $this->databaseIndexes(); // Validate relationship configuration $this->validateModelDefinitions(); - if($fields) { - $hasAutoIncPK = ($this->class == ClassInfo::baseDataClass($this->class)); - DB::require_table($this->class, $fields, $indexes, $hasAutoIncPK, $this->stat('create_table_options'), - $extensions); + $hasAutoIncPK = get_parent_class($this) === 'DataObject'; + DB::require_table( + $table, $fields, $indexes, $hasAutoIncPK, $this->stat('create_table_options'), $extensions + ); } else { - DB::dont_require_table($this->class); + DB::dont_require_table($table); } // Build any child tables for many_many items @@ -3423,9 +3350,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $extras = $this->uninherited('many_many_extraFields'); foreach($manyMany as $relationship => $childClass) { // Build field list + if($this->class === $childClass) { + $childField = "ChildID"; + } else { + $childTable = $this->getSchema()->tableName($childClass); + $childField = "{$childTable}ID"; + } $manymanyFields = array( - "{$this->class}ID" => "Int", - (($this->class == $childClass) ? "ChildID" : "{$childClass}ID") => "Int", + "{$table}ID" => "Int", + $childField => "Int", ); if(isset($extras[$relationship])) { $manymanyFields = array_merge($manymanyFields, $extras[$relationship]); @@ -3433,12 +3366,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Build index list $manymanyIndexes = array( - "{$this->class}ID" => true, - (($this->class == $childClass) ? "ChildID" : "{$childClass}ID") => true, + "{$table}ID" => true, + $childField => true, ); - - DB::require_table("{$this->class}_$relationship", $manymanyFields, $manymanyIndexes, true, null, - $extensions); + $manyManyTable = "{$table}_$relationship"; + DB::require_table($manyManyTable, $manymanyFields, $manymanyIndexes, true, null, $extensions); } } diff --git a/model/DataObjectSchema.php b/model/DataObjectSchema.php new file mode 100644 index 000000000..cdedc85cd --- /dev/null +++ b/model/DataObjectSchema.php @@ -0,0 +1,362 @@ +tableNames = []; + $this->databaseFields = []; + $this->compositeFields = []; + } + + /** + * Get all table names + * + * @return array + */ + public function getTableNames() { + $this->cacheTableNames(); + return $this->tableNames; + } + + /** + * Given a DataObject class and a field on that class, determine the appropriate SQL for + * selecting / filtering on in a SQL string. Note that $class must be a valid class, not an + * arbitrary table. + * + * The result will be a standard ANSI-sql quoted string in "Table"."Column" format. + * + * @param string $class Class name (not a table). + * @param string $field Name of field that belongs to this class (or a parent class) + * @return string The SQL identifier string for the corresponding column for this field + */ + public function sqlColumnForField($class, $field) { + $table = $this->tableForField($class, $field); + if(!$table) { + throw new InvalidArgumentException("\"{$field}\" is not a field on class \"{$class}\""); + } + return "\"{$table}\".\"{$field}\""; + } + + /** + * Get table name for the given class. + * + * Note that this does not confirm a table actually exists (or should exist), but returns + * the name that would be used if this table did exist. + * + * @param string $class + * @return string Returns the table name, or null if there is no table + */ + public function tableName($class) { + $tables = $this->getTableNames(); + $class = ClassInfo::class_name($class); + if(isset($tables[$class])) { + return $tables[$class]; + } + return null; + } + /** + * Returns the root class (the first to extend from DataObject) for the + * passed class. + * + * @param string|object $class + * @return string + * @throws InvalidArgumentException + */ + public function baseDataClass($class) { + $class = ClassInfo::class_name($class); + $current = $class; + while ($next = get_parent_class($current)) { + if ($next === 'DataObject') { + return $current; + } + $current = $next; + } + throw new InvalidArgumentException("$class is not a subclass of DataObject"); + } + + /** + * Get the base table + * + * @param string|object $class + * @return string + */ + public function baseDataTable($class) { + return $this->tableName($this->baseDataClass($class)); + } + + /** + * Find the class for the given table + * + * @param string $table + * @return string|null The FQN of the class, or null if not found + */ + public function tableClass($table) { + $tables = $this->getTableNames(); + $class = array_search($table, $tables, true); + if($class) { + return $class; + } + + // If there is no class for this table, strip table modifiers (e.g. _Live / _versions) + // from the end and re-attempt a search. + if(preg_match('/^(?.+)(_[^_]+)$/i', $table, $matches)) { + $table = $matches['class']; + $class = array_search($table, $tables, true); + if($class) { + return $class; + } + } + return null; + } + + /** + * Cache all table names if necessary + */ + protected function cacheTableNames() { + if($this->tableNames) { + return; + } + $this->tableNames = []; + foreach(ClassInfo::subclassesFor('DataObject') as $class) { + if($class === 'DataObject') { + continue; + } + $table = $this->buildTableName($class); + + // Check for conflicts + $conflict = array_search($table, $this->tableNames, true); + if($conflict) { + throw new LogicException( + "Multiple classes (\"{$class}\", \"{$conflict}\") map to the same table: \"{$table}\"" + ); + } + $this->tableNames[$class] = $table; + } + } + + /** + * Generate table name for a class. + * + * Note: some DB schema have a hard limit on table name length. This is not enforced by this method. + * See dev/build errors for details in case of table name violation. + * + * @param string $class + * @return string + */ + protected function buildTableName($class) { + $table = Config::inst()->get($class, 'table_name', Config::UNINHERITED); + + // Generate default table name + if(!$table) { + $separator = $this->config()->table_namespace_separator; + $table = str_replace('\\', $separator, trim($class, '\\')); + } + + return $table; + } + + /** + * Return the complete map of fields to specification on this object, including fixed_fields. + * "ID" will be included on every table. + * + * @param string $class Class name to query from + * @return array Map of fieldname to specification, similiar to {@link DataObject::$db}. + */ + public function databaseFields($class) { + $class = ClassInfo::class_name($class); + if($class === 'DataObject') { + return []; + } + $this->cacheDatabaseFields($class); + return $this->databaseFields[$class]; + } + + /** + * 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 List of composite fields and their class spec + */ + public function compositeFields($class, $aggregated = true) { + $class = ClassInfo::class_name($class); + if($class === 'DataObject') { + return []; + } + $this->cacheDatabaseFields($class); + + // Get fields for this class + $compositeFields = $this->compositeFields[$class]; + if(!$aggregated) { + return $compositeFields; + } + + // Recursively merge + $parentFields = $this->compositeFields(get_parent_class($class)); + return array_merge($compositeFields, $parentFields); + } + + /** + * Cache all database and composite fields for the given class. + * Will do nothing if already cached + * + * @param string $class Class name to cache + */ + protected function cacheDatabaseFields($class) { + // Skip if already cached + if (isset($this->databaseFields[$class]) && isset($this->compositeFields[$class])) { + return; + } + $compositeFields = array(); + $dbFields = array(); + + // Ensure fixed fields appear at the start + $fixedFields = DataObject::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(singleton($fieldClass) instanceof DBComposite) { + $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; + } + } + + // Prevent field-less tables + if(count($dbFields) < 2) { + $dbFields = []; + } + + // Return cached results + $this->databaseFields[$class] = $dbFields; + $this->compositeFields[$class] = $compositeFields; + } + + /** + * Returns the table name in the class hierarchy which contains a given + * field column for a {@link DataObject}. If the field does not exist, this + * will return null. + * + * @param string $candidateClass + * @param string $fieldName + * @return string + */ + public function tableForField($candidateClass, $fieldName) { + $class = $this->classForField($candidateClass, $fieldName); + if($class) { + return $this->tableName($class); + } + return null; + } + + /** + * Returns the class name in the class hierarchy which contains a given + * field column for a {@link DataObject}. If the field does not exist, this + * will return null. + * + * @param string $candidateClass + * @param string $fieldName + * @return string + */ + public function classForField($candidateClass, $fieldName) { + // normalise class name + $candidateClass = ClassInfo::class_name($candidateClass); + if($candidateClass === 'DataObject') { + return null; + } + + // Short circuit for fixed fields + $fixed = DataObject::config()->fixed_fields; + if(isset($fixed[$fieldName])) { + return $this->baseDataClass($candidateClass); + } + + // Find regular field + while($candidateClass) { + $fields = $this->databaseFields($candidateClass); + if(isset($fields[$fieldName])) { + return $candidateClass; + } + $candidateClass = get_parent_class($candidateClass); + } + return null; + } +} diff --git a/model/DataQuery.php b/model/DataQuery.php index b9a095e19..5772d75f2 100644 --- a/model/DataQuery.php +++ b/model/DataQuery.php @@ -24,6 +24,16 @@ class DataQuery { protected $query; /** + * Map of all field names to an array of conflicting column SQL + * + * E.g. + * array( + * 'Title' => array( + * '"MyTable"."Title"', + * '"AnotherTable"."Title"', + * ) + * ) + * * @var array */ protected $collidingFields = array(); @@ -31,7 +41,7 @@ class DataQuery { private $queriedColumns = null; /** - * @var Boolean + * @var bool */ private $queryFinalised = false; @@ -131,15 +141,12 @@ class DataQuery { * Set up the simplest initial query */ protected function initialiseQuery() { - // Get the tables to join to. - // Don't get any subclass tables - let lazy loading do that. - $tableClasses = ClassInfo::ancestry($this->dataClass, true); - if(!$tableClasses) { + // Join on base table and let lazy loading join subtables + $baseClass = DataObject::getSchema()->baseDataClass($this->dataClass()); + if(!$baseClass) { throw new InvalidArgumentException("DataQuery::create() Can't find data classes for '{$this->dataClass}'"); } - $baseClass = array_shift($tableClasses); - // Build our intial query $this->query = new SQLSelect(array()); $this->query->setDistinct(true); @@ -148,7 +155,8 @@ class DataQuery { $this->sort($sort); } - $this->query->setFrom("\"$baseClass\""); + $baseTable = DataObject::getSchema()->tableName($baseClass); + $this->query->setFrom("\"{$baseTable}\""); $obj = Injector::inst()->get($baseClass); $obj->extend('augmentDataQueryCreation', $this->query, $this); @@ -165,13 +173,18 @@ class DataQuery { * @return SQLSelect The finalised sql query */ public function getFinalisedQuery($queriedColumns = null) { - if(!$queriedColumns) $queriedColumns = $this->queriedColumns; + if(!$queriedColumns) { + $queriedColumns = $this->queriedColumns; + } if($queriedColumns) { $queriedColumns = array_merge($queriedColumns, array('Created', 'LastEdited', 'ClassName')); } + $schema = DataObject::getSchema(); $query = clone $this->query; - $ancestorTables = ClassInfo::ancestry($this->dataClass, true); + $baseDataClass = $schema->baseDataClass($this->dataClass()); + $baseIDColumn = $schema->sqlColumnForField($baseDataClass, 'ID'); + $ancestorClasses = ClassInfo::ancestry($this->dataClass(), true); // Generate the list of tables to iterate over and the list of columns required // by any existing where clauses. This second step is skipped if we're fetching @@ -180,20 +193,25 @@ class DataQuery { // Specifying certain columns allows joining of child tables $tableClasses = ClassInfo::dataClassesFor($this->dataClass); + // Ensure that any filtered columns are included in the selected columns foreach ($query->getWhereParameterised($parameters) as $where) { - // Check for just the column, in the form '"Column" = ?' and the form '"Table"."Column"' = ? - if (preg_match('/^"([^"]+)"/', $where, $matches) || - preg_match('/^"([^"]+)"\."[^"]+"/', $where, $matches)) { - if (!in_array($matches[1], $queriedColumns)) $queriedColumns[] = $matches[1]; + // Check for any columns in the form '"Column" = ?' or '"Table"."Column"' = ? + if(preg_match_all( + '/(?:"(?[^"]+)"\.)?"(?[^"]+)"(?:[^\.]|$)/', + $where, $matches, PREG_SET_ORDER + )) { + foreach($matches as $match) { + $column = $match['column']; + if (!in_array($column, $queriedColumns)) { + $queriedColumns[] = $column; + } + } } } } else { - $tableClasses = $ancestorTables; + $tableClasses = $ancestorClasses; } - $tableNames = array_values($tableClasses); - $baseClass = $tableNames[0]; - // Iterate over the tables and check what we need to select from them. If any selects are made (or the table is // required for a select) foreach($tableClasses as $tableClass) { @@ -208,7 +226,9 @@ class DataQuery { } // If this is a subclass without any explicitly requested columns, omit this from the query - if(!in_array($tableClass, $ancestorTables) && empty($selectColumns)) continue; + if(!in_array($tableClass, $ancestorClasses) && empty($selectColumns)) { + continue; + } // Select necessary columns (unless an explicitly empty array) if($selectColumns !== array()) { @@ -216,51 +236,61 @@ class DataQuery { } // Join if not the base table - if($tableClass !== $baseClass) { - $query->addLeftJoin($tableClass, "\"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"", $tableClass, 10); + if($tableClass !== $baseDataClass) { + $tableName = $schema->tableName($tableClass); + $query->addLeftJoin( + $tableName, + "\"{$tableName}\".\"ID\" = {$baseIDColumn}", + $tableName, + 10 + ); } } - - // Resolve colliding fields if($this->collidingFields) { - foreach($this->collidingFields as $k => $collisions) { + foreach($this->collidingFields as $collisionField => $collisions) { $caseClauses = array(); foreach($collisions as $collision) { - if(preg_match('/^"([^"]+)"/', $collision, $matches)) { - $collisionBase = $matches[1]; - if(class_exists($collisionBase)) { - $collisionClasses = ClassInfo::subclassesFor($collisionBase); - $collisionClasses = Convert::raw2sql($collisionClasses, true); - $caseClauses[] = "WHEN \"$baseClass\".\"ClassName\" IN (" - . implode(", ", $collisionClasses) . ") THEN $collision"; + if(preg_match('/^"(?
[^"]+)"\./', $collision, $matches)) { + $collisionTable = $matches['table']; + $collisionClass = $schema->tableClass($collisionTable); + if($collisionClass) { + $collisionClassColumn = $schema->sqlColumnForField($collisionClass, 'ClassName'); + $collisionClasses = ClassInfo::subclassesFor($collisionClass); + $collisionClassesSQL = implode(', ', Convert::raw2sql($collisionClasses, true)); + $caseClauses[] = "WHEN {$collisionClassColumn} IN ({$collisionClassesSQL}) THEN $collision"; } } else { user_error("Bad collision item '$collision'", E_USER_WARNING); } } - $query->selectField("CASE " . implode( " ", $caseClauses) . " ELSE NULL END", $k); + $query->selectField("CASE " . implode( " ", $caseClauses) . " ELSE NULL END", $collisionField); } } if($this->filterByClassName) { // If querying the base class, don't bother filtering on class name - if($this->dataClass != $baseClass) { + if($this->dataClass != $baseDataClass) { // Get the ClassName values to filter to $classNames = ClassInfo::subclassesFor($this->dataClass); $classNamesPlaceholders = DB::placeholders($classNames); + $baseClassColumn = $schema->sqlColumnForField($baseDataClass, 'ClassName'); $query->addWhere(array( - "\"$baseClass\".\"ClassName\" IN ($classNamesPlaceholders)" => $classNames + "{$baseClassColumn} IN ($classNamesPlaceholders)" => $classNames )); } } - $query->selectField("\"$baseClass\".\"ID\"", "ID"); + // Select ID + $query->selectField($baseIDColumn, "ID"); + + // Select RecordClassName + $baseClassColumn = $schema->sqlColumnForField($baseDataClass, 'ClassName'); $query->selectField(" - CASE WHEN \"$baseClass\".\"ClassName\" IS NOT NULL THEN \"$baseClass\".\"ClassName\" - ELSE ".Convert::raw2sql($baseClass, true)." END", + CASE WHEN {$baseClassColumn} IS NOT NULL THEN {$baseClassColumn} + ELSE ".Convert::raw2sql($baseDataClass, true)." END", "RecordClassName" ); @@ -283,9 +313,6 @@ class DataQuery { * @return null */ protected function ensureSelectContainsOrderbyColumns($query, $originalSelect = array()) { - $tableClasses = ClassInfo::dataClassesFor($this->dataClass); - $baseClass = array_shift($tableClasses); - if($orderby = $query->getOrderBy()) { $newOrderby = array(); $i = 0; @@ -309,10 +336,9 @@ class DataQuery { } if(count($parts) == 1) { - - if(DataObject::has_own_table_database_field($baseClass, $parts[0])) { - $qualCol = "\"$baseClass\".\"{$parts[0]}\""; - } else { + // Get expression for sort value + $qualCol = DataObject::getSchema()->sqlColumnForField($this->dataClass(), $parts[0]);; + if(!$qualCol) { $qualCol = "\"$parts[0]\""; } @@ -369,10 +395,12 @@ class DataQuery { /** * Return the number of records in this query. * Note that this will issue a separate SELECT COUNT() query. + * + * @return int */ public function count() { - $baseClass = ClassInfo::baseDataClass($this->dataClass); - return $this->getFinalisedQuery()->count("DISTINCT \"$baseClass\".\"ID\""); + $quotedColumn = DataObject::getSchema()->sqlColumnForField($this->dataClass(), 'ID'); + return $this->getFinalisedQuery()->count("DISTINCT {$quotedColumn}"); } /** @@ -450,7 +478,7 @@ class DataQuery { * Update the SELECT clause of the query with the columns from the given table * * @param SQLSelect $query - * @param string $tableClass + * @param string $tableClass Class to select from * @param array $columns */ protected function selectColumnsFromTable(SQLSelect &$query, $tableClass, $columns = null) { @@ -461,19 +489,23 @@ class DataQuery { 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)) { - if(!isset($this->collidingFields[$k])) $this->collidingFields[$k] = array($expressionForField); - $this->collidingFields[$k][] = "\"$tableClass\".\"$k\""; - + $expressionForField = $query->expressionForField($k); + $quotedField = DataObject::getSchema()->sqlColumnForField($tableClass, $k); + if($expressionForField) { + if(!isset($this->collidingFields[$k])) { + $this->collidingFields[$k] = array($expressionForField); + } + $this->collidingFields[$k][] = $quotedField; } else { - $query->selectField("\"$tableClass\".\"$k\"", $k); + $query->selectField($quotedField, $k); } } } foreach($compositeFields as $k => $v) { if((is_null($columns) || in_array($k, $columns)) && $v) { + $tableName = DataObject::getSchema()->tableName($tableClass); $dbO = Object::create_from_string($v, $k); - $dbO->setTable($tableClass); + $dbO->setTable($tableName); $dbO->addToQuery($query); } } @@ -724,18 +756,19 @@ class DataQuery { "Could not join polymorphic has_one relationship {$localField} on {$localClass}" ); } + $schema = DataObject::getSchema(); // Skip if already joined - if($this->query->isJoinedTo($foreignClass)) { + $foreignBaseClass = $schema->baseDataClass($foreignClass); + $foreignBaseTable = $schema->tableName($foreignBaseClass); + if($this->query->isJoinedTo($foreignBaseTable)) { return; } - $realModelClass = ClassInfo::table_for_object_field($localClass, "{$localField}ID"); - $foreignBase = ClassInfo::baseDataClass($foreignClass); - $this->query->addLeftJoin( - $foreignBase, - "\"$foreignBase\".\"ID\" = \"{$realModelClass}\".\"{$localField}ID\"" - ); + // Join base table + $foreignIDColumn = $schema->sqlColumnForField($foreignBaseClass, 'ID'); + $localColumn = $schema->sqlColumnForField($localClass, "{$localField}ID"); + $this->query->addLeftJoin($foreignBaseTable, "{$foreignIDColumn} = {$localColumn}"); /** * add join clause to the component's ancestry classes so that the search filter could search on @@ -745,8 +778,9 @@ class DataQuery { if(!empty($ancestry)){ $ancestry = array_reverse($ancestry); foreach($ancestry as $ancestor){ - if($ancestor != $foreignBase) { - $this->query->addLeftJoin($ancestor, "\"$foreignBase\".\"ID\" = \"$ancestor\".\"ID\""); + $ancestorTable = $schema->tableName($ancestor); + if($ancestorTable !== $foreignBaseTable) { + $this->query->addLeftJoin($ancestorTable, "{$foreignIDColumn} = \"{$ancestorTable}\".\"ID\""); } } } @@ -765,28 +799,30 @@ class DataQuery { if(!$foreignClass || $foreignClass === 'DataObject') { throw new InvalidArgumentException("Could not find a has_many relationship {$localField} on {$localClass}"); } + $schema = DataObject::getSchema(); // Skip if already joined - if($this->query->isJoinedTo($foreignClass)) { + $foreignTable = $schema->tableName($foreignClass); + if($this->query->isJoinedTo($foreignTable)) { return; } // Join table with associated has_one /** @var DataObject $model */ $model = singleton($localClass); - $ancestry = $model->getClassAncestry(); $foreignKey = $model->getRemoteJoinField($localField, 'has_many', $polymorphic); + $localIDColumn = $schema->sqlColumnForField($localClass, 'ID'); if($polymorphic) { + $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}ID"); + $foreignKeyClassColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}Class"); + $localClassColumn = $schema->sqlColumnForField($localClass, 'ClassName'); $this->query->addLeftJoin( - $foreignClass, - "\"$foreignClass\".\"{$foreignKey}ID\" = \"{$ancestry[0]}\".\"ID\" AND " - . "\"$foreignClass\".\"{$foreignKey}Class\" = \"{$ancestry[0]}\".\"ClassName\"" + $foreignTable, + "{$foreignKeyIDColumn} = {$localIDColumn} AND {$foreignKeyClassColumn} = {$localClassColumn}" ); } else { - $this->query->addLeftJoin( - $foreignClass, - "\"$foreignClass\".\"{$foreignKey}\" = \"{$ancestry[0]}\".\"ID\"" - ); + $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, $foreignKey); + $this->query->addLeftJoin($foreignTable, "{$foreignKeyIDColumn} = {$localIDColumn}"); } /** @@ -795,9 +831,10 @@ class DataQuery { */ $ancestry = ClassInfo::ancestry($foreignClass, true); $ancestry = array_reverse($ancestry); - foreach($ancestry as $ancestor){ - if($ancestor != $foreignClass){ - $this->query->addInnerJoin($ancestor, "\"$foreignClass\".\"ID\" = \"$ancestor\".\"ID\""); + foreach($ancestry as $ancestor) { + $ancestorTable = $schema->tableName($ancestor); + if($ancestorTable !== $foreignTable) { + $this->query->addInnerJoin($ancestorTable, "\"{$foreignTable}\".\"ID\" = \"{$ancestorTable}\".\"ID\""); } } } @@ -812,16 +849,23 @@ class DataQuery { * @param string $relationTable Name of relation table */ protected function joinManyManyRelationship($parentClass, $componentClass, $parentField, $componentField, $relationTable) { - $parentBaseClass = ClassInfo::baseDataClass($parentClass); - $componentBaseClass = ClassInfo::baseDataClass($componentClass); + $schema = DataObject::getSchema(); + + // Join on parent table + $parentIDColumn = $schema->sqlColumnForField($parentClass, 'ID'); $this->query->addLeftJoin( $relationTable, - "\"$relationTable\".\"$parentField\" = \"$parentBaseClass\".\"ID\"" + "\"$relationTable\".\"$parentField\" = {$parentIDColumn}" ); - if (!$this->query->isJoinedTo($componentBaseClass)) { + + // Join on base table of component class + $componentBaseClass = $schema->baseDataClass($componentClass); + $componentBaseTable = $schema->tableName($componentBaseClass); + $componentIDColumn = $schema->sqlColumnForField($componentBaseClass, 'ID'); + if (!$this->query->isJoinedTo($componentBaseTable)) { $this->query->addLeftJoin( - $componentBaseClass, - "\"$relationTable\".\"$componentField\" = \"$componentBaseClass\".\"ID\"" + $componentBaseTable, + "\"$relationTable\".\"$componentField\" = {$componentIDColumn}" ); } @@ -831,9 +875,10 @@ class DataQuery { */ $ancestry = ClassInfo::ancestry($componentClass, true); $ancestry = array_reverse($ancestry); - foreach($ancestry as $ancestor){ - if($ancestor != $componentBaseClass && !$this->query->isJoinedTo($ancestor)){ - $this->query->addInnerJoin($ancestor, "\"$componentBaseClass\".\"ID\" = \"$ancestor\".\"ID\""); + foreach($ancestry as $ancestor) { + $ancestorTable = $schema->tableName($ancestor); + if($ancestorTable != $componentBaseTable && !$this->query->isJoinedTo($ancestorTable)) { + $this->query->addLeftJoin($ancestorTable, "{$componentIDColumn} = \"{$ancestorTable}\".\"ID\""); } } } @@ -866,7 +911,7 @@ class DataQuery { */ public function selectFromTable($table, $fields) { $fieldExpressions = array_map(function($item) use($table) { - return "\"$table\".\"$item\""; + return "\"{$table}\".\"{$item}\""; }, $fields); $this->query->setSelect($fieldExpressions); @@ -908,7 +953,7 @@ class DataQuery { // Special case for ID, if not provided if($field === 'ID') { - return DataObject::quoted_column('ID', $this->dataClass); + return DataObject::getSchema()->sqlColumnForField($this->dataClass, 'ID'); } return null; } diff --git a/model/FieldType/DBClassName.php b/model/FieldType/DBClassName.php index 9c1ad7fb7..b95dcd9fe 100644 --- a/model/FieldType/DBClassName.php +++ b/model/FieldType/DBClassName.php @@ -91,13 +91,14 @@ class DBClassName extends DBEnum { return $this->baseClass; } // Default to the basename of the record + $schema = DataObject::getSchema(); if($this->record) { - return ClassInfo::baseDataClass($this->record); + return $schema->baseDataClass($this->record); } // During dev/build only the table is assigned - $tableClass = $this->getClassNameFromTable($this->getTable()); - if($tableClass) { - return $tableClass; + $tableClass = $schema->tableClass($this->getTable()); + if($tableClass && ($baseClass = $schema->baseDataClass($tableClass))) { + return $baseClass; } // Fallback to global default return 'DataObject'; @@ -114,28 +115,6 @@ class DBClassName extends DBEnum { return $this; } - /** - * Given a table name, find the base data class - * - * @param string $table - * @return string|null - */ - protected function getClassNameFromTable($table) { - if(empty($table)) { - return null; - } - $class = ClassInfo::baseDataClass($table); - if($class) { - return $class; - } - // If there is no class for this table, strip table modifiers (_Live / _versions) off the end - if(preg_match('/^(?.+)(_[^_]+)$/i', $table, $matches)) { - return $this->getClassNameFromTable($matches['class']); - } - - return null; - } - /** * Get list of classnames that should be selectable * diff --git a/model/HasManyList.php b/model/HasManyList.php index 86a39e35f..172b8267d 100644 --- a/model/HasManyList.php +++ b/model/HasManyList.php @@ -35,15 +35,18 @@ class HasManyList extends RelationList { } protected function foreignIDFilter($id = null) { - if ($id === null) $id = $this->getForeignID(); + if ($id === null) { + $id = $this->getForeignID(); + } // Apply relation filter - $key = "\"$this->foreignKey\""; + $key = DataObject::getSchema()->sqlColumnForField($this->dataClass(), $this->getForeignKey()); if(is_array($id)) { return array("$key IN (".DB::placeholders($id).")" => $id); } else if($id !== null){ return array($key => $id); } + return null; } /** diff --git a/model/Hierarchy.php b/model/Hierarchy.php index f4ded65e0..ca2575fd4 100644 --- a/model/Hierarchy.php +++ b/model/Hierarchy.php @@ -449,32 +449,32 @@ class Hierarchy extends DataExtension { * Mark this DataObject as expanded. */ public function markExpanded() { - self::$marked[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; - self::$expanded[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; + self::$marked[$this->owner->baseClass()][$this->owner->ID] = true; + self::$expanded[$this->owner->baseClass()][$this->owner->ID] = true; } /** * Mark this DataObject as unexpanded. */ public function markUnexpanded() { - self::$marked[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; - self::$expanded[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = false; + self::$marked[$this->owner->baseClass()][$this->owner->ID] = true; + self::$expanded[$this->owner->baseClass()][$this->owner->ID] = false; } /** * Mark this DataObject's tree as opened. */ public function markOpened() { - self::$marked[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; - self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID] = true; + self::$marked[$this->owner->baseClass()][$this->owner->ID] = true; + self::$treeOpened[$this->owner->baseClass()][$this->owner->ID] = true; } /** * Mark this DataObject's tree as closed. */ public function markClosed() { - if(isset(self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID])) { - unset(self::$treeOpened[ClassInfo::baseDataClass($this->owner->class)][$this->owner->ID]); + if(isset(self::$treeOpened[$this->owner->baseClass()][$this->owner->ID])) { + unset(self::$treeOpened[$this->owner->baseClass()][$this->owner->ID]); } } @@ -484,7 +484,7 @@ class Hierarchy extends DataExtension { * @return bool */ public function isMarked() { - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $id = $this->owner->ID; return isset(self::$marked[$baseClass][$id]) ? self::$marked[$baseClass][$id] : false; } @@ -495,7 +495,7 @@ class Hierarchy extends DataExtension { * @return bool */ public function isExpanded() { - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $id = $this->owner->ID; return isset(self::$expanded[$baseClass][$id]) ? self::$expanded[$baseClass][$id] : false; } @@ -506,7 +506,7 @@ class Hierarchy extends DataExtension { * @return bool */ public function isTreeOpened() { - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $id = $this->owner->ID; return isset(self::$treeOpened[$baseClass][$id]) ? self::$treeOpened[$baseClass][$id] : false; } @@ -593,7 +593,7 @@ class Hierarchy extends DataExtension { public function doAllChildrenIncludingDeleted($context = null) { if(!$this->owner) user_error('Hierarchy::doAllChildrenIncludingDeleted() called without $this->owner'); - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); if($baseClass) { $stageChildren = $this->owner->stageChildren(true); @@ -630,9 +630,13 @@ class Hierarchy extends DataExtension { throw new Exception('Hierarchy->AllHistoricalChildren() only works with Versioned extension applied'); } - $baseClass=ClassInfo::baseDataClass($this->owner->class); - return Versioned::get_including_deleted($baseClass, - "\"ParentID\" = " . (int)$this->owner->ID, "\"$baseClass\".\"ID\" ASC"); + $baseTable = $this->owner->baseTable(); + $parentIDColumn = $this->owner->getSchema()->sqlColumnForField($this->owner, 'ParentID'); + return Versioned::get_including_deleted( + $this->owner->baseClass(), + [ $parentIDColumn => $this->owner->ID ], + "\"{$baseTable}\".\"ID\" ASC" + ); } /** @@ -646,8 +650,7 @@ class Hierarchy extends DataExtension { throw new Exception('Hierarchy->AllHistoricalChildren() only works with Versioned extension applied'); } - return Versioned::get_including_deleted(ClassInfo::baseDataClass($this->owner->class), - "\"ParentID\" = " . (int)$this->owner->ID)->count(); + return $this->AllHistoricalChildren()->count(); } /** @@ -689,7 +692,7 @@ class Hierarchy extends DataExtension { * @return DataList */ public function stageChildren($showAll = false) { - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $hide_from_hierarchy = $this->owner->config()->hide_from_hierarchy; $hide_from_cms_tree = $this->owner->config()->hide_from_cms_tree; $staged = $baseClass::get() @@ -722,7 +725,7 @@ class Hierarchy extends DataExtension { throw new Exception('Hierarchy->liveChildren() only works with Versioned extension applied'); } - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $hide_from_hierarchy = $this->owner->config()->hide_from_hierarchy; $hide_from_cms_tree = $this->owner->config()->hide_from_cms_tree; $children = $baseClass::get() @@ -822,7 +825,7 @@ class Hierarchy extends DataExtension { } $nextNode = null; - $baseClass = ClassInfo::baseDataClass($this->owner->class); + $baseClass = $this->owner->baseClass(); $children = $baseClass::get() ->filter('ParentID', (int)$this->owner->ID) @@ -832,7 +835,7 @@ class Hierarchy extends DataExtension { } // Try all the siblings of this node after the given node - /*if( $siblings = DataObject::get( ClassInfo::baseDataClass($this->owner->class), + /*if( $siblings = DataObject::get( $this->owner->baseClass(), "\"ParentID\"={$this->owner->ParentID}" . ( $afterNode ) ? "\"Sort\" > {$afterNode->Sort}" : "" , '\"Sort\" ASC' ) ) $searchNodes->merge( $siblings );*/ diff --git a/model/ManyManyList.php b/model/ManyManyList.php index 3ec00ce81..3f5a2bd60 100644 --- a/model/ManyManyList.php +++ b/model/ManyManyList.php @@ -49,7 +49,7 @@ class ManyManyList extends RelationList { * @param string $joinTable The name of the table whose entries define the content of this many_many relation. * @param string $localKey The key in the join table that maps to the dataClass' PK. * @param string $foreignKey The key in the join table that maps to joined class' PK. - * @param string $extraFields A map of field => fieldtype of extra fields on the join table. + * @param array $extraFields A map of field => fieldtype of extra fields on the join table. * * @example new ManyManyList('Group','Group_Members', 'GroupID', 'MemberID'); */ @@ -69,8 +69,11 @@ class ManyManyList extends RelationList { */ protected function linkJoinTable() { // Join to the many-many join table - $baseClass = ClassInfo::baseDataClass($this->dataClass); - $this->dataQuery->innerJoin($this->joinTable, "\"{$this->joinTable}\".\"{$this->localKey}\" = \"{$baseClass}\".\"ID\""); + $dataClassIDColumn = DataObject::getSchema()->sqlColumnForField($this->dataClass(), 'ID'); + $this->dataQuery->innerJoin( + $this->joinTable, + "\"{$this->joinTable}\".\"{$this->localKey}\" = {$dataClassIDColumn}" + ); // Add the extra fields to the query if($this->extraFields) { @@ -184,6 +187,7 @@ class ManyManyList extends RelationList { * @param mixed $item * @param array $extraFields A map of additional columns to insert into the joinTable. * Column names should be ANSI quoted. + * @throws Exception */ public function add($item, $extraFields = array()) { // Ensure nulls or empty strings are correctly treated as empty arrays @@ -292,7 +296,9 @@ class ManyManyList extends RelationList { user_error("Can't call ManyManyList::remove() until a foreign ID is set", E_USER_WARNING); } - $query->addWhere(array("\"{$this->localKey}\"" => $itemID)); + $query->addWhere(array( + "\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID + )); $query->execute(); } @@ -303,15 +309,16 @@ class ManyManyList extends RelationList { * @return void */ public function removeAll() { - $base = ClassInfo::baseDataClass($this->dataClass()); // Remove the join to the join table to avoid MySQL row locking issues. $query = $this->dataQuery(); $foreignFilter = $query->getQueryParam('Foreign.Filter'); $query->removeFilterOn($foreignFilter); + // Select ID column $selectQuery = $query->query(); - $selectQuery->setSelect("\"{$base}\".\"ID\""); + $dataClassIDColumn = DataObject::getSchema()->sqlColumnForField($this->dataClass(), 'ID'); + $selectQuery->setSelect($dataClassIDColumn); $from = $selectQuery->getFrom(); unset($from[$this->joinTable]); @@ -364,7 +371,7 @@ class ManyManyList extends RelationList { user_error("Can't call ManyManyList::getExtraData() until a foreign ID is set", E_USER_WARNING); } $query->addWhere(array( - "\"{$this->localKey}\"" => $itemID + "\"{$this->joinTable}\".\"{$this->localKey}\"" => $itemID )); $queryResult = $query->execute()->current(); if ($queryResult) { diff --git a/model/connect/DBConnector.php b/model/connect/DBConnector.php index 59c859a7d..ac181928c 100644 --- a/model/connect/DBConnector.php +++ b/model/connect/DBConnector.php @@ -174,26 +174,6 @@ abstract class DBConnector { */ abstract public function quoteString($value); - /** - * Escapes an identifier (table / database name). Typically the value - * is simply double quoted. Don't pass in already escaped identifiers in, - * as this will double escape the value! - * - * @param string $value The identifier to escape - * @param string $separator optional identifier splitter - */ - public function escapeIdentifier($value, $separator = '.') { - // ANSI standard id escape is to surround with double quotes - if(empty($separator)) return '"'.trim($value).'"'; - - // Split, escape, and glue back multiple identifiers - $segments = array(); - foreach(explode($separator, $value) as $item) { - $segments[] = $this->escapeIdentifier($item, null); - } - return implode($separator, $segments); - } - /** * Executes the following query with the specified error level. * Implementations of this function should respect previewWrite and benchmarkQuery diff --git a/model/connect/Database.php b/model/connect/Database.php index 5113821ec..52cc62648 100644 --- a/model/connect/Database.php +++ b/model/connect/Database.php @@ -232,11 +232,18 @@ abstract class SS_Database { * is simply double quoted. Don't pass in already escaped identifiers in, * as this will double escape the value! * - * @param string $value The identifier to escape - * @param string $separator optional identifier splitter + * @param string|array $value The identifier to escape or list of split components + * @param string $separator Splitter for each component + * @return string */ public function escapeIdentifier($value, $separator = '.') { - return $this->connector->escapeIdentifier($value, $separator); + // Split string into components + if(!is_array($value)) { + $value = explode($separator, $value); + } + + // Implode quoted column + return '"' . implode('"'.$separator.'"', $value) . '"'; } /** diff --git a/model/versioning/ChangeSet.php b/model/versioning/ChangeSet.php index 19a882d4d..ec6aeda0d 100644 --- a/model/versioning/ChangeSet.php +++ b/model/versioning/ChangeSet.php @@ -117,7 +117,7 @@ class ChangeSet extends DataObject { $references = [ 'ObjectID' => $object->ID, - 'ObjectClass' => ClassInfo::baseDataClass($object) + 'ObjectClass' => $object->baseClass(), ]; // Get existing item in case already added @@ -146,7 +146,7 @@ class ChangeSet extends DataObject { public function removeObject(DataObject $object) { $item = ChangeSetItem::get()->filter([ 'ObjectID' => $object->ID, - 'ObjectClass' => ClassInfo::baseDataClass($object), + 'ObjectClass' => $object->baseClass(), 'ChangeSetID' => $this->ID ])->first(); @@ -159,9 +159,17 @@ class ChangeSet extends DataObject { $this->sync(); } - protected function implicitKey($item) { - if ($item instanceof ChangeSetItem) return $item->ObjectClass.'.'.$item->ObjectID; - return ClassInfo::baseDataClass($item).'.'.$item->ID; + /** + * Build identifying string key for this object + * + * @param DataObject $item + * @return string + */ + protected function implicitKey(DataObject $item) { + if ($item instanceof ChangeSetItem) { + return $item->ObjectClass.'.'.$item->ObjectID; + } + return $item->baseClass().'.'.$item->ID; } protected function calculateImplicit() { @@ -174,16 +182,18 @@ class ChangeSet extends DataObject { /** @var string[][] $references List of which explicit items reference each thing in referenced */ $references = array(); + /** @var ChangeSetItem $item */ foreach ($this->Changes()->filter(['Added' => ChangeSetItem::EXPLICITLY]) as $item) { $explicitKey = $this->implicitKey($item); $explicit[$explicitKey] = true; foreach ($item->findReferenced() as $referee) { + /** @var DataObject $referee */ $key = $this->implicitKey($referee); $referenced[$key] = [ 'ObjectID' => $referee->ID, - 'ObjectClass' => ClassInfo::baseDataClass($referee) + 'ObjectClass' => $referee->baseClass(), ]; $references[$key][] = $item->ID; @@ -220,6 +230,7 @@ class ChangeSet extends DataObject { $implicit = $this->calculateImplicit(); // Adjust the existing implicit ChangeSetItems for this ChangeSet + /** @var ChangeSetItem $item */ foreach ($this->Changes()->filter(['Added' => ChangeSetItem::IMPLICITLY]) as $item) { $objectKey = $this->implicitKey($item); diff --git a/model/versioning/ChangeSetItem.php b/model/versioning/ChangeSetItem.php index 93dbe8d5b..b3a47aecd 100644 --- a/model/versioning/ChangeSetItem.php +++ b/model/versioning/ChangeSetItem.php @@ -67,7 +67,7 @@ class ChangeSetItem extends DataObject implements Thumbnail { public function onBeforeWrite() { // Make sure ObjectClass refers to the base data class in the case of old or wrong code - $this->ObjectClass = ClassInfo::baseDataClass($this->ObjectClass); + $this->ObjectClass = $this->getSchema()->baseDataClass($this->ObjectClass); parent::onBeforeWrite(); } @@ -328,7 +328,7 @@ class ChangeSetItem extends DataObject implements Thumbnail { public static function get_for_object($object) { return ChangeSetItem::get()->filter([ 'ObjectID' => $object->ID, - 'ObjectClass' => ClassInfo::baseDataClass($object) + 'ObjectClass' => $object->baseClass(), ]); } @@ -342,7 +342,7 @@ class ChangeSetItem extends DataObject implements Thumbnail { public static function get_for_object_by_id($objectID, $objectClass) { return ChangeSetItem::get()->filter([ 'ObjectID' => $objectID, - 'ObjectClass' => ClassInfo::baseDataClass($objectClass) + 'ObjectClass' => static::getSchema()->baseDataClass($objectClass) ]); } diff --git a/model/versioning/VersionableExtension.php b/model/versioning/VersionableExtension.php new file mode 100644 index 000000000..5705623f9 --- /dev/null +++ b/model/versioning/VersionableExtension.php @@ -0,0 +1,24 @@ + 'suffix1', 'Extension2' => array('suffix2', 'suffix3'))); * * - * Make sure your extension has a static $enabled-property that determines if it is - * processed by Versioned. + * Your extension must implement VersionableExtension interface in order to + * apply custom tables for versioned. * * @config * @var array */ - private static $versionableExtensions = array('Translatable' => 'lang'); + private static $versionableExtensions = []; /** * Permissions necessary to view records outside of the live stage (e.g. archive / draft stage). @@ -275,7 +267,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ protected function getLastEditedForVersion($version) { // Cache key - $baseTable = ClassInfo::baseDataClass($this->owner); + $baseTable = $this->baseTable(); $id = $this->owner->ID; $key = "{$baseTable}#{$id}/{$version}"; @@ -298,7 +290,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return $date; } - + /** + * Updates query parameters of relations attached to versioned dataobjects + * + * @param array $params + */ public function updateInheritableQueryParams(&$params) { // Skip if versioned isn't set if(!isset($params['Versioned.mode'])) { @@ -306,7 +302,6 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } // Adjust query based on original selection criterea - $owner = $this->owner; switch($params['Versioned.mode']) { case 'all_versions': { // Versioned.mode === all_versions doesn't inherit very well, so default to stage @@ -350,8 +345,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return; } - $baseTable = ClassInfo::baseDataClass($dataQuery->dataClass()); - + $baseTable = $this->baseTable(); $versionedMode = $dataQuery->getQueryParam('Versioned.mode'); switch($versionedMode) { // Reading a specific stage (Stage or Live) @@ -392,7 +386,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } $tempName = 'ExclusionarySource_'.$excluding; - $excludingTable = $baseTable . ($excluding && $excluding != static::DRAFT ? "_$excluding" : ''); + $excludingTable = $this->baseTable($excluding); $query->addWhere('"'.$baseTable.'"."ID" NOT IN (SELECT "ID" FROM "'.$tempName.'")'); $query->renameTable($tempName, $excludingTable); @@ -486,6 +480,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { ]); break; } + case 'all_versions': default: { // If all versions are requested, ensure that records are sorted by this field $query->addOrderBy(sprintf('"%s_versions"."%s"', $baseTable, 'Version')); @@ -507,11 +502,26 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return bool True if this table should be versioned */ protected function isTableVersioned($table) { - if(!class_exists($table)) { + $schema = DataObject::getSchema(); + $tableClass = $schema->tableClass($table); + if(empty($tableClass)) { return false; } - $baseClass = ClassInfo::baseDataClass($this->owner); - return is_a($table, $baseClass, true); + + // Check that this class belongs to the same tree + $baseClass = $schema->baseDataClass($this->owner); + if(!is_a($tableClass, $baseClass, true)) { + return false; + } + + // Check that this isn't a derived table + // (e.g. _Live, or a many_many table) + $mainTable = $schema->tableName($tableClass); + if($mainTable !== $table) { + return false; + } + + return true; } /** @@ -527,7 +537,6 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { // metadata set on the query object. This prevents regular queries from // accidentally querying the *_versions tables. $versionedMode = $dataObject->getSourceQueryParam('Versioned.mode'); - $dataClass = ClassInfo::baseDataClass($dataQuery->dataClass()); $modesToAllowVersioning = array('all_versions', 'latest_versions', 'archive', 'version'); if( !empty($dataObject->Version) && @@ -535,36 +544,21 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { ) { // This will ensure that augmentSQL will select only the same version as the owner, // regardless of how this object was initially selected + $versionColumn = $this->owner->getSchema()->sqlColumnForField($this->owner, 'Version'); $dataQuery->where([ - "\"$dataClass\".\"Version\"" => $dataObject->Version + $versionColumn => $dataObject->Version ]); $dataQuery->setQueryParam('Versioned.mode', 'all_versions'); } } - - /** - * Called by {@link SapphireTest} when the database is reset. - * - * @todo Reduce the coupling between this and SapphireTest, somehow. - */ - public static function on_db_reset() { - // Drop all temporary tables - $db = DB::get_conn(); - foreach(static::$archive_tables as $tableName) { - if(method_exists($db, 'dropTable')) $db->dropTable($tableName); - else $db->query("DROP TABLE \"$tableName\""); - } - - // Remove references to them - static::$archive_tables = array(); - } - public function augmentDatabase() { $owner = $this->owner; - $classTable = $owner->class; + $class = get_class($owner); + $baseTable = $this->baseTable(); + $classTable = $owner->getSchema()->tableName($owner); - $isRootClass = ($owner->class == ClassInfo::baseDataClass($owner->class)); + $isRootClass = $class === $owner->baseClass(); // Build a list of suffixes whose tables need versioning $allSuffixes = array(); @@ -572,7 +566,6 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { if(count($versionableExtensions)){ foreach ($versionableExtensions as $versionableExtension => $suffixes) { if ($owner->hasExtension($versionableExtension)) { - $allSuffixes = array_merge($allSuffixes, (array)$suffixes); foreach ((array)$suffixes as $suffix) { $allSuffixes[$suffix] = $versionableExtension; } @@ -581,49 +574,56 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } // Add the default table with an empty suffix to the list (table name = class name) - array_push($allSuffixes,''); + $allSuffixes[''] = null; - foreach ($allSuffixes as $key => $suffix) { - // check that this is a valid suffix - if (!is_int($key)) continue; - - if ($suffix) $table = "{$classTable}_$suffix"; - else $table = $classTable; + foreach ($allSuffixes as $suffix => $extension) { + // Check tables for this build + if ($suffix) { + $suffixBaseTable = "{$baseTable}_{$suffix}"; + $suffixTable = "{$classTable}_{$suffix}"; + } else { + $suffixBaseTable = $baseTable; + $suffixTable = $classTable; + } $fields = DataObject::database_fields($owner->class); unset($fields['ID']); if($fields) { $options = Config::inst()->get($owner->class, 'create_table_options', Config::FIRST_SET); $indexes = $owner->databaseIndexes(); - if ($suffix && ($ext = $owner->getExtensionInstance($allSuffixes[$suffix]))) { - if (!$ext->isVersionedTable($table)) continue; - $ext->setOwner($owner); - $fields = $ext->fieldsInExtraTables($suffix); - $ext->clearOwner(); - $indexes = $fields['indexes']; - $fields = $fields['db']; + $extensionClass = $allSuffixes[$suffix]; + if ($suffix && ($extension = $owner->getExtensionInstance($extensionClass))) { + if (!$extension instanceof VersionableExtension) { + throw new LogicException( + "Extension {$extensionClass} must implement VersionableExtension" + ); + } + // Allow versionable extension to customise table fields and indexes + $extension->setOwner($owner); + if ($extension->isVersionedTable($suffixTable)) { + $extension->updateVersionableFields($suffix, $fields, $indexes); + } + $extension->clearOwner(); } - // Create tables for other stages + // Build _Live table if($this->hasStages()) { - // Extra tables for _Live, etc. - // Change unique indexes to 'index'. Versioned tables may run into unique indexing difficulties - // otherwise. - $liveTable = $this->stageTable($table, static::LIVE); - $indexes = $this->uniqueToIndex($indexes); + $liveTable = $this->stageTable($suffixTable, static::LIVE); DB::require_table($liveTable, $fields, $indexes, false, $options); } + // Build _versions table + //Unique indexes will not work on versioned tables, so we'll convert them to standard indexes: + $nonUniqueIndexes = $this->uniqueToIndex($indexes); if($isRootClass) { // Create table for all versions $versionFields = array_merge( Config::inst()->get('Versioned', 'db_for_versions_table'), (array)$fields ); - $versionIndexes = array_merge( Config::inst()->get('Versioned', 'indexes_for_versions_table'), - (array)$indexes + (array)$nonUniqueIndexes ); } else { // Create fields for any tables of subclasses @@ -634,86 +634,74 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { ), (array)$fields ); - - //Unique indexes will not work on versioned tables, so we'll convert them to standard indexes: - $indexes = $this->uniqueToIndex($indexes); $versionIndexes = array_merge( array( 'RecordID_Version' => array('type' => 'unique', 'value' => '"RecordID","Version"'), 'RecordID' => true, 'Version' => true, ), - (array)$indexes + (array)$nonUniqueIndexes ); } - if(DB::get_schema()->hasTable("{$table}_versions")) { - // Fix data that lacks the uniqueness constraint (since this was added later and - // bugs meant that the constraint was validated) - $duplications = DB::query("SELECT MIN(\"ID\") AS \"ID\", \"RecordID\", \"Version\" - FROM \"{$table}_versions\" GROUP BY \"RecordID\", \"Version\" - HAVING COUNT(*) > 1"); + // Cleanup any orphans + $this->cleanupVersionedOrphans("{$suffixBaseTable}_versions", "{$suffixTable}_versions"); - foreach($duplications as $dup) { - DB::alteration_message("Removing {$table}_versions duplicate data for " - ."{$dup['RecordID']}/{$dup['Version']}" ,"deleted"); - DB::prepared_query( - "DELETE FROM \"{$table}_versions\" WHERE \"RecordID\" = ? - AND \"Version\" = ? AND \"ID\" != ?", - array($dup['RecordID'], $dup['Version'], $dup['ID']) - ); - } - - // Remove junk which has no data in parent classes. Only needs to run the following - // when versioned data is spread over multiple tables - if(!$isRootClass && ($versionedTables = ClassInfo::dataClassesFor($table))) { - - foreach($versionedTables as $child) { - if($table === $child) break; // only need subclasses - - // Select all orphaned version records - $orphanedQuery = SQLSelect::create() - ->selectField("\"{$table}_versions\".\"ID\"") - ->setFrom("\"{$table}_versions\""); - - // If we have a parent table limit orphaned records - // to only those that exist in this - if(DB::get_schema()->hasTable("{$child}_versions")) { - $orphanedQuery - ->addLeftJoin( - "{$child}_versions", - "\"{$child}_versions\".\"RecordID\" = \"{$table}_versions\".\"RecordID\" - AND \"{$child}_versions\".\"Version\" = \"{$table}_versions\".\"Version\"" - ) - ->addWhere("\"{$child}_versions\".\"ID\" IS NULL"); - } - - $count = $orphanedQuery->count(); - if($count > 0) { - DB::alteration_message("Removing {$count} orphaned versioned records", "deleted"); - $ids = $orphanedQuery->execute()->column(); - foreach($ids as $id) { - DB::prepared_query( - "DELETE FROM \"{$table}_versions\" WHERE \"ID\" = ?", - array($id) - ); - } - } - } - } - } - - DB::require_table("{$table}_versions", $versionFields, $versionIndexes, true, $options); + // Build versions table + DB::require_table("{$suffixTable}_versions", $versionFields, $versionIndexes, true, $options); } else { - DB::dont_require_table("{$table}_versions"); + DB::dont_require_table("{$suffixTable}_versions"); if($this->hasStages()) { - $liveTable = $this->stageTable($table, static::LIVE); + $liveTable = $this->stageTable($suffixTable, static::LIVE); DB::dont_require_table($liveTable); } } } } + /** + * Cleanup orphaned records in the _versions table + * + * @param string $baseTable base table to use as authoritative source of records + * @param string $childTable Sub-table to clean orphans from + */ + protected function cleanupVersionedOrphans($baseTable, $childTable) { + // Skip if child table doesn't exist + if(!DB::get_schema()->hasTable($childTable)) { + return; + } + // Skip if tables are the same + if($childTable === $baseTable) { + return; + } + + // Select all orphaned version records + $orphanedQuery = SQLSelect::create() + ->selectField("\"{$childTable}\".\"ID\"") + ->setFrom("\"{$childTable}\""); + + // If we have a parent table limit orphaned records + // to only those that exist in this + if(DB::get_schema()->hasTable($baseTable)) { + $orphanedQuery + ->addLeftJoin( + $baseTable, + "\"{$childTable}\".\"RecordID\" = \"{$baseTable}\".\"RecordID\" + AND \"{$childTable}\".\"Version\" = \"{$baseTable}\".\"Version\"" + ) + ->addWhere("\"{$baseTable}\".\"ID\" IS NULL"); + } + + $count = $orphanedQuery->count(); + if($count > 0) { + DB::alteration_message("Removing {$count} orphaned versioned records", "deleted"); + $ids = $orphanedQuery->execute()->column(); + foreach($ids as $id) { + DB::prepared_query("DELETE FROM \"{$childTable}\" WHERE \"ID\" = ?", array($id)); + } + } + } + /** * Helper for augmentDatabase() to find unique indexes and convert them to non-unique * @@ -747,23 +735,26 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * Generates a ($table)_version DB manipulation and injects it into the current $manipulation * * @param array $manipulation Source manipulation data - * @param string $table Name of table + * @param string $class Class + * @param string $table Table Table for this class * @param int $recordID ID of record to version */ - protected function augmentWriteVersioned(&$manipulation, $table, $recordID) { - $baseDataClass = ClassInfo::baseDataClass($table); + protected function augmentWriteVersioned(&$manipulation, $class, $table, $recordID) { + $baseDataClass = DataObject::getSchema()->baseDataClass($class); + $baseDataTable = DataObject::getSchema()->tableName($baseDataClass); // Set up a new entry in (table)_versions $newManipulation = array( "command" => "insert", - "fields" => isset($manipulation[$table]['fields']) ? $manipulation[$table]['fields'] : null + "fields" => isset($manipulation[$table]['fields']) ? $manipulation[$table]['fields'] : null, + "class" => $class, ); // Add any extra, unchanged fields to the version record. - $data = DB::prepared_query("SELECT * FROM \"$table\" WHERE \"ID\" = ?", array($recordID))->record(); + $data = DB::prepared_query("SELECT * FROM \"{$table}\" WHERE \"ID\" = ?", array($recordID))->record(); if ($data) { - $fields = DataObject::database_fields($table); + $fields = DataObject::database_fields($class); if (is_array($fields)) { $data = array_intersect_key($data, $fields); @@ -784,13 +775,13 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $nextVersion = 0; if($recordID) { $nextVersion = DB::prepared_query("SELECT MAX(\"Version\") + 1 - FROM \"{$baseDataClass}_versions\" WHERE \"RecordID\" = ?", + FROM \"{$baseDataTable}_versions\" WHERE \"RecordID\" = ?", array($recordID) )->value(); } $nextVersion = $nextVersion ?: 1; - if($table === $baseDataClass) { + if($class === $baseDataClass) { // Write AuthorID for baseclass $userID = (Member::currentUser()) ? Member::currentUser()->ID : 0; $newManipulation['fields']['AuthorID'] = $userID; @@ -830,13 +821,13 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { // get Version number from base data table on write $version = null; $owner = $this->owner; - $baseDataClass = ClassInfo::baseDataClass($owner->class); - if(isset($manipulation[$baseDataClass]['fields'])) { + $baseDataTable = DataObject::getSchema()->baseDataTable($owner); + if(isset($manipulation[$baseDataTable]['fields'])) { if ($this->migratingVersion) { - $manipulation[$baseDataClass]['fields']['Version'] = $this->migratingVersion; + $manipulation[$baseDataTable]['fields']['Version'] = $this->migratingVersion; } - if (isset($manipulation[$baseDataClass]['fields']['Version'])) { - $version = $manipulation[$baseDataClass]['fields']['Version']; + if (isset($manipulation[$baseDataTable]['fields']['Version'])) { + $version = $manipulation[$baseDataTable]['fields']['Version']; } } @@ -845,7 +836,8 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { foreach($tables as $table) { // Make sure that the augmented write is being applied to a table that can be versioned - if( !$this->canBeVersioned($table) ) { + $class = isset($manipulation[$table]['class']) ? $manipulation[$table]['class'] : null; + if(!$class || !$this->canBeVersioned($class) ) { unset($manipulation[$table]); continue; } @@ -864,7 +856,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } elseif(empty($version)) { // If we haven't got a version #, then we're creating a new version. // Otherwise, we're just copying a version to another table - $this->augmentWriteVersioned($manipulation, $table, $id); + $this->augmentWriteVersioned($manipulation, $class, $table, $id); } // Remove "Version" column from subclasses of baseDataClass @@ -879,7 +871,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { // If we're editing Live, then use (table)_Live instead of (table) if($this->hasStages() && static::get_stage() === static::LIVE) { - $this->augmentWriteStaged($manipulation, $table, $id); + $this->augmentWriteStaged($manipulation, $class, $id); } } @@ -1008,9 +1000,9 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { protected function lookupReverseOwners() { // Find all classes with 'owns' config $lookup = array(); - foreach(ClassInfo::subclassesFor(DataObject::class) as $class) { + foreach(ClassInfo::subclassesFor('DataObject') as $class) { // Ensure this class is versioned - if(!Object::has_extension($class, Versioned::class)) { + if(!Object::has_extension($class, 'Versioned')) { continue; } @@ -1372,16 +1364,16 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } /** - * Determine if a table is supporting the Versioned extensions (e.g. + * Determine if a class is supporting the Versioned extensions (e.g. * $table_versions does exists). * - * @param string $table Table name + * @param string $class Class name * @return boolean */ - public function canBeVersioned($table) { - return ClassInfo::exists($table) - && is_subclass_of($table, 'DataObject') - && DataObject::has_own_table($table); + public function canBeVersioned($class) { + return ClassInfo::exists($class) + && is_subclass_of($class, 'DataObject') + && DataObject::has_own_table($class); } /** @@ -1392,14 +1384,9 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return boolean Returns false if the field isn't in the table, true otherwise */ public function hasVersionField($table) { - // Strip "_Live" from end of table - $live = static::LIVE; - if($this->hasStages() && preg_match("/^(?
.*)_{$live}$/", $table, $matches)) { - $table = $matches['table']; - } - // Base table has version field - return $table === ClassInfo::baseDataClass($table); + $class = DataObject::getSchema()->tableClass($table); + return $class === DataObject::getSchema()->baseDataClass($class); } /** @@ -1416,11 +1403,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { if ($owner->hasExtension($versionableExtension)) { $ext = $owner->getExtensionInstance($versionableExtension); $ext->setOwner($owner); - $table = $ext->extendWithSuffix($table); - $ext->clearOwner(); + $table = $ext->extendWithSuffix($table); + $ext->clearOwner(); + } } } - } return $table; } @@ -1433,12 +1420,12 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { public function latestPublished() { // Get the root data object class - this will have the version field $owner = $this->owner; - $table1 = ClassInfo::baseDataClass($owner); - $table2 = $this->stageTable($table1, static::LIVE); + $draftTable = $this->baseTable(); + $liveTable = $this->stageTable($draftTable, static::LIVE); - return DB::prepared_query("SELECT \"$table1\".\"Version\" = \"$table2\".\"Version\" FROM \"$table1\" - INNER JOIN \"$table2\" ON \"$table1\".\"ID\" = \"$table2\".\"ID\" - WHERE \"$table1\".\"ID\" = ?", + return DB::prepared_query("SELECT \"$draftTable\".\"Version\" = \"$liveTable\".\"Version\" FROM \"$draftTable\" + INNER JOIN \"$liveTable\" ON \"$draftTable\".\"ID\" = \"$liveTable\".\"ID\" + WHERE \"$draftTable\".\"ID\" = ?", array($owner->ID) )->value(); } @@ -1522,7 +1509,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $joinClass = $owner->hasManyComponent($relationship); $joinField = $owner->getRemoteJoinField($relationship, 'has_many', $polymorphic); $idField = $polymorphic ? "{$joinField}ID" : $joinField; - $joinTable = ClassInfo::table_for_object_field($joinClass, $idField); + $joinTable = DataObject::getSchema()->tableForField($joinClass, $idField); // Generate update query which will unlink disowned objects $targetTable = $this->stageTable($joinTable, $targetStage); @@ -1604,14 +1591,14 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $owner->invokeWithExtensions('onBeforeUnpublish'); - $origStage = static::get_stage(); + $origReadingMode = static::get_reading_mode(); static::set_stage(static::LIVE); // This way our ID won't be unset $clone = clone $owner; $clone->delete(); - static::set_stage($origStage); + static::set_reading_mode($origReadingMode); $owner->invokeWithExtensions('onAfterUnpublish'); return true; @@ -1688,7 +1675,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $owner = $this->owner; $owner->invokeWithExtensions('onBeforeVersionedPublish', $fromStage, $toStage, $createNewVersion); - $baseClass = ClassInfo::baseDataClass($owner->class); + $baseClass = $owner->baseClass(); /** @var Versioned|DataObject $from */ if(is_numeric($fromStage)) { @@ -1875,28 +1862,30 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { /** * Return the base table - the class that directly extends DataObject. * + * Protected so it doesn't conflict with DataObject::baseTable() + * * @param string $stage * @return string */ - public function baseTable($stage = null) { - $baseClass = ClassInfo::baseDataClass($this->owner); - return $this->stageTable($baseClass, $stage); + protected function baseTable($stage = null) { + $baseTable = $this->owner->baseTable(); + return $this->stageTable($baseTable, $stage); } /** - * Given a class and stage determine the table name. + * Given a table and stage determine the table name. * * Note: Stages this asset does not exist in will default to the draft table. * - * @param string $class + * @param string $table Main table * @param string $stage - * @return string Table name + * @return string Staged table name */ - public function stageTable($class, $stage) { + public function stageTable($table, $stage) { if($this->hasStages() && $stage === static::LIVE) { - return "{$class}_{$stage}"; + return "{$table}_{$stage}"; } - return $class; + return $table; } //-----------------------------------------------------------------------------------------------// @@ -2025,8 +2014,12 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * Set the reading stage. * * @param string $stage New reading stage. + * @throws InvalidArgumentException */ public static function set_stage($stage) { + if(!in_array($stage, [static::LIVE, static::DRAFT])) { + throw new \InvalidArgumentException("Invalid stage name \"{$stage}\""); + } static::set_reading_mode('Stage.' . $stage); } @@ -2069,8 +2062,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return int */ public static function get_versionnumber_by_stage($class, $stage, $id, $cache = true) { - $baseClass = ClassInfo::baseDataClass($class); - $stageTable = ($stage == static::DRAFT) ? $baseClass : "{$baseClass}_{$stage}"; + $baseClass = DataObject::getSchema()->baseDataClass($class); + $stageTable = DataObject::getSchema()->tableName($baseClass); + if($stage === static::LIVE) { + $stageTable .= "_{$stage}"; + } // cached call if($cache && isset(self::$cache_versionnumber[$baseClass][$stage][$id])) { @@ -2126,8 +2122,11 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $parameters = $idList; } - $baseClass = ClassInfo::baseDataClass($class); - $stageTable = ($stage == static::DRAFT) ? $baseClass : "{$baseClass}_{$stage}"; + /** @var Versioned|DataObject $singleton */ + $singleton = DataObject::singleton($class); + $baseClass = $singleton->baseClass(); + $baseTable = $singleton->baseTable(); + $stageTable = $singleton->stageTable($baseTable, $stage); $versions = DB::prepared_query("SELECT \"ID\", \"Version\" FROM \"$stageTable\" $filter", $parameters)->map(); @@ -2173,7 +2172,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { Versioned::set_reading_mode($oldMode); // Fix the version number cache (in case you go delete from stage and then check ExistsOnLive) - $baseClass = ClassInfo::baseDataClass($owner->class); + $baseClass = $owner->baseClass(); self::$cache_versionnumber[$baseClass][$stage][$owner->ID] = null; } @@ -2214,7 +2213,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { public function onAfterRollback($version) { // Find record at this version - $baseClass = ClassInfo::baseDataClass($this->owner); + $baseClass = DataObject::getSchema()->baseDataClass($this->owner); /** @var Versioned|DataObject $recordVersion */ $recordVersion = static::get_version($baseClass, $this->owner->ID, $version); @@ -2234,7 +2233,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return DataObject */ public static function get_latest_version($class, $id) { - $baseClass = ClassInfo::baseDataClass($class); + $baseClass = DataObject::getSchema()->baseDataClass($class); $list = DataList::create($baseClass) ->setDataQueryParam("Versioned.mode", "latest_versions"); @@ -2277,8 +2276,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return true; } - $baseClass = ClassInfo::baseDataClass($owner->class); - $table = $this->stageTable($baseClass, static::LIVE); + $table = $this->baseTable(static::LIVE); $result = DB::prepared_query( "SELECT COUNT(*) FROM \"{$table}\" WHERE \"{$table}\".\"ID\" = ?", array($owner->ID) @@ -2297,7 +2295,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { return false; } - $table = ClassInfo::baseDataClass($owner->class); + $table = $this->baseTable(); $result = DB::prepared_query( "SELECT COUNT(*) FROM \"{$table}\" WHERE \"{$table}\".\"ID\" = ?", array($owner->ID) @@ -2341,7 +2339,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { * @return DataObject */ public static function get_version($class, $id, $version) { - $baseClass = ClassInfo::baseDataClass($class); + $baseClass = DataObject::getSchema()->baseDataClass($class); $list = DataList::create($baseClass) ->setDataQueryParam([ "Versioned.mode" => 'version', diff --git a/search/SearchContext.php b/search/SearchContext.php index 53a119f6e..adae5b364 100644 --- a/search/SearchContext.php +++ b/search/SearchContext.php @@ -93,8 +93,11 @@ class SearchContext extends Object { */ protected function applyBaseTableFields() { $classes = ClassInfo::dataClassesFor($this->modelClass); - $fields = array("\"".ClassInfo::baseDataClass($this->modelClass).'".*'); - if($this->modelClass != $classes[0]) $fields[] = '"'.$classes[0].'".*'; + $baseTable = DataObject::getSchema()->baseDataTable($this->modelClass); + $fields = array("\"{$baseTable}\".*"); + if($this->modelClass != $classes[0]) { + $fields[] = '"'.$classes[0].'".*'; + } //$fields = array_keys($model->db()); $fields[] = '"'.$classes[0].'".\"ClassName\" AS "RecordClassName"'; return $fields; diff --git a/search/filters/FulltextFilter.php b/search/filters/FulltextFilter.php index 233a63f70..80596437d 100755 --- a/search/filters/FulltextFilter.php +++ b/search/filters/FulltextFilter.php @@ -51,6 +51,7 @@ class FulltextFilter extends SearchFilter { * MyDataObject::get()->filter('SearchFields:fulltext', 'search term') * * + * @throws Exception * @return string */ public function getDbName() { @@ -65,8 +66,11 @@ class FulltextFilter extends SearchFilter { if(preg_match('/^fulltext\s+\((.+)\)$/i', $index, $matches)) { return $this->prepareColumns($matches[1]); } else { - throw new Exception("Invalid fulltext index format for '" . $this->getName() - . "' on '" . $this->model . "'"); + throw new Exception(sprintf( + "Invalid fulltext index format for '%s' on '%s'", + $this->getName(), + $this->model + )); } } } @@ -78,13 +82,14 @@ class FulltextFilter extends SearchFilter { * Adds table identifier to the every column. * Columns must have table identifier to prevent duplicate column name error. * + * @param array $columns * @return string - */ + */ protected function prepareColumns($columns) { $cols = preg_split('/"?\s*,\s*"?/', trim($columns, '(") ')); - $class = ClassInfo::table_for_object_field($this->model, current($cols)); - $cols = array_map(function($col) use ($class) { - return sprintf('"%s"."%s"', $class, $col); + $table = DataObject::getSchema()->tableForField($this->model, current($cols)); + $cols = array_map(function($col) use ($table) { + return sprintf('"%s"."%s"', $table, $col); }, $cols); return implode(',', $cols); } diff --git a/search/filters/SearchFilter.php b/search/filters/SearchFilter.php index da5bea6ce..0dbceee9d 100644 --- a/search/filters/SearchFilter.php +++ b/search/filters/SearchFilter.php @@ -161,9 +161,10 @@ abstract class SearchFilter extends Object { */ public function getDbName() { // Special handler for "NULL" relations - if($this->name == "NULL") { + if($this->name === "NULL") { return $this->name; } + // Ensure that we're dealing with a DataObject. if (!is_subclass_of($this->model, 'DataObject')) { throw new InvalidArgumentException( @@ -171,19 +172,16 @@ abstract class SearchFilter extends Object { ); } - $candidateClass = ClassInfo::table_for_object_field( - $this->model, - $this->name - ); - - if($candidateClass == 'DataObject') { + // Find table this field belongs to + $table = DataObject::getSchema()->tableForField($this->model, $this->name); + if(!$table) { // fallback to the provided name in the event of a joined column // name (as the candidate class doesn't check joined records) $parts = explode('.', $this->fullName); return '"' . implode('"."', $parts) . '"'; } - return sprintf('"%s"."%s"', $candidateClass, $this->name); + return sprintf('"%s"."%s"', $table, $this->name); } /** diff --git a/security/Security.php b/security/Security.php index b82486540..23e5bbd75 100644 --- a/security/Security.php +++ b/security/Security.php @@ -992,33 +992,46 @@ class Security extends Controller implements TemplateGlobalProvider { */ public static function database_is_ready() { // Used for unit tests - if(self::$force_database_is_ready !== NULL) return self::$force_database_is_ready; + if(self::$force_database_is_ready !== null) { + return self::$force_database_is_ready; + } - if(self::$database_is_ready) return self::$database_is_ready; + if(self::$database_is_ready) { + return self::$database_is_ready; + } - $requiredTables = ClassInfo::dataClassesFor('Member'); - $requiredTables[] = 'Group'; - $requiredTables[] = 'Permission'; + $requiredClasses = ClassInfo::dataClassesFor('Member'); + $requiredClasses[] = 'Group'; + $requiredClasses[] = 'Permission'; - foreach($requiredTables as $table) { + foreach($requiredClasses as $class) { // Skip test classes, as not all test classes are scaffolded at once - if(is_subclass_of($table, 'TestOnly')) continue; + if(is_subclass_of($class, 'TestOnly')) { + continue; + } // if any of the tables aren't created in the database - if(!ClassInfo::hasTable($table)) return false; + $table = DataObject::getSchema()->tableName($class); + if(!ClassInfo::hasTable($table)) { + return false; + } // HACK: DataExtensions aren't applied until a class is instantiated for // the first time, so create an instance here. - singleton($table); + singleton($class); // if any of the tables don't have all fields mapped as table columns $dbFields = DB::field_list($table); - if(!$dbFields) return false; + if(!$dbFields) { + return false; + } - $objFields = DataObject::database_fields($table); + $objFields = DataObject::database_fields($class); $missingFields = array_diff_key($objFields, $dbFields); - if($missingFields) return false; + if($missingFields) { + return false; + } } self::$database_is_ready = true; diff --git a/tests/core/ClassInfoTest.php b/tests/core/ClassInfoTest.php index fd437f41e..76d36f1bb 100644 --- a/tests/core/ClassInfoTest.php +++ b/tests/core/ClassInfoTest.php @@ -8,10 +8,13 @@ class ClassInfoTest extends SapphireTest { protected $extraDataObjects = array( 'ClassInfoTest_BaseClass', + 'ClassInfoTest_BaseDataClass', 'ClassInfoTest_ChildClass', 'ClassInfoTest_GrandChildClass', - 'ClassInfoTest_BaseDataClass', + 'ClassInfoTest_HasFields', 'ClassInfoTest_NoFields', + 'ClassInfoTest_WithCustomTable', + 'ClassInfoTest_WithRelation', ); public function setUp() { @@ -26,6 +29,7 @@ class ClassInfoTest extends SapphireTest { $this->assertTrue(ClassInfo::exists('CLASSINFOTEST')); $this->assertTrue(ClassInfo::exists('stdClass')); $this->assertTrue(ClassInfo::exists('stdCLASS')); + $this->assertFalse(ClassInfo::exists('SomeNonExistantClass')); } public function testSubclassesFor() { @@ -50,12 +54,15 @@ class ClassInfoTest extends SapphireTest { ); } - public function testClassName() { + public function testClassName() + { $this->assertEquals('ClassInfoTest', ClassInfo::class_name($this)); $this->assertEquals('ClassInfoTest', ClassInfo::class_name('ClassInfoTest')); $this->assertEquals('ClassInfoTest', ClassInfo::class_name('CLaSsInfOTEsT')); + } - // This is for backwards compatiblity and will be removed in 4.0 + public function testNonClassName() { + $this->setExpectedException('ReflectionException', 'Class IAmAClassThatDoesNotExist does not exist'); $this->assertEquals('IAmAClassThatDoesNotExist', ClassInfo::class_name('IAmAClassThatDoesNotExist')); } @@ -76,21 +83,6 @@ class ClassInfoTest extends SapphireTest { ); } - /** - * @covers ClassInfo::baseDataClass() - */ - public function testBaseDataClass() { - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('ClassInfoTest_BaseClass')); - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('classinfotest_baseclass')); - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('ClassInfoTest_ChildClass')); - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('CLASSINFOTEST_CHILDCLASS')); - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('ClassInfoTest_GrandChildClass')); - $this->assertEquals('ClassInfoTest_BaseClass', ClassInfo::baseDataClass('ClassInfoTest_GRANDChildClass')); - - $this->setExpectedException('InvalidArgumentException'); - ClassInfo::baseDataClass('DataObject'); - } - /** * @covers ClassInfo::ancestry() */ @@ -125,7 +117,8 @@ class ClassInfoTest extends SapphireTest { $expect = array( 'ClassInfoTest_BaseDataClass' => 'ClassInfoTest_BaseDataClass', 'ClassInfoTest_HasFields' => 'ClassInfoTest_HasFields', - 'ClassInfoTest_WithRelation' => 'ClassInfoTest_WithRelation' + 'ClassInfoTest_WithRelation' => 'ClassInfoTest_WithRelation', + 'ClassInfoTest_WithCustomTable' => 'ClassInfoTest_WithCustomTable', ); $classes = array( @@ -152,62 +145,6 @@ class ClassInfoTest extends SapphireTest { $this->assertEquals($expect, ClassInfo::dataClassesFor(strtolower($classes[2]))); } - public function testTableForObjectField() { - $this->assertEquals('ClassInfoTest_WithRelation', - ClassInfo::table_for_object_field('ClassInfoTest_WithRelation', 'RelationID') - ); - - $this->assertEquals('ClassInfoTest_WithRelation', - ClassInfo::table_for_object_field('ClassInfoTest_withrelation', 'RelationID') - ); - - $this->assertEquals('ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('ClassInfoTest_BaseDataClass', 'Title') - ); - - $this->assertEquals('ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('ClassInfoTest_HasFields', 'Title') - ); - - $this->assertEquals('ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('ClassInfoTest_NoFields', 'Title') - ); - - $this->assertEquals('ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('classinfotest_nofields', 'Title') - ); - - $this->assertEquals('ClassInfoTest_HasFields', - ClassInfo::table_for_object_field('ClassInfoTest_HasFields', 'Description') - ); - - // existing behaviour fallback to DataObject? Should be null. - $this->assertEquals('DataObject', - ClassInfo::table_for_object_field('ClassInfoTest_BaseClass', 'Nonexist') - ); - - $this->assertNull( - ClassInfo::table_for_object_field('SomeFakeClassHere', 'Title') - ); - - $this->assertNull( - ClassInfo::table_for_object_field('Object', 'Title') - ); - - $this->assertNull( - ClassInfo::table_for_object_field(null, null) - ); - - // Test fixed fields - $this->assertEquals( - 'ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('ClassInfoTest_HasFields', 'ID') - ); - $this->assertEquals( - 'ClassInfoTest_BaseDataClass', - ClassInfo::table_for_object_field('ClassInfoTest_NoFields', 'Created') - ); - } } /** @@ -270,6 +207,13 @@ class ClassInfoTest_HasFields extends ClassInfoTest_NoFields { ); } +class ClassInfoTest_WithCustomTable extends ClassInfoTest_NoFields { + private static $table_name = 'CITWithCustomTable'; + private static $db = array( + 'Description' => 'Text' + ); +} + /** * @package framework * @subpackage tests diff --git a/tests/model/ChangeSetItemTest.php b/tests/model/ChangeSetItemTest.php index f311911c6..0d30c706d 100644 --- a/tests/model/ChangeSetItemTest.php +++ b/tests/model/ChangeSetItemTest.php @@ -28,7 +28,7 @@ class ChangeSetItemTest extends SapphireTest { $item = new ChangeSetItem([ 'ObjectID' => $object->ID, - 'ObjectClass' => ClassInfo::baseDataClass($object->ClassName) + 'ObjectClass' => $object->baseClass(), ]); $this->assertEquals( @@ -80,7 +80,7 @@ class ChangeSetItemTest extends SapphireTest { $item = new ChangeSetItem([ 'ObjectID' => $object->ID, - 'ObjectClass' => ClassInfo::baseDataClass($object) + 'ObjectClass' => $object->baseClass(), ]); $item->write(); diff --git a/tests/model/ChangeSetTest.php b/tests/model/ChangeSetTest.php index 665cba18b..76d66653d 100644 --- a/tests/model/ChangeSetTest.php +++ b/tests/model/ChangeSetTest.php @@ -150,7 +150,10 @@ class ChangeSetTest extends SapphireTest { $object = $this->objFromFixture($class, $identifier); foreach($items as $i => $item) { - if ($item->ObjectClass == ClassInfo::baseDataClass($object) && $item->ObjectID == $object->ID && $item->Added == $mode) { + if ( $item->ObjectClass == $object->baseClass() + && $item->ObjectID == $object->ID + && $item->Added == $mode + ) { unset($items[$i]); continue 2; } @@ -171,7 +174,7 @@ class ChangeSetTest extends SapphireTest { ); } } - + public function testAddObject() { $cs = new ChangeSet(); $cs->write(); diff --git a/tests/model/DataListTest.php b/tests/model/DataListTest.php index 90f9c74f5..b12422b3a 100755 --- a/tests/model/DataListTest.php +++ b/tests/model/DataListTest.php @@ -9,32 +9,10 @@ class DataListTest extends SapphireTest { // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( - // From DataObjectTest - 'DataObjectTest_Team', - 'DataObjectTest_Fixture', - 'DataObjectTest_SubTeam', - 'OtherSubclassWithSameField', - 'DataObjectTest_FieldlessTable', - 'DataObjectTest_FieldlessSubTable', - 'DataObjectTest_ValidatedObject', - 'DataObjectTest_Player', - 'DataObjectTest_TeamComment', - 'DataObjectTest_EquipmentCompany', - 'DataObjectTest_SubEquipmentCompany', - 'DataObjectTest\NamespacedClass', - 'DataObjectTest\RelationClass', - 'DataObjectTest_ExtendedTeamComment', - 'DataObjectTest_Company', - 'DataObjectTest_Staff', - 'DataObjectTest_CEO', - 'DataObjectTest_Fan', - 'DataObjectTest_Play', - 'DataObjectTest_Ploy', - 'DataObjectTest_Bogey', - 'ManyManyListTest_Product', - 'ManyManyListTest_Category', - ); + public function setUpOnce() { + $this->extraDataObjects = DataObjectTest::$extra_data_objects; + parent::setUpOnce(); + } public function testFilterDataObjectByCreatedDate() { // create an object to test with diff --git a/tests/model/DataObjectSchemaTest.php b/tests/model/DataObjectSchemaTest.php new file mode 100644 index 000000000..663559885 --- /dev/null +++ b/tests/model/DataObjectSchemaTest.php @@ -0,0 +1,354 @@ +assertEquals( + 'DataObjectSchemaTest_WithRelation', + $schema->tableName('DataObjectSchemaTest_WithRelation') + ); + $this->assertEquals( + 'DOSTWithCustomTable', + $schema->tableName('DataObjectSchemaTest_WithCustomTable') + ); + + // Namespaced tables + $this->assertEquals( + 'Namespaced\DOST\MyObject', + $schema->tableName('Namespaced\DOST\MyObject') + ); + $this->assertEquals( + 'CustomNamespacedTable', + $schema->tableName('Namespaced\DOST\MyObject_CustomTable') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NestedObject', + $schema->tableName('Namespaced\DOST\MyObject_NestedObject') + ); + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableName('Namespaced\DOST\MyObject_NamespacedTable') + ); + $this->assertEquals( + 'Custom\SubclassedTable', + $schema->tableName('Namespaced\DOST\MyObject_Namespaced_Subclass') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NoFields', + $schema->tableName('Namespaced\DOST\MyObject_NoFields') + ); + } + + /** + * Test that the class name is convertable from the table + */ + public function testClassNameForTable() { + $schema = DataObject::getSchema(); + + // Tables that aren't classes + $this->assertNull($schema->tableClass('NotARealTable')); + + + // Non-namespaced tables + $this->assertEquals( + 'DataObjectSchemaTest_WithRelation', + $schema->tableClass('DataObjectSchemaTest_WithRelation') + ); + $this->assertEquals( + 'DataObjectSchemaTest_WithCustomTable', + $schema->tableClass('DOSTWithCustomTable') + ); + + // Namespaced tables + $this->assertEquals( + 'Namespaced\DOST\MyObject', + $schema->tableClass('Namespaced\DOST\MyObject') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_CustomTable', + $schema->tableClass('CustomNamespacedTable') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NestedObject', + $schema->tableClass('Namespaced\DOST\MyObject_NestedObject') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NamespacedTable', + $schema->tableClass('Custom\NamespacedTable') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_Namespaced_Subclass', + $schema->tableClass('Custom\SubclassedTable') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NoFields', + $schema->tableClass('Namespaced\DOST\MyObject_NoFields') + ); + } + + /** + * Test non-namespaced tables + */ + public function testTableForObjectField() { + $schema = DataObject::getSchema(); + $this->assertEquals( + 'DataObjectSchemaTest_WithRelation', + $schema->tableForField('DataObjectSchemaTest_WithRelation', 'RelationID') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_WithRelation', + $schema->tableForField('DataObjectSchemaTest_withrelation', 'RelationID') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_BaseDataClass', 'Title') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_HasFields', 'Title') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_NoFields', 'Title') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_nofields', 'Title') + ); + + $this->assertEquals( + 'DataObjectSchemaTest_HasFields', + $schema->tableForField('DataObjectSchemaTest_HasFields', 'Description') + ); + + // Class and table differ for this model + $this->assertEquals( + 'DOSTWithCustomTable', + $schema->tableForField('DataObjectSchemaTest_WithCustomTable', 'Description') + ); + $this->assertEquals( + 'DataObjectSchemaTest_WithCustomTable', + $schema->classForField('DataObjectSchemaTest_WithCustomTable', 'Description') + ); + $this->assertNull( + $schema->tableForField('DataObjectSchemaTest_WithCustomTable', 'NotAField') + ); + $this->assertNull( + $schema->classForField('DataObjectSchemaTest_WithCustomTable', 'NotAField') + ); + + // Non-existant fields shouldn't match any table + $this->assertNull( + $schema->tableForField('DataObjectSchemaTest_BaseClass', 'Nonexist') + ); + + $this->assertNull( + $schema->tableForField('Object', 'Title') + ); + + // Test fixed fields + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_HasFields', 'ID') + ); + $this->assertEquals( + 'DataObjectSchemaTest_BaseDataClass', + $schema->tableForField('DataObjectSchemaTest_NoFields', 'Created') + ); + } + + /** + * Check table for fields with namespaced objects can be found + */ + public function testTableForNamespacedObjectField() { + $schema = DataObject::getSchema(); + + // MyObject + $this->assertEquals( + 'Namespaced\DOST\MyObject', + $schema->tableForField('Namespaced\DOST\MyObject', 'Title') + ); + + // MyObject_CustomTable + $this->assertEquals( + 'CustomNamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_CustomTable', 'Title') + ); + + // MyObject_NestedObject + $this->assertEquals( + 'Namespaced\DOST\MyObject', + $schema->tableForField('Namespaced\DOST\MyObject_NestedObject', 'Title') + ); + $this->assertEquals( + 'Namespaced\DOST\MyObject_NestedObject', + $schema->tableForField('Namespaced\DOST\MyObject_NestedObject', 'Content') + ); + + // MyObject_NamespacedTable + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_NamespacedTable', 'Description') + ); + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_NamespacedTable', 'OwnerID') + ); + + // MyObject_Namespaced_Subclass + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_Namespaced_Subclass', 'OwnerID') + ); + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_Namespaced_Subclass', 'Title') + ); + $this->assertEquals( + 'Custom\NamespacedTable', + $schema->tableForField('Namespaced\DOST\MyObject_Namespaced_Subclass', 'ID') + ); + $this->assertEquals( + 'Custom\SubclassedTable', + $schema->tableForField('Namespaced\DOST\MyObject_Namespaced_Subclass', 'Details') + ); + + // MyObject_NoFields + $this->assertEquals( + 'Namespaced\DOST\MyObject_NoFields', + $schema->tableForField('Namespaced\DOST\MyObject_NoFields', 'Created') + ); + } + + /** + * Test that relations join on the correct columns + */ + public function testRelationsQuery() { + $namespaced1 = $this->objFromFixture('Namespaced\DOST\MyObject_NamespacedTable', 'namespaced1'); + $nofields = $this->objFromFixture('Namespaced\DOST\MyObject_NoFields', 'nofields1'); + $subclass1 = $this->objFromFixture('Namespaced\DOST\MyObject_Namespaced_Subclass', 'subclass1'); + $customtable1 = $this->objFromFixture('Namespaced\DOST\MyObject_CustomTable', 'customtable1'); + $customtable3 = $this->objFromFixture('Namespaced\DOST\MyObject_CustomTable', 'customtable3'); + + // Check has_one / has_many + $this->assertEquals($nofields->ID, $namespaced1->Owner()->ID); + $this->assertDOSEquals([ + ['Title' => 'Namespaced 1'], + ], $nofields->Owns()); + + // Check many_many / belongs_many_many + $this->assertDOSEquals( + [ + ['Title' => 'Custom Table 1'], + ['Title' => 'Custom Table 2'], + ], + $subclass1->Children() + ); + $this->assertDOSEquals( + [ + ['Title' => 'Subclass 1', 'Details' => 'Oh, Hi!',]] + , + $customtable1->Parents() + ); + $this->assertEmpty($customtable3->Parents()->count()); + + } + + + /** + * @covers DataObjectSchema::baseDataClass() + */ + public function testBaseDataClass() { + $schema = DataObject::getSchema(); + + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_BaseClass')); + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_baseclass')); + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_ChildClass')); + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_CHILDCLASS')); + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_GrandChildClass')); + $this->assertEquals('DataObjectSchemaTest_BaseClass', $schema->baseDataClass('DataObjectSchemaTest_GRANDChildClass')); + + $this->setExpectedException('InvalidArgumentException'); + $schema->baseDataClass('DataObject'); + } +} + +class DataObjectSchemaTest_BaseClass extends DataObject implements TestOnly { + +} + +class DataObjectSchemaTest_ChildClass extends DataObjectSchemaTest_BaseClass { + +} + +class DataObjectSchemaTest_GrandChildClass extends DataObjectSchemaTest_ChildClass { + +} + +class DataObjectSchemaTest_BaseDataClass extends DataObject implements TestOnly { + + private static $db = array( + 'Title' => 'Varchar' + ); +} + + +class DataObjectSchemaTest_NoFields extends DataObjectSchemaTest_BaseDataClass { + +} + +class DataObjectSchemaTest_HasFields extends DataObjectSchemaTest_NoFields { + + private static $db = array( + 'Description' => 'Varchar' + ); +} + +class DataObjectSchemaTest_WithCustomTable extends DataObjectSchemaTest_NoFields { + private static $table_name = 'DOSTWithCustomTable'; + private static $db = array( + 'Description' => 'Text' + ); +} + +class DataObjectSchemaTest_WithRelation extends DataObjectSchemaTest_NoFields { + + private static $has_one = array( + 'Relation' => 'DataObjectSchemaTest_HasFields' + ); +} diff --git a/tests/model/DataObjectSchemaTest.yml b/tests/model/DataObjectSchemaTest.yml new file mode 100644 index 000000000..d8d7e986a --- /dev/null +++ b/tests/model/DataObjectSchemaTest.yml @@ -0,0 +1,36 @@ +Namespaced\DOST\MyObject: + object1: + Title: 'Object 1' + Description: 'Description 1' + +Namespaced\DOST\MyObject_CustomTable: + customtable1: + Title: 'Custom Table 1' + Description: 'Description A' + customtable2: + Title: 'Custom Table 2' + Description: 'Description B' + customtable3: + Title: 'Custom Table 3' + Description: 'Orphaned item' + +Namespaced\DOST\MyObject_NestedObject: + nested1: + Title: 'Nested 1' + Description: 'Nested Description' + Content: '

Hello!

' + +Namespaced\DOST\MyObject_NoFields: + nofields1: {} + +Namespaced\DOST\MyObject_NamespacedTable: + namespaced1: + Title: 'Namespaced 1' + Owner: =>Namespaced\DOST\MyObject_NoFields.nofields1 + +Namespaced\DOST\MyObject_Namespaced_Subclass: + subclass1: + Title: 'Subclass 1' + Details: 'Oh, Hi!' + Children: =>Namespaced\DOST\MyObject_CustomTable.customtable1, =>Namespaced\DOST\MyObject_CustomTable.customtable2 + diff --git a/tests/model/DataObjectSchemaTest_Namespaced.php b/tests/model/DataObjectSchemaTest_Namespaced.php new file mode 100644 index 000000000..cde4db5d7 --- /dev/null +++ b/tests/model/DataObjectSchemaTest_Namespaced.php @@ -0,0 +1,78 @@ + 'Varchar', + 'Description' => 'Text', + ]; +} + +/** + * Namespaced object with custom table + */ +class MyObject_CustomTable extends \DataObject implements \TestOnly { + private static $table_name = 'CustomNamespacedTable'; + private static $db = [ + 'Title' => 'Varchar', + 'Description' => 'Text', + ]; + + private static $belongs_many_many = [ + 'Parents' => 'Namespaced\DOST\MyObject_Namespaced_Subclass', + ]; +} + +/** + * Namespaced subclassed object + */ +class MyObject_NestedObject extends MyObject implements \TestOnly { + private static $db = [ + 'Content' => 'HTMLText', + ]; +} + +/** + * Namespaced object with custom table that itself is namespaced + */ +class MyObject_NamespacedTable extends \DataObject implements \TestOnly { + private static $table_name = 'Custom\NamespacedTable'; + private static $db = [ + 'Title' => 'Varchar', + 'Description' => 'Text', + ]; + private static $has_one = [ + 'Owner' => 'Namespaced\DOST\MyObject_NoFields', + ]; +} + +/** + * Subclass of a namespaced class + * Has a many_many to another namespaced table + */ +class MyObject_Namespaced_Subclass extends MyObject_NamespacedTable implements \TestOnly { + private static $table_name = 'Custom\SubclassedTable'; + private static $db = [ + 'Details' => 'Varchar', + ]; + private static $many_many = [ + 'Children' => 'Namespaced\DOST\MyObject_CustomTable', + ]; +} + +/** + * Namespaced class without any fields + * has a has_many to another namespaced table + */ +class MyObject_NoFields extends \DataObject implements \TestOnly { + private static $has_many = [ + 'Owns' => 'Namespaced\DOST\MyObject_NamespacedTable', + ]; +} diff --git a/tests/model/DataObjectTest.php b/tests/model/DataObjectTest.php index 3baa2df86..23756dac1 100644 --- a/tests/model/DataObjectTest.php +++ b/tests/model/DataObjectTest.php @@ -10,7 +10,12 @@ class DataObjectTest extends SapphireTest { protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( + /** + * Standard set of dataobject test classes + * + * @var array + */ + public static $extra_data_objects = array( 'DataObjectTest_Team', 'DataObjectTest_Fixture', 'DataObjectTest_SubTeam', @@ -32,10 +37,17 @@ class DataObjectTest extends SapphireTest { 'DataObjectTest_Play', 'DataObjectTest_Ploy', 'DataObjectTest_Bogey', + // From ManyManyListTest + 'ManyManyListTest_ExtraFields', 'ManyManyListTest_Product', 'ManyManyListTest_Category', ); + public function setUpOnce() { + $this->extraDataObjects = static::$extra_data_objects; + parent::setUpOnce(); + } + public function testDb() { $obj = new DataObjectTest_TeamComment(); $dbFields = $obj->db(); diff --git a/tests/model/DataQueryTest.php b/tests/model/DataQueryTest.php index 958f9c8f9..4d4258f22 100644 --- a/tests/model/DataQueryTest.php +++ b/tests/model/DataQueryTest.php @@ -75,7 +75,7 @@ class DataQueryTest extends SapphireTest { //test many_many with separate inheritance $newDQ = new DataQuery('DataQueryTest_C'); - $baseDBTable = ClassInfo::baseDataClass('DataQueryTest_C'); + $baseDBTable = DataObject::getSchema()->baseDataTable('DataQueryTest_C'); $newDQ->applyRelation('ManyTestAs'); //check we are "joined" to the DataObject's table (there is no distinction between FROM or JOIN clauses) $this->assertTrue($newDQ->query()->isJoinedTo($baseDBTable)); @@ -84,7 +84,7 @@ class DataQueryTest extends SapphireTest { //test many_many with shared inheritance $newDQ = new DataQuery('DataQueryTest_E'); - $baseDBTable = ClassInfo::baseDataClass('DataQueryTest_E'); + $baseDBTable = DataObject::getSchema()->baseDataTable('DataQueryTest_E'); //check we are "joined" to the DataObject's table (there is no distinction between FROM or JOIN clauses) $this->assertTrue($newDQ->query()->isJoinedTo($baseDBTable)); //check we are explicitly selecting "FROM" the DO's table diff --git a/tests/model/HasManyListTest.php b/tests/model/HasManyListTest.php index 06f3a49df..137dc05d2 100644 --- a/tests/model/HasManyListTest.php +++ b/tests/model/HasManyListTest.php @@ -5,32 +5,10 @@ class HasManyListTest extends SapphireTest { // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( - // From DataObjectTest - 'DataObjectTest_Team', - 'DataObjectTest_Fixture', - 'DataObjectTest_SubTeam', - 'OtherSubclassWithSameField', - 'DataObjectTest_FieldlessTable', - 'DataObjectTest_FieldlessSubTable', - 'DataObjectTest_ValidatedObject', - 'DataObjectTest_Player', - 'DataObjectTest_TeamComment', - 'DataObjectTest_EquipmentCompany', - 'DataObjectTest_SubEquipmentCompany', - 'DataObjectTest\NamespacedClass', - 'DataObjectTest\RelationClass', - 'DataObjectTest_ExtendedTeamComment', - 'DataObjectTest_Company', - 'DataObjectTest_Staff', - 'DataObjectTest_CEO', - 'DataObjectTest_Fan', - 'DataObjectTest_Play', - 'DataObjectTest_Ploy', - 'DataObjectTest_Bogey', - 'ManyManyListTest_Product', - 'ManyManyListTest_Category', - ); + public function setUpOnce() { + $this->extraDataObjects = DataObjectTest::$extra_data_objects; + parent::setUpOnce(); + } public function testRelationshipEmptyOnNewRecords() { // Relies on the fact that (unrelated) comments exist in the fixture file already diff --git a/tests/model/ManyManyListTest.php b/tests/model/ManyManyListTest.php index d71641dca..bd507d77f 100644 --- a/tests/model/ManyManyListTest.php +++ b/tests/model/ManyManyListTest.php @@ -10,35 +10,10 @@ class ManyManyListTest extends SapphireTest { protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( - // From DataObjectTest - 'DataObjectTest_Team', - 'DataObjectTest_Fixture', - 'DataObjectTest_SubTeam', - 'OtherSubclassWithSameField', - 'DataObjectTest_FieldlessTable', - 'DataObjectTest_FieldlessSubTable', - 'DataObjectTest_ValidatedObject', - 'DataObjectTest_Player', - 'DataObjectTest_TeamComment', - 'DataObjectTest_EquipmentCompany', - 'DataObjectTest_SubEquipmentCompany', - 'DataObjectTest\NamespacedClass', - 'DataObjectTest\RelationClass', - 'DataObjectTest_ExtendedTeamComment', - 'DataObjectTest_Company', - 'DataObjectTest_Staff', - 'DataObjectTest_CEO', - 'DataObjectTest_Fan', - 'DataObjectTest_Play', - 'DataObjectTest_Ploy', - 'DataObjectTest_Bogey', - // From ManyManyListTest - 'ManyManyListTest_ExtraFields', - 'ManyManyListTest_Product', - 'ManyManyListTest_Category', - ); - + public function setUpOnce() { + $this->extraDataObjects = DataObjectTest::$extra_data_objects; + parent::setUpOnce(); + } public function testAddCompositedExtraFields() { $obj = new ManyManyListTest_ExtraFields(); diff --git a/tests/model/MapTest.php b/tests/model/MapTest.php index 2f229c0fe..494326fa1 100755 --- a/tests/model/MapTest.php +++ b/tests/model/MapTest.php @@ -5,37 +5,15 @@ * @subpackage tests */ class SS_MapTest extends SapphireTest { - + // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( - // From DataObjectTest - 'DataObjectTest_Team', - 'DataObjectTest_Fixture', - 'DataObjectTest_SubTeam', - 'OtherSubclassWithSameField', - 'DataObjectTest_FieldlessTable', - 'DataObjectTest_FieldlessSubTable', - 'DataObjectTest_ValidatedObject', - 'DataObjectTest_Player', - 'DataObjectTest_TeamComment', - 'DataObjectTest_EquipmentCompany', - 'DataObjectTest_SubEquipmentCompany', - 'DataObjectTest\NamespacedClass', - 'DataObjectTest\RelationClass', - 'DataObjectTest_ExtendedTeamComment', - 'DataObjectTest_Company', - 'DataObjectTest_Staff', - 'DataObjectTest_CEO', - 'DataObjectTest_Fan', - 'DataObjectTest_Play', - 'DataObjectTest_Ploy', - 'DataObjectTest_Bogey', - 'ManyManyListTest_Product', - 'ManyManyListTest_Category', - ); - + public function setUpOnce() { + $this->extraDataObjects = DataObjectTest::$extra_data_objects; + parent::setUpOnce(); + } + public function testValues() { $list = DataObjectTest_TeamComment::get()->sort('Name'); @@ -91,13 +69,13 @@ class SS_MapTest extends SapphireTest { . "Bob: This is a team comment by Bob\n" . "Phil: Phil is a unique guy, and comments on team2\n", $text); } - + public function testDefaultConfigIsIDAndTitle() { $list = DataObjectTest_Team::get(); $map = new SS_Map($list); $this->assertEquals('Team 1', $map[$this->idFromFixture('DataObjectTest_Team', 'team1')]); } - + public function testSetKeyFieldAndValueField() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list); @@ -105,7 +83,7 @@ class SS_MapTest extends SapphireTest { $map->setValueField('Comment'); $this->assertEquals('This is a team comment by Joe', $map['Joe']); } - + public function testToArray() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list, 'Name', 'Comment'); @@ -183,10 +161,10 @@ class SS_MapTest extends SapphireTest { "Phil" => "Phil is a unique guy, and comments on team2"), $map->toArray()); $map->unshift(0, '(Select)'); - + $this->assertEquals('(All)', $map[-1]); $this->assertEquals('(Select)', $map[0]); - + $this->assertEquals(array( 0 => "(Select)", -1 => "(All)", @@ -232,7 +210,7 @@ class SS_MapTest extends SapphireTest { 1 => "(All)" ), $map->toArray()); } - + public function testCount() { $list = DataObjectTest_TeamComment::get(); $map = new SS_Map($list, 'Name', 'Comment'); @@ -277,7 +255,7 @@ class SS_MapTest extends SapphireTest { $list = DataObjectTest_TeamComment::get()->sort('ID'); $map = new SS_Map($list, 'Name', 'Comment'); $map->push(1, 'Pushed'); - + $text = ""; foreach($map as $k => $v) { @@ -300,7 +278,7 @@ class SS_MapTest extends SapphireTest { foreach($map as $k => $v) { $text .= "$k: $v\n"; } - + $this->assertEquals("1: unshifted\n", $text); } @@ -313,7 +291,7 @@ class SS_MapTest extends SapphireTest { foreach($map as $k => $v) { $text .= "$k: $v\n"; } - + $this->assertEquals("1: pushed\n", $text); } } diff --git a/tests/model/PolymorphicHasManyListTest.php b/tests/model/PolymorphicHasManyListTest.php index f31316c3b..1646bce13 100644 --- a/tests/model/PolymorphicHasManyListTest.php +++ b/tests/model/PolymorphicHasManyListTest.php @@ -15,30 +15,10 @@ class PolymorphicHasManyListTest extends SapphireTest { // Borrow the model from DataObjectTest protected static $fixture_file = 'DataObjectTest.yml'; - protected $extraDataObjects = array( - // From DataObjectTest - 'DataObjectTest_Team', - 'DataObjectTest_Fixture', - 'DataObjectTest_SubTeam', - 'OtherSubclassWithSameField', - 'DataObjectTest_FieldlessTable', - 'DataObjectTest_FieldlessSubTable', - 'DataObjectTest_ValidatedObject', - 'DataObjectTest_Player', - 'DataObjectTest_TeamComment', - 'DataObjectTest_EquipmentCompany', - 'DataObjectTest_SubEquipmentCompany', - 'DataObjectTest\NamespacedClass', - 'DataObjectTest\RelationClass', - 'DataObjectTest_ExtendedTeamComment', - 'DataObjectTest_Company', - 'DataObjectTest_Staff', - 'DataObjectTest_CEO', - 'DataObjectTest_Fan', - 'DataObjectTest_Play', - 'DataObjectTest_Ploy', - 'DataObjectTest_Bogey', - ); + public function setUpOnce() { + $this->extraDataObjects = DataObjectTest::$extra_data_objects; + parent::setUpOnce(); + } public function testRelationshipEmptyOnNewRecords() { // Relies on the fact that (unrelated) comments exist in the fixture file already diff --git a/tests/model/VersionableExtensionsTest.php b/tests/model/VersionableExtensionsTest.php index e08209c52..c53d5c29f 100644 --- a/tests/model/VersionableExtensionsTest.php +++ b/tests/model/VersionableExtensionsTest.php @@ -68,30 +68,24 @@ class VersionableExtensionsTest_DataObject extends DataObject implements TestOnl } -class VersionableExtensionsTest_Extension extends DataExtension implements TestOnly { +class VersionableExtensionsTest_Extension extends DataExtension implements VersionableExtension, TestOnly { - public function isVersionedTable($table){ + public function isVersionedTable($table) { return true; } /** - * fieldsInExtraTables function. + * Update fields and indexes for the versonable suffix table * - * @access public - * @param mixed $suffix + * @param string $suffix Table suffix being built + * @param array $fields List of fields in this model + * @param array $indexes List of indexes in this model * @return array */ - public function fieldsInExtraTables($suffix){ - $fields = array(); - //$fields['db'] = DataObject::database_fields($this->owner->class); - $fields['indexes'] = $this->owner->databaseIndexes(); - - $fields['db'] = array_merge( - DataObject::database_fields($this->owner->class) - ); - - return $fields; + public function updateVersionableFields($suffix, &$fields, &$indexes){ + $indexes['ExtraField'] = true; + $fields['ExtraField'] = 'Varchar()'; } } diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index fd0601399..cc7d4a65f 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -33,7 +33,7 @@ class VersionedTest extends SapphireTest { 'VersionedTest_WithIndexes_versions' => array('value' => false, 'message' => 'Unique indexes are no longer unique in _versions table'), 'VersionedTest_WithIndexes_Live' => - array('value' => false, 'message' => 'Unique indexes are no longer unique in _Live table'), + array('value' => true, 'message' => 'Unique indexes are unique in _Live table'), ); // Test each table's performance @@ -56,7 +56,7 @@ class VersionedTest extends SapphireTest { if (in_array($indexSpec['value'], $expectedColumns)) { $isUnique = $indexSpec['type'] === 'unique'; $this->assertEquals($isUnique, $expectation['value'], $expectation['message']); -} + } } } } @@ -314,7 +314,7 @@ class VersionedTest extends SapphireTest { } public function testWritingNewToStage() { - $origStage = Versioned::get_stage(); + $origReadingMode = Versioned::get_reading_mode(); Versioned::set_stage(Versioned::DRAFT); $page = new VersionedTest_DataObject(); @@ -333,7 +333,7 @@ class VersionedTest extends SapphireTest { $this->assertEquals(1, $stage->count()); $this->assertEquals($stage->First()->Title, 'testWritingNewToStage'); - Versioned::set_stage($origStage); + Versioned::set_reading_mode($origReadingMode); } /** @@ -343,7 +343,7 @@ class VersionedTest extends SapphireTest { * the VersionedTest_DataObject record though. */ public function testWritingNewToLive() { - $origStage = Versioned::get_stage(); + $origReadingMode = Versioned::get_reading_mode(); Versioned::set_stage(Versioned::LIVE); $page = new VersionedTest_DataObject(); @@ -362,7 +362,7 @@ class VersionedTest extends SapphireTest { )); $this->assertEquals(0, $stage->count()); - Versioned::set_stage($origStage); + Versioned::set_reading_mode($origReadingMode); } /** diff --git a/tests/view/SSViewerCacheBlockTest.php b/tests/view/SSViewerCacheBlockTest.php index 2b916061f..7d5fff8ed 100644 --- a/tests/view/SSViewerCacheBlockTest.php +++ b/tests/view/SSViewerCacheBlockTest.php @@ -144,8 +144,7 @@ class SSViewerCacheBlockTest extends SapphireTest { } public function testVersionedCache() { - - $origStage = Versioned::get_stage(); + $origReadingMode = Versioned::get_reading_mode(); // Run without caching in stage to prove data is uncached $this->_reset(false); @@ -211,7 +210,7 @@ class SSViewerCacheBlockTest extends SapphireTest { $this->_runtemplate('<% cached %>$Inspect<% end_cached %>', $data) ); - Versioned::set_stage($origStage); + Versioned::set_reading_mode($origReadingMode); } /**