Skip to content

Commit d5d1315

Browse files
authored
fix(sidekick): retain trailing resource names containing intermediate literals (#4621)
Previously, the backward-scanning loop assumed that every path segment preceding a known `collection/variable` pair had to be another variable. If it encountered an unpacked string literal like `global` in the path `/projects/{project}/global/backendServices/{backend_service}`, it broke the chain early. The loop now explicitly handle two distinct cases: Case 1 (Standalone Literal): If the preceding path segment is a standalone literal string (that isn't an API version like `v1`), the scanner steps past it instead of breaking. Case 2 (Variable + Literal Pair): Normal lookup and matching of `collection/variable` pairs like `projects` and `{project}`. Fixes #4620
1 parent 8d789e4 commit d5d1315

2 files changed

Lines changed: 78 additions & 11 deletions

File tree

internal/sidekick/api/resource_identification.go

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,21 +99,42 @@ func identifyHeuristicTarget(method *Method, binding *PathBinding, vocabulary ma
9999
// beginning of a resource pattern chain.
100100
firstIndex := i
101101
if vocabulary[token] {
102-
// Walk backwards to find the start of the (literal, variable) chain
102+
// Walk backwards to find the start of the resource pattern chain
103103
firstIndex = i - 1
104-
for firstIndex >= 2 {
105-
if tmpl.Segments[firstIndex-1].Variable == nil || tmpl.Segments[firstIndex-2].Literal == nil {
106-
break
104+
for firstIndex > 0 {
105+
prevSeg := tmpl.Segments[firstIndex-1]
106+
107+
// Case 1: The preceding segment is a standalone literal (e.g., "global")
108+
if prevSeg.Literal != nil {
109+
if isVersionString(*prevSeg.Literal) {
110+
break // Stop at version strings (e.g., "v1")
111+
}
112+
firstIndex--
113+
continue
107114
}
108-
// Stop matching if the preceding segment isn't a known collection.
109-
if !vocabulary[*tmpl.Segments[firstIndex-2].Literal] {
110-
// Include root-level resource variables immediately after version string.
111-
if isVersionString(*tmpl.Segments[firstIndex-2].Literal) {
112-
firstIndex -= 1
115+
116+
// Case 2: The preceding segment is a variable. It must be paired with a literal.
117+
if prevSeg.Variable != nil {
118+
if firstIndex < 2 || tmpl.Segments[firstIndex-2].Literal == nil {
119+
break // Variable near the beginning, cannot form a pair
120+
}
121+
122+
prevLiteralVal := *tmpl.Segments[firstIndex-2].Literal
123+
124+
// If the literal is a known collection, include the pair and continue
125+
if vocabulary[prevLiteralVal] {
126+
firstIndex -= 2
127+
continue
113128
}
129+
130+
// If the literal is a version string, include the variable but stop here
131+
if isVersionString(prevLiteralVal) {
132+
firstIndex--
133+
}
134+
135+
// The chain is broken, so stop scanning.
114136
break
115137
}
116-
firstIndex -= 2
117138
}
118139
}
119140

internal/sidekick/api/resource_identification_test.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,53 @@ func TestIdentifyTargetResources_Heuristic(t *testing.T) {
266266
Template: ParseTemplateForTest("//test-api.googleapis.com/projects/{project}/zones/{zone}/instances/{instance}"),
267267
},
268268
},
269-
269+
{
270+
name: "heuristic: paths with standalone literals without variables (e.g. global)",
271+
serviceID: ".google.cloud.compute.v1.BackendServices",
272+
path: NewPathTemplate().
273+
WithLiteral("projects").WithVariableNamed("project").
274+
WithLiteral("global").
275+
WithLiteral("backendServices").WithVariableNamed("backend_service"),
276+
fields: []*Field{
277+
{Name: "project", Typez: STRING_TYPE},
278+
{Name: "backend_service", Typez: STRING_TYPE},
279+
},
280+
getPaths: []*PathTemplate{
281+
NewPathTemplate().
282+
WithLiteral("projects").WithVariableNamed("project").
283+
WithLiteral("global").
284+
WithLiteral("backendServices").WithVariableNamed("backend_service"),
285+
},
286+
want: &TargetResource{
287+
FieldPaths: [][]string{{"project"}, {"backend_service"}},
288+
Template: ParseTemplateForTest("//test-api.googleapis.com/projects/{project}/global/backendServices/{backend_service}"),
289+
},
290+
},
291+
{
292+
name: "heuristic: path with non-variable standalone literal",
293+
serviceID: ".google.cloud.example.v1.Service",
294+
path: NewPathTemplate().
295+
WithLiteral("v1").
296+
WithLiteral("projects").
297+
WithLiteral("xyz").
298+
WithLiteral("global").
299+
WithLiteral("foos").WithVariableNamed("foo"),
300+
fields: []*Field{
301+
{Name: "foo", Typez: STRING_TYPE},
302+
},
303+
getPaths: []*PathTemplate{
304+
NewPathTemplate().
305+
WithLiteral("v1").
306+
WithLiteral("projects").
307+
WithLiteral("xyz").
308+
WithLiteral("global").
309+
WithLiteral("foos").WithVariableNamed("foo"),
310+
},
311+
want: &TargetResource{
312+
FieldPaths: [][]string{{"foo"}},
313+
Template: ParseTemplateForTest("//test-api.googleapis.com/projects/xyz/global/foos/{foo}"),
314+
},
315+
},
270316
{
271317
name: "heuristic: paths with un-grouped variable after version string",
272318
serviceID: ".google.cloud.compute.v1.Instances",

0 commit comments

Comments
 (0)