Skip to content

Commit 9d27531

Browse files
shollymanshollyman
authored
fix(gengapic): revert "fix(gengapic): ordered dynamic header values (#1661)"
This reverts commit 6b4035e. Reverts #1661 Related: b/460298510 and a regression in the storage API
1 parent b419687 commit 9d27531

File tree

5 files changed

+46
-59
lines changed

5 files changed

+46
-59
lines changed

‎internal/gengapic/gengapic.go‎

Lines changed: 8 additions & 7 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(`var routingHeaders []string`)
445-
g.printf("seen := make(map[string]bool)")
444+
g.printf(`routingHeaders := ""`)
445+
g.printf("routingHeadersMap := make(map[string]string)")
446446
for i := range headers {
447447
namedCaptureRegex := headers[i][0]
448448
field := headers[i][1]
@@ -455,14 +455,15 @@ 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(" 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(" }")
458+
g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper)
462459
g.printf("}")
463460
}
464-
g.printf(`hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}`)
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, "&")`)
465465
g.imports[pbinfo.ImportSpec{Path: "strings"}] = true
466+
g.printf(`hds := []string{"x-goog-request-params", routingHeaders}`)
466467
g.imports[pbinfo.ImportSpec{Path: "regexp"}] = true
467468
return nil
468469
}

‎internal/gengapic/testdata/method_GetAnotherThing.want‎

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,32 @@
11
func (c *fooGRPCClient) GetAnotherThing(ctx context.Context, req *mypackagepb.InputType, opts ...gax.CallOption) (*mypackagepb.OutputType, error) {
2-
var routingHeaders []string
3-
seen := make(map[string]bool)
2+
routingHeaders := ""
3+
routingHeadersMap := make(map[string]string)
44
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
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-
}
5+
routingHeadersMap["other"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
96
}
107
if reg := regexp.MustCompile("(?P<name>projects/[^/]+)/foos"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
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-
}
8+
routingHeadersMap["name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])
159
}
1610
if reg := regexp.MustCompile("(?P<foo_name>projects/[^/]+)/bars/[^/]+(?:/.*)?"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
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-
}
11+
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
2112
}
2213
if reg := regexp.MustCompile("(?P<foo_name>projects/[^/]+/foos/[^/]+)/bars/[^/]+(?:/.*)?"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
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-
}
14+
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
2715
}
2816
if reg := regexp.MustCompile("(?P<foo_name>.*)"); reg.MatchString(req.GetAnother()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])) > 0 {
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-
}
17+
routingHeadersMap["foo_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetAnother())[1])
3318
}
3419
if reg := regexp.MustCompile("(?P<nested_name>.*)"); reg.MatchString(req.GetFieldName().GetNested()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])) > 0 {
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-
}
20+
routingHeadersMap["nested_name"] = url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])
3921
}
4022
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 {
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-
}
23+
routingHeadersMap["part_of_nested"] = url.QueryEscape(reg.FindStringSubmatch(req.GetFieldName().GetNested())[1])
4524
}
46-
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
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}
4730

4831
hds = append(c.xGoogHeaders, hds...)
4932
ctx = gax.InsertMetadataIntoOutgoingContext(ctx, hds...)

‎internal/gengapic/testdata/rest_HttpBodyRPC.want‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ 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-
var routingHeaders []string
16-
seen := make(map[string]bool)
15+
routingHeaders := ""
16+
routingHeadersMap := make(map[string]string)
1717
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
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-
}
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)
2222
}
23-
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
23+
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
24+
hds := []string{"x-goog-request-params", routingHeaders}
2425

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

‎internal/gengapic/testdata/rest_ServerStreamRPC.want‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,16 @@ 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-
var routingHeaders []string
16-
seen := make(map[string]bool)
15+
routingHeaders := ""
16+
routingHeadersMap := make(map[string]string)
1717
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
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-
}
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)
2222
}
23-
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
23+
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
24+
hds := []string{"x-goog-request-params", routingHeaders}
2425

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

‎internal/gengapic/testdata/rest_UnaryRPC.want‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,16 @@ 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-
var routingHeaders []string
24-
seen := make(map[string]bool)
23+
routingHeaders := ""
24+
routingHeadersMap := make(map[string]string)
2525
if reg := regexp.MustCompile("(.*)"); reg.MatchString(req.GetOther()) && len(url.QueryEscape(reg.FindStringSubmatch(req.GetOther())[1])) > 0 {
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-
}
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)
3030
}
31-
hds := []string{"x-goog-request-params", strings.Join(routingHeaders, "&")}
31+
routingHeaders = strings.TrimSuffix(routingHeaders, "&")
32+
hds := []string{"x-goog-request-params", routingHeaders}
3233

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

0 commit comments

Comments
 (0)