Skip to content

Commit 131f169

Browse files
Raymie Stataclaude
Raymie Stata
and
claude
committed
Promote FastSchemaGenerator to @experimentalapi with safer defaults
- Change from @internal to @experimentalapi, making it part of the public API - Add @NullMarked and @nullable annotations for null safety - Enable schema validation by default (previously skipped for performance) - Add withValidation option to SchemaGenerator.Options for explicit opt-out - Update documentation to reference FastBuilder limitations - Add tests for validation behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent cd32b5c commit 131f169

File tree

6 files changed

+136
-26
lines changed

6 files changed

+136
-26
lines changed

‎src/jmh/java/benchmark/BuildSchemaBenchmark.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package benchmark;
22

3-
import graphql.schema.GraphQLSchema;
43
import graphql.schema.idl.FastSchemaGenerator;
54
import graphql.schema.idl.RuntimeWiring;
65
import graphql.schema.idl.SchemaGenerator;
@@ -47,6 +46,9 @@ public void benchmarkBuildSchemaAvgTime(Blackhole blackhole) {
4746
@BenchmarkMode(Mode.AverageTime)
4847
@OutputTimeUnit(TimeUnit.MILLISECONDS)
4948
public void benchmarkBuildSchemaAvgTimeFast(Blackhole blackhole) {
50-
blackhole.consume(new FastSchemaGenerator().makeExecutableSchema(registry, RuntimeWiring.MOCKED_WIRING));
49+
blackhole.consume(new FastSchemaGenerator().makeExecutableSchema(
50+
SchemaGenerator.Options.defaultOptions().withValidation(false),
51+
registry,
52+
RuntimeWiring.MOCKED_WIRING));
5153
}
5254
}

‎src/jmh/java/benchmark/CreateSchemaBenchmark.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ private static GraphQLSchema createSchema(String sdl) {
5252

5353
private static GraphQLSchema createSchemaFast(String sdl) {
5454
TypeDefinitionRegistry registry = new SchemaParser().parse(sdl);
55-
return new FastSchemaGenerator().makeExecutableSchema(registry, RuntimeWiring.MOCKED_WIRING);
55+
return new FastSchemaGenerator().makeExecutableSchema(
56+
SchemaGenerator.Options.defaultOptions().withValidation(false),
57+
registry,
58+
RuntimeWiring.MOCKED_WIRING);
5659
}
5760

5861
@SuppressWarnings("InfiniteLoopStatement")

‎src/main/java/graphql/schema/idl/FastSchemaGenerator.java‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package graphql.schema.idl;
22

3+
import graphql.ExperimentalApi;
34
import graphql.GraphQLError;
4-
import graphql.Internal;
55
import graphql.language.OperationTypeDefinition;
6+
import org.jspecify.annotations.NullMarked;
7+
import org.jspecify.annotations.Nullable;
68
import graphql.schema.GraphQLCodeRegistry;
79
import graphql.schema.GraphQLDirective;
810
import graphql.schema.GraphQLNamedType;
@@ -17,22 +19,27 @@
1719
import static graphql.schema.idl.SchemaGeneratorHelper.buildDescription;
1820

1921
/**
20-
* A schema generator that uses GraphQLSchema.FastBuilder for improved performance.
21-
* This is intended for benchmarking and performance testing purposes.
22+
* A schema generator that uses {@link GraphQLSchema.FastBuilder} to construct the schema.
23+
* {@link GraphQLSchema.FastBuilder} has a number of important limitations, so please read
24+
* its documentation carefully to understand if you should use this instead of the standard
25+
* {@link SchemaGenerator}.
26+
*
27+
* @see GraphQLSchema.FastBuilder
28+
* @see SchemaGenerator
2229
*/
23-
@Internal
30+
@ExperimentalApi
31+
@NullMarked
2432
public class FastSchemaGenerator {
2533

2634
private final SchemaTypeChecker typeChecker = new SchemaTypeChecker();
2735
private final SchemaGeneratorHelper schemaGeneratorHelper = new SchemaGeneratorHelper();
2836

2937
/**
3038
* Creates an executable schema from a TypeDefinitionRegistry using FastBuilder.
31-
* This method is optimized for performance and skips validation by default.
3239
*
3340
* @param typeRegistry the type definition registry
3441
* @param wiring the runtime wiring
35-
* @return an executable schema
42+
* @return a validated, executable schema
3643
*/
3744
public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) {
3845
return makeExecutableSchema(SchemaGenerator.Options.defaultOptions(), typeRegistry, wiring);
@@ -46,7 +53,9 @@ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, R
4653
* @param wiring the runtime wiring
4754
* @return an executable schema
4855
*/
49-
public GraphQLSchema makeExecutableSchema(SchemaGenerator.Options options, TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) {
56+
public GraphQLSchema makeExecutableSchema(SchemaGenerator.Options options,
57+
TypeDefinitionRegistry typeRegistry,
58+
RuntimeWiring wiring) {
5059
// Make a copy and add default directives
5160
TypeDefinitionRegistry typeRegistryCopy = new TypeDefinitionRegistry();
5261
typeRegistryCopy.merge(typeRegistry);
@@ -122,17 +131,15 @@ private GraphQLSchema makeExecutableSchemaImpl(ImmutableTypeDefinitionRegistry t
122131
// Add all directive definitions
123132
fastBuilder.additionalDirectives(additionalDirectives);
124133

125-
// Add schema description if present
134+
// Add schema description and definition if present
126135
typeRegistry.schemaDefinition().ifPresent(schemaDefinition -> {
127136
String description = buildDescription(buildCtx, schemaDefinition, schemaDefinition.getDescription());
128137
fastBuilder.description(description);
138+
fastBuilder.definition(schemaDefinition);
129139
});
130140

131-
// Add schema definition
132-
fastBuilder.definition(typeRegistry.schemaDefinition().orElse(null));
133-
134-
// Disable validation for performance
135-
fastBuilder.withValidation(false);
141+
// Configure validation
142+
fastBuilder.withValidation(options.isWithValidation());
136143

137144
return fastBuilder.build();
138145
}
@@ -147,7 +154,7 @@ private String getOperationTypeName(Map<String, OperationTypeDefinition> operati
147154
return defaultTypeName;
148155
}
149156

150-
private GraphQLObjectType findOperationType(Set<GraphQLNamedType> types, String typeName) {
157+
private @Nullable GraphQLObjectType findOperationType(Set<GraphQLNamedType> types, String typeName) {
151158
for (GraphQLNamedType type : types) {
152159
if (type instanceof GraphQLObjectType) {
153160
GraphQLObjectType objectType = (GraphQLObjectType) type;

‎src/main/java/graphql/schema/idl/SchemaGenerator.java‎

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package graphql.schema.idl;
22

3+
import graphql.ExperimentalApi;
34
import graphql.GraphQLError;
45
import graphql.PublicApi;
56
import graphql.language.OperationTypeDefinition;
@@ -98,6 +99,9 @@ public GraphQLSchema makeExecutableSchema(TypeDefinitionRegistry typeRegistry, R
9899
* @throws SchemaProblem if there are problems in assembling a schema such as missing type resolvers or no operations defined
99100
*/
100101
public GraphQLSchema makeExecutableSchema(Options options, TypeDefinitionRegistry typeRegistry, RuntimeWiring wiring) throws SchemaProblem {
102+
if (!options.isWithValidation()) {
103+
throw new IllegalArgumentException("SchemaGenerator does not support disabling validation. Use FastSchemaGenerator instead.");
104+
}
101105

102106
TypeDefinitionRegistry typeRegistryCopy = new TypeDefinitionRegistry();
103107
typeRegistryCopy.merge(typeRegistry);
@@ -167,11 +171,18 @@ public static class Options {
167171
private final boolean useCommentsAsDescription;
168172
private final boolean captureAstDefinitions;
169173
private final boolean useAppliedDirectivesOnly;
174+
private final boolean withValidation;
170175

171176
Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly) {
177+
this(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, true);
178+
}
179+
180+
@ExperimentalApi
181+
Options(boolean useCommentsAsDescription, boolean captureAstDefinitions, boolean useAppliedDirectivesOnly, boolean withValidation) {
172182
this.useCommentsAsDescription = useCommentsAsDescription;
173183
this.captureAstDefinitions = captureAstDefinitions;
174184
this.useAppliedDirectivesOnly = useAppliedDirectivesOnly;
185+
this.withValidation = withValidation;
175186
}
176187

177188
public boolean isUseCommentsAsDescription() {
@@ -186,8 +197,13 @@ public boolean isUseAppliedDirectivesOnly() {
186197
return useAppliedDirectivesOnly;
187198
}
188199

200+
@ExperimentalApi
201+
public boolean isWithValidation() {
202+
return withValidation;
203+
}
204+
189205
public static Options defaultOptions() {
190-
return new Options(true, true, false);
206+
return new Options(true, true, false, true);
191207
}
192208

193209
/**
@@ -200,7 +216,7 @@ public static Options defaultOptions() {
200216
* @return a new Options object
201217
*/
202218
public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) {
203-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
219+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, withValidation);
204220
}
205221

206222
/**
@@ -212,7 +228,7 @@ public Options useCommentsAsDescriptions(boolean useCommentsAsDescription) {
212228
* @return a new Options object
213229
*/
214230
public Options captureAstDefinitions(boolean captureAstDefinitions) {
215-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
231+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, withValidation);
216232
}
217233

218234
/**
@@ -225,7 +241,23 @@ public Options captureAstDefinitions(boolean captureAstDefinitions) {
225241
* @return a new Options object
226242
*/
227243
public Options useAppliedDirectivesOnly(boolean useAppliedDirectivesOnly) {
228-
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly);
244+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, withValidation);
245+
}
246+
247+
/**
248+
* Controls whether the generated schema is validated after construction.
249+
* <p>
250+
* <b>Note:</b> This option is only supported by {@link FastSchemaGenerator}.
251+
* The standard {@link SchemaGenerator} will throw {@link IllegalArgumentException}
252+
* if validation is disabled.
253+
*
254+
* @param withValidation true to enable validation (default), false to skip validation
255+
*
256+
* @return a new Options object
257+
*/
258+
@ExperimentalApi
259+
public Options withValidation(boolean withValidation) {
260+
return new Options(useCommentsAsDescription, captureAstDefinitions, useAppliedDirectivesOnly, withValidation);
229261
}
230262
}
231-
}
263+
}

‎src/test/groovy/graphql/schema/idl/FastSchemaGeneratorTest.groovy‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,16 @@ package graphql.schema.idl
22

33
import graphql.schema.GraphQLSchema
44
import graphql.schema.idl.errors.SchemaProblem
5+
import graphql.schema.validation.InvalidSchemaException
56
import spock.lang.Specification
67

8+
/**
9+
* Tests for {@link FastSchemaGenerator}.
10+
*
11+
* Note: {@link GraphQLSchema.FastBuilder} is subject to extensive testing directly.
12+
* The tests in this file are intended to test {@code FastSchemaGenerator} specifically
13+
* and not the underlying builder.
14+
*/
715
class FastSchemaGeneratorTest extends Specification {
816

917
def "can create simple schema using FastSchemaGenerator"() {
@@ -138,7 +146,7 @@ class FastSchemaGeneratorTest extends Specification {
138146

139147
// Regression tests to ensure FastSchemaGenerator behaves like SchemaGenerator
140148

141-
def "should throw SchemaProblem for missing type reference"() {
149+
def "should throw SchemaProblem for missing type reference even with validation disabled"() {
142150
given:
143151
def sdl = '''
144152
type Query {
@@ -148,6 +156,7 @@ class FastSchemaGeneratorTest extends Specification {
148156

149157
when:
150158
new FastSchemaGenerator().makeExecutableSchema(
159+
SchemaGenerator.Options.defaultOptions().withValidation(false),
151160
new SchemaParser().parse(sdl),
152161
RuntimeWiring.MOCKED_WIRING
153162
)
@@ -156,7 +165,7 @@ class FastSchemaGeneratorTest extends Specification {
156165
thrown(SchemaProblem)
157166
}
158167

159-
def "should throw SchemaProblem for duplicate field definitions"() {
168+
def "should throw SchemaProblem for duplicate field definitions even with validation disabled"() {
160169
given:
161170
def sdl = '''
162171
type Query {
@@ -167,6 +176,7 @@ class FastSchemaGeneratorTest extends Specification {
167176

168177
when:
169178
new FastSchemaGenerator().makeExecutableSchema(
179+
SchemaGenerator.Options.defaultOptions().withValidation(false),
170180
new SchemaParser().parse(sdl),
171181
RuntimeWiring.MOCKED_WIRING
172182
)
@@ -175,7 +185,7 @@ class FastSchemaGeneratorTest extends Specification {
175185
thrown(SchemaProblem)
176186
}
177187

178-
def "should throw SchemaProblem for invalid interface implementation"() {
188+
def "should throw SchemaProblem for invalid interface implementation even with validation disabled"() {
179189
given:
180190
def sdl = '''
181191
type Query {
@@ -193,12 +203,14 @@ class FastSchemaGeneratorTest extends Specification {
193203

194204
when:
195205
new FastSchemaGenerator().makeExecutableSchema(
206+
SchemaGenerator.Options.defaultOptions().withValidation(false),
196207
new SchemaParser().parse(sdl),
197208
RuntimeWiring.MOCKED_WIRING
198209
)
199210

200211
then:
201212
// User claims to implement Node but doesn't have the required 'id' field
213+
// This is caught by SchemaTypeChecker, not SchemaValidator
202214
thrown(SchemaProblem)
203215
}
204216

@@ -277,4 +289,44 @@ class FastSchemaGeneratorTest extends Specification {
277289
assert standardTypeNames.contains(introspectionType) : "Extra introspection type: $introspectionType"
278290
}
279291
}
292+
293+
// Validation tests - test that 2-arg method validates and 4-arg with validation=false skips validation
294+
295+
def "default makeExecutableSchema validates and throws InvalidSchemaException for non-null self-referencing input type"() {
296+
given:
297+
// Non-null self-reference in input type is impossible to satisfy
298+
def sdl = '''
299+
type Query { test(input: BadInput): String }
300+
input BadInput { self: BadInput! }
301+
'''
302+
303+
when:
304+
new FastSchemaGenerator().makeExecutableSchema(
305+
new SchemaParser().parse(sdl),
306+
RuntimeWiring.MOCKED_WIRING
307+
)
308+
309+
then:
310+
thrown(InvalidSchemaException)
311+
}
312+
313+
def "3-arg makeExecutableSchema with withValidation=false allows non-null self-referencing input type"() {
314+
given:
315+
// Non-null self-reference in input type - passes without validation
316+
def sdl = '''
317+
type Query { test(input: BadInput): String }
318+
input BadInput { self: BadInput! }
319+
'''
320+
321+
when:
322+
def schema = new FastSchemaGenerator().makeExecutableSchema(
323+
SchemaGenerator.Options.defaultOptions().withValidation(false),
324+
new SchemaParser().parse(sdl),
325+
RuntimeWiring.MOCKED_WIRING
326+
)
327+
328+
then:
329+
notThrown(InvalidSchemaException)
330+
schema != null
331+
}
280332
}

‎src/test/groovy/graphql/schema/idl/SchemaGeneratorTest.groovy‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2525,7 +2525,7 @@ class SchemaGeneratorTest extends Specification {
25252525
type Query {
25262526
f(arg : OneOfInputType) : String
25272527
}
2528-
2528+
25292529
input OneOfInputType @oneOf {
25302530
a : String
25312531
b : String
@@ -2541,4 +2541,18 @@ class SchemaGeneratorTest extends Specification {
25412541
inputObjectType.isOneOf()
25422542
inputObjectType.hasAppliedDirective("oneOf")
25432543
}
2544+
2545+
def "should throw IllegalArgumentException when withValidation is false"() {
2546+
given:
2547+
def sdl = '''
2548+
type Query { hello: String }
2549+
'''
2550+
def options = SchemaGenerator.Options.defaultOptions().withValidation(false)
2551+
2552+
when:
2553+
new SchemaGenerator().makeExecutableSchema(options, new SchemaParser().parse(sdl), RuntimeWiring.MOCKED_WIRING)
2554+
2555+
then:
2556+
thrown(IllegalArgumentException)
2557+
}
25442558
}

0 commit comments

Comments
 (0)