Skip to content

Relax type checking #111

Open
Open

Description

@ccamel

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.

Activity

  1. oexza commented on Jan 18, 2018

    @oexza
  2. ccamel commented on Jan 18, 2018

    @ccamel
    Author

    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.

  3. apottere commented on Jan 18, 2018

    @apottere
    Collaborator

    @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.
  4. ccamel commented on Jan 23, 2018

    @ccamel
    Author

    @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.

  5. pm-dev commented on Jan 24, 2018

    @pm-dev

    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.

  6. apottere commented on Feb 2, 2018

    @apottere
    Collaborator

    @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.

  7. oexza commented on Feb 2, 2018

    @oexza

    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.

  8. pm-dev commented on Feb 2, 2018

    @pm-dev

    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.

  9. oexza commented on Mar 21, 2018

    @oexza

    Hi @apottere any traction on this??

  10. apottere commented on Mar 21, 2018

    @apottere
    Collaborator

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

  11. oexza commented on May 5, 2018

    @oexza

    Hey @apottere friendly reminder. Enjoy the weekend!!

  12. oliemansm commented on Sep 22, 2018

    @oliemansm
    Member

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

  13. micheal-hill commented on Jan 13, 2020

    @micheal-hill

    Did this end up going anywhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions