Skip to content

Commit bc6b57a

Browse files
brutal-factoriesdbu
brutal-factories
and
dbu
authored
Better DateTime type information merging (#44)
* allow multiple datetime deserialization formats * DateTimeOptions: allow both string and array<string> for DateTimeOptions::$deserializeFormats, until v2.x --------- Co-authored-by: David Buchmann <david@liip.ch>
1 parent 41520e7 commit bc6b57a

File tree

7 files changed

+98
-16
lines changed

7 files changed

+98
-16
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@
22

33
# Version 1.x
44

5-
# 1.3.0 (unreleased)
5+
# 1.2.0 (unreleased)
6+
7+
* DateTimeOptions now features a list of deserialization formats instead of a single string one. Passing a string instead of an array to its `__construct`or is deprecated, and will be forbidden in the next version
8+
Similarly, `getDeserializeFormat(): ?string` is deprecated in favor of `getDeserializeFormats(): ?array`
9+
610

711
* Added `PropertyTypeIterable`, which generalizes `PropertyTypeArray` to allow merging Collection informations like one would with arrays, including between interfaces and concrete classes
812
* Deprecated `PropertyTypeArray`, please prefer using `PropertyTypeIterable` instead

‎src/Metadata/DateTimeOptions.php‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,22 @@ final class DateTimeOptions implements \JsonSerializable
2020
private $zone;
2121

2222
/**
23-
* Use if a different format should be used for parsing dates than for generating dates.
23+
* Use if different formats should be used for parsing dates than for generating dates.
2424
*
25-
* @var string|null
25+
* @var string[]|null
2626
*/
27-
private $deserializeFormat;
27+
private $deserializeFormats;
2828

29-
public function __construct(?string $format, ?string $zone, ?string $deserializeFormat)
29+
/**
30+
* @note Passing a string for $deserializeFormats is deprecated, please pass an array instead
31+
*
32+
* @param string[]|string|null $deserializeFormats
33+
*/
34+
public function __construct(?string $format, ?string $zone, $deserializeFormats)
3035
{
3136
$this->format = $format;
3237
$this->zone = $zone;
33-
$this->deserializeFormat = $deserializeFormat;
38+
$this->deserializeFormats = \is_string($deserializeFormats) ? [$deserializeFormats] : $deserializeFormats;
3439
}
3540

3641
public function getFormat(): ?string
@@ -43,17 +48,30 @@ public function getZone(): ?string
4348
return $this->zone;
4449
}
4550

51+
/**
52+
* @deprecated Please use {@see getDeserializeFormats}
53+
*/
4654
public function getDeserializeFormat(): ?string
4755
{
48-
return $this->deserializeFormat;
56+
foreach ($this->deserializeFormats ?? [] as $format) {
57+
return $format;
58+
}
59+
60+
return null;
61+
}
62+
63+
public function getDeserializeFormats(): ?array
64+
{
65+
return $this->deserializeFormats;
4966
}
5067

5168
public function jsonSerialize(): array
5269
{
5370
return array_filter([
5471
'format' => $this->format,
5572
'zone' => $this->zone,
56-
'deserialize_format' => $this->deserializeFormat,
73+
'deserialize_format' => $this->getDeserializeFormat(),
74+
'deserialize_formats' => $this->deserializeFormats,
5775
]);
5876
}
5977
}

‎src/Metadata/PropertyTypeClass.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,9 @@ public function merge(PropertyType $other): PropertyType
7171
if (is_a($this->getClassName(), Collection::class, true) && (($other instanceof PropertyTypeIterable) && $other->isCollection())) {
7272
return $other->merge($this);
7373
}
74+
if (is_a($this->getClassName(), \DateTimeInterface::class, true) && ($other instanceof PropertyTypeDateTime)) {
75+
return $other->merge($this);
76+
}
7477
if (!$other instanceof self) {
7578
throw new \UnexpectedValueException(sprintf('Can\'t merge type %s with %s, they must be the same or unknown', self::class, \get_class($other)));
7679
}

‎src/Metadata/PropertyTypeDateTime.php‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ public function getZone(): ?string
5858
return null;
5959
}
6060

61+
/**
62+
* @deprecated Please prefer {@link getDeserializeFormats}
63+
*/
6164
public function getDeserializeFormat(): ?string
6265
{
6366
if ($this->dateTimeOptions) {
@@ -67,18 +70,37 @@ public function getDeserializeFormat(): ?string
6770
return null;
6871
}
6972

73+
public function getDeserializeFormats(): ?array
74+
{
75+
if ($this->dateTimeOptions) {
76+
return $this->dateTimeOptions->getDeserializeFormats();
77+
}
78+
79+
return null;
80+
}
81+
7082
public function merge(PropertyType $other): PropertyType
7183
{
7284
$nullable = $this->isNullable() && $other->isNullable();
7385

7486
if ($other instanceof PropertyTypeUnknown) {
7587
return new self($this->immutable, $nullable, $this->dateTimeOptions);
7688
}
89+
if (($other instanceof PropertyTypeClass) && is_a($other->getClassName(), \DateTimeInterface::class, true)) {
90+
if (is_a($other->getClassName(), \DateTimeImmutable::class, true)) {
91+
return new self(true, $nullable, $this->dateTimeOptions);
92+
}
93+
if (is_a($other->getClassName(), \DateTime::class, true) || (\DateTimeInterface::class === $other->getClassName())) {
94+
return new self(false, $nullable, $this->dateTimeOptions);
95+
}
96+
97+
throw new \UnexpectedValueException("Can't merge type '{$this}' with '{$other}', they must be the same or unknown");
98+
}
7799
if (!$other instanceof self) {
78-
throw new \UnexpectedValueException(sprintf('Can\'t merge type %s with %s, they must be the same or unknown', self::class, \get_class($other)));
100+
throw new \UnexpectedValueException("Can't merge type '{$this}' with '{$other}', they must be the same or unknown");
79101
}
80102
if ($this->isImmutable() !== $other->isImmutable()) {
81-
throw new \UnexpectedValueException(sprintf('Can\'t merge type %s with %s, they must be equal', self::class, \get_class($other)));
103+
throw new \UnexpectedValueException("Can't merge type '{$this}' with '{$other}', they must be equal");
82104
}
83105

84106
$options = $this->dateTimeOptions ?: $other->dateTimeOptions;
@@ -99,4 +121,27 @@ public static function isTypeDateTime(string $typeName): bool
99121
{
100122
return \in_array($typeName, self::DATE_TIME_TYPES, true);
101123
}
124+
125+
/**
126+
* Find the most derived class that doesn't deny both class hints, meaning the most derived
127+
* between left and right if one is a child of the other
128+
*/
129+
protected function findCommonDateTimeClass(?string $left, ?string $right): ?string
130+
{
131+
if (null === $right) {
132+
return $left;
133+
}
134+
if (null === $left) {
135+
return $right;
136+
}
137+
138+
if (is_a($left, $right, true)) {
139+
return $left;
140+
}
141+
if (is_a($right, $left, true)) {
142+
return $right;
143+
}
144+
145+
throw new \UnexpectedValueException("Collection classes '{$left}' and '{$right}' do not match.");
146+
}
102147
}

‎src/TypeParser/JMSTypeParser.php‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ final class JMSTypeParser
2020
{
2121
private const TYPE_ARRAY = 'array';
2222
private const TYPE_ARRAY_COLLECTION = 'ArrayCollection';
23+
private const TYPE_DATETIME_INTERFACE = 'DateTimeInterface';
2324

2425
/**
2526
* @var Parser
@@ -80,15 +81,23 @@ private function parseType(array $typeInfo, bool $isSubType = false): PropertyTy
8081
throw new InvalidTypeException(sprintf('JMS property type array can\'t have more than 2 parameters (%s)', var_export($typeInfo, true)));
8182
}
8283

83-
if (PropertyTypeDateTime::isTypeDateTime($typeInfo['name'])) {
84+
if (PropertyTypeDateTime::isTypeDateTime($typeInfo['name']) || (self::TYPE_DATETIME_INTERFACE === $typeInfo['name'])) {
8485
// the case of datetime without params is already handled above, we know we have params
86+
$serializeFormat = $typeInfo['params'][0] ?: null;
87+
// {@link \JMS\Serializer\Handler\DateHandler} of jms/serializer defaults to using the serialization format as a deserialization format if none was supplied...
88+
$deserializeFormats = ($typeInfo['params'][2] ?? null) ?: $serializeFormat;
89+
// ... and always converts single strings to arrays
90+
$deserializeFormats = \is_string($deserializeFormats) ? [$deserializeFormats] : $deserializeFormats;
91+
// Jms defaults to DateTime when given DateTimeInterface despite the documentation saying DateTimeImmutable, {@see \JMS\Serializer\Handler\DateHandler} in jms/serializer
92+
$className = (self::TYPE_DATETIME_INTERFACE === $typeInfo['name']) ? \DateTime::class : $typeInfo['name'];
93+
8594
return PropertyTypeDateTime::fromDateTimeClass(
86-
$typeInfo['name'],
95+
$className,
8796
$nullable,
8897
new DateTimeOptions(
89-
$typeInfo['params'][0] ?: null,
98+
$serializeFormat,
9099
($typeInfo['params'][1] ?? null) ?: null,
91-
($typeInfo['params'][2] ?? null) ?: null
100+
$deserializeFormats,
92101
)
93102
);
94103
}

‎tests/ModelParser/JMSParserTestCase.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,6 +1062,7 @@ public function foo(): ?\DateTime
10621062
$this->assertSame('Y-m-d H:i:s', $type->getFormat(), 'Date time format should match');
10631063
$this->assertSame('Europe/Zurich', $type->getZone(), 'Date time zone should match');
10641064
$this->assertSame('Y-m-d', $type->getDeserializeFormat(), 'Date time deserialize format should match');
1065+
$this->assertSame(['Y-m-d'], $type->getDeserializeFormats(), 'Date time deserialize format should match');
10651066
}
10661067

10671068
public function testVirtualPropertyTypeExtendingDateTimeWithUnknown(): void
@@ -1093,6 +1094,7 @@ public function foo()
10931094
$this->assertSame('Y-m-d H:i:s', $type->getFormat(), 'Date time format should match');
10941095
$this->assertSame('Europe/Zurich', $type->getZone(), 'Date time zone should match');
10951096
$this->assertSame('Y-m-d', $type->getDeserializeFormat(), 'Date time deserialize format should match');
1097+
$this->assertSame(['Y-m-d'], $type->getDeserializeFormats(), 'Date time deserialize format should match');
10961098
}
10971099

10981100
public function testVirtualPropertyInvalidType(): void

‎tests/TypeParser/JMSTypeParserTest.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ public function provideDateTimeTypeCases(): iterable
122122
'DateTime|null',
123123
'Y-m-d H:i:s',
124124
null,
125-
null,
125+
'Y-m-d H:i:s',
126126
];
127127

128128
yield [
@@ -162,7 +162,7 @@ public function provideDateTimeTypeCases(): iterable
162162
'DateTimeImmutable|null',
163163
'Y-m-d H:i:s',
164164
null,
165-
null,
165+
'Y-m-d H:i:s',
166166
];
167167

168168
yield [
@@ -202,6 +202,7 @@ public function testDateTimeType(string $rawType, string $expectedType, ?string
202202
$this->assertSame($expectedFormat, $type->getFormat(), 'Date time format should match');
203203
$this->assertSame($expectedZone, $type->getZone(), 'Date time zone should match');
204204
$this->assertSame($expectedDeserializeFormat, $type->getDeserializeFormat(), 'Date time deserialize format should match');
205+
$this->assertSame($expectedDeserializeFormat ? [$expectedDeserializeFormat] : null, $type->getDeserializeFormats(), 'Date time deserialize format should match');
205206
}
206207

207208
public function testInvalidTypeWithParameters(): void

0 commit comments

Comments
 (0)