From aa2c71424dbd4937abdcda63c37fca1ef56d3a51 Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 30 Jan 2018 18:28:28 +1300 Subject: [PATCH] API Implement cascade_duplications API Add DataObject::setComponent() API Support unary components as getter and setter fields API ManyManyList::add() now supports unsaved records ENHANCEMENT Animal farm --- .../00_Model/02_Relations.md | 39 ++ src/Forms/FileUploadReceiver.php | 10 +- src/ORM/DataList.php | 1 + src/ORM/DataObject.php | 347 +++++++++++++++--- src/ORM/DataObjectSchema.php | 13 + src/ORM/ManyManyList.php | 6 +- src/ORM/UnsavedRelationList.php | 4 - tests/php/ORM/DataObjectDuplicationTest.php | 277 ++++---------- tests/php/ORM/DataObjectDuplicationTest.yml | 42 +++ .../DataObjectDuplicationTest/Antelope.php | 46 +++ .../ORM/DataObjectDuplicationTest/Bobcat.php | 30 ++ .../ORM/DataObjectDuplicationTest/Caribou.php | 23 ++ .../ORM/DataObjectDuplicationTest/Class1.php | 23 -- .../ORM/DataObjectDuplicationTest/Class2.php | 19 - .../ORM/DataObjectDuplicationTest/Class3.php | 19 - .../{Class4.php => Dingo.php} | 12 +- .../DataObjectDuplicationTest/Elephant.php | 29 ++ .../ORM/DataObjectDuplicationTest/Frog.php | 25 ++ .../ORM/DataObjectDuplicationTest/Goat.php | 23 ++ 19 files changed, 661 insertions(+), 327 deletions(-) create mode 100644 tests/php/ORM/DataObjectDuplicationTest.yml create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Antelope.php create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Bobcat.php create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Caribou.php delete mode 100644 tests/php/ORM/DataObjectDuplicationTest/Class1.php delete mode 100644 tests/php/ORM/DataObjectDuplicationTest/Class2.php delete mode 100644 tests/php/ORM/DataObjectDuplicationTest/Class3.php rename tests/php/ORM/DataObjectDuplicationTest/{Class4.php => Dingo.php} (61%) create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Elephant.php create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Frog.php create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Goat.php diff --git a/docs/en/02_Developer_Guides/00_Model/02_Relations.md b/docs/en/02_Developer_Guides/00_Model/02_Relations.md index c6933e2a7..2a9f2da4f 100644 --- a/docs/en/02_Developer_Guides/00_Model/02_Relations.md +++ b/docs/en/02_Developer_Guides/00_Model/02_Relations.md @@ -420,6 +420,45 @@ If your object is versioned, cascade_deletes will also act as "cascade unpublish on a parent object will trigger unpublish on the child, similarly to how `owns` causes triggered publishing. See the [versioning docs](/developer_guides/versioning) for more information on ownership. +## Cascading duplications + +Similar to `cascade_deletes` there is also a `cascade_duplicates` config which works in much the same way. +When you invoke `$dataObject->duplicate()`, relation names specified by this config will be duplicated +and saved against the new clone object. + +Note that duplications will act differently depending on the kind of relation: + - Exclusive relationships (e.g. has_many, belongs_to) will be explicitly duplicated. + - Non-exclusive many_many will not be duplicated, but the mapping table values will instead + be copied for this record. + - Non-exclusive has_one relationships are not normally necessary to duplicate, since both parent and clone + can normally share the same relation ID. However, if this is declared in `cascade_duplicates` any + has one will be similarly duplicated as though it were an exclusive relationship. + +For example: + +```php +use SilverStripe\ORM\DataObject; + +class ParentObject extends DataObject { + private static $many_many = [ + 'RelatedChildren' => ChildObject::class, + ]; + private static $cascade_duplicates = [ 'RelatedChildren' ]; +} +class ChildObject extends DataObject { +} +``` + +When duplicating objects you can disable recursive duplication by passing in `false` to the second +argument of duplicate(). + +E.g. + +```php +$parent = ParentObject::get()->first(); +$dupe = $parent->duplicate(true, false); +``` + ## Adding relations Adding new items to a relations works the same, regardless if you're editing a **has_many** or a **many_many**. They are diff --git a/src/Forms/FileUploadReceiver.php b/src/Forms/FileUploadReceiver.php index 2a7839467..6a7c5688a 100644 --- a/src/Forms/FileUploadReceiver.php +++ b/src/Forms/FileUploadReceiver.php @@ -139,7 +139,13 @@ trait FileUploadReceiver $items = new ArrayList(); // Determine format of presented data - if (empty($value) && $record) { + if ($value instanceof File) { + $items = ArrayList::create([$value]); + $value = null; + } elseif ($value instanceof SS_List) { + $items = $value; + $value = null; + } elseif (empty($value) && $record) { // If a record is given as a second parameter, but no submitted values, // then we should inspect this instead for the form values @@ -158,7 +164,7 @@ trait FileUploadReceiver // If directly passing a list then save the items directly $items = $record; } - } elseif (!empty($value['Files'])) { + } elseif (is_array($value) && !empty($value['Files'])) { // If value is given as an array (such as a posted form), extract File IDs from this $class = $this->getRelationAutosetClass(); $items = DataObject::get($class)->byIDs($value['Files']); diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index 570ecaef0..4766db9f8 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -1088,6 +1088,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li * list manipulation * * @param mixed $item + * @param array|null $extraFields Any extra fields, if supported by this list */ public function add($item) { diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 420ad6dea..30d2c426d 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -259,7 +259,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * * @var DataObject[] */ - protected $components; + protected $components = []; /** * Non-static cache of has_many and many_many relations that can't be written until this object is saved. @@ -279,6 +279,18 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ private static $cascade_deletes = []; + /** + * List of relations that should be cascade duplicate. + * many_many duplications are shallow only. + * + * Note: If duplicating a many_many through you should refer to the + * has_many intermediary relation instead, otherwise extra fields + * will be omitted from the duplicated relation. + * + * @var array + */ + private static $cascade_duplicates = []; + /** * Get schema object * @@ -292,7 +304,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity /** * Construct a new DataObject. * - * @param array|null $record Used internally for rehydrating an object from database content. * Bypasses setters on this class, and hence should not be used * for populating data on new records. @@ -396,40 +407,105 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * * @param bool $doWrite Perform a write() operation before returning the object. * If this is true, it will create the duplicate in the database. - * @param bool|string $manyMany Which many-many to duplicate. Set to true to duplicate all, false to duplicate none. - * Alternatively set to the string of the relation config to duplicate - * (supports 'many_many', or 'belongs_many_many') + * @param array|null|false $relations List of relations to duplicate. + * Will default to `cascade_duplicates` if null. + * Set to 'false' to force none. + * Set to specific array of names to duplicate to override these. + * Note: If using versioned, this will additionally failover to `owns` config. * @return static A duplicate of this node. The exact type will be the type of this node. */ - public function duplicate($doWrite = true, $manyMany = 'many_many') + public function duplicate($doWrite = true, $relations = null) { + // Handle legacy behaviour + if (is_string($relations) || $relations === true) { + if ($relations === true) { + $relations = 'many_many'; + } + Deprecation::notice('5.0', 'Use cascade_duplicates config instead of providing a string to duplicate()'); + $relations = array_keys($this->config()->get($relations)) ?: []; + } + + // Get duplicates + if ($relations === null) { + $relations = $this->config()->get('cascade_duplicates'); + } + + // Create unsaved raw duplicate $map = $this->toMap(); unset($map['Created']); /** @var static $clone */ $clone = Injector::inst()->create(static::class, $map, false, $this->getSourceQueryParams()); $clone->ID = 0; - $clone->invokeWithExtensions('onBeforeDuplicate', $this, $doWrite, $manyMany); - if ($manyMany) { - $this->duplicateManyManyRelations($this, $clone, $manyMany); + // Note: Extensions such as versioned may update $relations here + $clone->invokeWithExtensions('onBeforeDuplicate', $this, $doWrite, $relations); + if ($relations) { + $this->duplicateRelations($this, $clone, $relations); } if ($doWrite) { $clone->write(); } - $clone->invokeWithExtensions('onAfterDuplicate', $this, $doWrite, $manyMany); + $clone->invokeWithExtensions('onAfterDuplicate', $this, $doWrite, $relations); return $clone; } + /** + * Copies the given relations from this object to the destination + * + * @param DataObject $sourceObject the source object to duplicate from + * @param DataObject $destinationObject the destination object to populate with the duplicated relations + * @param array $relations List of relations + */ + protected function duplicateRelations($sourceObject, $destinationObject, $relations) + { + // Get list of duplicable relation types + $manyMany = $sourceObject->manyMany(); + $hasMany = $sourceObject->hasMany(); + $hasOne = $sourceObject->hasOne(); + $belongsTo = $sourceObject->belongsTo(); + + // Duplicate each relation based on type + foreach ($relations as $relation) { + switch (true) { + case array_key_exists($relation, $manyMany): { + $this->duplicateManyManyRelation($sourceObject, $destinationObject, $relation); + break; + } + case array_key_exists($relation, $hasMany): { + $this->duplicateHasManyRelation($sourceObject, $destinationObject, $relation); + break; + } + case array_key_exists($relation, $hasOne): { + $this->duplicateHasOneRelation($sourceObject, $destinationObject, $relation); + break; + } + case array_key_exists($relation, $belongsTo): { + $this->duplicateBelongsToRelation($sourceObject, $destinationObject, $relation); + break; + } + default: { + $sourceType = get_class($sourceObject); + throw new InvalidArgumentException( + "Cannot duplicate unknown relation {$relation} on parent type {$sourceType}" + ); + } + } + } + } + /** * Copies the many_many and belongs_many_many relations from one object to another instance of the name of object. * + * @deprecated 4.1...5.0 Use duplicateRelations() instead * @param DataObject $sourceObject the source object to duplicate from * @param DataObject $destinationObject the destination object to populate with the duplicated relations * @param bool|string $filter */ protected function duplicateManyManyRelations($sourceObject, $destinationObject, $filter) { + Deprecation::notice('5.0', 'Use duplicateRelations() instead'); + // Get list of relations to duplicate if ($filter === 'many_many' || $filter === 'belongs_many_many') { $relations = $sourceObject->config()->get($filter); @@ -444,25 +520,93 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Duplicates a single many_many relation from one object to another + * Duplicates a single many_many relation from one object to another. * * @param DataObject $sourceObject * @param DataObject $destinationObject - * @param string $manyManyName + * @param string $relation */ - protected function duplicateManyManyRelation($sourceObject, $destinationObject, $manyManyName) + protected function duplicateManyManyRelation($sourceObject, $destinationObject, $relation) { - // Ensure this component exists on the destination side as well - if (!static::getSchema()->manyManyComponent(get_class($destinationObject), $manyManyName)) { + // Copy all components from source to destination + $source = $sourceObject->getManyManyComponents($relation); + $dest = $destinationObject->getManyManyComponents($relation); + $extraFieldNames = $source->getExtraFields(); + foreach ($source as $item) { + // Merge extra fields + $extraFields = []; + foreach ($extraFieldNames as $fieldName => $fieldType) { + $extraFields[$fieldName] = $item->getField($fieldName); + } + $dest->add($item, $extraFields); + } + } + + /** + * Duplicates a single many_many relation from one object to another. + * + * @param DataObject $sourceObject + * @param DataObject $destinationObject + * @param string $relation + */ + protected function duplicateHasManyRelation($sourceObject, $destinationObject, $relation) + { + // Copy all components from source to destination + $source = $sourceObject->getComponents($relation); + $dest = $destinationObject->getComponents($relation); + + /** @var DataObject $item */ + foreach ($source as $item) { + // Don't write on duplicate; Wait until ParentID is available later. + // writeRelations() will eventually write these records when converting + // from UnsavedRelationList + $clonedItem = $item->duplicate(false); + $dest->add($clonedItem); + } + } + + /** + * Duplicates a single has_one relation from one object to another. + * Note: Child object will be force written. + * + * @param DataObject $sourceObject + * @param DataObject $destinationObject + * @param string $relation + */ + protected function duplicateHasOneRelation($sourceObject, $destinationObject, $relation) + { + // Check if original object exists + $item = $sourceObject->getComponent($relation); + if (!$item->isInDB()) { return; } - // Copy all components from source to destination - $source = $sourceObject->getManyManyComponents($manyManyName); - $dest = $destinationObject->getManyManyComponents($manyManyName); - foreach ($source as $item) { - $dest->add($item); + $clonedItem = $item->duplicate(false); + $destinationObject->setComponent($relation, $clonedItem); + } + + /** + * Duplicates a single belongs_to relation from one object to another. + * Note: This will force a write on both parent / child objects. + * + * @param DataObject $sourceObject + * @param DataObject $destinationObject + * @param string $relation + */ + protected function duplicateBelongsToRelation($sourceObject, $destinationObject, $relation) + { + // Check if original object exists + $item = $sourceObject->getComponent($relation); + if (!$item->isInDB()) { + return; } + + $clonedItem = $item->duplicate(false); + $destinationObject->setComponent($relation, $clonedItem); + // After $clonedItem is assigned the appropriate FieldID / FieldClass, force write + // @todo Write this component in onAfterWrite instead, assigning the FieldID then + // https://github.com/silverstripe/silverstripe-framework/issues/7818 + $clonedItem->write(); } /** @@ -561,7 +705,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity parent::defineMethods(); if (static::class === self::class) { - return; + return; } // Set up accessors for joined items @@ -639,7 +783,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return i18n::_t( static::class . '.PLURALS', $default, - [ 'count' => $count ] + ['count' => $count] ); } @@ -808,7 +952,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $parentObj = $relObj; $relObj = $relObj->$relation(); // If the intermediate relationship objects haven't been created, then write them - if ($iID || (!$relObj->ID && $parentObj !== $this)) { + if ($i < sizeof($relations) - 1 && !$relObj->ID || (!$relObj->ID && $parentObj !== $this)) { $relObj->write(); $relatedFieldName = $relation . "ID"; $parentObj->$relatedFieldName = $relObj->ID; @@ -1122,11 +1266,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if ($defaults) { foreach ($defaults as $fieldName => $fieldValue) { - // SRM 2007-03-06: Stricter check + // SRM 2007-03-06: Stricter check if (!isset($this->$fieldName) || $this->$fieldName === null) { $this->$fieldName = $fieldValue; } - // Set many-many defaults with an array of ids + // Set many-many defaults with an array of ids if (is_array($fieldValue) && $this->getSchema()->manyManyComponent(static::class, $fieldName)) { /** @var ManyManyList $manyManyJoin */ $manyManyJoin = $this->$fieldName(); @@ -1232,7 +1376,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } // Ensure this field pertains to this table - $specification = $schema->fieldSpec($class, $fieldName, DataObjectSchema::DB_ONLY | DataObjectSchema::UNINHERITED); + $specification = $schema->fieldSpec( + $class, + $fieldName, + DataObjectSchema::DB_ONLY | DataObjectSchema::UNINHERITED + ); if (!$specification) { continue; } @@ -1251,10 +1399,9 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if ($baseTable === $table) { $manipulation[$table]['fields']['LastEdited'] = $now; if ($isNewRecord) { - $manipulation[$table]['fields']['Created'] - = empty($this->record['Created']) - ? $now - : $this->record['Created']; + $manipulation[$table]['fields']['Created'] = empty($this->record['Created']) + ? $now + : $this->record['Created']; $manipulation[$table]['fields']['ClassName'] = static::class; } } @@ -1325,7 +1472,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * - $this->onBeforeWrite() gets called beforehand. * - Extensions such as Versioned will ammend the database-write to ensure that a version is saved. * - * @uses DataExtension->augmentWrite() + * @uses DataExtension->augmentWrite() * * @param boolean $showDebug Show debugging information * @param boolean $forceInsert Run INSERT command rather than UPDATE, even if record already exists @@ -1414,10 +1561,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity */ public function writeComponents($recursive = false) { - if ($this->components) { - foreach ($this->components as $component) { - $component->write(false, false, false, $recursive); - } + foreach ($this->components as $component) { + $component->write(false, false, false, $recursive); } if ($join = $this->getJoin()) { @@ -1431,7 +1576,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Delete this data object. * $this->onBeforeDelete() gets called. * Note that in Versioned objects, both Stage and Live will be deleted. - * @uses DataExtension->augmentSQL() + * @uses DataExtension->augmentSQL() */ public function delete() { @@ -1501,7 +1646,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Return a component object from a one to one relationship, as a DataObject. + * Return a unary component object from a one to one relationship, as a DataObject. * If no component is available, an 'empty component' will be returned for * non-polymorphic relations, or for polymorphic relations with a class set. * @@ -1518,7 +1663,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $schema = static::getSchema(); if ($class = $schema->hasOneComponent(static::class, $componentName)) { $joinField = $componentName . 'ID'; - $joinID = $this->getField($joinField); + $joinID = $this->getField($joinField); // Extract class name for polymorphic relations if ($class === self::class) { @@ -1582,6 +1727,52 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity return $component; } + /** + * Assign an item to the given component + * + * @param string $componentName + * @param DataObject|null $item + * @return $this + */ + public function setComponent($componentName, $item) + { + // Validate component + $schema = static::getSchema(); + if ($class = $schema->hasOneComponent(static::class, $componentName)) { + // Force item to be written if not by this point + // @todo This could be lazy-written in a beforeWrite hook, but force write here for simplicity + // https://github.com/silverstripe/silverstripe-framework/issues/7818 + if ($item && !$item->isInDB()) { + $item->write(); + } + + // Update local ID + $joinField = $componentName . 'ID'; + $this->setField($joinField, $item ? $item->ID : null); + // Update Class (Polymorphic has_one) + // Extract class name for polymorphic relations + if ($class === self::class) { + $this->setField($componentName . 'Class', $item ? get_class($item) : null); + } + } elseif ($class = $schema->belongsToComponent(static::class, $componentName)) { + if ($item) { + // For belongs_to, add to has_one on other component + $joinField = $schema->getRemoteJoinField(static::class, $componentName, 'belongs_to', $polymorphic); + if (!$polymorphic) { + $joinField = substr($joinField, 0, -2); + } + $item->setComponent($joinField, $this); + } + } else { + throw new InvalidArgumentException( + "DataObject->setComponent(): Could not find component '$componentName'." + ); + } + + $this->components[$componentName] = $item; + return $this; + } + /** * Returns a one-to-many relation as a HasManyList * @@ -1655,7 +1846,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $remoteClass = $candidates[$relationName]; // If dot notation is present, extract just the first part that contains the class. - if (($fieldPos = strpos($remoteClass, '.'))!==false) { + if (($fieldPos = strpos($remoteClass, '.')) !== false) { return substr($remoteClass, 0, $fieldPos); } @@ -1752,7 +1943,12 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity case 'belongs_to': case 'has_many': { // These relations must have a has_one on the other end, so find it - $joinField = $schema->getRemoteJoinField($remoteClass, $remoteRelation, $relationType, $polymorphic); + $joinField = $schema->getRemoteJoinField( + $remoteClass, + $remoteRelation, + $relationType, + $polymorphic + ); if ($polymorphic) { throw new InvalidArgumentException(sprintf( "%s cannot generate opposite component of relation %s.%s, as the other end appears" . @@ -1806,7 +2002,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * Returns a many-to-many component, as a ManyManyList. * @param string $componentName Name of the many-many component * @param int|array $id Optional ID for parent of this relation, if not the current record - * @return RelationList|UnsavedRelationList The set of components + * @return ManyManyList|UnsavedRelationList The set of components */ public function getManyManyComponents($componentName, $id = null) { @@ -1827,7 +2023,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if (!$id) { if (!isset($this->unsavedRelations[$componentName])) { $this->unsavedRelations[$componentName] = - new UnsavedRelationList($manyManyComponent['parentClass'], $componentName, $manyManyComponent['childClass']); + new UnsavedRelationList( + $manyManyComponent['parentClass'], + $componentName, + $manyManyComponent['childClass'] + ); } return $this->unsavedRelations[$componentName]; } @@ -2180,9 +2380,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $tableClass = $this->record[$field . '_Lazy']; $this->loadLazyFields($tableClass); } + $schema = static::getSchema(); + + // Support unary relations as fields + if ($schema->unaryComponent(static::class, $field)) { + return $this->getComponent($field); + } // In case of complex fields, return the DBField object - if (static::getSchema()->compositeField(static::class, $field)) { + if ($schema->compositeField(static::class, $field)) { $this->record[$field] = $this->dbObject($field); } @@ -2331,9 +2537,9 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity if ($fields) { foreach ($fields as $name => $level) { $changedFields[$name] = array( - 'before' => array_key_exists($name, $this->original) ? $this->original[$name] : null, - 'after' => array_key_exists($name, $this->record) ? $this->record[$name] : null, - 'level' => $level + 'before' => array_key_exists($name, $this->original) ? $this->original[$name] : null, + 'after' => array_key_exists($name, $this->record) ? $this->record[$name] : null, + 'level' => $level ); } } @@ -2382,6 +2588,18 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $this->loadLazyFields($tableClass); } + // Support component assignent via field setter + $schema = static::getSchema(); + if ($schema->unaryComponent(static::class, $fieldName)) { + // Assign component directly + if (is_null($val) || $val instanceof DataObject) { + return $this->setComponent($fieldName, $val); + } + // Assign by ID instead of object + unset($this->components[$fieldName]); + $fieldName .= 'ID'; + } + // Situation 1: Passing an DBField if ($val instanceof DBField) { $val->setName($fieldName); @@ -2396,7 +2614,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Situation 2: Passing a literal or non-DBField object } else { // If this is a proper database field, we shouldn't be getting non-DBField objects - if (is_object($val) && static::getSchema()->fieldSpec(static::class, $fieldName)) { + if (is_object($val) && $schema->fieldSpec(static::class, $fieldName)) { throw new InvalidArgumentException('DataObject::setField: passed an object that is not a DBField'); } @@ -2483,8 +2701,9 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $schema = static::getSchema(); return ( array_key_exists($field, $this->record) + || array_key_exists($field, $this->components) || $schema->fieldSpec(static::class, $field) - || (substr($field, -2) == 'ID') && $schema->hasOneComponent(static::class, substr($field, 0, -2)) + || $schema->unaryComponent(static::class, $field) || $this->hasMethod("get{$field}") ); } @@ -2717,7 +2936,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity // Inspect relation type if (ClassInfo::hasMethod($component, $relation)) { - $component = $component->$relation(); + $component = $component->$relation(); } elseif ($component instanceof Relation || $component instanceof DataList) { // $relation could either be a field (aggregate), or another relation $singleton = DataObject::singleton($component->dataClass()); @@ -3120,7 +3339,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity ], $childField => [ 'type' => 'index', - 'name' =>$childField, + 'name' => $childField, 'columns' => [$childField], ], ]; @@ -3174,7 +3393,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $labels = $this->fieldLabels(); // fallback to summary fields (unless empty array is explicitly specified) - if (! $fields && ! is_array($fields)) { + if (!$fields && !is_array($fields)) { $summaryFields = array_keys($this->summaryFields()); $fields = array(); @@ -3288,13 +3507,28 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity ]; if ($includerelations) { $types['has_one'] = (array)Config::inst()->get($ancestorClass, 'has_one', Config::UNINHERITED); - $types['has_many'] = (array)Config::inst()->get($ancestorClass, 'has_many', Config::UNINHERITED); - $types['many_many'] = (array)Config::inst()->get($ancestorClass, 'many_many', Config::UNINHERITED); - $types['belongs_many_many'] = (array)Config::inst()->get($ancestorClass, 'belongs_many_many', Config::UNINHERITED); + $types['has_many'] = (array)Config::inst()->get( + $ancestorClass, + 'has_many', + Config::UNINHERITED + ); + $types['many_many'] = (array)Config::inst()->get( + $ancestorClass, + 'many_many', + Config::UNINHERITED + ); + $types['belongs_many_many'] = (array)Config::inst()->get( + $ancestorClass, + 'belongs_many_many', + Config::UNINHERITED + ); } foreach ($types as $type => $attrs) { foreach ($attrs as $name => $spec) { - $autoLabels[$name] = _t("{$ancestorClass}.{$type}_{$name}", FormField::name_to_label($name)); + $autoLabels[$name] = _t( + "{$ancestorClass}.{$type}_{$name}", + FormField::name_to_label($name) + ); } } } @@ -3428,6 +3662,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity { self::$subclass_access = false; } + public static function enable_subclass_access() { self::$subclass_access = true; @@ -3523,7 +3758,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity * * Note that you cannot have a has_one and belongs_to relationship with the same name. * - * @var array + * @var array * @config */ private static $has_one = []; diff --git a/src/ORM/DataObjectSchema.php b/src/ORM/DataObjectSchema.php index 01db82927..e309a1660 100644 --- a/src/ORM/DataObjectSchema.php +++ b/src/ORM/DataObjectSchema.php @@ -908,6 +908,19 @@ class DataObjectSchema return $classOnly ? $belongsToClass : $belongsTo; } + /** + * Check class for any unary component + * + * Alias for hasOneComponent() ?: belongsToComponent() + * @param string $class + * @param string $component + * @return string|null + */ + public function unaryComponent($class, $component) + { + return $this->hasOneComponent($class, $component) ?: $this->belongsToComponent($class, $component); + } + /** * * @param string $parentClass Parent class name diff --git a/src/ORM/ManyManyList.php b/src/ORM/ManyManyList.php index c259890fa..68fee0d91 100644 --- a/src/ORM/ManyManyList.php +++ b/src/ORM/ManyManyList.php @@ -225,6 +225,10 @@ class ManyManyList extends RelationList if (is_numeric($item)) { $itemID = $item; } elseif ($item instanceof $this->dataClass) { + // Ensure record is saved + if (!$item->isInDB()) { + $item->write(); + } $itemID = $item->ID; } else { throw new InvalidArgumentException( @@ -232,7 +236,7 @@ class ManyManyList extends RelationList ); } if (empty($itemID)) { - throw new InvalidArgumentException("ManyManyList::add() doesn't accept unsaved records"); + throw new InvalidArgumentException("ManyManyList::add() couldn't add this record"); } // Validate foreignID diff --git a/src/ORM/UnsavedRelationList.php b/src/ORM/UnsavedRelationList.php index ea11dddbd..2885c89fc 100644 --- a/src/ORM/UnsavedRelationList.php +++ b/src/ORM/UnsavedRelationList.php @@ -82,10 +82,6 @@ class UnsavedRelationList extends ArrayList implements Relation public function changeToList(RelationList $list) { foreach ($this->items as $key => $item) { - if (is_object($item)) { - /** @var DataObject $item */ - $item->write(); - } $list->add($item, $this->extraFields[$key]); } } diff --git a/tests/php/ORM/DataObjectDuplicationTest.php b/tests/php/ORM/DataObjectDuplicationTest.php index b3186d53f..48b3fbf38 100644 --- a/tests/php/ORM/DataObjectDuplicationTest.php +++ b/tests/php/ORM/DataObjectDuplicationTest.php @@ -2,32 +2,30 @@ namespace SilverStripe\ORM\Tests; -use SilverStripe\ORM\DataObject; use SilverStripe\Dev\SapphireTest; -use SilverStripe\ORM\FieldType\DBDatetime; class DataObjectDuplicationTest extends SapphireTest { + protected static $fixture_file = 'DataObjectDuplicationTest.yml'; - protected $usesDatabase = true; - - protected static $extra_dataobjects = array( - DataObjectDuplicationTest\Class1::class, - DataObjectDuplicationTest\Class2::class, - DataObjectDuplicationTest\Class3::class, - DataObjectDuplicationTest\Class4::class, - ); + protected static $extra_dataobjects = [ + DataObjectDuplicationTest\Antelope::class, + DataObjectDuplicationTest\Bobcat::class, + DataObjectDuplicationTest\Caribou::class, + DataObjectDuplicationTest\Dingo::class, + DataObjectDuplicationTest\Elephant::class, + DataObjectDuplicationTest\Frog::class, + DataObjectDuplicationTest\Goat::class, + ]; public function testDuplicate() { - DBDatetime::set_mock_now('2016-01-01 01:01:01'); - $orig = new DataObjectDuplicationTest\Class1(); - $orig->text = 'foo'; - $orig->write(); - DBDatetime::set_mock_now('2016-01-02 01:01:01'); + /** @var DataObjectDuplicationTest\Antelope $orig */ + $orig = $this->objFromFixture(DataObjectDuplicationTest\Antelope::class, 'one'); + /** @var DataObjectDuplicationTest\Antelope $duplicate */ $duplicate = $orig->duplicate(); $this->assertInstanceOf( - DataObjectDuplicationTest\Class1::class, + DataObjectDuplicationTest\Antelope::class, $duplicate, 'Creates the correct type' ); @@ -36,198 +34,79 @@ class DataObjectDuplicationTest extends SapphireTest $orig->ID, 'Creates a unique record' ); - $this->assertEquals( - 'foo', - $duplicate->text, - 'Copies fields' - ); - $this->assertEquals( - 2, - DataObjectDuplicationTest\Class1::get()->Count(), - 'Only creates a single duplicate' - ); - $this->assertEquals(DBDatetime::now()->Nice(), $duplicate->dbObject('Created')->Nice()); - $this->assertNotEquals($orig->dbObject('Created')->Nice(), $duplicate->dbObject('Created')->Nice()); - } - public function testDuplicateHasOne() - { - $relationObj = new DataObjectDuplicationTest\Class1(); - $relationObj->text = 'class1'; - $relationObj->write(); - - $orig = new DataObjectDuplicationTest\Class2(); - $orig->text = 'class2'; - $orig->oneID = $relationObj->ID; - $orig->write(); - - $duplicate = $orig->duplicate(); - $this->assertEquals( - $relationObj->ID, - $duplicate->oneID, - 'Copies has_one relationship' - ); - $this->assertEquals( - 2, - DataObjectDuplicationTest\Class2::get()->Count(), - 'Only creates a single duplicate' - ); - $this->assertEquals( - 1, - DataObjectDuplicationTest\Class1::get()->Count(), - 'Does not create duplicate of has_one relationship' - ); - } - - - public function testDuplicateManyManyClasses() - { - //create new test classes below - $one = new DataObjectDuplicationTest\Class1(); - $two = new DataObjectDuplicationTest\Class2(); - $three = new DataObjectDuplicationTest\Class3(); - - //set some simple fields - $text1 = "Test Text 1"; - $text2 = "Test Text 2"; - $text3 = "Test Text 3"; - $one->text = $text1; - $two->text = $text2; - $three->text = $text3; - - //write the to DB - $one->write(); - $two->write(); - $three->write(); - - //create relations - $one->twos()->add($two); - $one->threes()->add($three); - - $one = DataObject::get_by_id(DataObjectDuplicationTest\Class1::class, $one->ID); - $two = DataObject::get_by_id(DataObjectDuplicationTest\Class2::class, $two->ID); - $three = DataObject::get_by_id(DataObjectDuplicationTest\Class3::class, $three->ID); - - //test duplication - $oneCopy = $one->duplicate(true, true); - $twoCopy = $two->duplicate(true, true); - $threeCopy = $three->duplicate(true, true); - - $oneCopy = DataObject::get_by_id(DataObjectDuplicationTest\Class1::class, $oneCopy->ID); - $twoCopy = DataObject::get_by_id(DataObjectDuplicationTest\Class2::class, $twoCopy->ID); - $threeCopy = DataObject::get_by_id(DataObjectDuplicationTest\Class3::class, $threeCopy->ID); - - $this->assertNotNull($oneCopy, "Copy of 1 exists"); - $this->assertNotNull($twoCopy, "Copy of 2 exists"); - $this->assertNotNull($threeCopy, "Copy of 3 exists"); - - $this->assertEquals($text1, $oneCopy->text); - $this->assertEquals($text2, $twoCopy->text); - $this->assertEquals($text3, $threeCopy->text); - - $this->assertNotEquals( - $one->twos()->Count(), - $oneCopy->twos()->Count(), - "Many-to-one relation not copied (has_many)" - ); - $this->assertEquals( - $one->threes()->Count(), - $oneCopy->threes()->Count(), - "Object has the correct number of relations" - ); - $this->assertEquals( - $three->ones()->Count(), - $threeCopy->ones()->Count(), - "Object has the correct number of relations" - ); - - $this->assertEquals( - $one->ID, - $twoCopy->one()->ID, - "Match between relation of copy and the original" - ); - $this->assertEquals( - 0, - $oneCopy->twos()->Count(), - "Many-to-one relation not copied (has_many)" - ); - $this->assertEquals( - $three->ID, - $oneCopy->threes()->First()->ID, - "Match between relation of copy and the original" - ); - $this->assertEquals( - $one->ID, - $threeCopy->ones()->First()->ID, - "Match between relation of copy and the original" - ); - } - - public function testDuplicateManyManyFiltered() - { - $parent = new DataObjectDuplicationTest\Class4(); - $parent->Title = 'Parent'; - $parent->write(); - - $child = new DataObjectDuplicationTest\Class4(); - $child->Title = 'Child'; - $child->write(); - - $grandChild = new DataObjectDuplicationTest\Class4(); - $grandChild->Title = 'GrandChild'; - $grandChild->write(); - - $parent->Children()->add($child); - $child->Children()->add($grandChild); - - // Duplcating $child should only duplicate grandchild - $childDuplicate = $child->duplicate(true, 'many_many'); - $this->assertEquals(0, $childDuplicate->Parents()->count()); + // Check 'bobcats' relation duplicated + $twoOne = $this->objFromFixture(DataObjectDuplicationTest\Bobcat::class, 'one'); + $twoTwo = $this->objFromFixture(DataObjectDuplicationTest\Bobcat::class, 'two'); $this->assertListEquals( - [['Title' => 'GrandChild']], - $childDuplicate->Children() + [ + ['Title' => 'Bobcat two'], + ['Title' => 'Bobcat three'], + ], + $duplicate->bobcats() ); - - // Duplicate belongs_many_many only - $belongsDuplicate = $child->duplicate(true, 'belongs_many_many'); - $this->assertEquals(0, $belongsDuplicate->Children()->count()); - $this->assertListEquals( - [['Title' => 'Parent']], - $belongsDuplicate->Parents() + $this->assertEmpty( + array_intersect( + $orig->bobcats()->getIDList(), + $duplicate->bobcats()->getIDList() + ) ); + /** @var DataObjectDuplicationTest\Bobcat $twoTwoDuplicate */ + $twoTwoDuplicate = $duplicate->bobcats()->filter('Title', 'Bobcat two')->first(); + $this->assertNotEmpty($twoTwoDuplicate); + $this->assertNotEquals($twoTwo->ID, $twoTwoDuplicate->ID); - // Duplicate all - $allDuplicate = $child->duplicate(true, true); + // Check 'bobcats.self' relation duplicated + /** @var DataObjectDuplicationTest\Bobcat $twoOneDuplicate */ + $twoOneDuplicate = $twoTwoDuplicate->self(); + $this->assertNotEmpty($twoOneDuplicate); + $this->assertNotEquals($twoOne->ID, $twoOneDuplicate->ID); + + // Ensure 'bobcats.seven' instance is not duplicated + $sevenOne = $this->objFromFixture(DataObjectDuplicationTest\Goat::class, 'one'); + $sevenTwo = $this->objFromFixture(DataObjectDuplicationTest\Goat::class, 'two'); + $this->assertEquals($sevenOne->ID, $twoOneDuplicate->goat()->ID); + $this->assertEquals($sevenTwo->ID, $twoTwoDuplicate->goat()->ID); + + // Ensure that 'caribou' many_many list exists on both, but only the mapping table is duplicated + // many_many_extraFields are also duplicated + $caribouList = [ + [ + 'Title' => 'Caribou one', + 'Sort' => 4, + ], + [ + 'Title' => 'Caribou two', + 'Sort' => 5, + ], + ]; + // Original and duplicate lists have the same content $this->assertListEquals( - [['Title' => 'Parent']], - $allDuplicate->Parents() + $caribouList, + $orig->caribou() ); $this->assertListEquals( - [['Title' => 'GrandChild']], - $allDuplicate->Children() + $caribouList, + $duplicate->caribou() + ); + // Ids of each record are the same (only mapping content is duplicated) + $this->assertEquals( + $orig->caribou()->getIDList(), + $duplicate->caribou()->getIDList() ); - } - /** - * Test duplication of UnsavedRelations - */ - public function testDuplicateUnsaved() - { - $one = new DataObjectDuplicationTest\Class1(); - $one->text = "Test Text 1"; - $three = new DataObjectDuplicationTest\Class3(); - $three->text = "Test Text 3"; - $one->threes()->add($three); - $this->assertListEquals( - [['text' => 'Test Text 3']], - $one->threes() - ); - // Test duplicate - $dupe = $one->duplicate(false, true); - $this->assertEquals('Test Text 1', $dupe->text); - $this->assertListEquals( - [['text' => 'Test Text 3']], - $dupe->threes() - ); + // Ensure 'five' belongs_to is duplicated + $fiveOne = $this->objFromFixture(DataObjectDuplicationTest\Elephant::class, 'one'); + $fiveOneDuplicate = $duplicate->elephant(); + $this->assertNotEmpty($fiveOneDuplicate); + $this->assertEquals('Elephant one', $fiveOneDuplicate->Title); + $this->assertNotEquals($fiveOne->ID, $fiveOneDuplicate->ID); + + // Ensure 'five.Child' is duplicated + $sixOne = $this->objFromFixture(DataObjectDuplicationTest\Frog::class, 'one'); + $sixOneDuplicate = $fiveOneDuplicate->Child(); + $this->assertNotEmpty($sixOneDuplicate); + $this->assertEquals('Frog one', $sixOneDuplicate->Title); + $this->assertNotEquals($sixOne->ID, $sixOneDuplicate->ID); } } diff --git a/tests/php/ORM/DataObjectDuplicationTest.yml b/tests/php/ORM/DataObjectDuplicationTest.yml new file mode 100644 index 000000000..8ab748d9f --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest.yml @@ -0,0 +1,42 @@ +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope: + one: + Title: 'Antelope one' +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Goat: + one: + Title: 'Goat one' + two: + Title: 'Goat two' +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Bobcat: + one: + Title: 'Bobcat one' + goat: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Goat.one + two: + Title: 'Bobcat two' + antelope: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope.one + self: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Bobcat.one + goat: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Goat.two + three: + Title: 'Bobcat three' + antelope: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope.one +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Caribou: + one: + Title: 'Caribou one' + two: + Title: 'Caribou two' +DataObjectDuplicateTest_Antelope_caribou: + one: + DataObjectDuplicateTest_AntelopeID: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope.one + DataObjectDuplicateTest_CaribouID: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Caribou.one + Sort: 4 + two: + DataObjectDuplicateTest_AntelopeID: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope.one + DataObjectDuplicateTest_CaribouID: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Caribou.two + Sort: 5 +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Frog: + one: + Title: 'Frog one' +SilverStripe\ORM\Tests\DataObjectDuplicationTest\Elephant: + one: + Title: 'Elephant one' + Parent: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Antelope.one + Child: =>SilverStripe\ORM\Tests\DataObjectDuplicationTest\Frog.one diff --git a/tests/php/ORM/DataObjectDuplicationTest/Antelope.php b/tests/php/ORM/DataObjectDuplicationTest/Antelope.php new file mode 100644 index 000000000..5507168ec --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Antelope.php @@ -0,0 +1,46 @@ + 'Varchar', + ]; + + private static $has_many = [ + 'bobcats' => Bobcat::class, + ]; + + private static $many_many = [ + 'caribou' => Caribou::class, + ]; + + private static $many_many_extraFields = [ + 'caribou' => [ + 'Sort' => 'Int', + ], + ]; + + private static $belongs_to = [ + 'elephant' => Elephant::class, + ]; +} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Bobcat.php b/tests/php/ORM/DataObjectDuplicationTest/Bobcat.php new file mode 100644 index 000000000..53c8e4ea2 --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Bobcat.php @@ -0,0 +1,30 @@ + 'Varchar', + ]; + + private static $has_one = [ + 'antelope' => Antelope::class, + 'self' => Bobcat::class, + 'goat' => Goat::class, + ]; +} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Caribou.php b/tests/php/ORM/DataObjectDuplicationTest/Caribou.php new file mode 100644 index 000000000..85f279678 --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Caribou.php @@ -0,0 +1,23 @@ + 'Varchar', + ]; + + private static $belongs_many_many = [ + 'antelopes' => Antelope::class, + ]; +} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Class1.php b/tests/php/ORM/DataObjectDuplicationTest/Class1.php deleted file mode 100644 index 57a6c8302..000000000 --- a/tests/php/ORM/DataObjectDuplicationTest/Class1.php +++ /dev/null @@ -1,23 +0,0 @@ - 'Varchar' - ); - - private static $has_many = array( - 'twos' => Class2::class - ); - - private static $many_many = array( - 'threes' => Class3::class - ); -} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Class2.php b/tests/php/ORM/DataObjectDuplicationTest/Class2.php deleted file mode 100644 index b1b96bf61..000000000 --- a/tests/php/ORM/DataObjectDuplicationTest/Class2.php +++ /dev/null @@ -1,19 +0,0 @@ - 'Varchar' - ); - - private static $has_one = array( - 'one' => Class1::class - ); -} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Class3.php b/tests/php/ORM/DataObjectDuplicationTest/Class3.php deleted file mode 100644 index c018152da..000000000 --- a/tests/php/ORM/DataObjectDuplicationTest/Class3.php +++ /dev/null @@ -1,19 +0,0 @@ - 'Varchar' - ); - - private static $belongs_many_many = array( - 'ones' => Class1::class - ); -} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Class4.php b/tests/php/ORM/DataObjectDuplicationTest/Dingo.php similarity index 61% rename from tests/php/ORM/DataObjectDuplicationTest/Class4.php rename to tests/php/ORM/DataObjectDuplicationTest/Dingo.php index 8d5aad1e4..879ecbd5d 100644 --- a/tests/php/ORM/DataObjectDuplicationTest/Class4.php +++ b/tests/php/ORM/DataObjectDuplicationTest/Dingo.php @@ -11,19 +11,23 @@ use SilverStripe\ORM\ManyManyList; * @method ManyManyList Children() * @method ManyManyList Parents() */ -class Class4 extends DataObject implements TestOnly +class Dingo extends DataObject implements TestOnly { - private static $table_name = 'DataObjectDuplicateTest_Class4'; + private static $table_name = 'DataObjectDuplicateTest_Dingo'; + + private static $cascade_duplicates = [ + 'Children', + ]; private static $db = [ 'Title' => 'Varchar', ]; private static $many_many = [ - 'Children' => Class4::class, + 'Children' => Dingo::class, ]; private static $belongs_many_many = [ - 'Parents' => Class4::class, + 'Parents' => Dingo::class, ]; } diff --git a/tests/php/ORM/DataObjectDuplicationTest/Elephant.php b/tests/php/ORM/DataObjectDuplicationTest/Elephant.php new file mode 100644 index 000000000..75df638f4 --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Elephant.php @@ -0,0 +1,29 @@ + 'Varchar', + ]; + + private static $has_one = [ + 'Parent' => Antelope::class, + 'Child' => Frog::class, + ]; +} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Frog.php b/tests/php/ORM/DataObjectDuplicationTest/Frog.php new file mode 100644 index 000000000..95d00856a --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Frog.php @@ -0,0 +1,25 @@ + 'Varchar', + ]; + + private static $belongs_to = [ + 'Parent' => Elephant::class, + ]; +} diff --git a/tests/php/ORM/DataObjectDuplicationTest/Goat.php b/tests/php/ORM/DataObjectDuplicationTest/Goat.php new file mode 100644 index 000000000..e18c0c6ea --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Goat.php @@ -0,0 +1,23 @@ + 'Varchar', + ]; + + private static $belongs_to = [ + 'bobcats' => Bobcat::class, + ]; +}