Skip to content

Remove legacy data loader registry lookup from execution context #357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 9, 2023
Merged

Remove legacy data loader registry lookup from execution context #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4fbd31f
fix batch loader registry lookup to use fallback from environment
igdianov Mar 9, 2023
ee658a3
remove legacy dataLoaderRegistry lookup from execution context
igdianov Mar 9, 2023
0e9b020
fix missing import
igdianov Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions ...res/graphql/jpa/query/autoconfigure/GraphQLJpaQueryGraphQlExecutionAutoConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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})
Expand All @@ -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))));
};
}
}
32 changes: 17 additions & 15 deletions ...n/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLSchemaFactoryBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 1 addition & 3 deletions ...query/boot/test/boot/autoconfigure/GraphQLJpaQueryGraphQlSourceAutoConfigurationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
18 changes: 6 additions & 12 deletions ...in/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaExecutorContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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)
Expand All @@ -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() {
Expand Down
4 changes: 1 addition & 3 deletions .../java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToManyDataFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
18 changes: 6 additions & 12 deletions ...n/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaToOneDataFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
Expand Down