From f5f6fdce12abd6074de210b11b98a68fd929c62c Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 11 Oct 2016 17:50:19 +1300 Subject: [PATCH] API Duplication of many_many relationships now defaults to many_many only Fixes https://github.com/silverstripe/silverstripe-cms/issues/1453 --- docs/en/04_Changelogs/4.0.0.md | 2 + src/ORM/DataObject.php | 89 ++++++++----------- tests/php/ORM/DataObjectDuplicationTest.php | 77 +++++++++++++++- .../ORM/DataObjectDuplicationTest/Class4.php | 27 ++++++ 4 files changed, 141 insertions(+), 54 deletions(-) create mode 100644 tests/php/ORM/DataObjectDuplicationTest/Class4.php diff --git a/docs/en/04_Changelogs/4.0.0.md b/docs/en/04_Changelogs/4.0.0.md index b08bfbe53..7f30d97d4 100644 --- a/docs/en/04_Changelogs/4.0.0.md +++ b/docs/en/04_Changelogs/4.0.0.md @@ -1317,6 +1317,8 @@ A very small number of methods were chosen for deprecation, and will be removed * `DataObject::db` now returns composite fields. * `DataObject::ClassName` field has been refactored into a `DBClassName` type field. * `DataObject::can` has new method signature with `$context` parameter. +* `DataObject::duplicate` Now requires explicit flag to duplicate belongs_many_many (off by default), + but now works with unsaved relations. By default only many_many are duplicated. * `DBHTMLText` no longer enables shortcodes by default. Two injector aliases have been created for this class which can be used to select the correct behaviour: * `HTMLText`: `DBHTMLText` with shortcodes enabled diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index f8ca55c86..70bbd1ce9 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -399,14 +399,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Create a duplicate of this node. - * Note: now also duplicates relations. + * Create a duplicate of this node. Can duplicate many_many relations * * @param bool $doWrite Perform a write() operation before returning the object. * If this is true, it will create the duplicate in the database. - * @return DataObject A duplicate of this node. The exact type will be the type of this node. + * @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') + * @return static A duplicate of this node. The exact type will be the type of this node. */ - public function duplicate($doWrite = true) + public function duplicate($doWrite = true, $manyMany = 'many_many') { $map = $this->toMap(); unset($map['Created']); @@ -414,72 +416,59 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity $clone = Injector::inst()->create(static::class, $map, false, $this->model); $clone->ID = 0; - $clone->invokeWithExtensions('onBeforeDuplicate', $this, $doWrite); + $clone->invokeWithExtensions('onBeforeDuplicate', $this, $doWrite, $manyMany); + if ($manyMany) { + $this->duplicateManyManyRelations($this, $clone, $manyMany); + } if ($doWrite) { $clone->write(); - $this->duplicateManyManyRelations($this, $clone); } - $clone->invokeWithExtensions('onAfterDuplicate', $this, $doWrite); + $clone->invokeWithExtensions('onAfterDuplicate', $this, $doWrite, $manyMany); return $clone; } /** - * Copies the many_many and belongs_many_many relations from one object to another instance of the name of object - * The destinationObject must be written to the database already and have an ID. Writing is performed - * automatically when adding the new relations. + * Copies the many_many and belongs_many_many relations from one object to another instance of the name of object. * * @param DataObject $sourceObject the source object to duplicate from * @param DataObject $destinationObject the destination object to populate with the duplicated relations - * @return DataObject with the new many_many relations copied in + * @param bool|string $filter */ - protected function duplicateManyManyRelations($sourceObject, $destinationObject) + protected function duplicateManyManyRelations($sourceObject, $destinationObject, $filter) { - if (!$destinationObject || $destinationObject->ID < 1) { - user_error( - "Can't duplicate relations for an object that has not been written to the database", - E_USER_ERROR - ); + // Get list of relations to duplicate + if ($filter === 'many_many' || $filter === 'belongs_many_many') { + $relations = $sourceObject->config()->get($filter); + } elseif ($filter === true) { + $relations = $sourceObject->manyMany(); + } else { + throw new InvalidArgumentException("Invalid many_many duplication filter"); } - - //duplicate complex relations - // DO NOT copy has_many relations, because copying the relation would result in us changing the has_one - // relation on the other side of this relation to point at the copy and no longer the original (being a - // has_one, it can only point at one thing at a time). So, all relations except has_many can and are copied - if ($sourceObject->hasOne()) { - foreach ($sourceObject->hasOne() as $name => $type) { - $this->duplicateRelations($sourceObject, $destinationObject, $name); - } + foreach ($relations as $manyManyName => $type) { + $this->duplicateManyManyRelation($sourceObject, $destinationObject, $manyManyName); } - if ($sourceObject->manyMany()) { - foreach ($sourceObject->manyMany() as $name => $type) { - //many_many include belongs_many_many - $this->duplicateRelations($sourceObject, $destinationObject, $name); - } - } - - return $destinationObject; } /** - * Helper function to duplicate relations from one object to another - * @param DataObject $sourceObject the source object to duplicate from - * @param DataObject $destinationObject the destination object to populate with the duplicated relations - * @param string $name the name of the relation to duplicate (e.g. members) + * Duplicates a single many_many relation from one object to another + * + * @param DataObject $sourceObject + * @param DataObject $destinationObject + * @param string $manyManyName */ - private function duplicateRelations($sourceObject, $destinationObject, $name) + protected function duplicateManyManyRelation($sourceObject, $destinationObject, $manyManyName) { - $relations = $sourceObject->$name(); - if ($relations) { - if ($relations instanceof RelationList) { //many-to-something relation - if ($relations->count() > 0) { //with more than one thing it is related to - foreach ($relations as $relation) { - $destinationObject->$name()->add($relation); - } - } - } else { //one-to-one relation - $destinationObject->{"{$name}ID"} = $relations->ID; - } + // Ensure this component exists on the destination side as well + if (!static::getSchema()->manyManyComponent(get_class($destinationObject), $manyManyName)) { + return; + } + + // Copy all components from source to destination + $source = $sourceObject->getManyManyComponents($manyManyName); + $dest = $destinationObject->getManyManyComponents($manyManyName); + foreach ($source as $item) { + $dest->add($item); } } diff --git a/tests/php/ORM/DataObjectDuplicationTest.php b/tests/php/ORM/DataObjectDuplicationTest.php index c19b52c23..1ec0f3635 100644 --- a/tests/php/ORM/DataObjectDuplicationTest.php +++ b/tests/php/ORM/DataObjectDuplicationTest.php @@ -14,7 +14,8 @@ class DataObjectDuplicationTest extends SapphireTest protected static $extra_dataobjects = array( DataObjectDuplicationTest\Class1::class, DataObjectDuplicationTest\Class2::class, - DataObjectDuplicationTest\Class3::class + DataObjectDuplicationTest\Class3::class, + DataObjectDuplicationTest\Class4::class, ); public function testDuplicate() @@ -108,9 +109,9 @@ class DataObjectDuplicationTest extends SapphireTest $three = DataObject::get_by_id(DataObjectDuplicationTest\Class3::class, $three->ID); //test duplication - $oneCopy = $one->duplicate(); - $twoCopy = $two->duplicate(); - $threeCopy = $three->duplicate(); + $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); @@ -161,4 +162,72 @@ class DataObjectDuplicationTest extends SapphireTest "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()); + $this->assertDOSEquals( + [['Title' => 'GrandChild']], + $childDuplicate->Children() + ); + + // Duplicate belongs_many_many only + $belongsDuplicate = $child->duplicate(true, 'belongs_many_many'); + $this->assertEquals(0, $belongsDuplicate->Children()->count()); + $this->assertDOSEquals( + [['Title' => 'Parent']], + $belongsDuplicate->Parents() + ); + + // Duplicate all + $allDuplicate = $child->duplicate(true, true); + $this->assertDOSEquals( + [['Title' => 'Parent']], + $allDuplicate->Parents() + ); + $this->assertDOSEquals( + [['Title' => 'GrandChild']], + $allDuplicate->Children() + ); + } + + /** + * 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->assertDOSEquals( + [['text' => 'Test Text 3']], + $one->threes() + ); + // Test duplicate + $dupe = $one->duplicate(false, true); + $this->assertEquals('Test Text 1', $dupe->text); + $this->assertDOSEquals( + [['text' => 'Test Text 3']], + $dupe->threes() + ); + } } diff --git a/tests/php/ORM/DataObjectDuplicationTest/Class4.php b/tests/php/ORM/DataObjectDuplicationTest/Class4.php new file mode 100644 index 000000000..2bef1b24f --- /dev/null +++ b/tests/php/ORM/DataObjectDuplicationTest/Class4.php @@ -0,0 +1,27 @@ + 'Varchar', + ]; + + private static $many_many = [ + 'Children' => Class4::class, + ]; + + private static $belongs_many_many = [ + 'Parents' => Class4::class, + ]; +}