Skip to content

Commit f2d1a10

Browse files
coryancoryan
authored
feat(sidekick/disco): parse most object fields (#2318)
Parse most fields of object in a discovery doc. Fields with an inline type definition still need some custom work.
1 parent 1c71bd9 commit f2d1a10

6 files changed

Lines changed: 73 additions & 33 deletions

File tree

‎internal/sidekick/internal/parser/discovery/discovery.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func NewAPI(serviceConfig *serviceconfig.Service, contents []byte) (*api.API, er
7272
if schema.Type != "object" {
7373
return nil, fmt.Errorf("schema %s is not an object: %q", id, schema.Type)
7474
}
75-
fields, err := makeMessageFields(id, schema)
75+
fields, err := makeMessageFields(result, id, schema)
7676
if err != nil {
7777
return nil, err
7878
}

‎internal/sidekick/internal/parser/discovery/discovery_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,14 +146,14 @@ func TestMessage(t *testing.T) {
146146
Typez: api.STRING_TYPE,
147147
TypezID: "string",
148148
},
149-
// TODO(#1850) - parse object fields.
150-
// {
151-
// Name: "headerAction",
152-
// JSONName: "headerAction",
153-
// Documentation: "Specifies changes to request and response headers that need to take effect for the selected backendService. headerAction specified here take effect before headerAction in the enclosing HttpRouteRule, PathMatcher and UrlMap. headerAction is not supported for load balancers that have their loadBalancingScheme set to EXTERNAL. Not supported when the URL map is bound to a target gRPC proxy that has validateForProxyless field set to true.",
154-
// Typez: api.MESSAGE_TYPE,
155-
// TypezID: "..HttpHeaderAction",
156-
// },
149+
{
150+
Name: "headerAction",
151+
JSONName: "headerAction",
152+
Documentation: "Specifies changes to request and response headers that need to take effect for the selected backendService. headerAction specified here take effect before headerAction in the enclosing HttpRouteRule, PathMatcher and UrlMap. headerAction is not supported for load balancers that have their loadBalancingScheme set to EXTERNAL. Not supported when the URL map is bound to a target gRPC proxy that has validateForProxyless field set to true.",
153+
Typez: api.MESSAGE_TYPE,
154+
TypezID: "..HttpHeaderAction",
155+
Optional: true,
156+
},
157157
{
158158
Name: "weight",
159159
JSONName: "weight",

‎internal/sidekick/internal/parser/discovery/message_fields.go‎

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ import (
2020
"github.com/googleapis/librarian/internal/sidekick/internal/api"
2121
)
2222

23-
func makeMessageFields(messageID string, schema *schema) ([]*api.Field, error) {
23+
func makeMessageFields(model *api.API, messageID string, schema *schema) ([]*api.Field, error) {
2424
var fields []*api.Field
2525
for _, input := range schema.Properties {
26-
field, err := makeField(messageID, input)
26+
field, err := makeField(model, messageID, input)
2727
if err != nil {
2828
return nil, err
2929
}
@@ -35,21 +35,24 @@ func makeMessageFields(messageID string, schema *schema) ([]*api.Field, error) {
3535
return fields, nil
3636
}
3737

38-
func makeField(messageID string, input *property) (*api.Field, error) {
39-
switch input.Schema.Type {
40-
case "":
38+
func makeField(model *api.API, messageID string, input *property) (*api.Field, error) {
39+
if input.Schema.Type == "array" {
40+
// TODO(#2266) - handle array fields
4141
return nil, nil
42-
case "array":
42+
}
43+
if input.Schema.AdditionalProperties != nil {
44+
// TODO(#2283) - handle map fields
4345
return nil, nil
44-
case "object":
46+
}
47+
if input.Schema.Type == "object" && input.Schema.Properties != nil {
48+
// TODO(#2265) - handle inline object...
4549
return nil, nil
46-
default:
47-
return makeScalarField(messageID, input)
4850
}
51+
return makeScalarField(model, messageID, input)
4952
}
5053

51-
func makeScalarField(messageID string, input *property) (*api.Field, error) {
52-
typez, typezID, err := scalarType(messageID, input)
54+
func makeScalarField(model *api.API, messageID string, input *property) (*api.Field, error) {
55+
typez, typezID, err := scalarType(model, messageID, input)
5356
if err != nil {
5457
return nil, err
5558
}
@@ -59,14 +62,18 @@ func makeScalarField(messageID string, input *property) (*api.Field, error) {
5962
Documentation: input.Schema.Description,
6063
Typez: typez,
6164
TypezID: typezID,
62-
// TODO(#1850) - deprecated fields?
63-
// TODO(#1850) - optional fields?
65+
// TODO(#2268) - deprecated fields?
66+
// TODO(#2270) - optional fields?
67+
Optional: typez == api.MESSAGE_TYPE,
6468
}, nil
6569
}
6670

67-
func scalarType(messageID string, input *property) (api.Typez, string, error) {
71+
func scalarType(model *api.API, messageID string, input *property) (api.Typez, string, error) {
72+
if input.Schema.Type == "" && input.Schema.Ref != "" {
73+
typezID := fmt.Sprintf(".%s.%s", model.PackageName, input.Schema.Ref)
74+
return api.MESSAGE_TYPE, typezID, nil
75+
}
6876
switch input.Schema.Type {
69-
// TODO(#1850) - handle "any", "object":
7077
case "boolean":
7178
return api.BOOL_TYPE, "bool", nil
7279
case "integer":
@@ -75,8 +82,12 @@ func scalarType(messageID string, input *property) (api.Typez, string, error) {
7582
return scalarTypeForNumberFormats(messageID, input)
7683
case "string":
7784
return scalarTypeForStringFormats(messageID, input)
85+
case "any":
86+
return scalarTypeForAny(messageID, input)
87+
case "object":
88+
return scalarTypeForObject(messageID, input)
7889
}
79-
return 0, "", fmt.Errorf("unknown scalar type for field %s.%s: %v", messageID, input.Name, input.Schema.Type)
90+
return unknownFormat("scalar", messageID, input)
8091
}
8192

8293
func scalarTypeForIntegerFormats(messageID string, input *property) (api.Typez, string, error) {
@@ -125,6 +136,24 @@ func scalarTypeForStringFormats(messageID string, input *property) (api.Typez, s
125136
return unknownFormat("string", messageID, input)
126137
}
127138

139+
func scalarTypeForAny(messageID string, input *property) (api.Typez, string, error) {
140+
switch input.Schema.Format {
141+
case "google.protobuf.Value":
142+
return api.MESSAGE_TYPE, ".google.protobuf.Value", nil
143+
}
144+
return unknownFormat("any", messageID, input)
145+
}
146+
147+
func scalarTypeForObject(messageID string, input *property) (api.Typez, string, error) {
148+
switch input.Schema.Format {
149+
case "google.protobuf.Struct":
150+
return api.MESSAGE_TYPE, ".google.protobuf.Struct", nil
151+
case "google.protobuf.Any":
152+
return api.MESSAGE_TYPE, ".google.protobuf.Any", nil
153+
}
154+
return unknownFormat("object", messageID, input)
155+
}
156+
128157
func unknownFormat(baseType, messageID string, input *property) (api.Typez, string, error) {
129158
return 0, "", fmt.Errorf("unknown %s format (%s) for field %s.%s", baseType, input.Schema.Format, messageID, input.Name)
130159
}

‎internal/sidekick/internal/parser/discovery/message_fields_test.go‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
)
2424

2525
func TestMakeMessageFields(t *testing.T) {
26+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
2627
input := &schema{
2728
Properties: []*property{
2829
{
@@ -45,7 +46,7 @@ func TestMakeMessageFields(t *testing.T) {
4546
},
4647
},
4748
}
48-
got, err := makeMessageFields(".package.Message", input)
49+
got, err := makeMessageFields(model, ".package.Message", input)
4950
if err != nil {
5051
t.Fatal(err)
5152
}
@@ -72,6 +73,7 @@ func TestMakeMessageFields(t *testing.T) {
7273
}
7374

7475
func TestMakeMessageFieldsError(t *testing.T) {
76+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
7577
input := &schema{
7678
Properties: []*property{
7779
{
@@ -85,12 +87,13 @@ func TestMakeMessageFieldsError(t *testing.T) {
8587
},
8688
},
8789
}
88-
if got, err := makeMessageFields(".package.Message", input); err == nil {
90+
if got, err := makeMessageFields(model, ".package.Message", input); err == nil {
8991
t.Errorf("expected error makeScalarField(), got=%v, Input=%v", got, input)
9092
}
9193
}
9294

9395
func TestMakeScalarFieldError(t *testing.T) {
96+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
9497
input := &property{
9598
Name: "field",
9699
Schema: &schema{
@@ -100,7 +103,7 @@ func TestMakeScalarFieldError(t *testing.T) {
100103
Format: "--unused--",
101104
},
102105
}
103-
if got, err := makeScalarField(".package.Message", input); err == nil {
106+
if got, err := makeScalarField(model, ".package.Message", input); err == nil {
104107
t.Errorf("expected error makeScalarField(), got=%v, Input=%v", got, input)
105108
}
106109
}
@@ -128,7 +131,11 @@ func TestScalarTypes(t *testing.T) {
128131
{"string", "google-fieldmask", api.MESSAGE_TYPE, ".google.protobuf.FieldMask"},
129132
{"string", "int64", api.INT64_TYPE, "int64"},
130133
{"string", "uint64", api.UINT64_TYPE, "uint64"},
134+
{"any", "google.protobuf.Value", api.MESSAGE_TYPE, ".google.protobuf.Value"},
135+
{"object", "google.protobuf.Struct", api.MESSAGE_TYPE, ".google.protobuf.Struct"},
136+
{"object", "google.protobuf.Any", api.MESSAGE_TYPE, ".google.protobuf.Any"},
131137
} {
138+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
132139
input := &property{
133140
Name: "field",
134141
Schema: &schema{
@@ -138,7 +145,7 @@ func TestScalarTypes(t *testing.T) {
138145
Format: test.Format,
139146
},
140147
}
141-
gotTypez, gotTypeID, err := scalarType(".package.Message", input)
148+
gotTypez, gotTypeID, err := scalarType(model, ".package.Message", input)
142149
if err != nil {
143150
t.Errorf("error in scalarType(), Type=%q, Format=%q: %v", test.Type, test.Format, err)
144151
}
@@ -154,6 +161,7 @@ func TestScalarTypes(t *testing.T) {
154161
}
155162

156163
func TestScalarUnknownType(t *testing.T) {
164+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
157165
input := &property{
158166
Name: "field",
159167
Schema: &schema{
@@ -163,18 +171,21 @@ func TestScalarUnknownType(t *testing.T) {
163171
Format: "--unused--",
164172
},
165173
}
166-
if gotTypez, gotTypeID, err := scalarType(".package.Message", input); err == nil {
174+
if gotTypez, gotTypeID, err := scalarType(model, ".package.Message", input); err == nil {
167175
t.Errorf("expected error scalarType(), gotTypez=%d, gotTypezID=%q, Input=%v", gotTypez, gotTypeID, input)
168176
}
169177
}
170178

171179
func TestScalarUnknownFormats(t *testing.T) {
180+
model := api.NewTestAPI([]*api.Message{}, []*api.Enum{}, []*api.Service{})
172181
for _, test := range []struct {
173182
Type string
174183
}{
175184
{"integer"},
176185
{"number"},
177186
{"string"},
187+
{"any"},
188+
{"object"},
178189
} {
179190
input := &property{
180191
Name: "field",
@@ -185,7 +196,7 @@ func TestScalarUnknownFormats(t *testing.T) {
185196
Format: "--invalid--",
186197
},
187198
}
188-
if gotTypez, gotTypeID, err := scalarType(".package.Message", input); err == nil {
199+
if gotTypez, gotTypeID, err := scalarType(model, ".package.Message", input); err == nil {
189200
t.Errorf("expected error scalarType(), gotTypez=%d, gotTypezID=%q, Input=%v", gotTypez, gotTypeID, input)
190201
}
191202
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ func makeMethod(model *api.API, parent *api.Message, doc *document, input *metho
9898
Name: p.Name,
9999
Schema: &p.schema,
100100
}
101-
field, err := makeField(fmt.Sprintf(requestMessage.ID, id), prop)
101+
field, err := makeField(model, fmt.Sprintf(requestMessage.ID, id), prop)
102102
if err != nil {
103103
return nil, err
104104
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ func TestMakeMethodError(t *testing.T) {
271271
{"responseMustHaveRef", method{Response: &schema{}}},
272272
{"badPath", method{Path: "{+var"}},
273273
{"badParameter", method{Path: "a/b", Parameters: []*parameter{
274-
{Name: "badParameter", schema: schema{Type: "--invalid--"}},
274+
{Name: "badParameter", schema: schema{Type: "string", Format: "--invalid--"}},
275275
}}},
276276
{"badParameterName", method{
277277
Path: "a/b",

0 commit comments

Comments
 (0)