Skip to content

Comments

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

Open
ndolgov wants to merge 3 commits intobtlines:masterfrom
ndolgov:feature/20180117/url-parameter-template
Open

GRPCGATEWAY-20 match against named URL template slots correctly#21
ndolgov wants to merge 3 commits intobtlines:masterfrom
ndolgov:feature/20180117/url-parameter-template

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