From 1f9a21e6ba03c00f3502b0776050821479947761 Mon Sep 17 00:00:00 2001 From: "do.vantrinh" Date: Sat, 24 May 2025 17:21:38 +0900 Subject: [PATCH 1/3] Remove redundant interface declaration WebGraphQlRequest already implements ExecutionGraphQlRequest through its parent class DefaultExecutionGraphQlRequest, making the explicit interface declaration redundant. See gh-1218 Signed-off-by: do.vantrinh --- .../org/springframework/graphql/server/WebGraphQlRequest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/server/WebGraphQlRequest.java b/spring-graphql/src/main/java/org/springframework/graphql/server/WebGraphQlRequest.java index 52c735e7..4ea50174 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/server/WebGraphQlRequest.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/server/WebGraphQlRequest.java @@ -22,7 +22,6 @@ import java.util.Collections; import java.util.Locale; import java.util.Map; -import org.springframework.graphql.ExecutionGraphQlRequest; import org.springframework.graphql.GraphQlRequest; import org.springframework.graphql.support.DefaultExecutionGraphQlRequest; import org.springframework.http.HttpCookie; @@ -47,7 +46,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Rossen Stoyanchev * @since 1.0.0 */ -public class WebGraphQlRequest extends DefaultExecutionGraphQlRequest implements ExecutionGraphQlRequest { +public class WebGraphQlRequest extends DefaultExecutionGraphQlRequest { private static final MultiValueMap EMPTY_COOKIES = CollectionUtils.unmodifiableMultiValueMap(new LinkedMultiValueMap<>()); From d3ff9687eb015c4e14498c635775a1e8c1b14eca Mon Sep 17 00:00:00 2001 From: Josh Allen Date: Wed, 28 May 2025 14:23:22 -0500 Subject: [PATCH 2/3] Exempt unresolvable federated entities from checks Update check ensuring federated types have @EntityMapping to exempt entities with 'resolvable: false', since by definition those should not have an @EntityMapping. See gh-1225 Signed-off-by: Josh Allen --- .../federation/FederationSchemaFactory.java | 27 ++++++++++++++----- .../books/federation-schema.graphqls | 7 +++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java index c6182917..095d55fd 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java @@ -26,6 +26,9 @@ import java.util.stream.Collectors; import com.apollographql.federation.graphqljava.Federation; import com.apollographql.federation.graphqljava.SchemaTransformer; +import graphql.language.Argument; +import graphql.language.BooleanValue; +import graphql.language.Directive; import graphql.language.TypeDefinition; import graphql.schema.DataFetcher; import graphql.schema.GraphQLSchema; @@ -190,12 +193,9 @@ public final class FederationSchemaFactory private void checkEntityMappings(TypeDefinitionRegistry registry) { List unmappedEntities = new ArrayList<>(); for (TypeDefinition type : registry.types().values()) { - type.getDirectives().forEach((directive) -> { - boolean isEntityType = directive.getName().equalsIgnoreCase("key"); - if (isEntityType && !this.handlerMethods.containsKey(type.getName())) { - unmappedEntities.add(type.getName()); - } - }); + if (isEntityMappingExpected(type) && !this.handlerMethods.containsKey(type.getName())) { + unmappedEntities.add(type.getName()); + } } if (!unmappedEntities.isEmpty()) { throw new IllegalStateException("Unmapped entity types: " + @@ -203,6 +203,21 @@ public final class FederationSchemaFactory } } + /** + * Determine if a handler method is expected for this type: there is at least one '@key' directive + * whose 'resolvable' argument resolves to true (either explicitly, or if the argument is not set). + * @param type the type to inspect. + * @return true if a handler method is expected for this type + */ + private boolean isEntityMappingExpected(TypeDefinition type) { + List keyDirectives = type.getDirectives("key"); + return !keyDirectives.isEmpty() && keyDirectives.stream() + .anyMatch((keyDirective) -> { + Argument resolvableArg = keyDirective.getArgument("resolvable"); + return resolvableArg == null || + (resolvableArg.getValue() instanceof BooleanValue) && ((BooleanValue) resolvableArg.getValue()).isValue(); + }); + } public record EntityMappingInfo(String typeName, HandlerMethod handlerMethod) { diff --git a/spring-graphql/src/test/resources/books/federation-schema.graphqls b/spring-graphql/src/test/resources/books/federation-schema.graphqls index 251dca33..0f891859 100644 --- a/spring-graphql/src/test/resources/books/federation-schema.graphqls +++ b/spring-graphql/src/test/resources/books/federation-schema.graphqls @@ -1,6 +1,9 @@ +extend schema @link(url: "https://specs.apollo.dev/federation/v2.9", import: ["@key", "@extends", "@external"] ) + type Book @key(fields: "id") @extends { id: ID! @external author: Author + publisher: Publisher } type Author { @@ -8,3 +11,7 @@ type Author { firstName: String lastName: String } + +type Publisher @key(fields: "id", resolvable: false) { + id: ID! @external +} From 14baa4e32a906ad415a0d48c0665e79b6ee8f6a4 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 29 May 2025 17:07:52 +0100 Subject: [PATCH 3/3] Polishing contribution Closes: gh-1225 --- .../federation/FederationSchemaFactory.java | 33 +++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java index 095d55fd..3e2835e1 100644 --- a/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java +++ b/spring-graphql/src/main/java/org/springframework/graphql/data/federation/FederationSchemaFactory.java @@ -193,9 +193,11 @@ public final class FederationSchemaFactory private void checkEntityMappings(TypeDefinitionRegistry registry) { List unmappedEntities = new ArrayList<>(); for (TypeDefinition type : registry.types().values()) { - if (isEntityMappingExpected(type) && !this.handlerMethods.containsKey(type.getName())) { - unmappedEntities.add(type.getName()); - } + type.getDirectives().forEach((directive) -> { + if (isResolvableKeyDirective(directive) && !this.handlerMethods.containsKey(type.getName())) { + unmappedEntities.add(type.getName()); + } + }); } if (!unmappedEntities.isEmpty()) { throw new IllegalStateException("Unmapped entity types: " + @@ -203,22 +205,19 @@ public final class FederationSchemaFactory } } - /** - * Determine if a handler method is expected for this type: there is at least one '@key' directive - * whose 'resolvable' argument resolves to true (either explicitly, or if the argument is not set). - * @param type the type to inspect. - * @return true if a handler method is expected for this type - */ - private boolean isEntityMappingExpected(TypeDefinition type) { - List keyDirectives = type.getDirectives("key"); - return !keyDirectives.isEmpty() && keyDirectives.stream() - .anyMatch((keyDirective) -> { - Argument resolvableArg = keyDirective.getArgument("resolvable"); - return resolvableArg == null || - (resolvableArg.getValue() instanceof BooleanValue) && ((BooleanValue) resolvableArg.getValue()).isValue(); - }); + private boolean isResolvableKeyDirective(Directive directive) { + if (!directive.getName().equalsIgnoreCase("key")) { + return false; + } + Argument argument = directive.getArgument("resolvable"); + if (argument != null) { + Object value = argument.getValue(); + return (value instanceof BooleanValue bv && bv.isValue()); + } + return true; } + public record EntityMappingInfo(String typeName, HandlerMethod handlerMethod) { public boolean isBatchHandlerMethod() {