Repository navigation
Conversation
Contributor
Test ReportTest Results
Code Coverage (Java 25)
Changed Class Coverage (4 classes)
|
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend Good Faith's merged-field calculation to general
QueryComplexityLimits. Numeric defaults stay the same. Previously, general limits counted expanded AST occurrences independently of Good Faith's merged counts; now both use merged validation groups.Which limits apply?
Limits apply independently to each operation in the validated document, including unselected operations.
__typenamealone__schemaor__type, whether pure introspection or mixed with ordinary fieldsGood Faith covers the whole mixed operation, including ordinary fields. Aliases/fragments and skipped root introspection selections also activate it. All branches count regardless of
@skip/@include; variables are not coerced.Good Faith permits at most one merged occurrence per operation of each: root
__schema, root__type,__Type.fields,__Type.inputFields,__Type.interfaces, and__Type.possibleTypes. Aliases or distinct parent positions count separately toward these caps.Configuration remains independent:
QueryComplexityLimits.NONEdisables general limits, but enabled Good Faith limits and coordinate caps still apply.How AST versus merged counting changes acceptance
Both object and leaf fields count. AST counting charges for occurrences, including repeated fragment spreads. Merged counting combines compatible fields and their children by response name and common parent at the same position. Aliases and different positions contribute separately.
{ node { name name } }{ node { ...F ...F } }, withfragment F on Node { name }{ a: node { name } b: node { name } }With a general field limit of 2, the first two examples now pass; aliases still fail. Depth remains 2 in all three. Repeated-fragment trees can likewise pass despite exceeding the old expanded-AST budget. No separate expanded-AST field budget remains, including for introspection. Its existing Good Faith merged-count and coordinate protections remain.
Merged counting can also be stricter for abstract types. Suppose
Iis implemented byAandB, all havename: String, and theirchildfields returnI,AandB, respectively;Query.nodereturnsI:{ node { child { a: name b: name c: name } ... on A { child { name } } ... on B { child { name } } } }This counts 9 AST fields but 11 merged validation fields: the shared child selections contribute to both concrete-parent groups, giving 1 for
nodeplus 5 per child group. A field limit of 9 therefore changes from accepted to rejected. These are validation-group counts, not ENO/runtime-type counts or estimates of list lengths and resolver cost.Merge state remains document-wide; count summaries remain per operation. Depth is checked before child collection, and long counts prevent overflow. The separate application-specific
QueryComplexityCalculatorAPI remains available.Performance measurements and validation
Performance
Three async-profiler/JMH optimization cycles led to singleton-set reuse, canonical validation-state keys for the count cache, reusable traversal paths, fewer empty summaries, and reduced map/rule-array allocation.
Full-validation benchmarks cover 16 workloads on a dedicated Intel E3-1270 v6, Java 25: 3 forks, 45 seconds warmup and 30 seconds measurement per fork; introspection uses 75 seconds warmup. Parsing/schema setup are outside timing. Baseline: master
c3884529, before subsequent unrelated upstream changes.Counting is not free: DEFAULT versus NONE in the final implementation adds approximately 5–29% across four controls, including the required merge-pass ordering. Shared validator optimizations explain the net improvements over master. All 189 retained timed forks passed the warmup/measurement-drift review. The committed
GeneralQueryComplexityBenchmarkcontains the fixtures.Protection boundaries and validation
Merged counts bound response structure, not resolver cost, list sizes, or all validation work. Repeated identical fragment spreads can now be accepted where expanded-AST counting rejected them; aliased fanout remains bounded. Parser limits, fragment-cycle detection, error limits and merge caches remain in place. A pre-existing 1,500-fragment acyclic chain still overflows a 1 MiB stack on both master and this implementation, reproduced locally and on Linux.
Full tracked Java 25 tests/TCK and build checks passed (5,999 Spock cases and 190 TCK cases; 57 skips; jcstress excluded), plus 504 targeted cases on each of Java 11/17/21 on the measured revision. No per-class coverage regressions. Tests cover exact limits, aliases/fragments, operation isolation, directives, abstract types, disabled rules, Good Faith, conflict paths, cycles, depth bypasses and count overflow.