Remove legacy data loader registry lookup from execution context (#357)

igdianov · web-flow · commit 97fafa972d0e · 2023-03-09T14:01:02.000-08:00
* fix batch loader registry lookup to use fallback from environment

* remove legacy dataLoaderRegistry lookup from execution context

* fix missing import
diff --git a/graphql-jpa-query-autoconfigure/src/main/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLJpaQueryGraphQlExecutionAutoConfiguration.java b/graphql-jpa-query-autoconfigure/src/main/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLJpaQueryGraphQlExecutionAutoConfiguration.java
@@ -1,9 +1,6 @@
 package com.introproventures.graphql.jpa.query.autoconfigure;
 
 import graphql.GraphQL;
-import org.dataloader.DataLoaderOptions;
-import org.dataloader.MappedBatchLoaderWithContext;
-import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.ListableBeanFactory;
 import org.springframework.boot.autoconfigure.AutoConfiguration;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
@@ -13,9 +10,6 @@
 import org.springframework.graphql.ExecutionGraphQlService;
 import org.springframework.graphql.execution.BatchLoaderRegistry;
 import org.springframework.graphql.execution.GraphQlSource;
-import reactor.core.publisher.Mono;
-
-import static com.introproventures.graphql.jpa.query.schema.impl.BatchLoaderRegistry.newDataLoaderRegistry;
 
 @AutoConfiguration(after = GraphQlAutoConfiguration.class)
 @ConditionalOnClass({GraphQL.class, GraphQlSource.class})
@@ -36,19 +30,4 @@ ExecutionGraphQlService executionGraphQlService(ListableBeanFactory beanFactory,
                                                                                  batchLoaderRegistry);
     }
 
-    @Bean
-    InitializingBean batchLoaderRegistryConfigurer(BatchLoaderRegistry batchLoaderRegistry) {
-        return () -> {
-            DataLoaderOptions options = DataLoaderOptions.newOptions()
-                                                         .setCachingEnabled(false);
-            newDataLoaderRegistry(options)
-                               .getDataLoadersMap()
-                               .entrySet()
-                               .stream()
-                               .forEach(entry -> batchLoaderRegistry.forName(entry.getKey())
-                                                                    .withOptions(options)
-                                                                    .registerMappedBatchLoader((keys, env) ->
-                                                                           Mono.fromCompletionStage(((MappedBatchLoaderWithContext) entry.getValue()).load(keys, env))));
-        };
-    }
 }
diff --git a/graphql-jpa-query-autoconfigure/src/main/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLSchemaFactoryBean.java b/graphql-jpa-query-autoconfigure/src/main/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLSchemaFactoryBean.java
@@ -1,5 +1,17 @@
 package com.introproventures.graphql.jpa.query.autoconfigure;
 
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.Function;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import graphql.Internal;
 import graphql.schema.DataFetcher;
 import graphql.schema.FieldCoordinates;
@@ -20,19 +32,7 @@
 import graphql.util.TraversalControl;
 import graphql.util.TraverserContext;
 import org.springframework.beans.factory.config.AbstractFactoryBean;
-
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Function;
-import java.util.function.Predicate;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
+import org.springframework.util.Assert;
 
 import static graphql.Assert.assertTrue;
 import static graphql.schema.FieldCoordinates.coordinates;
@@ -62,12 +62,14 @@ public class GraphQLSchemaFactoryBean extends AbstractFactoryBean<GraphQLSchema>
     
     
     public GraphQLSchemaFactoryBean(GraphQLSchema[] managedGraphQLSchemas) {
+        Assert.notEmpty(managedGraphQLSchemas, "Managed GraphQLSchema registrations can't be empty.");
+
         this.managedGraphQLSchemas = managedGraphQLSchemas;
     }
 
     @Override
-    protected GraphQLSchema createInstance() throws Exception {
-        
+    protected GraphQLSchema createInstance() {
+
         GraphQLSchema.Builder schemaBuilder = GraphQLSchema.newSchema();
         GraphQLCodeRegistry.Builder codeRegistryBuilder = GraphQLCodeRegistry.newCodeRegistry();
         TypeTraverser typeTraverser = new TypeTraverser();
diff --git a/graphql-jpa-query-boot-starter-graphql/src/test/java/com/introproventures/graphql/jpa/query/boot/test/boot/autoconfigure/GraphQLJpaQueryGraphQlSourceAutoConfigurationTest.java b/graphql-jpa-query-boot-starter-graphql/src/test/java/com/introproventures/graphql/jpa/query/boot/test/boot/autoconfigure/GraphQLJpaQueryGraphQlSourceAutoConfigurationTest.java
@@ -84,8 +84,6 @@ public void contextIsAutoConfigured() {
         DataLoaderRegistry dataLoaderRegistry = newRegistry().build();
         batchLoaderRegistry.registerDataLoaders(dataLoaderRegistry, newContext().build());
 
-        assertThat(dataLoaderRegistry.getDataLoadersMap())
-                                     .isNotEmpty()
-                                     .containsOnlyKeys("Author.books", "Book.author");
+        assertThat(dataLoaderRegistry.getDataLoadersMap()).isEmpty();
     }
 }
diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaExecutorContext.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaExecutorContext.java
@@ -19,11 +19,6 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.function.Supplier;
-
-import org.dataloader.DataLoaderRegistry;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import com.introproventures.graphql.jpa.query.schema.GraphQLExecutionInputFactory;
 import com.introproventures.graphql.jpa.query.schema.GraphQLExecutorContext;
 import graphql.ExecutionInput;
@@ -37,6 +32,9 @@
 import graphql.schema.GraphQLCodeRegistry;
 import graphql.schema.GraphQLSchema;
 import graphql.schema.visibility.GraphqlFieldVisibility;
+import org.dataloader.DataLoaderRegistry;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class GraphQLJpaExecutorContext implements GraphQLExecutorContext {
 
@@ -69,7 +67,7 @@ private GraphQLJpaExecutorContext(Builder builder) {
     @Override
     public ExecutionInput.Builder newExecutionInput() {
         DataLoaderRegistry dataLoaderRegistry = newDataLoaderRegistry();
-        GraphQLContext context = newGraphQLContext(dataLoaderRegistry);
+        GraphQLContext context = newGraphQLContext();
 
         return executionInputFactory.create()
                                     .dataLoaderRegistry(dataLoaderRegistry)
@@ -87,12 +85,8 @@ public GraphQL.Builder newGraphQL() {
                       .subscriptionExecutionStrategy(subscriptionExecutionStrategy.get());
     }
 
-    public GraphQLContext newGraphQLContext(DataLoaderRegistry dataLoaderRegistry) {
-        GraphQLContext context = graphqlContext.get();
-
-        context.put("dataLoaderRegistry", dataLoaderRegistry);
-
-        return context;
+    public GraphQLContext newGraphQLContext() {
+        return graphqlContext.get();
     }
 
     public DataLoaderRegistry newDataLoaderRegistry() {
diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToManyDataFetcher.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToManyDataFetcher.java
@@ -28,7 +28,6 @@
 import org.dataloader.MappedBatchLoaderWithContext;
 
 import com.introproventures.graphql.jpa.query.support.GraphQLSupport;
-import graphql.GraphQLContext;
 import graphql.language.Argument;
 import graphql.language.Field;
 import graphql.schema.DataFetcher;
@@ -79,8 +78,7 @@ public Object get(DataFetchingEnvironment environment) {
 
     protected DataLoader<Object, List<Object>> getDataLoader(DataFetchingEnvironment environment,
                                                              String dataLoaderKey) {
-        GraphQLContext context = environment.getContext();
-        DataLoaderRegistry dataLoaderRegistry = context.get("dataLoaderRegistry");
+        DataLoaderRegistry dataLoaderRegistry = environment.getDataLoaderRegistry();
 
         if (!dataLoaderRegistry.getKeys()
                               .contains(dataLoaderKey)) {
diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToOneDataFetcher.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToOneDataFetcher.java
@@ -16,24 +16,19 @@
 
 package com.introproventures.graphql.jpa.query.schema.impl;
 
-import static javax.persistence.metamodel.Attribute.PersistentAttributeType.EMBEDDED;
-
 import java.util.Optional;
-
 import javax.persistence.metamodel.SingularAttribute;
-
+import graphql.language.Argument;
+import graphql.language.Field;
+import graphql.schema.DataFetcher;
+import graphql.schema.DataFetchingEnvironment;
 import graphql.schema.GraphQLNamedType;
 import org.dataloader.DataLoader;
 import org.dataloader.DataLoaderOptions;
 import org.dataloader.DataLoaderRegistry;
 import org.dataloader.MappedBatchLoaderWithContext;
 
-import graphql.GraphQLContext;
-import graphql.language.Argument;
-import graphql.language.Field;
-import graphql.schema.DataFetcher;
-import graphql.schema.DataFetchingEnvironment;
-import graphql.schema.GraphQLType;
+import static javax.persistence.metamodel.Attribute.PersistentAttributeType.EMBEDDED;
 
 /**
  * One-To-Many DataFetcher that uses where argument to filter collection attributes
@@ -81,8 +76,7 @@ public Object get(DataFetchingEnvironment environment) {
 
     protected DataLoader<Object, Object> getDataLoader(DataFetchingEnvironment environment,
                                                        String dataLoaderKey) {
-        GraphQLContext context = environment.getContext();
-        DataLoaderRegistry dataLoaderRegistry = context.get("dataLoaderRegistry");
+        DataLoaderRegistry dataLoaderRegistry = environment.getDataLoaderRegistry();
 
         if (!dataLoaderRegistry.getKeys()
                                .contains(dataLoaderKey)) {

Original file line number	Diff line number	Diff line change
`@@ -84,8 +84,6 @@ public void contextIsAutoConfigured() {`
`84`	`84`	`DataLoaderRegistry dataLoaderRegistry = newRegistry().build();`
`85`	`85`	`batchLoaderRegistry.registerDataLoaders(dataLoaderRegistry, newContext().build());`
`86`	`86`
`87`		`- assertThat(dataLoaderRegistry.getDataLoadersMap())`
`88`		`- .isNotEmpty()`
`89`		`- .containsOnlyKeys("Author.books", "Book.author");`
	`87`	`+ assertThat(dataLoaderRegistry.getDataLoadersMap()).isEmpty();`
`90`	`88`	`}`
`91`	`89`	`}`