Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Solution to Issue #751 #773

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

roy-kim-33
Copy link

@roy-kim-33 roy-kim-33 commented Oct 11, 2024

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Go Version Update
  • Dependency Update

Description

Address Issue #751 by using method mismatch flag in Route.Match()

The flag allows Route.Match() to return MethodNotAllowed (405) from an earlier mismatch instead of returning NotFound (404) from a later mismatch (refer to example from #751).

For nested endpoints,

r := mux.NewRouter()
subrouter := r.PathPrefix("/prefix").Subrouter()
subrouter.HandleFunc("/foo", fooHandler).Methods("POST")
subrouter.HandleFunc("/foo", fooIdHandler).Methods("GET").Queries("id", "{id:[0-9]+}")

The following results show some edge cases:
PUT /v1/foo?id=1 returns 405 (match second endpoint, desired behavior)
GET /v1/foo?id=abc returns 404 (match second endpoint, 400 on second endpoint preferred)
POST /v1/foo?id=abc returns 200 (match first endpoint, 405 on second endpoint might be preferred)
POST /v1/foo?id=1 returns 200 (match first endpoint, 405 on second endpoint might be preferred)
PUT /v1/foo returns 404 (match second endpoint, 405 on first endpoint preferred)
GET /v1/foo returns 404 (match second endpoint, 400 on first or 405 on second preferred)

These edge cases can be partially resolved(?) by declaring endpoints in descending order of specificity.

subrouter.HandleFunc("/foo", fooIdHandler).Methods("GET").Queries("id", "{id:[0-9]+}")
subrouter.HandleFunc("/foo", fooHandler).Methods("POST")

PUT /v1/foo?id=1 returns 405 (match first endpoint, desired behavior)
GET /v1/foo?id=abc returns 405 (match second endpoint, 400 on first endpoint preferred)
POST /v1/foo?id=abc returns 200 (match second endpoint, 405 on first endpoint might be preferred)
POST /v1/foo?id=1 returns 200 (match second endpoint, 405 on first endpoint might be preferred)
PUT /v1/foo returns 405 (match second endpoint, desired behavior)
GET /v1/foo returns 405 (match second endpoint, 400 on first endpoint might be preferred)

NOTE: might be is a soft suggestion

Related Tickets & Documents

  • Related Issue #
  • Closes #

Added/updated tests?

  • Yes
  • No, and this is why: existing test already tests this and passes, but, for an unknown reason, when using the package (by importing, not within the package), the test fails.
  • I need help with writing tests

Run verifications and test

  • make verify is passing
  • make test is passing

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

Successfully merging this pull request may close these issues.

1 participant