graphql-java-tools

graphql-java-tools copied to clipboard

I currently extensively use the graphql-java-tools library with success.

But I have a use case where I would like to relax/bypass the type checking of objects returned from a resolver.

Let's take a simple example with a graphql service A which has the following resolver:

public class FooResolver implements GraphQLQueryResolver {
    public [Map or JsonNode] foo(String id, DataFetchingEnvironment env) {
       // make graphql call to other service B
       JsonObject foo = callRemoteService()

      // here, i don't want to return an umarshalled instance of FooDto, but I want to return either
      // a json object or a map.
      return foo.
    }

The type Foo is declared in the graphql schema B and imported in the schema A. So we have the full type definition.

But in the code, I do not want to have the burden of redeclaring all the java types coming from schema B.

Is there any way to address this case ?

Thanks.

I've been mulling over type checking in this library since i ran into problems using kotlin co-routines, and the more i think about it the more it makes sense to relax type checking in this library. It should be that you can return different data classes for a graphql type in different field locations, and one Data class should also be returnable for different graphql types. I think the library should just check that field names in data classes and resolvers correspond with the field names on the graphql type and get out of the way.

If I remember correctly, this was the behavior in version 1.x of this library. @apottere

That's really interesting. I subscribe to your point of view. I would be really interested in having one Data class for different graphql types. Ideally, it would be great to have a way to cut the type introspection at some point and let the data go through without any type check.

@oexza That wasn't quite the behavior in 1.x - in 1.x it didn't introspect any types, but it required you to provide a map of all the types you would ever use.

I'm open to removing the introspection (it would make the library a lot simpler), but the two reasons I added it in the first place are:

1. I wanted all reflection to be done on startup - w/r/t app startup time reflection is fast, but w/r/t the critical path of the application, reflection is slow. If we relax the type checking, that means we'll have to be scanning classes for methods/fields in the critical path. This can be mitigated by caching the discovered field -> method mapping after the first time it's discovered, but that brings me to:
2. Introspecting types allows the library to do a sanity check on app startup and fail if there are situations it finds won't work, in addition to giving warnings for less serious issues. Since the schema is decoupled from the code, I think this is really valuable (when it works, historically, this has been really difficult to get right). Removing type discovery means that all those errors will be found at request time, and not app startup time.

@apottere I don't want to completely remove the introspection, I'd just like to bypass the introspection at some point of the model tree, particularly on all fields which are intended to represent an edge.

Of course I still want to benefit from the security of typing, I think it's really valuable too.

I'd also support relaxing type checking. I'm using an orm library where concrete types of data objects are created at runtime. When these objects are returned from a resolver, graphql-java-tools is unable to map the data object's type to the corresponding gql type, despite the fact that these runtime-generated data objects implement the interface required by the gql type.

@oexza @pm-dev what about this approach:

1. Strict validation of resolver method names, resolver method arguments, and input types
2. Attempt to find (and cache) fields on graphql types based on the return type, and remove 1:1 restriction on class -> graphql type.
3. If a field can't be found on a return type, either throw an error or allow it based on a user-defined "relaxed type checking" option
4. In the data fetcher:
   1. Look up the field on the associated resolver
   2. Look up the field in the cache
   3. If the "relaxed type checking option" is given, attempt to find the field on whatever object is given to the datafetcher and cache the relationship

I think that would satisfy both of your requirements, and it would make the library a little simpler to maintain and use.

Hi @apottere Sorry i didn't reply to your previous message, somehow i managed to miss it.

Yes this seems like a plausible middle ground. It will certainly work for my use case.

That approach sounds reasonable and would satisfy my use-case.

FYI this is the workaround I was using https://github.com/pm-dev/graphql-java-tools/commit/ac43f74e224f7017f6f17cc17cc2e657f9e7b1e9 It's un-ideal because it changes the getType method from O(1) to O(n) where n is the size of types and there's no caching. This approach still verified that the returned data class type is an instance of the class type found at startup, which I liked.

Hi @apottere any traction on this??

All my spare time has been going to subscriptions in graphql-java-servlet, but this is next on the list.

Hey @apottere friendly reminder. Enjoy the weekend!!

Andrew seems to be pretty tied up with other projects atm. Any of you guys up for creating a PR for this one?

Did this end up going anywhere?