[BUG][PHP] Runtime exception when property is both nullable and required

[MustansirS]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• Have you tested with the latest master to confirm the issue still exists?
• Have you searched for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

When a response object contains a property that is:

1. type: string
2. nullable: true
3. Must conform to some defined pattern
4. Is required in the object

Then, the generated code for that particular model has the following logic (taken from the generated code in file lib/Model/Foo.php of the example below):

     /**
     * Sets bar
     *
     * @param string $bar bar
     *
     * @return self
     */
    public function setBar($bar)
    {

        if ((!preg_match("/^0[xX][a-fA-F0-9]{40}$/", $bar))) {
            throw new \InvalidArgumentException("invalid value for \$bar when calling Foo., must conform to the pattern /^0[xX][a-fA-F0-9]{40}$/.");
        }


        if (is_null($bar)) {
            array_push($this->openAPINullablesSetToNull, 'bar');
        } else {
            $nullablesSetToNull = $this->getOpenAPINullablesSetToNull();
            $index = array_search('bar', $nullablesSetToNull);
            if ($index !== FALSE) {
                unset($nullablesSetToNull[$index]);
                $this->setOpenAPINullablesSetToNull($nullablesSetToNull);
            }
        }

        $this->container['bar'] = $bar;

        return $this;
    }

Here, even though bar is nullable, when bar == null there is no check for it before the isNullable logic and there is an attempt to preg_match the string with the defined pattern, which throws an exception.

openapi-generator version

6.2.1

OpenAPI declaration file content or url

phpbug.yaml

openapi: 3.0.2
info:
  title: Bug API
  version: '0.0'
  contact:
    name: Curvegrid
  description: PHP Client Bug API.
servers:
  - url: 'http://{hostname}'
    variables:
      hostname:
        default: localhost:8080
        description: Bug Test Server.
tags:
  - name: test
paths:
  /foo:
    get:
      operationId: foo
      summary: FooBaz
      description: FooBar.
      tags:
        - test
      responses:
        '200':
          description: OK
          content:
            application/json:
              schema:
                $ref: '#/components/schemas/Foo'
components:
  schemas:
    Foo:
      title: Foo
      type: object
      properties:
        bar:
          type: string
          nullable: true
          pattern: '^0[xX][a-fA-F0-9]{40}$'
      required:
        - bar

Generation Details

openapi-generator-cli generate -g php -i phpbug.yaml -o phpclient

Suggest a fix

I noticed looking at https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/model_generic.mustache#L388 that the main problem here is that the case where a property can be both nullable and required, which is completely valid in the specs, isn't handled well in template. I propose that the logic of isNullable should be moved to the top of the function, before pattern matching and everything else, or the required section in lines like https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/model_generic.mustache#L435 should be removed so there is an !is_null check before trying to match patterns or anything else.

[MustansirS]

@jebentier @dkarlovi @mandrean @jfastnacht @ybelenko @renepardon
I would like to propose that I can provide the fix for this issue and open a PR. I would appreciate your input on what path I should take regarding the Suggested Fix I stated in the issue.

[wing328]

@MustansirS thanks for the suggestion.

I propose that the logic of isNullable should be moved to the top of the function, before pattern matching and everything else, or the required section in lines like https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/php/model_generic.mustache#L435 should be removed so there is an !is_null check before trying to match patterns or anything else.

can you please file a PR with the suggested fix to start with? we'll review accordingly. thanks