Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support finding queries and mutations in the same class #197

Closed
pabl0rg opened this issue Mar 19, 2019 · 10 comments
Closed

Support finding queries and mutations in the same class #197

pabl0rg opened this issue Mar 19, 2019 · 10 comments
Labels
status: wontfix This will not be worked on type: enhancement New feature or request

Comments

@pabl0rg
Copy link

pabl0rg commented Mar 19, 2019

Is your feature request related to a problem? Please describe.
I have a traditional java/kotlin service (that handles both mutations and queries) and I'd like to expose most of it's methods without repeating myself too much, while adding a necessary layer for authorization checks.

Currently, I have to create two classes: one that exposes my existing service's queries and another that exposes the mutations.

For example:

    class UserService {
        val users = mutableMapOf<Int, String>()
        fun getUser(id: Int) = users[id]
        fun createUser(name: String) {
            val id = users.keys.max() ?: 0 + 1
            users[id] = name
        }
    }

    class UserQueries(private val userService: UserService) {
        fun getUser(id: Int) = userService.getUser(id)
    }
    
    class UserMutations(private val userService: UserService) {
        fun createUser(name: String) = userService.createUser(name)
    }

This gets tedious when user has a lot of fields

Describe the solution you'd like
I'm not sure, but I think this might be easier if there was a way to pass lists of functions or function references to SchemaGenerator or QueryBuilder instead of having to define a class. Your library would allow users to compose a graphql api from functions and existing services.

Describe alternatives you've considered
You are probably aware of https://github.com/pgutkowski/KGraphQL. That project seems to have lost some steam, but has some cool characteristics in that it allows composing the graphql api in a direct, simple manner. Unfortunately, there's a lot of boilerplate involved.

Additional context
Add any other context or screenshots about the feature request here.

@pabl0rg pabl0rg added the type: enhancement New feature or request label Mar 19, 2019
@smyrick smyrick added the status: wontfix This will not be worked on label Mar 21, 2019
@smyrick
Copy link
Contributor

smyrick commented Mar 21, 2019

Hello!

Thank you for the detailed issue. You are correct that it is usually the pattern to have your classes contained by scope and not operation, and this translates to REST as well, where you will have multiple controllers but each one is usually scoped to one data point. However GraphQL changes that pattern.

We want to keep the queries and mutations separate because that is how it is represented in the schema. It also helps you as the developer to know the separation in the code and that way there isn't a magic link that says this function in your class appears here on the schema, while this other one does not.

It still would be possible to implement, but we would have to either add a new toSchema that accepted a list of functions, which changes the way we do reflection, or we have to have some way of including certain functions like @GraphQLQuery. This might be better suited for a use case like https://github.com/leangen/graphql-spqr

So we are going to mark this as won't fix for now, but if we hear others who find this feature to be lacking we can revisit.

@smyrick smyrick closed this as completed Mar 21, 2019
@pabl0rg
Copy link
Author

pabl0rg commented Mar 21, 2019

Thanks for the link to graphql-spqr! When the kotlin type annotation bug gets fixed, it will be a good option, though if it were possible to assemble the schema from function references, we could keep schema definition more decoupled from services.

@smyrick
Copy link
Contributor

smyrick commented Mar 21, 2019

@pabl0rg How do you invision specifying the functions that need to be in a query vs the ones in a mutation? This still requires you have one location that lists all the operations, which is what a query class is doing

@pabl0rg
Copy link
Author

pabl0rg commented Mar 21, 2019

I may be totally off, but something like the kgraphql DSL that could accept function references would be nice. For example, I modified fun resolver to be public, (but this did not actually work):

val schema = KGraphQL.schema{
    //users
    query("allUsers"){
        resolver(FunctionWrapper.on(mlService::getAllUsers))
    }

    query("getUser"){
        resolver(FunctionWrapper.on(mlService::getUser))
    }

    mutation("createUser"){
        resolver(FunctionWrapper.on(mlService::createUser))
    }

    mutation("deleteUser"){
        resolver(FunctionWrapper.Companion.on(mlService::deleteUser))
    }
}

@pabl0rg
Copy link
Author

pabl0rg commented Mar 22, 2019

I think i found a way to add what I wanted to KGraphQL. It's working in a sample project, at least.

pgutkowski/KGraphQL#61

@smyrick
Copy link
Contributor

smyrick commented Mar 22, 2019

Since functions can be assigned to values you can do the same with a query class so you don't have to repeat the arguments

class UserService {
    val users = mutableMapOf<Int, String>()
    fun getUser(id: Int) = users[id]
    fun createUser(name: String) {
        val id = users.keys.max() ?: 0 + 1
        users[id] = name
   }
}

class UserQueries(private val userService: UserService) {
    val getUserCustomName = userService::getUser
}
    
class UserMutations(private val userService: UserService) {
    val createUserCustomName = userService::createUser
}

@pabl0rg
Copy link
Author

pabl0rg commented Mar 22, 2019

🤦‍♂️ thanks!

@pabl0rg
Copy link
Author

pabl0rg commented Mar 26, 2019

Hi Shane.

I found a non-spring graphql example project on github and replaced KGraphQL with your library using your suggestion and it kind of worked. But, a lot of methods are missing from the queries/mutations in the schema.

I put the changed version here:
https://github.com/pabl0rg/graphql-example/tree/expedia-graphql-kotlin

This:

class MovieLensQueries(private val movieLensService: MovieLensService) {
    fun allUsers() = movieLensService.getAllUsers()
    fun user(id: Int) = movieLensService.getUser(id)

    val allRatings = movieLensService::getAllRates
    val rating = movieLensService::getRate

    val allGenres = movieLensService::getAllGenres
    val genre = movieLensService::getGenre

    val allOccupations = movieLensService::getAllOccupations
    val occupation = movieLensService::getOccupation
}

class MovieLensMutations(private val movieLensService: MovieLensService) {
    fun createUser(age: Int, gender: String, occupationId: Long, zipCode: String) =
            movieLensService.createUser(age, gender, occupationId, zipCode)

    fun deleteUser(id: Long) = movieLensService.deleteUser(id)
    fun updateUser(id: Long, age: Int, gender: String, occupationId: Long, zipCode: String) = movieLensService

    val createRating = movieLensService::createRating
    val deleteRating = movieLensService::deleteRating
    val updateRating = movieLensService::updateRating

    val createGenre = movieLensService::createGenre
    val deleteGenre = movieLensService::deleteGenre
    val updateGenre = movieLensService::updateGenre

    val createOccupation = movieLensService::createOccupation
    val deleteOccupation = movieLensService::deleteOccupation
    val updateOccupation = movieLensService::updateOccupation
}

fun initGraphQLSchema(movieLensService: MovieLensService): GraphQLSchema {

    val config = SchemaGeneratorConfig(supportedPackages = listOf(
            "com.github.jmlb23.movielense.domain",
            "com.github.jmlb23.movielense"
    ))

    val queries = listOf(TopLevelObject(MovieLensQueries(movieLensService)))
    val mutations = listOf(TopLevelObject(MovieLensMutations(movieLensService)))

    return toSchema(config, queries, mutations)
}

Is Generating this schema (missing queries, has extraneous MovieLensService type)

type Genre {
  id: Int!
  name: String!
}

type MovieLensService {
  createGenre(name: String!): Long!
  createOccupation(name: String!): Long!
  createRating(movieId: Long!, rating: Int!, userId: Long!): Long!
  createUser(age: Int!, gender: String!, occupationId: Long!, zipCode: String!): User!
  deleteGenre(id: Int!): Long!
  deleteOccupation(id: Int!): Long!
  deleteRating(id: Int!): Long!
  deleteUser(id: Long!): Long!
  getAllGenres: [Genre!]!
  getAllOccupations: [Occupation!]!
  getAllRates: [Rating!]!
  getAllUsers: [User!]!
  getGenre(id: Int!): Genre
  getOccupation(id: Int!): Occupation
  getRate(id: Int!): Rating
  getUser(id: Int!): User
  updateGenre(id: Int!, name: String!): Long!
  updateOccupation(id: Int!, name: String!): Long!
  updateRating(movieId: Long!, rating: Int!, ratingId: Int!, userId: Long!): Long!
  updateUser(age: Int!, gender: String!, id: Long!, occupationId: Long!, zipCode: String!): Long!
}

type Mutation {
  createUser(age: Int!, gender: String!, occupationId: Long!, zipCode: String!): User!
  deleteUser(id: Long!): Long!
  updateUser(age: Int!, gender: String!, id: Long!, occupationId: Long!, zipCode: String!): MovieLensService!
}

type Occupation {
  id: Long!
  name: String!
}

type Query {
  allUsers: [User!]!
  user(id: Int!): User
}

type Rating {
  id: Long!
  movieId: Long!
  rating: Int!
  userId: Long!
}

type User {
  age: Int!
  gender: Gender!
  id: Long!
  occupationId: Long!
  zipCode: String!
}

enum Gender {
  FEMALE
  MALE
  OTHER
}

@smyrick
Copy link
Contributor

smyrick commented Mar 26, 2019

Oh, I see what's happening. The value reference works and compiles but our reflection sees the values as actually under the movie service so it generates that type.

Sorry about that, I guess it's not valid. Then the only way I see to fix this is to have the methods explicitly on a new query class. This will at least give some separation of legacy code and code that is new for GraphQL

Or return the movie service as a type in a query field but that name wouldn't make much sense from a GraphQL perspective

@pabl0rg
Copy link
Author

pabl0rg commented Mar 26, 2019

This will at least give some separation of legacy code and code that is new for GraphQL

I was hoping the MovieLensQueries and MovieLensMutations classes could be that layer and not require much boilerplate. (We like having something like the MovieLensService for consumption by other kotlin/java modules in the same process. Monoliths are fast and easy to maintain)

I'll try another approach. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wontfix This will not be worked on type: enhancement New feature or request
Development

No branches or pull requests

2 participants