Skip to content

Errors from Symfony's PropertyAccessor now result in Purgatory's PropertyNotAccessibleException #110

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
klezaic wants to merge 9 commits into
base: 2.x
Choose a base branch
from
Open

Errors from Symfony's PropertyAccessor now result in Purgatory's PropertyNotAccessibleException #110

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
91ab85c
Errors from symfony property accessor now result in PropertyNotAccess…
klezaic Jul 1, 2025
3ba9030
Static code analysis fix
klezaic Jul 1, 2025
e427672
Lint
klezaic Jul 1, 2025
3dfd226
Static code analysis fix
klezaic Jul 1, 2025
a9a5ba3
Improve test coverage
klezaic Jul 1, 2025
f931880
Improve test coverage
klezaic Jul 1, 2025
ca27025
Improve test coverage
klezaic Jul 1, 2025
7af442d
Added previous throwable to exception
klezaic Aug 27, 2025
76eca5b
Previous should be optional
klezaic Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions src/RouteProvider/PropertyAccess/PurgatoryPropertyAccessor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Sofascore\PurgatoryBundle\RouteProvider\PropertyAccess;

use Sofascore\PurgatoryBundle\Exception\PropertyNotAccessibleException;
use Sofascore\PurgatoryBundle\Exception\ValueNotIterableException;
use Symfony\Component\PropertyAccess\Exception\AccessException;
use Symfony\Component\PropertyAccess\Exception\UnexpectedTypeException;
Expand All @@ -25,17 +26,34 @@ public function __construct(
/**
* @param object|array<array-key, mixed> $objectOrArray
* @param string|PropertyPathInterface $propertyPath
*
* @throws PropertyNotAccessibleException
* @throws ValueNotIterableException
*/
public function getValue($objectOrArray, $propertyPath): mixed
{
if (!str_contains((string) $propertyPath, self::DELIMITER)) {
return $this->propertyAccessor->getValue($objectOrArray, $propertyPath);
try {
return $this->propertyAccessor->getValue($objectOrArray, $propertyPath);
} catch (\InvalidArgumentException|AccessException|UnexpectedTypeException) {
throw new PropertyNotAccessibleException(
\is_array($objectOrArray) ? 'array' : $objectOrArray::class,
(string) $propertyPath,
);
Copy link
Member

@Brajk19 Brajk19 Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably add previous throwable to PropertyNotAccessibleException so we don't lose error context from property accessor

Copy link
Author

@klezaic klezaic Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added in 7af442d

}
}

/** @var array{0: string, 1: string} $propertyPathParts */
$propertyPathParts = explode(separator: self::DELIMITER, string: (string) $propertyPath, limit: 2);

$collection = $this->propertyAccessor->getValue($objectOrArray, $propertyPathParts[0]);
try {
$collection = $this->propertyAccessor->getValue($objectOrArray, $propertyPathParts[0]);
} catch (\InvalidArgumentException|AccessException|UnexpectedTypeException) {
throw new PropertyNotAccessibleException(
\is_array($objectOrArray) ? 'array' : $objectOrArray::class,
$propertyPathParts[0],
);
}

if (!is_iterable($collection)) {
throw new ValueNotIterableException($collection, $propertyPathParts[0]);
Expand All @@ -60,6 +78,8 @@ public function getValue($objectOrArray, $propertyPath): mixed
/**
* @param object|array<array-key, mixed> $objectOrArray
* @param string|PropertyPathInterface $propertyPath
*
* @param-out object|array<array-key, mixed> $objectOrArray
*/
public function setValue(&$objectOrArray, $propertyPath, mixed $value): void
{
Expand Down Expand Up @@ -89,7 +109,7 @@ public function isReadable($objectOrArray, $propertyPath): bool
$this->getValue($objectOrArray, $propertyPath);

return true;
} catch (AccessException|UnexpectedTypeException) {
} catch (PropertyNotAccessibleException) {
return false;
}
}
Expand Down
1 change: 1 addition & 0 deletions tests/RouteProvider/PropertyAccess/Fixtures/Foo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class Foo
public function __construct(
public readonly int $id,
public readonly Collection $children,
private readonly ?string $privateProperty = 'value',
) {
}

Expand Down
189 changes: 189 additions & 0 deletions tests/RouteProvider/PropertyAccess/PurgatoryPropertyAccessorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
use Sofascore\PurgatoryBundle\Exception\PropertyNotAccessibleException;
use Sofascore\PurgatoryBundle\Exception\ValueNotIterableException;
use Sofascore\PurgatoryBundle\RouteProvider\PropertyAccess\PurgatoryPropertyAccessor;
use Sofascore\PurgatoryBundle\Tests\RouteProvider\PropertyAccess\Fixtures\Foo;
Expand Down Expand Up @@ -130,6 +131,60 @@ public static function traversableProvider(): iterable
];
}

public function testReadPathPropertyNotExist(): void
{
self::assertFalse($this->purgatoryPropertyAccessor->isReadable(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'nonExistentProperty',
));
}

public function testReadPathPropertyNotAccessible(): void
{
self::assertFalse($this->purgatoryPropertyAccessor->isReadable(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'privateProperty',
));
}

public function testReadTraversableChildPropertyNotExist(): void
{
self::assertFalse($this->purgatoryPropertyAccessor->isReadable(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([
new Foo(
id: 1,
children: new ArrayCollection([]),
),
]),
),
propertyPath: 'children[*].nonExistentProperty',
));
}

public function testReadTraversableChildPropertyNotAccessible(): void
{
self::assertFalse($this->purgatoryPropertyAccessor->isReadable(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([
new Foo(
id: 1,
children: new ArrayCollection([]),
),
]),
),
propertyPath: 'children[*].privateProperty',
));
}

public function testNotTraversable(): void
{
$this->expectException(ValueNotIterableException::class);
Expand All @@ -143,4 +198,138 @@ public function testNotTraversable(): void
propertyPath: 'id[*].id',
);
}

public function testPropertyNotAccessible(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Copy link
Member

@Brajk19 Brajk19 Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure about this message, it fits the context it was used for previously, but i don't think is good for errors with property accessor.
maybe we should change the message? @HypeMC

Foo::class,
'privateProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'privateProperty',
);
}

public function testTraversablePropertyNotAccessible(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Foo::class,
'privateProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'privateProperty[*].values',
);
}

public function testTraversableChildPropertyNotAccessible(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Foo::class,
'privateProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection(
[
new Foo(
id: 1,
children: new ArrayCollection([]),
),
],
),
),
propertyPath: 'children[*].privateProperty',
);
}

public function testPropertyNotExist(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Foo::class,
'nonExistentProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'nonExistentProperty',
);
}

public function testTraversablePropertyNotExist(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Foo::class,
'nonExistentProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection([]),
),
propertyPath: 'nonExistentProperty[*].values',
);
}

public function testTraversableChildPropertyNotExist(): void
{
$this->expectException(PropertyNotAccessibleException::class);
$this->expectExceptionMessage(
\sprintf(
'Unable to create a getter for property "%s::%s".',
Foo::class,
'nonExistentProperty',
),
);

$this->purgatoryPropertyAccessor->getValue(
objectOrArray: new Foo(
id: 1,
children: new ArrayCollection(
[
new Foo(
id: 1,
children: new ArrayCollection([]),
),
],
),
),
propertyPath: 'children[*].nonExistentProperty',
);
}
}