Skip to content

Commit 9171f1c

Browse files
fix: use camelCase for google.api.http path parameters and fix double path-prefix
Three fixes for google.api.http OpenAPI generation: 1. Path parameters now use camelCase (JSON names) by default instead of snake_case proto field names. E.g., {account_id} becomes {accountId}. Respects the with-proto-names option to keep snake_case when needed. 2. Fix double path-prefix application for additional_bindings. The prefix was applied both in httpRuleToPathMap and in the central addPathItem handler, causing paths like /api/node/api/node/.... 3. Fix nested path params (e.g., source_public_id.value) not tracking the top-level field, which caused the parent field to not be excluded from the request body when body: "*" was used. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent da01f8b commit 9171f1c

25 files changed

Lines changed: 831 additions & 63 deletions

internal/converter/converter_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ var scenarios = []Scenario{
3535
{Name: "with_specification_extensions", Options: "base=testdata/with_specification_extensions/base.yaml,trim-unused-types"},
3636
{Name: "additional_bindings"},
3737
{Name: "path_params"},
38+
{Name: "additional_bindings_prefix", Options: "path-prefix=/api/v1"},
3839
{Name: "with_override", Options: "override=testdata/with_override/override.yaml"},
3940
{Name: "with_service_filters", Options: "services=**.User*"},
4041
{Name: "with_google_error_detail", Options: "with-google-error-detail"},

internal/converter/googleapi/paths.go

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
112112
}
113113

114114
fieldNamesInPath := map[string]struct{}{}
115+
paramRenames := map[string]string{} // maps proto path param name to display name (camelCase by default)
115116
var pathParams []*v3.Parameter
116117
var deferredParams []*v3.Parameter
117118
for _, param := range partsToParameter(tokens) {
@@ -125,9 +126,27 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
125126
// query/param or request body
126127
fieldNamesInPath[string(field.FullName())] = struct{}{}
127128
fieldNamesInPath[strings.Join(jsonPath, ".")] = struct{}{} // sometimes JSON field names are used
129+
130+
// For nested params (e.g., "source_public_id.value"), also track the
131+
// top-level field so it can be excluded from the request body schema
132+
paramParts := strings.SplitN(param, ".", 2)
133+
if len(paramParts) > 1 {
134+
if topField := fieldByName(opts, md.Input(), paramParts[0]); topField != nil {
135+
fieldNamesInPath[string(topField.FullName())] = struct{}{}
136+
fieldNamesInPath[util.MakeFieldName(opts, topField)] = struct{}{}
137+
}
138+
}
139+
140+
// Compute display name for the parameter (camelCase by default)
141+
displayName := param
142+
if !opts.WithProtoNames {
143+
displayName = strings.Join(jsonPath, ".")
144+
}
145+
paramRenames[param] = displayName
146+
128147
loc := fd.SourceLocations().ByDescriptor(field)
129148
newParameter := &v3.Parameter{
130-
Name: param,
149+
Name: displayName,
131150
Required: proto.Bool(true),
132151
In: "path",
133152
Description: util.FormatComments(loc),
@@ -157,7 +176,13 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
157176
if len(matches) == 3 {
158177
if matches[2] == "**" {
159178
paramName := matches[1]
160-
field, _ := resolveField(opts, md.Input(), paramName)
179+
field, jsonPath := resolveField(opts, md.Input(), paramName)
180+
// Compute display name (camelCase by default)
181+
displayName := paramName
182+
if field != nil && !opts.WithProtoNames {
183+
displayName = strings.Join(jsonPath, ".")
184+
}
185+
paramRenames[paramName] = displayName
161186
var newParameter *v3.Parameter
162187
if field != nil {
163188
fieldNamesInPath[string(field.FullName())] = struct{}{}
@@ -169,7 +194,7 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
169194
parameterSchema = base.CreateSchemaProxy(&base.Schema{Type: []string{"string"}})
170195
}
171196
newParameter = &v3.Parameter{
172-
Name: paramName,
197+
Name: displayName,
173198
Required: proto.Bool(true),
174199
In: "path",
175200
Description: util.FormatComments(loc),
@@ -178,7 +203,7 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
178203
}
179204
} else {
180205
newParameter = &v3.Parameter{
181-
Name: paramName,
206+
Name: displayName,
182207
Required: proto.Bool(true),
183208
In: "path",
184209
Description: "The trailing part of the path.",
@@ -359,7 +384,7 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
359384
pathItem.Patch = op
360385
default:
361386
}
362-
openAPIPath := partsToOpenAPIPath(tokens)
387+
openAPIPath := partsToOpenAPIPath(tokens, paramRenames)
363388
paths.Set(openAPIPath, pathItem)
364389

365390
allDeferred := orderedmap.New[string, []*v3.Parameter]()
@@ -370,12 +395,10 @@ func httpRuleToPathMap(opts options.Options, md protoreflect.MethodDescriptor, r
370395
for _, binding := range rule.AdditionalBindings {
371396
sub := httpRuleToPathMap(opts, md, binding)
372397
for pair := sub.PathItems.First(); pair != nil; pair = pair.Next() {
373-
path := util.MakePath(opts, pair.Key())
374-
paths.Set(path, pair.Value())
398+
paths.Set(pair.Key(), pair.Value())
375399
}
376400
for pair := sub.DeferredParams.First(); pair != nil; pair = pair.Next() {
377-
path := util.MakePath(opts, pair.Key())
378-
allDeferred.Set(path, pair.Value())
401+
allDeferred.Set(pair.Key(), pair.Value())
379402
}
380403
}
381404
dedupeOperations(op.OperationId, paths.ValuesFromOldest())
@@ -444,7 +467,7 @@ func partsToParameter(tokens []Token) []string {
444467
return params
445468
}
446469

447-
func partsToOpenAPIPath(tokens []Token) string {
470+
func partsToOpenAPIPath(tokens []Token, renames map[string]string) string {
448471
var b strings.Builder
449472
for _, token := range tokens {
450473
switch token.Type {
@@ -467,8 +490,12 @@ func partsToOpenAPIPath(tokens []Token) string {
467490
matches := namedPathPattern.FindStringSubmatch("{" + token.Value + "}")
468491
if len(matches) == 3 {
469492
if matches[2] == "**" {
493+
name := matches[1]
494+
if renamed, ok := renames[name]; ok {
495+
name = renamed
496+
}
470497
b.WriteString("{")
471-
b.WriteString(matches[1])
498+
b.WriteString(name)
472499
b.WriteString("}")
473500
continue
474501
}
@@ -494,8 +521,12 @@ func partsToOpenAPIPath(tokens []Token) string {
494521
b.WriteByte('}')
495522
}
496523
} else {
524+
name := token.Value
525+
if renamed, ok := renames[name]; ok {
526+
name = renamed
527+
}
497528
b.WriteByte('{')
498-
b.WriteString(token.Value)
529+
b.WriteString(name)
499530
b.WriteByte('}')
500531
}
501532
}

internal/converter/googleapi/paths_test.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,52 @@ func TestPartsToOpenAPIPath(t *testing.T) {
1111
t.Run("with annotation", func(t *testing.T) {
1212
v, err := RunPathPatternLexer("/pet/{pet_id}:addPet")
1313
require.NoError(t, err)
14-
path := partsToOpenAPIPath(v)
14+
path := partsToOpenAPIPath(v, nil)
1515
assert.Equal(t, "/pet/{pet_id}:addPet", path)
1616
})
1717

1818
t.Run("with glob pattern", func(t *testing.T) {
1919
v, err := RunPathPatternLexer("/users/v1/{name=organizations/*/teams/*/members/*}:activate")
2020
require.NoError(t, err)
21-
path := partsToOpenAPIPath(v)
21+
path := partsToOpenAPIPath(v, nil)
2222
assert.Equal(t, "/users/v1/organizations/{organization}/teams/{team}/members/{member}:activate", path)
2323
})
2424

2525
t.Run("with glob pattern containing literal segment", func(t *testing.T) {
2626
v, err := RunPathPatternLexer("/users/v1/{name=organizations/*/teams/*/all/members/*}:activate")
2727
require.NoError(t, err)
28-
path := partsToOpenAPIPath(v)
28+
path := partsToOpenAPIPath(v, nil)
2929
assert.Equal(t, "/users/v1/organizations/{organization}/teams/{team}/all/members/{member}:activate", path)
3030
})
31+
32+
t.Run("with renames applies camelCase to simple variable", func(t *testing.T) {
33+
v, err := RunPathPatternLexer("/v1/resources/{resource_id}")
34+
require.NoError(t, err)
35+
renames := map[string]string{"resource_id": "resourceId"}
36+
path := partsToOpenAPIPath(v, renames)
37+
assert.Equal(t, "/v1/resources/{resourceId}", path)
38+
})
39+
40+
t.Run("with renames applies camelCase to multiple variables", func(t *testing.T) {
41+
v, err := RunPathPatternLexer("/v1/{account_id}/items/{item_id}")
42+
require.NoError(t, err)
43+
renames := map[string]string{"account_id": "accountId", "item_id": "itemId"}
44+
path := partsToOpenAPIPath(v, renames)
45+
assert.Equal(t, "/v1/{accountId}/items/{itemId}", path)
46+
})
47+
48+
t.Run("with renames applies camelCase to glob pattern name", func(t *testing.T) {
49+
v, err := RunPathPatternLexer("/v1/{resource_name=**}")
50+
require.NoError(t, err)
51+
renames := map[string]string{"resource_name": "resourceName"}
52+
path := partsToOpenAPIPath(v, renames)
53+
assert.Equal(t, "/v1/{resourceName}", path)
54+
})
55+
56+
t.Run("without renames keeps original variable names", func(t *testing.T) {
57+
v, err := RunPathPatternLexer("/v1/resources/{resource_id}")
58+
require.NoError(t, err)
59+
path := partsToOpenAPIPath(v, nil)
60+
assert.Equal(t, "/v1/resources/{resource_id}", path)
61+
})
3162
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
syntax = "proto3";
2+
3+
package additional_bindings_prefix;
4+
5+
import "google/api/annotations.proto";
6+
7+
// Tests that additional_bindings with path-prefix don't double-apply the prefix.
8+
service PrefixService {
9+
rpc GetItem(GetItemRequest) returns (GetItemResponse) {
10+
option (google.api.http) = {
11+
get: "/items/{item_id}"
12+
13+
additional_bindings: {
14+
get: "/items/latest"
15+
}
16+
};
17+
}
18+
}
19+
20+
message GetItemRequest {
21+
string item_id = 1;
22+
}
23+
24+
message GetItemResponse {
25+
string id = 1;
26+
string name = 2;
27+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
{
2+
"openapi": "3.1.0",
3+
"info": {
4+
"title": "additional_bindings_prefix"
5+
},
6+
"paths": {
7+
"/api/v1/items/latest": {
8+
"get": {
9+
"tags": [
10+
"additional_bindings_prefix.PrefixService"
11+
],
12+
"summary": "GetItem",
13+
"operationId": "additional_bindings_prefix.PrefixService.GetItem2",
14+
"parameters": [
15+
{
16+
"name": "itemId",
17+
"in": "query",
18+
"schema": {
19+
"type": "string",
20+
"title": "item_id"
21+
}
22+
}
23+
],
24+
"responses": {
25+
"200": {
26+
"description": "Success",
27+
"content": {
28+
"application/json": {
29+
"schema": {
30+
"$ref": "#/components/schemas/additional_bindings_prefix.GetItemResponse"
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
},
38+
"/api/v1/items/{itemId}": {
39+
"get": {
40+
"tags": [
41+
"additional_bindings_prefix.PrefixService"
42+
],
43+
"summary": "GetItem",
44+
"operationId": "additional_bindings_prefix.PrefixService.GetItem",
45+
"parameters": [
46+
{
47+
"name": "itemId",
48+
"in": "path",
49+
"required": true,
50+
"schema": {
51+
"type": "string",
52+
"title": "item_id"
53+
}
54+
}
55+
],
56+
"responses": {
57+
"200": {
58+
"description": "Success",
59+
"content": {
60+
"application/json": {
61+
"schema": {
62+
"$ref": "#/components/schemas/additional_bindings_prefix.GetItemResponse"
63+
}
64+
}
65+
}
66+
}
67+
}
68+
}
69+
}
70+
},
71+
"components": {
72+
"schemas": {
73+
"additional_bindings_prefix.GetItemRequest": {
74+
"type": "object",
75+
"properties": {
76+
"itemId": {
77+
"type": "string",
78+
"title": "item_id"
79+
}
80+
},
81+
"title": "GetItemRequest",
82+
"additionalProperties": false
83+
},
84+
"additional_bindings_prefix.GetItemResponse": {
85+
"type": "object",
86+
"properties": {
87+
"id": {
88+
"type": "string",
89+
"title": "id"
90+
},
91+
"name": {
92+
"type": "string",
93+
"title": "name"
94+
}
95+
},
96+
"title": "GetItemResponse",
97+
"additionalProperties": false
98+
}
99+
}
100+
},
101+
"security": [],
102+
"tags": [
103+
{
104+
"name": "additional_bindings_prefix.PrefixService",
105+
"description": "Tests that additional_bindings with path-prefix don't double-apply the prefix."
106+
}
107+
]
108+
}

0 commit comments

Comments
 (0)