Skip to content

Commit 751eac2

Browse files
haphungwhaphungw
authored
feat(sidekick): implement explicit target resource identification (#4105)
Implement Priority 1 of the resource naming logic. Identify the target resource by resolving path variables to input message fields and checking for `google.api.resource_reference` annotations. Similar logic does exist but resides in the Rust generator (`internal/sidekick/rust/annotate.go`, `findResourceNameCandidates`). Later, Rust will use this instead for resource naming. Fixes #4099
1 parent 5f43847 commit 751eac2

5 files changed

Lines changed: 235 additions & 18 deletions

File tree

‎internal/sidekick/api/model.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,4 @@ type TargetResource struct {
15161516
// FieldPaths is a list of field name sequences that, when joined, form a resource name.
15171517
// For example, [["project"], ["zone"], ["instance"]] identifies a multi-part resource.
15181518
FieldPaths [][]string
1519-
// PathTemplate is the template for the resource name.
1520-
// It uses AIP-122 style placeholders, e.g., "projects/{projects}/zones/{zones}/instances/{instance}".
1521-
PathTemplate *PathTemplate
15221519
}

‎internal/sidekick/api/model_test.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,6 @@ func TestPathTemplateBuilder(t *testing.T) {
237237
func TestPathBindingHeuristic(t *testing.T) {
238238
heuristic := &TargetResource{
239239
FieldPaths: [][]string{{"project"}, {"zone"}, {"instance"}},
240-
PathTemplate: NewPathTemplate().
241-
WithLiteral("projects").WithVariableNamed("project").
242-
WithLiteral("zones").WithVariableNamed("zone").
243-
WithLiteral("instances").WithVariableNamed("instance"),
244240
}
245241
binding := &PathBinding{
246242
Verb: "GET",

‎internal/sidekick/api/resource_identification.go‎

Lines changed: 80 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@
1414

1515
package api
1616

17+
import (
18+
"fmt"
19+
"slices"
20+
)
21+
1722
// IdentifyTargetResources populates the TargetResource field in PathBinding
1823
// for all methods in the API.
1924
//
@@ -22,38 +27,104 @@ package api
2227
// with fields present in the PathTemplate.
2328
// 2. Heuristic Identification: For allow-listed services, uses path segment
2429
// patterns to identify resources when annotations are missing.
25-
func IdentifyTargetResources(model *API) {
30+
func IdentifyTargetResources(model *API) error {
2631
for _, service := range model.Services {
2732
for _, method := range service.Methods {
2833
if method.PathInfo == nil {
2934
continue
3035
}
3136
for _, binding := range method.PathInfo.Bindings {
32-
identifyTargetResourceForBinding(method, binding)
37+
if err := identifyTargetResourceForBinding(method, binding); err != nil {
38+
return err
39+
}
3340
}
3441
}
3542
}
43+
return nil
3644
}
3745

3846
// identifyTargetResourceForBinding processes a single path binding to identify its target resource.
39-
func identifyTargetResourceForBinding(method *Method, binding *PathBinding) {
47+
func identifyTargetResourceForBinding(method *Method, binding *PathBinding) error {
4048
if binding.PathTemplate == nil {
41-
return
49+
return nil
4250
}
4351

4452
// Priority 1: Explicit Identification
4553
// Matches google.api.resource_reference annotations.
46-
if target := identifyExplicitTarget(method, binding); target != nil {
54+
target, err := identifyExplicitTarget(method, binding)
55+
if err != nil {
56+
return err
57+
}
58+
if target != nil {
4759
binding.TargetResource = target
48-
return
60+
return nil
4961
}
5062

5163
// Priority 2: Heuristic Identification
5264
// Uses path segment patterns to guess the resource.
5365
// TODO(#4100): Implement IdentifyTargetResources for allow-listed services using heuristic path segment patterns.
66+
return nil
5467
}
5568

56-
func identifyExplicitTarget(_ *Method, _ *PathBinding) *TargetResource {
57-
// TODO(#4099): Implement IdentifyTargetResources for consistent explicit resource identification in the sidekick parser.
58-
return nil
69+
func identifyExplicitTarget(method *Method, binding *PathBinding) (*TargetResource, error) {
70+
var fieldPaths [][]string
71+
if method.InputType == nil {
72+
return nil, fmt.Errorf("consistency error: method %q has no InputType", method.Name)
73+
}
74+
75+
// Collect field paths corresponding to variable segments in the path template
76+
for _, segment := range binding.PathTemplate.Segments {
77+
if segment.Variable == nil {
78+
continue
79+
}
80+
81+
fieldPath := segment.Variable.FieldPath
82+
field, err := findField(method.InputType, fieldPath)
83+
if err != nil {
84+
return nil, err
85+
}
86+
if field == nil {
87+
return nil, fmt.Errorf("consistency error: field %v not found in message %q", fieldPath, method.InputType.Name)
88+
}
89+
if !field.IsResourceReference() {
90+
return nil, nil
91+
}
92+
fieldPaths = append(fieldPaths, fieldPath)
93+
}
94+
95+
if len(fieldPaths) == 0 {
96+
return nil, nil
97+
}
98+
return &TargetResource{
99+
FieldPaths: fieldPaths,
100+
}, nil
101+
}
102+
103+
// findField traverses the (nested) message structure to find a field by its field path.
104+
func findField(msg *Message, path []string) (*Field, error) {
105+
if len(path) == 0 {
106+
return nil, fmt.Errorf("consistency error: empty field path in msg: %s", msg.ID)
107+
}
108+
109+
current := msg
110+
var field *Field
111+
112+
for i, name := range path {
113+
idx := slices.IndexFunc(current.Fields, func(f *Field) bool {
114+
return f.Name == name
115+
})
116+
if idx == -1 {
117+
return nil, fmt.Errorf("consistency error: field %s not found in message %q", name, current.Name)
118+
}
119+
field = current.Fields[idx]
120+
121+
if i < len(path)-1 {
122+
if field.MessageType == nil {
123+
return nil, fmt.Errorf("consistency error: field %s in message %s has no MessageType", field.Name, current.Name)
124+
}
125+
current = field.MessageType
126+
}
127+
}
128+
129+
return field, nil
59130
}

‎internal/sidekick/api/resource_identification_test.go‎

Lines changed: 152 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,159 @@ package api
1616

1717
import (
1818
"testing"
19+
20+
"github.com/google/go-cmp/cmp"
1921
)
2022

23+
// setupTestModel helper creates a minimal API model for testing resource identification.
24+
func setupTestModel(serviceID string, pathTemplate *PathTemplate, fields []*Field) (*API, *PathBinding) {
25+
binding := &PathBinding{PathTemplate: pathTemplate}
26+
method := &Method{
27+
Name: "TestMethod",
28+
InputType: &Message{
29+
Fields: fields,
30+
},
31+
PathInfo: &PathInfo{
32+
Bindings: []*PathBinding{binding},
33+
},
34+
}
35+
service := &Service{
36+
ID: serviceID,
37+
Methods: []*Method{method},
38+
}
39+
model := &API{
40+
Services: []*Service{service},
41+
}
42+
return model, binding
43+
}
44+
2145
func TestIdentifyTargetResources(t *testing.T) {
22-
// TODO(#4099): Implement IdentifyTargetResources for consistent explicit resource identification in the sidekick parser.
46+
for _, test := range []struct {
47+
name string
48+
serviceID string
49+
path *PathTemplate
50+
fields []*Field
51+
want *TargetResource
52+
}{
53+
{
54+
name: "explicit: standard resource reference",
55+
serviceID: "any.service",
56+
path: NewPathTemplate().
57+
WithLiteral("projects").WithVariableNamed("project"),
58+
fields: []*Field{
59+
{
60+
Name: "project",
61+
Typez: STRING_TYPE,
62+
ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
63+
},
64+
},
65+
want: &TargetResource{
66+
FieldPaths: [][]string{{"project"}},
67+
},
68+
},
69+
{
70+
name: "explicit: multiple resource references",
71+
serviceID: "any.service",
72+
path: NewPathTemplate().
73+
WithLiteral("projects").WithVariableNamed("project").
74+
WithLiteral("locations").WithVariableNamed("location"),
75+
fields: []*Field{
76+
{
77+
Name: "project",
78+
Typez: STRING_TYPE,
79+
ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
80+
},
81+
{
82+
Name: "location",
83+
Typez: STRING_TYPE, // Often locations are string IDs
84+
ResourceReference: &ResourceReference{Type: "locations.googleapis.com/Location"},
85+
},
86+
},
87+
want: &TargetResource{
88+
FieldPaths: [][]string{{"project"}, {"location"}},
89+
},
90+
},
91+
{
92+
name: "explicit: nested field reference",
93+
serviceID: "any.service",
94+
path: NewPathTemplate().
95+
WithLiteral("projects").WithVariableNamed("parent", "project"),
96+
fields: []*Field{
97+
{
98+
Name: "parent",
99+
Typez: MESSAGE_TYPE,
100+
MessageType: &Message{
101+
Fields: []*Field{
102+
{
103+
Name: "project",
104+
Typez: STRING_TYPE,
105+
ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
106+
},
107+
},
108+
},
109+
},
110+
},
111+
want: &TargetResource{
112+
FieldPaths: [][]string{{"parent", "project"}},
113+
},
114+
},
115+
} {
116+
t.Run(test.name, func(t *testing.T) {
117+
model, binding := setupTestModel(test.serviceID, test.path, test.fields)
118+
IdentifyTargetResources(model)
119+
120+
got := binding.TargetResource
121+
if diff := cmp.Diff(test.want, got); diff != "" {
122+
t.Errorf("mismatch (-want +got):\n%s", diff)
123+
}
124+
})
125+
}
126+
}
127+
128+
func TestIdentifyTargetResources_NoMatch(t *testing.T) {
129+
for _, test := range []struct {
130+
name string
131+
serviceID string
132+
path *PathTemplate
133+
fields []*Field
134+
}{
135+
{
136+
name: "Explicit: missing reference returns nil",
137+
serviceID: "any.service",
138+
path: NewPathTemplate().
139+
WithLiteral("projects").WithVariableNamed("project"),
140+
fields: []*Field{
141+
{Name: "project", Typez: STRING_TYPE}, // No ResourceReference
142+
},
143+
},
144+
{
145+
name: "Explicit: partial reference returns nil",
146+
serviceID: "any.service",
147+
path: NewPathTemplate().
148+
WithLiteral("projects").WithVariableNamed("project").
149+
WithLiteral("glossaries").WithVariableNamed("glossary"),
150+
fields: []*Field{
151+
{
152+
Name: "project",
153+
Typez: STRING_TYPE,
154+
ResourceReference: &ResourceReference{Type: "cloudresourcemanager.googleapis.com/Project"},
155+
},
156+
{
157+
Name: "glossary",
158+
Typez: STRING_TYPE,
159+
// No ResourceReference on the second variable
160+
},
161+
},
162+
},
163+
} {
164+
t.Run(test.name, func(t *testing.T) {
165+
model, binding := setupTestModel(test.serviceID, test.path, test.fields)
166+
IdentifyTargetResources(model)
167+
168+
got := binding.TargetResource
169+
if got != nil {
170+
t.Errorf("IdentifyTargetResources() = %v, want nil", got)
171+
}
172+
})
173+
}
23174
}

‎internal/sidekick/parser/parser.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ func CreateModel(cfg *ModelConfig) (*api.API, error) {
7979
if err := api.CrossReference(model); err != nil {
8080
return nil, err
8181
}
82-
api.IdentifyTargetResources(model)
82+
if err := api.IdentifyTargetResources(model); err != nil {
83+
return nil, err
84+
}
8385
if err := api.SkipModelElements(model, cfg.Override); err != nil {
8486
return nil, err
8587
}

0 commit comments

Comments
 (0)