Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[HttpKernel] Add the UidValueResolver argument value resolver#44665

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
nicolas-grekas merged 1 commit intosymfony:6.1fromfancyweb:uid/arg-resolver
Feb 24, 2022

Conversation

@fancyweb
Copy link
Contributor

@fancywebfancyweb commentedDec 16, 2021
edited
Loading

QA
Branch?6.1
Bug fix?no
New feature?yes
Deprecations?no
Tickets#41690
LicenseMIT
Doc PR-

This feature adds an argument value resolver for UIDs.

Before:

#[Route(path:'/token/{token}')]publicfunction__invoke(string$token):Response{$token = UuidV4::fromRfc4122($token);// ...}

After:

#[Route(path:'/token/{token}')]publicfunction__invoke(UuidV4$token):Response{// ...}

By default, all formats are accepted because the resolver uses thefromString() method from the target UID class.

It's possible to restrict to one format or a combination of formats by using a route parameter requirement, for example:

#[Route(path:'/token/{token}', requirements: ['id' => Requirement::UUID])]publicfunction__invoke(UuidV4$token):Response{// ...}

chapterjason and ro0NL reacted with rocket emoji94noni reacted with eyes emoji
useSymfony\Component\Uid\UuidV4;
useSymfony\Component\Uid\UuidV6;

class UidController
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Check this file for different use cases.

@fancywebfancywebforce-pushed theuid/arg-resolver branch 2 times, most recently from0188adb to6712733CompareDecember 16, 2021 16:48
@aurimasniekis
Copy link
Contributor

Nice, I was just thinking of making something like this 👍 I am just trying to figure out now how is the best way to make route parameter requirements based on Uid

@fancywebfancywebforce-pushed theuid/arg-resolver branch 2 times, most recently fromb2c1b7f tofd22d35CompareDecember 17, 2021 08:23
@nicolas-grekas
Copy link
Member

I am just trying to figure out now how is the best way to make route parameter requirements based on Uid

That's a good question actually. If we figure out how to make this, we might be able to remove the proposedAsUid attribute and move the check to the router level.

Any idea how to do this?

@nicolas-grekas
Copy link
Member

Now I'm wondering: what about removing the "format" configuration setting and attribute, and instead accept only the canonical representation?
Eg here, if one prefers representing their UUID as base58, shouldn't they create a custom class that extends Uuid?
Adding config options is increasing complexity at all levels, we'd better be sure that this is needed before doing it.

@aurimasniekis
Copy link
Contributor

Any idea how to do this?

I haven't yet looked into the router, but I am wondering is there some kind of parser level for parameters before generating URL matchers? If so we could add some extension in that point which would mark that param as UID

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedDec 17, 2021
edited
Loading

We could define auid_base58 parameter and use{id<%uid_base58%>}. Or we could improve the routing configuration to add support for predefined requirements (aka aliased regexps).

If this looks like a useful path forward, we could then simplify this PR and make the resolver accept any format.

I think that's my current preference (and we could define theseuid_* parameters or do this improvement of the routing system in a separate PR.)

fancyweb and aurimasniekis reacted with heart emoji

@aurimasniekis
Copy link
Contributor

My previous implementation was basically like you mention parameter + argument resolver, but I was planning to look into something nicer

@fancywebfancywebforce-pushed theuid/arg-resolver branch 2 times, most recently from49c3f64 to4b61f8eCompareDecember 20, 2021 15:31
@fancywebfancyweb changed the title[Uid] Add the UidValueResolver argument value resolver[HttpKernel] Add the UidValueResolver argument value resolverFeb 23, 2022
@carsonbotcarsonbot changed the title[HttpKernel] Add the UidValueResolver argument value resolver[HttpKernel][Uid] Add the UidValueResolver argument value resolverFeb 23, 2022
@fancywebfancyweb removed the Uid labelFeb 23, 2022
@carsonbotcarsonbot changed the title[HttpKernel][Uid] Add the UidValueResolver argument value resolver[HttpKernel] Add the UidValueResolver argument value resolverFeb 23, 2022
@fancywebfancywebforce-pushed theuid/arg-resolver branch 2 times, most recently from11ef7d9 to785c6efCompareFebruary 23, 2022 16:16
@nicolas-grekas
Copy link
Member

Thank you@fancyweb.

@nicolas-grekasnicolas-grekas merged commit8a31425 intosymfony:6.1Feb 24, 2022
@fabpotfabpot mentioned this pull requestApr 15, 2022
fabpot added a commit that referenced this pull requestApr 15, 2022
…egular-expressions constants to use as route parameter requirements (fancyweb)This PR was merged into the 6.1 branch.Discussion----------[Routing] Add Requirement, a collection of universal regular-expressions constants to use as route parameter requirements| Q             | A| ------------- | ---| Branch?       | 6.1| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | -| License       | MIT| Doc PR        | -Ref#44665.We need a way to easily reference some universal complicated regexes in routes parameters requirements, for example:```php#[Route(path: '/users/{id}', requirements: ['id' => Requirement::UUID])]#[Route(path: '/users/{id<'.Requirement::UID_BASE58.'>}')]```What about having a collection in the routing component itself? I'm sure there are other common cases we would add here.Commits-------f49cb6a [Routing] Add Requirement, a collection of universal regular-expression constants to use as route parameter requirements
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+1 more reviewer

@ogizanagiogizanagiogizanagi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.1

Development

Successfully merging this pull request may close these issues.

5 participants

@fancyweb@aurimasniekis@nicolas-grekas@ogizanagi@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp