Make read_only work with references using AllOf (#3082)

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.
grpc-ecosystem · Dec 21, 2022 · 4f8ee09 · 4f8ee09
1 parent 4cceda0
commit 4f8ee09
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 0 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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