[Feat] CRUD and Nested Mutations

[spawnia]

I am currently trying to build an API on top of an existing Laravel backend, using laravel-graphql. Querying data seems to work fine, although the amount of boilerplate code is quite high.

However, i found myself struggling with implementing mutations.

We have 2 major requirements that make it quite hard:

1. We have quite a complex schema with about 100 inter-related models and a few thousand columns so we do need a way to either generate code or deduce basic CRUD operations from a schema.
2. We need to be able to both query and mutate data with deeply nested relationships.

Prisma does offer a sensible structure for how nested mutations can be dealt with and i feel it would play very nicely with Eloquent, have a look: https://www.prisma.io/docs/1.9/reference/prisma-api/mutations-ol0yuoz6go#nested-mutations

I think we might be able to deduce sensible defaults for CRUD operations simply by defining the types in the schema definition, what do you think?

[olivernybroe]

For your question about generating code, you should look into making a custom directive. Then you can make that pretty much do whatever you want.

[chrissm79]

Hey @spawnia, I was originally going to do something like this for v2.0 but it quickly spiraled into a much bigger issue than I originally thought. I've built on my initial ideas and I could see a nested implementation that would look something like this:

# The `crud` argument tells Lighthouse to create crud mutations for this model.
#
# The `connect` argument allows you to specify which 
# nested relationships you can include in a auto-generated
# crud mutation.
# 
# The @hidden directive prevents these fields from being included in the final schema,
# the fields are just there for validation purposes.
# 
type User @model(
  class: "App\\User", 
  crud: true, 
  connect: ["posts"]
  ) {
  id: ID!
  name: String @validate(rules: ["required"])
  email: String @validate(rules: ["required", "emails", "unique:users,email"])
  password: String @hidden @validate(rules: ["required", "min:8", "confirmed"])
  password_confirmation: String @hidden
  posts: [Post] @hasMany
}

# The `validators` argument allows you to specify validator 
# classes for the `create`, `update`, or `delete` crud mutations.
# 
# The `policies` argument allows you to specify a Laravel policy for
# each crud mutation
# 
# The `transformers` argument allows you to transform the input before 
# it gets injected into the model.
# 
type Post @model(
  class: "App\\Post",
  crud: true,
  validators: { 
    # create: "App\\GraphQL\\Validators\\CreatePostValidator" -- Handled by @validate below
    update: "App\\GraphQL\\Validators\\UpdatePostValidator" 
  },
  policies: {
    create: "create",
    update: "update",
    delete: "delete"
  },
  transformers: {
    create: "App\\GraphQL\\Transfomers\\PostTransformer@create",
    update: "App\\GraphQL\\Transfomers\\PostTransformer@update",
  }
  ) {
  id: ID!
  title: String @validate(rules: ["required", "min:4"])
  body: String @validate(rules: ["required", "min:4"])
  author: User
}

There's a ton going on here, but this is something I think could work for most use-cases while also allowing for custom validation and injecting policies when needed. The crud operations would be wrapped in a transaction so if a nested mutation fails nothing is committed to the DB.

Right now, the biggest problem is getting proper validation error messages when you're creating an array of nested relationships.

I labeled this issue as v2.x because I do plan on adding this functionality into Lighthouse. Let me know if you have any questions/thoughts!

[spawnia]

Thank you for your quick answer, i like what you planned so far. It clearly shows that you have been thinking about this a lot.

What i would suggest for a first working version is to leave out the additional configuration options for @model and focus on just making CRUD operations work. It should be challenging enough.

We might be able to cut down on the necessary configuration even more if we have the crud option default to true. I guess when you define a model in your schema it makes sense to be able to do CRUD on it in most cases?

While i get the intent behind the connect option, i think it might not be necessary. Again, i think the default should be to allow nested mutations, considering the relationship is defined in the fields below. To explicitely exclude fields from mutations, how about we mark them with a @readOnly directive?

On a side note, i think that validating with required should not be done. It is rather preferrable to use GraphQLs own built-in Non-Null type. This improves DX substantially, as it helps with autocompletion, query validation and such via third-party tools.

There are some cases where it might be sensible to add in validation rules such as required_with, but i think that in general the native type should be sufficient.

Below would be an example of a stripped down type definition.

type User @model(class: "App\\User") {
  id: ID!
  email: String! @validate(rules: ["email", "unique:users,email"])
  password: String! @hidden @validate(rules: ["min:8"])
  posts: [Post!] @hasMany
}

type Post @model(class: "App\\Post") {
  id: ID!
  title: String! @validate(rules: ["min:4"])
  body: String @validate(rules: ["min:4"])
  views: Int! @readOnly
  author: User! @belongsTo
}

And this is what the generated schema would be:

# Those two types are used as return types for Queries
# I am not going to spell out the actual Queries since
# i feel that would be out of scope.
# There may be all kinds of filtering, grouping etc. going on.
type User {
  id: ID!
  email: String!
  posts: [Post!]
}

type Post {
  id: ID!
  title: String!
  body: String
  views: Int!
  author: User!
}

type Mutation {
  createUser(data: CreateUser!): User
  createPost(data: CreatePost!): Post
  updateUser(id: ID!, data: UpdateUserData!): User
  updatePost(id: ID!, data: UpdatePostData!): Post
  deleteUser(id: ID!): User
  deletePost(id: ID!): Post
}

input CreateUser {
  email: String!
  password: String!
  posts: [CreateUserPostRelation!]
}

# Only one of the actions in here is allowed
input CreateUserPostRelation {
  connect: ID
  create: CreateUserPost
}

input CreateUserPost {
  title: String!
  body: String
}

input CreatePost {
  title: String!
  body: String
  author: CreatePostAuthorRelation!
}

input CreatePostAuthorRelation {
  connect: ID
  create: CreatePostAuthor
}

input CreatePostAuthor {
  email: String!
  password: String!
}

# Similar to the Create inputs, although all fields are optional
# If they were marked as non-null before, we can still
# validate that if a value is given, the value must not be null.
input UpdateUserData {
  email: String
  password: String
  posts: [UpdateUserPostRelation!]
}

input UpdateUserPostRelation {
  connect: ID
  # If posts were not required to have a user attached we might have:
  # disconnect: ID
  create: CreateUserPost
  update: UpdatePost
  delete: ID
}

input UpdatePost {
  id: ID!
  data: UpdatePostData
}

input UpdatePostData {
  title: String
  body: String
  author: UpdatePostAuthorRelation
}

input UpdatePostAuthorRelation {
  connect: ID
  create: CreatePostAuthor
  update: UpdateUserData
  # If author were nullable, we could have:
  # disconnect: ID
  # delete: ID
}

Notice how it is really long? This is exactly the reason why i think this should be generated automatically. It looks a bit ugly, but it offers complex mutations while still keeping the type safety that makes GraphQL so special.

[chrissm79]

@spawnia, really appreciate the feedback!!!

So, defaulting crud to true would probably be the way I'd go if CRUD were released with v2.0. However, now that people are using it I think the safer route would be to add a config option that allows users to default that option to true, that way they're aware of what's going to happen.

I should probably set some expectations about how CRUD will work in Lighthouse (at least initially). Inputs like UpdatePostAuthorRelation would not be something that Lighthouse would create because that's a bit too domain specific.

If the primary goal of Lighthouse were to be more of a backend ORM type service such as Prisma, that you wouldn't necessarily provide as an open API to the public (i.e., it would be hidden behind another "frontend" schema), I would be all about this approach! However, in my own project, there's probably one or two instances where I would ever want to allow a foreign key to be changed by the end-user.

The CRUD being generated should be thought of more like Laravel's fill, with the addition of filling related models if you choose to allow the users to do so. Personally, my biggest concern of auto-generating relationship crud is when a type is owned by a User. I wouldn't want a end-user to have the option to create a new User in the DB when they're creating a Post. But again, a config option could be set to allow all relationships to be generated via CRUD.

The required validation rule is just a personal preference thing. For instance, I hardly ever use Non-Null on any of my inputs so I can keep the validation errors clean. That way you get a Laravel validation message saying the field is required (along w/ the added benefit that it validates the rest of your input) rather than a GraphQL schema error that the end user likely wouldn't understand or you're frontend needs to handle differently.

Thanks again for your feedback on this @spawnia! Great discussion on a pretty big topic!!

[olivernybroe]

@chrissm79 I like the idea and I think this should be in Lighthouse. However I think we soon need to figure out what we want to be part of Lighthouse and what should be a package.

[chrissm79]

@olivernybroe Couldn't agree more! I think it's reasonable to have Lighthouse optionally include some basic CRUD, but beyond that it should probably be a separate package that manages a more complex schema.

But that's the beauty of Lighthouse, a complex crud system can be generated with a custom directive!

[spawnia]

Enabling CRUD by default

I can get behind not defaulting to true in this major release series. Actually, it seems like CRUD in the way i envisioned it would be a great fit for internal or backend-facing API but might add just a little bit too much power to public APIs.

I get that creating Users as a related object is something that usually would not be allowed. The example is a bit weak in this regard, it makes a lot more sense in other cases. For example, when creating a post, you might want to connect it to some categories and be able to create new categories in the process.

Many times, you will not even expose such options through your UI, but nonetheless it is nice to have a flexible, powerful schema that is able to perform such operations without writing any additional code. If you really do not want someone to perform such actions, there could be an option to explicitely disallow creation or updating as part of a relation.

Reasoning behind the nested inputs

Inputs like UpdatePostAuthorRelation would not be something that Lighthouse would create because that's a bit too domain specific.

Actually, that just follows from a naming convention: Update[Original][Related]Relation. The purpose of this Input is to clearly encapsulate the two options you would have when creating a new Post: Either you connect it to a specific author, or create a new User and set that as the author.

This class of RelationInputs helps to keep the schema dry and actually makes for a pretty nice structure when calling the API. This is what a query for creating a User could be like:

mutation {
  createUser(data: {
               email: "ex@amp.le",
               password: "safetyfirst",
               posts: [
                 {connect: 42},
                 {create: {title: "GraphQL is awesome", body: "Especially nested mutations ;)"}
               ]})
{
  id
  posts {
    id
    title
}        

How to define required arguments

I can definitely see the benefits of using Laravel for validating Inputs as Non-Null. You make some great points for why it is beneficial. In a perfect world, we would be able to change the behaviour of graphql-php to enable validation of the whole input with additional rules before throwing, while still using the native type. This topic could be another issue, as of now it is up to the personal choice which is fine, as you said.

Inclusion into Lighthouse

I really like what Lighthouse is doing in general and hope to avoid having to write yet another GraphQL implementation for Laravel. There is a bunch of them already and all of them do just a few things well but can not really bring it all together. While it would be quite doable to have multiple packages for serving REST endpoints, the nature of GraphQL makes it advantageous to have it all in one package.

The implementation of nested mutations is special in the regard that it has to be integrated quite deeply with how Lighthouse works. Simply defining a model as CRUD has wide-ranging implications: Transactional handling of the mutations, schema generation, validating nested inputs... This makes me think it would be too complex for an extension package.

Should the need arise, it might be a good idea to add in some kind of plugin system to Lighthouse or allow easy extensibility with some custom directives. At a certain point, adding all kinds of use cases could be too much for Lighthouse to handle. However i feel that offering complex CRUD operations with little to no additional coding would be a huge benefit and i would really like it as a part of Lighthouse.

I have been struggling to find a way to deal with nested mutations for our project, and so far Lighthouse looks like the most promising candidate. If you are interested in implementing this, i can get started on this tommorrow. As this is currently a top priority for our project, i will have plenty of time to work on this in the next few weeks.

[olivernybroe]

@spawnia It is possible to do this in Lighthouse. I am for example working on a filter directive which generates the filter methods automatically based on the fields and gives support for filtering relationships fields recursively also. All of this is done with a directive.

I think when you are making this, you will realise that you don't need to change anything in graphql and this could actually be a package. However I vote for adding this in graphql, but having it disabled by default.

[spawnia]

My experimentation so far has lead to me the following idea: We can introduce a special directive called @generate - alternatively a whole class of directives which are prefixed with generate

These directives are handled differently from the other directives. They can only be used on Object Types and are evaluated before all other directives. They are passed the original Document and return a modified version.

Most of the time, they will actually remove the type they came on and define multiple other types, fields, inputs, etc. I will expose methods for common schema manipulations (such as adding a field to the Query root type)

There are multiple benefits to this approach:

• Generated definitions may reuse other directives which are then evaluated
• This clearly marks which directives affect the schema definition itself
• It becomes easier to define custom generators as a clear path for schema manipulation is defined

Example:

type User @generate(
  # This is required when using @generate
  class: "App\\User"
  # Add all available CRUD operations
  crud: true
  ) {
  # List of fields here
}

type Post @generate(
  class: "App\Post"
  # Generate a Query to get a post by ID and another to Query posts paginated
  read: ["single", "paginated"]
  # Expose create mutation but not update or delete
  create: true
  # Those would be the defaults, just adding them in for clarity
  update: false
  delete: false
) {
  # List of fields here
}

@chrissm79 The additional configuration options you mentioned for defining transformers, validators or policies can be added in as well.

[kikoseijo]

Hello people, big feature here,,,

Why not just make a @resource, and point to a class as we doing with routes?

Life should continue as it was,,, some people will use a controller some will use a repository,,,

Laravels naming conventions should work for any class provided, otherwise will look into the repository.

Keep it simple!

[spawnia]

Just a quick update on this, if anyone is following along.

We made some big steps towards this with formal schema manipulation and are starting to make some necessary changes how Input Objects are handled. There are still a bunch of things that need to happen before this, but it is definitely still planned.

[spawnia]

Closing this as #207 implements basic nested mutations.