Skip to content
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.

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

graphql-java-support: Remove federation directive and type definitions from SDL and use stricter SchemaGenerator checks #53

Merged
merged 9 commits into from
Mar 27, 2020
Merged
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
package com.apollographql.federation.graphqljava;

import graphql.language.DirectiveDefinition;
import graphql.language.ObjectTypeDefinition;
import graphql.schema.GraphQLSchema;
import graphql.schema.idl.RuntimeWiring;
import graphql.schema.idl.SchemaGenerator;
import graphql.schema.idl.SchemaParser;
import graphql.schema.idl.TypeDefinitionRegistry;
import graphql.schema.idl.TypeRuntimeWiring;
import org.jetbrains.annotations.NotNull;

import java.io.File;
import java.io.Reader;
import java.util.Comparator;

public final class Federation {
private static final SchemaGenerator.Options generatorOptions = SchemaGenerator.Options.defaultOptions()
.enforceSchemaDirectives(false);
.enforceSchemaDirectives(true);

private Federation() {
}
Expand All @@ -25,10 +28,11 @@ public static SchemaTransformer transform(final GraphQLSchema schema) {

public static SchemaTransformer transform(final TypeDefinitionRegistry typeRegistry, final RuntimeWiring runtimeWiring) {
ensureQueryTypeExists(typeRegistry);
RuntimeWiring newRuntimeWiring = ensureFederationDirectiveDefinitionsExist(typeRegistry, runtimeWiring);
final GraphQLSchema original = new SchemaGenerator().makeExecutableSchema(
generatorOptions,
typeRegistry,
runtimeWiring);
newRuntimeWiring);
return transform(original);
}

Expand Down Expand Up @@ -76,4 +80,69 @@ private static void ensureQueryTypeExists(TypeDefinitionRegistry typeRegistry) {
typeRegistry.add(ObjectTypeDefinition.newObjectTypeDefinition().name(queryName).build());
}
}

private static RuntimeWiring ensureFederationDirectiveDefinitionsExist(
TypeDefinitionRegistry typeRegistry,
RuntimeWiring runtimeWiring
) {
// Add Federation directives if they don't exist.
FederationDirectives.allDefinitions
.stream()
.filter(def -> !typeRegistry.getDirectiveDefinition(def.getName()).isPresent())
.forEachOrdered(typeRegistry::add);

// Add scalar type for _FieldSet, since the directives depend on it.
if (!typeRegistry.getType(_FieldSet.typeName).isPresent()) {
typeRegistry.add(_FieldSet.definition);
}

// Also add the implementation for _FieldSet.
if (!runtimeWiring.getScalars().containsKey(_FieldSet.typeName)) {
return copyRuntimeWiring(runtimeWiring).scalar(_FieldSet.type).build();
} else {
return runtimeWiring;
}
}

private static RuntimeWiring.Builder copyRuntimeWiring(RuntimeWiring runtimeWiring) {
// Annoyingly graphql-java doesn't have a copy constructor for RuntimeWiring.Builder.
RuntimeWiring.Builder builder = RuntimeWiring.newRuntimeWiring();

runtimeWiring.getDataFetchers()
.entrySet()
.stream()
.map(entry -> {
String name = entry.getKey();
TypeRuntimeWiring.Builder typeWiring = TypeRuntimeWiring.newTypeWiring(name);
typeWiring.dataFetchers(entry.getValue());
if (runtimeWiring.getDefaultDataFetcherForType(name) != null) {
typeWiring.defaultDataFetcher(runtimeWiring.getDefaultDataFetcherForType(name));
}
if (runtimeWiring.getTypeResolvers().get(name) != null) {
typeWiring.typeResolver(runtimeWiring.getTypeResolvers().get(name));
}
if (runtimeWiring.getEnumValuesProviders().get(name) != null) {
typeWiring.enumValues(runtimeWiring.getEnumValuesProviders().get(name));
}
return typeWiring.build();
})
.forEach(builder::type);

if (runtimeWiring.getWiringFactory() != null) {
builder.wiringFactory(runtimeWiring.getWiringFactory());
}
if (runtimeWiring.getCodeRegistry() != null) {
builder.codeRegistry(runtimeWiring.getCodeRegistry());
}
runtimeWiring.getScalars().forEach((name, scalar) -> builder.scalar(scalar));
if (runtimeWiring.getFieldVisibility() != null) {
builder.fieldVisibility(runtimeWiring.getFieldVisibility());
}
runtimeWiring.getRegisteredDirectiveWiring().forEach(builder::directive);
runtimeWiring.getDirectiveWiring().forEach(builder::directiveWiring);
builder.comparatorRegistry(runtimeWiring.getComparatorRegistry());
runtimeWiring.getSchemaTransformers().forEach(builder::transformer);

return builder;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,17 @@
import graphql.language.DirectiveLocation;
import graphql.language.InputValueDefinition;
import graphql.language.NonNullType;
import graphql.language.SDLDefinition;
import graphql.language.TypeName;
import graphql.schema.GraphQLArgument;
import graphql.schema.GraphQLDirective;
import graphql.schema.GraphQLNonNull;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static graphql.introspection.Introspection.DirectiveLocation.FIELD_DEFINITION;
import static graphql.introspection.Introspection.DirectiveLocation.INTERFACE;
Expand Down Expand Up @@ -158,19 +160,35 @@ private FederationDirectives() {

/* Sets */

public static final Set<GraphQLDirective> allDirectives = new HashSet<>();
public static final Set<SDLDefinition> allDefinitions = new HashSet<>();
public static final Set<String> allNames;
public static final Set<GraphQLDirective> allDirectives;
public static final Set<DirectiveDefinition> allDefinitions;

static {
allDirectives.add(key);
allDirectives.add(external);
allDirectives.add(requires);
allDirectives.add(provides);
allDirectives.add(extends_);
allDefinitions.add(keyDefinition);
allDefinitions.add(externalDefinition);
allDefinitions.add(requiresDefinition);
allDefinitions.add(providesDefinition);
allDefinitions.add(extendsDefinition);
// We need to maintain sorted order here for tests, since SchemaPrinter doesn't sort
// directive definitions.
allDirectives = Stream.of(
key,
external,
requires,
provides,
extends_
)
.sorted(Comparator.comparing(GraphQLDirective::getName))
.collect(Collectors.toCollection(LinkedHashSet::new));
allDefinitions = Stream.of(
keyDefinition,
externalDefinition,
requiresDefinition,
providesDefinition,
extendsDefinition
)
.sorted(Comparator.comparing(DirectiveDefinition::getName))
.collect(Collectors.toCollection(LinkedHashSet::new));
allNames = allDefinitions
.stream()
.map(DirectiveDefinition::getName)
.collect(Collectors.toCollection(LinkedHashSet::new));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,27 @@ public static class Options {

private final boolean includeDirectives;

private final Predicate<GraphQLDirective> includeDirectiveDefinition;

private final Predicate<GraphQLType> includeTypeDefinition;

private final GraphqlTypeComparatorRegistry comparatorRegistry;

private Options(boolean includeIntrospectionTypes,
boolean includeScalars,
boolean includeExtendedScalars,
boolean includeSchemaDefinition,
boolean includeDirectives,
Predicate<GraphQLDirective> includeDirectiveDefinition,
Predicate<GraphQLType> includeTypeDefinition,
GraphqlTypeComparatorRegistry comparatorRegistry) {
this.includeIntrospectionTypes = includeIntrospectionTypes;
this.includeScalars = includeScalars;
this.includeExtendedScalars = includeExtendedScalars;
this.includeSchemaDefinition = includeSchemaDefinition;
this.includeDirectives = includeDirectives;
this.includeDirectiveDefinition = includeDirectiveDefinition;
this.includeTypeDefinition = includeTypeDefinition;
this.comparatorRegistry = comparatorRegistry;
}

Expand All @@ -106,6 +114,7 @@ public boolean isIncludeDirectives() {

public static Options defaultOptions() {
return new Options(false, false, false, false, true,
directiveDefinition -> true, typeDefinition -> true,
DefaultGraphqlTypeComparatorRegistry.defaultComparators());
}

Expand All @@ -117,7 +126,7 @@ public static Options defaultOptions() {
* @return options
*/
public Options includeIntrospectionTypes(boolean flag) {
return new Options(flag, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.comparatorRegistry);
return new Options(flag, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
Expand All @@ -128,7 +137,7 @@ public Options includeIntrospectionTypes(boolean flag) {
* @return options
*/
public Options includeScalarTypes(boolean flag) {
return new Options(this.includeIntrospectionTypes, flag, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.comparatorRegistry);
return new Options(this.includeIntrospectionTypes, flag, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
Expand All @@ -140,7 +149,7 @@ public Options includeScalarTypes(boolean flag) {
* @return options
*/
public Options includeExtendedScalarTypes(boolean flag) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, flag, this.includeSchemaDefinition, this.includeDirectives, this.comparatorRegistry);
return new Options(this.includeIntrospectionTypes, this.includeScalars, flag, this.includeSchemaDefinition, this.includeDirectives, this.includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
Expand All @@ -154,7 +163,7 @@ public Options includeExtendedScalarTypes(boolean flag) {
* @return options
*/
public Options includeSchemaDefintion(boolean flag) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, flag, this.includeDirectives, this.comparatorRegistry);
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, flag, this.includeDirectives, this.includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
Expand All @@ -166,7 +175,32 @@ public Options includeSchemaDefintion(boolean flag) {
* @return new instance of options
*/
public Options includeDirectives(boolean flag) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, flag, this.comparatorRegistry);
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, flag, this.includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
* Filter printing of directive definitions. In Apollo Federation, some directive
* definitions need to be hidden, and this predicate allows filtering out such definitions.
* Prints all definitions by default. Note that both this predicate and the flag in
* {@link #includeDirectives(boolean)} must be true for a definition to be printed.
*
* @param includeDirectiveDefinition returns true if the definition should be printed
* @return new instance of options
*/
public Options includeDirectiveDefinitions(Predicate<GraphQLDirective> includeDirectiveDefinition) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, includeDirectiveDefinition, this.includeTypeDefinition, this.comparatorRegistry);
}

/**
* Filter printing of type definitions. In Apollo Federation, some type definitions need to
* be hidden, and this predicate allows filtering out such definitions. Prints all
* definitions by default.
*
* @param includeTypeDefinition returns true if the definition should be printed
* @return new instance of options
*/
public Options includeTypeDefinitions(Predicate<GraphQLType> includeTypeDefinition) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.includeDirectiveDefinition, includeTypeDefinition, this.comparatorRegistry);
}

/**
Expand All @@ -179,7 +213,7 @@ public Options includeDirectives(boolean flag) {
* @return options
*/
public Options setComparators(GraphqlTypeComparatorRegistry comparatorRegistry) {
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives,
return new Options(this.includeIntrospectionTypes, this.includeScalars, this.includeExtendedScalars, this.includeSchemaDefinition, this.includeDirectives, this.includeDirectiveDefinition, this.includeTypeDefinition,
comparatorRegistry);
}

Expand Down Expand Up @@ -240,6 +274,7 @@ public String print(GraphQLSchema schema) {
List<GraphQLType> typesAsList = schema.getAllTypesAsList()
.stream()
.sorted(Comparator.comparing(GraphQLType::getName))
.filter(options.includeTypeDefinition)
.collect(toList());

printType(out, typesAsList, GraphQLInterfaceType.class, visibility);
Expand Down Expand Up @@ -523,7 +558,10 @@ private List<GraphQLDirective> getDirectives(GraphQLSchema schema) {
"deprecated");

Predicate<GraphQLDirective> standard = directive -> standardDirectives.contains(directive.getName());
return schema.getDirectives().stream().filter(standard.negate()).collect(Collectors.toList());
return schema.getDirectives().stream()
.filter(standard.negate())
.filter(options.includeDirectiveDefinition)
.collect(Collectors.toList());
}

String typeString(GraphQLType rawType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.jetbrains.annotations.NotNull;

import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -69,7 +70,7 @@ public final GraphQLSchema build() throws SchemaProblem {
GraphQLCodeRegistry.newCodeRegistry(originalSchema.getCodeRegistry());

// Print the original schema as sdl and expose it as query { _service { sdl } }
final String sdl = sdl();
final String sdl = sdl(originalSchema);
final GraphQLObjectType.Builder newQueryType = GraphQLObjectType.newObject(originalQueryType)
.field(_Service.field);
newCodeRegistry.dataFetcher(FieldCoordinates.coordinates(
Expand Down Expand Up @@ -137,15 +138,34 @@ public final GraphQLSchema build() throws SchemaProblem {
.build();
}

private String sdl() {
// Note that FederationSdlPrinter is a copy of graphql-java's SchemaPrinter that fixes a
// specific bug. Once graphql-java releases a bugfix (the PR in question is specifically
// graphql-java/graphql-java#1798 ) and backports it, we should revert to SchemaPrinter.
public static String sdl(GraphQLSchema schema) {
// Gather directive definitions to hide.
final Set<String> hiddenDirectiveDefinitions = new HashSet<>(FederationDirectives.allNames);

// Gather type definitions to hide.
final Set<String> hiddenTypeDefinitions = new HashSet<>();
hiddenTypeDefinitions.add(_Any.typeName);
hiddenTypeDefinitions.add(_Entity.typeName);
hiddenTypeDefinitions.add(_FieldSet.typeName);
hiddenTypeDefinitions.add(_Service.typeName);

// Note that FederationSdlPrinter is a copy of graphql-java's SchemaPrinter that:
// - fixes a specific bug in graphql-java that hasn't been backported yet, specifically
// graphql-java/graphql-java#1798
// - adds the ability to filter out directive and type definitions, which is required
// by federation spec.
//
// FederationSdlPrinter will need to be updated whenever graphql-java changes versions. It
// can be removed when the bug is fixed/backported, and when either graphql-java adds
// native support for filtering out directive and type definitions or federation spec
// changes to allow the currently forbidden directive and type definitions.
final FederationSdlPrinter.Options options = FederationSdlPrinter.Options.defaultOptions()
.includeScalarTypes(true)
.includeExtendedScalarTypes(true)
.includeSchemaDefintion(true)
.includeDirectives(true);
return new FederationSdlPrinter(options).print(originalSchema);
.includeDirectives(true)
.includeDirectiveDefinitions(def -> !hiddenDirectiveDefinitions.contains(def.getName()))
.includeTypeDefinitions(def -> !hiddenTypeDefinitions.contains(def.getName()));
return new FederationSdlPrinter(options).print(schema);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public final class _FieldSet {

public static GraphQLScalarType type = GraphQLScalarType.newScalar(Scalars.GraphQLString)
.name(typeName)
.description(null)
.coercing(Scalars.GraphQLString.getCoercing())
.build();

Expand Down