Skip to content

Conversation

ilyongcho-moloco
Copy link
Contributor

References to other Issues or PRs

Fixing unnecessary nested schema definition for body parameter issue

#3058
open. -->

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

If there are no path parameters, the code now directly uses schemaOfFieldBase() to obtain the schema, without propagating target message's schema to the body parameter's schema.

Other comments

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for this PR! Could you please add a case in an existing proto, or add a new proto that exercises this bug fix? Thanks!

@ilyongcho-moloco
Copy link
Contributor Author

ilyongcho-moloco commented Aug 13, 2025

@johanbrandhorst

I updated the example.

Swagger doc with the fix applied

"/v1/{parent}/books": {
      "post": {
        "summary": "Create a book.",
        "operationId": "ABitOfEverythingService_CreateBook",
        "parameters": [
          {
            "name": "book",
            "description": "The book to create.",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/examplepbBook"
            }
          },
          ...
        ],
        ...
      }
    },

Swagger doc without the fix

"/v1/{parent}/books": {
      "post": {
        "summary": "Create a book.",
        "operationId": "ABitOfEverythingService_CreateBook",
        "parameters": [
          {
            "name": "book",
            "description": "The book to create.",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/examplepbBook",
              "required": [
                "book"
              ]
            }
          },
          ...
        ],
        ...
      }
    },

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@johanbrandhorst johanbrandhorst merged commit 77e504f into grpc-ecosystem:main Aug 19, 2025
14 checks passed
@johanbrandhorst
Copy link
Collaborator

Thank you for your contribution!

@ilyongcho-moloco ilyongcho-moloco deleted the issue-3058-nested-required branch August 20, 2025 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants