FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Consolidate validation rules into single OperationValidator and improve performance by andimarek · Pull Request #4228 · graphql-java/graphql-java · GitHub

/ graphql-java Public

Consolidate validation rules into single OperationValidator and improve performance #4228

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.

Sign up for GitHub

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

Merged
andimarek merged 13 commits into from
Feb 19, 2026
Conversation 4 Commits 13 Checks 10 Files changed
Merged

Consolidate validation rules into single OperationValidator and improve performance #4228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8884d2d
Consolidate 31 validation rule classes into single OperationValidator
andimarek Jan 27, 2026
b3e3f3e
Restore 10 test coverage gaps lost during validation refactor
andimarek Jan 28, 2026
1f6d39d
Move VariablesTypesMatcher and all validation tests from rules sub-pa…
andimarek Jan 28, 2026
142bad7
Improve OperationValidator documentation and naming
andimarek Jan 29, 2026
72b2828
Remove redundant shouldRunDocumentLevelRules() checks
andimarek Jan 29, 2026
e0c5f0b
Add JSpecify @NullMarked annotations to ValidationContext and Operati…
andimarek Jan 29, 2026
b65e8c3
Add JSpecify @NullMarked annotations to TraversalContext
andimarek Jan 29, 2026
23dd3db
Merge origin/master into validation-refactor
andimarek Feb 13, 2026
7a4e1fb
Optimize validation hot paths to reduce collection overhead
andimarek Feb 15, 2026
c0b4b1c
Merge branch 'master' into validation-refactor
andimarek Feb 15, 2026
051f73d
Remove unnecessary null checks on getCodeRegistry()
andimarek Feb 15, 2026
c0cf41c
Revert Locale.getDefault() fallback in GraphQL and ParseAndValidate
andimarek Feb 19, 2026
072165b
Clean up OperationValidator: remove dead code and fix warnings
andimarek Feb 19, 2026
File filter

Filter by extension

Filter by extension .groovy  (38) .java  (48) All 2 file types selected
Deleted files Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
10 changes: 5 additions & 5 deletions src/jmh/java/benchmark/OverlappingFieldValidationBenchmark.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@
import graphql.schema.GraphQLSchema;
import graphql.schema.idl.SchemaGenerator;
import graphql.validation.LanguageTraversal;
import graphql.validation.RulesVisitor;
import graphql.validation.OperationValidationRule;
import graphql.validation.OperationValidator;
import graphql.validation.ValidationContext;
import graphql.validation.ValidationError;
import graphql.validation.ValidationErrorCollector;
import graphql.validation.rules.OverlappingFieldsCanBeMerged;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
Expand All @@ -25,7 +25,6 @@
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;

import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -78,9 +77,10 @@ private List<ValidationError> validateQuery(GraphQLSchema schema, Document docum
ValidationErrorCollector errorCollector = new ValidationErrorCollector();
I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH);
ValidationContext validationContext = new ValidationContext(schema, document, i18n);
OverlappingFieldsCanBeMerged overlappingFieldsCanBeMerged = new OverlappingFieldsCanBeMerged(validationContext, errorCollector);
OperationValidator operationValidator = new OperationValidator(validationContext, errorCollector,
rule -> rule == OperationValidationRule.OVERLAPPING_FIELDS_CAN_BE_MERGED);
LanguageTraversal languageTraversal = new LanguageTraversal();
languageTraversal.traverse(document, new RulesVisitor(validationContext, Collections.singletonList(overlappingFieldsCanBeMerged)));
languageTraversal.traverse(document, operationValidator);
return errorCollector.getErrors();
}
}
10 changes: 5 additions & 5 deletions src/jmh/java/performance/OverlappingFieldValidationPerformance.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
import graphql.schema.GraphQLSchema;
import graphql.schema.idl.SchemaGenerator;
import graphql.validation.LanguageTraversal;
import graphql.validation.RulesVisitor;
import graphql.validation.OperationValidationRule;
import graphql.validation.OperationValidator;
import graphql.validation.ValidationContext;
import graphql.validation.ValidationError;
import graphql.validation.ValidationErrorCollector;
import graphql.validation.rules.OverlappingFieldsCanBeMerged;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
Expand All @@ -27,7 +27,6 @@
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;

import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -151,9 +150,10 @@ private List<ValidationError> validateQuery(GraphQLSchema schema, Document docum
ValidationErrorCollector errorCollector = new ValidationErrorCollector();
I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH);
ValidationContext validationContext = new ValidationContext(schema, document, i18n);
OverlappingFieldsCanBeMerged overlappingFieldsCanBeMerged = new OverlappingFieldsCanBeMerged(validationContext, errorCollector);
OperationValidator operationValidator = new OperationValidator(validationContext, errorCollector,
rule -> rule == OperationValidationRule.OVERLAPPING_FIELDS_CAN_BE_MERGED);
LanguageTraversal languageTraversal = new LanguageTraversal();
languageTraversal.traverse(document, new RulesVisitor(validationContext, Collections.singletonList(overlappingFieldsCanBeMerged)));
languageTraversal.traverse(document, operationValidator);
Assert.assertTrue(errorCollector.getErrors().size() == 0);
return errorCollector.getErrors();
}
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/graphql/GraphQL.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import graphql.execution.preparsed.PreparsedDocumentProvider;
import graphql.language.Document;
import graphql.schema.GraphQLSchema;
import graphql.validation.OperationValidationRule;
import graphql.validation.ValidationError;
import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.NullUnmarked;
Expand Down Expand Up @@ -598,7 +599,7 @@ private List<ValidationError> validate(ExecutionInput executionInput, Document d
InstrumentationContext<List<ValidationError>> validationCtx = nonNullCtx(instrumentation.beginValidation(new InstrumentationValidationParameters(executionInput, document, graphQLSchema), instrumentationState));
validationCtx.onDispatched();

Predicate<Class<?>> validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true);
Predicate<OperationValidationRule> validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true);
Locale locale = executionInput.getLocale();
List<ValidationError> validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale);

Expand Down
20 changes: 11 additions & 9 deletions src/main/java/graphql/ParseAndValidate.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import graphql.parser.ParserEnvironment;
import graphql.parser.ParserOptions;
import graphql.schema.GraphQLSchema;
import graphql.validation.OperationValidationRule;
import graphql.validation.ValidationError;
import graphql.validation.Validator;
import org.jspecify.annotations.NonNull;
import org.jspecify.annotations.NullMarked;

import java.util.List;
Expand All @@ -28,7 +30,7 @@ public class ParseAndValidate {

/**
* This {@link GraphQLContext} hint can be used to supply a Predicate to the Validator so that certain rules can be skipped.
* <p>
*
* This is an internal capability that you should use at your own risk. While we intend for this to be present for some time, the validation
* rule class names may change, as may this mechanism.
*/
Expand All @@ -44,7 +46,7 @@ public class ParseAndValidate {
*
* @return a result object that indicates how this operation went
*/
public static ParseAndValidateResult parseAndValidate(GraphQLSchema graphQLSchema, ExecutionInput executionInput) {
public static ParseAndValidateResult parseAndValidate(@NonNull GraphQLSchema graphQLSchema, @NonNull ExecutionInput executionInput) {
ParseAndValidateResult result = parse(executionInput);
if (!result.isFailure()) {
List<ValidationError> errors = validate(graphQLSchema, assertNotNull(result.getDocument(), "Parse result document cannot be null when parse succeeded"), executionInput.getLocale());
Expand All @@ -60,7 +62,7 @@ public static ParseAndValidateResult parseAndValidate(GraphQLSchema graphQLSchem
*
* @return a result object that indicates how this operation went
*/
public static ParseAndValidateResult parse(ExecutionInput executionInput) {
public static ParseAndValidateResult parse(@NonNull ExecutionInput executionInput) {
try {
//
// we allow the caller to specify new parser options by context
Expand Down Expand Up @@ -89,8 +91,8 @@ public static ParseAndValidateResult parse(ExecutionInput executionInput) {
*
* @return a result object that indicates how this operation went
*/
public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument, Locale locale) {
return validate(graphQLSchema, parsedDocument, ruleClass -> true, locale);
public static List<ValidationError> validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Locale locale) {
return validate(graphQLSchema, parsedDocument, rule -> true, locale);
}

/**
Expand All @@ -101,8 +103,8 @@ public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Docume
*
* @return a result object that indicates how this operation went
*/
public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument) {
return validate(graphQLSchema, parsedDocument, ruleClass -> true, Locale.getDefault());
public static List<ValidationError> validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument) {
return validate(graphQLSchema, parsedDocument, rule -> true, Locale.getDefault());
}

/**
Expand All @@ -115,7 +117,7 @@ public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Docume
*
* @return a result object that indicates how this operation went
*/
public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument, Predicate<Class<?>> rulePredicate, Locale locale) {
public static List<ValidationError> validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate<OperationValidationRule> rulePredicate, @NonNull Locale locale) {
Validator validator = new Validator();
return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, locale);
}
Expand All @@ -129,7 +131,7 @@ public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Docume
*
* @return a result object that indicates how this operation went
*/
public static List<ValidationError> validate(GraphQLSchema graphQLSchema, Document parsedDocument, Predicate<Class<?>> rulePredicate) {
public static List<ValidationError> validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate<OperationValidationRule> rulePredicate) {
Validator validator = new Validator();
return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, Locale.getDefault());
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/MergedField.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
* </pre>
* These examples make clear that you need to consider all merged fields together to have the full picture.
* <p>
* The actual logic when fields can be successfully merged together is implemented in {#graphql.validation.rules.OverlappingFieldsCanBeMerged}
* The actual logic when fields can be successfully merged together is implemented in {#graphql.validation.OperationValidator}
*/
@PublicApi
@NullMarked
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/normalized/ENFMerger.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private static boolean compareWithoutChildren(ExecutableNormalizedField one, Exe
return true;
}

// copied from graphql.validation.rules.OverlappingFieldsCanBeMerged
// copied from graphql.validation.OperationValidator
private static boolean sameArguments(List<Argument> arguments1, List<Argument> arguments2) {
if (arguments1.size() != arguments2.size()) {
return false;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/normalized/nf/NormalizedFieldsMerger.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ private static boolean compareWithoutChildren(NormalizedField one, NormalizedFie
return true;
}

// copied from graphql.validation.rules.OverlappingFieldsCanBeMerged
// copied from graphql.validation.OperationValidator
private static boolean sameArguments(List<Argument> arguments1, List<Argument> arguments2) {
if (arguments1.size() != arguments2.size()) {
return false;
Expand Down
20 changes: 17 additions & 3 deletions src/main/java/graphql/schema/GraphQLSchema.java
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ public Map<String, GraphQLDirective> getDirectivesByName() {
*
* @return the directive or null if there is not one with that name
*/
public GraphQLDirective getDirective(String directiveName) {
public @Nullable GraphQLDirective getDirective(String directiveName) {
return directiveDefinitionsHolder.getDirective(directiveName);
}

Expand Down Expand Up @@ -1215,6 +1215,7 @@ public FastBuilder(GraphQLCodeRegistry.Builder codeRegistryBuilder,
* multiple FastBuilder instances.
*
* @param type the named type to add
*
* @return this builder for chaining
*/
public FastBuilder addType(GraphQLNamedType type) {
Expand Down Expand Up @@ -1264,6 +1265,7 @@ public FastBuilder addType(GraphQLNamedType type) {
* All non-root types added via this method will be included in {@link GraphQLSchema#getAdditionalTypes()}.
*
* @param types the named types to add
*
* @return this builder for chaining
*/
public FastBuilder addTypes(Collection<? extends GraphQLNamedType> types) {
Expand All @@ -1275,6 +1277,7 @@ public FastBuilder addTypes(Collection<? extends GraphQLNamedType> types) {
* Adds a directive definition to the schema.
*
* @param directive the directive to add
*
* @return this builder for chaining
*/
public FastBuilder additionalDirective(GraphQLDirective directive) {
Expand All @@ -1296,6 +1299,7 @@ public FastBuilder additionalDirective(GraphQLDirective directive) {
* Adds multiple directive definitions to the schema.
*
* @param directives the directives to add
*
* @return this builder for chaining
*/
public FastBuilder additionalDirectives(Collection<? extends GraphQLDirective> directives) {
Expand All @@ -1307,6 +1311,7 @@ public FastBuilder additionalDirectives(Collection<? extends GraphQLDirective> d
* Adds a schema-level directive (deprecated, use applied directives).
*
* @param directive the directive to add
*
* @return this builder for chaining
*/
public FastBuilder withSchemaDirective(GraphQLDirective directive) {
Expand All @@ -1318,6 +1323,7 @@ public FastBuilder withSchemaDirective(GraphQLDirective directive) {
* Adds multiple schema-level directives.
*
* @param directives the directives to add
*
* @return this builder for chaining
*/
public FastBuilder withSchemaDirectives(Collection<? extends GraphQLDirective> directives) {
Expand All @@ -1329,6 +1335,7 @@ public FastBuilder withSchemaDirectives(Collection<? extends GraphQLDirective> d
* Adds a schema-level applied directive.
*
* @param applied the applied directive to add
*
* @return this builder for chaining
*/
public FastBuilder withSchemaAppliedDirective(GraphQLAppliedDirective applied) {
Expand All @@ -1342,6 +1349,7 @@ public FastBuilder withSchemaAppliedDirective(GraphQLAppliedDirective applied) {
* Adds multiple schema-level applied directives.
*
* @param appliedList the applied directives to add
*
* @return this builder for chaining
*/
public FastBuilder withSchemaAppliedDirectives(Collection<? extends GraphQLAppliedDirective> appliedList) {
Expand All @@ -1355,6 +1363,7 @@ public FastBuilder withSchemaAppliedDirectives(Collection<? extends GraphQLAppli
* Sets the schema definition (AST).
*
* @param def the schema definition
*
* @return this builder for chaining
*/
public FastBuilder definition(SchemaDefinition def) {
Expand All @@ -1366,6 +1375,7 @@ public FastBuilder definition(SchemaDefinition def) {
* Sets the schema extension definitions (AST).
*
* @param defs the extension definitions
*
* @return this builder for chaining
*/
public FastBuilder extensionDefinitions(List<SchemaExtensionDefinition> defs) {
Expand All @@ -1377,6 +1387,7 @@ public FastBuilder extensionDefinitions(List<SchemaExtensionDefinition> defs) {
* Sets the schema description.
*
* @param description the description
*
* @return this builder for chaining
*/
public FastBuilder description(String description) {
Expand All @@ -1388,6 +1399,7 @@ public FastBuilder description(String description) {
* Sets the introspection schema type.
*
* @param type the introspection schema type
*
* @return this builder for chaining
*/
public FastBuilder introspectionSchemaType(GraphQLObjectType type) {
Expand All @@ -1399,6 +1411,7 @@ public FastBuilder introspectionSchemaType(GraphQLObjectType type) {
* Enables or disables schema validation.
*
* @param enabled true to enable validation, false to disable
*
* @return this builder for chaining
*/
public FastBuilder withValidation(boolean enabled) {
Expand All @@ -1415,9 +1428,10 @@ public FastBuilder withValidation(boolean enabled) {
* should not be reused with another FastBuilder.
*
* @return the built schema
*
* @throws InvalidSchemaException if validation is enabled and the schema is invalid
* @throws AssertException if a type reference cannot be resolved or if an interface/union
* type is missing a type resolver
* @throws AssertException if a type reference cannot be resolved or if an interface/union
* type is missing a type resolver
*/
public GraphQLSchema build() {
// Validate type resolvers for all interfaces and unions
Expand Down
Loading
Loading
Back | FazBrowse Home | New Git URL