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

[3.4] Improving handling of BackedEnums with ApiResource class attributes #6298

Open
GwendolenLynch opened this issue Apr 6, 2024 · 13 comments · Fixed by #6309
Open

[3.4] Improving handling of BackedEnums with ApiResource class attributes #6298

GwendolenLynch opened this issue Apr 6, 2024 · 13 comments · Fixed by #6309

Comments

@GwendolenLynch
Copy link
Contributor

GwendolenLynch commented Apr 6, 2024

To better understand where to go with #6288 I spent some time researching the history of (PHP 8.1+) enums in this library … i.e. what are the correct behavioural expectations, and any missing bits. So this issue is an attempt to bring together a bunch of context to discuss & find a clear path forward.

Right now the key questions are around how to handle \BackedEnums containing the ApiResource class attribute:

  • Have metadata generation remove operations; or
  • Add/fix support for serialization & schema generation (see example below)
    • Properties of type \BackedEnum should always normalize to the enum ->value not an iri
    • Schema properties of type \BackedEnum should always be {"enum": [value1, value2, value3, ...]} not the iri type
    • Enum resource routes return only name & value properties by default
    • cases should probably never be included as an enum item property
    • Automatic registration of item & collection GET providers

One use-case for the latter is where there is a need to provide enum metadata lookup, e.g. in the example below a human friendly description of a Status' integer.

I'm happy to try and cover json and jsonapi, and @soyuka is already working on jsonldjsonhal is probably easy for me to fit in too if needed.

Background

@soyuka's blog post API Platform 3.1 is out!

Finally after quite some time (this issue was opened in 2018!), we have support for Enums in OpenAPI, REST and GraphQL. Both Doctrine ORM and ODM have support for enums through enumType which is supported by API Platform 3.0. Alan added their support over OpenAPI and GraphQL.

How to expose Enums with API Platform — a blog post that got attention showing how to expose enums as resources. This is what I have based my assumptions/approach around here. One has to start somewhere.

Issues

Implementation
Misc
Related to Symfony Changes

PRs

Current State Examples

Note

For simplicity I'm just sticking with Accept: application/json here.

Enum as Property Value

This works and both the schema and JSON response are as expected.

Model
use ApiPlatform\Metadata\ApiResource;

#[ApiResource]
class Article
{
    public ?int $id = null;
    public ?string $title = null;
    public ?Audit $audit = null;
    public ?Status $status = null;
}

enum Audit: string
{
    case Pending = 'pending';
    case Passed = 'passed';
    case Failed = 'failed';
}

enum Status: int
{
    case DRAFT = 0;
    case PUBLISHED = 1;
    case ARCHIVED = 2;
}
Schema
{
    "Article": {
        "type": "object",
        "description": "",
        "deprecated": false,
        "properties": {
            "id": {
                "readOnly": true,
                "type": "integer"
            },
            "title": {
                "type": "string"
            },
            "audit": {
                "type": "string",
                "enum": [
                    "pending",
                    "passed",
                    "failed"
                ]
            },
            "status": {
                "type": "integer",
                "enum": [0, 1, 2]
            }
        }
    }
}
curl -X 'GET' 'https://example.com/articles/1' -H 'accept: application/json'
{
  "id": 1,
  "title": "Once Upon A Title",
  "audit": "passed",
  "status": 1
}

Enum Resource as Property Value

Model
#[ApiResource]
#[GetCollection(provider: Audit::class.'::getCases')]
#[Get(provider: Audit::class.'::getCase')]
enum Audit: string
{
    use EnumApiResourceTrait;

    case Pending = 'pending';
    case Passed = 'passed';
    case Failed = 'failed';
}

#[ApiResource]
#[GetCollection(provider: Status::class.'::getCases')]
#[Get(provider: Status::class.'::getCase')]
enum Status: int
{
    use EnumApiResourceTrait;

    case DRAFT = 0;
    case PUBLISHED = 1;
    case ARCHIVED = 2;

    #[ApiProperty]
    public function getDescription(): string
    {
        return match ($this) {
            self::DRAFT => 'Article is not ready for public consumption',
            self::PUBLISHED => 'Article is publicly available',
            self::ARCHIVED => 'Article content is outdated or superseded',
        };
    }
}

use ApiPlatform\Metadata\Operation;

trait EnumApiResourceTrait
{
    public function getId(): string|int
    {
        return $this->value;
    }

    public function getValue(): int|string
    {
        return $this->value;
    }

    public static function getCases(): array
    {
        return self::cases();
    }

    public static function getCase(Operation $operation, array $uriVariables): ?BackedEnum
    {
        $id = is_numeric($uriVariables['id']) ? (int) $uriVariables['id'] : $uriVariables['id'];

        return array_reduce(self::cases(), static fn($c, \BackedEnum $case) => $case->name === $id || $case->value === $id ? $case : $c, null);
    }
}
Schema
{
    "Article": {
        "type": "object",
        "description": "",
        "deprecated": false,
        "properties": {
            "id": {
                "readOnly": true,
                "type": "integer"
            },
            "title": {
                "type": "string"
            },
            "audit": {
                "type": "string",
                "format": "iri-reference",
                "example": "https://example.com/"
            },
            "status": {
                "type": "string",
                "format": "iri-reference",
                "example": "https://example.com/"
            }
        }
    },
    "Audit": {
        "type": "object",
        "description": "",
        "deprecated": false,
        "properties": {
            "name": {
                "readOnly": true,
                "type": "string"
            },
            "value": {
                "readOnly": true,
                "allOf": [
                    {
                        "type": "string"
                    },
                    {
                        "type": "integer"
                    }
                ]
            },
            "id": {
                "readOnly": true,
                "anyOf": [
                    {
                        "type": "string"
                    },
                    {
                        "type": "integer"
                    }
                ]
            },
            "cases": {
                "readOnly": true
            }
        }
    },
    "Status": {
        "See above": "Same as Audit"
    }
}
curl -X 'GET' 'https://example.com/articles/1' -H 'accept: application/json'
{
  "id": 1,
  "title": "Once Upon A Title",
  "audit": "/audits/passed",
  "status": "/statuses/1"
}

This immediately breaks Article::status as it becomes an iri instead. Normal and correct for everything that's not an enum.

… and the enum resource routes produce "interesting" responses …

curl -X 'GET' 'https://example.com/audits/pending' -H 'accept: application/json'
Response
{
  "name": "Pending",
  "value": "pending",
  "id": "pending",
  "cases": [
    "/audits/pending",
    {
      "name": "Passed",
      "value": "passed",
      "id": "passed",
      "cases": [
        "/audits/pending",
        "/audits/passed",
        {
          "name": "Failed",
          "value": "failed",
          "id": "failed",
          "cases": [
            "/audits/pending",
            "/audits/passed",
            "/audits/failed"
          ]
        }
      ]
    },
    {
      "name": "Failed",
      "value": "failed",
      "id": "failed",
      "cases": [
        "/audits/pending",
        {
          "name": "Passed",
          "value": "passed",
          "id": "passed",
          "cases": [
            "/audits/pending",
            "/audits/passed",
            "/audits/failed"
          ]
        },
        "/audits/failed"
      ]
    }
  ]
}
curl -X 'GET' 'https://example.com/statuses/0' -H 'accept: application/json'
Response
{
  "name": "DRAFT",
  "value": 0,
  "description": "Article is not ready for public consumption",
  "id": 0,
  "cases": [
    "/statuses/0",
    {
      "name": "PUBLISHED",
      "value": 1,
      "description": "Article is publicly available",
      "id": 1,
      "cases": [
        "/statuses/0",
        "/statuses/1",
        {
          "name": "ARCHIVED",
          "value": 2,
          "description": "Article content is outdated or superseded",
          "id": 2,
          "cases": [
            "/statuses/0",
            "/statuses/1",
            "/statuses/2"
          ]
        }
      ]
    },
    {
      "name": "ARCHIVED",
      "value": 2,
      "description": "Article content is outdated or superseded",
      "id": 2,
      "cases": [
        "/statuses/0",
        {
          "name": "PUBLISHED",
          "value": 1,
          "description": "Article is publicly available",
          "id": 1,
          "cases": [
            "/statuses/0",
            "/statuses/1",
            "/statuses/2"
          ]
        },
        "/statuses/2"
      ]
    }
  ]
}
@lermontex
Copy link

lermontex commented Apr 6, 2024

Sorry, maybe off-topic, but since you touched on the entire history of implementing Enum support in the Api Platform, I decided to mention it

Working with Symfony + ApiPlatform (GraphQL) + FrankenPHP I had this strange problem

dunglas/frankenphp#467

To fix the issue I had to use unique Enum names

User (entity)
	Status (enum)
	Gender (enum)
	Invoice (entity)
		Status (enum)
	Report (entity)
		Type (enum)
		Status (enum)
--->>>
User (entity)
	UserStatus (enum)
	UserGender (enum)
	Invoice (entity)
		UserInvoiceStatus (enum)
	Report (entity)
		ReportType (enum)
		ReportStatus (enum)

I tried using aliases (like use Entity/User/Status as UserStatus), but it didn't help

Could this have something to do with ApiPlatform and the current implementation of BackedEnum in GraphQL?

@soyuka
Copy link
Member

soyuka commented Apr 7, 2024

Schema properties of type \BackedEnum should always be {"enum": [value1, value2, value3, ...]} not the iri type

I disagree it should be IRIs, /game_mode/single_player /game_mode/multiplayer I think we should base our examples uppon https://schema.org/GamePlayMode.

On graphQl we can also add: #6185 which isn't that wrong as maybe that backed enum is more about keys then values?

@GwendolenLynch
Copy link
Contributor Author

I disagree it should be IRIs

Easy. I had incorrectly interpreted/assumed parts of what you explained in #6288 (comment) while trying to mentally reduce it down to the base parts.

@GwendolenLynch
Copy link
Contributor Author

Properties of type \BackedEnum should always normalize to the enum ->value not an iri

I presume this is also invalid then?

@soyuka
Copy link
Member

soyuka commented Apr 8, 2024

From my point of view indeed, to use an Enum one should do:

PATCH /video_games/1

{"playMode": "/game_play_mode/single_player"}

(based on https://schema.org/VideoGame)

API Platform's focuses on hypermedia formats, not sure that we really want to set up rules for plain JSON (application/json) that are different from the json-ld ones. Let's include @dunglas here also.

@GwendolenLynch
Copy link
Contributor Author

To be sure I understand correctly, these would roughly be the PHP classes that your example's implementation would have?

#[ApiResource]
class VideoGame
{
    public GamePlayMode $playMode;
}

#[ApiResource]
enum GamePlayMode: string
{
    case CoOp = 'co_op';
    case MultiPlayer = 'multi_player';
    case SinglePlayer = 'single_player';
}

@GwendolenLynch
Copy link
Contributor Author

not sure that we really want to set up rules for plain JSON

I couldn't agree more. 👍 Honestly, I am just endlessly getting confused by what an PHP enum represents in terms of an API response. So my comments are probably inconsistent as a result. 🌞

An ongoing point of confusion for me is enum name versus value, take #6185 as an example here…

Enum:

   case ENGLISH = 'en';

JSON-(LD|API|HAL):

GET users/1
locale: 'en'

GraphQL

query getUser {
  user(id: "/users/1") {
    locale
  }
}

locale: 'ENGLISH'

We know that is due to how webonyx/graphql-php treats PHP BackedEnums, but it is inconsistent with others. Ergo it is a) wrong and needs workaround/fixing; b) other formats need to be adjusted/fixed; c) none-of-the-above.

Simply, should the API output of a property value of a (non-resource) enum typed property equal a) the PHP enums name; or b) the value field?

@soyuka
Copy link
Member

soyuka commented Apr 9, 2024

From what I understand from https://www.php.net/manual/fr/language.enumerations.backed.php, Backed enums wants their representation into databases or text as a string (or int). I suggest that we follow that and use values.

Regular enums can't be serialized from what I read:

If a Pure Enum is serialized to JSON, an error will be thrown. If a Backed Enum is serialized to JSON, it will be represented by its scalar value only, in the appropriate type. The behavior of both may be overridden by implementing JsonSerializable.

Wouldn't a patch on webonyx/graphql-php be better if graphql doesn't follow that?

@GwendolenLynch
Copy link
Contributor Author

tl;dr: it looks like the name field is what should be used for data "exchange", and the value field is reserved for data storage.

Wouldn't a patch on webonyx/graphql-php be better if graphql doesn't follow that?

Maybe, but is the webonyx/graphql-php approach wrong?

They also seem to be following the approach used in the reference implementation, and they have used that approach since "initial commit" in 2015 (v0.1).

Digging in, I went back to the PHP RFC: Enumerations and this piqued my curiosity:

By default, Enumerated Cases have no scalar equivalent. They are simply singleton objects. However, there are ample cases where an Enumerated Case needs to be able to round-trip to a database or similar datastore, so having a built-in scalar (and thus trivially serializable) equivalent defined intrinsically is useful.

A case that has a scalar equivalent is called a Backed Case, as it is “Backed” by a simpler value.

If I understand the author's point correctly, the value property is there for the purpose of storage, e.g. a database. Whereas the contents of name is the used/exchanged value.

To visualize, the RFC gives this example:

enum Foo {
  case Bar;
}
 
enum Baz: int {
  case Beep = 5;
}

They each print_r() to

Foo Enum
(
    [name] => Bar
)
Baz Enum:int
(
    [name] => Beep
    [value] => 5
)

… and serialize() to:

E:7:"Foo:Bar";
E:8:"Baz:Beep";

But … json_encode(Baz::Beep) gives the backing value of the enum … Nice (in)consistency 🙄

5

Note that Foo::Barcan't be json_encode()ed as it doesn't have a backing value.

Something also worth including here for consideration is this from the RFC:

The most popular case of enumerations is boolean, which is an enumerated type with legal values true and false.

i.e. it would conceptually be something like this pseudo-code:

enum Boolean: int {
  case true = 1;
  case false = 0;
}

Basically, in code we use true (pseudo-code's Boolean::true) for exchange, but internally it is just the integer 1 … the backed value.

Conclusion

Adopting the assumption that the backing value is for storage and the name for everything else, then this:

enum Kingdom: int {
    case Animalia = 0;
    // ...
}

class Bird {
    public int $id = 1;
    public Kingdom $kingdom = Kingdom::Animalia;
}

… would be inserted into the database as:

INSERT INTO bird (id, kingdom) VALUES (1, 0);

… and GET/POST/PATCH/PUT via /birds/1 as:

{
    "kingdom": "Animalia"
}

Moving Forward

Suggestions:

  • Update serializers to default to returning \BackedEnum->name for a non-resource enum
  • Deprecate serializers returning \BackedEnum->value in JSON-(API|LD|HAL)
  • Add api_platform.defaults.extra_properties.use_legacy_php_enum_values: true|false config parameter to cover existing installs

@lermontex
Copy link

lermontex commented Apr 10, 2024

It seems to me that the option of returning a key (as implemented in GraphQL) is more correct

This makes it easier to use Enum values on the front end because we don't need to know what value we are storing in the database.

Backend:

enum InvoiceStatus: int
{
    case CREATED = 1;
    case PAID = 2;
    case CANCELED = 3;
}
#[ApiResource]
class Invoice
{

    #[ORM\Column(type: "int", enumType: InvoiceStatus::class)]
    private InvoiceStatus $status = InvoiceStatus::CREATED;
}

$invoice->setStatus(InvoiceStatus::PAID)

Frontend:

<template v-if="invoice.status === 'CREATED'"></template>
<template v-else-if="invoice.status === 'PAID'"></template>
<template v-else-if="invoice.status === 'CANCELED'"></template>

@GwendolenLynch
Copy link
Contributor Author

For the alternate option, GraphQL\Type\Definition\EnumType can be extended and is created/injected here

$enumType = new EnumType($enumConfig);
$this->typesContainer->set($enumName, $enumType);

@soyuka
Copy link
Member

soyuka commented Apr 11, 2024

We should follow symfony:

symfony/symfony#40830 symfony/symfony#52676

And getCases is used to get custom values if needed as shown at https://les-tilleuls.coop/en/blog/how-to-expose-enums-with-api-platform

Copy link

stale bot commented Jun 10, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 10, 2024
@soyuka soyuka added enhancement and removed stale labels Jun 13, 2024
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