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

feat(rest): add strict option for routers #2419

Merged
merged 1 commit into from
Mar 11, 2019

Conversation

raymondfeng
Copy link
Contributor

@raymondfeng raymondfeng commented Feb 18, 2019

This PR replaces #2355.

It introduces strict option for REST routers to deal with trailing slashes.

  1. strict is true:
  • request /orders matches route /orders but not /orders/
  • request /orders/ matches route /orders/ but not /orders
  1. strict is false (default)
  • request /orders matches route /orders first and falls back to /orders
  • request /orders/ matches route /orders/ first and falls back to /orders

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@raymondfeng raymondfeng requested a review from bajtos as a code owner February 18, 2019 18:02
@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 8bdfa7c to 0482aa2 Compare February 18, 2019 21:58
@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch 3 times, most recently from 3dee3fa to 460bb00 Compare February 27, 2019 16:56
@bajtos bajtos added feature REST Issues related to @loopback/rest package and REST transport in general labels Mar 5, 2019
@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 460bb00 to 0c6d48d Compare March 5, 2019 17:46
@raymondfeng
Copy link
Contributor Author

@hacksparrow Please take a look.

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

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

I don't have bandwidth to review the changes in full detail. I don't see any obvious major problems, and since existing tests are passing, I guess this change is safe enough.

Please wait for @hacksparrow's review & approval before landing.

packages/rest/src/router/rest-router.ts Outdated Show resolved Hide resolved
@bajtos
Copy link
Member

bajtos commented Mar 7, 2019

Documentation in /docs/site was updated

Should we describe this new option our documentation too?

@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 0c6d48d to 0ae7f44 Compare March 7, 2019 15:41
@raymondfeng
Copy link
Contributor Author

@hacksparrow Please review.

@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 0ae7f44 to 6af71de Compare March 7, 2019 15:49
@raymondfeng
Copy link
Contributor Author

Should we describe this new option our documentation too?

Added.

@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 6af71de to 3794f43 Compare March 7, 2019 15:51
docs/site/Server.md Outdated Show resolved Hide resolved
@raymondfeng raymondfeng force-pushed the rest-router-strict-option branch from 3794f43 to 3725913 Compare March 11, 2019 15:14
@raymondfeng raymondfeng requested a review from hacksparrow March 11, 2019 15:14
@raymondfeng raymondfeng merged commit c3c5dab into master Mar 11, 2019
@raymondfeng raymondfeng deleted the rest-router-strict-option branch March 11, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature REST Issues related to @loopback/rest package and REST transport in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants