Skip to content

Commit

Permalink
Fixed required body parameter when part of the proto message is path …
Browse files Browse the repository at this point in the history
…param (#4850)

* checking field behavior options in renderFieldAsDefinition even in the case where path params are present inside the field

* added testcase with wildcard body; one of the fields in the body is a message field and one of the field within this message field is part of path parameters; the message field now gets marked as required

* make generate - file changes
  • Loading branch information
MahikaJaguste authored Oct 21, 2024
1 parent d2f2f33 commit 9f6d32f
Show file tree
Hide file tree
Showing 3 changed files with 214 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -5080,7 +5080,10 @@
}
},
"description": "The book's `name` field is used to identify the book to be updated.\nFormat: publishers/{publisher}/books/{book}",
"title": "The book to update."
"title": "The book to update.",
"required": [
"book"
]
}
},
{
Expand Down
7 changes: 7 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,13 @@ func renderFieldAsDefinition(f *descriptor.Field, reg *descriptor.Registry, refs
schema.Title = strings.TrimSpace(paragraphs[0])
schema.Description = strings.TrimSpace(strings.Join(paragraphs[1:], paragraphDeliminator))
}

// to handle case where path param is present inside the field of descriptorpb.FieldDescriptorProto_TYPE_MESSAGE type
// it still needs to consider the behaviour of the field which was being done by schemaOfField() in case there are no path params
if j, err := getFieldBehaviorOption(reg, f); err == nil {
updateSwaggerObjectFromFieldBehavior(&schema, j, reg, f)
}

return schema, nil
}

Expand Down
203 changes: 203 additions & 0 deletions protoc-gen-openapiv2/internal/genopenapi/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9636,6 +9636,209 @@ func TestRenderServicesOpenapiPathsOrderPreservedAdditionalBindings(t *testing.T
}
}

func TestRenderServicesOpenapiRequiredBodyFieldContainingPathParam(t *testing.T) {

var fieldBehaviorRequired = []annotations.FieldBehavior{annotations.FieldBehavior_REQUIRED}
var requiredFieldOptions = new(descriptorpb.FieldOptions)
proto.SetExtension(requiredFieldOptions, annotations.E_FieldBehavior, fieldBehaviorRequired)

bookDesc := &descriptorpb.DescriptorProto{
Name: proto.String("Book"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("name"),
Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
Number: proto.Int32(1),
},
{
Name: proto.String("type"),
Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(),
Number: proto.Int32(2),
},
},
}
addBookReqDesc := &descriptorpb.DescriptorProto{
Name: proto.String("AddBookReq"),
Field: []*descriptorpb.FieldDescriptorProto{
{
Name: proto.String("book"),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".Book"),
Number: proto.Int32(1),
Options: requiredFieldOptions,
},
{
Name: proto.String("libraryId"),
Type: descriptorpb.FieldDescriptorProto_TYPE_UINT32.Enum(),
Number: proto.Int32(2),
Options: requiredFieldOptions,
},
{
Name: proto.String("isLatestEdition"),
Type: descriptorpb.FieldDescriptorProto_TYPE_BOOL.Enum(),
Number: proto.Int32(3),
},
},
}
meth := &descriptorpb.MethodDescriptorProto{
Name: proto.String("AddBook"),
InputType: proto.String("AddBookReq"),
OutputType: proto.String("Book"),
}
svc := &descriptorpb.ServiceDescriptorProto{
Name: proto.String("BookService"),
Method: []*descriptorpb.MethodDescriptorProto{meth},
}
bookMsg := &descriptor.Message{
DescriptorProto: bookDesc,
}
addBookReqMsg := &descriptor.Message{
DescriptorProto: addBookReqDesc,
}

nameField := &descriptor.Field{
Message: bookMsg,
FieldDescriptorProto: bookMsg.GetField()[0],
}
typeField := &descriptor.Field{
Message: bookMsg,
FieldDescriptorProto: bookMsg.GetField()[1],
}
bookMsg.Fields = []*descriptor.Field{nameField, typeField}

bookField := &descriptor.Field{
Message: addBookReqMsg,
FieldMessage: bookMsg,
FieldDescriptorProto: addBookReqMsg.GetField()[0],
}
libraryIdField := &descriptor.Field{
Message: addBookReqMsg,
FieldDescriptorProto: addBookReqMsg.GetField()[1],
}
isLatestEditionField := &descriptor.Field{
Message: addBookReqMsg,
FieldDescriptorProto: addBookReqMsg.GetField()[2],
}
addBookReqMsg.Fields = []*descriptor.Field{bookField, libraryIdField, isLatestEditionField}

file := descriptor.File{
FileDescriptorProto: &descriptorpb.FileDescriptorProto{
SourceCodeInfo: &descriptorpb.SourceCodeInfo{},
Name: proto.String("book.proto"),
MessageType: []*descriptorpb.DescriptorProto{bookDesc, addBookReqDesc},
Service: []*descriptorpb.ServiceDescriptorProto{svc},
Options: &descriptorpb.FileOptions{
GoPackage: proto.String("github.com/grpc-ecosystem/grpc-gateway/runtime/internal/examplepb;example"),
},
},
GoPkg: descriptor.GoPackage{
Path: "example.com/path/to/example/example.pb",
Name: "example_pb",
},
Messages: []*descriptor.Message{bookMsg, addBookReqMsg},
Services: []*descriptor.Service{
{
ServiceDescriptorProto: svc,
Methods: []*descriptor.Method{
{
MethodDescriptorProto: meth,
RequestType: addBookReqMsg,
ResponseType: bookMsg,
Bindings: []*descriptor.Binding{
{
HTTPMethod: "POST",
PathTmpl: httprule.Template{
Version: 1,
OpCodes: []int{0, 0},
Template: "/v1/books/{book.type}",
},
PathParams: []descriptor.Parameter{
{
FieldPath: descriptor.FieldPath([]descriptor.FieldPathComponent{
{
Name: "book",
},
{
Name: "type",
},
}),
Target: typeField,
},
},
Body: &descriptor.Body{
FieldPath: []descriptor.FieldPathComponent{},
},
},
},
},
},
},
},
}
reg := descriptor.NewRegistry()
fileCL := crossLinkFixture(&file)
err := reg.Load(reqFromFile(fileCL))
if err != nil {
t.Errorf("reg.Load(%#v) failed with %v; want success", file, err)
return
}
result, err := applyTemplate(param{File: fileCL, reg: reg})
if err != nil {
t.Fatalf("applyTemplate(%#v) failed with %v; want success", file, err)
}

paths := GetPaths(result)
if got, want := len(paths), 1; got != want {
t.Fatalf("Results path length differed, got %d want %d", got, want)
}

if got, want := paths[0], "/v1/books/{book.type}"; got != want {
t.Fatalf("Wrong results path, got %s want %s", got, want)
}

var operation = *result.getPathItemObject("/v1/books/{book.type}").Post

if got, want := operation.Parameters[0].Name, "book.type"; got != want {
t.Fatalf("Wrong parameter name 0, got %s want %s", got, want)
}

if got, want := operation.Parameters[0].In, "path"; got != want {
t.Fatalf("Wrong parameter location 0, got %s want %s", got, want)
}

if got, want := operation.Parameters[1].Name, "body"; got != want {
t.Fatalf("Wrong parameter name 1, got %s want %s", got, want)
}

if got, want := operation.Parameters[1].In, "body"; got != want {
t.Fatalf("Wrong parameter location 1, got %s want %s", got, want)
}

if want, is, name := "#/definitions/BookServiceAddBookBody", operation.Parameters[1].Schema.schemaCore.Ref, "operation.Parameters[1].Schema.schemaCore.Ref"; !reflect.DeepEqual(is, want) {
t.Fatalf("%s = %s want to be %s", name, want, is)
}

definition, found := result.Definitions["BookServiceAddBookBody"]
if !found {
t.Fatalf("expecting definition to contain BookServiceAddBookBody")
}

if want, is, name := 3, len(*definition.Properties), "len(*definition.Properties)"; !reflect.DeepEqual(is, want) {
t.Fatalf("%s = %d want to be %d", name, want, is)
}

for index, keyValue := range []string{"book", "libraryId", "isLatestEdition"} {
if got, want := (*definition.Properties)[index].Key, keyValue; got != want {
t.Fatalf("Wrong definition property %d, got %s want %s", index, got, want)
}
}

correctRequiredFields := []string{"book", "libraryId"}
if got, want := definition.Required, correctRequiredFields; !reflect.DeepEqual(got, want) {
t.Fatalf("Wrong required fields in body definition, got = %s, want = %s", got, want)
}
}

func TestArrayMessageItemsType(t *testing.T) {

msgDesc := &descriptorpb.DescriptorProto{
Expand Down

0 comments on commit 9f6d32f

Please sign in to comment.