From 23bf1fd8f30a30396daec021ef1d978500eb1ea9 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 15 Apr 2024 09:12:09 +0100 Subject: [PATCH] Expand interface into object type mappings See gh-871 --- .../AnnotatedControllerConfigurer.java | 102 ++++++- .../support/DataFetcherMappingInfo.java | 22 ++ ...ultSchemaResourceGraphQlSourceBuilder.java | 9 +- .../execution/RuntimeWiringConfigurer.java | 10 + ...rollerConfigurerInterfaceMappingTests.java | 282 ++++++++++++++++++ 5 files changed, 409 insertions(+), 16 deletions(-) create mode 100644 spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurerInterfaceMappingTests.java diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java index 7cbfdb38..9ab4f54f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurer.java @@ -21,6 +21,7 @@ import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -34,11 +35,16 @@ import java.util.function.Predicate; import java.util.stream.Collectors; import graphql.execution.DataFetcherResult; +import graphql.language.ObjectTypeDefinition; +import graphql.language.Type; +import graphql.language.TypeDefinition; +import graphql.language.TypeName; import graphql.schema.DataFetcher; import graphql.schema.DataFetchingEnvironment; import graphql.schema.FieldCoordinates; import graphql.schema.GraphQLCodeRegistry; import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.TypeDefinitionRegistry; import org.dataloader.DataLoader; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; @@ -72,6 +78,8 @@ import org.springframework.lang.Nullable; import org.springframework.stereotype.Controller; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; import org.springframework.validation.DataBinder; @@ -118,6 +126,8 @@ public class AnnotatedControllerConfigurer private final List customArgumentResolvers = new ArrayList<>(8); + private final InterfaceMappingHelper interfaceMappingHelper = new InterfaceMappingHelper(); + @Nullable private ValidationHelper validationHelper; @@ -133,6 +143,11 @@ public class AnnotatedControllerConfigurer this.customArgumentResolvers.add(resolver); } + @Override + public void setTypeDefinitionRegistry(TypeDefinitionRegistry registry) { + this.interfaceMappingHelper.setTypeDefinitionRegistry(registry); + } + /** * Configure an initializer that configures the {@link DataBinder} before the binding process. * @param consumer the data binder initializer @@ -228,19 +243,16 @@ public class AnnotatedControllerConfigurer @Override public void configure(RuntimeWiring.Builder runtimeWiringBuilder) { - detectHandlerMethods().forEach((info) -> { - DataFetcher dataFetcher; - if (!info.isBatchMapping()) { - dataFetcher = new SchemaMappingDataFetcher( - info, getArgumentResolvers(), this.validationHelper, getExceptionResolver(), getExecutor()); - } - else { - dataFetcher = registerBatchLoader(info); - } - FieldCoordinates coordinates = info.getCoordinates(); - runtimeWiringBuilder.type(coordinates.getTypeName(), (typeBuilder) -> - typeBuilder.dataFetcher(coordinates.getFieldName(), dataFetcher)); - }); + + Set allInfos = detectHandlerMethods(); + Set subTypeInfos = this.interfaceMappingHelper.removeInterfaceMappings(allInfos); + + allInfos.forEach((info) -> registerDataFetcher(info, runtimeWiringBuilder)); + + RuntimeWiring wiring = runtimeWiringBuilder.build(); + subTypeInfos = this.interfaceMappingHelper.filterExistingMappings(subTypeInfos, wiring.getDataFetchers()); + + subTypeInfos.forEach((info) -> registerDataFetcher(info, runtimeWiringBuilder)); } @Override @@ -313,6 +325,20 @@ public class AnnotatedControllerConfigurer return mappingInfo.getHandlerMethod(); } + private void registerDataFetcher(DataFetcherMappingInfo info, RuntimeWiring.Builder runtimeWiringBuilder) { + DataFetcher dataFetcher; + if (!info.isBatchMapping()) { + dataFetcher = new SchemaMappingDataFetcher( + info, getArgumentResolvers(), this.validationHelper, getExceptionResolver(), getExecutor()); + } + else { + dataFetcher = registerBatchLoader(info); + } + FieldCoordinates coordinates = info.getCoordinates(); + runtimeWiringBuilder.type(coordinates.getTypeName(), (typeBuilder) -> + typeBuilder.dataFetcher(coordinates.getFieldName(), dataFetcher)); + } + private DataFetcher registerBatchLoader(DataFetcherMappingInfo info) { if (!info.isBatchMapping()) { throw new IllegalArgumentException("Not a @BatchMapping method: " + info); @@ -506,6 +532,9 @@ public class AnnotatedControllerConfigurer } + /** + * {@link DataFetcher} that uses a DataLoader. + */ static class BatchMappingDataFetcher implements DataFetcher, SelfDescribingDataFetcher { private final DataFetcherMappingInfo mappingInfo; @@ -538,4 +567,51 @@ public class AnnotatedControllerConfigurer } } + + /** + * Helper to expand schema interface mappings into object type mappings. + */ + private static final class InterfaceMappingHelper { + + private final MultiValueMap interfaceMappings = new LinkedMultiValueMap<>(); + + void setTypeDefinitionRegistry(TypeDefinitionRegistry registry) { + for (TypeDefinition definition : registry.types().values()) { + if (definition instanceof ObjectTypeDefinition objectDefinition) { + for (Type type : objectDefinition.getImplements()) { + this.interfaceMappings.add(((TypeName) type).getName(), objectDefinition.getName()); + } + } + } + } + + Set removeInterfaceMappings(Set infos) { + Set subTypeMappings = new LinkedHashSet<>(); + Iterator it = infos.iterator(); + while (it.hasNext()) { + DataFetcherMappingInfo info = it.next(); + List names = this.interfaceMappings.get(info.getTypeName()); + if (names != null) { + for (String name : names) { + subTypeMappings.add(new DataFetcherMappingInfo(name, info)); + } + it.remove(); + } + } + return subTypeMappings; + } + + @SuppressWarnings("rawtypes") + Set filterExistingMappings( + Set infos, Map> dataFetchers) { + + return infos.stream() + .filter((info) -> { + Map registrations = dataFetchers.get(info.getTypeName()); + return (registrations == null || !registrations.containsKey(info.getFieldName())); + }) + .collect(Collectors.toSet()); + } + } + } diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherMappingInfo.java b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherMappingInfo.java index c281d04f..7b386f55 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherMappingInfo.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/method/annotation/support/DataFetcherMappingInfo.java @@ -52,6 +52,13 @@ public final class DataFetcherMappingInfo { this.handlerMethod = handlerMethod; } + public DataFetcherMappingInfo(String typeName, DataFetcherMappingInfo info) { + this.coordinates = FieldCoordinates.coordinates(typeName, info.getCoordinates().getFieldName()); + this.batchMapping = info.batchMapping; + this.maxBatchSize = info.maxBatchSize; + this.handlerMethod = info.handlerMethod; + } + /** * The field to bind the controller method to. @@ -60,6 +67,21 @@ public final class DataFetcherMappingInfo { return this.coordinates; } + /** + * Shortcut for the typeName from the coordinates. + */ + public String getTypeName() { + return this.coordinates.getTypeName(); + } + + + /** + * Shortcut for the fieldName from the coordinates. + */ + public String getFieldName() { + return this.coordinates.getFieldName(); + } + /** * Whether it is an {@link BatchMapping} method or not in which case it is * an {@link SchemaMapping} method. diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java index 9146d93e..1224546f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/DefaultSchemaResourceGraphQlSourceBuilder.java @@ -152,7 +152,7 @@ final class DefaultSchemaResourceGraphQlSourceBuilder logger.debug("Loaded GraphQL schema resources: (" + resources + ")"); } - RuntimeWiring runtimeWiring = initRuntimeWiring(); + RuntimeWiring runtimeWiring = initRuntimeWiring(registry); updateForCustomRootOperationTypeNames(registry, runtimeWiring); TypeResolver typeResolver = initTypeResolver(); @@ -198,9 +198,12 @@ final class DefaultSchemaResourceGraphQlSourceBuilder } } - private RuntimeWiring initRuntimeWiring() { + private RuntimeWiring initRuntimeWiring(TypeDefinitionRegistry typeRegistry) { RuntimeWiring.Builder builder = RuntimeWiring.newRuntimeWiring(); - this.runtimeWiringConfigurers.forEach((configurer) -> configurer.configure(builder)); + this.runtimeWiringConfigurers.forEach((configurer) -> { + configurer.setTypeDefinitionRegistry(typeRegistry); + configurer.configure(builder); + }); List factories = new ArrayList<>(); WiringFactory factory = builder.build().getWiringFactory(); diff --git a/spring-graphql/src/main/java/org/springframework/graphql/execution/RuntimeWiringConfigurer.java b/spring-graphql/src/main/java/org/springframework/graphql/execution/RuntimeWiringConfigurer.java index 23b568a6..8aad2f0f 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/execution/RuntimeWiringConfigurer.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/execution/RuntimeWiringConfigurer.java @@ -19,6 +19,7 @@ package org.springframework.graphql.execution; import java.util.List; import graphql.schema.idl.RuntimeWiring; +import graphql.schema.idl.TypeDefinitionRegistry; import graphql.schema.idl.WiringFactory; /** @@ -30,6 +31,15 @@ import graphql.schema.idl.WiringFactory; */ public interface RuntimeWiringConfigurer { + /** + * Provides the configurer access to the {@link TypeDefinitionRegistry}. + * @param registry the registry + * @since 1.3.0 + */ + default void setTypeDefinitionRegistry(TypeDefinitionRegistry registry) { + // no-op + } + /** * Apply changes to the {@link RuntimeWiring.Builder} such as registering * {@link graphql.schema.DataFetcher}s, custom scalar types, and more. diff --git a/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurerInterfaceMappingTests.java b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurerInterfaceMappingTests.java new file mode 100644 index 00000000..d6efb561 --- /dev/null +++ b/spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/AnnotatedControllerConfigurerInterfaceMappingTests.java @@ -0,0 +1,282 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.graphql.data.method.annotation.support; + +import java.util.List; +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; + +import graphql.schema.FieldCoordinates; +import graphql.schema.GraphQLCodeRegistry; +import org.junit.jupiter.api.Test; + +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.graphql.GraphQlSetup; +import org.springframework.graphql.ResponseHelper; +import org.springframework.graphql.TestExecutionGraphQlService; +import org.springframework.graphql.data.method.annotation.BatchMapping; +import org.springframework.graphql.data.method.annotation.QueryMapping; +import org.springframework.graphql.data.method.annotation.SchemaMapping; +import org.springframework.graphql.execution.BatchLoaderRegistry; +import org.springframework.graphql.execution.DefaultBatchLoaderRegistry; +import org.springframework.stereotype.Controller; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Tests for registration of mappings with schema interfaces types. + * @author Rossen Stoyanchev + */ +public class AnnotatedControllerConfigurerInterfaceMappingTests { + + private static final String SCHEMA = """ + type Query { + activities: [Activity!]! + } + interface Activity { + id: ID! + labels: [Label!]! + coordinator: User! + } + type FooActivity implements Activity { + id: ID! + labels: [Label!]! + coordinator: User! + } + type BarActivity implements Activity { + id: ID! + labels: [Label!]! + coordinator: User! + } + type Label { + name: String! + } + type User { + name: String! + } + """; + + private final DefaultBatchLoaderRegistry batchLoaderRegistry = new DefaultBatchLoaderRegistry(); + + + @Test + void schemaMapping() { + GraphQLCodeRegistry registry = initCodeRegistry(SchemaMappingController.class); + + assertDataFetcher(registry, "FooActivity", "labels"); + assertDataFetcher(registry, "BarActivity", "labels"); + assertDataFetcher(registry, "FooActivity", "coordinator"); + assertDataFetcher(registry, "BarActivity", "coordinator"); + } + + @Test + void batchMapping() { + GraphQLCodeRegistry registry = initCodeRegistry(BatchMappingController.class); + + assertDataFetcher(registry, "FooActivity", "labels"); + assertDataFetcher(registry, "BarActivity", "labels"); + assertDataFetcher(registry, "FooActivity", "coordinator"); + assertDataFetcher(registry, "BarActivity", "coordinator"); + } + + @Test + void schemaMappingOverride() { + testOverride(SchemaMappingOverrideController.class); + } + + @Test + void batchMappingOverride() { + testOverride(BatchMappingOverrideController.class); + } + + private void testOverride(Class controllerClass) { + String document = """ + { + activities { + labels { + name + } + coordinator { + name + } + } + } + """; + + TestExecutionGraphQlService service = + initGraphQLSetup(controllerClass).dataLoaders(this.batchLoaderRegistry).toGraphQlService(); + + ResponseHelper helper = ResponseHelper.forResponse(service.execute(document)); + List> activities = helper.rawValue("activities"); + + assertThat(activities).hasSize(2); + + assertThat(activities.get(0).get("labels")).isEqualTo(List.of(Map.of("name", "foo-label"))); + assertThat(activities.get(0).get("coordinator")).isEqualTo(Map.of("name", "foo-user")); + + assertThat(activities.get(1).get("labels")).isEqualTo(List.of(Map.of("name", "label"))); + assertThat(activities.get(1).get("coordinator")).isEqualTo(Map.of("name", "user")); + } + + private GraphQLCodeRegistry initCodeRegistry(Class controllerClass) { + return initGraphQLSetup(controllerClass).toGraphQl().getGraphQLSchema().getCodeRegistry(); + } + + private GraphQlSetup initGraphQLSetup(Class controllerClass) { + AnnotationConfigApplicationContext appContext = new AnnotationConfigApplicationContext(); + appContext.registerBean(controllerClass); + appContext.registerBean(BatchLoaderRegistry.class, () -> batchLoaderRegistry); + appContext.refresh(); + + AnnotatedControllerConfigurer configurer = new AnnotatedControllerConfigurer(); + configurer.setApplicationContext(appContext); + configurer.afterPropertiesSet(); + + return GraphQlSetup.schemaContent(SCHEMA).runtimeWiring(configurer); + } + + private static void assertDataFetcher(GraphQLCodeRegistry registry, String typeName, String fieldName) { + assertThat(registry.hasDataFetcher(FieldCoordinates.coordinates(typeName, fieldName))).isTrue(); + } + + + private static class BaseController { + + @QueryMapping + List activities() { + return List.of(new FooActivity(), new BarActivity()); + } + + } + + + @SuppressWarnings("unused") + @Controller + private static class SchemaMappingController extends BaseController { + + @SchemaMapping + List