Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PHP] Generator doesn't respect the optional parameters in request body when content type is application/x-www-form-urlencoded #11129

Open
mmihalev opened this issue Aug 12, 2021 · 5 comments

Comments

@mmihalev
Copy link

Description

We have a request body with a schema with "application/x-www-form-urlencoded" content type. There are some parameters which are set as required. One of the parameters is optional. Swagger UI renders everything correctly and there are no issues there. However, when we generate PHP client with the swagger-codegen, we see that all the parameters are required... even the optional one. This basically makes the generated client useless.

Swagger-codegen version

3.0.26, 3.0.27

Swagger declaration file content or url

This is the definition:

post:
  tags:
    - Support
  summary: Create message
  description: Write message into a ticket
  operationId: support_create_message
  parameters:
    - name: ticket_id
      in: path
      description: Ticket ID
      required: true
      schema:
        type: string
  requestBody:
    $ref: '#/components/requestBodies/ticketsMessagesCreate'

And this is the schema of the requestBody:

ticketsMessagesCreate:
      description: ""
      required: true
      content:
        application/x-www-form-urlencoded:
          schema:
            type: object
            properties:
              message:
                description: "Ticket message."
                type: string
                minLength: 1
                maxLength: 5000
              display_name:
                description: "Name of the person who wrote the message"
                type: string
                minLength: 1
                maxLength: 100
              display_email:
                description: "Email of the person who wrote the message"
                type: string
                format: email
              attachments[]:
                description: "File attachments. Must be an array with attachment identifiers obtained from the [/support/messages/attachment](#operation/support_upload_attachment) method."
                type: array
                items:
                  type: integer
                  nullable: false
            required: 
              - message
              - display_name
              - display_email		

Full specs can be found here: https://api.tuningfiles.com/swagger.json

As you can see from the above, only message, display_name and display_email are required from the request body. attachments[] is optional. However generated php code looks like this:

    /**
     * Create request for operation 'supportCreateMessage'
     *
     * @param  string $message (required)
     * @param  string $display_name (required)
     * @param  string $display_email (required)
     * @param  int[] $attachments (required)
     * @param  string $ticket_id Ticket ID (required)
     *
     * @throws \InvalidArgumentException
     * @return \GuzzleHttp\Psr7\Request
     */
    protected function supportCreateMessageRequest($message, $display_name, $display_email, $attachments, $ticket_id)
    {
        .....
        // verify the required parameter 'attachments' is set
        if ($attachments === null || (is_array($attachments) && count($attachments) === 0)) {
            throw new \InvalidArgumentException(
                'Missing the required parameter $attachments when calling supportCreateMessage'
            );
        }
       .....

As you can see from the code above, $attachments are also required , but it shouldn't.

Command line used for generation

swagger-codegen generate -i https://api.tuningfiles.com/swagger.json -l php -o /output_dir -c config.json

config.json:

{
    "composerProjectName": "project name",
    "artifactVersion": "version",
    "sortParamsByRequiredFlag": true,
    "packagePath": "",
    "apiPackage": "Api",
    "invokerPackage": "Package name",
    "composerVendorName": "vendor name",
    "ensureUniqueParams": false,
    "variableNamingConvention": "snake_case",
    "allowUnicodeIdentifiers": false,
    "gitUserId": "git user",
    "srcBasePath": "",
    "hideGenerationTimestamp": true,
    "modelPackage": "",
    "gitRepoId": "repo id"
}
Steps to reproduce

Using the schema from https://api.tuningfiles.com/swagger.json and config.json from above, use the following command to generate php client (replace the output dir with your own directory):

  1. swagger-codegen generate -i https://api.tuningfiles.com/swagger.json -l php -o /output_dir -c config.json
  2. Navigate to the folder where client is generated and open Api/SupportApi.php
  3. Locate protected function supportCreateMessageRequest function (around line 596) and you will see that all the parameters are required. Even the $attachments parameter. No matter that it's not included into the required params in the specification.
Related issues/PRs

I can't find anything related.

Suggest a fix/enhancement

N/A

@mmihalev
Copy link
Author

Nothing? :(

@ampedandwired
Copy link

I'm getting this problem as well, with the "python" generator. I'm not really familiar with the swagger-codegen code, but after a quick dig I think the problem is here.

This if statement seems to add form body properties to the "requiredParams" list if the body is required, not if the individual property itself is required. This can be verified by removing the required: true from the form schema (ticketsMessagesCreate in OPs example).

A fix might be to add a check for formParameter.getRequired() in that if statement. But I think you'd need to set that property further up as well, where the formParameter var is created, maybe initialising it from the required list in propertyMap? Not sure, stretching my understanding of how this works.

@ampedandwired
Copy link

These two issues also seem to be related:

@ampedandwired
Copy link

Hmm, based on the tests, this seems to be intentional behaviour.

    /**
     * Tests when a 'application/x-www-form-urlencoded' request body is marked as required that all form
     * params are also marked as required.
     * 
     * @see #testOptionalFormParams()
     */
    @Test
    public void testRequiredFormParams() {

It seems like there's no way to have a required form body, but some optional properties within that body?

@knoxg
Copy link

knoxg commented Mar 3, 2025

Looks like that test, and the code it is testing, was created as part of this PR ( swagger-api/swagger-codegen-generators#331 ), but I would be surprised if that interpretation of the 'required' property is correct.

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

No branches or pull requests

3 participants