Skip to content

Commit

Permalink
fix oneOf rendering when the oneOf field value is a proto Struct/Value
Browse files Browse the repository at this point in the history
oneOf schema for a proto oneof containing an opaque field (Struct/Value)
were not being generated. This fixes it.

Signed-off-by: Shashank Ram <shashank.ram@solo.io>
  • Loading branch information
shashankram committed Apr 8, 2024
1 parent 510e2d5 commit d03b18c
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 6 deletions.
7 changes: 7 additions & 0 deletions changelog/v0.2.1/oneof-fix.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
changelog:
- type: FIX
issueLink: https://github.com/solo-io/protoc-gen-openapi/issues/20
resolvesIssue: true
description: |
Fixes the rendering of oneOf schema for proto Struct/Value types
that are special cased to avoid recursion.
12 changes: 6 additions & 6 deletions openapiGenerator.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,12 @@ func (g *openapiGenerator) generateMessageSchema(message *protomodel.MessageDesc
fieldDesc := g.generateDescription(field)
fieldRules := g.validationRules(field)

// If the field is a oneof, we need to add the oneof property to the schema
if field.OneofIndex != nil {
idx := *field.OneofIndex
oneOfFields[idx] = append(oneOfFields[idx], fieldName)
}

schemaType := g.markerRegistry.GetSchemaType(fieldRules)
if schemaType != "" {
tmp := getSoloSchemaForMarkerType(schemaType)
Expand All @@ -439,12 +445,6 @@ func (g *openapiGenerator) generateMessageSchema(message *protomodel.MessageDesc
sr := g.fieldTypeRef(field)
g.markerRegistry.MustApplyRulesToSchema(fieldRules, sr.Value, markers.TargetType)
o.WithProperty(fieldName, sr.Value)

// If the field is a oneof, we need to add the oneof property to the schema
if field.OneofIndex != nil {
idx := *field.OneofIndex
oneOfFields[idx] = append(oneOfFields[idx], fieldName)
}
}

if g.protoOneof {
Expand Down
22 changes: 22 additions & 0 deletions testdata/golden/test5/openapiv3.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,21 @@ components:
schemas:
test5.Msg:
description: This is a top-level message.
oneOf:
- not:
anyOf:
- required:
- oa
- required:
- ob
- required:
- oc
- required:
- oa
- required:
- ob
- required:
- oc
properties:
list:
items:
Expand All @@ -25,6 +40,13 @@ components:
rule: self.b != 'default'
nonNested:
type: string
oa:
type: string
ob:
x-kubernetes-preserve-unknown-fields: true
oc:
type: object
x-kubernetes-preserve-unknown-fields: true
recursiveObj:
description: Recursive object.
items:
Expand Down
95 changes: 95 additions & 0 deletions testdata/struct.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Protocol Buffers - Google's data interchange format
// Copyright 2008 Google Inc. All rights reserved.
// https://developers.google.com/protocol-buffers/
//
// Redistribution and use in source and binary forms, with or without
// modification, are permitted provided that the following conditions are
// met:
//
// * Redistributions of source code must retain the above copyright
// notice, this list of conditions and the following disclaimer.
// * Redistributions in binary form must reproduce the above
// copyright notice, this list of conditions and the following disclaimer
// in the documentation and/or other materials provided with the
// distribution.
// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.
//
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

syntax = "proto3";

package google.protobuf;

option cc_enable_arenas = true;
option go_package = "google.golang.org/protobuf/types/known/structpb";
option java_package = "com.google.protobuf";
option java_outer_classname = "StructProto";
option java_multiple_files = true;
option objc_class_prefix = "GPB";
option csharp_namespace = "Google.Protobuf.WellKnownTypes";

// `Struct` represents a structured data value, consisting of fields
// which map to dynamically typed values. In some languages, `Struct`
// might be supported by a native representation. For example, in
// scripting languages like JS a struct is represented as an
// object. The details of that representation are described together
// with the proto support for the language.
//
// The JSON representation for `Struct` is JSON object.
message Struct {
// Unordered map of dynamically typed values.
map<string, Value> fields = 1;
}

// `Value` represents a dynamically typed value which can be either
// null, a number, a string, a boolean, a recursive struct value, or a
// list of values. A producer of value is expected to set one of these
// variants. Absence of any variant indicates an error.
//
// The JSON representation for `Value` is JSON value.
message Value {
// The kind of value.
oneof kind {
// Represents a null value.
NullValue null_value = 1;
// Represents a double value.
double number_value = 2;
// Represents a string value.
string string_value = 3;
// Represents a boolean value.
bool bool_value = 4;
// Represents a structured value.
Struct struct_value = 5;
// Represents a repeated `Value`.
ListValue list_value = 6;
}
}

// `NullValue` is a singleton enumeration to represent the null value for the
// `Value` type union.
//
// The JSON representation for `NullValue` is JSON `null`.
enum NullValue {
// Null value.
NULL_VALUE = 0;
}

// `ListValue` is a wrapper around a repeated field of values.
//
// The JSON representation for `ListValue` is JSON array.
message ListValue {
// Repeated field of dynamically typed values.
repeated Value values = 1;
}
13 changes: 13 additions & 0 deletions testdata/test5/rules.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ syntax = "proto3";

package test5;

import "google/protobuf/struct.proto";

// This is a top-level message.
//
// +kubebuilder:validation:XValidation:rule="self.msgNested.a > 0",message="must be a postive value"
Expand Down Expand Up @@ -29,6 +31,17 @@ message Msg {
// +kubebuilder:validation:XValidation:rule="self.x = 1"
repeated Recursive recursive_val = 5;

// OneOf message.
oneof oneof_field {
string oa = 6;

// +kubebuilder:validation:Type=value
google.protobuf.Value ob = 7;

// +kubebuilder:validation:Type=object
google.protobuf.Struct oc = 8;
}

// This is a nested message.
//
// +kubebuilder:validation:XValidation:rule="self.a > 0",message="must be a postive value"
Expand Down

0 comments on commit d03b18c

Please sign in to comment.