Skip to content

Commit 589e5ec

Browse files
haphungwhaphungw
authored
feat(sidekick/rust): revert: decouple option extraction and support resource names (#4352) (#4364)
Revert #4352
1 parent d81b502 commit 589e5ec

3 files changed

Lines changed: 16 additions & 160 deletions

File tree

‎internal/sidekick/rust/annotate.go‎

Lines changed: 10 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -416,11 +416,6 @@ type pathBindingAnnotation struct {
416416

417417
// The codec is configured to generated detailed tracing attributes.
418418
DetailedTracingAttributes bool
419-
420-
// Resource name generation fields, propagated from method scope.
421-
HasResourceNameGeneration bool
422-
ResourceNameTemplate string
423-
ResourceNameArgs []string
424419
}
425420

426421
// QueryParamsCanFail returns true if we serialize certain query parameters, which can fail. The code we generate
@@ -1613,36 +1608,21 @@ func (c *codec) annotateResourceNameGeneration(m *api.Method, annotation *method
16131608
if m.PathInfo != nil {
16141609
for _, b := range m.PathInfo.Bindings {
16151610
if b.TargetResource != nil {
1616-
annotation.HasResourceNameGeneration = true
1617-
break
1618-
}
1619-
}
1620-
1621-
if annotation.HasResourceNameGeneration {
1622-
for _, b := range m.PathInfo.Bindings {
1623-
bAnn, ok := b.Codec.(*pathBindingAnnotation)
1624-
if !ok {
1625-
continue
1611+
tmpl, err := formatResourceNameTemplateFromPath(m, b)
1612+
if err != nil {
1613+
return err
16261614
}
1627-
// To make sure the Rust code for each binding returns the same type, we set HasResourceNameGeneration = true for all bindings to cue each binding to produce the same typed result (even if this specific binding does not have a TargetResource.)
1628-
bAnn.HasResourceNameGeneration = true
1629-
1630-
if b.TargetResource != nil {
1631-
tmpl, err := formatResourceNameTemplateFromPath(m, b)
1615+
annotation.ResourceNameTemplate = tmpl
1616+
for _, path := range b.TargetResource.FieldPaths {
1617+
accSegments, err := makeAccessors(path, m)
16321618
if err != nil {
16331619
return err
16341620
}
1635-
bAnn.ResourceNameTemplate = tmpl
1636-
bAnn.ResourceNameArgs = formatResourceNameArgs(b.TargetResource.FieldPaths)
1637-
1638-
if annotation.ResourceNameTemplate == "" {
1639-
annotation.ResourceNameTemplate = bAnn.ResourceNameTemplate
1640-
annotation.ResourceNameArgs = bAnn.ResourceNameArgs
1641-
}
1642-
} else {
1643-
bAnn.ResourceNameTemplate = ""
1644-
bAnn.ResourceNameArgs = nil
1621+
fullAcc := "Some(&req)" + strings.Join(accSegments, "") + ".unwrap_or(\"\")"
1622+
annotation.ResourceNameArgs = append(annotation.ResourceNameArgs, fullAcc)
16451623
}
1624+
annotation.HasResourceNameGeneration = true
1625+
break
16461626
}
16471627
}
16481628
}
@@ -1671,20 +1651,6 @@ func formatResourceNameTemplateFromPath(m *api.Method, b *api.PathBinding) (stri
16711651
return sb.String(), nil
16721652
}
16731653

1674-
// formatResourceNameArgs creates the corresponding Rust template variables for the resource name.
1675-
func formatResourceNameArgs(fieldPaths [][]string) []string {
1676-
var args []string
1677-
for _, path := range fieldPaths {
1678-
var rustNames []string
1679-
for _, p := range path {
1680-
rustNames = append(rustNames, toSnakeNoMangling(p))
1681-
}
1682-
varName := fmt.Sprintf("var_%s", strings.Join(rustNames, "_"))
1683-
args = append(args, varName)
1684-
}
1685-
return args
1686-
}
1687-
16881654
// isIdempotent returns "true" if the method is idempotent by default, and "false", if not.
16891655
func isIdempotent(p *api.PathInfo) string {
16901656
if len(p.Bindings) == 0 {

‎internal/sidekick/rust/annotate_method_test.go‎

Lines changed: 6 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -197,9 +197,6 @@ func annotateMethodModel(t *testing.T) *api.API {
197197
{Name: "project", ID: ".test.v1.Request.project", Typez: api.STRING_TYPE},
198198
{Name: "zone", ID: ".test.v1.Request.zone", Typez: api.STRING_TYPE},
199199
{Name: "type", ID: ".test.v1.Request.type", Typez: api.STRING_TYPE},
200-
{Name: "name", ID: ".test.v1.Request.name", Typez: api.STRING_TYPE},
201-
{Name: "location", ID: ".test.v1.Request.location", Typez: api.STRING_TYPE},
202-
{Name: "cluster", ID: ".test.v1.Request.cluster", Typez: api.STRING_TYPE},
203200
},
204201
}
205202
response := &api.Message{
@@ -314,61 +311,26 @@ func TestAnnotateMethodResourceNameTemplate(t *testing.T) {
314311
{"type"},
315312
})
316313

317-
// Setup: Inject multiple bindings for the "Self" method
318-
mSelf := model.State.MethodByID[".test.v1.ResourceService.Self"]
319-
mSelf.PathInfo.Bindings = []*api.PathBinding{
320-
{
321-
Verb: "GET",
322-
PathTemplate: api.NewPathTemplate().
323-
WithLiteral("v1").
324-
WithVariableNamed("name"),
325-
TargetResource: &api.TargetResource{
326-
Template: api.ParseTemplateForTest("//Test.googleapis.com/projects/{project}/locations/{location}/clusters/{cluster}"),
327-
FieldPaths: [][]string{{"name"}},
328-
},
329-
},
330-
{
331-
Verb: "GET",
332-
PathTemplate: api.NewPathTemplate().
333-
WithLiteral("v1").
334-
WithLiteral("projects").
335-
WithVariableNamed("project").
336-
WithLiteral("locations").
337-
WithVariableNamed("location").
338-
WithLiteral("clusters").
339-
WithVariableNamed("cluster"),
340-
TargetResource: &api.TargetResource{
341-
Template: api.ParseTemplateForTest("//Test.googleapis.com/projects/{project}/locations/{location}/clusters/{cluster}"),
342-
FieldPaths: [][]string{{"project"}, {"location"}, {"cluster"}},
343-
},
344-
},
345-
{
346-
Verb: "GET",
347-
PathTemplate: api.NewPathTemplate(),
348-
},
349-
}
350-
351314
codec := newTestCodec(t, libconfig.SpecProtobuf, "", map[string]string{})
352315
_, err = annotateModel(model, codec)
353316
if err != nil {
354317
t.Fatal(err)
355318
}
356319

357320
tests := []struct {
358-
name string
359-
id string
360-
want *methodAnnotation
361-
wantBindings []*pathBindingAnnotation
321+
name string
322+
id string
323+
want *methodAnnotation
362324
}{
363325
{
364326
name: "WithTargetResource",
365327
id: ".test.v1.ResourceService.move",
366328
want: &methodAnnotation{
367329
ResourceNameTemplate: "//Test.googleapis.com/projects/{}/zones/{}/types/{}",
368330
ResourceNameArgs: []string{
369-
"var_project",
370-
"var_zone",
371-
"var_type",
331+
"Some(&req).map(|m| &m.project).map(|s| s.as_str()).unwrap_or(\"\")",
332+
"Some(&req).map(|m| &m.zone).map(|s| s.as_str()).unwrap_or(\"\")",
333+
"Some(&req).map(|m| &m.r#type).map(|s| s.as_str()).unwrap_or(\"\")",
372334
},
373335
HasResourceNameGeneration: true,
374336
},
@@ -380,34 +342,6 @@ func TestAnnotateMethodResourceNameTemplate(t *testing.T) {
380342
HasResourceNameGeneration: false,
381343
},
382344
},
383-
{
384-
name: "MultipleBindings",
385-
id: ".test.v1.ResourceService.Self",
386-
want: &methodAnnotation{
387-
ResourceNameTemplate: "//Test.googleapis.com/projects/{}/locations/{}/clusters/{}",
388-
ResourceNameArgs: []string{
389-
"var_name",
390-
},
391-
HasResourceNameGeneration: true,
392-
},
393-
wantBindings: []*pathBindingAnnotation{
394-
{
395-
HasResourceNameGeneration: true,
396-
ResourceNameTemplate: "//Test.googleapis.com/projects/{}/locations/{}/clusters/{}",
397-
ResourceNameArgs: []string{"var_name"},
398-
},
399-
{
400-
HasResourceNameGeneration: true,
401-
ResourceNameTemplate: "//Test.googleapis.com/projects/{}/locations/{}/clusters/{}",
402-
ResourceNameArgs: []string{"var_project", "var_location", "var_cluster"},
403-
},
404-
{
405-
HasResourceNameGeneration: true,
406-
ResourceNameTemplate: "",
407-
ResourceNameArgs: nil,
408-
},
409-
},
410-
},
411345
}
412346

413347
for _, tc := range tests {
@@ -425,15 +359,6 @@ func TestAnnotateMethodResourceNameTemplate(t *testing.T) {
425359
"HasResourceNameFields", "InternalBuilders")); diff != "" {
426360
t.Errorf("mismatch (-want, +got):\n%s", diff)
427361
}
428-
429-
if tc.wantBindings != nil {
430-
for i, wantBinding := range tc.wantBindings {
431-
gotBinding := m.PathInfo.Bindings[i].Codec.(*pathBindingAnnotation)
432-
if diff := cmp.Diff(wantBinding, gotBinding, cmpopts.IgnoreFields(pathBindingAnnotation{}, "DetailedTracingAttributes", "PathFmt", "QueryParams", "Substitutions")); diff != "" {
433-
t.Errorf("binding %d mismatch (-want, +got):\n%s", i, diff)
434-
}
435-
}
436-
}
437362
})
438363
}
439364
}

‎internal/sidekick/rust/templates/crate/src/transport.rs.mustache‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -95,22 +95,12 @@ impl super::stub::{{Codec.Name}} for {{Codec.Name}} {
9595
true,
9696
);
9797
{{/HasAutoPopulatedFields}}
98-
{{#Codec.HasResourceNameGeneration}}
99-
{{#Codec.DetailedTracingAttributes}}
100-
let (builder, method, _path_template, _resource_name) = None
101-
{{/Codec.DetailedTracingAttributes}}
102-
{{^Codec.DetailedTracingAttributes}}
103-
let (builder, method, _resource_name) = None
104-
{{/Codec.DetailedTracingAttributes}}
105-
{{/Codec.HasResourceNameGeneration}}
106-
{{^Codec.HasResourceNameGeneration}}
10798
{{#Codec.DetailedTracingAttributes}}
10899
let (builder, method, _path_template) = None
109100
{{/Codec.DetailedTracingAttributes}}
110101
{{^Codec.DetailedTracingAttributes}}
111102
let (builder, method) = None
112103
{{/Codec.DetailedTracingAttributes}}
113-
{{/Codec.HasResourceNameGeneration}}
114104
{{#PathInfo.Bindings}}
115105
.or_else(|| {
116106
{{#Codec.HasVariablePath}}
@@ -131,15 +121,6 @@ impl super::stub::{{Codec.Name}} for {{Codec.Name}} {
131121
let path_template = "{{Codec.PathTemplate}}";
132122
{{/Codec.DetailedTracingAttributes}}
133123
134-
{{#Codec.HasResourceNameGeneration}}
135-
let _resource_name = format!(
136-
"{{{Codec.ResourceNameTemplate}}}",
137-
{{#Codec.ResourceNameArgs}}
138-
{{{.}}},
139-
{{/Codec.ResourceNameArgs}}
140-
);
141-
{{/Codec.HasResourceNameGeneration}}
142-
143124
let builder = self.inner.builder(Method::{{Verb}}, path);
144125
{{#Codec.QueryParamsCanFail}}
145126
let builder = (|| {
@@ -155,22 +136,12 @@ impl super::stub::{{Codec.Name}} for {{Codec.Name}} {
155136
{{/Codec.QueryParams}}
156137
let builder = Ok(builder);
157138
{{/Codec.QueryParamsCanFail}}
158-
{{#Codec.HasResourceNameGeneration}}
159-
{{#Codec.DetailedTracingAttributes}}
160-
Some(builder.map(|b| (b, Method::{{Verb}}, path_template, _resource_name)))
161-
{{/Codec.DetailedTracingAttributes}}
162-
{{^Codec.DetailedTracingAttributes}}
163-
Some(builder.map(|b| (b, Method::{{Verb}}, _resource_name)))
164-
{{/Codec.DetailedTracingAttributes}}
165-
{{/Codec.HasResourceNameGeneration}}
166-
{{^Codec.HasResourceNameGeneration}}
167139
{{#Codec.DetailedTracingAttributes}}
168140
Some(builder.map(|b| (b, Method::{{Verb}}, path_template)))
169141
{{/Codec.DetailedTracingAttributes}}
170142
{{^Codec.DetailedTracingAttributes}}
171143
Some(builder.map(|b| (b, Method::{{Verb}})))
172144
{{/Codec.DetailedTracingAttributes}}
173-
{{/Codec.HasResourceNameGeneration}}
174145
})
175146
{{/PathInfo.Bindings}}
176147
.ok_or_else(|| {
@@ -207,12 +178,6 @@ impl super::stub::{{Codec.Name}} for {{Codec.Name}} {
207178
options.insert_extension(PathTemplate(_path_template))
208179
};
209180
{{/Codec.DetailedTracingAttributes}}
210-
{{#Codec.HasResourceNameGeneration}}
211-
let options = {
212-
use google_cloud_gax::options::internal::{RequestOptionsExt, ResourceName};
213-
options.insert_extension(ResourceName(_resource_name))
214-
};
215-
{{/Codec.HasResourceNameGeneration}}
216181
let options = google_cloud_gax::options::internal::set_default_idempotency(
217182
options,
218183
{{! TODO(#4156) - return idempotency from the above closure }}

0 commit comments

Comments
 (0)