Git Product home page Git Product logo

Comments (16)

siderakis avatar siderakis commented on April 28, 2024

Sorry for the delay.

The simplest thing you could do is move the schema creation code into the handler, so you can access the request directly. The downside of this approach is the schema would be recreated on each request.

GraphQLSchema SCHEMA =
          Guice.createInjector(
              new SchemaProviderModule(),
              new BookClientModule(),
              new BookSchemaModule(),
              new ShelfClientModule(),
              new ShelfSchemaModule(),
              new LibrarySchemaModule(),
              new SeedLibrarySchemaModule(),
              new AbstractModule(){
                @Override
                protected void configure() {
                  //TODO: Use cookie or ouath token from request to get the users id.
                  // For now the userId is their IP address
                  String userId = httpServletRequest.getRemoteHost();
                  bind(String.class).annotatedWith(Annotations.AuthenticatedUserId.class).toInstance(userId);
                }
              }
          )
              .getInstance(Key.get(GraphQLSchema.class, Schema.class));

With that schema you can use a parameter such as @Annotations.AuthenticatedUserId String userId in a Query definition.

I'm working on a more complete & production ready example that includes authentication and uses Guice servlet extension.

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

@siderakis Thank you so much for this. I was looking to lean into the Guice servlet extension as well so its good to know that I'm on the right path. I suspect we may need something similar for managing the dispatch for DataFetchers.

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

I think I have a version sussed out using Guice Servlet. Does this setup make sense? This is +/- the boilerplate in the examples/ repo but reworked as a Servlet. I haven't leaned in yet at all, but this should preserve the schema across requests and allow me to start injecting request based instances where needed (Dataloaders, Auth'd users, etc...)

public class RQLServer {

  private static final int HTTP_PORT = 8080;
  private static final Logger logger = Logger.getLogger(Server.class.getName());

  public static void main(String[] args) throws Exception {
    Server server = new Server(HTTP_PORT);

    ServletContextHandler context = new ServletContextHandler(
        server,
        "/",
        ServletContextHandler.SESSIONS
    );

    context.addEventListener(new GuiceServletContextListener() {
      @Override
      protected Injector getInjector() {
        return Guice.createInjector(
            new SchemaProviderModule(),
            new RQLModule(),
            new RQLSchema(),
            new ServletModule() {
              @Override
              protected void configureServlets() {
                serve("/graphql").with(RQLHandler.class);
              }
            });
      }
    });

    context.addFilter(
        GuiceFilter.class,
        "/*",
        EnumSet.of(javax.servlet.DispatcherType.REQUEST, javax.servlet.DispatcherType.ASYNC)
    );

    context.addServlet(DefaultServlet.class, "/");

    try {
      logger.info("Server running on port " + HTTP_PORT);
      server.start();
      server.join();
    } catch (Exception e) {
      logger.info("error" + e.getMessage());
    } finally {
      server.destroy();
    }
  }
}
public class RQLModule extends AbstractModule {

  @Override
  protected void configure() {
  // ...
  }

  @Provides
  GraphQL getGraphQL(Injector injector) {
    GraphQLSchema schema = injector.getInstance(Key.get(GraphQLSchema.class, Schema.class));

    Instrumentation INSTRUMENTATION =
        new ChainedInstrumentation(
            Arrays.asList(
                GuavaListenableFutureSupport.listenableFutureInstrumentation(),
                new TracingInstrumentation()));

    return GraphQL.newGraphQL(schema).instrumentation(INSTRUMENTATION).build();
  }
}
@Singleton
public class RQLHandler extends HttpServlet {

  private static final TypeToken<Map<String, Object>> MAP_TYPE_TOKEN =
      new TypeToken<Map<String, Object>>() {
      };
  private static final Gson GSON = new GsonBuilder().serializeNulls().create();

  @Inject
  GraphQL graphQL;

  @Override
  protected void doPost(HttpServletRequest req, HttpServletResponse resp)
      throws ServletException, IOException {

    Map<String, Object> json = readJson(req);
    String query = (String) json.get("query");
    if (query == null) {
      resp.setStatus(400);
      return;
    }
    String operationName = (String) json.get("operationName");
    Map<String, Object> variables = getVariables(json.get("variables"));

    ExecutionInput executionInput =
        ExecutionInput.newExecutionInput()
            .query(query)
            .operationName(operationName)
            .variables(variables)
            .context(new Object())
            .build();

    ExecutionResult executionResult = graphQL.execute(executionInput);
    resp.setContentType("application/json");
    resp.setStatus(HttpServletResponse.SC_OK);

    GSON.toJson(executionResult.toSpecification(), resp.getWriter());
  }

  private static Map<String, Object> getVariables(Object variables) {
    Map<String, Object> variablesWithStringKey = new HashMap<>();
    if (variables instanceof Map) {
      ((Map) variables).forEach((k, v) -> variablesWithStringKey.put(String.valueOf(k), v));
    }
    return variablesWithStringKey;
  }

  private static Map<String, Object> readJson(HttpServletRequest request) {
    try {
      String json = CharStreams.toString(request.getReader());
      return jsonToMap(json);
    } catch (IOException e) {
      throw new RuntimeException(e);
    }
  }

  private static Map<String, Object> jsonToMap(String json) {
    if (Strings.isNullOrEmpty(json)) {
      return ImmutableMap.of();
    }
    return Optional.<Map<String, Object>>ofNullable(GSON.fromJson(json, MAP_TYPE_TOKEN.getType()))
        .orElse(ImmutableMap.of());
  }
}

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

This looks great! I need to take a closer look but one small suggestion might be to inject the GraphQL schema directly rather than injecting the Injector in RQLModule.

Something like:


  @Override
  protected void configure() {
  // ...
  }

  @Provides
  GraphQL getGraphQL(@Schema  GraphQLSchema schema) { ...}

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

Thats a much better call. Thank you so much for your guidance. Rejoiner has been a joy to use so far! Let me know if you'd like me to wrap that up in an example. I could probably change the library example to do this instead?

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

@siderakis This got me pretty far until I tried using something request scoped in a SchemaModule.

// AbstractModule
  @Provides
  @RequestScoped
  DataLoaderRegistry getLoaders(CoreClient client) {
    DataLoaderRegistry registry = new DataLoaderRegistry();

    BatchLoader<String, Book> batchBookLoader = ids -> {
      return FutureConverter.toCompletableFuture(client.getBooks(ids));
    };

    registry.register("books", new DataLoader<>(batchBookLoader));

    return registry;
  }
// Handler
  @Inject
  Provider<DataLoaderRegistry> registryProvider;

  @Override
  protected void doPost(HttpServletRequest req, HttpServletResponse resp)
      throws IOException {

    DataLoaderRegistry registry = registryProvider.get();

    Map<String, Object> json = readJson(req);
    String query = (String) json.get("query");
    if (query == null) {
      resp.setStatus(400);
      return;
    }
    String operationName = (String) json.get("operationName");
    Map<String, Object> variables = getVariables(json.get("variables"));

    Instrumentation INSTRUMENTATION =
        new ChainedInstrumentation(
            Arrays.asList(
                GuavaListenableFutureSupport.listenableFutureInstrumentation(),
                new DataLoaderDispatcherInstrumentation(registry),
                new TracingInstrumentation()
            )
        );

    GraphQL graphQL = builder.instrumentation(INSTRUMENTATION).build();

    ExecutionInput executionInput =
        ExecutionInput.newExecutionInput()
            .query(query)
            .operationName(operationName)
            .variables(variables)
            .build();

    ExecutionResult executionResult = graphQL.execute(executionInput);
    resp.setContentType("application/json");
    resp.setStatus(HttpServletResponse.SC_OK);

    GSON.toJson(executionResult.toSpecification(), resp.getWriter());
  }
// SchemaModule
  @SchemaModification(addField = "book", onType = Shelf.class)
  ListenableFuture<Book> addBooksToShel(DataLoaderRegistry registry,
      Shelf shelf) {
    DataLoader<String, Book> loader = registry.getDataLoader("books");
    CompletableFuture<Listing> res = loader
        .load(String.valueOf(shelf.getBookIds()));

    return FutureConverter.toListenableFuture(res);
  }

This wires fine, the server starts up, but then Guice complains that I'm not in a RequestContext when attempting to inject the DataLoaderRegistry in that @SchemaModification

"Exception while fetching data (/shelf[0]/books) : Unable to provision, see the following errors:\n\n1) Error in custom provider, com.google.inject.OutOfScopeException: Cannot access scoped [org.dataloader.DataLoaderRegistry]. Either we are not currently inside an HTTP Servlet request, or you may have forgotten to apply com.google.inject.servlet.GuiceFilter as a servlet filter for this request.\n at com.reverb.rql.RQLModule.getLoaders(RQLModule.java:55)\n while locating org.dataloader.DataLoaderRegistry\n\n1 error"

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

I figured out the issue here. The root of my query returned a Future, which was resolved by a multi-threaded HTTP client (asyncHttp). When it finally did resolved, it invokes the schema modification handler, but at that point I've lost my request context and Guice refuses to supply my request scoped DataloaderRegistry to the schema modification call.

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

Interesting, where does builder come from in doPost?

Referenced in GraphQL graphQL = builder.instrumentation(INSTRUMENTATION).build();

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

The builder is injected via a Provider

// AbstractModule
  @Provides
  @Singleton
  Builder getGraphQL(@Schema GraphQLSchema schema) {
    return GraphQL.newGraphQL(schema);
  }
// Handler
  @Inject
  GraphQL.Builder builder;

  @Inject
  Provider<DataLoaderRegistry> registryProvider;

  @Override
  protected void doPost(HttpServletRequest req, HttpServletResponse resp)
      throws IOException {
// ...

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

Turns out this isn't the async client necessarily, but any time I jump threads (i.e. when returning a ListenableFuture). I lose the request context and Guice refuses to inject the request scoped DataLoaderRegistry.

This is very simple to reproduce. I suspect this is an issue with how I'm setting up the servlet and Guice, but I'm not sure how else to set this up. Any guidance here would be greatly appreciated and once I have all of these moving parts worked out I am happy to submit an example to this repo to help other folks out.

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

I was able to get past this issue, but unfortunately I'm now running into what I expect is a bug with the Dataloaders themselves. I believe its related to this issue: graphql-java/graphql-java#979 .

For posterity, I was able to transfer the servlet context, which got the DI working again, but exposed this issue with the Dataloader.

public class RQLDataLoaderProvider implements Provider<DataLoaderRegistry> {

  @Inject
  CoreClient core;

  @Inject
  LPClient lp;

  public static <T> CompletableFuture<T> asFuture(Callable<? extends T> callable,
      Executor executor) {
    CompletableFuture<T> future = new CompletableFuture<>();
    executor.execute(() -> {
      try {
        future.complete(callable.call());
      } catch (Throwable t) {
        future.completeExceptionally(t);
      }
    });

    return future;
  }

  BatchLoader<String, Listing> batchListingsLoader = ids -> asFuture(
      ServletScopes.transferRequest(() -> core.getListings(ids).getListingsList()),
      MoreExecutors.directExecutor());

  BatchLoader<String, Release> batchReleasesLoader = ids -> asFuture(
      ServletScopes.transferRequest(() -> lp.getReleases(ids).getReleasesList()),
      MoreExecutors.directExecutor());

  public DataLoaderRegistry get() {
    DataLoaderRegistry registry = new DataLoaderRegistry();

    registry.register("listings", new DataLoader<>(batchListingsLoader));
    registry.register("releases", new DataLoader<>(batchReleasesLoader));

    return registry;
  }
}

This results in listings being resolved in batch, correctly. Unfortunately when the batch loader is invoked for releases, which joins to listings, the batch loader immediately executes, creating an N+1 problem.

  @SchemaModification(addField = "release", onType = Listing.class)
  ListenableFuture<Release> listingToRelease(DataLoaderRegistry registry, Listing listing) {
    DataLoader<String, Release> loader = registry.getDataLoader("releases");
    return FutureConverter.toListenableFuture(loader.load(listing.getMerchandisingUuid()));
  }

  @SchemaModification(addField = "listing", onType = FeedEntry.class)
  ListenableFuture<Listing> feedEntryToListing(DataLoaderRegistry registry,
      FeedEntry entry) {
    if (entry.getType() == EntryType.LISTING) {
      DataLoader<String, Listing> loader = registry.getDataLoader("listings");
      CompletableFuture<Listing> res = loader.load(String.valueOf(entry.getId()));
      return FutureConverter.toListenableFuture(res);
    }

    return null;
  }

  @SchemaModification(addField = "lpFeed", onType = AuthUser.class)
  ImmutableList<FeedEntry> userToLPFeedEntries(
      @LPFeed FeedServiceGrpc.FeedServiceBlockingStub client,
      AuthUser user) {
    EntriesRequest.Builder req = EntriesRequest.newBuilder();
    req.setUserId(user.getResourceOwnerId());

    FeedResponse resp = client.getFeedEntries(req.build());
    return ImmutableList.copyOf(resp.getEntriesList());
  }
{
  user(input: { token: "token"}) {
    lpFeed {
      listing {
        title
        release {
          title
        }
      }
    }
  }
}

This project is so close to what we need, but if I can't resolve this issue, I'll likely need to move on. Thanks again for your help and the fantastic project.

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

Where are you calling dataloader.dispatch() or dataloader.dispatchAndJoin()?

Can you confirm there is only one instance of DataLoaderRegistry per request?

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

I created a branch named dataloaders if you want to see what I'm trying

from rejoiner.

siderakis avatar siderakis commented on April 28, 2024

Please ignore the previous questions I figured it out.

I got the library example working with a data loader for simple queries but ended up getting the same com.google.inject.OutOfScopeException exception. I tired ServletScopes.transferRequest with no luck yet.

I'm going to try to pass the DataloadRegistry as part of the context when executing the query. This may allow us to avoid the need to use the request scope all together.

from rejoiner.

ebenoist avatar ebenoist commented on April 28, 2024

Got it- I took a look at your branch and its a similar approach that I took. I was able to work through the OutOfScopeException with transferRequest, but unfortunately I ended up running into some bugs in the underlying dataloader library that prevents joining two bulk requests. The second batch load is dispatched on every load request, creating an N+1 problem. I'm tracking that bug on the graphql-java repo.

I really like the approach here. I'm currently building out a protoc -> graphql schema generator to attempt to replicate some of this library with the apollo-server library, but I'd be eager to come back to this if the underlying issue gets worked out.

from rejoiner.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.