-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Allow a shortcut for UUID when defining requirements #18599
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
Conversation
I'm 👎 for this change. It looks like a "hack", it introduces a special use case (why Using UUIDs, like using autoincremental IDs, has some pros and some cons. I'm afraid this horrible regexp is one of the cons (the increase of the index sizes and the performance penalty are other cons too). |
Thank you for the feedback.
I would welcome other shortcuts as well.
There are 5 versions and they all have the same format. See https://en.wikipedia.org/wiki/Universally_unique_identifier#Variants_and_Versions
You should not replace the auto incremental IDs with UUID on your entities. You should use both. You should still use ID in your indexes etc but use the UUID in your URLs. Then you will have the best of two worlds. |
Just for your information, we have solved this issue with a parameter in the container: # config.yml
parameters:
uuid_regex: '[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}' and the usage in a controller: /**
* @Route(
* path="/{id}",
* requirements = {
* "id": "%uuid_regex%"
* }
* )
* |
I did the same than @lyrixx and I also vote -1 about adding such hack in Symfony |
In fact, it will be a nice example to add it in the doc |
I agree that a doc example might be nice here. @javiereguiluz you can use the doctrine paramconverter to achieve this exact behavior (transforming the uuid into an entity), which will provide a more generic solution. It would of course be possible to make an argument resolver for a specific Object, but this would only be deemed valuable if you have additional logic in the resolving (security perhaps). |
@iltar we are not talking about converting the UUID to an entity in this issue. We are talking about putting a routing requirement for a UUID. |
@stof if it's about matching, you can throw an exception when you cannot find an entity matching the UUID given, that's what I mean. Matching routes or throwing exceptions is a slightly different thing though. |
@iltar we are not talking about matching that the UUID has an entity. We are talking about matching the URL only when the placeholder is a UUID (instead of having to accept any string, which can create issue by shadowing another route). Defining the regex matched by the placeholder is part of the route matching. |
@iltar More over, when you are using postgres, if the type is |
In that case the |
You will also get the exception when using Doctrine and MySql when you use the Ramsey uuid-doctrine lib. |
Thank you for all the feedback. I'll go back to the original PR at the FrameworkExtraBundle. I really like @lyrixx solution. I did not know you could use parameters in the |
Related to sensiolabs/SensioFrameworkExtraBundle#412
I use an uuid (v4) in my URL to identify an entity (like you always should). This is a recommended security measure if you compare with just using an auto incremental numeric id (which is standard for a Doctrine/Mysql setup). I want of course make sure that my routes are properly defined and that I have a
requirement
on all my variables. So I write something like this:The regex for the uuid is quite long and horrible to write/remember. This PR allows you to write a shortcut for that ugly regex. So instead you would write:
This will also work in Yaml, XML and PHP.
BC break?
This is not a BC break because nobody will have the string
\uuid
as an requirement. If you would write\uuid
or\uxxx
in the requirements you would get an error like: