- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 936
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.
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
base: main
Are you sure you want to change the base?
Conversation
         VincentLanglet
  
      
      
      commented
      
            VincentLanglet
  
      
      
      commented
        Oct 22, 2025 
      
    
  
| Q | A | 
|---|---|
| Branch? | main | 
| Tickets | Closes #7476 | 
| License | MIT | 
| Doc PR | api-platform/docs#... | 
| Test are failing because of this line 
 
 I feel like the error is still legit because the resource has which means that  But it still make me think I cannot add automatically a requirement if a Provider is used. | 
840e975    to
    76d0e52      
    Compare
  
    | if (null === $operation->getProvider()) { | ||
| $operation = $operation->withProvider($this->getProvider($operation)); | ||
|  | ||
| if ($operation instanceof HttpOperation) { | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if ($operation instanceof HttpOperation) { | |
| if ($operation instanceof HttpOperation && null === $operation->getRequirements()) { | 
| } | ||
| } | ||
|  | ||
| return $requirements; | 
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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