Graphql namespaces

[RoMiRoSSaN]

I decided open issues after discussion with @jmartisk in PR #2169
Jan said that using namespaces are not recomended. However grpahql spec allows it
Default behavior of graphql and graphql-client does not raise questions.

Server part allows now using annotation Name for grouping apis. I think it is not bad if someone whant grouping api in same namespaces.

At this moment I found some bugs related with namespaces

1. Uncorrect description - Set correct api group description #2168
2. Typesafe client can not working with Name annotation - Add api group names for typesafe clients #2169
3. FederationDataFetcher also working not corretly with namespaces - Fix for federation data fetcher correct support namespaces #2172

May be I can found something else..

So, I want discuss them. What are think about it?
Thanks

[phillip-kruger]

However grpahql spec allows it

Can you point to where in the spec this is defined ?

[RoMiRoSSaN]

Hi. I carefully reviewed the specification and did not find a description of this. However, there is a fairly old discussion on this topic. As well as documentation from Apollo on this topic.
graphql/graphql-spec#163
https://www.apollographql.com/docs/technotes/TN0012-namespacing-by-separation-of-concern

[phillip-kruger]

@jmartisk - your call.

[jmartisk]

I don't know man, I'm a bit hesitant to go through with this approach to namespaces when there's an open proposal to add namespacing to GraphQL - see graphql/graphql-spec#163 and https://medium.com/@oleg.ilyenko/graphql-namespaces-proposal-2a097ce81d2a - the approach in the second link is quite different as it uses a / syntax to denote namespaces, so it may happen that this will be added to GraphQL in the future, and we would be then using a different, incompatible approach :/

Btw I can imagine a way to do this with the current typesafe client too:

With a query like

Namespace {
   customersList(someArgument: Int) {
     name
   }
}

you, I think, could do

class Namespace {
  // empty
}

@GraphQLClientApi
public class MyClient {

  @Query("Namespace")
  Namespace namespace();
  
  List<Customer> customersList(@Source Namespace namespace, Integer someArgument);
  
}

It's not super clean but I think it should work

[RoMiRoSSaN]

Hi. I tested it now. Yes, it can work.
1 - namespaces can be created with using empty classes and Source methods. It is not bad example of how nested namespaces can be done, but it has complexity. Here you need to explicitly separate the code into query/mutations, which can add more code and confusion
2 - namespaces also can be crerated by beans. And partially using annotation query mutation. But only if methods has not arguments. With args need use Source annotation.

Simple example - namespace-demo.zip

These code are, as you say, not super clean. And adds complexity in development. But it works (just need check how subscriptions works).

Works while you don't start using federation. And FederationDataFetcher breaks down here, like if using Name on GraphQLApi (I can create simple reproducer of this, based on two services and cosmo gateway if you need)

And #2172 fixes it for case Name on GraphQLApi. And for your example, but here need use recursion for deep type checks, becouce required method can be deep nested in GraphQLFieldDefinition. This part of code i can modify more.

Just for example, Spring too supports namespaces, but how good it works there I don't know. Just as fact
https://docs.spring.io/spring-graphql/reference/controllers.html#controllers.namespacing

So, maybe it makes sense to allow you to write in any style? Because at a minimum, basic support for Name-based namespaces is already in place, and it works.
The main problem for server is that federations don't work fully. On the client side, such queries can only be written dynamically

[RoMiRoSSaN]

#2172 updated. added recursive traversal to find the required field defenition

[t1]

I thought we're talking about the client side; in my eyes, the server side works good enough.

I played around with the client side and came up with this:

@GraphQLClientApi
interface Api {
    @Query("Product") AllProducts products();
    @Query("Product") GetProduct products(@NestedParameter("get") int id);
}

record AllProducts(List<Product> all) {}

record GetProduct(Product get) {}

record Product(String name) {}

...

var all = api.products().all;
var product = api.products(1).get;

The syntax is not nice, but I didn't get the @Source proposal by @jmartisk working on a @GraphQLClientApi.

And the namespace-demo.zip of @RoMiRoSSaN only contains server side code, doesn't it?

I haven't analyzed the PR, yet, but how would the code look then?

[RoMiRoSSaN]

Hi. Sorry, I guess I didn't set the problems correctly initially.
There are several of them - namespaces and federation

Namespaces

Case 1.

Server code

@GraphQLApi
public class Resource {
    @Query
    @Description("Find users")
    public List<User> findAllUsers() {
        // ...
    }

    @Query
    @Description("Find roles")
    public List<Role> findAllRoles() {
        // ...
    }
}

Client code

@GraphQLClientApi
public interface ResourceClient {
    @Query
    List<User> findAllUsers();

    @Query
    List<Role> findAllRoles();
}

This works without problems. The schema is generated correctly. Everything works fine

query {
    findAllUsers {
       ....
    }
    findAllRoles {
       ....
    }
}

Case 2.

Adding Name and Description annotations to the GraphQLApi class and GraphQLClientApi

@GraphQLApi
@Name("users")
@Description("Users operations")
public class Resource {
    @Query
    @Description("Find users")
    public List<User> findAll() {
        // ...
    }
}

@GraphQLApi
@Name("roles")
@Description("Roles operations")
public class Resource {
    @Query
    @Description("Find roles")
    public List<Role> findAll() {
        // ...
    }
}

In this case, the types rolesQueries/rolesMutation/rolesSubscription are generated separately.

Adding the annotation allows you to split the code in the final schema into different namespaces. And it works out of the box, apparently, for a long time now.

query {
    users {
        findAll {
           ....
        }
    }
    roles {
        findAll {
           ....
        }
    }
}

It works. Here, on the server side, one small flaw pops up, that the group description is not correctly entered in the schema. The fix for this is - #2168
Of course, a feature appears that mutations are not recommended to be allocated in a separate namespace. This is written about on the graphql website, as well as examples on the apollo website and how to bypass the restrictions. Links above.

As for the client part, adding the Name annotation does not give any result, the query continues to be generated without it.

@Name("users")
@GraphQLClientApi
public interface ResourceClient {
    @Query
    List<User> findAll();
}

Query generated like

query {
  findAll() {
     ....
  }
}

In my opinion, it would be nice to fix this (done here - #2169), so that inside smallrye it works out of the box.
Of course, on the client, you can do it like this, for example

record UsersWrapper(List<User> findAll) {
}

@GraphQLClientApi
public interface ResourceClient {
    @Query("users")
    UsersWrapper findAll();
}

But still, it would be nice to have the ability to just use Name for such cases and that's it. It seems to me that this is not a bad option. The one who uses smallrye/quarkus should take care of the problems with mutations and know the solutions. Maybe it is worth adding documentation about this, with some warnings - read about possible problems here (link to graphql/apollo).

Case 3.
Large investment. This can only be done using Source.

Server code

@GraphQLApi
public class Resource {
    public static class CrmNamespaceQueries {
    }
    
    public static class CrmNamespaceMutations {
    }

    public static class UserNamespaceQueries {
    }
    
    public static class UserNamespaceMutations {
    }

    @Query("crm")
    public CrmNamespaceQueries crmNamespaceQueries() {
        return new CrmNamespaceQueries();
    }

    public UserNamespaceQueries users(@Source CrmNamespaceQueries crmNamespaceQueries) {
        return new UserNamespaceQueries();
    }

    public List<User> findAll(@Source UserNamespaceQueries userNamespace) {
        //
    }

    @Mutation("crm")
    public CrmNamespaceMutations crmNamespaceMutations() {
        return new CrmNamespaceMutations();
    }

    public UserNamespaceMutations users(@Source CrmNamespaceMutations crmNamespaceMutations) {
        return new UserNamespaceMutations();
    }

    public SaveResponse save(@Source UserNamespaceMutations userNamespaceMutations, User user) {
        // save user
    }
} 

Then you can execute queries like

query {
    crm {
        users {
            findAll {
                ...
            }
        }
    }
}

mutation {
    crm {
        users {
            save {
                ...
            }
        }
    }
}

The code above can also be simplified a bit by using Name (slightly).

@GraphQLApi
@Name("crm")
public class Resource {
    public static class UserNamespaceQueries {
    }

    public static class UserNamespaceMutations {
    }

    @Query("users")
    public UserNamespaceQueries usersNamespaceQueries() {
        return new UserNamespaceQueries();
    }

    public List<User> findAll(@Source UserNamespaceQueries userNamespace) {
        //
    }

    @Mutation("users")
    public UserNamespaceMutations usersNamespaceMutations() {
        return new UserNamespaceMutations();
    }

    public SaveResponse save(@Source UserNamespaceMutations userNamespaceMutations, User user) {
        // save user
    }
}

And it will work (if you forget about the mutation issues for a while)
This is a very specific case. Few people will do this, but I think there may be enthusiasts.

But with the client it becomes a bit more complicated. you will need something similar like this

record CrmWrapperFindAll(UsersWrapper users) {
    record UsersWrapper(List<User> findAll) {
    }
}

record CrmWrapperSave(UsersWrapper users) {
    record UsersWrapper(SaveResponse save) {
    }
}

@GraphQLClientApi
public interface ResourceClient {
    @Query("crm")
    CrmWrapperFindAll findAll();
    
    @Mutation("crm")
    CrmWrapperSave save(@NestedParameter("users.save") User user);
}

It becomes necessary to either strictly adhere to variable names or use annotation Name. There is a severe lack of examples in the documentation for such specific things.

Documentation with the NestedParameter example only contains 1 sentence about nesting at the end. Maybe you should expand the documentation a bit with an example? For example, unfortunately, I only realized now that it works like this. while write it.

Intermediate result.

It all works. But it would be nice to expand the documentation with more examples of different tricks (case 3).
But also add to the documentation that there is a very simple way to divide requests into groups using Name. If necessary, warn that in case 2 and 3, there may be problems with mutations.
Well, and accept small corrections (#2169 #2168) so that option 2 works completely out of the box.

Federations
Now, the implementation in smallrye does not support working with namespace in any form. Neither case 2, nor case 3. At all.

The fix for this is here (#2172 ). A full type traversal has been added to find the required method and caching of this information.
The fix is ​​small, but it fixes the server part for the federation to work with namespaces.

[t1]

@RoMiRoSSaN: thanks for the extensive explanation. Now it all makes sense 😁

@jmartisk: namespaces are actually relevant for very large models. The article you linked to, is very well thought through, but it requires a change in the spec, and nobody seems to have been working on that for years. OTOH, the "Apollo" solution uses existing schema elements, so they already work and are in use in larger projects. This is what we already support on the server side, so maybe we should stick to that approach for now. If there will be a spec change in the future, maybe it would be possible to just add a configuration option to produce the new kind of schema and queries. (If we have too much time 🤪, we could even add that option now, but use underscores instead of slashes.)

Maybe we should add a new annotation @Namespace instead of "naming" the API. This would make the use-case much more explicit. Esp. for the client, the @Name annotation on an API interface is not self explaining.

For nested namespaces, maybe we could use slashes, i.e.:

@GraphQLApi
@Namespace("crm/users")
public class Resource {
    @Query("all")
    public List<User> findAll() {
        //
    }
}

or even:

@GraphQLApi
public class Resource {
    @Query("crm/users/all")
    public List<User> findAll() {
        //
    }
}

One little detail: the generated schema uses the namespace for the field as well as the type name:

type Query {
  products: productsQuery
}

It would be nice, if the productsQuery would have a capital P, i.e. ProductsQuery.

[RoMiRoSSaN]

So what do you think?
Please consider the changes I have proposed.
1 - #2168 GraphQLApi with Name, so that there is a correct description in the generated schema. In theory, it can be rejected if you accept the Namespace annotation. Well, it does not carry much semantic load.
2 - #2169 GraphQLApiClient with Name, so that there is a correspondence between the server and client code. Until the Namespace annotation support is made. Let it not be too obvious on the server now, but it works. But on the client, it does not.
3 - #2172 The most important thing is for namespaces to start being friends with the federation. This is the most important thing in my opinion.

If you want, I can try to make a version of the Namespace annotation according to @t1 suggestion. In this case, it will probably be necessary to explicitly prohibit the use of Name over GraphQLApi/GraphQLApiClient (throw an error - use Namespace instead of Name). I can try do this in a few days. For now, I want to try to fix the work with the federation completely - #2110 here is the beginning of the problem and an addition from me.

[jmartisk]

Ok I'm fine with adding support for @Name on the client then. While I agree @Namespace would be more understandable for some, I'd refrain from it until it's something that's part of the GraphQL spec, for the reasons I said above (the danger that the spec adds namespaces in some different way). Let's discuss some more details in #2169

[t1]

While I agree @namespace would be more understandable for some, I'd refrain from it until it's something that's part of the GraphQL spec, for the reasons I said above (the danger that the spec adds namespaces in some different way).

I agree that we have to be prepared for a change in the GraphQL spec. The concept of a namespace can be implemented in different ways, and maybe we could support all such possible ways with the same Java code and make the namespace style a configuration option. Something like this:

@GraphQLApi
@Namespace("crm/users")
public class Resource {
    @Query("all")
    public List<User> findAll() {
        //
    }
}

As discussed, by default, we would turn this into grouping types:

type Query {
  crm: CrmQuery
}

type CrmQuery {
  users: UsersQuery
}

type UsersQuery {
  all: [User]
}

And a query like:

query {
    crm {
        users {
            all {
                name
            }
        }
    }
}

If the standard comes as suggested in the article, we can add a config option like quarkus.smallrye-graphql.schema-namespace-style; and if the user sets that to slashes, the schema becomes:

type Query {
  crm/users/all: [User]
}

and a query like:

query {
    crm/users/all {
        name
    }
}

Probably, this wouldn't work with the current version of graphql-java nor with any other GraphQL client or service. But we could already allow it to be underscores; then the schema becomes:

type Query {
  crm_users_all: [User]
}

and a query like:

query {
    crm_users_all {
        name
    }
}

Point is: the Java code could stay unchanged!

[jmartisk]

We can't say for sure what the spec approach will look like, and trying to create a generic Java API sounds quite dangerous to me. What if you then need to, for example, deal with parameters or directives on the 'namespace' fields..
One single Java API that can 'adapt' to multiple outputs can never be made fully concise and developer-friendly. It would make sense in a world where one organization may need to easily switch between the approaches, but I don't see a use case for that here.

[RoMiRoSSaN]

By the way, I found another problem. It is related to subscriptions. If they are inside the namespace, then an error is thrown directly from their graphql-java. This example not working.

@GraphQLApi
@Name("named")
public class Resource {
    @Query
    public String test() {
        return "test";
    }
    @Subscription
    public Multi<Long> testSubscription() {
        return Multi.createFrom().ticks().every(Duration.ofSeconds(1));
    }
}

subscription {
  named {
    testSubscription
  }
}

As I see from the commit history, support for the Name annotation over GraphQLApi was added by @phillip-kruger in 2020. And in 2021 added subscriptins. So even the solution that has existed for a long time does not seem to work correctly.

[t1]

But it works for queries and mutations; subscriptions are by far rarer to be seen, so I think we can live with this limitation.

[RoMiRoSSaN]

I made a draft of how to extend the namespaces suggested by @t1. #2184

If group by parameters from Namespace (by /), then not so many changes are required. This example only replaces Name with Namespace, adds some checks, changes opearionName in the type-safe client.
In my opinion, if continue to use just Name (which expand with a '/' in the name), much fewer changes may be required. And no changes to quarkus will be required.

But it’s not up to me to decide =) And maybe this issue can be closed.