apollographql / federation-next

Moved to the Router repository
https://github.com/apollographql/router/tree/dev/apollo-federation
Other
19 stars 1 forks source link

FED-46: Implemented `ConditionResolver` trait for `QueryPlanningTraversal` #265

Closed duckki closed 4 months ago

duckki commented 4 months ago

Background

I was tasked to implement the CachingConditionResolver, which is used in QueryPlanningTraversal. The TypeScript version of QueryPlanningTraversal has a field conditionResolver, which is a closure that captures this (QueryPlanningTraversal) as mutable reference and calls this.resolveConditionPlan. The problem is that there is a cyclic reference structure: QueryPlanningTraversal -owns-> conditionResolver -captures-> QueryPlanningTraversal.

In Rust, CachingConditionResolver can't mutably reference QueryPlanningTraversal, since we want QueryPlanningTraversal to hold the CachingConditionResolver itself. (Not considering smart pointers or unsafe references)

Proposed Solution

I decided not to implement CachingConditionResolver. Instead, I made QueryPlanningTraversal to implement ConditionResolver trait directly and hold its own cache. I separated the caching logic in the newConditionResolverCache struct. But, the separation of caching and resolver is not as nice as the TypeScript version.

An alternative solution would be to create a wrapper struct that holds both QueryPlanningTraversal and a cache. But, then every references of QueryPlanningTraversal needs to be changed to point to the new wrapper. I didn't think it'd be worthwhile, only to gain a slightly better separation and reusability.