Skip to content

feat(doctrine): add requirements dynamically based on doctrine field #7477

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
wants to merge 2 commits into
base: main
Choose a base branch
from
+143 −9
Open

feat(doctrine): add requirements dynamically based on doctrine field #7477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a22ba9
feat(doctrine): add requirements
VincentLanglet Oct 22, 2025
76d0e52
fix(doctrine): resource definition
VincentLanglet Oct 23, 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
55 changes: 55 additions & 0 deletions src/Doctrine/Odm/Metadata/Resource/DoctrineMongoDbOdmResourceCollectionMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@
use ApiPlatform\Doctrine\Odm\State\Options;
use ApiPlatform\Metadata\CollectionOperationInterface;
use ApiPlatform\Metadata\DeleteOperationInterface;
use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
use ApiPlatform\State\Util\StateOptionsTrait;
use Doctrine\ODM\MongoDB\DocumentManager;
use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
use Doctrine\Persistence\ManagerRegistry;

final class DoctrineMongoDbOdmResourceCollectionMetadataFactory implements ResourceMetadataCollectionFactoryInterface
Expand Down Expand Up @@ -86,6 +89,10 @@ private function addDefaults(Operation $operation): Operation

if (null === $operation->getProvider()) {
$operation = $operation->withProvider($this->getProvider($operation));

if ($operation instanceof HttpOperation) {
Copy link
Member

@soyuka soyuka Oct 24, 2025

Choose a reason for hiding this comment

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

Suggested change
if ($operation instanceof HttpOperation) {
if ($operation instanceof HttpOperation && null === $operation->getRequirements()) {

$operation = $operation->withRequirements($this->getRequirements($operation));
}
}

if (null === $operation->getProcessor()) {
Expand All @@ -95,6 +102,54 @@ private function addDefaults(Operation $operation): Operation
return $operation;
}

/**
* @return array<string, string>
*/
private function getRequirements(HttpOperation $operation): array
{
$requirements = $operation->getRequirements() ?? [];
$uriVariables = (array) ($operation->getUriVariables() ?? []);

foreach ($uriVariables as $paramName => $uriVariable) {
if (isset($requirements[$paramName])) {
continue;
}

if (!$uriVariable instanceof Link) {
continue;
}

$identifiers = $uriVariable->getIdentifiers();
if (1 !== \count($identifiers)) {
continue;
}
$fieldName = $identifiers[0];

$fromClass = $uriVariable->getFromClass();
if (null === $fromClass) {
continue;
}
$classMetadata = $this->managerRegistry->getManagerForClass($fromClass)?->getClassMetadata($fromClass);

$requirement = null;
if ($classMetadata instanceof ClassMetadata && $classMetadata->hasField($fieldName)) {
$fieldMapping = $classMetadata->getFieldMapping($fieldName);
$requirement = match ($fieldMapping['type']) {
'uuid', 'guid' => '^[0-9a-fA-F]{8}(?:-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}$',
'ulid' => '^[0-7][0-9a-hjkmnp-tv-zA-HJKMNP-TV-Z]{25}$',
'smallint', 'integer', 'bigint' => '^-?[0-9]+$',
default => null,
};
}

if (null !== $requirement) {
$requirements[$paramName] = $requirement;
}
}

return $requirements;
Copy link
Member

@soyuka soyuka Oct 24, 2025

Choose a reason for hiding this comment

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

Not sure that this should be enabeld like this, especially with regular expression. I largely prefer using validation constraints (that are now enabled on query parameters and that we should activate on uri variables).

IMO this should likely be opt-in.

Copy link
Contributor Author

@VincentLanglet VincentLanglet Oct 24, 2025

Choose a reason for hiding this comment

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

I'll give you my use case (which is maybe wrong).

When let's say I declare a Get() route on project resource, without specifying the url.
By default ApiPlatform will generate project/{id}

Let's say I also declare manually a GET project/settings url, this will conflict and ApiPlatform will try to resolve setting as an id even if

  • It's not digit (if I use integer ids)
  • It's not a uuid (if I use uuids)

By adding requirements I solve the conflict. What would recommend you instead ?

When people are defining manually they template we could consider it's legit to ask them to add the requirement too. But when it's api platform which auto-generate the url, it's an issue.

Copy link
Member

@soyuka soyuka Oct 31, 2025

Choose a reason for hiding this comment

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

can you not set the requirements yourself?

Copy link
Contributor Author

@VincentLanglet VincentLanglet Oct 31, 2025

Choose a reason for hiding this comment

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

Sure I can do, and will do (manually or with a RequirementResourceMetadataCollectionFactory). But it felt "weird" to me to have a definition like

new Get(
     requirements: ['uuid' => '...']
);

when the route is not explicitly given. And I wanted to avoid to redefine

new Get(
     uriTemplate: 'foo/{uuid}',
     requirements: ['uuid' => '...']
);

since it's already the default uriTemplate.

But, not a big deal

}

private function getProvider(Operation $operation): string
{
if ($operation instanceof CollectionOperationInterface) {
Expand Down
61 changes: 61 additions & 0 deletions src/Doctrine/Orm/Metadata/Resource/DoctrineOrmResourceCollectionMetadataFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@
use ApiPlatform\Doctrine\Orm\State\Options;
use ApiPlatform\Metadata\CollectionOperationInterface;
use ApiPlatform\Metadata\DeleteOperationInterface;
use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
use ApiPlatform\State\Util\StateOptionsTrait;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadata;
use Doctrine\ORM\Mapping\FieldMapping;
use Doctrine\Persistence\ManagerRegistry;

final class DoctrineOrmResourceCollectionMetadataFactory implements ResourceMetadataCollectionFactoryInterface
Expand Down Expand Up @@ -83,6 +87,10 @@ private function addDefaults(Operation $operation): Operation
{
if (null === $operation->getProvider()) {
$operation = $operation->withProvider($this->getProvider($operation));

if ($operation instanceof HttpOperation) {
$operation = $operation->withRequirements($this->getRequirements($operation));
}
}

$options = $operation->getStateOptions() ?: new Options();
Expand All @@ -98,6 +106,59 @@ private function addDefaults(Operation $operation): Operation
return $operation;
}

/**
* @return array<string, string>
*/
private function getRequirements(HttpOperation $operation): array
{
$requirements = $operation->getRequirements() ?? [];
$uriVariables = (array) ($operation->getUriVariables() ?? []);

foreach ($uriVariables as $paramName => $uriVariable) {
if (isset($requirements[$paramName])) {
continue;
}

if (!$uriVariable instanceof Link) {
continue;
}
$identifiers = $uriVariable->getIdentifiers();
if (1 !== \count($identifiers)) {
continue;
}
$fieldName = $identifiers[0];

$fromClass = $uriVariable->getFromClass();
if (null === $fromClass) {
continue;
}
$classMetadata = $this->managerRegistry->getManagerForClass($fromClass)?->getClassMetadata($fromClass);

$requirement = null;
if ($classMetadata instanceof ClassMetadata && $classMetadata->hasField($fieldName)) {
$fieldMapping = $classMetadata->getFieldMapping($fieldName);
if (class_exists(FieldMapping::class)) {
$type = $fieldMapping->type;
} else {
$type = $fieldMapping['type'];
}

$requirement = match ($type) {
'uuid', 'guid' => '^[0-9a-fA-F]{8}(?:-[0-9a-fA-F]{4}){3}-[0-9a-fA-F]{12}$',
'ulid' => '^[0-7][0-9a-hjkmnp-tv-zA-HJKMNP-TV-Z]{25}$',
'smallint', 'integer', 'bigint' => '^-?[0-9]+$',
default => null,
};
}

if (null !== $requirement) {
$requirements[$paramName] = $requirement;
}
}

return $requirements;
}

private function getProvider(Operation $operation): string
{
if ($operation instanceof CollectionOperationInterface) {
Expand Down
16 changes: 16 additions & 0 deletions src/Metadata/Link.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::TARGET_PARAMETER)]
final class Link extends Parameter
{
/**
* @param class-string|null $fromClass
* @param class-string|null $toClass
*/
public function __construct(
private ?string $parameterName = null,
private ?string $fromProperty = null,
Expand Down Expand Up @@ -87,11 +91,17 @@ public function withParameterName(string $parameterName): self
return $self;
}

/**
* @return class-string|null
*/
public function getFromClass(): ?string
{
return $this->fromClass;
}

/**
* @param class-string $fromClass
*/
public function withFromClass(string $fromClass): self
{
$self = clone $this;
Expand All @@ -100,11 +110,17 @@ public function withFromClass(string $fromClass): self
return $self;
}

/**
* @return class-string|null
*/
public function getToClass(): ?string
{
return $this->toClass;
}

/**
* @param class-string $toClass
*/
public function withToClass(string $toClass): self
{
$self = clone $this;
Expand Down
12 changes: 6 additions & 6 deletions src/State/Tests/Provider/SecurityParameterProviderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public function testIsGrantedLink(): void
$obj = new \stdClass();
$barObj = new \stdClass();
$operation = new GetCollection(uriVariables: [
'barId' => new Link(toProperty: 'bar', fromClass: 'Bar', security: 'is_granted("some_voter", "bar")'),
'barId' => new Link(toProperty: 'bar', fromClass: $barObj::class, security: 'is_granted("some_voter", "bar")'),
], class: \stdClass::class);
$decorated = $this->createMock(ProviderInterface::class);
$decorated->method('provide')->willReturn($obj);
Expand All @@ -39,7 +39,7 @@ public function testIsGrantedLink(): void
$request->attributes = $parameterBag;
$request->attributes->set('bar', $barObj);
$resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with('Bar', 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(true);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with($barObj::class, 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(true);
$accessChecker = new SecurityParameterProvider($decorated, $resourceAccessChecker);
$accessChecker->provide($operation, ['barId' => 1], ['request' => $request]);
}
Expand All @@ -51,7 +51,7 @@ public function testIsNotGrantedLink(): void
$obj = new \stdClass();
$barObj = new \stdClass();
$operation = new GetCollection(uriVariables: [
'barId' => new Link(toProperty: 'bar', fromClass: 'Bar', security: 'is_granted("some_voter", "bar")'),
'barId' => new Link(toProperty: 'bar', fromClass: $barObj::class, security: 'is_granted("some_voter", "bar")'),
], class: \stdClass::class);
$decorated = $this->createMock(ProviderInterface::class);
$decorated->method('provide')->willReturn($obj);
Expand All @@ -60,7 +60,7 @@ public function testIsNotGrantedLink(): void
$request->attributes = $parameterBag;
$request->attributes->set('bar', $barObj);
$resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with('Bar', 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(false);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with($barObj::class, 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(false);
$accessChecker = new SecurityParameterProvider($decorated, $resourceAccessChecker);
$accessChecker->provide($operation, ['barId' => 1], ['request' => $request]);
}
Expand All @@ -73,7 +73,7 @@ public function testSecurityMessageLink(): void
$obj = new \stdClass();
$barObj = new \stdClass();
$operation = new GetCollection(uriVariables: [
'barId' => new Link(toProperty: 'bar', fromClass: 'Bar', security: 'is_granted("some_voter", "bar")', securityMessage: 'You are not admin.'),
'barId' => new Link(toProperty: 'bar', fromClass: $barObj::class, security: 'is_granted("some_voter", "bar")', securityMessage: 'You are not admin.'),
], class: \stdClass::class);
$decorated = $this->createMock(ProviderInterface::class);
$decorated->method('provide')->willReturn($obj);
Expand All @@ -82,7 +82,7 @@ public function testSecurityMessageLink(): void
$request->attributes = $parameterBag;
$request->attributes->set('bar', $barObj);
$resourceAccessChecker = $this->createMock(ResourceAccessCheckerInterface::class);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with('Bar', 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(false);
$resourceAccessChecker->expects($this->once())->method('isGranted')->with($barObj::class, 'is_granted("some_voter", "bar")', ['object' => $obj, 'previous_object' => null, 'request' => $request, 'bar' => $barObj, 'barId' => 1, 'operation' => $operation])->willReturn(false);
$accessChecker = new SecurityParameterProvider($decorated, $resourceAccessChecker);
$accessChecker->provide($operation, ['barId' => 1], ['request' => $request]);
}
Expand Down
6 changes: 4 additions & 2 deletions tests/Fixtures/TestBundle/Entity/ContainNonResource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
#[ORM\Entity]
class ContainNonResource
{
#[ORM\Column(type: 'integer')]
/**
* @var string
*/
#[ORM\Column(type: 'string')]
#[ORM\Id]
#[ORM\GeneratedValue(strategy: 'AUTO')]
#[Groups('contain_non_resource')]
public $id;
/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Fixtures/TestBundle/State/ContainNonResourceProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public function provide(Operation $operation, array $uriVariables = [], array $c
$resourceClass = $operation->getClass();
// Retrieve the blog post item from somewhere
$cnr = new $resourceClass();
$cnr->id = $id;
$cnr->id = (string) $id;
$cnr->notAResource = new NotAResource('f1', 'b1');
$cnr->nested = new $resourceClass();
$cnr->nested->id = "$id-nested";
Expand Down
Loading