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

Support optional last param #25

Open
MarkoMackic opened this issue Oct 26, 2022 · 7 comments
Open

Support optional last param #25

MarkoMackic opened this issue Oct 26, 2022 · 7 comments
Labels
feature New feature or request

Comments

@MarkoMackic
Copy link

MarkoMackic commented Oct 26, 2022

Let's suppose you have common handler for REST type API on some resource,
like /companies/.

So usual request pattern is :

  • GET /companies(/?) - > get all companies
  • GET /companies/:id -> get specific company
  • PUT/POST/PATH/DELETE /companies/:id -> do something to specific company

Now all of the work can be done by the same handler , if it knows if :id is present.

I'm suggesting introduction of optional last param, e.g.
router.insert("/companies/:id?", whatever);

  • accepts route /companies/
  • accepts route /companies/some_id

router.insert("/companies/*catch_all?", whatever)

  • same as above for route without param
  • and general catchall behaviour is applied if param exists

Optional params should be supported only if they are in the route end ( which is obvious ) .

This would be easier than writing double insert in code that uses the library.

@ibraheemdev
Copy link
Owner

ibraheemdev commented Nov 9, 2022

I agree this would be useful to have, I would accept a pull request implementing this :)

@MarkoMackic
Copy link
Author

I'll see if I can do this :)

@cometkim
Copy link

I've run into a situation where I need an optional parameter, but not at the end.

I'm implementing the Docker registry. Docker image tags can be ubuntu or library/ubuntu. Therefore, the path the client requests will be in the form of /v2/:group?/:name/blobs/uploads.

Attempting to add each of the two routes will result in a conflict error.

@MarkoMackic
Copy link
Author

MarkoMackic commented Mar 28, 2023

Well you can't add optional params in the middle :) That's not how it works. It's not logical. For that case you'd have two routes.

@cometkim
Copy link

For that case you'd have two routes.

I tried. But I got a conflict error. is this a bug?

panicked at 'failed to register Post route for /v2/:name/blobs/uploads/ pattern: insertion failed due to conflict with previously registered route: /v2/:pathname/:name/blobs/uploads/'

@ibraheemdev
Copy link
Owner

That's #13, it would have to be /v2/:name/blobs/uploads/ and /v2/:name/:subname/blobs/uploads/ (the first parameter's name has to stay the same).

@ibraheemdev ibraheemdev added the feature New feature or request label May 23, 2023
@Bromles
Copy link

Bromles commented Aug 8, 2023

@cometkim I suggest taking a look at this issue #39
Docker Registry API V2 can contain more than 2 segments

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

No branches or pull requests

4 participants