From e2f7467cd7208d2d156850e543eab6d178075200 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 24 Jun 2025 13:55:47 +1000 Subject: [PATCH 1/2] Added a read only copy of a type registry --- .../idl/ImmutableTypeDefinitionRegistry.java | 132 ++++++++++++ .../graphql/schema/idl/SchemaGenerator.java | 10 +- .../schema/idl/SchemaGeneratorHelper.java | 4 +- .../graphql/schema/idl/SchemaTypeChecker.java | 2 +- .../schema/idl/TypeDefinitionRegistry.java | 76 +++++-- ...ImmutableTypeDefinitionRegistryTest.groovy | 196 ++++++++++++++++++ 6 files changed, 400 insertions(+), 20 deletions(-) create mode 100644 src/main/java/graphql/schema/idl/ImmutableTypeDefinitionRegistry.java create mode 100644 src/test/groovy/graphql/schema/idl/ImmutableTypeDefinitionRegistryTest.groovy diff --git a/src/main/java/graphql/schema/idl/ImmutableTypeDefinitionRegistry.java b/src/main/java/graphql/schema/idl/ImmutableTypeDefinitionRegistry.java new file mode 100644 index 0000000000..1604551f25 --- /dev/null +++ b/src/main/java/graphql/schema/idl/ImmutableTypeDefinitionRegistry.java @@ -0,0 +1,132 @@ +package graphql.schema.idl; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import graphql.GraphQLError; +import graphql.PublicApi; +import graphql.language.DirectiveDefinition; +import graphql.language.EnumTypeExtensionDefinition; +import graphql.language.InputObjectTypeExtensionDefinition; +import graphql.language.InterfaceTypeExtensionDefinition; +import graphql.language.ObjectTypeExtensionDefinition; +import graphql.language.SDLDefinition; +import graphql.language.ScalarTypeDefinition; +import graphql.language.ScalarTypeExtensionDefinition; +import graphql.language.SchemaExtensionDefinition; +import graphql.language.TypeDefinition; +import graphql.language.UnionTypeExtensionDefinition; +import graphql.schema.idl.errors.SchemaProblem; +import org.jspecify.annotations.NullMarked; + +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static com.google.common.collect.ImmutableMap.copyOf; + +/** + * A {@link ImmutableTypeDefinitionRegistry} contains an immutable set of type definitions that come from compiling + * a graphql schema definition file via {@link SchemaParser#parse(String)} and is more performant because it + * uses {@link ImmutableMap} structures. + */ +@SuppressWarnings("rawtypes") +@PublicApi +@NullMarked +public class ImmutableTypeDefinitionRegistry extends TypeDefinitionRegistry { + ImmutableTypeDefinitionRegistry(TypeDefinitionRegistry registry) { + super( + copyOf(registry.objectTypeExtensions), + copyOf(registry.interfaceTypeExtensions), + copyOf(registry.unionTypeExtensions), + copyOf(registry.enumTypeExtensions), + copyOf(registry.scalarTypeExtensions), + copyOf(registry.inputObjectTypeExtensions), + copyOf(registry.types), + copyOf(registry.scalars()), // has an extra side effect + copyOf(registry.directiveDefinitions), + ImmutableList.copyOf(registry.schemaExtensionDefinitions), + registry.schema, + registry.schemaParseOrder + ); + } + + private UnsupportedOperationException unsupportedOperationException() { + return new UnsupportedOperationException("The TypeDefinitionRegistry is in read only mode"); + } + + @Override + public TypeDefinitionRegistry merge(TypeDefinitionRegistry typeRegistry) throws SchemaProblem { + throw unsupportedOperationException(); + } + + @Override + public Optional addAll(Collection definitions) { + throw unsupportedOperationException(); + } + + @Override + public Optional add(SDLDefinition definition) { + throw unsupportedOperationException(); + } + + @Override + public void remove(SDLDefinition definition) { + throw unsupportedOperationException(); + } + + @Override + public void remove(String key, SDLDefinition definition) { + throw unsupportedOperationException(); + } + + @Override + public Map types() { + return types; + } + + @Override + public Map scalars() { + return scalarTypes; + } + + @Override + public Map> objectTypeExtensions() { + return objectTypeExtensions; + } + + @Override + public Map> interfaceTypeExtensions() { + return interfaceTypeExtensions; + } + + @Override + public Map> unionTypeExtensions() { + return unionTypeExtensions; + } + + @Override + public Map> enumTypeExtensions() { + return enumTypeExtensions; + } + + @Override + public Map> scalarTypeExtensions() { + return scalarTypeExtensions; + } + + @Override + public Map> inputObjectTypeExtensions() { + return inputObjectTypeExtensions; + } + + @Override + public List getSchemaExtensionDefinitions() { + return schemaExtensionDefinitions; + } + + @Override + public Map getDirectiveDefinitions() { + return directiveDefinitions; + } +} diff --git a/src/main/java/graphql/schema/idl/SchemaGenerator.java b/src/main/java/graphql/schema/idl/SchemaGenerator.java index 868aec4b76..c53ef08b8f 100644 --- a/src/main/java/graphql/schema/idl/SchemaGenerator.java +++ b/src/main/java/graphql/schema/idl/SchemaGenerator.java @@ -103,17 +103,19 @@ public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistr schemaGeneratorHelper.addDirectivesIncludedByDefault(typeRegistryCopy); - List errors = typeChecker.checkTypeRegistry(typeRegistryCopy, wiring); + // by making it read only all the traversal and checks run faster + ImmutableTypeDefinitionRegistry fasterImmutableRegistry = typeRegistryCopy.readOnly(); + List errors = typeChecker.checkTypeRegistry(fasterImmutableRegistry, wiring); if (!errors.isEmpty()) { throw new SchemaProblem(errors); } - Map operationTypeDefinitions = SchemaExtensionsChecker.gatherOperationDefs(typeRegistry); + Map operationTypeDefinitions = SchemaExtensionsChecker.gatherOperationDefs(fasterImmutableRegistry); - return makeExecutableSchemaImpl(typeRegistryCopy, wiring, operationTypeDefinitions, options); + return makeExecutableSchemaImpl(fasterImmutableRegistry, wiring, operationTypeDefinitions, options); } - private GraphQLSchema makeExecutableSchemaImpl(TypeDefinitionRegistry typeRegistry, + private GraphQLSchema makeExecutableSchemaImpl(ImmutableTypeDefinitionRegistry typeRegistry, RuntimeWiring wiring, Map operationTypeDefinitions, Options options) { diff --git a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java index e658c85d17..521bb185b0 100644 --- a/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java +++ b/src/main/java/graphql/schema/idl/SchemaGeneratorHelper.java @@ -111,7 +111,7 @@ public class SchemaGeneratorHelper { * it gives us helper functions */ static class BuildContext { - private final TypeDefinitionRegistry typeRegistry; + private final ImmutableTypeDefinitionRegistry typeRegistry; private final RuntimeWiring wiring; private final Deque typeStack = new ArrayDeque<>(); @@ -123,7 +123,7 @@ static class BuildContext { public final SchemaGenerator.Options options; public boolean directiveWiringRequired; - BuildContext(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring, Map operationTypeDefinitions, SchemaGenerator.Options options) { + BuildContext(ImmutableTypeDefinitionRegistry typeRegistry, RuntimeWiring wiring, Map operationTypeDefinitions, SchemaGenerator.Options options) { this.typeRegistry = typeRegistry; this.wiring = wiring; this.codeRegistry = GraphQLCodeRegistry.newCodeRegistry(wiring.getCodeRegistry()); diff --git a/src/main/java/graphql/schema/idl/SchemaTypeChecker.java b/src/main/java/graphql/schema/idl/SchemaTypeChecker.java index 6294fd432f..5d702b6444 100644 --- a/src/main/java/graphql/schema/idl/SchemaTypeChecker.java +++ b/src/main/java/graphql/schema/idl/SchemaTypeChecker.java @@ -52,7 +52,7 @@ @Internal public class SchemaTypeChecker { - public List checkTypeRegistry(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { + public List checkTypeRegistry(ImmutableTypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem { List errors = new ArrayList<>(); checkForMissingTypes(errors, typeRegistry); diff --git a/src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java b/src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java index 599c3a1340..dcbfb9c413 100644 --- a/src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java +++ b/src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java @@ -54,20 +54,70 @@ @NullMarked public class TypeDefinitionRegistry implements Serializable { - private final Map> objectTypeExtensions = new LinkedHashMap<>(); - private final Map> interfaceTypeExtensions = new LinkedHashMap<>(); - private final Map> unionTypeExtensions = new LinkedHashMap<>(); - private final Map> enumTypeExtensions = new LinkedHashMap<>(); - private final Map> scalarTypeExtensions = new LinkedHashMap<>(); - private final Map> inputObjectTypeExtensions = new LinkedHashMap<>(); - - private final Map types = new LinkedHashMap<>(); - private final Map scalarTypes = new LinkedHashMap<>(); - private final Map directiveDefinitions = new LinkedHashMap<>(); - private @Nullable SchemaDefinition schema; - private final List schemaExtensionDefinitions = new ArrayList<>(); - private final SchemaParseOrder schemaParseOrder = new SchemaParseOrder(); + protected final Map> objectTypeExtensions; + protected final Map> interfaceTypeExtensions; + protected final Map> unionTypeExtensions; + protected final Map> enumTypeExtensions; + protected final Map> scalarTypeExtensions; + protected final Map> inputObjectTypeExtensions; + + protected final Map types; + protected final Map scalarTypes; + protected final Map directiveDefinitions; + protected @Nullable SchemaDefinition schema; + protected final List schemaExtensionDefinitions; + protected final SchemaParseOrder schemaParseOrder; + + public TypeDefinitionRegistry() { + objectTypeExtensions = new LinkedHashMap<>(); + interfaceTypeExtensions = new LinkedHashMap<>(); + unionTypeExtensions = new LinkedHashMap<>(); + enumTypeExtensions = new LinkedHashMap<>(); + scalarTypeExtensions = new LinkedHashMap<>(); + inputObjectTypeExtensions = new LinkedHashMap<>(); + types = new LinkedHashMap<>(); + scalarTypes = new LinkedHashMap<>(); + directiveDefinitions = new LinkedHashMap<>(); + schemaExtensionDefinitions = new ArrayList<>(); + schemaParseOrder = new SchemaParseOrder(); + } + + protected TypeDefinitionRegistry(Map> objectTypeExtensions, + Map> interfaceTypeExtensions, + Map> unionTypeExtensions, + Map> enumTypeExtensions, + Map> scalarTypeExtensions, + Map> inputObjectTypeExtensions, + Map types, + Map scalarTypes, + Map directiveDefinitions, + List schemaExtensionDefinitions, + @Nullable SchemaDefinition schema, + SchemaParseOrder schemaParseOrder) { + this.objectTypeExtensions = objectTypeExtensions; + this.interfaceTypeExtensions = interfaceTypeExtensions; + this.unionTypeExtensions = unionTypeExtensions; + this.enumTypeExtensions = enumTypeExtensions; + this.scalarTypeExtensions = scalarTypeExtensions; + this.inputObjectTypeExtensions = inputObjectTypeExtensions; + this.types = types; + this.scalarTypes = scalarTypes; + this.directiveDefinitions = directiveDefinitions; + this.schemaExtensionDefinitions = schemaExtensionDefinitions; + this.schema = schema; + this.schemaParseOrder = schemaParseOrder; + } + /** + * @return an immutable view of this {@link TypeDefinitionRegistry} that is more performant + * when in read only mode. + */ + public ImmutableTypeDefinitionRegistry readOnly() { + if (this instanceof ImmutableTypeDefinitionRegistry) { + return (ImmutableTypeDefinitionRegistry) this; + } + return new ImmutableTypeDefinitionRegistry(this); + } /** * @return the order in which {@link SDLDefinition}s were parsed diff --git a/src/test/groovy/graphql/schema/idl/ImmutableTypeDefinitionRegistryTest.groovy b/src/test/groovy/graphql/schema/idl/ImmutableTypeDefinitionRegistryTest.groovy new file mode 100644 index 0000000000..c29636b3e1 --- /dev/null +++ b/src/test/groovy/graphql/schema/idl/ImmutableTypeDefinitionRegistryTest.groovy @@ -0,0 +1,196 @@ +package graphql.schema.idl + +import graphql.language.TypeDefinition +import spock.lang.Specification + +class ImmutableTypeDefinitionRegistryTest extends Specification { + + def serializableSchema = ''' + "the schema" + schema { + query : Q + } + + "the query type" + type Q { + field( arg : String! = "default") : FieldType @deprecated(reason : "no good") + } + + interface FieldType { + f : UnionType + } + + type FieldTypeImpl implements FieldType { + f : UnionType + } + + union UnionType = Foo | Bar + + type Foo { + foo : String + } + + type Bar { + bar : String + } + + scalar MyScalar + + input InputType { + in : String + } + ''' + + def "immutable registry can be serialized and hence cacheable"() { + def registryOut = new SchemaParser().parse(serializableSchema).readOnly() + + when: + + TypeDefinitionRegistry registryIn = serialise(registryOut).readOnly() + + then: + + TypeDefinition typeIn = registryIn.getType(typeName).get() + TypeDefinition typeOut = registryOut.getType(typeName).get() + typeIn.isEqualTo(typeOut) + + where: + typeName | _ + "Q" | _ + "FieldType" | _ + "FieldTypeImpl" | _ + "UnionType" | _ + "Foo" | _ + "Bar" | _ + } + + def "immutable registry is a perfect copy of the starting registry"() { + when: + def mutableRegistry = new SchemaParser().parse(serializableSchema) + def immutableRegistry = mutableRegistry.readOnly() + + then: + + containsSameObjects(mutableRegistry.types(), immutableRegistry.types()) + + TypeDefinition typeIn = mutableRegistry.getType(typeName).get() + TypeDefinition typeOut = immutableRegistry.getType(typeName).get() + typeIn.isEqualTo(typeOut) + + where: + typeName | _ + "Q" | _ + "FieldType" | _ + "FieldTypeImpl" | _ + "UnionType" | _ + "Foo" | _ + "Bar" | _ + } + + def "extensions are also present in immutable copy"() { + def sdl = serializableSchema + """ + extend type FieldTypeImpl { + extra : String + } + + extend input InputType { + out : String + } + + extend scalar MyScalar @specifiedBy(url: "myUrl.example") + + extend union UnionType @deprecated + + extend interface FieldType @deprecated + + """ + + when: + def mutableRegistry = new SchemaParser().parse(sdl) + def immutableRegistry = mutableRegistry.readOnly() + + then: + + containsSameObjects(mutableRegistry.types(), immutableRegistry.types()) + containsSameObjects(mutableRegistry.objectTypeExtensions(), immutableRegistry.objectTypeExtensions()) + containsSameObjects(mutableRegistry.inputObjectTypeExtensions(), immutableRegistry.inputObjectTypeExtensions()) + containsSameObjects(mutableRegistry.interfaceTypeExtensions(), immutableRegistry.interfaceTypeExtensions()) + containsSameObjects(mutableRegistry.unionTypeExtensions(), immutableRegistry.unionTypeExtensions()) + containsSameObjects(mutableRegistry.scalarTypeExtensions(), immutableRegistry.scalarTypeExtensions()) + + } + + def "readonly is aware if itself"() { + when: + def mutableRegistry = new SchemaParser().parse(serializableSchema) + def immutableRegistry1 = mutableRegistry.readOnly() + + then: + mutableRegistry !== immutableRegistry1 + + when: + def immutableRegistry2 = immutableRegistry1.readOnly() + + then: + immutableRegistry2 === immutableRegistry1 + + + } + + def "is in read only mode"() { + when: + def mutableRegistry = new SchemaParser().parse(serializableSchema) + def immutableRegistry = mutableRegistry.readOnly() + + immutableRegistry.merge(mutableRegistry) + + then: + thrown(UnsupportedOperationException) + + when: + immutableRegistry.addAll([]) + then: + thrown(UnsupportedOperationException) + + + def someDef = mutableRegistry.getTypes(TypeDefinition.class)[0] + + when: + immutableRegistry.add(someDef) + then: + thrown(UnsupportedOperationException) + + when: + immutableRegistry.remove(someDef) + then: + thrown(UnsupportedOperationException) + + when: + immutableRegistry.remove("key", someDef) + then: + thrown(UnsupportedOperationException) + + } + + void containsSameObjects(Map leftMap, Map rightMap) { + assert leftMap.size() > 0, "The map must have some entries" + assert leftMap.size() == rightMap.size(), "The maps are not the same size" + for (String leftKey : leftMap.keySet()) { + def leftVal = leftMap.get(leftKey) + def rightVal = rightMap.get(leftKey) + assert leftVal === rightVal, "$leftKey : $leftVal dont not strictly equal $rightVal" + } + } + + static TypeDefinitionRegistry serialise(TypeDefinitionRegistry registryOut) { + ByteArrayOutputStream baOS = new ByteArrayOutputStream() + ObjectOutputStream oos = new ObjectOutputStream(baOS) + + oos.writeObject(registryOut) + + ByteArrayInputStream baIS = new ByteArrayInputStream(baOS.toByteArray()) + ObjectInputStream ois = new ObjectInputStream(baIS) + + ois.readObject() as TypeDefinitionRegistry + } +} From 3409acfffcdfe2fcb22f4fa78d943849fcdafee5 Mon Sep 17 00:00:00 2001 From: bbaker Date: Tue, 24 Jun 2025 18:19:32 +1000 Subject: [PATCH 2/2] Added a read only copy of a type registry - tweak tests --- src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy b/src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy index b558c97c85..7bdd2fea01 100644 --- a/src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy +++ b/src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy @@ -139,7 +139,7 @@ class SchemaTypeCheckerTest extends Specification { for (String name : resolvingNames) { runtimeBuilder.type(TypeRuntimeWiring.newTypeWiring(name).typeResolver(resolver)) } - return new SchemaTypeChecker().checkTypeRegistry(types, runtimeBuilder.build()) + return new SchemaTypeChecker().checkTypeRegistry(types.readOnly(), runtimeBuilder.build()) } def "test missing type in object"() {