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

How does the engine analyze Enums? #1337

Closed
CxDevLead opened this issue Nov 3, 2022 · 4 comments · Fixed by #1381
Closed

How does the engine analyze Enums? #1337

CxDevLead opened this issue Nov 3, 2022 · 4 comments · Fixed by #1381

Comments

@CxDevLead
Copy link

I commonly use int backed enums; however, for API output, the string name values are used instead of the integer, for example:

enum ExampleEnum: int implements JsonSerializable
{
    use JsonSerializeEnumNameTrait;

    case One = 1;
    case Two = 2;
    case Three = 3;
}

trait JsonSerializeEnumNameTrait
{
    public function jsonSerialize(): string
    {
        return $this->name;
    }
}

Due to implementing the JsonSerializable interface, and including the jsonSerialize() function in the used trait, the output in a json_encode is the case name, instead of the value.

When I use the generator, it sets the type to integer, and the values are displayed as the names:

My swagger looks as follows:

ExampleEnum integer

Enum:
[ One, Two, Three ]

This is interesting, as I assumed the default would be the int backed value, which is good for my case; however, I would prefer to change the type to string, so if I use the following attribute:

#[Schema(
    type: "string"
)]

The output changes to:

ExampleEnum string

Enum:
[ 1, 2, 3 ]

This does the opposite of what I would have expected, it is displaying the int backed values, even though the type is set to string.

Is there a way I can get the output to display as "string", and use the name's instead of the int backed values? I'm having trouble understanding the logic of how the generator is choosing the enum values to display

@jacyimp
Copy link

jacyimp commented Nov 6, 2022

It seems like:

  • For int-backed enums, if the type is undefined, it defaults to string type, with enum names as the list of possible values.
  • For string-backed enums, however, if the type is undefined, it defaults to string type, with the enum's backing value as the list of possible values.

From the looks of it there could be a missing $schemaType = $backingType->getName(); in Processors/ExpandEnums.php:48 to avoid the weird outcome with int-backed enums but I'm not sure.

I am also using the same approach as you(usage of custom JsonSerialize), and was in need of a way to customize the behaviour. I'm new to this library but here's my solution:

  • Use a code-based generation as described in: ZircoteGuide
  • Define your own set of processors, with a customized ExpandEnum processor.

I basically ended up copying the ExpandEnum.php located in swagger-php/src/Processors/ExpandEnums.php into my own project, modifying its behaviour, and then handed it to the scan() function instead of the original ExpandEnums processor as such:

$openapi = Generator::scan(['./src'],
    [
        'processors' => [
            new MyCustomExpandEnum(),  // This is my own version of the enum processor, basically a copy of the original thing with modified behaviour ofc.
            new Processors\DocBlockDescriptions(),
            new Processors\MergeIntoOpenApi(),
            new Processors\MergeIntoComponents(),
            new Processors\ExpandClasses(),
            new Processors\ExpandInterfaces(),
            new Processors\ExpandTraits(),
            new Processors\AugmentSchemas(),
            new Processors\AugmentProperties(),
            new Processors\BuildPaths(),
            new Processors\AugmentParameters(),
            new Processors\AugmentRefs(),
            new Processors\MergeJsonContent(),
            new Processors\MergeXmlContent(),
            new Processors\OperationId(),
            new Processors\CleanUnmerged(),
        ],
    ],
);

You can then modify your own ExpandEnums however you want. I ended up copying the original code, and then modifying one function:

private function expandContextEnum(Analysis $analysis): void
    {
        /** @var array<OA\Schema> $schemas */
        $schemas = $analysis->getAnnotationsOfType([OA\Schema::class, OAT\Schema::class], true);

        foreach ($schemas as $schema) {
            if ($schema->_context->is('enum')) {
                $source = $schema->_context->enum;
                $re = new \ReflectionEnum($schema->_context->fullyQualifiedName($source));
                $schema->schema = !Generator::isDefault($schema->schema) ? $schema->schema : $re->getShortName();
                $type = 'string';
                $schemaType = 'string';
                if ($re->isBacked() && ($backingType = $re->getBackingType()) && method_exists($backingType, 'getName')) {
                    if (Generator::isDefault($schema->type)) {
                        $type = $backingType->getName();
                        $schemaType = $backingType->getName();  // Modified
                    } else {
                        $type = $schema->type;
                        $schemaType = $schema->type;
                    }
                }
                $fqn = $schema->_context->fullyQualifiedName($source);
                $schema->enum = array_map(function ($case) use ($re, $fqn, $type, $schemaType) {
                    if ($re->isBacked() && method_exists($fqn, 'jsonSerialize')) {  // Modified
                        return $case->jsonSerialize();
                    } elseif ($re->isBacked() && $type === $schemaType) {
                        return $case->value;  // Modified
                    }

                    return $case->name;
                }, $schema->_context->fullyQualifiedName($source)::cases());  // Modified, no idea if this is a good idea ^^
                $this->mapNativeType($schema, $type);
            }
        }
    }

If you don't specify the type, it'll read from the backed type; but then you can also define it as 'string', or in my case 'object'.

I'm sure there are better ways to do this though!

@DerManoMann
Copy link
Collaborator

We could make this a configurable feature of the processor. Also, there should be a better way to control processors if we want to promote use of custom ones .. hmmm 🤔

@DerManoMann
Copy link
Collaborator

To be honest the enum processor has grown quite organically.
Maybe it's time to be a bit more organised in the test cases and the behaviour.

@DerManoMann
Copy link
Collaborator

Been playing around with the code a bit lately and I propose a simplification of the rules.

Tagging all contributors of the ExpandEnums processor for compleness... (apologies if I get a name wrong....)
@k2tzumi @cidosx @recca0120 @akalineskou

There are two main cases:

  1. enum value that is_a UnitEnum
    This is pretty simple: If the UnitEnum has a value use it, otherwise use the name.
    No change here.
  2. Enum with Schema annotation
    a. If the Schema does not have a type always use the name.
    b. If a type is specified and it matches the backing value type use the backing value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants