Skip to content

Commit 81ac5a2

Browse files
Brajk19HypeMC
Brajk19
authored and
HypeMC
committed
fix usage of nullsafe operator with collection accessor
1 parent 688668f commit 81ac5a2

3 files changed

Lines changed: 142 additions & 9 deletions

File tree

‎src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Symfony\Component\PropertyAccess\Exception\AccessException;
99
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
1010
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
11+
use Symfony\Component\PropertyAccess\PropertyPath;
1112
use Symfony\Component\PropertyAccess\PropertyPathInterface;
1213

1314
/**
@@ -34,10 +35,19 @@ public function getValue(object|array $objectOrArray, string|PropertyPathInterfa
3435
/** @var array{0: string, 1: string} $propertyPathParts */
3536
$propertyPathParts = explode(separator: self::DELIMITER, string: (string) $propertyPath, limit: 2);
3637

37-
$collection = $this->propertyAccessor->getValue($objectOrArray, $propertyPathParts[0]);
38+
$basePropertyPath = new PropertyPath($propertyPathParts[0]);
39+
$collection = $this->propertyAccessor->getValue($objectOrArray, $basePropertyPath);
3840

3941
if (!is_iterable($collection)) {
40-
throw new ValueNotIterableException($collection, $propertyPathParts[0]);
42+
// Honor the null-safe operator: when the collection segment resolves to null because of
43+
// a null-safe short-circuit (e.g. "property?.collection[*].id" with a null "property"),
44+
// yield no values instead of throwing. A null collection not guarded by "?."
45+
// (e.g. "property.collection[*].id" where "collection" itself is null) is still an error.
46+
if (null === $collection && $this->isNullSafeCollection($objectOrArray, $basePropertyPath)) {
47+
return null;
48+
}
49+
50+
throw new ValueNotIterableException($collection, (string) $basePropertyPath);
4151
}
4252

4353
$values = [];
@@ -87,8 +97,25 @@ public function isReadable(object|array $objectOrArray, string|PropertyPathInter
8797
$this->getValue($objectOrArray, $propertyPath);
8898

8999
return true;
90-
} catch (AccessException|UnexpectedTypeException) {
100+
} catch (AccessException|UnexpectedTypeException|ValueNotIterableException) {
91101
return false;
92102
}
93103
}
104+
105+
/**
106+
* Determines whether a null collection segment is the legitimate result of a null-safe ("?.")
107+
* short-circuit, rather than the collection itself resolving to null.
108+
*
109+
* @param object|array<array-key, mixed> $objectOrArray
110+
*/
111+
private function isNullSafeCollection(object|array $objectOrArray, PropertyPath $path): bool
112+
{
113+
if ($path->isNullSafe($path->getLength() - 1)) {
114+
return true;
115+
}
116+
117+
$parent = $path->getParent();
118+
119+
return null !== $parent && null === $this->propertyAccessor->getValue($objectOrArray, $parent);
120+
}
94121
}

‎tests/RouteProvider/PropertyAccess/Fixtures/Foo.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ class Foo
1111
public function __construct(
1212
public readonly int $id,
1313
public readonly Collection $children,
14+
public readonly ?self $linked = null,
15+
public readonly ?Collection $nullableChildren = null,
1416
) {
1517
}
1618

‎tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php‎

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,17 +130,121 @@ public static function traversableProvider(): iterable
130130
];
131131
}
132132

133-
public function testNotTraversable(): void
133+
#[DataProvider('nullSafeProvider')]
134+
public function testNullSafeCollectionPath(object $object, string $propertyPath, mixed $expectedResult): void
134135
{
136+
self::assertTrue($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath));
137+
self::assertSame(
138+
expected: $expectedResult,
139+
actual: $this->purgatoryPropertyAccessor->getValue($object, $propertyPath),
140+
);
141+
}
142+
143+
public static function nullSafeProvider(): iterable
144+
{
145+
yield 'null-safe parent short-circuits to null' => [
146+
'object' => new Foo(
147+
id: 100,
148+
children: new ArrayCollection([]),
149+
linked: null,
150+
),
151+
'propertyPath' => 'linked?.children[*].id',
152+
'expectedResult' => null,
153+
];
154+
155+
yield 'null-safe parent present with populated collection yields values' => [
156+
'object' => new Foo(
157+
id: 100,
158+
children: new ArrayCollection([]),
159+
linked: new Foo(
160+
id: 1,
161+
children: new ArrayCollection([
162+
new Foo(id: 10, children: new ArrayCollection([])),
163+
new Foo(id: 11, children: new ArrayCollection([])),
164+
]),
165+
),
166+
),
167+
'propertyPath' => 'linked?.children[*].id',
168+
'expectedResult' => [10, 11],
169+
];
170+
171+
yield 'null-safe collection element resolving to null short-circuits to null' => [
172+
'object' => new Foo(
173+
id: 100,
174+
children: new ArrayCollection([]),
175+
nullableChildren: null,
176+
),
177+
'propertyPath' => 'nullableChildren?[*].id',
178+
'expectedResult' => null,
179+
];
180+
181+
yield 'nested null-safe short-circuit contributes a null entry, consistent with a scalar leaf' => [
182+
'object' => new Foo(
183+
id: 0,
184+
children: new ArrayCollection([
185+
new Foo(id: 1, children: new ArrayCollection([]), linked: null),
186+
new Foo(
187+
id: 2,
188+
children: new ArrayCollection([]),
189+
linked: new Foo(
190+
id: 20,
191+
children: new ArrayCollection([
192+
new Foo(id: 100, children: new ArrayCollection([])),
193+
new Foo(id: 101, children: new ArrayCollection([])),
194+
]),
195+
),
196+
),
197+
]),
198+
),
199+
'propertyPath' => 'children[*].linked?.children[*].id',
200+
'expectedResult' => [null, 100, 101],
201+
];
202+
}
203+
204+
#[DataProvider('notTraversableProvider')]
205+
public function testNotTraversableThrows(object $object, string $propertyPath, string $expectedMessage): void
206+
{
207+
self::assertFalse($this->purgatoryPropertyAccessor->isReadable($object, $propertyPath));
208+
135209
$this->expectException(ValueNotIterableException::class);
136-
$this->expectExceptionMessage('Expected an iterable, "int" given at property path "id[*]".');
210+
$this->expectExceptionMessage($expectedMessage);
137211

138-
$this->purgatoryPropertyAccessor->getValue(
139-
objectOrArray: new Foo(
212+
$this->purgatoryPropertyAccessor->getValue($object, $propertyPath);
213+
}
214+
215+
public static function notTraversableProvider(): iterable
216+
{
217+
yield 'scalar value is not iterable' => [
218+
'object' => new Foo(
140219
id: 1,
141220
children: new ArrayCollection([]),
142221
),
143-
propertyPath: 'id[*].id',
144-
);
222+
'propertyPath' => 'id[*].id',
223+
'expectedMessage' => 'Expected an iterable, "int" given at property path "id[*]".',
224+
];
225+
226+
yield 'non-null-safe collection resolving to null' => [
227+
'object' => new Foo(
228+
id: 100,
229+
children: new ArrayCollection([]),
230+
nullableChildren: null,
231+
),
232+
'propertyPath' => 'nullableChildren[*].id',
233+
'expectedMessage' => 'Expected an iterable, "null" given at property path "nullableChildren[*]".',
234+
];
235+
236+
yield 'null-safe parent present but non-null-safe collection resolving to null' => [
237+
'object' => new Foo(
238+
id: 100,
239+
children: new ArrayCollection([]),
240+
linked: new Foo(
241+
id: 1,
242+
children: new ArrayCollection([]),
243+
nullableChildren: null,
244+
),
245+
),
246+
'propertyPath' => 'linked?.nullableChildren[*].id',
247+
'expectedMessage' => 'Expected an iterable, "null" given at property path "linked?.nullableChildren[*]".',
248+
];
145249
}
146250
}

0 commit comments

Comments
 (0)