Repository navigation
SchemaTransformer.transformSchema produces schema with dangling reference to deleted type #4133
Description
Describe the bug
We've implemented a visitor that prunes our schema based on the presence of a directive.
This has worked well until quite recently, but has suddenly started failing with graphql.AssertException: Assert type Rom not found in schema.
I've analyzed the problem and reduced the schema and our visitor down to the minimal needed to reproduce the issue. Note that our actual implementation also handles other schema elements, but I don't think this is relevant for the issue at hand.
To Reproduce
Given the following schema, our visitor crashes with AssertException:
directive @remove on FIELD_DEFINITION
type Query {
rental: Rental @remove
customer: Customer
}
type Store {
inventory: Inventory @remove
}
type Inventory {
store: Store @remove
}
type Customer {
rental: Rental
payment: Payment @remove
}
type Payment {
inventory: Inventory @remove
}
type Rental {
id: ID
customer: Customer @remove
}
Our visitor is defined as follows:
class SchemaPruningVisitor extends GraphQLTypeVisitorStub {
@Override
public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition node, TraverserContext<GraphQLSchemaElement> context) {
if (node.hasAppliedDirective("remove")) {
return deleteNode(context);
}
return CONTINUE;
}
@Override
public TraversalControl visitGraphQLObjectType(GraphQLObjectType node, TraverserContext<GraphQLSchemaElement> context) {
if (node.getFields().stream().allMatch(field -> field.hasAppliedDirective("remove"))) {
return deleteNode(context);
}
return CONTINUE;
}
}Interesting observation
Note that if we flip the order of the Query.rental and Query.customer fields then our visitor no longer crashes and produces the schema we expect:
type Query {
customer: Customer
}
type Customer {
rental: Rental
}
type Rental {
id: ID
}
Activity
bbakerman commentedon Oct 16, 2025
I have created a repo PR and confirm the assertion. We will look into this further
andimarek commentedon Oct 17, 2025
@alf in which version did this actually work, meaning from which version to version did you notice it failing?
alf commentedon Oct 17, 2025
I believe it started failing because we suddenly ended up with a schema that has the problematic structure, not because we upgraded graphql-java.
andimarek commentedon Oct 17, 2025
@alf yes ... I just wanted to comment on this. Indeed the problem comes from your circular structure and order of your fields.
For example: the visitor actually never visits Rental because when it hits
Query.rentalthe field is deleted and the visitor doesn't continue traversing down a deleted element.And when the traverser reaches Rental again via
Query.customer -> Customer. rentaltheCustomer.rentalfield definition doesn't actually reference the real rental Object but uses aGraphQLTypeReference. AGraphQLTypeReferenceis an internal placeholder type to deal with circular references.This means the actual
Rentalis never visited and hence still references the originalCustomerwhich indirectly referencesInventory. But then on other handInventoryis actually deleted implicitly becauseCustomer.paymentis deleted.This causes the exception you see.
We need to look into the best way to fix this.
alf commentedon Oct 17, 2025
That's a good explanation, thank you.
andimarek commentedon Oct 17, 2025
Hi @alf ... please have a look at this: https://github.com/graphql-java/graphql-java/pull/4142/files#r2441444727
in order to make sure you will visit every Object you need to add all object types to the schema as
additionalTypes. This ensures that in your example you will always visitRentalregardless of the order of the fields (or how you change them).The small caveat here is that you need to avoid adding the Query/Mutation/Subscription types as this will lead to another strange error.
alf commentedon Oct 18, 2025
Nice, thank you. Is this something that the SchemaParser should be doing? That’s what we’re using to build our TDR.
andimarek commentedon Oct 18, 2025
Not sure we should do that as default as for very large schemas it increases the traversal time and is simply unnecessary if you are not deleting nodes like in your use case.
But we probably add a helper util or so to enable "alwaysTraverseAllNodes" to not make people write this on their own and to document this case better.
@alf would you be open to contribute this improvement?
alf commentedon Oct 19, 2025
Sounds perfect for our use case. I’ll take a shot at contributing this.
Make sure SchemaFeatureFilter visits all nodes
andimarek commentedon Jan 19, 2026
I finally found time to add a built in way to handle these edge cases better. See #4209