From 4f8ee0904ad515e35a50c9d1ccfcb440fa5da95f Mon Sep 17 00:00:00 2001 From: Sam Eiderman Date: Wed, 21 Dec 2022 20:03:06 +0200 Subject: [PATCH] Make read_only work with references using AllOf (#3082) As described here: https://github.com/grpc-ecosystem/grpc-gateway/issues/1137 This partially reverts the recent commit: a0fe8d7c8676db9674ccbc4c4fc642790a8b8065 Which doesn't solve the problem for read_only being set on the field. This feature will require setting the "use_allof_for_refs" to true since this breaks existing compatibility with swagger-codegen generated files from the AllOf'd schema - type is now "object" and not $ref. Test added + ReDoc now works with read_only message/enum fields. --- internal/descriptor/registry.go | 14 +++++ protoc-gen-openapiv2/defs.bzl | 6 +++ .../internal/genopenapi/template.go | 18 +++++++ .../internal/genopenapi/template_test.go | 54 +++++++++++++++++++ .../internal/genopenapi/types.go | 6 +++ protoc-gen-openapiv2/main.go | 2 + 6 files changed, 100 insertions(+) diff --git a/internal/descriptor/registry.go b/internal/descriptor/registry.go index fdb7717fb8a..3d8b88fc1e2 100644 --- a/internal/descriptor/registry.go +++ b/internal/descriptor/registry.go @@ -136,6 +136,10 @@ type Registry struct { // disableDefaultResponses disables the generation of default responses. // Useful if you have to support custom response codes that are not 200. disableDefaultResponses bool + + // useAllOfForRefs, if set, will use allOf as container for $ref to preserve same-level + // properties + useAllOfForRefs bool } type repeatedFieldSeparator struct { @@ -772,3 +776,13 @@ func (r *Registry) SetDisableDefaultResponses(use bool) { func (r *Registry) GetDisableDefaultResponses() bool { return r.disableDefaultResponses } + +// SetUseAllOfForRefs sets useAllOfForRefs +func (r *Registry) SetUseAllOfForRefs(use bool) { + r.useAllOfForRefs = use +} + +// GetUseAllOfForRefs returns useAllOfForRefs +func (r *Registry) GetUseAllOfForRefs() bool { + return r.useAllOfForRefs +} diff --git a/protoc-gen-openapiv2/defs.bzl b/protoc-gen-openapiv2/defs.bzl index 25c8847b20b..e22548f4c77 100644 --- a/protoc-gen-openapiv2/defs.bzl +++ b/protoc-gen-openapiv2/defs.bzl @@ -356,6 +356,12 @@ protoc_gen_openapiv2 = rule( " Repeat this option to supply multiple values. Elements without" + " visibility annotations are unaffected by this setting.", ), + "use_allof_for_refs": attr.bool( + default = False, + mandatory = False, + doc = "if set, will use allOf as container for $ref to preserve" + + " same-level properties.", + ), "_protoc": attr.label( default = "@com_google_protobuf//:protoc", executable = True, diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index cb3fedc00b2..62105ba3177 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -515,6 +515,24 @@ func renderMessageAsDefinition(msg *descriptor.Message, reg *descriptor.Registry fieldSchema.Required = nil } + if reg.GetUseAllOfForRefs() { + if fieldSchema.Ref != "" { + // Per the JSON Reference syntax: Any members other than "$ref" in a JSON Reference object SHALL be ignored. + // https://tools.ietf.org/html/draft-pbryan-zyp-json-ref-03#section-3 + // However, use allOf to specify Title/Description/readOnly fields. + if fieldSchema.Title != "" || fieldSchema.Description != "" || fieldSchema.ReadOnly { + fieldSchema = openapiSchemaObject{ + Title: fieldSchema.Title, + Description: fieldSchema.Description, + ReadOnly: fieldSchema.ReadOnly, + AllOf: []allOfEntry{{Ref: fieldSchema.Ref}}, + } + } else { + fieldSchema = openapiSchemaObject{schemaCore: schemaCore{Ref: fieldSchema.Ref}} + } + } + } + kv := keyVal{Value: fieldSchema} kv.Key = reg.FieldName(f) if schema.Properties == nil { diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 4f6daf6efb3..ab64b9b4be2 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -4690,6 +4690,7 @@ func TestRenderMessagesAsDefinition(t *testing.T) { openAPIOptions *openapiconfig.OpenAPIOptions pathParams []descriptor.Parameter UseJSONNamesForFields bool + UseAllOfForRefs bool }{ { descr: "no OpenAPI options", @@ -5292,6 +5293,55 @@ func TestRenderMessagesAsDefinition(t *testing.T) { }, UseJSONNamesForFields: true, }, + { + descr: "JSONSchema with a read_only nested field", + msgDescs: []*descriptorpb.DescriptorProto{ + { + Name: proto.String("Message"), + Field: []*descriptorpb.FieldDescriptorProto{ + { + Name: proto.String("nested"), + Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(), + TypeName: proto.String(".example.Message.Nested"), + Number: proto.Int32(1), + Options: fieldBehaviorOutputOnlyOptions, + }, + }, + NestedType: []*descriptorpb.DescriptorProto{{ + Name: proto.String("Nested"), + }}, + }, + }, + UseAllOfForRefs: true, + schema: map[string]*openapi_options.Schema{ + "Message": { + JsonSchema: &openapi_options.JSONSchema{ + Title: "title", + Description: "desc", + Required: []string{}, + }, + }, + }, + defs: map[string]openapiSchemaObject{ + "exampleMessage": { + schemaCore: schemaCore{ + Type: "object", + }, + Title: "title", + Description: "desc", + Required: nil, + Properties: &openapiSchemaObjectProperties{ + { + Key: "nested", + Value: openapiSchemaObject{ + AllOf: []allOfEntry{{Ref: "#/definitions/MessageNested"}}, + ReadOnly: true, + }, + }, + }, + }, + }, + }, } for _, test := range tests { @@ -5328,6 +5378,10 @@ func TestRenderMessagesAsDefinition(t *testing.T) { reg.SetUseJSONNamesForFields(true) } + if test.UseAllOfForRefs { + reg.SetUseAllOfForRefs(true) + } + if err != nil { t.Fatalf("failed to load code generator request: %v", err) } diff --git a/protoc-gen-openapiv2/internal/genopenapi/types.go b/protoc-gen-openapiv2/internal/genopenapi/types.go index 2520116c22b..54d6ff5bedd 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/types.go +++ b/protoc-gen-openapiv2/internal/genopenapi/types.go @@ -183,6 +183,10 @@ type schemaCore struct { Default string `json:"default,omitempty" yaml:"default,omitempty"` } +type allOfEntry struct { + Ref string `json:"$ref,omitempty" yaml:"$ref,omitempty"` +} + type RawExample json.RawMessage func (m RawExample) MarshalJSON() ([]byte, error) { @@ -332,6 +336,8 @@ type openapiSchemaObject struct { Required []string `json:"required,omitempty" yaml:"required,omitempty"` extensions []extension + + AllOf []allOfEntry `json:"allOf,omitempty" yaml:"allOf,omitempty"` } // http://swagger.io/specification/#definitionsObject diff --git a/protoc-gen-openapiv2/main.go b/protoc-gen-openapiv2/main.go index 092d6fa9dbc..88ab247a6ce 100644 --- a/protoc-gen-openapiv2/main.go +++ b/protoc-gen-openapiv2/main.go @@ -42,6 +42,7 @@ var ( visibilityRestrictionSelectors = utilities.StringArrayFlag(flag.CommandLine, "visibility_restriction_selectors", "list of `google.api.VisibilityRule` visibility labels to include in the generated output when a visibility annotation is defined. Repeat this option to supply multiple values. Elements without visibility annotations are unaffected by this setting.") disableServiceTags = flag.Bool("disable_service_tags", false, "if set, disables generation of service tags. This is useful if you do not want to expose the names of your backend grpc services.") disableDefaultResponses = flag.Bool("disable_default_responses", false, "if set, disables generation of default responses. Useful if you have to support custom response codes that are not 200.") + useAllOfForRefs = flag.Bool("use_allof_for_refs", false, "if set, will use allOf as container for $ref to preserve same-level properties.") ) // Variables set by goreleaser at build time @@ -127,6 +128,7 @@ func main() { reg.SetVisibilityRestrictionSelectors(*visibilityRestrictionSelectors) reg.SetDisableServiceTags(*disableServiceTags) reg.SetDisableDefaultResponses(*disableDefaultResponses) + reg.SetUseAllOfForRefs(*useAllOfForRefs) if err := reg.SetRepeatedPathParamSeparator(*repeatedPathParamSeparator); err != nil { emitError(err) return