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

Splitting Root Schema to multiple Schemas #646

Open
kunjee17 opened this issue May 1, 2020 · 39 comments
Open

Splitting Root Schema to multiple Schemas #646

kunjee17 opened this issue May 1, 2020 · 39 comments

Comments

@kunjee17
Copy link
Contributor

kunjee17 commented May 1, 2020

First of all Sorry, as I m new to Rust language and framework so I might be asking totally dumb question.

I am already working in GraphQL project having 100s of queries and mutations. It is TypeScript / JavaScript based project. So, there we can easily merge all things in single big fat schema as root node.

I tried similar thing with Juniper but was unable to do it. I thought multiple impl of Query will work as it is, but it didn't. Don't know if I m using rust wrong way or juniper is doing some magic.

If only one Query or Mutation is allowed it would be difficult to accommodate too many queries and mutations. I have gone through documentations and examples couple of times but couldn't find any solutions.

It would be great if someone can point me to right direction for the same. Do let me know if any details are unclear or missing.

@jmpunkt
Copy link
Contributor

jmpunkt commented May 2, 2020

For me it is not clear what "merge" means in this context. I would assume that merge means that two queries Q1 and Q2 should be merged path wise? For example, if the path a.b.c exists in Q1 and path a.b.d in Q2, then merge(Q1, Q2) = a.b.{c,d}. If that is the case, then this should not be possible with Juniper, at least with the code generation approach. In general, it is not possible to define Q1 and Q2 independent and combine their code generation result. We faced a similar problem in #553.

However, maybe you could provide a small example and describe your desired behavior.

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 2, 2020

@jmpunkt sorry that it is not clear. Let me take https://typegraphql.com/ example.

@Resolver(Recipe)
class RecipeResolver {
  constructor(private recipeService: RecipeService) {}

  @Query(returns => Recipe)
  async recipe(@Arg("id") id: string) {
    const recipe = await this.recipeService.findById(id);
    if (recipe === undefined) {
      throw new RecipeNotFoundError(id);
    }
    return recipe;
  }

  @Query(returns => [Recipe])
  recipes(@Args() { skip, take }: RecipesArgs) {
    return this.recipeService.findAll({ skip, take });
  }

  @Mutation(returns => Recipe)
  @Authorized()
  addRecipe(
    @Arg("newRecipeData") newRecipeData: NewRecipeInput,
    @Ctx("user") user: User,
  ): Promise<Recipe> {
    return this.recipeService.addNew({ data: newRecipeData, user });
  }

  @Mutation(returns => Boolean)
  @Authorized(Roles.Admin)
  async removeRecipe(@Arg("id") id: string) {
    try {
      await this.recipeService.removeById(id);
      return true;
    } catch {
      return false;
    }
  }
}

I can write similar n number of classes. And eventually they are merged as on root object that is served as GraphQL point.

Now, in juniper case.

impl Query {

    fn apiVersion() -> &str {
        "1.0"
    }

    // Arguments to resolvers can either be simple types or input objects.
    // To gain access to the context, we specify a argument
    // that is a reference to the Context type.
    // Juniper automatically injects the correct context here.
    fn human(context: &Context, id: String) -> FieldResult<Human> {
        // Get a db connection.
        let connection = context.pool.get_connection()?;
        // Execute a db query.
        // Note the use of `?` to propagate errors.
        let human = connection.find_human(&id)?;
        // Return the result.
        Ok(human)
    }
}

This is the one and only Query impl I can have.

I was looking something like having

impl UserQuery {}
impl ProductQuery{}
imp Query [...UserQuery, ...ProductQuery ]

Sorry for little mix of Rust and TypeScript. I m new to Rust and TypeScript is day job language. But I guess you get the point. If I have all the queries in one big fact impl Query {} it would be very difficult to manager that single file. So, there a way or alternative for same?

@jmpunkt
Copy link
Contributor

jmpunkt commented May 2, 2020

So to be clear, you define two queries, then all fields in the these two queries should be in the RootQuery. So we define the queries.

struct UserQuery;

#[juniper::graphql_object]
impl UserQuery {
     fn user(&self, user: UserId) -> User { todo!() }
}

struct ProductQuery;

#[juniper::graphql_object]
impl ProductQuery{
    fn product(&self, id: ProductId) -> Product { todo!() }
}

Then after the "merge", the RootQuery should be the following.

#[juniper::graphql_object]
impl RootQuery {
     fn user(&self, user: UserId) -> User { todo!() }
     fn product(&self, id: ProductId) -> Product { todo!() }
}

Sadly there is no way to tell Juniper to merge these objects. Implementing such behavior in Juniper should be possible.

For this example, the easiest workaround with Juniper would be

pub struct RootQuery;

#[juniper::graphql_object]
impl RootQuery {
     fn users(&self -> UserQuery { UserQuery }
     fn products(&self) -> ProductQuery { ProductQuery }
}

or with a different syntax but the same object

#[derive(juniper::GraphQLObject)]
pub struct RootQuery {
     users: UserQuery,
     products: ProductQuery,
}

impl RootQuery {
    pub fn new() -> Self {
        Self {
             users: UserQuery,
             products: ProductQuery,
       }
    }
}

Your GraphQL query requires an additional path segment (users or products).

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 2, 2020

@jmpunkt thanks for explaining. I guess I did get my answer. I was looking for similar thing. I don't mind path segment until things stays clear.

You can close this issue. Please do the honors. :)

@LegNeato
Copy link
Member

LegNeato commented May 2, 2020

We had previously discussed something like serde's flatten...would that do what you want?

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 3, 2020

@LegNeato I guess you are asking @jmpunkt . Sorry to pitch in, but you are right. Something similar to serde's flattern .

PS: It might need warning or error message if there is duplicate schema definition. Like findById in users and findById in products. Normally graphql peple don't use this kind of names but JavaScript / TypeScript gives that kind of check while merging schema.

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 7, 2020

@LegNeato @jmpunkt side question. Any plans to add this flatten option in Juniper? Obviously not now but in future before some concrete release?

@tyranron
Copy link
Member

tyranron commented May 7, 2020

@kunjee17 @LegNeato @jmpunkt I think the new design described in #553 will solve this issue too without providing any flattening capabilities. Simply, it will allow to specify multiple impl blocks, which can be defined in different modules, so the "merge" will happen naturally.

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 7, 2020

@tyranron that would be great to have. BTW I did tried to have different Query impl block but it crashed. I m new to rust and juniper so it is more difficult for me to find the real issue.

Dumb question. That is incoming feature right. Or I just missed something already there in Juniper ?

@tyranron
Copy link
Member

tyranron commented May 7, 2020

@kunjee17 it will be incoming feature if community and maintainers will decide so. At the moment it's only a possible incoming feature 🙃

Current juniper implemetation doesn't contain such capabilities.

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 7, 2020

@tyranron thanks for reply. It would be nice to have such feature. Let's see what community and maintainers decide. :)

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 8, 2020

@tyranron @LegNeato @jmpunkt is it possible to achieve something like this in Rust. Below code is from Scala.

val api1 = graphQL(...)
val api2 = graphQL(...)

val api = api1 |+| api2

It will merge all the root in one big root. And I m not asking about current feature but in general probable feature.

@tyranron
Copy link
Member

tyranron commented May 8, 2020

@kunjee17 i think, yes.

enum UnitedApiQueryRoot {
    Api1(Api1QueryRoot),
    Api2(Api2QueryRoot),
}

But at the moment, juniper doesn't provide capabilities to derive code for this use case automatically. So, you need to implement GraphQLType manually for this enum, where you can provide the merged information about two APIs.

@kunjee17
Copy link
Contributor Author

kunjee17 commented May 8, 2020

@tyranron neat. I should give it a try to this as well. Thanks for info.

@vladinator1000
Copy link

vladinator1000 commented Jun 5, 2020

This is also related to #182

My dream is to make a macro that lets you merge an arbitrary number of schemas or gaphql objects like this

merge!(user, post, friends);

For inspiration, in JavaScript you can merge type definitions, as well as resolvers and schemas.

I'm not sure how to iterate over a graphql_object's internals to merge them but I feel that looking at the derive object macro code would be a good starting point because that's where the GraphQLTrait implementation happens.

I feel like this would be great for apps with big schemas. It's also worth mentioning this design should be kept strictly compile-time and not be used for distributing graphql schemas and merging them at runtime, which has it's own pitfalls.

@kunjee17
Copy link
Contributor Author

kunjee17 commented Jun 6, 2020

@vladinator1000 As in one project which is type script based we are indeed merging schema but in another project where I am trying Rust. We are not. And frankly I am better of not merging it.

It will more readable queries

query  {
      findPersonById(id : String) {
         name
      }
}

against

query {
  person {
       findById(id : string) {
         name 
      }
  }
}

Again it is question of choice but I like the the second option, where I don't need to merge.

@vladinator1000
Copy link

@kunjee17 yeah that seems totally reasonable, but dependent on how you design your schema. It's also worth noting the graphql spec has an extends keyword that lets you do something similar

@vladinator1000
Copy link

Rust supports multiple impl blocks, I wonder if we could use this to our advantage?

@kunjee17
Copy link
Contributor Author

@vladinator1000 I did tried that but as of now current version, juniper don't support that. Also, with my limited knowledge of macros I don't know how it will pick it up. Again, I will say I am quite happy with solution provided here. Even C# graphql advise to go with separated schemas.

query {
   person {
       getById(id) {
       .... 
      }
   }
   customer {
       getById(id) {
       .... 
      }
   }
}

instead of

query {
   getPersonById(id) {}
   getCustomerById(id) {}
}

I still get that feature might be useful but it will surely not the road block for using Juniper. There are may couple issues blocked though which you might have to look if you want to use async version of it.

@jerel
Copy link
Contributor

jerel commented Jul 14, 2020

Just to add one observation to this conversation... I've also been needing to split my schema across multiple domains and this approach has worked well for both queries and mutations:

pub struct QueryRoot;

#[juniper::graphql_object(Context = Context)]
impl QueryRoot {
  fn settings(&self) -> settings::SettingsQuery {
    settings::SettingsQuery
  }
  fn users(&self) -> users::UsersQuery {
    users::UsersQuery
  }
}

and then I can implement settings and users in their respective modules. However this doesn't appear to work the same for subscriptions. As far as I can tell subscriptions have to be implemented directly in the root. Trying the same pattern gives:

#[juniper::graphql_subscription(Context = Context)]
impl SubscriptionRoot {
  async fn settings(&self) -> settings::SettingsSubscription {
    settings::SettingsSubscription
  }
}

the trait bound `gql::settings::SettingsSubscription: 
juniper::macros::subscription_helpers::ExtractTypeFromStream<_,
juniper::value::scalar::DefaultScalarValue>` is not satisfied
. . .

@kunjee17
Copy link
Contributor Author

@jerel I guess that is Subscription issue. I guess it would be good to raise a separate issue for same. @tyranron @LegNeato would be the better judge for that.

@Ericnr
Copy link

Ericnr commented Jul 19, 2020

@kunjee17 @LegNeato @jmpunkt I think the new design described in #553 will solve this issue too without providing any flattening capabilities. Simply, it will allow to specify multiple impl blocks, which can be defined in different modules, so the "merge" will happen naturally.

This proposal is very interesting imo. Being able to use default resolvers + impl is nice qol and allowing multiple impl of QueryRoot/MutationRoot is a MUST imo.

@Ciantic
Copy link

Ciantic commented Dec 18, 2020

So to be clear, you define two queries, then all fields in the these two queries should be in the RootQuery. So we define the queries.

struct UserQuery;

#[juniper::graphql_object]
impl UserQuery {
     fn user(&self, user: UserId) -> User { todo!() }
}

struct ProductQuery;

#[juniper::graphql_object]
impl ProductQuery{
    fn product(&self, id: ProductId) -> Product { todo!() }
}

Then after the "merge", the RootQuery should be the following.

#[juniper::graphql_object]
impl RootQuery {
     fn user(&self, user: UserId) -> User { todo!() }
     fn product(&self, id: ProductId) -> Product { todo!() }
}

Sadly there is no way to tell Juniper to merge these objects. Implementing such behavior in Juniper should be possible.

I think this should be worthy goal. It would allow DDD type of splitting of concerns.

In practice this could mean like having crates that handle different features. E.g.

  • users crate that implements the users part of the root query, and mutation
  • products crate that implements the products pat of the root query and mutation

Then in your app crate you just merge all the schemas you want.

It would be easy then to just add users crate and get all GraphQL and logic for free, instead of adding the source code related to users handling to each project.

@Ericnr
Copy link

Ericnr commented Dec 29, 2020

Async-graphql was able to achieve this, albeit its not as simple as having multiple impl blocks https://async-graphql.github.io/async-graphql/en/merging_objects.html, but Ive been using it anyway. @tyranron has there been advancements with that new design?

@tyranron
Copy link
Member

tyranron commented Jan 4, 2021

@Ericnr no, I haven't worked on it recently.

@videni
Copy link

videni commented May 26, 2021

I also created a issue Best way to organize root resolvers, why Juniper force us to put all root resolves in a single file? for me, this idea is totally insane. even 1 root resolver got 100 lines of code in my case, I can see I will have serval hundreds very soon, it is a nightmare to maintain a file like this.

@videni
Copy link

videni commented May 26, 2021

@kunjee17 did you find an alternative please? it seems the comunity is not serious about this, which frustrates me a lots.

@kunjee17
Copy link
Contributor Author

@videni currently I'm going with multiple resolvers . Combined in single resolver in root query. As mentioned earlier. Same way dotnet guys are doing it. If you like I ll share example of same.

@videni
Copy link

videni commented May 28, 2021

@kunjee17 yes, please, sorry for the late response.

@kunjee17
Copy link
Contributor Author

Hi @videni Here is how current code likes for me.

impl Query {
    fn api(&self) -> ApiQuery {
        ApiQuery
    }
    fn database(&self) -> DatabaseQuery {
        DatabaseQuery
    }
    fn auth_query(&self) -> AuthQuery {
        AuthQuery
    }
}

While ApiQuery looks like this

impl ApiQuery {
    fn api_version() -> &'static str {
        "1.0"
    }
}

I have skipped the macros for readability purpose. Same goes for mutation and all. This allows separation of modules even at graphQL level. I did have worked with stitching schema approach while working with node js. That is also an option. I feel it is just question of choice.

In above approach you have to query

query user {
   getById {
      username   
   }
}

and if you are stitching the schema it would be like below

query getUserById {
   username
}

I hope I m clear in explaining. Let me know if something is still not clear.

@arthurfiorette
Copy link

arthurfiorette commented Oct 31, 2021

The only problem with using this work around is that the graphql response looks like this:

#[juniper::graphql_object]
impl RootQuery {
  fn api(&self) -> ApiQuery {
    ApiQuery
  }
}

#[juniper::graphql_object]
impl ApiQuery {
  fn version() -> &'static str {
        "1.0"
  }
}

-->

{
  "data": {
    "api": {
      "version": "1.0"
    }
  }
}

And that implies that client side code generation also accounts on that "api' property prefix.

// Example in js
const response = await makeGeneratedGraphqlResponse();

let usefulResponse = response.data.api.version;

With this example of "api" it looks fine, but with compound names it can lead to readability and compatibility problems so easily.

I think all this client-side effect is too much effort to just not allow queries and mutations across multiple files instead a single QueryRoot struct in a single file. @tyranron, juniper has any plans to change this?

@tyranron
Copy link
Member

tyranron commented Nov 3, 2021

@arthurfiorette it's kinda untrivial to change this preserving ergonomics. So, definitely not in the near future, but yes in longer prespective.

@nikis05
Copy link

nikis05 commented Jan 22, 2022

I managed to put together a working solution that relies on undocumented fields and a minuscule runtime overhead, with an attribute macro named composable_object and a proc macro named composite_object. Works only with unit-like structs that implement Default. Example usage:

#[derive(Default)]
struct UserQueries;

#[composable_object]
#[graphql_object(Context = Context)]
impl UserQueries {
    // ...
}

#[derive(Default)]
struct OrganizationQueries;

#[composable_object]
#[graphql_object(Context = Context)]
impl OrganizationQueries {
    // ...
}

composite_object!(Query<Context=Context>(
    UserQueries,
    OrganizationQueries
));

If anyone is interested let me know, I'll publish it as a crate.

I would like to point out that this feature is very important imo. For me it was the biggest downside when weighing this excellent crate against async-graphql. It is impossible to group controllers by features without it, e.g. instead of having field resolvers for User grouped together with queries related to User we are forced to put all queries in one file. Typescript GraphQL libraries (nestjs, type-graphql) even have separate decorators for queries / mutations because it is such a common pattern, e.g.

class UserController {
    @FieldResolver()
    username(): String {
        // ...
    }

    @Query()
    users(): User[] {
        // ...
    }

    @Mutation()
    createUser(): User {
        // ...
    }
}

Obviously it is harder to do in Rust because it would require some sort of a global registry which doesn't blend well with macro generated code, but at least first class support for merging unit-like objects would be nice.

@kunjee17
Copy link
Contributor Author

@tyranron @LegNeato this would be good to have feature. Not as outside create but as part of Juniper. It will follow most Graphql libraries are following. I am in favor of providing this option.

@tyranron
Copy link
Member

@kunjee17 we're aware of this issue and have plans to resolve it, but in a sligtly different manner. It's not quite critical, so is not in our priority list at the moment. Not in 0.16 release, certainly, but in 0.17, probably, yes.

@kunjee17
Copy link
Contributor Author

@tyranron thanks for reply. Looking forward to it.

@fdaciuk
Copy link

fdaciuk commented Jul 23, 2022

I managed to put together a working solution that relies on undocumented fields and a minuscule runtime overhead, with an attribute macro named composable_object and a proc macro named composite_object. Works only with unit-like structs that implement Default.

If anyone is interested let me know, I'll publish it as a crate.


Hey @nikis05! Did you publish the crate? I'm really interested :D

@nikis05
Copy link

nikis05 commented Jul 23, 2022

@fdaciuk https://crates.io/crates/juniper-compose enjoy :)

@kelvinmandlik
Copy link

@tyranron It's been more then 2 years any updates on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests