Muatation namespace

[Lappihuan]

This PR adds the ability to turn a Controller into a Mutation Namespace as described in #440

The specific Implementation adds a MutationNamespace Attribute/Annotation which can optionally be added to a Controller to wrap all its mutations into a Mutation Namespace.

This would look something like this:

#[GQL\MutationNamespace]
class UserController {
    #[GQL\Query]
    public function user()...

    #[GQL\Mutation]
    public function create()...

    #[GQL\Mutation]
    public function update()...

    #[GQL\Mutation]
    public function delete()...

Which turns into:

UserMutations {
   create()...
   update()....
   delete()...
}

There is a optional name parameter on the MutationNamespace Attribute to rename automatically generated Namespace.
The automatically generated Namespace is the Classname - Controller + Muatations which turns UserController into UserMutations.

All queries in the Controller are completely unaffected.

[oojacoboo]

Thanks for this @Lappihuan.

Firstly, is there any reason why we're limiting namespaces to mutations and not queries as well? I'm assuming we can also namespace queries? And, in this case, a GQL\Namespace attribute seems appropriate. Can types be namespaced? Should it be GQL\OperationNamespace if not?

Regarding the default namespacing name, this is going to be contentious, as everyone is going to have different "controller" class names. For instance, our "controller" namespace is Operation/Foo/Mutation. I don't think we can settle on a default naming scheme that will have much unanimity. At any rate, I don't think the chosen default is ideal. We should probably just stick with whatever the class is, or even the entire namespace of the class. I suspect most people will need to define the name option.

[Lappihuan]

Firstly, is there any reason why we're limiting namespaces to mutations and not queries as well? I'm assuming we can also namespace queries? And, in this case, a GQL\Namespace attribute seems appropriate. Can types be namespaced? Should it be GQL\OperationNamespace if not?

@oojacoboo
This is a simple convention to group C*UD from CRUD since GraphQL only offers two Root queries (Mutation, Query) and since R from CRUD already maps 1:1 to Query, CUD could be grouped into a "Namespace".
But all thats it, is a parent Mutation user with the subType UserMutations which has all the mutations as fields.

As for the Attribute name, Namespace is a reserved word in PHP thats why i went with MutationNamespace since it describes the feature best.

I don't see this being usefull for Queries. And about Types, its just that a Type with fields, you'd be better to nest two types.

As for the naming, its just masking Controller out and Adds Mutations i don't mind adding a bunch common words to mask out and as soon as that convention doesn't work the name property can be used.

I actually forgot to set this as a Draft since there is quite a bit of stuff in the Technical implementation that should be improved.
Or at least discussed.

I'm unsure if those fields and Types should be created by a TypeMapper so they can be cached and so on. And if so i don't know a way of doing so, but it might be easier than i think.

[andrew-demb]

Can we add a possibility to specify a namespace as of #[Query] / #[Mutation] attributes too?
Or allow using new separated namespace attribute specified over methods?

We use invokable controllers with a single method in every class, so adding attribute over class (operation-related attribute already specified over method) would be a bit uncomfortable :)

[Lappihuan]

@andrew-demb do you have an example of such a invokable Controller?
I'm wondering if you shouldn't just use #[Mutation] as your "namespace" level and use #[Field] for whatever is nested in it.
Which would already be supported for your case.
See #182

Maybe to clarify, this is not a spec or anything special, its just a convention that should bring some DX to graphqlites way of grouping Root Level Mutations with controllers.

[andrew-demb]

@Lappihuan
Here is an example:

// directory structure
Controller/
  Session/
    RevokeController.php
    StartController.php
_________________
// RevokeController.php
// ...
use TheCodingMachine\GraphQLite\Annotations\Mutation;

final class RevokeController
{
    #[Mutation(name: 'revokeSession')]
    public function __invoke(): bool
    {
        // just for example
        return true;
    }
}

I'm wondering if you shouldn't just use #[Mutation] as your "namespace" level and use #[Field] for whatever is nested in it.
Which would already be supported for your case.

Am I be correct if it will require to make some additional layer of classes with logic for "namespaced mutations"?

I think best variant for me will be a possibility to specify namespace parameter for #[Mutation] over controller method. It should be simple to use and namespace declaration will be near to mutation declaration

[oojacoboo]

@Lappihuan makes sense and after reviewing our schema, applying namespaces to mutations is really all that's needed. We already have around 100 mutations and only a portion of our apps are using GraphQL. Luckily we have a decent pattern in place, but it's already becoming more problematic to find mutations.

#[MutationNamespace] sounds like an appropriate name to me.

https://graphql-rules.com/rules/mutation-namespaces

This list of rules are great overall. The one on mutation namespaces does a pretty good job explaining the problem namespaces solve for others interested.

@andrew-demb makes a good point about defining namespaces on #[Mutation] directly. But, not necessarily for the reason he mentioned, as that's a fairly strange implementation. That said, I could easily see cases where you might have mutation/controller methods scattered across various classes that you want grouped together into a single namespace. I could also see a desire to include a mutation into multiple namespaces.

What if we keep #[MutationNamespace] on a class which puts all mutations in that class within the defined namespace. Then, also allow the attribute to be applied to mutation methods (must have Mutation attribute applied). If the MutationNamespace is provided for a mutation method, it's placed within the defined namespaces and not the class' namespace - full override. To allow for multiple namespaces, the MutationNamespace could instead take an array for a property called names. Or, we could keep name and make name a string|array type for API simplicity.

#[GQL\MutationNamespace(name: 'User')]
class UserController {
    #[GQL\Query]
    public function user()...

    #[GQL\Mutation]
    public function create()...

    #[GQL\Mutation]
    public function update()...

    #[GQL\Mutation]
    #[GQL\MutationNamespace(name: ['User', 'Role'])]
    public function delete()...

[Lappihuan]

@oojacoboo @andrew-demb
I'm actually thinking about following the pattern @devmaslov started with #[Input] where you can define multiple Namespaces on classes and have a for property on #[Muatation] and #[Query] similar to how #[Field] works:

#[GQL\MutationNamespace(name: 'UserMuatations')]
#[GQL\MutationNamespace(name: 'UserQueries')]
#[GQL\MutationNamespace(name: 'Misc')]
class UserController {
    #[GQL\Query(for: "UserQueries")]
    public function user()...

    #[GQL\Query(for: "UserQueries")]
    public function userAlt()...

    #[GQL\Mutation(for: "UserMuatations")]
    public function create()...

    #[GQL\Mutation(for: "UserMuatations")]
    public function update()...

    #[GQL\Mutation(for: ["UserMuatations","Misc"])]
    public function delete()...

    #[GQL\Mutation(for: "Misc")]
    public function computeX()...

This gives quite a lot of flexibility and should also integrate with your Invokable Controller pattern as far as i can tell @andrew-demb

I'd actually also like to add the ability to define multiple #[Type] Annotations since i recently was suprised that Type doesn't support that and i really enjoy that feature on Input.

[oojacoboo]

@Lappihuan The MutationNamespace can be defined anywhere though, right? You could even include them all in a single namespace file if you wanted?

What #[Type] annotations are you referring to?

[Lappihuan]

@oojacoboo yes i'd hope so, but i'm still a bit unsure about the implementation of the whole thing.

The output Type annotation https://graphqlite.thecodingmachine.io/docs/annotations-reference#type-annotation

[oojacoboo]

Misread you on the #[Type] annotations - sounds good there.

As for the API design, I love it 🎉

[Lappihuan]

Now i'd actually call it RequestGroup or something alike since it can be used for both Mutations and Queries.

[oojacoboo]

Mutations and queries are called "operations". So, OperationNamespace would be appropriate. Or just Namespace

[oojacoboo]

@Lappihuan now that 6.0 is released, it'd be great to get this merged in for 6.1.

[oojacoboo]

@Lappihuan hope we haven't lost you. What do we need to do to get this one merged in?

[oojacoboo]

Anyone else interested in helping wrap up this PR?

[Lappihuan]

I've since learned this should not be done with the current gql spec: MichalLytek/type-graphql#1219 (comment)

[oojacoboo]

@Lappihuan thanks for the update. I did a little more digging into this topic - interesting and certainly not obvious without a pretty strict interpretation of the spec. Nonetheless, it probably is something we should avoid.

Here is a good article that discusses the topic in detail.
https://graphqlme.com/2020/07/09/namespaced-graphql-mutations-its-a-trap/o

There is an ongoing proposal to add metadata support to the GraphQL spec. This is probably the ideal way of handling this need.