Skip to content

Commit 0ebbd52

Browse files
authored
Merge pull request #1578 from scyzoryck/union-serialization
Do not change types during deseralization
2 parents 1c528c1 + 7867d8e commit 0ebbd52

File tree

11 files changed

+146
-39
lines changed

11 files changed

+146
-39
lines changed

.github/workflows/benchmark.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ on:
99
jobs:
1010
phpbench:
1111
name: "PHPBench"
12-
runs-on: "ubuntu-20.04"
12+
runs-on: ubuntu-latest
1313

1414
strategy:
1515
matrix:

.github/workflows/ci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ on:
99
jobs:
1010
phpunit:
1111
name: "PHPUnit"
12-
runs-on: "ubuntu-20.04"
12+
runs-on: ubuntu-latest
1313

1414
strategy:
1515
fail-fast: false

.github/workflows/coding-standards.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ on:
99
jobs:
1010
coding-standards:
1111
name: "Coding Standards"
12-
runs-on: "ubuntu-20.04"
12+
runs-on: ubuntu-latest
1313

1414
strategy:
1515
matrix:

.github/workflows/static-analysis.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ on:
99
jobs:
1010
static-analysis-phpstan:
1111
name: "Static Analysis with PHPStan"
12-
runs-on: "ubuntu-20.04"
12+
runs-on: ubuntu-latest
1313

1414
strategy:
1515
fail-fast: false

src/Handler/UnionHandler.php

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
namespace JMS\Serializer\Handler;
66

7-
use JMS\Serializer\Context;
87
use JMS\Serializer\DeserializationContext;
98
use JMS\Serializer\Exception\NonVisitableTypeException;
9+
use JMS\Serializer\Exception\NotAcceptableException;
1010
use JMS\Serializer\Exception\RuntimeException;
1111
use JMS\Serializer\GraphNavigatorInterface;
1212
use JMS\Serializer\SerializationContext;
@@ -50,15 +50,18 @@ public function serializeUnion(
5050
SerializationContext $context
5151
): mixed {
5252
if ($this->isPrimitiveType(gettype($data))) {
53-
return $this->matchSimpleType($data, $type, $context);
53+
$resolvedType = [
54+
'name' => gettype($data),
55+
'params' => [],
56+
];
5457
} else {
5558
$resolvedType = [
5659
'name' => get_class($data),
5760
'params' => [],
5861
];
59-
60-
return $context->getNavigator()->accept($data, $resolvedType);
6162
}
63+
64+
return $context->getNavigator()->accept($data, $resolvedType);
6265
}
6366

6467
public function deserializeUnion(DeserializationVisitorInterface $visitor, mixed $data, array $type, DeserializationContext $context): mixed
@@ -87,38 +90,35 @@ public function deserializeUnion(DeserializationVisitorInterface $visitor, mixed
8790
return $context->getNavigator()->accept($data, $finalType);
8891
}
8992

90-
foreach ($type['params'][0] as $possibleType) {
91-
if ($this->isPrimitiveType($possibleType['name']) && $this->testPrimitive($data, $possibleType['name'], $context->getFormat())) {
92-
return $context->getNavigator()->accept($data, $possibleType);
93-
}
94-
}
93+
$dataType = gettype($data);
9594

96-
return null;
97-
}
95+
if (
96+
array_filter(
97+
$type['params'][0],
98+
static fn (array $type): bool => $type['name'] === $dataType || (isset(self::$aliases[$dataType]) && $type['name'] === self::$aliases[$dataType]),
99+
)
100+
) {
101+
return $context->getNavigator()->accept($data, [
102+
'name' => $dataType,
103+
'params' => [],
104+
]);
105+
}
98106

99-
private function matchSimpleType(mixed $data, array $type, Context $context): mixed
100-
{
101107
foreach ($type['params'][0] as $possibleType) {
102-
if ($this->isPrimitiveType($possibleType['name']) && !$this->testPrimitive($data, $possibleType['name'], $context->getFormat())) {
103-
continue;
104-
}
105-
106-
try {
108+
if ($this->isPrimitiveType($possibleType['name']) && $this->testPrimitive($data, $possibleType['name'])) {
107109
return $context->getNavigator()->accept($data, $possibleType);
108-
} catch (NonVisitableTypeException $e) {
109-
continue;
110110
}
111111
}
112112

113-
return null;
113+
throw new NotAcceptableException();
114114
}
115115

116116
private function isPrimitiveType(string $type): bool
117117
{
118118
return in_array($type, ['int', 'integer', 'float', 'double', 'bool', 'boolean', 'true', 'false', 'string', 'array'], true);
119119
}
120120

121-
private function testPrimitive(mixed $data, string $type, string $format): bool
121+
private function testPrimitive(mixed $data, string $type): bool
122122
{
123123
switch ($type) {
124124
case 'array':
@@ -143,7 +143,7 @@ private function testPrimitive(mixed $data, string $type, string $format): bool
143143
return false === $data;
144144

145145
case 'string':
146-
return is_string($data);
146+
return !is_array($data) && !is_object($data);
147147
}
148148

149149
return false;
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace JMS\Serializer\Tests\Fixtures\TypedProperties;
6+
7+
class BoolOrString
8+
{
9+
public bool|string $data;
10+
11+
public function __construct($data)
12+
{
13+
$this->data = $data;
14+
}
15+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace JMS\Serializer\Tests\Fixtures\TypedProperties;
6+
7+
class FalseOrString
8+
{
9+
public false|string $data;
10+
11+
public function __construct(false|string $data)
12+
{
13+
$this->data = $data;
14+
}
15+
}

tests/Fixtures/TypedProperties/UnionTypedProperties.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
class UnionTypedProperties
88
{
9-
private int|bool|float|string|array $data;
9+
public bool|float|string|array|int $data;
1010

1111
private int|bool|float|string|null $nullableData = null;
1212

tests/Serializer/JsonSerializationTest.php

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@
2424
use JMS\Serializer\Tests\Fixtures\ObjectWithInlineArray;
2525
use JMS\Serializer\Tests\Fixtures\ObjectWithObjectProperty;
2626
use JMS\Serializer\Tests\Fixtures\Tag;
27+
use JMS\Serializer\Tests\Fixtures\TypedProperties\BoolOrString;
2728
use JMS\Serializer\Tests\Fixtures\TypedProperties\ComplexDiscriminatedUnion;
29+
use JMS\Serializer\Tests\Fixtures\TypedProperties\FalseOrString;
2830
use JMS\Serializer\Tests\Fixtures\TypedProperties\UnionTypedProperties;
2931
use JMS\Serializer\Visitor\Factory\JsonSerializationVisitorFactory;
3032
use JMS\Serializer\Visitor\SerializationVisitorInterface;
@@ -151,9 +153,12 @@ protected static function getContent($key)
151153
$outputs['uninitialized_typed_props'] = '{"virtual_role":{},"id":1,"role":{},"tags":[]}';
152154
$outputs['custom_datetimeinterface'] = '{"custom":"2021-09-07"}';
153155
$outputs['data_integer'] = '{"data":10000}';
156+
$outputs['data_integer_one'] = '{"data":1}';
154157
$outputs['data_float'] = '{"data":1.236}';
155158
$outputs['data_bool'] = '{"data":false}';
156159
$outputs['data_string'] = '{"data":"foo"}';
160+
$outputs['data_string_empty'] = '{"data":""}';
161+
$outputs['data_string_zero'] = '{"data":"0"}';
157162
$outputs['data_array'] = '{"data":[1,2,3]}';
158163
$outputs['data_true'] = '{"data":true}';
159164
$outputs['data_false'] = '{"data":false}';
@@ -452,12 +457,16 @@ public static function getTypeHintedArraysAndStdClass()
452457

453458
public static function getSimpleUnionProperties(): iterable
454459
{
455-
yield 'int' => [[10000, null, false], 'union_typed_properties_integer'];
456-
yield 'float' => [[1.236, null, false], 'union_typed_properties_float'];
457-
yield 'bool' => [[false, null, false], 'union_typed_properties_bool'];
458-
yield 'string' => [['foo', null, false], 'union_typed_properties_string'];
459-
yield 'array' => [[[1, 2, 3], null, false], 'union_typed_properties_array'];
460-
yield 'false_array' => [[false, null, 'foo'], 'union_typed_properties_false_string'];
460+
yield [10000, 'data_integer'];
461+
yield [1.236, 'data_float'];
462+
yield [false, 'data_bool'];
463+
yield ['foo', 'data_string'];
464+
yield [[1, 2, 3], 'data_array'];
465+
yield [1, 'data_integer_one'];
466+
yield ['0', 'data_string_zero'];
467+
yield ['', 'data_string_empty'];
468+
yield [true, 'data_true'];
469+
yield [false, 'data_false'];
461470
}
462471

463472
/**
@@ -472,9 +481,57 @@ public function testUnionProperties($data, string $expected): void
472481
return;
473482
}
474483

475-
$object = new UnionTypedProperties(...$data);
476-
self::assertEquals($object, $this->deserialize(static::getContent($expected), UnionTypedProperties::class));
477-
self::assertEquals($this->serialize($object), static::getContent($expected));
484+
$deserialized = $this->deserialize(static::getContent($expected), UnionTypedProperties::class);
485+
486+
self::assertSame($data, $deserialized->data);
487+
self::assertSame($this->serialize($deserialized), static::getContent($expected));
488+
}
489+
490+
public static function getUnionCastableTypes(): iterable
491+
{
492+
yield ['10000', 'data_integer'];
493+
yield ['1.236', 'data_float'];
494+
yield [true, 'data_integer_one'];
495+
}
496+
497+
/**
498+
* @dataProvider getUnionCastableTypes
499+
*/
500+
#[DataProvider('getUnionCastableTypes')]
501+
public function testUnionPropertiesWithCastableType($data, string $expected)
502+
{
503+
if (PHP_VERSION_ID < 80000) {
504+
$this->markTestSkipped(sprintf('%s requires PHP 8.0', TypedPropertiesDriver::class));
505+
506+
return;
507+
}
508+
509+
$deserialized = $this->deserialize(static::getContent($expected), BoolOrString::class);
510+
511+
self::assertSame($data, $deserialized->data);
512+
}
513+
514+
public static function getUnionNotCastableTypes(): iterable
515+
{
516+
yield ['data_array'];
517+
}
518+
519+
/**
520+
* @dataProvider getUnionNotCastableTypes
521+
*/
522+
#[DataProvider('getUnionNotCastableTypes')]
523+
public function testUnionPropertiesWithNotCastableType(string $expected)
524+
{
525+
if (PHP_VERSION_ID < 80000) {
526+
$this->markTestSkipped(sprintf('%s requires PHP 8.0', TypedPropertiesDriver::class));
527+
528+
return;
529+
}
530+
531+
$deserialized = $this->deserialize(static::getContent($expected), BoolOrString::class);
532+
533+
$this->expectException(\Error::class);
534+
$deserialized->data;
478535
}
479536

480537
public function testTrueDataType()
@@ -489,7 +546,6 @@ public function testTrueDataType()
489546
static::getContent('data_true'),
490547
$this->serialize(new DataTrue(true)),
491548
);
492-
493549
self::assertEquals(
494550
new DataTrue(true),
495551
$this->deserialize(static::getContent('data_true'), DataTrue::class),
@@ -517,6 +573,16 @@ public function testFalseDataType()
517573
$this->deserialize(static::getContent('data_false'), DataFalse::class),
518574
);
519575

576+
self::assertEquals(
577+
static::getContent('data_false'),
578+
$this->serialize(new FalseOrString(false)),
579+
);
580+
581+
self::assertEquals(
582+
new FalseOrString(false),
583+
$this->deserialize(static::getContent('data_false'), FalseOrString::class),
584+
);
585+
520586
$this->expectException(TypeError::class);
521587
$this->deserialize(static::getContent('data_true'), DataFalse::class);
522588
}

tests/Serializer/JsonStrictSerializationTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
namespace JMS\Serializer\Tests\Serializer;
66

7+
use JMS\Serializer\Exception\NonVisitableTypeException;
78
use JMS\Serializer\SerializerBuilder;
89
use JMS\Serializer\Visitor\Factory\JsonDeserializationVisitorFactory;
910
use PHPUnit\Framework\Attributes\DataProvider;
@@ -25,4 +26,15 @@ public function testFirstClassMapCollections(array $items, string $expected): vo
2526
{
2627
self::markTestIncomplete('Fixtures are broken');
2728
}
29+
30+
/**
31+
* @dataProvider getUnionCastableTypes
32+
*/
33+
#[DataProvider('getUnionCastableTypes')]
34+
public function testUnionPropertiesWithCastableType($data, string $expected): void
35+
{
36+
$this->expectException(NonVisitableTypeException::class);
37+
38+
parent::testUnionPropertiesWithCastableType($data, $expected);
39+
}
2840
}

0 commit comments

Comments
 (0)