Consolidate validation rules into single OperationValidator and improve performance by andimarek · Pull Request #4228 · graphql-java/graphql-java
Replace the AbstractRule + RulesVisitor + 31 individual rule class architecture with a single OperationValidator class that implements DocumentVisitor directly. BREAKING CHANGE: The rule filtering predicate type changes from Predicate<Class<?>> to Predicate<OperationValidationRule> in Validator, ParseAndValidate, and GraphQL. Code that filters validation rules by class reference must migrate to the new OperationValidationRule enum. - Create OperationValidationRule enum with 31 values (one per rule) - Create OperationValidator implementing DocumentVisitor with all validation logic consolidated from the 31 rule classes - Simplify Validator to create OperationValidator directly - Update ParseAndValidate and GraphQL predicate types - Delete AbstractRule, RulesVisitor, and all 31 rule classes (keep VariablesTypesMatcher as utility) - Convert all test files from mock-based rule instantiation to integration tests or OperationValidator with rule predicates - Update JMH benchmarks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add integration tests replacing removed mock-based tests: - Per-operation fragment visiting (RulesVisitorTest) - Invalid input object field, list, nested list, and simple list types (ArgumentsOfCorrectTypeTest) - Null value for non-null input object field (ArgumentsOfCorrectTypeTest) - Unknown type on inline fragment not triggering composite type error (FragmentsOnCompositeTypeTest) - Directive argument scope isolation (KnownArgumentNamesTest) - Defaulted non-null field and directive arguments (ProvidedNonNullArgumentsTest) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ckage to validation package The rules sub-package is no longer needed since all 31 rule classes were consolidated into OperationValidator. Move the remaining utility class (VariablesTypesMatcher) and all 32 test files into the parent graphql.validation package, then delete the empty rules directories. Also remove stale JSpecifyAnnotationsCheck entries for deleted rule classes and update comments referencing the old package. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename methods for clarity: - shouldRunNonFragmentSpreadChecks() -> shouldRunDocumentLevelRules() - shouldRunFragmentSpreadChecks() -> shouldRunOperationScopedRules() - fragmentSpreadVisitDepth -> fragmentRetraversalDepth - Add comprehensive class Javadoc explaining: - Two independent state variables and their three valid combinations - Visual matrices showing when each rule category runs - Step-by-step traversal example with state at each step - Add detailed Javadoc to OperationValidationRule enum categorizing all rules by their traversal behavior - Simplify code by removing duplicate checks and redundant comments Co-authored-by: Cursor <cursoragent@cursor.com>
These checks are always true in methods that are only called at document level (depth=0): - checkDocument(): Document node visited once at start - checkOperationDefinition(): Operations are never inside fragments - checkVariableDefinition(): Variable definitions only exist in operations - documentFinished(): Called when leaving Document Co-authored-by: Cursor <cursoragent@cursor.com>
…onValidator ValidationContext: - Add @NullMarked at class level - Add @nullable to methods that can return null based on traversal state: getFragment, getParentType, getInputType, getDefaultValue, getFieldDef, getDirective, getArgument, getOutputType, getQueryPath OperationValidator: - Add @NullMarked at class level - Add @nullable to methods that can return null: getQueryPath, requireSameNameAndArguments, findArgumentByName, requireSameOutputTypeShape - Add @nullable to parameters that can be null: addError (SourceLocation), mkNotSameTypeError (typeA, typeB), overlappingFieldsImpl, overlappingFields_collectFields, overlappingFields_collectFieldsForInlineFragment, overlappingFields_collectFieldsForField, sameType, sameArguments - Add @nullable to fields that can be null: variableDefinitionMap, FieldAndType.graphQLType Co-authored-by: Cursor <cursoragent@cursor.com>
- Add @NullMarked at class level - Add @nullable to fields: directive, argument - Add @nullable to public methods that can return null: getOutputType, getParentType, getInputType, getDefaultValue, getFieldDef, getQueryPath, getDirective, getArgument - Add @nullable to private methods that can return null: lastElement, find, getFieldDef(schema, parentType, field), getNullableType - Add @nullable to parameters that accept null: addOutputType, addParentType, addInputType, addDefaultValue, addFieldDef, getNullableType Co-authored-by: Cursor <cursoragent@cursor.com>
Fix NullAway errors from master's @NullMarked additions to GraphQLSchema and GraphQLTypeUtil. Add null checks in OperationValidator and TraversalContext for nullable getCodeRegistry(), unwrapAll(), and type comparison methods. Update JSpecifyAnnotationsCheck exemption list to match master's annotated classes, removing deleted DeferDirective rule entries. Add @NullMarked to ParseAndValidate to match master.
Replace HashMap/LinkedHashMap allocations with linear scans for small argument lists, single-pass partitioning in groupByCommonParents, pre-sized maps, computeIfAbsent, HashSet for fragment tracking, and ArrayList over LinkedList. Short-circuit isRuleEnabled when all rules are enabled. ~12% improvement on ValidatorBenchmark (largeSchema4: 7.86→6.94ms, manyFragments: 5.83→5.16ms). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
andimarek
changed the title
Consolidate validation rules into single OperationValidator
Consolidate validation rules into single OperationValidator and improve performance
GraphQLSchema.getCodeRegistry() is no longer @nullable, so these guards are unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Keep the original behavior of passing locale through as-is from ExecutionInput.getLocale() without falling back to Locale.getDefault(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused import (GraphQLCodeRegistry) - Remove unused method parameters (checkSelectionSet, leaveOperationDefinition, leaveSelectionSet, documentFinished, validateUniqueArgumentNames_directive) - Remove always-false null check (validationContext in isExperimentalApiKeyEnabled) - Remove redundant @SuppressWarnings("deprecation") - Remove always-false value == null check in validateProvidedNonNullArguments - Simplify typeA/typeB null check (typeA never null in context) - Rename sameType -> notSameType, doTypesOverlap -> typesDoNotOverlap (always inverted) - Extract common validateUniqueArgumentNames to deduplicate field/directive variants - Convert indexed for-loop to enhanced for-each - Simplify ifArgumentMightBeFalse return - Remove dead query-type branch (schema always has query type) - Add null safety for getSourceLocation() and StringValue.getValue() - Add @nullable to GraphQLSchema.getDirective() (can return null per javadoc) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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