Skip to content

Commit

Permalink
Make read_only work with references using AllOf
Browse files Browse the repository at this point in the history
As described here:

#1137

This partially reverts the recent commit:

a0fe8d7

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.
  • Loading branch information
same-id committed Dec 20, 2022
1 parent 1e64a41 commit e9737de
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 0 deletions.
14 changes: 14 additions & 0 deletions internal/descriptor/registry.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
Expand Up @@ -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 {
Expand Down
54 changes: 54 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/template_test.go
Expand Up @@ -4690,6 +4690,7 @@ func TestRenderMessagesAsDefinition(t *testing.T) {
openAPIOptions *openapiconfig.OpenAPIOptions
pathParams []descriptor.Parameter
UseJSONNamesForFields bool
UseAllOfForRefs bool
}{
{
descr: "no OpenAPI options",
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
6 changes: 6 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/types.go
Expand Up @@ -182,6 +182,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) {
Expand Down Expand Up @@ -321,6 +325,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
Expand Down
2 changes: 2 additions & 0 deletions protoc-gen-openapiv2/main.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit e9737de

Please sign in to comment.