Skip to content

Fix schema rebuild after root type deletion#4390

Merged
andimarek merged 6 commits into
masterfrom
fix-idempotent-generated-annotations
May 20, 2026
Merged

Fix schema rebuild after root type deletion#4390
andimarek merged 6 commits into
masterfrom
fix-idempotent-generated-annotations

Conversation

@andimarek
Copy link
Copy Markdown
Member

@andimarek andimarek commented May 20, 2026

Summary

Fixes #3384.

The core issue is that a built GraphQLSchema can contain two views of the same relationship:

  • the original child list used for rebuild traversal, which may still contain GraphQLTypeReference("Node")
  • the resolved accessor view used by runtime/schema APIs, where Person.getInterfaces() returns the actual GraphQLInterfaceType Node instance

In the #3384 repro, Person was originally created with .withInterface(typeRef("Node")). During schema build, that reference is resolved and Person.getInterfaces() starts returning the real Node interface. Later, when the mutation root is removed, Node is no longer reachable through the deleted mutation field. The rebuilt schema still reaches Person from Query, and Person still implements Node, so Node must remain in the rebuilt type map.

The catch is that schema build collection intentionally traverses getChildrenWithTypeReferences(). For Person, that traversal sees the original GraphQLTypeReference("Node"), not the already-resolved Node object returned by Person.getInterfaces(). So the collector can visit Person without collecting the resolved Node interface. Later, GraphQLTypeResolvingVisitor uses Person.getInterfaces(), looks up Node in the rebuilt type map, and fails because Node was missed during collection.

This is what the code calls an indirect strong reference:

  • indirect: the traverser did not visit it as a normal child node, because the child list exposes the original type reference
  • strong: the schema element already holds the actual GraphQLNamedType object through an accessor such as getInterfaces() or getTypes()

GraphQLTypeCollectingVisitor already handled this pattern for field, argument, input field, and applied directive argument types. This PR extends the same collection logic to:

  • object implemented interfaces
  • interface implemented interfaces
  • union member types

The same original-reference versus resolved-accessor split exists for unions and interface inheritance:

  • GraphQLUnionType.getChildrenWithTypeReferences() exposes original possible types, while getTypes() can return resolved object types
  • GraphQLInterfaceType.getChildrenWithTypeReferences() exposes original implemented interfaces, while getInterfaces() can return resolved interfaces

This does not keep every deleted-path extra type alive. It only preserves resolved named types that are still referenced by a schema element that remains reachable in the rebuilt schema.

Repro Coverage

Adds #3384 Spock coverage for these schema rebuild cases:

  • object implementing an interface through a type reference
  • union containing a member through a type reference
  • interface implementing another interface through a type reference

The object/interface case is covered through both paths:

  • direct rebuild with GraphQLSchema.newSchema(schema).mutation((GraphQLObjectType) null).build()
  • traversal-based deletion with SchemaTransformer.transformSchemaWithDeletes(...)

The union and interface-inheritance cases specifically prove why GraphQLTypeCollectingVisitor needs to follow getTypes() and getInterfaces(), not just field/argument/input references.

Before the collector change, these cases fail during rebuild with an NPE in GraphQLTypeResolvingVisitor; they now pass.

Local Test Stability

This PR also keeps the local build cleanup from the previous version of #4390: markGeneratedEqualsHashCode now cleans classes-jacoco before rewriting and does not append duplicate graphql.coverage.Generated annotations. That prevents local-only ArchUnit failures in JMHForkArchRuleTest after interrupted or repeated incremental runs.

Verification

./gradlew test --tests "graphql.schema.impl.SchemaUtilTest.can rebuild schema after removing root type that made an implemented interface reachable" --tests "graphql.schema.impl.SchemaUtilTest.can transform schema after deleting root type that made an implemented interface reachable" --tests "graphql.schema.impl.SchemaUtilTest.can rebuild schema after removing root type that made a union member reachable" --tests "graphql.schema.impl.SchemaUtilTest.can rebuild schema after removing root type that made an interface implemented by another interface reachable"

./gradlew test --tests graphql.schema.impl.SchemaUtilTest

Also ran:

git diff --check

@andimarek andimarek changed the title Make generated annotation marking idempotent Fix schema rebuild type collection for resolved references May 20, 2026
@andimarek andimarek changed the title Fix schema rebuild type collection for resolved references Fix schema rebuild after root type deletion May 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 20, 2026

Test Report

Test Results

Java Version Total Passed Failed Errors Skipped
Java 11 5837 (+4 🟢) 5781 (+4 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 17 5837 (+4 🟢) 5780 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 21 5837 (+4 🟢) 5780 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 25 5837 (+4 🟢) 5780 (+4 🟢) 0 (±0) 0 (±0) 57 (±0)
jcstress 32 (±0) 32 (±0) 0 (±0) 0 (±0) 0 (±0)
Total 23380 (+16 🟢) 23153 (+16 🟢) 0 (±0) 0 (±0) 227 (±0)

Code Coverage (Java 25)

Metric Covered Missed Coverage vs Master
Lines 29586 3130 90.4% ±0.0%
Branches 8680 1532 85.0% ±0.0%
Methods 7906 1210 86.7% ±0.0%

No per-class coverage changes detected.

Full HTML report: build artifact jacoco-html-report

Updated: 2026-05-20 05:51:54 UTC

@andimarek andimarek enabled auto-merge (squash) May 20, 2026 05:44
@andimarek andimarek merged commit 790627d into master May 20, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SchemaUtil#replaceTypeReferences Issue When trying to re-built a GraphqlSchema

1 participant