Skip to content

Commit 6b4035e

Browse files
bhshkhbhshkh
authored
fix(gengapic): ordered dynamic header values (#1661)
1 parent a098fba commit 6b4035e

File tree

5 files changed

+59
-46
lines changed

5 files changed

+59
-46
lines changed

‎internal/gengapic/gengapic.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,8 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor
441441
return nil
442442
}
443443

444-
g.printf(`routingHeaders := ""`)
445-
g.printf("routingHeadersMap := make(map[string]string)")
444+
g.printf(`var routingHeaders []string`)
445+
g.printf("seen := make(map[string]bool)")
446446
for i := range headers {
447447
namedCaptureRegex := headers[i][0]
448448
field := headers[i][1]
@@ -455,15 +455,14 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor
455455
}
456456
// There could be an edge case where the request field is empty and the path template is a wildcard. In that case, we still don't want to send an empty header name.
457457
g.printf("if reg := regexp.MustCompile(%q); reg.MatchString(%s) && len(%s) > 0 {", namedCaptureRegex, accessor, regexHelper)
458-
g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper)
458+
g.printf(" if !seen[%q] {", headerName)
459+
g.printf(" routingHeaders = append(routingHeaders, fmt.Sprintf(\"%%s=%%s\", %q, %s))", headerName, regexHelper)
460+
g.printf(" seen[%q] = true", headerName)
461+
g.printf(" }")
459462
g.printf("}")
460463
}
461-
g.printf("for headerName, headerValue := range routingHeadersMap {")
462-
g.printf(` routingHeaders = fmt.Sprintf("%%s%%s=%%s&", routingHeaders, headerName, headerValue)`)
463-
g.printf("}")
464-
g.printf(`routingHeaders = strings.TrimSuffix(routingHeaders, "&")`)
464+
g.printf(`hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}`)
465465
g.imports[pbinfo.ImportSpec{Path: "strings"}] = true
466-
g.printf(`hds := []string{"x-goog-request-params", routingHeaders}`)
467466
g.imports[pbinfo.ImportSpec{Path: "regexp"}] = true
468467
return nil
469468
}

‎internal/gengapic/testdata/method_GetAnotherThing.want‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,49 @@
11
func (c *fooGRPCClient) GetAnotherThing(ctx context.Context, req *mypackagepb.InputType, opts ...gax.CallOption) (*mypackagepb.OutputType, error) {
2-
routingHeaders := ""
3-
routingHeadersMap := make(map[string]string)
2+
var routingHeaders []string
3+
seen := make(map[string]bool)
44
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
5-
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
5+
if !seen["other"] {
6+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "other", url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])))
7+
seen["other"] = true
8+
}
69
}
710
if reg := regexp.MustCompile("(?P<name>projects/[^/]+)/foos"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
8-
routingHeadersMap["name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
11+
if !seen["name"] {
12+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "name", url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])))
13+
seen["name"] = true
14+
}
915
}
1016
if reg := regexp.MustCompile("(?P<foo_name>projects/[^/]+)/bars/[^/]+(?:/.*)?"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
11-
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
17+
if !seen["foo_name"] {
18+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "foo_name", url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])))
19+
seen["foo_name"] = true
20+
}
1221
}
1322
if reg := regexp.MustCompile("(?P<foo_name>projects/[^/]+/foos/[^/]+)/bars/[^/]+(?:/.*)?"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
14-
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
23+
if !seen["foo_name"] {
24+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "foo_name", url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])))
25+
seen["foo_name"] = true
26+
}
1527
}
1628
if reg := regexp.MustCompile("(?P<foo_name>.*)"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
17-
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
29+
if !seen["foo_name"] {
30+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "foo_name", url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])))
31+
seen["foo_name"] = true
32+
}
1833
}
1934
if reg := regexp.MustCompile("(?P<nested_name>.*)"); reg.MatchString(req.GetFieldName().GetNested()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])) > 0 {
20-
routingHeadersMap["nested_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])
35+
if !seen["nested_name"] {
36+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "nested_name", url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])))
37+
seen["nested_name"] = true
38+
}
2139
}
2240
if reg := regexp.MustCompile("(?P<part_of_nested>projects/[^/]+)/bars"); reg.MatchString(req.GetFieldName().GetNested()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])) > 0 {
23-
routingHeadersMap["part_of_nested"] = url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])
41+
if !seen["part_of_nested"] {
42+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "part_of_nested", url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])))
43+
seen["part_of_nested"] = true
44+
}
2445
}
25-
for headerName, headerValue := range routingHeadersMap {
26-
routingHeaders = fmt.Sprintf("%s%s=%s&", routingHeaders, headerName, headerValue)
27-
}
28-
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
29-
hds := []string{"x-goog-request-params", routingHeaders}
46+
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
3047

3148
hds = append(c.xGoogHeaders, hds...)
3249
ctx = gax.InsertMetadataIntoOutgoingContext(ctx, hds...)

‎internal/gengapic/testdata/rest_HttpBodyRPC.want‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,15 @@ func (c *fooRESTClient) HttpBodyRPC(ctx context.Context, req *foopb.Foo, opts ..
1212
baseUrl.Path += fmt.Sprintf("/v1/foo")
1313

1414
// Build HTTP headers from client and context metadata.
15-
routingHeaders := ""
16-
routingHeadersMap := make(map[string]string)
15+
var routingHeaders []string
16+
seen := make(map[string]bool)
1717
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
18-
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
19-
}
20-
for headerName, headerValue := range routingHeadersMap {
21-
routingHeaders = fmt.Sprintf("%s%s=%s&", routingHeaders, headerName, headerValue)
18+
if !seen["other"] {
19+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "other", url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])))
20+
seen["other"] = true
21+
}
2222
}
23-
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
24-
hds := []string{"x-goog-request-params", routingHeaders}
23+
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
2524

2625
hds = append(c.xGoogHeaders, hds...)
2726
hds = append(hds, "Content-Type", "application/json")

‎internal/gengapic/testdata/rest_ServerStreamRPC.want‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,15 @@ func (c *fooRESTClient) ServerStreamRPC(ctx context.Context, req *foopb.Foo, opt
1212
baseUrl.Path += fmt.Sprintf("/v1/foo")
1313

1414
// Build HTTP headers from client and context metadata.
15-
routingHeaders := ""
16-
routingHeadersMap := make(map[string]string)
15+
var routingHeaders []string
16+
seen := make(map[string]bool)
1717
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
18-
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
19-
}
20-
for headerName, headerValue := range routingHeadersMap {
21-
routingHeaders = fmt.Sprintf("%s%s=%s&", routingHeaders, headerName, headerValue)
18+
if !seen["other"] {
19+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "other", url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])))
20+
seen["other"] = true
21+
}
2222
}
23-
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
24-
hds := []string{"x-goog-request-params", routingHeaders}
23+
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
2524

2625
hds = append(c.xGoogHeaders, hds...)
2726
hds = append(hds, "Content-Type", "application/json")

‎internal/gengapic/testdata/rest_UnaryRPC.want‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@ func (c *fooRESTClient) UnaryRPC(ctx context.Context, req *foopb.Foo, opts ...ga
2020
baseUrl.RawQuery = params.Encode()
2121

2222
// Build HTTP headers from client and context metadata.
23-
routingHeaders := ""
24-
routingHeadersMap := make(map[string]string)
23+
var routingHeaders []string
24+
seen := make(map[string]bool)
2525
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
26-
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
27-
}
28-
for headerName, headerValue := range routingHeadersMap {
29-
routingHeaders = fmt.Sprintf("%s%s=%s&", routingHeaders, headerName, headerValue)
26+
if !seen["other"] {
27+
routingHeaders = append(routingHeaders, fmt.Sprintf("%s=%s", "other", url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])))
28+
seen["other"] = true
29+
}
3030
}
31-
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
32-
hds := []string{"x-goog-request-params", routingHeaders}
31+
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
3332

3433
hds = append(c.xGoogHeaders, hds...)
3534
hds = append(hds, "Content-Type", "application/json")

0 commit comments

Comments
 (0)