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

Add default HTTP route for all methods #124

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/examples/pets/internal/proto/buf.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ deps:
- remote: buf.build
owner: googleapis
repository: googleapis
commit: 28151c0d0a1641bf938a7672c500e01d
digest: shake256:49215edf8ef57f7863004539deff8834cfb2195113f0b890dd1f67815d9353e28e668019165b9d872395871eeafcbab3ccfdb2b5f11734d3cca95be9e8d139de
commit: 4ed3bc159a8b4ac68fe253218760d035
digest: shake256:7149cf5e9955c692d381e557830555d4e93f205a0f1b8e2dfdae46d029369aa3fc1980e35df0d310f7cc3b622f93e19ad276769a283a967dd3065ddfd3a40e13
223 changes: 147 additions & 76 deletions internal/gen/vanguard/test/v1/content.pb.go

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions internal/gen/vanguard/test/v1/content_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions internal/gen/vanguard/test/v1/testv1connect/content.connect.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions internal/proto/buf.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ deps:
- remote: buf.build
owner: googleapis
repository: googleapis
commit: 711e289f6a384c4caeebaff7c6931ade
digest: shake256:e08fb55dad7469f69df00304eed31427d2d1576e9aab31e6bf86642688e04caaf0372f15fe6974cf79432779a635b3ea401ca69c943976dc42749524e4c25d94
commit: 4ed3bc159a8b4ac68fe253218760d035
digest: shake256:7149cf5e9955c692d381e557830555d4e93f205a0f1b8e2dfdae46d029369aa3fc1980e35df0d310f7cc3b622f93e19ad276769a283a967dd3065ddfd3a40e13
10 changes: 10 additions & 0 deletions internal/proto/vanguard/test/v1/content.proto
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ service ContentService {
}
// Subscribe to updates for changes to content.
rpc Subscribe(stream SubscribeRequest) returns (stream SubscribeResponse);
// Delete a file at the given path.
rpc Delete(DeleteRequest) returns (google.protobuf.Empty) {
// This method does not have an HTTP mapping, but is still callable via its
// default POST HTTP mapping.
}
}

message IndexRequest {
Expand All @@ -66,6 +71,11 @@ message DownloadResponse {
google.api.HttpBody file = 1;
}

message DeleteRequest {
// The path to the file to delete.
string filename = 1;
}

message SubscribeRequest {
repeated string filename_patterns = 1;
}
Expand Down
65 changes: 51 additions & 14 deletions router.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,18 @@ func (t *routeTrie) addRoute(config *methodConfig, rule *annotations.HttpRule) (
if err != nil {
return nil, err
}
if err := t.insert(method, target, segments); err != nil {
if err := t.insert(target); err != nil {
return nil, err
}
return target, nil
}

// addDefaultRoute adds a default target to the router for the given method.
// This may be overridden by a explicit rule. The default route is of the form
// POST /service/method.
func (t *routeTrie) addDefaultRoute(config *methodConfig) (*routeTarget, error) {
target := makeDefaultTarget(config)
if err := t.insert(target); err != nil {
return nil, err
}
return target, nil
Expand Down Expand Up @@ -103,17 +114,15 @@ func (t *routeTrie) insertVerb(verb string) routeMethods {

// insert the target into the trie using the given method and segment path.
// The path is followed until the final segment is reached.
func (t *routeTrie) insert(method string, target *routeTarget, segments pathSegments) error {
func (t *routeTrie) insert(target *routeTarget) error {
cursor := t
for _, segment := range segments.path {
for _, segment := range target.path {
cursor = cursor.insertChild(segment)
}
if existing := cursor.verbs[segments.verb][method]; existing != nil {
return alreadyExistsError{
existing: existing, pathPattern: segments.String(), method: method,
}
if existing := cursor.verbs[target.verb][target.method]; existing != nil && !existing.isImplicit {
return alreadyExistsError{target: target, existing: existing}
}
cursor.insertVerb(segments.verb)[method] = target
cursor.insertVerb(target.verb)[target.method] = target
return nil
}

Expand All @@ -124,9 +133,8 @@ func (t *routeTrie) match(uriPath, httpMethod string) (*routeTarget, []routeTarg
// Must start with "/" or if it ends with ":" it won't match
return nil, nil, nil
}
uriPath = uriPath[1:] // skip the leading slash

path := strings.Split(uriPath, "/")
path := strings.Split(uriPath[1:], "/") // skip the leading slash
var verb string
if len(path) > 0 {
lastElement := path[len(path)-1]
Expand Down Expand Up @@ -208,6 +216,14 @@ type routeTarget struct {
responseBodyFieldPath string
responseBodyFields []protoreflect.FieldDescriptor
vars []routeTargetVar

// isImplicit is true if the target was created implicitly, such as for a
// default method. This is used to prevent overwriting an explicit target.
isImplicit bool
}

func (t *routeTarget) String() string {
return t.method + " " + pathSegments{path: t.path, verb: t.verb}.String()
}

func makeTarget(
Expand Down Expand Up @@ -266,6 +282,25 @@ func makeTarget(
}, nil
}

// makeDefaultTarget of the form POST /service/method.
func makeDefaultTarget(
config *methodConfig,
) *routeTarget {
return &routeTarget{
config: config,
method: http.MethodPost,
path: []string{
string(config.descriptor.Parent().FullName()),
string(config.descriptor.Name()),
},
requestBodyFieldPath: "*",
requestBodyFields: []protoreflect.FieldDescriptor{},
responseBodyFieldPath: "*",
responseBodyFields: []protoreflect.FieldDescriptor{},
isImplicit: true,
}
}

type routeTargetVar struct {
pathVariable

Expand Down Expand Up @@ -359,7 +394,7 @@ func resolvePathToDescriptors(msg protoreflect.MessageDescriptor, path string) (
msg = field.Message()
if msg == nil {
return nil, fmt.Errorf("in field path %q: field %q of type %s should be a message but is instead %s",
path, part, msg.FullName(), field.Kind())
path, part, protoreflect.MessageKind, field.Kind())
emcfarlane marked this conversation as resolved.
Show resolved Hide resolved
}
fields = msg.Fields()
}
Expand All @@ -380,10 +415,12 @@ func resolveFieldDescriptorsToPath(fields []protoreflect.FieldDescriptor) string
}

type alreadyExistsError struct {
existing *routeTarget
pathPattern, method string
target, existing *routeTarget
}

func (a alreadyExistsError) Error() string {
return fmt.Sprintf("target for %s, method %s already exists: %s", a.pathPattern, a.method, a.existing.config.descriptor.FullName())
return fmt.Sprintf(
"target for %s already exists: %s",
a.target.String(), a.existing.config.descriptor.FullName(),
)
}
4 changes: 2 additions & 2 deletions router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,9 @@ func initTrie(tb testing.TB) *routeTrie {
name: fmt.Sprintf("%s %s", method, route),
},
}
target, err := makeTarget(config, "POST", "*", "*", segments, variables)
target, err := makeTarget(config, method, "*", "*", segments, variables)
require.NoError(tb, err)
err = trie.insert(method, target, segments)
err = trie.insert(target)
require.NoError(tb, err)
}
}
Expand Down
Loading
Loading