Skip to content

Commit f5699bc

Browse files
chrispennychrispenny
committed
Simplify polymorphic has_one handling from PR #32 
1 parent 22497c6 commit f5699bc

File tree

3 files changed

+99
-58
lines changed

3 files changed

+99
-58
lines changed

‎src/Service/FixtureService.php‎

Lines changed: 21 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -231,11 +231,23 @@ private function addDataObjectHasOneFields(DataObject $dataObject): void
231231
}
232232

233233
foreach ($hasOneRelationships as $relationName => $relationClassName) {
234-
$relationClassName = $this->handlePolymorphicRelationship($relationName, $relationClassName, $dataObject);
235-
if ($relationClassName === null) {
236-
continue;
234+
// Polymorphic has_one relationships can be defined as arrays, eg:
235+
// ['class' => DataObject::class, 'type' => 'polymorphic']
236+
// Extract the class name so the rest of the method can treat it as a string
237+
if (is_array($relationClassName)) {
238+
$relationClassName = $relationClassName['class'] ?? null;
239+
240+
if ($relationClassName === null) {
241+
$this->addWarning(sprintf(
242+
'Polymorphic relationship "%s" in class "%s" has no "class" key defined',
243+
$relationName,
244+
$dataObject->ClassName
245+
));
246+
247+
continue;
248+
}
237249
}
238-
250+
239251
// Relationship field names (as represented in the Database) are always appended with `ID`
240252
$relationFieldName = sprintf('%sID', $relationName);
241253
// field_classname_map provides devs with the opportunity to describe polymorphic relationships (see the
@@ -245,15 +257,14 @@ private function addDataObjectHasOneFields(DataObject $dataObject): void
245257
// Apply the map that has been specified
246258
if ($fieldClassNameMap !== null && array_key_exists($relationFieldName, $fieldClassNameMap)) {
247259
$relationClassName = $dataObject->relField($fieldClassNameMap[$relationFieldName]);
248-
249-
// Ensure we have a valid class name string
250-
if (!is_string($relationClassName) || empty($relationClassName)) {
260+
261+
if (!is_string($relationClassName) || $relationClassName === '') {
251262
$this->addWarning(sprintf(
252-
'field_classname_map for field "%s" in class "%s" returned invalid class name: %s',
263+
'field_classname_map for "%s" in "%s" did not resolve to a valid class name',
253264
$relationFieldName,
254-
$dataObject->ClassName,
255-
is_array($relationClassName) ? 'array' : gettype($relationClassName)
265+
$dataObject->ClassName
256266
));
267+
257268
continue;
258269
}
259270
}
@@ -331,48 +342,6 @@ private function addDataObjectHasOneFields(DataObject $dataObject): void
331342
}
332343
}
333344

334-
/**
335-
* Handle polymorphic relationship configurations where the relationship class is defined as an array
336-
*
337-
* @param string $relationName The name of the relationship
338-
* @param string|array $relationClassName The class name or polymorphic configuration array
339-
* @param DataObject $dataObject The data object being processed
340-
* @return string|null The resolved class name, or null if the relationship should be skipped
341-
*/
342-
private function handlePolymorphicRelationship(string $relationName, string|array $relationClassName, DataObject $dataObject): ?string
343-
{
344-
// If it's already a string, no processing needed
345-
if (is_string($relationClassName)) {
346-
return $relationClassName;
347-
}
348-
349-
// Handle polymorphic relationships where relationClassName is an array
350-
if (is_array($relationClassName)) {
351-
// For polymorphic relationships, get the base class
352-
if (isset($relationClassName['class'])) {
353-
return $relationClassName['class'];
354-
} else {
355-
$this->addWarning(sprintf(
356-
'Polymorphic relationship "%s" in class "%s" has invalid configuration: %s',
357-
$relationName,
358-
$dataObject->ClassName,
359-
json_encode($relationClassName, JSON_PARTIAL_OUTPUT_ON_ERROR | JSON_UNESCAPED_SLASHES, 2)
360-
));
361-
return null;
362-
}
363-
}
364-
365-
// If it's neither string nor array, something's wrong
366-
$this->addWarning(sprintf(
367-
'Relationship "%s" in class "%s" has unexpected type "%s": %s',
368-
$relationName,
369-
$dataObject->ClassName,
370-
gettype($relationClassName),
371-
json_encode($relationClassName, JSON_PARTIAL_OUTPUT_ON_ERROR | JSON_UNESCAPED_SLASHES, 2)
372-
));
373-
return null;
374-
}
375-
376345
/**
377346
* @throws Exception
378347
*/

‎tests/Mocks/Pages/MockNativePoly.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
namespace ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages;
4+
5+
use SilverStripe\Dev\TestOnly;
6+
use SilverStripe\ORM\DataObject;
7+
8+
/**
9+
* Uses the Silverstripe 5 array-format polymorphic has_one definition.
10+
*
11+
* @property string $Title
12+
* @property int $OwnerID
13+
* @property string $OwnerClass
14+
* @method DataObject Owner()
15+
*/
16+
class MockNativePoly extends DataObject implements TestOnly
17+
{
18+
19+
private static string $table_name = 'DOToFixture_MockNativePoly';
20+
21+
private static array $db = [
22+
'Title' => 'Varchar',
23+
];
24+
25+
private static array $has_one = [
26+
'Owner' => [
27+
'class' => DataObject::class,
28+
'type' => 'polymorphic',
29+
],
30+
];
31+
32+
private static array $field_classname_map = [
33+
'OwnerID' => 'OwnerClass',
34+
];
35+
36+
}

‎tests/Service/FixtureServiceTest.php‎

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@
88
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockImage;
99
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockTag;
1010
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Models\MockThroughTarget;
11+
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockNativePoly;
1112
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPage;
1213
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPageWithExclusions;
1314
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Pages\MockPolymorphicPage;
1415
use ChrisPenny\DataObjectToFixture\Tests\Mocks\Relations\MockThroughObject;
1516
use Exception;
16-
use InvalidArgumentException;
1717
use SilverStripe\Dev\SapphireTest;
1818
use Symfony\Component\Yaml\Yaml;
1919

@@ -35,6 +35,7 @@ class FixtureServiceTest extends SapphireTest
3535
MockThroughTarget::class,
3636
MockThroughObject::class,
3737
MockExcludedObject::class,
38+
MockNativePoly::class,
3839
];
3940

4041
public function testAddDataObjectThrowsExceptionWhenNotInDb(): void
@@ -277,7 +278,7 @@ public function testPolymorphicHasOneWithFieldClassnameMap(): void
277278
$this->assertSame($expected, Yaml::parse($service->outputFixture()));
278279
}
279280

280-
public function testPolymorphicHasOneWithoutClassnameThrowsException(): void
281+
public function testPolymorphicHasOneWithoutClassnameWarns(): void
281282
{
282283
$image = MockImage::create();
283284
$image->Name = 'no-map-image.jpg';
@@ -286,16 +287,51 @@ public function testPolymorphicHasOneWithoutClassnameThrowsException(): void
286287
$page = MockPolymorphicPage::create();
287288
$page->Title = 'Unmapped Polymorphic';
288289
$page->PolymorphicHasOneID = $image->ID;
289-
// Don't set PolymorphicHasOneClass — the map resolves to empty string
290+
// Don't set PolymorphicHasOneClass — the map resolves to empty/null
290291
$page->write();
291292

292293
$service = new FixtureService();
294+
$service->addDataObject($page);
295+
296+
$this->assertSame(
297+
[sprintf(
298+
'field_classname_map for "PolymorphicHasOneID" in "%s" did not resolve to a valid class name',
299+
MockPolymorphicPage::class
300+
)],
301+
$service->getWarnings()
302+
);
303+
}
293304

294-
// When the field_classname_map resolves to an empty/invalid class name, Silverstripe throws
295-
// an InvalidArgumentException from DataObject::get()
296-
$this->expectException(InvalidArgumentException::class);
305+
public function testArrayFormatPolymorphicHasOne(): void
306+
{
307+
$image = MockImage::create();
308+
$image->Name = 'native-poly.jpg';
309+
$image->write();
297310

311+
$page = MockNativePoly::create();
312+
$page->Title = 'Native Polymorphic';
313+
$page->OwnerID = $image->ID;
314+
$page->OwnerClass = MockImage::class;
315+
$page->write();
316+
317+
$service = new FixtureService();
298318
$service->addDataObject($page);
319+
320+
$parsed = Yaml::parse($service->outputFixture());
321+
322+
$expected = [
323+
MockImage::class => [
324+
$image->ID => ['Name' => 'native-poly.jpg'],
325+
],
326+
MockNativePoly::class => [
327+
$page->ID => [
328+
'Title' => 'Native Polymorphic',
329+
'Owner' => sprintf('=>%s.%s', MockImage::class, $image->ID),
330+
],
331+
],
332+
];
333+
334+
$this->assertSame($expected, $parsed);
299335
}
300336

301337
public function testHasOneSkipsWhenNoValue(): void

0 commit comments

Comments
 (0)