From 9f6d32f2b075f85dc87c53bc4ea3c6d41242b5d3 Mon Sep 17 00:00:00 2001 From: Mahika Jaguste <64633235+MahikaJaguste@users.noreply.github.com> Date: Mon, 21 Oct 2024 22:59:41 +0100 Subject: [PATCH] Fixed required body parameter when part of the proto message is path 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 --- .../a_bit_of_everything.swagger.json | 5 +- .../internal/genopenapi/template.go | 7 + .../internal/genopenapi/template_test.go | 203 ++++++++++++++++++ 3 files changed, 214 insertions(+), 1 deletion(-) diff --git a/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json b/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json index 92389ecd78e..308db0a36f8 100644 --- a/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json +++ b/examples/internal/proto/examplepb/a_bit_of_everything.swagger.json @@ -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" + ] } }, { diff --git a/protoc-gen-openapiv2/internal/genopenapi/template.go b/protoc-gen-openapiv2/internal/genopenapi/template.go index 8008a71e2a5..723a5833bc5 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template.go @@ -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 } diff --git a/protoc-gen-openapiv2/internal/genopenapi/template_test.go b/protoc-gen-openapiv2/internal/genopenapi/template_test.go index 004bdf28342..e61c48ed7db 100644 --- a/protoc-gen-openapiv2/internal/genopenapi/template_test.go +++ b/protoc-gen-openapiv2/internal/genopenapi/template_test.go @@ -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{