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

bbakerman · 2026-03-06T05:50:56Z

github-actions · 2026-03-06T05:59:58Z

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

bbakerman · 2026-03-06T22:44:54Z

src/main/java/graphql/execution/directives/DirectivesResolver.java

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


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

…ts - refactor

bbakerman · 2026-03-06T22:49:50Z

src/main/java/graphql/execution/directives/OperationDirectivesResolver.java

+
+@Internal
+@NullMarked
+public class OperationDirectivesResolver {


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

…ts - use Immutable signature

…ts - used field to make it cheaper

bbakerman · 2026-03-06T23:44:59Z

src/main/java/graphql/execution/ExecutionContextBuilder.java

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


moved build down to the bottom for code OCD reasons

bbakerman · 2026-03-06T23:45:54Z

src/test/groovy/graphql/execution/directives/OperationDirectivesResolverTest.groovy

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


here the ENO has the same values

bbakerman · 2026-03-07T00:49:07Z

src/main/java/graphql/execution/directives/QueryDirectivesImpl.java

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


moved methods so they can be re-used

…ts - made it a supplier for performance

bbakerman · 2026-03-07T01:15:09Z

src/main/java/graphql/execution/Execution.java

                .responseMapFactory().getOr(ResponseMapFactory.DEFAULT);

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


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

bbakerman · 2026-03-07T02:06:34Z

src/main/java/graphql/execution/ExecutionContext.java

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


rejigged this code to a maker method

bbakerman · 2026-03-07T02:06:55Z

src/main/java/graphql/execution/ExecutionContext.java

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


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

bbakerman · 2026-03-07T02:07:28Z

src/main/java/graphql/execution/ExecutionContextBuilder.java

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


…ts -added more tests

…on-directives

andimarek

import

andimarek · 2026-03-08T23:34:31Z

src/main/java/graphql/execution/ExecutionContext.java

 import java.util.function.Consumer;
 import java.util.function.Supplier;

+import static graphql.normalized.ExecutableNormalizedOperationFactory.*;


import not correct :-)

…ts - PR feedback

…' into extract-out-operation-directives

andimarek · 2026-03-09T18:08:17Z

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

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

Open

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

Open

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

Open

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

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bbakerman commented Mar 6, 2026 •

edited

Loading

github-actions bot commented Mar 6, 2026 •

edited

Loading

andimarek commented Mar 9, 2026 •

edited

Loading

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

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