search

api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.45k stars 881 forks source link

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

Open GwendolenLynch opened 7 months ago

GwendolenLynch commented 7 months ago

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:

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 jsonld … jsonhal 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 ```php 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 ```json { "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 ```php #[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 ```json { "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 ```json { "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 ```json { "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 commented 7 months ago

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

https://github.com/dunglas/frankenphp/issues/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 commented 7 months ago

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: https://github.com/api-platform/core/issues/6185 which isn't that wrong as maybe that backed enum is more about keys then values?

GwendolenLynch commented 7 months ago

I disagree it should be IRIs

Easy. I had incorrectly interpreted/assumed parts of what you explained in https://github.com/api-platform/core/pull/6288#issuecomment-2037383301 while trying to mentally reduce it down to the base parts.

GwendolenLynch commented 7 months ago

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

I presume this is also invalid then?

soyuka commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

I couldn't agree more. :+1: 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. :sun_with_face:

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 commented 7 months ago

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 commented 7 months ago

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 :roll_eyes:

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:

lermontex commented 7 months ago

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 commented 7 months ago

For the alternate option, GraphQL\Type\Definition\EnumType can be extended and is created/injected here https://github.com/api-platform/core/blob/5e908c898dd6dca3a6abfd3f083e4d7b4fc4e85a/src/GraphQl/Type/TypeBuilder.php#L216-L217

soyuka commented 7 months ago

We should follow symfony:

https://github.com/symfony/symfony/pull/40830 https://github.com/symfony/symfony/pull/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

stale[bot] commented 5 months ago

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.