From eeefa48ba54634b928d39b447a15a3f0808729e6 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Thu, 31 Oct 2024 18:07:57 -0400 Subject: [PATCH 1/3] impl(generator): support more field types Support more field types, including several string formats. Remove code duplication between array and scalar field handling. Make it easier to write smaller unit tests. --- .../genclient/translator/openapi/openapi.go | 174 +++++++++++---- .../translator/openapi/openapi_test.go | 198 +++++++++++++++++- 2 files changed, 334 insertions(+), 38 deletions(-) diff --git a/generator/internal/genclient/translator/openapi/openapi.go b/generator/internal/genclient/translator/openapi/openapi.go index 7702869d..9faf1823 100644 --- a/generator/internal/genclient/translator/openapi/openapi.go +++ b/generator/internal/genclient/translator/openapi/openapi.go @@ -18,6 +18,7 @@ package openapi import ( "fmt" + "strings" "github.com/googleapis/google-cloud-rust/generator/internal/genclient" "github.com/googleapis/google-cloud-rust/generator/internal/genclient/language" @@ -123,7 +124,14 @@ func makeMessageFields(messageName string, message *base.Schema) ([]*genclient.F if err != nil { return nil, err } - field, err := makeField(messageName, name, schema) + optional := true + for _, r := range message.Required { + if name == r { + optional = false + break + } + } + field, err := makeField(messageName, name, optional, schema) if err != nil { return nil, err } @@ -132,39 +140,26 @@ func makeMessageFields(messageName string, message *base.Schema) ([]*genclient.F return fields, nil } -func makeField(messageName, name string, field *base.Schema) (*genclient.Field, error) { +func makeField(messageName, name string, optional bool, field *base.Schema) (*genclient.Field, error) { + if len(field.AllOf) != 0 { + return makeAllOfField(messageName, name, field) + } if len(field.Type) == 0 { return nil, fmt.Errorf("missing field type for field %s.%s", messageName, name) } type_name := field.Type[0] - optional := true - for _, r := range field.Required { - if name == r { - optional = false - break - } - } switch type_name { - case "string": + case "boolean": return &genclient.Field{ Name: name, Documentation: field.Description, - Typez: genclient.STRING_TYPE, + Typez: genclient.BOOL_TYPE, Optional: optional, }, nil + case "string": + return makeStringType(messageName, name, field.Format, optional, field) case "integer": - { - typez, err := makeIntegerType(field.Format) - if err != nil { - return nil, err - } - return &genclient.Field{ - Name: name, - Documentation: field.Description, - Typez: typez, - Optional: optional, - }, nil - } + return makeIntegerField(messageName, name, field.Format, optional, field) case "object": return &genclient.Field{ Name: name, @@ -172,7 +167,6 @@ func makeField(messageName, name string, field *base.Schema) (*genclient.Field, Typez: genclient.MESSAGE_TYPE, Optional: true, }, nil - case "array": return makeArrayField(messageName, name, field) default: @@ -180,18 +174,111 @@ func makeField(messageName, name string, field *base.Schema) (*genclient.Field, } } -func makeIntegerType(format string) (genclient.Typez, error) { +func makeAllOfField(messageName, name string, field *base.Schema) (*genclient.Field, error) { + for _, proxy := range field.AllOf { + typezID := strings.TrimPrefix(proxy.GetReference(), "#/components/schemas/") + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.MESSAGE_TYPE, + TypezID: typezID, + Optional: true, + }, nil + } + return nil, fmt.Errorf("cannot build any AllOf schema for field %s.%s", messageName, name) +} + +func makeStringType(messageName, name, format string, optional bool, field *base.Schema) (*genclient.Field, error) { + switch format { + case "": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.STRING_TYPE, + Optional: optional, + }, nil + case "int64": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.INT64_TYPE, + Optional: optional, + }, nil + case "uint64": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.UINT64_TYPE, + Optional: optional, + }, nil + case "byte": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.BYTES_TYPE, + Optional: optional, + }, nil + case "google-duration": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.MESSAGE_TYPE, + TypezID: ".google.protobuf.Duration", + Optional: true, + }, nil + case "date-time": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.MESSAGE_TYPE, + TypezID: ".google.protobuf.Timestamp", + Optional: true, + }, nil + case "google-fieldmask": + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.MESSAGE_TYPE, + TypezID: ".google.protobuf.FieldMask", + Optional: true, + }, nil + default: + return nil, fmt.Errorf("unknown string format (%q) for field %s.%s", field.Format, messageName, name) + } +} + +func makeIntegerField(messageName, name, format string, optional bool, field *base.Schema) (*genclient.Field, error) { switch format { case "int32": - return genclient.INT32_TYPE, nil + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.INT32_TYPE, + Optional: optional, + }, nil case "int64": - return genclient.INT64_TYPE, nil + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.INT64_TYPE, + Optional: optional, + }, nil case "uint32": - return genclient.UINT32_TYPE, nil + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.UINT32_TYPE, + Optional: optional, + }, nil case "uint64": - return genclient.UINT64_TYPE, nil + return &genclient.Field{ + Name: name, + Documentation: field.Description, + Typez: genclient.UINT64_TYPE, + Optional: optional, + }, nil default: - return 0, fmt.Errorf("unknown integer format %q", format) + return nil, fmt.Errorf("unknown integer format (%q) for field %s.%s", format, messageName, name) } } @@ -203,16 +290,32 @@ func makeArrayField(messageName, name string, field *base.Schema) (*genclient.Fi if err != nil { return nil, fmt.Errorf("cannot build schema for %s.%s error=%q", messageName, name, err) } - if schema.Type[0] == "string" { + switch schema.Type[0] { + case "string": + field, err := makeStringType(messageName, name, schema.Format, false, field) + if err != nil { + return nil, err + } + field.Repeated = true + field.Optional = false + return field, nil + case "integer": + field, err := makeIntegerField(messageName, name, schema.Format, false, field) + if err != nil { + return nil, err + } + field.Repeated = true + field.Optional = false + return field, nil + case "boolean": return &genclient.Field{ Name: name, Documentation: field.Description, - Typez: genclient.STRING_TYPE, + Typez: genclient.BOOL_TYPE, Optional: false, Repeated: true, }, nil - } - if schema.Type[0] == "object" { + case "object": return &genclient.Field{ Name: name, Documentation: field.Description, @@ -220,6 +323,7 @@ func makeArrayField(messageName, name string, field *base.Schema) (*genclient.Fi Optional: false, Repeated: true, }, nil + default: + return nil, fmt.Errorf("unknown array field type for %s.%s %q", messageName, name, schema.Type[0]) } - return nil, fmt.Errorf("unknown array field type for %s.%s %q", messageName, name, schema.Type[0]) } diff --git a/generator/internal/genclient/translator/openapi/openapi_test.go b/generator/internal/genclient/translator/openapi/openapi_test.go index f5ac402d..448d3e20 100644 --- a/generator/internal/genclient/translator/openapi/openapi_test.go +++ b/generator/internal/genclient/translator/openapi/openapi_test.go @@ -22,13 +22,177 @@ import ( "github.com/googleapis/google-cloud-rust/generator/internal/genclient" ) -func TestMakeAPI(t *testing.T) { - tDir := t.TempDir() +func TestAllOf(t *testing.T) { + // A message with AllOf and its transitive closure of dependent messages. + const messageWithAllOf = ` + "Automatic": { + "description": "A replication policy that replicates the Secret payload without any restrictions.", + "type": "object", + "properties": { + "customerManagedEncryption": { + "description": "Optional. The customer-managed encryption configuration of the Secret.", + "allOf": [{ + "$ref": "#/components/schemas/CustomerManagedEncryption" + }] + } + } + }, + "CustomerManagedEncryption": { + "description": "Configuration for encrypting secret payloads using customer-managed\nencryption keys (CMEK).", + "type": "object", + "properties": { + "kmsKeyName": { + "description": "Required. The resource name of the Cloud KMS CryptoKey used to encrypt secret payloads.", + "type": "string" + } + }, + "required": [ + "kmsKeyName" + ] + }, +` + contents := []byte(singleMessagePreample + messageWithAllOf + singleMessageTrailer) + translator, err := NewTranslator(contents, &Options{ + Language: "not used", + OutDir: "not used", + TemplateDir: "not used", + }) + if err != nil { + t.Errorf("Error in NewTranslator() %q", err) + } + + api, err := translator.makeAPI() + if err != nil { + t.Errorf("Error in makeAPI() %q", err) + } + + checkMessage(t, *api.Messages[0], genclient.Message{ + Name: "Automatic", + Documentation: "A replication policy that replicates the Secret payload without any restrictions.", + Fields: []*genclient.Field{ + {Name: "customerManagedEncryption", + Documentation: "Optional. The customer-managed encryption configuration of the Secret.", + Typez: genclient.MESSAGE_TYPE, + TypezID: "CustomerManagedEncryption", + Optional: true}, + }, + }) +} + +func TestBasicTypes(t *testing.T) { + // A message with basic types. + const messageWithBasicTypes = ` + "Fake": { + "description": "A test message.", + "type": "object", + "properties": { + "fBool": { "type": "boolean" }, + "fInt64": { "type": "integer", "format": "int64" }, + "fInt32": { "type": "integer", "format": "int32" }, + "fString": { "type": "string" }, + "fSInt64": { "type": "string", "format": "int64" }, + "fSUInt64": { "type": "string", "format": "uint64" }, + "fDuration": { "type": "string", "format": "google-duration" }, + "fTimestamp": { "type": "string", "format": "date-time" }, + "fFieldMask": { "type": "string", "format": "google-fieldmask" }, + "fBytes": { "type": "string", "format": "byte" } + }, + "required": [ + "fBool", "fInt64", "fInt32", "fString", "fSInt64", + "fSUInt64", "fDuration", "fTimestamp", "fFieldMask", "fBytes" ] + }, +` + contents := []byte(singleMessagePreample + messageWithBasicTypes + singleMessageTrailer) + translator, err := NewTranslator(contents, &Options{ + Language: "not used", + OutDir: "not used", + TemplateDir: "not used", + }) + if err != nil { + t.Errorf("Error in NewTranslator() %q", err) + } + + api, err := translator.makeAPI() + if err != nil { + t.Errorf("Error in makeAPI() %q", err) + } + + checkMessage(t, *api.Messages[0], genclient.Message{ + Name: "Fake", + Documentation: "A test message.", + Fields: []*genclient.Field{ + {Name: "fBool", Typez: genclient.BOOL_TYPE}, + {Name: "fInt64", Typez: genclient.INT64_TYPE}, + {Name: "fInt32", Typez: genclient.INT32_TYPE}, + {Name: "fString", Typez: genclient.STRING_TYPE}, + {Name: "fSInt64", Typez: genclient.INT64_TYPE}, + {Name: "fSUInt64", Typez: genclient.UINT64_TYPE}, + {Name: "fDuration", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.Duration", Optional: true}, + {Name: "fTimestamp", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.Timestamp", Optional: true}, + {Name: "fFieldMask", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.FieldMask", Optional: true}, + {Name: "fBytes", Typez: genclient.BYTES_TYPE}, + }, + }) +} + +func TestArrayTypes(t *testing.T) { + // A message with basic types. + const messageWithBasicTypes = ` + "Fake": { + "description": "A test message.", + "type": "object", + "properties": { + "fBool": { "type": "array", "items": { "type": "boolean" }}, + "fInt64": { "type": "array", "items": { "type": "integer", "format": "int64" }}, + "fInt32": { "type": "array", "items": { "type": "integer", "format": "int32" }}, + "fString": { "type": "array", "items": { "type": "string" }}, + "fSInt64": { "type": "array", "items": { "type": "string", "format": "int64" }}, + "fSUInt64": { "type": "array", "items": { "type": "string", "format": "uint64" }}, + "fDuration": { "type": "array", "items": { "type": "string", "format": "google-duration" }}, + "fTimestamp": { "type": "array", "items": { "type": "string", "format": "date-time" }}, + "fFieldMask": { "type": "array", "items": { "type": "string", "format": "google-fieldmask" }}, + "fBytes": { "type": "array", "items": { "type": "string", "format": "byte" }}, + } + }, +` + contents := []byte(singleMessagePreample + messageWithBasicTypes + singleMessageTrailer) + translator, err := NewTranslator(contents, &Options{ + Language: "not used", + OutDir: "not used", + TemplateDir: "not used", + }) + if err != nil { + t.Errorf("Error in NewTranslator() %q", err) + } + api, err := translator.makeAPI() + if err != nil { + t.Errorf("Error in makeAPI() %q", err) + } + + checkMessage(t, *api.Messages[0], genclient.Message{ + Name: "Fake", + Documentation: "A test message.", + Fields: []*genclient.Field{ + {Repeated: true, Name: "fBool", Typez: genclient.BOOL_TYPE}, + {Repeated: true, Name: "fInt64", Typez: genclient.INT64_TYPE}, + {Repeated: true, Name: "fInt32", Typez: genclient.INT32_TYPE}, + {Repeated: true, Name: "fString", Typez: genclient.STRING_TYPE}, + {Repeated: true, Name: "fSInt64", Typez: genclient.INT64_TYPE}, + {Repeated: true, Name: "fSUInt64", Typez: genclient.UINT64_TYPE}, + {Repeated: true, Name: "fDuration", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.Duration"}, + {Repeated: true, Name: "fTimestamp", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.Timestamp"}, + {Repeated: true, Name: "fFieldMask", Typez: genclient.MESSAGE_TYPE, TypezID: ".google.protobuf.FieldMask"}, + {Repeated: true, Name: "fBytes", Typez: genclient.BYTES_TYPE}, + }, + }) +} + +func TestMakeAPI(t *testing.T) { contents := []byte(testDocument) translator, err := NewTranslator(contents, &Options{ Language: "rust", - OutDir: tDir, + OutDir: "not used", TemplateDir: "not used", }) if err != nil { @@ -110,6 +274,34 @@ func checkMessage(t *testing.T, got genclient.Message, want genclient.Message) { } } +const singleMessagePreample = ` +{ + "openapi": "3.0.3", + "info": { + "title": "Secret Manager API", + "description": "Stores sensitive data such as API keys, passwords, and certificates. Provides convenience while improving security.", + "version": "v1" + }, + "servers": [ + { + "url": "https://secretmanager.googleapis.com", + "description": "Global Endpoint" + } + ], + "components": { + "schemas": { +` + +const singleMessageTrailer = ` + }, + }, + "externalDocs": { + "description": "Find more info here.", + "url": "https://cloud.google.com/secret-manager/" + } +} +` + // This is a subset of the secret manager OpenAPI v3 spec circa 2023-10. It is // just intended to drive some of the initial development and testing. const testDocument = ` From 61668edf012a20cb0306559cb61526e9ed6ab550 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 1 Nov 2024 05:48:57 -0400 Subject: [PATCH 2/3] Fix formatting --- .../internal/genclient/translator/openapi/openapi_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/generator/internal/genclient/translator/openapi/openapi_test.go b/generator/internal/genclient/translator/openapi/openapi_test.go index 448d3e20..b08fc37b 100644 --- a/generator/internal/genclient/translator/openapi/openapi_test.go +++ b/generator/internal/genclient/translator/openapi/openapi_test.go @@ -70,11 +70,13 @@ func TestAllOf(t *testing.T) { Name: "Automatic", Documentation: "A replication policy that replicates the Secret payload without any restrictions.", Fields: []*genclient.Field{ - {Name: "customerManagedEncryption", + { + Name: "customerManagedEncryption", Documentation: "Optional. The customer-managed encryption configuration of the Secret.", Typez: genclient.MESSAGE_TYPE, TypezID: "CustomerManagedEncryption", - Optional: true}, + Optional: true, + }, }, }) } From fb540cb5329214505960bbd63ae6751f1681d283 Mon Sep 17 00:00:00 2001 From: Carlos O'Ryan Date: Fri, 1 Nov 2024 05:50:32 -0400 Subject: [PATCH 3/3] Fix typo --- .../internal/genclient/translator/openapi/openapi_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/generator/internal/genclient/translator/openapi/openapi_test.go b/generator/internal/genclient/translator/openapi/openapi_test.go index b08fc37b..41a156dd 100644 --- a/generator/internal/genclient/translator/openapi/openapi_test.go +++ b/generator/internal/genclient/translator/openapi/openapi_test.go @@ -51,7 +51,7 @@ func TestAllOf(t *testing.T) { ] }, ` - contents := []byte(singleMessagePreample + messageWithAllOf + singleMessageTrailer) + contents := []byte(singleMessagePreamble + messageWithAllOf + singleMessageTrailer) translator, err := NewTranslator(contents, &Options{ Language: "not used", OutDir: "not used", @@ -104,7 +104,7 @@ func TestBasicTypes(t *testing.T) { "fSUInt64", "fDuration", "fTimestamp", "fFieldMask", "fBytes" ] }, ` - contents := []byte(singleMessagePreample + messageWithBasicTypes + singleMessageTrailer) + contents := []byte(singleMessagePreamble + messageWithBasicTypes + singleMessageTrailer) translator, err := NewTranslator(contents, &Options{ Language: "not used", OutDir: "not used", @@ -157,7 +157,7 @@ func TestArrayTypes(t *testing.T) { } }, ` - contents := []byte(singleMessagePreample + messageWithBasicTypes + singleMessageTrailer) + contents := []byte(singleMessagePreamble + messageWithBasicTypes + singleMessageTrailer) translator, err := NewTranslator(contents, &Options{ Language: "not used", OutDir: "not used", @@ -276,7 +276,7 @@ func checkMessage(t *testing.T, got genclient.Message, want genclient.Message) { } } -const singleMessagePreample = ` +const singleMessagePreamble = ` { "openapi": "3.0.3", "info": {