[REQ][PHP] Discriminator does not actually work when using ObjectSerializer::deserialize()

[jtreminio]

Is your feature request related to a problem? Please describe.

I don't think the current implementation of discriminator actually works as expected. Passing an array of data to ObjectSerializer::deserialize() does not instantiate the correct child class.

openapi: 3.0.3
info:
  title: 'API'
  version: 1.0.0
servers:
  -
    url: 'https://example.com'
paths:
  /foo:
    post:
      tags:
        - Bar
      summary: 'Summary'
      operationId: foo
      requestBody:
        required: true
        content:
          application/json:
            schema:
              $ref: '#/components/schemas/PetBaseObject'
      responses:
        '200':
          description: 'successful operation'

components:
  schemas:
    PetBaseObject:
      type: object
      required:
        - pet_type
      properties:
        pet_type:
          type: string
      discriminator:
        propertyName: pet_type
        mapping:
          dog: '#/components/schemas/PetDog'
          cat: '#/components/schemas/PetCat'
          fish: '#/components/schemas/PetFish'
    PetDog:
      type: object
      allOf:
        - $ref: '#/components/schemas/PetBaseObject'
        -
          required:
            - pet_type
          properties:
            pet_type:
              type: string
              default: dog
            likes_fetch:
              type: boolean
    PetCat:
      type: object
      allOf:
        - $ref: '#/components/schemas/PetBaseObject'
        -
          required:
            - pet_type
          properties:
            pet_type:
              type: string
              default: cat
            likes_catnip:
              type: boolean
    PetFish:
      type: object
      allOf:
        - $ref: '#/components/schemas/PetBaseObject'
        -
          required:
            - pet_type
          properties:
            pet_type:
              type: string
              default: fish
            water_type:
              type: string

<?php

require_once __DIR__ . '/vendor/autoload.php';

$pet = OpenAPI\Client\ObjectSerializer::deserialize([
    'pet_type' => 'cat',
    'likes_catnip' => false,
], OpenAPI\Client\Model\PetBaseObject::class);

var_dump($pet);

$ php test.php
object(OpenAPI\Client\Model\PetBaseObject)#3 (1) {
  ["container":protected]=>
  array(1) {
    ["pet_type"]=>
    string(13) "PetBaseObject"
  }
}

Describe the solution you'd like

I would expect discriminator instantiation to work similarly to the typescript-fetch generator. It reads the discriminator field value and does a simple comparison and returns the correct class type.

In the PHP generator it would look like this:

class PetBaseObject implements ModelInterface, ArrayAccess, \JsonSerializable
{
    // ...

    public static function discriminatorClassName(array $data): ?string
    {
        if (!array_key_exists('pet_type', $data)) {
            return null;
        }

        if ($data['pet_type'] === 'cat') {
            return PetCat::class;
        }
        if ($data['pet_type'] === 'dog') {
            return PetDog::class;
        }
        if ($data['pet_type'] === 'fish') {
            return PetFish::class;
        }

        return null;
    }

In my mind the whole point of passing an array of data to ObjectSerializer::deserialize() is for it to do all instantiation for the user. Otherwise the user would simply do

$cat = new OpenAPI\Client\Model\PetCat();
$cat->setLikesCatnip(true);

Was this choice done on purpose? I have code ready for a PR if it is something you think would be a good addition to the generator:

$ php test.php
object(OpenAPI\Client\Model\PetCat)#3 (1) {
  ["container":protected]=>
  array(2) {
    ["pet_type"]=>
    string(3) "cat"
    ["likes_catnip"]=>
    bool(false)
  }
}

@jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko @renepardon

[SebastianStehle]

Upvote on this. The discriminator is also not set automatically correctly.

[wing328]

@SebastianStehle would you like to contribute a PR or sponsor a fix?

[SebastianStehle]

I don't know yet. But I think I have the solution that works for me. I had custom templates anyway.

Generate Mappings

Right now the generator assumes that the discriminator value is the same as the class name, which is often not the case. Therefore I create 2 properties per class with the mappings:

In model__generic.mustache

    /**
      * Array of mapping. Used for (de)serialization
      *
      * @var string[]
      */
    protected static $openAPIMappings = [
        {{#discriminator}}
        {{#mappedModels}}'{{mappingName}}' => '{{{modelName}}}'{{^-last}},
        {{/-last}}{{/mappedModels}}
        {{/discriminator}}
    ];

    /**
      * Array of mapping. Used for (de)serialization
      *
      * @var string[]
      */
    protected static $openAPIMappingsReverse = [
        {{#discriminator}}
        {{#mappedModels}}'{{modelName}}' => '{{{mappingName}}}'{{^-last}},
        {{/-last}}{{/mappedModels}}
        {{/discriminator}}
    ];

    /**
     * Array of discriminator mappings. Used for (de)serialization
     *
     * @return array
     */
    public static function openAPIMappings()
    {
        return self::$openAPIMappings;
    }

Set Discriminator

Inherited classes do not set the discriminator value. I was able to fix that with:

In model__generic.mustache

    public function __construct(array $data = null)
    {
        ....
        {{#parentModel}}
        {{#discriminator}}

        // Initialize discriminator property with the model name.
        $this->container['{{discriminatorName}}'] = parent::$openAPIMappingsReverse['{{model.classname}}'];
        {{/discriminator}}
        {{/parentModel}}
        {{/discriminator}}
    }

Update the serializer

Next we have to fix the serializer to use the mapping:

In ObjectSerializer.mustache:

            // If a discriminator is defined and points to a valid subclass, use it.
            $discriminator = $class::DISCRIMINATOR;
            if (!empty($discriminator)) {
                $discriminatorProperty = $class::attributeMap()[$discriminator];

                if (isset($data->{$discriminatorProperty}) && is_string($data->{$discriminatorProperty})) {
                    $discriminatorValue = $data->{$discriminatorProperty};
                    $discriminatorType = $class::openAPIMAppings()[$discriminatorValue];

                    $subclass = '\{{invokerPackage}}\Model\\' . $discriminatorType;
                    if (is_subclass_of($subclass, $class)) {
                        $class = $subclass;
                    }
                }
            }

So far this works for me, but I don't understand if I have found all cases and I would like to focus on my client for now.

[SomeBdyElse]

Thank you @SebastianStehle ! We used your code to patch the client generator. It seems to work fine and now we are able to use discriminators with mappings. Thank you for sharing!