fix(gengapic): Ensure stable order of dynamic Firestore routing header parameters (#1685)

Author: bhshkh

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,

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}`)

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{