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

airframe-http: #1128 Add Router.verifyRoutes to check duplicated endpoints #1137

Merged
merged 2 commits into from
Jun 18, 2020

Conversation

xerial
Copy link
Member

@xerial xerial commented Jun 15, 2020

Because of the complexity of mapping RPC functions to HTTP endpoints, we should avoid using method overload (e.g., def m(p1), def m(p1, p2,...)). The current behavior is IllegalArgumentException is thrown when Router is used for building HTTP filters.

This PR adds Router.verifyRoutes so that we can manually find the presence of method overload early.

@xerial xerial requested review from shimamoto and takezoe June 15, 2020 05:47
@codecov
Copy link

codecov bot commented Jun 15, 2020

Codecov Report

Merging #1137 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1137   +/-   ##
=======================================
  Coverage   82.57%   82.57%           
=======================================
  Files         276      276           
  Lines       10544    10545    +1     
  Branches      641      673   +32     
=======================================
+ Hits         8707     8708    +1     
  Misses       1837     1837           
Impacted Files Coverage Δ
...vm/src/main/scala/wvlet/airframe/http/Router.scala 87.30% <100.00%> (+0.20%) ⬆️
...ala/wvlet/airframe/http/codegen/RouteScanner.scala 40.00% <100.00%> (+3.15%) ⬆️
...shared/src/main/scala/wvlet/log/io/StopWatch.scala 80.00% <0.00%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b70cdbd...fb4c4b2. Read the comment docs.

Copy link
Member

@takezoe takezoe left a comment

Choose a reason for hiding this comment

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

Worked as expected. Thanks for adding this!

@xerial xerial merged commit c6768ed into wvlet:master Jun 18, 2020
@xerial xerial deleted the rpc-overload branch June 18, 2020 06:33
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

Successfully merging this pull request may close these issues.

2 participants