diff --git a/internal/gengapic/generator.go b/internal/gengapic/generator.go index fea58959bb..e84d70931e 100644 --- a/internal/gengapic/generator.go +++ b/internal/gengapic/generator.go @@ -41,6 +41,11 @@ var enableWrapperTypesForPageSize = map[string]bool{ "google.cloud.bigquery.v2": true, } +var enableOrderedRoutingHeaders = map[string]bool{ + "google.firestore.v1": true, + "google.firestore.admin.v1": true, +} + var enableMtlsHardBoundTokens = map[string]bool{ "bigquery.googleapis.com": true, "cloudasset.googleapis.com": true, diff --git a/internal/gengapic/gengapic.go b/internal/gengapic/gengapic.go index a102787cf4..a963d3a5be 100644 --- a/internal/gengapic/gengapic.go +++ b/internal/gengapic/gengapic.go @@ -457,9 +457,26 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper) g.printf("}") } - g.printf("for headerName, headerValue := range routingHeadersMap {") - g.printf(` routingHeaders = fmt.Sprintf("%%s%%s=%%s&", routingHeaders, headerName, headerValue)`) - g.printf("}") + var orderedHeaders bool + for p, ok := range enableOrderedRoutingHeaders { + if g.descInfo.ParentFile[m].GetPackage() == p && ok { + orderedHeaders = true + break + } + } + if orderedHeaders { + for i := range headers { + headerName := headers[i][2] + g.printf("if headerValue, ok := routingHeadersMap[%q]; ok {", headerName) + g.printf(` routingHeaders = fmt.Sprintf("%%s%%s=%%s&", routingHeaders, %q, headerValue)`, headerName) + g.printf(" delete(routingHeadersMap, %q)", headerName) + g.printf("}") + } + } else { + g.printf("for headerName, headerValue := range routingHeadersMap {") + g.printf(` routingHeaders = fmt.Sprintf("%%s%%s=%%s&", routingHeaders, headerName, headerValue)`) + g.printf("}") + } g.printf(`routingHeaders = strings.TrimSuffix(routingHeaders, "&")`) g.imports[pbinfo.ImportSpec{Path: "strings"}] = true g.printf(`hds := []string{"x-goog-request-params", routingHeaders}`) diff --git a/internal/gengapic/gengapic_test.go b/internal/gengapic/gengapic_test.go index daf5702558..00b4700637 100644 --- a/internal/gengapic/gengapic_test.go +++ b/internal/gengapic/gengapic_test.go @@ -1414,6 +1414,114 @@ func TestGenAndCommentHelpers(t *testing.T) { } } +func TestInsertDynamicRequestHeaders_Ordering(t *testing.T) { + g := &generator{ + imports: make(map[pbinfo.ImportSpec]bool), + descInfo: pbinfo.Info{ + Type: make(map[string]pbinfo.ProtoType), + ParentFile: make(map[protoreflect.ProtoMessage]*descriptorpb.FileDescriptorProto), + }, + } + + m := &descriptorpb.MethodDescriptorProto{ + Name: proto.String("TestMethod"), + InputType: proto.String(".InputType"), + Options: &descriptorpb.MethodOptions{}, + } + + // Setup InputType with fields used in headers + inputType := &descriptorpb.DescriptorProto{ + Name: proto.String("InputType"), + Field: []*descriptorpb.FieldDescriptorProto{ + {Name: proto.String("field1"), Type: typePtr(descriptorpb.FieldDescriptorProto_TYPE_STRING)}, + {Name: proto.String("field2"), Type: typePtr(descriptorpb.FieldDescriptorProto_TYPE_STRING)}, + {Name: proto.String("field3"), Type: typePtr(descriptorpb.FieldDescriptorProto_TYPE_STRING)}, + }, + } + g.descInfo.Type[".InputType"] = inputType + + // The input headers list defines the order: + // 1. header_a (from field1) + // 2. header_b (from field2) + // 3. header_a (from field3) -- Duplicate key + headers := [][]string{ + {"regex1", "field1", "header_a"}, + {"regex2", "field2", "header_b"}, + {"regex3", "field3", "header_a"}, + } + + tests := []struct { + pkgName string + wantOrdered bool + }{ + {"google.firestore.v1", true}, + {"google.firestore.admin.v1", true}, + {"google.cloud.storage.v1", false}, + {"other.package", false}, + } + + for _, tc := range tests { + t.Run(tc.pkgName, func(t *testing.T) { + g.reset() + + file := &descriptorpb.FileDescriptorProto{ + Package: proto.String(tc.pkgName), + } + g.descInfo.ParentFile[m] = file + g.descInfo.ParentFile[inputType] = file + + err := g.insertDynamicRequestHeaders(m, headers) + if err != nil { + t.Fatalf("insertDynamicRequestHeaders failed: %v", err) + } + + got := g.pt.String() + + if tc.wantOrdered { + // Verify strict order of checks in the generated code. + // We expect code blocks for header_a, then header_b, then header_a again. + + idxA1 := strings.Index(got, `if headerValue, ok := routingHeadersMap["header_a"]; ok {`) + if idxA1 == -1 { + t.Errorf("expected first check for header_a") + } + + // Look for header_b check AFTER header_a check + idxB := strings.Index(got[idxA1+1:], `if headerValue, ok := routingHeadersMap["header_b"]; ok {`) + if idxB == -1 { + t.Errorf("expected check for header_b after first header_a") + } + idxB += idxA1 + 1 // Adjust index to be absolute + + // Look for second header_a check AFTER header_b check + idxA2 := strings.Index(got[idxB+1:], `if headerValue, ok := routingHeadersMap["header_a"]; ok {`) + if idxA2 == -1 { + t.Errorf("expected second check for header_a after header_b") + } + + // Verify delete is called + if !strings.Contains(got, `delete(routingHeadersMap, "header_a")`) { + t.Errorf("expected delete map key for %s", tc.pkgName) + } + + // Verify absence of range loop + if strings.Contains(got, `range routingHeadersMap`) { + t.Errorf("expected NO range map iteration for %s", tc.pkgName) + } + } else { + // Check for unordered iteration pattern: range loop + if !strings.Contains(got, `for headerName, headerValue := range routingHeadersMap {`) { + t.Errorf("expected range map iteration for %s, got:\n%s", tc.pkgName, got) + } + } + }) + } +} + +func typePtr(t descriptorpb.FieldDescriptorProto_Type) *descriptorpb.FieldDescriptorProto_Type { + return &t +} + func setHTTPOption(o *descriptorpb.MethodOptions, pattern string) { proto.SetExtension(o, annotations.E_Http, &annotations.HttpRule{ Pattern: &annotations.HttpRule_Get{