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

@@ -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

src/ORM/DataObject.php

@@ -399,14 +399,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
     }
 
     /**
-     * Create a duplicate of this node.
+     * Create a duplicate of this node. Can duplicate many_many relations
-     * Note: now also duplicates 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
+     * 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.
      *
      * @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) {
+        // Get list of relations to duplicate
-            user_error(
+        if ($filter === 'many_many' || $filter === 'belongs_many_many') {
-                "Can't duplicate relations for an object that has not been written to the database",
+            $relations = $sourceObject->config()->get($filter);
-                E_USER_ERROR
+        } elseif ($filter === true) {
-            );
+            $relations = $sourceObject->manyMany();
+        } else {
+            throw new InvalidArgumentException("Invalid many_many duplication filter");
         }
-
+        foreach ($relations as $manyManyName => $type) {
-        //duplicate complex relations
+            $this->duplicateManyManyRelation($sourceObject, $destinationObject, $manyManyName);
-        // 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);
         }
     }
-        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
+     * Duplicates a single many_many relation 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 DataObject $sourceObject
-     * @param string $name the name of the relation to duplicate (e.g. members)
+     * @param DataObject $destinationObject
+     * @param string $manyManyName
      */
-    private function duplicateRelations($sourceObject, $destinationObject, $name)
+    protected function duplicateManyManyRelation($sourceObject, $destinationObject, $manyManyName)
     {
-        $relations = $sourceObject->$name();
+        // Ensure this component exists on the destination side as well
-        if ($relations) {
+        if (!static::getSchema()->manyManyComponent(get_class($destinationObject), $manyManyName)) {
-            if ($relations instanceof RelationList) {   //many-to-something relation
+            return;
-                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;
         }
+
+        // Copy all components from source to destination
+        $source = $sourceObject->getManyManyComponents($manyManyName);
+        $dest = $destinationObject->getManyManyComponents($manyManyName);
+        foreach ($source as $item) {
+            $dest->add($item);
         }
     }
 

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();
+        $oneCopy = $one->duplicate(true, true);
-        $twoCopy = $two->duplicate();
+        $twoCopy = $two->duplicate(true, true);
-        $threeCopy = $three->duplicate();
+        $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()
+        );
+    }
 }

tests/php/ORM/DataObjectDuplicationTest/Class4.php

@@ -0,0 +1,27 @@
+<?php
+
+
+namespace SilverStripe\ORM\Tests\DataObjectDuplicationTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\ManyManyList;
+
+/**
+ * @method ManyManyList Children()
+ * @method ManyManyList Parents()
+ */
+class Class4 extends DataObject implements TestOnly
+{
+    private static $db = [
+        'Title' => 'Varchar',
+    ];
+
+    private static $many_many = [
+        'Children' => Class4::class,
+    ];
+
+    private static $belongs_many_many = [
+        'Parents' => Class4::class,
+    ];
+}