Skip to content

Commit cde8c5f

Browse files
authored
cleanup(sidekick): no synthetic fields (#2357)
We no longer need to label fields as synthetic because either the full message is synthetic or not.
1 parent cb21cf1 commit cde8c5f

File tree

9 files changed

+10
-59
lines changed

9 files changed

+10
-59
lines changed

internal/sidekick/internal/api/model.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -697,10 +697,6 @@ type Field struct {
697697
// IsOneOf is true if the field is related to a one-of and not
698698
// a proto3 optional field.
699699
IsOneOf bool
700-
// The OpenAPI specifications have incomplete `*Request` messages. We inject
701-
// some helper fields. These need to be marked so they can be excluded
702-
// from serialized messages and in other places.
703-
Synthetic bool
704700
// Some fields have a type that refers (sometimes indirectly) to the
705701
// containing message. That triggers slightly different code generation for
706702
// some languages.

internal/sidekick/internal/parser/disco_test.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,11 @@ func TestDisco_ParsePagination(t *testing.T) {
9999
t.Fatalf("expected method %s in the API model", wantID)
100100
}
101101
wantPagination := &api.Field{
102-
Name: "pageToken",
103-
JSONName: "pageToken",
104-
Typez: api.STRING_TYPE,
105-
TypezID: "string",
106-
Optional: true,
107-
Synthetic: true,
102+
Name: "pageToken",
103+
JSONName: "pageToken",
104+
Typez: api.STRING_TYPE,
105+
TypezID: "string",
106+
Optional: true,
108107
}
109108
if diff := cmp.Diff(wantPagination, got.Pagination, cmpopts.IgnoreFields(api.Field{}, "Documentation")); diff != "" {
110109
t.Errorf("mismatch (-want, +got):\n%s", diff)

internal/sidekick/internal/parser/discovery/methods.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ func makeMethod(model *api.API, parent *api.Message, doc *document, input *metho
102102
if err != nil {
103103
return nil, err
104104
}
105-
field.Synthetic = true
106105
field.Optional = !p.Required
107106
requestMessage.Fields = append(requestMessage.Fields, field)
108107
fieldNames[field.Name] = true

internal/sidekick/internal/parser/discovery/methods_test.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,15 +77,13 @@ func TestMethodEmptyBody(t *testing.T) {
7777
Documentation: "Project ID for this request.",
7878
Typez: api.STRING_TYPE,
7979
TypezID: "string",
80-
Synthetic: true,
8180
},
8281
{
8382
Name: "zone",
8483
JSONName: "zone",
8584
Documentation: "Name of the zone resource to return.",
8685
Typez: api.STRING_TYPE,
8786
TypezID: "string",
88-
Synthetic: true,
8987
},
9088
},
9189
}
@@ -122,7 +120,6 @@ func TestMethodWithQueryParameters(t *testing.T) {
122120
Documentation: "A filter expression that filters resources listed in the response. Most Compute resources support two types of filter expressions: expressions that support regular expressions and expressions that follow API improvement proposal AIP-160. These two types of filter expressions cannot be mixed in one request. If you want to use AIP-160, your expression must specify the field name, an operator, and the value that you want to use for filtering. The value must be a string, a number, or a boolean. The operator must be either `=`, `!=`, `>`, `<`, `<=`, `>=` or `:`. For example, if you are filtering Compute Engine instances, you can exclude instances named `example-instance` by specifying `name != example-instance`. The `:*` comparison can be used to test whether a key has been defined. For example, to find all objects with `owner` label use: ``` labels.owner:* ``` You can also filter nested fields. For example, you could specify `scheduling.automaticRestart = false` to include instances only if they are not scheduled for automatic restarts. You can use filtering on nested fields to filter based on resource labels. To filter on multiple expressions, provide each separate expression within parentheses. For example: ``` (scheduling.automaticRestart = true) (cpuPlatform = \"Intel Skylake\") ``` By default, each expression is an `AND` expression. However, you can include `AND` and `OR` expressions explicitly. For example: ``` (cpuPlatform = \"Intel Skylake\") OR (cpuPlatform = \"Intel Broadwell\") AND (scheduling.automaticRestart = true) ``` If you want to use a regular expression, use the `eq` (equal) or `ne` (not equal) operator against a single un-parenthesized expression with or without quotes or against multiple parenthesized expressions. Examples: `fieldname eq unquoted literal` `fieldname eq 'single quoted literal'` `fieldname eq \"double quoted literal\"` `(fieldname1 eq literal) (fieldname2 ne \"literal\")` The literal value is interpreted as a regular expression using Google RE2 library syntax. The literal value must match the entire field. For example, to filter for instances that do not end with name \"instance\", you would use `name ne .*instance`. You cannot combine constraints on multiple fields using regular expressions.",
123121
Typez: api.STRING_TYPE,
124122
TypezID: "string",
125-
Synthetic: true,
126123
Optional: true,
127124
},
128125
{
@@ -131,7 +128,6 @@ func TestMethodWithQueryParameters(t *testing.T) {
131128
Documentation: "The maximum number of results per page that should be returned. If the number of available results is larger than `maxResults`, Compute Engine returns a `nextPageToken` that can be used to get the next page of results in subsequent list requests. Acceptable values are `0` to `500`, inclusive. (Default: `500`)",
132129
Typez: api.UINT32_TYPE,
133130
TypezID: "uint32",
134-
Synthetic: true,
135131
Optional: true,
136132
},
137133
{
@@ -140,7 +136,6 @@ func TestMethodWithQueryParameters(t *testing.T) {
140136
Documentation: "Sorts list results by a certain order. By default, results are returned in alphanumerical order based on the resource name. You can also sort results in descending order based on the creation timestamp using `orderBy=\"creationTimestamp desc\"`. This sorts results based on the `creationTimestamp` field in reverse chronological order (newest result first). Use this to sort resources like operations so that the newest operation is returned first. Currently, only sorting by `name` or `creationTimestamp desc` is supported.",
141137
Typez: api.STRING_TYPE,
142138
TypezID: "string",
143-
Synthetic: true,
144139
Optional: true,
145140
},
146141
{
@@ -149,7 +144,6 @@ func TestMethodWithQueryParameters(t *testing.T) {
149144
Documentation: "Specifies a page token to use. Set `pageToken` to the `nextPageToken` returned by a previous list request to get the next page of results.",
150145
Typez: api.STRING_TYPE,
151146
TypezID: "string",
152-
Synthetic: true,
153147
Optional: true,
154148
},
155149
{
@@ -158,15 +152,13 @@ func TestMethodWithQueryParameters(t *testing.T) {
158152
Documentation: "Project ID for this request.",
159153
Typez: api.STRING_TYPE,
160154
TypezID: "string",
161-
Synthetic: true,
162155
},
163156
{
164157
Name: "returnPartialSuccess",
165158
JSONName: "returnPartialSuccess",
166159
Documentation: "Opt-in for partial success behavior which provides partial results in case of failure. The default value is false. For example, when partial success behavior is enabled, aggregatedList for a single zone scope either returns all resources in the zone or no resources, with an error code.",
167160
Typez: api.BOOL_TYPE,
168161
TypezID: "bool",
169-
Synthetic: true,
170162
Optional: true,
171163
},
172164
},
@@ -204,23 +196,20 @@ func TestMethodWithBody(t *testing.T) {
204196
Documentation: "Project ID for this request.",
205197
Typez: api.STRING_TYPE,
206198
TypezID: "string",
207-
Synthetic: true,
208199
},
209200
{
210201
Name: "region",
211202
JSONName: "region",
212203
Documentation: "Name of the region for this request.",
213204
Typez: api.STRING_TYPE,
214205
TypezID: "string",
215-
Synthetic: true,
216206
},
217207
{
218208
Name: "requestId",
219209
JSONName: "requestId",
220210
Documentation: "An optional request ID to identify requests. Specify a unique request ID so that if you must retry your request, the server will know to ignore the request if it has already been completed. For example, consider a situation where you make an initial request and the request times out. If you make the request again with the same request ID, the server can check if original operation with the same request ID was received, and if so, will ignore the second request. This prevents clients from accidentally creating duplicate commitments. The request ID must be a valid UUID with the exception that zero UUID is not supported ( 00000000-0000-0000-0000-000000000000).",
221211
Typez: api.STRING_TYPE,
222212
TypezID: "string",
223-
Synthetic: true,
224213
Optional: true,
225214
},
226215
},

internal/sidekick/internal/parser/openapi.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,6 @@ func makeRequestMessage(a *api.API, parent *api.Message, operation *v3.Operation
291291
Optional: openapiFieldIsOptional(p),
292292
Typez: typez,
293293
TypezID: typezID,
294-
Synthetic: true,
295294
AutoPopulated: openapiIsAutoPopulated(typez, schema, p),
296295
Behavior: openapiParameterBehavior(p),
297296
}

internal/sidekick/internal/parser/openapi_test.go

Lines changed: 3 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -658,7 +658,6 @@ func TestOpenAPI_MakeAPI(t *testing.T) {
658658
Documentation: "The `{project}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/locations`.",
659659
Typez: api.STRING_TYPE,
660660
TypezID: "string",
661-
Synthetic: true,
662661
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
663662
},
664663
{
@@ -668,10 +667,9 @@ func TestOpenAPI_MakeAPI(t *testing.T) {
668667
"\nThe filtering language accepts strings like `\"displayName=tokyo" +
669668
"\"`, and\nis documented in more detail in [AIP-160](https://google" +
670669
".aip.dev/160).",
671-
Typez: api.STRING_TYPE,
672-
TypezID: "string",
673-
Optional: true,
674-
Synthetic: true,
670+
Typez: api.STRING_TYPE,
671+
TypezID: "string",
672+
Optional: true,
675673
},
676674
{
677675
Name: "pageSize",
@@ -680,7 +678,6 @@ func TestOpenAPI_MakeAPI(t *testing.T) {
680678
Typez: api.INT32_TYPE,
681679
TypezID: "int32",
682680
Optional: true,
683-
Synthetic: true,
684681
},
685682
{
686683
Name: "pageToken",
@@ -689,7 +686,6 @@ func TestOpenAPI_MakeAPI(t *testing.T) {
689686
Typez: api.STRING_TYPE,
690687
TypezID: "string",
691688
Optional: true,
692-
Synthetic: true,
693689
},
694690
},
695691
}
@@ -753,7 +749,6 @@ func TestOpenAPI_MakeAPI(t *testing.T) {
753749
Typez: api.STRING_TYPE,
754750
TypezID: "string",
755751
Optional: true,
756-
Synthetic: true,
757752
},
758753
})
759754

@@ -864,7 +859,6 @@ func TestOpenAPI_SyntheticMessageWithExistingBody(t *testing.T) {
864859
Documentation: "The `{project}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/locations/{location}/secrets/{secret}:setIamPolicy`.",
865860
Typez: api.STRING_TYPE,
866861
TypezID: "string",
867-
Synthetic: true,
868862
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
869863
},
870864
{
@@ -873,7 +867,6 @@ func TestOpenAPI_SyntheticMessageWithExistingBody(t *testing.T) {
873867
Documentation: "The `{location}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/locations/{location}/secrets/{secret}:setIamPolicy`.",
874868
Typez: api.STRING_TYPE,
875869
TypezID: "string",
876-
Synthetic: true,
877870
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
878871
},
879872
{
@@ -882,7 +875,6 @@ func TestOpenAPI_SyntheticMessageWithExistingBody(t *testing.T) {
882875
Documentation: "The `{secret}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/locations/{location}/secrets/{secret}:setIamPolicy`.",
883876
Typez: api.STRING_TYPE,
884877
TypezID: "string",
885-
Synthetic: true,
886878
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
887879
},
888880
{
@@ -914,7 +906,6 @@ func TestOpenAPI_SyntheticMessageWithExistingBody(t *testing.T) {
914906
Documentation: "The `{project}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/secrets/{secret}:setIamPolicy`.",
915907
Typez: api.STRING_TYPE,
916908
TypezID: "string",
917-
Synthetic: true,
918909
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
919910
},
920911
{
@@ -923,7 +914,6 @@ func TestOpenAPI_SyntheticMessageWithExistingBody(t *testing.T) {
923914
Documentation: "The `{secret}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/secrets/{secret}:setIamPolicy`.",
924915
Typez: api.STRING_TYPE,
925916
TypezID: "string",
926-
Synthetic: true,
927917
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
928918
},
929919
{
@@ -992,7 +982,6 @@ func TestOpenAPI_Pagination(t *testing.T) {
992982
Typez: api.STRING_TYPE,
993983
TypezID: "string",
994984
Optional: true,
995-
Synthetic: true,
996985
},
997986
},
998987
},
@@ -1087,7 +1076,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
10871076
Documentation: "Test-only Description",
10881077
Typez: api.STRING_TYPE,
10891078
TypezID: "string",
1090-
Synthetic: true,
10911079
Optional: true,
10921080
AutoPopulated: true,
10931081
}
@@ -1097,7 +1085,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
10971085
Documentation: "Test-only Description",
10981086
Typez: api.STRING_TYPE,
10991087
TypezID: "string",
1100-
Synthetic: true,
11011088
Optional: true,
11021089
AutoPopulated: true,
11031090
}
@@ -1114,7 +1101,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
11141101
Documentation: "The `{project}` component of the target path.\n\nThe full target path will be in the form `/v1/projects/{project}/foos`.",
11151102
Typez: api.STRING_TYPE,
11161103
TypezID: "string",
1117-
Synthetic: true,
11181104
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
11191105
},
11201106
{
@@ -1123,7 +1109,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
11231109
Documentation: "Test-only Description",
11241110
Typez: api.STRING_TYPE,
11251111
TypezID: "string",
1126-
Synthetic: true,
11271112
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
11281113
},
11291114
request_id,
@@ -1134,7 +1119,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
11341119
Typez: api.STRING_TYPE,
11351120
TypezID: "string",
11361121
JSONName: "notRequestIdRequired",
1137-
Synthetic: true,
11381122
Behavior: []api.FieldBehavior{api.FIELD_BEHAVIOR_REQUIRED},
11391123
},
11401124
{
@@ -1143,7 +1127,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
11431127
Typez: api.STRING_TYPE,
11441128
TypezID: "string",
11451129
JSONName: "notRequestIdMissingFormat",
1146-
Synthetic: true,
11471130
Optional: true,
11481131
},
11491132
{
@@ -1152,7 +1135,6 @@ func TestOpenAPI_AutoPopulated(t *testing.T) {
11521135
Typez: api.STRING_TYPE,
11531136
TypezID: "string",
11541137
JSONName: "notRequestIdMissingServiceConfig",
1155-
Synthetic: true,
11561138
Optional: true,
11571139
// This just denotes that the field is eligible
11581140
// to be auto-populated

internal/sidekick/internal/rust/annotate.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,6 @@ type messageAnnotation struct {
183183
HasNestedTypes bool
184184
// All the fields except OneOfs.
185185
BasicFields []*api.Field
186-
// If true, this is a synthetic message, some generation is skipped for
187-
// synthetic messages
188-
HasSyntheticFields bool
189186
// If set, this message is only enabled when some features are enabled.
190187
FeatureGates []string
191188
FeatureGatesOp string
@@ -720,21 +717,13 @@ func (c *codec) annotateMessage(m *api.Message, model *api.API, full bool) {
720717
for _, child := range m.Messages {
721718
c.annotateMessage(child, model, true)
722719
}
723-
hasSyntheticFields := false
724-
for _, f := range m.Fields {
725-
if f.Synthetic {
726-
hasSyntheticFields = true
727-
break
728-
}
729-
}
730720
basicFields := language.FilterSlice(m.Fields, func(f *api.Field) bool {
731721
return !f.IsOneOf
732722
})
733723

734724
annotations.DocLines = c.formatDocComments(m.Documentation, m.ID, model.State, m.Scopes())
735725
annotations.HasNestedTypes = language.HasNestedTypes(m)
736726
annotations.BasicFields = basicFields
737-
annotations.HasSyntheticFields = hasSyntheticFields
738727
annotations.Internal = slices.Contains(c.internalTypes, m.ID)
739728
}
740729

internal/sidekick/internal/rust/templates/common/message.mustache

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,15 +214,15 @@ impl {{Codec.Name}} {
214214
{{/Fields}}
215215
{{/OneOfs}}
216216
}
217-
{{^Codec.HasSyntheticFields}}
217+
{{^SyntheticRequest}}
218218

219219
{{> /templates/common/feature_gate}}
220220
impl wkt::message::Message for {{Codec.Name}} {
221221
fn typename() -> &'static str {
222222
"type.googleapis.com/{{Codec.SourceFQN}}"
223223
}
224224
}
225-
{{/Codec.HasSyntheticFields}}
225+
{{/SyntheticRequest}}
226226
{{#Pagination}}
227227
228228
{{> /templates/common/feature_gate}}

internal/sidekick/internal/rust/templates/common/model/serialize/message.mustache

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,12 @@ impl serde::ser::Serialize for super::{{Codec.RelativeName}} {
2828
use serde::ser::SerializeMap;
2929
let mut state = serializer.serialize_map(std::option::Option::None)?;
3030
{{#Fields}}
31-
{{^Synthetic}}
3231
{{^Codec.RequiresSerdeAs}}
3332
{{> /templates/common/ser_plain_message_field}}
3433
{{/Codec.RequiresSerdeAs}}
3534
{{#Codec.RequiresSerdeAs}}
3635
{{> /templates/common/ser_with_message_field}}
3736
{{/Codec.RequiresSerdeAs}}
38-
{{/Synthetic}}
3937
{{/Fields}}
4038
if !self._unknown_fields.is_empty() {
4139
for (key, value) in self._unknown_fields.iter() {

0 commit comments

Comments
 (0)