Skip to content

GRPCGATEWAY-20 match against named URL template slots correctly #21

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ndolgov
Copy link

@ndolgov ndolgov commented Jan 19, 2018

  • update GrpcGatewayHandler to use a single supportsCall method to both match a URI and extract parameters from it in one pass
  • update GatewayGenerator to support the new code template
  • add URL template parsing logic to the runtime package
  • add a GatewayGenerator unit tests that uses a hard-coded CodeGeneratorRequest to quickly generate a representative demo handler

in a generated Handler

  • create a map for every HTTP verb used in the protobuf
  • each map entry represents a URL template matcher to request processor method mapping
  • for every request, try all applicable matchers
  • if a match is found, return the corresponding processor method
  • make GrpcGatewayHandler execute the method

while (pathIndex < path.length) {
val from = pathIndex

val matcher = matchers(matcherIndex)
Copy link

Choose a reason for hiding this comment

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

there is a bug here. in cases when matchers.length < path.length this line will thrown IndexOutOfBoundsException.

@saurcery
Copy link

saurcery commented Oct 9, 2018

@ndolgov @btlines @xuwei-k any further interest in getting this PR to master ?
I resolved the merge conflicts and other import issues here saurcery@c3998fc#diff-42123b068abeef330b1c78c05cbee1d5

Its quite useful feature for RESTful API support imo. Let me know how I can help!

@ndolgov
Copy link
Author

ndolgov commented Oct 10, 2018

I have a feeling @btlines has abandoned this repo. It's a pity because the idea was intriguing.

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