Help with Default Generic Wrappers

[bentatham]

I am trying to use graphql-java-tools to wrap my vertx/reactivex with jooq application. For now, I'm trying to just use the default generic wrappers, so I convert all my reactivex Single's to Future, so in theory graphql-java-tools should be happy. But I still get:

Expected source object to be an instance of 'ca.nanometrics.halo.graphql.schema.Organization' but instead got 'io.reactivex.internal.observers.FutureSingleObserver' com.coxautodev.graphql.tools.ResolverError: Expected source object to be an instance of 'ca.nanometrics.halo.graphql.schema.Organization' but instead got 'io.reactivex.internal.observers.FutureSingleObserver'
(where FutureSingleObserver is a subclass of java.util.concurrent.Future)

My query class is defined as:

public class QueryResolver implements GraphQLQueryResolver
{
  private final Users m_users;
  private final Assets m_assets;

  @Inject
  QueryResolver(Users users, Assets assets)
  {
    m_users = users;
    m_assets = assets;
  }

  public Future<Organization> organization(int organizationId)
  {
    return m_users.getOrganizationById(organizationId).onErrorReturn(e -> {
      LOG.error("Error getting organization", e);
      return Optional.empty();
    }).map(optional -> optional.map(org -> new Organization(org, m_users, m_assets)).orElse(null))//
        .toFuture();
  }

Obviously, I need some better error handling in the code itself, but I'm just trying to make this work before moving forward.

My schema is created with the default generic wrappers:

    return com.coxautodev.graphql.tools.SchemaParser.newParser()//
        .file("halo-users.graphqls")//
        .dictionary(Organization.class) //
        .dictionary(Asset.class) //
        .dictionary(User.class) //
        .dictionary(AssetType.class) //
        .resolvers(//
            query //
        )//
        .options(SchemaParserOptions.newOptions()//
            .useDefaultGenericWrappers(true) //
            .build())//
        .build().makeExecutableSchema();

I can't find any good documentation on how I am supposed to do this, so perhaps I am missing something obvious? Based on #103 , it sounds like this should work. (Based on that issue, I should probably be able to get Single to work as well, although there seems to be no guidance on actually implementing the custom resolvers, but I'm happy to start with Future.)

[oliemansm]

@bentatham
Documentation needs to be improved. We've just setup a new documentation home to be able to start improving that, but it'll take time.

Are you using the latest version? I just created a small unit test myself to try to reproduce, since using futures should just work. The following worked for me. Note that you don't have to add all those classes to the dictionary. You only have to add the ones that can't be automatically found by the scan. If you've provided your starting point, the Query in this case, it will try to find all the necessary classes from there.

In this test case I've added those classes as static inner classes, but that's only for this test case.

public class ReactiveTest {

    @Test
    public void futureSucceeds() {
        GraphQLSchema schema = SchemaParser.newParser().file("Reactive.graphqls")
                .resolvers(new Query())
                .build()
                .makeExecutableSchema();

        GraphQL gql = GraphQL.newGraphQL(schema)
                .queryExecutionStrategy(new AsyncExecutionStrategy())
                .build();
        Utils.assertNoGraphQlErrors(gql, new HashMap<>(), new Object(), new Closure<String>(null) {
            @Override
            public String call() {
                return "query { organization(organizationId: 1) { user { id } } }";
            }
        });
    }

    static class Query implements GraphQLQueryResolver {
        Future<Organization> organization(int organizationid) {
            return null;
        }
    }

    static class Organization {
        private User user;
    }

    static class User {
        private Long id;
        private String name;
    }
}

The SDL used for this example.

type Query {
    organization(organizationId: ID): Organization
}

type Organization {
    user: User
}

type User {
    id: ID
    name: String
}

[bentatham]

Thanks so much for the detailed response.

Yes, I'm using com.graphql-java-kickstart:graphql-java-tools:5.3.5, which seems to be the latest in maven central (after a couple of groupId moves in the past).

If I'm doing something different, I can't figure out what it is yet. I'll keep debugging...

On that note, is the graphql-java-tools test jar available in maven central, or perhaps in another repository? I can't seem to get it. Having access to the Utils class would be great! Perhaps I'll create a separate issue and PR for that.

[bentatham]

Still no luck. I've trimmed down my schema to have just the QueryResolver, and rely on the fact the useDefaultGenericWrapper is true by default. I've checked out the project, so I can use your Utils class to test it myself, and am getting the same thing.

For a "query { organization(organizationId: 20) { organizationName } }", I still get an error, for what looks like getting the organization.organizationName field. Something to do with the MethodFieldResolverDataFetcher not expecting the Future.

018-11-13 11:32:31,455 D [main] graphql.GraphQL                  Executing 'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63'. operation name: 'null'. query: 'query { organization(organizationId: 20) { organizationName } }'. variables '{}'
2018-11-13 11:32:31,464 D [main] graphql.execution.Execution      Executing 'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63' query operation: 'QUERY' using 'graphql.execution.AsyncExecutionStrategy' execution strategy
2018-11-13 11:32:31,473 D [main] g.execution.ExecutionStrategy    'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63' fetching field '/organization' using data fetcher 'com.coxautodev.graphql.tools.MethodFieldResolverDataFetcher'...
2018-11-13 11:32:31,552 D [main] g.execution.ExecutionStrategy    'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63' field '/organization' fetch returned 'io.reactivex.internal.observers.FutureSingleObserver'
2018-11-13 11:32:31,557 D [main] g.execution.ExecutionStrategy    'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63' completing field '/organization'...
2018-11-13 11:32:31,559 D [main] g.execution.ExecutionStrategy    'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63' fetching field '/organization/organizationName' using data fetcher 'com.coxautodev.graphql.tools.MethodFieldResolverDataFetcher'...
2018-11-13 11:32:31,562 D [main] g.execution.ExecutionStrategy    'a30e96a1-d5c1-4aa5-a70a-7adc75f37c63', field '/organization/organizationName' fetch threw exception
com.coxautodev.graphql.tools.ResolverError: Expected source object to be an instance of 'ca.nanometrics.halo.graphql.schema.Organization' but instead got 'io.reactivex.internal.observers.FutureSingleObserver'
	at com.coxautodev.graphql.tools.FieldResolver$getSourceResolver$2.invoke(FieldResolver.kt:27)

In QueryResolver (this maps the jooq dao into a simple object for graphql/jackson, behind a future):

  public Future<Organization> organization(int organizationId)
  {
    return m_users.getOrganizationById(organizationId).onErrorReturn(e -> {
      LOG.error("Error getting organization", e);
      return Optional.empty();
    }).map(optional -> optional.map(org -> new Organization(org, m_users, m_assets)).orElse(null))//
        .toFuture();
  }

From Organization:

  public String getOrganizationName()
  {
    return m_pojo.getOrganizationName();
  }

[oliemansm]

Are you able to share this example in a small repo by any chance? Might be a bit easier and quicker if I can take a look at it that way to see what's going on.

[bentatham]

Thanks for offering...I've extracted some code into this project:
https://github.com/bentatham/graphql-vertx-jooq-example

There is a GraphQLModuleTest that you can run (junit) and will show the error I'm getting.

[bentatham]

In your example in the initial response, it "works" because the object (Future) is null, so the error does not get hit. If I return null for the Future, it also passes the tests, but in reality, if you return an actual object that is a Future, it does not work, as per the exception shown above.

[oliemansm]

I'm getting a compiler error due to missing package:

<dependency>
      <groupId>com.github.tibor-kocsis</groupId>
      <artifactId>vertx-graphql-utils</artifactId>
      <version>${vertx-graphql-utils.version}</version>
    </dependency>

But perhaps it's not necessary if it can be reproduced by returning an actual Future in the existing unit test.

[oliemansm]

Using Future with an actual return value works as well in my case:

Future<Organization> organization(int organizationid) {
    return CompletableFuture.completedFuture(new Organization());
}

I see your using Single.just which is causing the problem. Those aren't included in the default generic wrappers. When I simply try to add them by customizing the schema parser options it won't work yet either, so that requires some more debugging to figure out what's going on.

So I've tried with this config but without success unfortunately:

SchemaParserOptions options = SchemaParserOptions.newOptions()
        .genericWrappers(
                new SchemaParserOptions.GenericWrapper(Single.class, 0),
                new SchemaParserOptions.GenericWrapper(SingleJust.class, 0)
        )
        .build();
GraphQLSchema schema = SchemaParser.newParser().file("Reactive.graphqls")
        .resolvers(new Query())
        .options(options)
        .build()
        .makeExecutableSchema();

[bentatham]

Perhaps it is hard to see from the code, but QueryResolver, it is calling toFuture on the SingleJust, so it is actually returning a Future (FutureSingleObserver to be precise, as per the error message) to the graphql-java-tools code.

[bentatham]

I've updated the project to remove the dependency (which is in jcenter, not maven central)..

[oliemansm]

Thanks I can reproduce it now. Requires some debugging unfortunately to figure out why it fails in this case.

Btw, I see you're explicitly removing unwrapping te Optional to replace null if not present. You don't necessarily have to do that though. You could just have it return Future<Optional<Organization>>.

Right now you're passing users and assets down into your model. Not sure if that's your preferred approach or not. Know that you could also instead define resolvers implementing GraphQLResolver<Organization> for example. That could contain methods for fetching more specific data (lists or something like that).

[tfadsilva]

I'm having a similar issue in my application. I have the follow generic wrappers being passed onto my schema parser options:

val schemaParserOptions = SchemaParserOptions.newOptions()
            .genericWrappers(
                SchemaParserOptions.GenericWrapper.withTransformer(
                    Single::class.java,
                    0,
                    { single, _ -> single.toFuture() }),
                SchemaParserOptions.GenericWrapper.listCollectionWithTransformer(
                    Flowable::class.java,
                    0
                ) { flowable -> flowable.toList().toFuture() }
            )
            .build()

And have the following query resolver method:

fun books(): Flowable<Book> {
   return transaction {
            BookDAO().selectAll()
        }
    }

Where the BookDAO().selectAll() returns a Flowable<Book>

The error I get is the following:

graphql.execution.ExecutionStrategy - Can't resolve value (/books) : type mismatch error, expected type LIST got class io.reactivex.internal.observers.FutureSingleObserver

Since it complains about FutureSingleObserver, which is what the Flowable.toList().toFuture() returns, and not about the Flowable itself, I assume the wrapper is working as expected. I just don't understand why it's not using the Future default generic wrapper already specified in the schema parser to further unwrap the value. Can someone help me?