Skip to content
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

Set defaults on a per class basis #1

Open
sorenmalling opened this issue Mar 20, 2023 · 10 comments
Open

Set defaults on a per class basis #1

sorenmalling opened this issue Mar 20, 2023 · 10 comments

Comments

@sorenmalling
Copy link
Contributor

How about we introduce a Defaults annotation kind of like this

#[Route\Defaults(prefix: 'uri-prefix', format: 'json')
@mhsdesign
Copy link

mhsdesign commented Mar 20, 2023

i dont really understand but like:

#[Route\Defaults(prefix: 'uri-prefix', format: 'json')
class MyController
{
   #[Route('lol', method: "GET")
   public function lolAction()
   {}
}

would result in uri-prefix/lol ?

@bwaidelich
Copy link

Why not re-use the existing names from the routing config (it's not super nice, but at least that would make it consistent), i.e.:

class MyController
{
   #[Route(uriPatter: "foo", defaults: ['@format': 'json'], httpMethods: ['GET'])
   public function lolAction()
   {}
}

@sorenmalling
Copy link
Contributor Author

Why not re-use the existing names from the routing config (it's not super nice, but at least that would make it consistent), i.e.:

My attempt to make config key more simple. format is a part of default - but we read all other defaults from the position of the annotation. So, defaults: {@format: ..} can simply be a format property :)

@sorenmalling
Copy link
Contributor Author

i dont really understand but like:

#[Route\Defaults(prefix: 'uri-prefix', format: 'json')
class MyController
{
   #[Route('lol', method: "GET")
   public function lolAction()
   {}
}

would result in uri-prefix/lol ?

Exactly, routes for this class all have contacts as a "prefix" - son instead of repeating yourself in each annotation, we can "prefix" the class. It's possible now by setting a Route('prefix') on the class - this is about making single purpose annotations

@mhsdesign
Copy link

yes id consider it more clean to have a separate annotation for this than to reuse the existing one though i dont like your proposed name ^^ naming things i know ...

@lorenzulrich
Copy link

I like prefix very much, because currently I must repeat api/v1 everywhere. This way I can just put it to the AbstractApiController.

@lorenzulrich
Copy link

However, the question if method(s) should be an array is valid. With the current approach, I think an action can only be used with one method, while it can make sense to have PUT and POST in the same action.

@sorenmalling
Copy link
Contributor Author

dont like your proposed name ^^ naming things i know ...

@mhsdesign

Route Defaults - the idea from the defaults configuration of the Routes.yaml, but with keys that add values to defaults in this new context

#Route\Defaults(prefix: 'api/v1', format: 'json')

Say it out load: Route Defaults is prefix api/v1 and format json

and on the method

#Route\Post(path: 'contacts')

Route is POST contacts

Together it will be be

Route is POST api/v1/contacts format json

@sorenmalling
Copy link
Contributor Author

And for @lorenzulrich example:

on the class

#Route\Defaults(prefix: 'api/v1', format: 'json')

and on the method

#Route\Post(path: 'contacts', method: {'GET','POST'})

Route is GET and POST contacts

Together it will be be

Route is GET and POST api/v1/contacts format json

@mficzel
Copy link

mficzel commented Mar 23, 2023

I would like to see the first implementation of annotation routes be close to the yaml configuration and not introduce other new features yet.

We may want that later after actually working with route annotations. If it can be treated as a seperate feature it probably should.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants