This adds support for QueryAppliedDirective on operations and documents by bbakerman · Pull Request #4297 · graphql-java/graphql-java · GitHub

Test Report

Test Results

Java Version	Total	Passed	Failed	Errors	Skipped
Java 11	5706 (+4 🟢)	5650 (+4 🟢)	0 (±0)	0 (±0)	56 (±0)
Java 17	5706 (+4 🟢)	5649 (+4 🟢)	0 (±0)	0 (±0)	57 (±0)
Java 21	5706 (+4 🟢)	5649 (+4 🟢)	0 (±0)	0 (±0)	57 (±0)
Java 25	5706 (+4 🟢)	5649 (+4 🟢)	0 (±0)	0 (±0)	57 (±0)
jcstress	32 (±0)	32 (±0)	0 (±0)	0 (±0)	0 (±0)
Total	22856 (+16 🟢)	22629 (+16 🟢)	0 (±0)	0 (±0)	227 (±0)

Code Coverage (Java 25)

Metric	Covered	Missed	Coverage	vs Master
Lines	28773	3122	90.2%	±0.0%
Branches	8354	1508	84.7%	±0.0%
Methods	7698	1223	86.3%	±0.0%

Changed Class Coverage (7 classes)

Class	Line	Branch	Method
g.e.d.DirectivesResolver	±0.0%	+8.3% 🟢	±0.0%
g.e.d.OperationDirectivesResolver	+100.0% 🟢	+100.0% 🟢	+100.0% 🟢
g.e.Execution	+0.3% 🟢	±0.0%	±0.0%
g.e.ExecutionContext	+0.1% 🟢	±0.0%	+0.2% 🟢
g.n.ExecutableNormalizedOperation	+0.8% 🟢	±0.0%	+1.0% 🟢
g.n.ExecutableNormalizedOperationFactory	-8.0% 🔴	±0.0%	-12.5% 🔴
g.v.OperationValidator	±0.0%	+0.2% 🟢	±0.0%

ExecutableNormalizedOperationFactory — method details

Method	Line	Branch
createExecutableNormalizedOperation	0.0% (-100.0% 🔴)	—

Full HTML report: build artifact jacoco-html-report

Updated: 2026-03-09 23:31:31 UTC

…ts - tests added

These methods used to be in QueryDirectivesImpl but I moved them here to be common and used now in two places

…ts - refactor

The new extracting class is Internal - exposed later via ExecutionContext and ENO

…ts - use Immutable signature

…ts - used field to make it cheaper

moved build down to the bottom for code OCD reasons

here the ENO has the same values

moved methods so they can be re-used

…ts - made it a supplier for performance

This is a memoizing supplier for performance reasons. if you never ask for the directives - no work is done - only the allocation of a interThreadMemoize object

rejigged this code to a maker method

fixed a bug here - graphql context and locale was never respected before

move down

…ts -added more tests

…on-directives

import

import not correct :-)

fixed

…ts - PR feedback

…' into extract-out-operation-directives

Coverage Drop Analysis

The "Per-Class Coverage Gate" CI check is failing due to two classes showing coverage regressions. Here's a deep analysis of each:

1. QueryDirectivesImpl — Line: 97.3% (was 97.6%), Branch: 75.0% (was 80.0%), Method: 93.8% (was 94.4%)

Root cause: Code extraction shifted coverage ratios — no actual coverage was lost.

This PR moves two private methods (toAppliedDirective and toAppliedArgument, 18 source lines) from QueryDirectivesImpl to DirectivesResolver as public methods. The call site at line 83 changes from toAppliedDirective(resolvedDirective) to directivesResolver.toAppliedDirective(resolvedDirective).

These extracted methods were fully covered by existing tests. Removing well-covered lines from a class while the same 2 uncovered lines remain causes the coverage percentage to drop even though no test coverage was actually lost.

The math in detail

Baseline from test-baseline.json:

Line: 82 covered, 2 missed → total 84 → 82/84 = 97.6%
Branch: 8 covered, 2 missed → total 10 → 8/10 = 80.0%
Method: 17 covered, 1 missed → total 18 → 17/18 = 94.4%

What was removed — the two methods contained (as counted by JaCoCo):

~10 executable lines, all covered
2 branches (the for loop's empty-vs-nonempty paths in toAppliedDirective), all covered
2 methods, all covered

After the PR — subtract the removed covered items from both numerator and denominator:

Metric	Before	Removed (all covered)	After	Reported
Line	82 / 84 = 97.6%	10 covered lines	(82−10) / (84−10) = 72/74 = 97.3%	97.3% ✓
Branch	8 / 10 = 80.0%	2 covered branches	(8−2) / (10−2) = 6/8 = 75.0%	75.0% ✓
Method	17 / 18 = 94.4%	2 covered methods	(17−2) / (18−2) = 15/16 = 93.75%	93.8% ✓

Why does this happen? If a class has coverage C/T and you remove R items that were all covered:

New coverage = (C − R) / (T − R)

This is less than C/T whenever R/R > C/T, i.e. whenever 100% > C/T — which is true for any class not already at 100%. Removing perfectly-covered code from a not-perfectly-covered class always lowers the percentage, even though nothing became less tested. The 2 missed lines/branches now represent a larger fraction of a smaller total (2/74 instead of 2/84, 2/8 instead of 2/10).

This is a mathematical artifact of the ratio, not a real regression. The extracted code is now tested via DirectivesResolver and the new OperationDirectivesResolverTest.

2. ExecutableNormalizedOperationFactory (outer class) — Line: 84.0% (was 92.0%), Method: 75.0% (was 87.5%)

Root cause: The 4-arg convenience overload of createExecutableNormalizedOperation(schema, operationDefinition, fragments, coercedVars) (line 311-320) lost its only caller.

Previously, ExecutionContext constructed the normalized query tree by calling this 4-arg overload:

ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(
    graphQLSchema, operationDefinition, fragmentsByName, coercedVariables);

This PR changes ExecutionContext.mkExecutableNormalizedOperation() to call the 5-arg overload instead, passing Options with graphQLContext and locale:

Options options = Options.defaultOptions().graphQLContext(graphQLContext).locale(locale);
return createExecutableNormalizedOperation(
    graphQLSchema, operationDefinition, fragmentsByName, coercedVariables, options);

Integration tests that execute full queries go through ExecutionContext, which was the only path exercising the 4-arg (schema, OperationDefinition, fragments, coercedVars) overload. Direct unit tests in ExecutableNormalizedOperationFactoryTest use the (schema, Document, operationName, coercedVars, options) overload instead. This matches the numbers exactly:

Baseline: 23 covered, 2 missed lines (92.0%) / 7 covered, 1 missed method (87.5%)

Line:   (23−2) / (25+0) = 21/25 = 84.0%   ← 2 lines in the 4-arg method body lost coverage
Method: (7−1)  / (8+0)  = 6/8   = 75.0%   ← the 4-arg method itself is now uncovered

This is a real but trivial gap — the uncovered method is a one-line delegator to the 5-arg version. It could be fixed by adding a test that calls the 4-arg overload, or the baseline can be updated since the method is a trivial convenience wrapper.

Summary

Class	Verdict	Action needed
QueryDirectivesImpl	False positive (refactor shifted ratios)	Update baseline
ExecutableNormalizedOperationFactory	Real but trivial (convenience overload lost its only caller)	Update baseline or add a trivial test

https://claude.ai/code/session_01XVBWUo2VM4B82ssZm1f2mu

Is it worth calling this a breaking change, given this is a public API class?

Even though it's highly unlikely anyone uses this constructor

There is a factory for creating them - this should have been package level contructor - so no I think

A JSpecify observation - this class hasn't yet been annotated, but if it was, it would be clear this is not null, that an operation definition always exists

This adds support for QueryAppliedDirective on operations and documents

9f65d67

andimarek and others added 3 commits March 7, 2026 05:47

Merge branch 'master' into extract-out-operation-directives

c897548

Merge branch 'master' into extract-out-operation-directives

0f689b0

This adds support for QueryAppliedDirective on operations and documen…

1740162

…ts - tests added

bbakerman commented Mar 6, 2026

View reviewed changes

This adds support for QueryAppliedDirective on operations and documen…

1951ed6

…ts - refactor

bbakerman commented Mar 6, 2026

View reviewed changes

This adds support for QueryAppliedDirective on operations and documen…

2e7c4fc

…ts - use Immutable signature

bbakerman changed the title WIP This adds support for QueryAppliedDirective on operations and documents This adds support for QueryAppliedDirective on operations and documents Mar 6, 2026

This adds support for QueryAppliedDirective on operations and documen…

8f049fe

…ts - used field to make it cheaper

bbakerman commented Mar 6, 2026

View reviewed changes

bbakerman commented Mar 7, 2026

View reviewed changes

This adds support for QueryAppliedDirective on operations and documen…

17e7ccd

…ts - made it a supplier for performance

bbakerman commented Mar 7, 2026

View reviewed changes

bbakerman and others added 4 commits March 7, 2026 21:37

This adds support for QueryAppliedDirective on operations and documen…

fdd9568

…ts -added more tests

Merge branch 'master' into extract-out-operation-directives

9da09ed

Merge remote-tracking branch 'origin/master' into extract-out-operati…

b47540c

…on-directives

Merge branch 'master' into extract-out-operation-directives

7b92be4

andimarek requested changes Mar 8, 2026

View reviewed changes

This adds support for QueryAppliedDirective on operations and documen…

601195a

…ts - PR feedback

github-actions Bot mentioned this pull request Mar 8, 2026

CI Failure DoctorCI Failure - Run #22832374633 — Coverage regression in ExecutableNormalizedOperationFactory and QueryDirectivesImpl #4309

Closed

2 tasks

bbakerman and others added 2 commits March 9, 2026 10:54

Merge remote-tracking branch 'origin/extract-out-operation-directives…

93b53e4

…' into extract-out-operation-directives

Merge branch 'master' into extract-out-operation-directives

bdf5163

github-actions Bot mentioned this pull request Mar 9, 2026

CI Failure DoctorCI Failure - Run #22834065174 (PR #4297) - Per-Class Coverage Gate #4311

Closed

2 tasks

bbakerman requested a review from andimarek March 9, 2026 04:32

bbakerman added this to the 26.x is a breaking change release (other fixes as well) milestone Mar 9, 2026

bbakerman enabled auto-merge (rebase) March 9, 2026 05:05

andimarek approved these changes Mar 9, 2026

View reviewed changes

andimarek mentioned this pull request Mar 9, 2026

Improve coverage gate: hybrid regression check + method-level detail reporting #4324

Merged

5 tasks

Merge branch 'master' into extract-out-operation-directives

0752dc1

github-actions Bot mentioned this pull request Mar 9, 2026

CI Failure DoctorCI Failure - Per-Class Coverage Gate - Run #22870173052 #4325

Closed

3 tasks

andimarek and others added 2 commits March 10, 2026 08:20

Merge branch 'master' into extract-out-operation-directives

878478b

Merge branch 'master' into extract-out-operation-directives

47d3101

dondonz reviewed Mar 15, 2026

View reviewed changes

dondonz approved these changes Mar 15, 2026

View reviewed changes

bbakerman disabled auto-merge March 16, 2026 01:19

bbakerman merged commit 6d6a262 into master Mar 16, 2026
9 of 11 checks passed

                       this.executionInput = builder.executionInput;
                       this.dataLoaderDispatcherStrategy = builder.dataLoaderDispatcherStrategy;
-                      this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
                       this.propagateErrorsOnNonNullContractFailure = builder.propagateErrorsOnNonNullContractFailure;

                   public ExecutableNormalizedOperation(
                           OperationDefinition.Operation operation,
                           String operationName,
+                          Map<String, ImmutableList<QueryAppliedDirective>> operationDirectives,

+                      return directives.stream().map(this::toAppliedDirective).collect(ImmutableList.toImmutableList());
+                  }
+                  public QueryAppliedDirective toAppliedDirective(GraphQLDirective directive) {

+              @Internal
+              @NullMarked
+              public class OperationDirectivesResolver {

+                      // preconditions
+                      assertNotNull(executionId, "You must provide a query identifier");
+                      return new ExecutionContext(this);
+                  }

+                      commonIntegrationAsserts(resolveDirectives)
+                      when:
+                      def normalizedOperation = executionContext.getNormalizedQueryTree().get()

-                              .inputValueWithState(argument.getArgumentValue())
-                              .build();
-                  }

                               .responseMapFactory().getOr(ResponseMapFactory.DEFAULT);
+                      Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> operationDirectives = FpKit.interThreadMemoize(() ->
+                              operationDirectivesResolver.resolveDirectives(document, graphQLSchema, coercedVariables, graphQLContext, locale));

+                  private Supplier<ExecutableNormalizedOperation> mkExecutableNormalizedOperation() {
+                      return FpKit.interThreadMemoize(() -> {
+                          Options options = Options.defaultOptions().graphQLContext(graphQLContext).locale(locale);

               import java.util.function.Consumer;
               import java.util.function.Supplier;
+              import static graphql.normalized.ExecutableNormalizedOperationFactory.*;

+                  private Supplier<Map<String, ImmutableList<QueryAppliedDirective>>> mkOpDirectives(Supplier<Map<OperationDefinition, ImmutableList<QueryAppliedDirective>>> allOperationsDirectives) {
+                      return FpKit.interThreadMemoize(() -> {
+                          List<QueryAppliedDirective> list = allOperationsDirectives.get().get(operationDefinition);

This adds support for QueryAppliedDirective on operations and documents by bbakerman · Pull Request #4297 · graphql-java/graphql-java · GitHub

Uh oh!

This adds support for QueryAppliedDirective on operations and documents - #4297

This adds support for QueryAppliedDirective on operations and documents#4297bbakerman merged 18 commits intomastergraphql-java/graphql-java:masterfrom extract-out-operation-directivesgraphql-java/graphql-java:extract-out-operation-directivesCopy head branch name to clipboard

Conversation

bbakerman commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Report

Test Results

Code Coverage (Java 25)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andimarek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andimarek commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Drop Analysis

1. QueryDirectivesImpl — Line: 97.3% (was 97.6%), Branch: 75.0% (was 80.0%), Method: 93.8% (was 94.4%)

The math in detail

2. ExecutableNormalizedOperationFactory (outer class) — Line: 84.0% (was 92.0%), Method: 75.0% (was 87.5%)

Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

This adds support for QueryAppliedDirective on operations and documents#4297
bbakerman merged 18 commits into
masterfrom
extract-out-operation-directives

bbakerman commented Mar 6, 2026 •
edited
Loading

github-actions Bot commented Mar 6, 2026 •
edited
Loading

andimarek commented Mar 9, 2026 •
edited
Loading