[php] fixed model_enum.mustache for real usage

[77web]

fixed Enum usage in PHP APIClients.
I tested actual API requests with this new EnumInterface, which performs API requests successfully 😄

before

// code which causes error in static analysis
$someModel = new SomeModel();
$someModel->setSomeEnumField(SomeEnum::VALUE_A); // error, setter has type declaration in phpdoc with "SomeEnum" , SomeEnum::VALUE_A is not a "SomeEnum" instance, but just a string in php

// code to avoid error
$someModel = new SomeModel([
    'some_enum_field' => SomeEnum::VALUE_A,
]);

after

$someModel = new SomeModel();
$someModel->setSomeEnumField(new SomeEnum(SomeEnum::VALUE_A));

note:
php8.1 has native Enum feature and it allows code like this.

$someModel = new SomeModel();
$someModel->setSomeEnumField(SomeEnum::VALUE_A);

But, many projects have not yet updated their environments to php8.1.

PR checklist

• Read the contribution guidelines.
• Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
• Run the following to build the project and update samples:

  ./mvnw clean package 
  ./bin/generate-samples.sh
  ./bin/utils/export_docs_generators.sh
  

  Commit all changed files.
  This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
  These must match the expectations made by your contribution.
  You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*.
  For Windows users, please run the script in Git BASH.
• File the PR against the correct branch: master (5.3.0), 6.0.x
• If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

[wing328]

@77web thanks for the PR. Can you please follow step 3 in the PR checklist to update the samples?

@jebentier (2017/07), @dkarlovi (2017/07), @mandrean (2017/08), @jfastnacht (2017/09), @ackintosh (2017/09) ❤️, @ybelenko (2018/07), @renepardon (2018/12)

[77web]

@wing328 Hi, thank you for your advice. Unfortunately, this PR is still work in progress. I'am now trying to find the best enum implementation for APIClients of PHP. So please wait for moment 🙏

[wing328]

No problem. Let us know if you need any help.

[77web]

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

Could you review please? 🙇

[thomasphansen]

Hello all,
Just my two cents here:

1. This is a breaking change - the current issue is just an IDE warning, but when this change is pushed, it will actually break the code of anyone using enums with the current implementation.
   The reason for the BC relies on the modifications in the ObjectSerializer: the former code checked the value of the property no matter which type it had, while now it will only checks for enum values if it is an instance of an enum object. Which means: if an existing library is calling the setter with a string value, the serializer will simply ignore the validation. and accept whatever was given as a value.
   Another problem is the response - and I think this is actually a bug, please correct me if I'm wrong: if one of its fields is an enum, then the endpoint will most likely just return a scalar field - and the new ObjectSerializer will fail to identify it as an enum. Which means, if you call the Model's getter for the enum property, you will receive back a scalar value, not an EnumInterface.
   Personally, I don't see this as a good trade, but of course it's up to you guys @ openapi-generator team to decide...

2. I can see 2 possible solutions for this problem: a workaround, and a compromise solution:

   1. Workaround: on the model, change the WhateverEnum docbloc usage to WhateverEnum|string. This would preserve the link between the Enum class and the constants usage, while fixing the IDE warning.
   2. Compromise solution: use the proposed PR, with some twists:
   • Modify the changes in the ObjectSerializer to:

                      # Line 81
                      $callable = [$openAPIType, 'getAllowableEnumValues'];
                      if ($value instanceof EnumInterface) {
                          $value = $value->getValue();
                      }
                      if (is_callable($callable)) {
                          /** array $callable */
   

   • on the model, for all properties using the enum type: make the setter receive either a scalar or the Enum class. if the received value is scalar, create the Enum class inside the setter, using the value as parameter for the constructor. (it could even perform validation here)
   • add a setting in the php generator to explicitly enable the getters for enum properties to return an object, instead of a scalar (default: OFF)
   • on the model, for all properties using the enum type: based on the suggested setting, if it is off (default), make the getters for enum properties return the value of the enum object, instead of the object itself. Otherwise, return the object.

Adopting the compromise solution would fix the issue, while still using this PR and allowing users to choose what model to use - and preserve the current behavior, avoiding the BC. 😄

I'm writing this as a heavy user of OpenAPIGenerator - we use it for dozens of API clients in our company. Accepting this PR as it is would definitely cause us a lot of harm, since we now and then need to recreate the clients (due to changes in the service APIs), and would have to check all of them (and their usage scenarios) for possible breaks.

[thomasphansen]

Hello again,
Since I was not sure about the second BC I pointed in my previous comments (regarding enums in the response object), I did a bit more of investigation in the ObjectSerializer class. I was wrong in the sense that the validation will still work - but the response field will actually contain only a string, instead of the Enum object:

        # v6.0.1 - ObjectSerializer.mustache, line #441
        if (method_exists($class, 'getAllowableEnumValues')) {
            if (!in_array($data, $class::getAllowableEnumValues(), true)) {
                $imploded = implode("', '", $class::getAllowableEnumValues());
                throw new \InvalidArgumentException("Invalid value for enum '$class', must be one of: '$imploded'");
            }
            return $data;

Which means, there will be an inconsistency between the objects in the request and in the response.

This means, the "compromise" solution also needs to care about the ObjectSerializer::deserialize() method:

• ObjectSerializer::deserialize(): if the suggested setting for returning Enum objects is on, then change the snippet above to return new $class($data) instead