Merge pull request #36 from chrispenny/fix/polymorphic-relationships

Co-authored-by: Jason Irish <jirish@dynamicagency.com>

Author: chrispenny

src/Service/FixtureService.php

@@ -231,6 +231,23 @@ private function addDataObjectHasOneFields(DataObject $dataObject): void
         }
 
         foreach ($hasOneRelationships as $relationName => $relationClassName) {
+            // Polymorphic has_one relationships can be defined as arrays, eg:
+            // ['class' => DataObject::class, 'type' => 'polymorphic']
+            // Extract the class name so the rest of the method can treat it as a string
+            if (is_array($relationClassName)) {
+                $relationClassName = $relationClassName['class'] ?? null;
+
+                if ($relationClassName === null) {
+                    $this->addWarning(sprintf(
+                        'Polymorphic relationship "%s" in class "%s" has no "class" key defined',
+                        $relationName,
+                        $dataObject->ClassName
+                    ));
+
+                    continue;
+                }
+            }
+
             // Relationship field names (as represented in the Database) are always appended with `ID`
             $relationFieldName = sprintf('%sID', $relationName);
             // field_classname_map provides devs with the opportunity to describe polymorphic relationships (see the
@@ -240,6 +257,16 @@ private function addDataObjectHasOneFields(DataObject $dataObject): void
             // Apply the map that has been specified
             if ($fieldClassNameMap !== null && array_key_exists($relationFieldName, $fieldClassNameMap)) {
                 $relationClassName = $dataObject->relField($fieldClassNameMap[$relationFieldName]);
+
+                if (!is_string($relationClassName) || $relationClassName === '') {
+                    $this->addWarning(sprintf(
+                        'field_classname_map for "%s" in "%s" did not resolve to a valid class name',
+                        $relationFieldName,
+                        $dataObject->ClassName
+                    ));
+
+                    continue;
+                }
             }
 
             // Check to see if class has requested that it not be included in relationship maps

tests/Mocks/Pages/MockNativePoly.php

@@ -0,0 +1,36 @@
+<?php
+
+namespace ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+/**
+ * Uses the Silverstripe 5 array-format polymorphic has_one definition.
+ *
+ * @property string $Title
+ * @property int $OwnerID
+ * @property string $OwnerClass
+ * @method DataObject Owner()
+ */
+class MockNativePoly extends DataObject implements TestOnly
+{
+
+    private static string $table_name = 'DOToFixture_MockNativePoly';
+
+    private static array $db = [
+        'Title' => 'Varchar',
+    ];
+
+    private static array $has_one = [
+        'Owner' => [
+            'class' => DataObject::class,
+            'type' => 'polymorphic',
+        ],
+    ];
+
+    private static array $field_classname_map = [
+        'OwnerID' => 'OwnerClass',
+    ];
+
+}

tests/Service/FixtureServiceTest.php

@@ -8,12 +8,12 @@
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockImage;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockTag;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockThroughTarget;
+use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockNativePoly;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPage;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPageWithExclusions;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPolymorphicPage;
 use ChrisPenny\DataObjectToFixture\Tests\Mocks\Relations\MockThroughObject;
 use Exception;
-use InvalidArgumentException;
 use SilverStripe\Dev\SapphireTest;
 use Symfony\Component\Yaml\Yaml;
 
@@ -35,6 +35,7 @@ class FixtureServiceTest extends SapphireTest
         MockThroughTarget::class,
         MockThroughObject::class,
         MockExcludedObject::class,
+        MockNativePoly::class,
     ];
 
     public function testAddDataObjectThrowsExceptionWhenNotInDb(): void
@@ -277,7 +278,7 @@ public function testPolymorphicHasOneWithFieldClassnameMap(): void
         $this->assertSame($expected, Yaml::parse($service->outputFixture()));
     }
 
-    public function testPolymorphicHasOneWithoutClassnameThrowsException(): void
+    public function testPolymorphicHasOneWithoutClassnameWarns(): void
     {
         $image = MockImage::create();
         $image->Name = 'no-map-image.jpg';
@@ -286,16 +287,51 @@ public function testPolymorphicHasOneWithoutClassnameThrowsException(): void
         $page = MockPolymorphicPage::create();
         $page->Title = 'Unmapped Polymorphic';
         $page->PolymorphicHasOneID = $image->ID;
-        // Don't set PolymorphicHasOneClass — the map resolves to empty string
+        // Don't set PolymorphicHasOneClass — the map resolves to empty/null
         $page->write();
 
         $service = new FixtureService();
+        $service->addDataObject($page);
+
+        $this->assertSame(
+            [sprintf(
+                'field_classname_map for "PolymorphicHasOneID" in "%s" did not resolve to a valid class name',
+                MockPolymorphicPage::class
+            )],
+            $service->getWarnings()
+        );
+    }
 
-        // When the field_classname_map resolves to an empty/invalid class name, Silverstripe throws
-        // an InvalidArgumentException from DataObject::get()
-        $this->expectException(InvalidArgumentException::class);
+    public function testArrayFormatPolymorphicHasOne(): void
+    {
+        $image = MockImage::create();
+        $image->Name = 'native-poly.jpg';
+        $image->write();
 
+        $page = MockNativePoly::create();
+        $page->Title = 'Native Polymorphic';
+        $page->OwnerID = $image->ID;
+        $page->OwnerClass = MockImage::class;
+        $page->write();
+
+        $service = new FixtureService();
         $service->addDataObject($page);
+
+        $parsed = Yaml::parse($service->outputFixture());
+
+        $expected = [
+            MockImage::class => [
+                $image->ID => ['Name' => 'native-poly.jpg'],
+            ],
+            MockNativePoly::class => [
+                $page->ID => [
+                    'Title' => 'Native Polymorphic',
+                    'Owner' => sprintf('=>%s.%s', MockImage::class, $image->ID),
+                ],
+            ],
+        ];
+
+        $this->assertSame($expected, $parsed);
     }
 
     public function testHasOneSkipsWhenNoValue(): void