GraphQl: interface implementation

[CvekCoding]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tickets	#3116
License	MIT
Doc PR

Added interface feature to Graphql.

How to use:

/**
 * @ApiResource(isInterface=true)
*/
abstract class AA {}

/**
 * @ApiResource(isInterface=true)
*/
abstract class AB {}

/**
 * @ApiResource(
 *     implements={AA::class},
 * )
*/
class A extends AA {}

/**
 * @ApiResource(
 *     implements={AA::class, AB::class},
 * )
*/
class C {}

In a scheme you will see that:

• AAInterface has two implementations: A and C;
• ABInterface has one implementations: C;
• A implements one interface AAInterface;
• C implements two interfaces AAInterface and ABInterface.

I'm far from thought that my implementation is ready to be merged. Feature was developed in haste because I needed it in my project, but I didnt have time to think about the best architecture.

I'm ready to answer the questions.

[lukasluecke]

@CvekCoding I think this should target master, and maybe have a more descriptive title 🙂

@alanpoulain Do you think you could take a look at this? I think it would be a big step forward for our GraphQL-support 🚀

[lukasluecke]

I am not sure skipping the operations for interfaces is the correct approach. And if we do, we should at least trigger warnings or something if they are defined anyway.

[CvekCoding]

Weak place, agree.
I did this because in my project I dont need mutations in interfaces. And I use custom operations very few.
Additionally I think I had some issues with mutations in interface types, but dont remember details now :(

We can safely move this conditions to include custom query operations here

core/src/GraphQl/Type/SchemaBuilder.php

Line 90 in a1570c8

[lukasluecke]

This should probably be under the graphql-key, as this is only supported for the GraphQL format / endpoints.

[CvekCoding]

@lukasluecke can you explain graphql-key - what does it mean? Prefix for property name?

[lukasluecke]

@CvekCoding There is already a configuration key graphql, under which we configure the operations etc. - I think it would make sense to include it inside of that, instead of at the root level.

[lukasluecke]

This should probably not be used as a fallback, but instead implement the proper checks (i.e. use Item postfix if the serialization context differs between Item and Collection queries)

[lukasluecke]

@alanpoulain What do you think about this? I've been using it in my project for some time now and it's working quite well.

I think we should address both the name resolution handling (based on serialization context) and the resource config layout (under graphql key), and also simplify this PR so it can be merged with #3321 better (i.e. improve the buildResourceObjectType extraction).

Handling the other operations for interface resources could be addressed in a follow-up PR I think.

[alanpoulain]

I plan to take some time to take a look at it once subscriptions are merged.

[ollietb]

It would be great to see this merged!

[lukasluecke]

@alanpoulain Any chance to get this revisited? I'd be happy to help and contribute, but would be good to have your input first 🙂

[ollietb]

@lukasluecke I can't speak for @alanpoulain but I would imagine this PR will at least need to have tests for it to be considered for merging

[lukasluecke]

@ollietb Of course, but he said he would take a look - not merge it 🙂 this should just serve as an inspiration for a "proper" implementation in my mind, we would first need to define what behavior we want to have, before writing any tests.

[ollietb]

It would be good to see this PR finished, my front-end developers are always complaining about missing features in the GraphQL spec, like interfaces and enums. @alanpoulain would you be able to look at this PR and advise @lukasluecke if he is on the right path before he writes some tests and submits the completed PR?

[stale]

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]

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.