core icon indicating copy to clipboard operation
core copied to clipboard

test: Backed enum resource tests

Open GwendolenLynch opened this issue 1 year ago • 7 comments

Q A
Branch? 3.2
Tickets #6264 #6279 #6283
License MIT

@soyuka this is just an updated version of the tests I linked yesterday when discussing #6264 & #6279 … and if I m still wrong please just close this PR. :+1:

These tests illustrate what was working for json, jsonhal, and jsonld formats, but breaks now with the change in https://github.com/symfony/symfony/pull/54315

However, if a fix/workaround is found these tests will still add additional coverage for enum resources.

Notes:

  • Tests pass when with:
    • core 3.2 as-of v3.2.19 (i.e. pre #6283)
    • symfony/framework-bundle <=7.0.5 (i.e. pre https://github.com/symfony/symfony/pull/54315)
  • ~Missing jsonapi tests due to target branch~ (Derp! serializer !== schema factory)

GwendolenLynch avatar Apr 04 '24 07:04 GwendolenLynch

Symfony have merged #54484 so I've taken this out of draft.

I've added a conflict for 6.4.6 || 7.0.6 of symfony/framework-bundle and that should have core covered.

GwendolenLynch avatar Apr 04 '24 11:04 GwendolenLynch

Nice addition, API Platform supports enums only on properties for documentation via OpenAPI and using GraphQl but we did not add the support for RDF formats such as JSON-LD. I started working on this (https://github.com/soyuka/core/commit/48db6fa3e63f2207048069db99c022898860f5e1) but it needs more work. Currently it implements the context:

#[ApiResource(operations: [])] // An enum can't have operations so this needs to be fixed in he Metadata Factory
enum BackgroundColor: string
{
    case Sand = 'sand';
    case Magenta = 'magenta';
    case Teal = 'teal';
    case Blue = 'blue';
    case Mauve = 'mauve';

    /**
     * @return string[]
     */
    public static function values(): array
    {
        return array_column(self::cases(), 'value');
    }
}

enumcontext

My reference for the work is what's done on schema.org, for example:

https://schema.org/EnergyEfficiencyEnumeration

This is of type Enumeration and you can see it's "Enumeration members" at https://schema.org/EUEnergyEfficiencyEnumeration. When looking at the schema.org jsonld https://github.com/schemaorg/schemaorg/blob/main/data/releases/26.0/schemaorg-all-https.jsonld we find lots of examples:

{
      "@id": "schema:GamePlayMode",
      "@type": "rdfs:Class",
      "rdfs:comment": "Indicates whether this game is multi-player, co-op or single-player.",
      "rdfs:label": "GamePlayMode",
      "rdfs:subClassOf": {
        "@id": "schema:Enumeration"
      }
},

https://github.com/schemaorg/schemaorg/blob/c7dc8e5a3ad9f1e1645e282a18e46b5d2c19a657/data/releases/26.0/schemaorg-all-https.jsonld#L19759-L19767

And one of the value of the enum has this type:

{
      "@id": "schema:SinglePlayer",
      "@type": "schema:GamePlayMode",
      "rdfs:comment": "Play mode: SinglePlayer. Which is played by a lone player.",
      "rdfs:label": "SinglePlayer"
},

https://github.com/schemaorg/schemaorg/blob/c7dc8e5a3ad9f1e1645e282a18e46b5d2c19a657/data/releases/26.0/schemaorg-all-https.jsonld#L24689-L24694 (there are other values on this enum, just like for EnergyEfficiency).

So, for each value of the Backed Enum we should provide a given type to be referenced in the docs.jsonld.

soyuka avatar Apr 04 '24 14:04 soyuka

I reverted the priority change on our source code https://github.com/api-platform/core/commit/2a8767108690c35d873a936f9e2383365fdb4f00

Suggestion on that, it is probably safe to revert the change to docs.feature as well.

Oh, and it is probably worth at least cherry-picking the composer change from here as that will prevent Symfony "broken" versions.

GwendolenLynch avatar Apr 04 '24 14:04 GwendolenLynch

I really like this but I won't be able to merge it right away as we need to work on the Hydra implementation.

No stress. I was more trying to get coverage for how it is now, as it is "in the wild" as far as use (I'm also guilty there) … supported or not.

GwendolenLynch avatar Apr 04 '24 14:04 GwendolenLynch

Suggestion on that, it is probably safe to revert the change to docs.feature as well.

Actually this is a bad test, I already picked the change on main (it's due to a property-info fix) and has no impact on the expected feature.

Suggestion on that, it is probably safe to revert the change to docs.feature as well.

I plan to do that indeed.

No stress. I was more trying to get coverage for how it is now, as it is "in the wild" as far as use (I'm also guilty there) … supported or not.

No problem we can work on integrating this, I'm afraid that it won't get into 3.3 as I'm already a month late on that release but it's definitely going to be a top priority for API Platform 3.4. Anyways if you want to work on this feel free to do so, I'll see if I can in the upcoming weeks although I'll be afk for 3 weeks (except on weekends). I'll open a new PR with this test and my work for starters.

soyuka avatar Apr 04 '24 14:04 soyuka

To catch future versions of some of the issues hit, I have added test for int and string enum resources, as well as GraphQL tests too.

~The latter seems to have uncovered some internal-use of deprecated code in the GraphQL component.~

Edit: GraphQL additions are based on https://gist.github.com/dunglas/f2e756725b1b842331dc8cdd38b4cc75

GwendolenLynch avatar Apr 09 '24 08:04 GwendolenLynch

also we need to add the enum to the jsonld context, something like this:

    {
      "@id": "schema:CoOp",
      "@type": "schema:GamePlayMode",
      "rdfs:comment": "Play mode: CoOp. Co-operative games, where you play on the same team with friends.",
      "rdfs:label": "CoOp"
    },
{
      "@id": "schema:GamePlayMode",
      "@type": "rdfs:Class",
      "rdfs:comment": "Indicates whether this game is multi-player, co-op or single-player.",
      "rdfs:label": "GamePlayMode",
      "rdfs:subClassOf": {
        "@id": "schema:Enumeration"
      }
    },
    {
      "@id": "schema:SinglePlayer",
      "@type": "schema:GamePlayMode",
      "rdfs:comment": "Play mode: SinglePlayer. Which is played by a lone player.",
      "rdfs:label": "SinglePlayer"
    },

soyuka avatar Apr 15 '24 17:04 soyuka

tyvm @GwendolenLynch ! Looking forward to merging the enum feature next (I still need to try the branch and see what I can improve on the URI thing).

soyuka avatar May 24 '24 07:05 soyuka