diff --git a/federation-2/router-bridge/benches/query_planning.rs b/federation-2/router-bridge/benches/query_planning.rs index e42fc30fc..a8ec2ae7a 100644 --- a/federation-2/router-bridge/benches/query_planning.rs +++ b/federation-2/router-bridge/benches/query_planning.rs @@ -1,6 +1,7 @@ use criterion::criterion_group; use criterion::criterion_main; use criterion::Criterion; +use router_bridge::planner::PlanOptions; use router_bridge::planner::Planner; use router_bridge::planner::QueryPlannerConfig; @@ -19,7 +20,7 @@ fn from_elem(c: &mut Criterion) { b.to_async(runtime).iter(|| async { planner - .plan(QUERY.to_string(), None) + .plan(QUERY.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() diff --git a/federation-2/router-bridge/js-src/plan.ts b/federation-2/router-bridge/js-src/plan.ts index 410a65028..9cb36bb0f 100644 --- a/federation-2/router-bridge/js-src/plan.ts +++ b/federation-2/router-bridge/js-src/plan.ts @@ -51,6 +51,12 @@ export interface QueryPlanResult { queryPlan: QueryPlan; } +export interface PlanOptions { + // We receive these across the bridge as an array of strings, + // but ultimately build a Map object out of it for use in the planner. + overrideConditions?: string[]; +} + export class BridgeQueryPlanner { private readonly supergraph: Supergraph; private readonly apiSchema: GraphQLSchema; @@ -72,7 +78,8 @@ export class BridgeQueryPlanner { plan( operationString: string, - providedOperationName?: string + providedOperationName?: string, + options?: PlanOptions ): ExecutionResultWithUsageReporting { let operationResult = this.operation( operationString, @@ -87,8 +94,18 @@ export class BridgeQueryPlanner { let usageReporting = operationResult.usageReporting; let operation = operationResult.data; const operationName = operation?.name; - - const queryPlan = this.planner.buildQueryPlan(operation); + const buildQueryPlanOptions = options + ? { + overrideConditions: new Map( + options.overrideConditions.map((override) => [override, true]) + ), + } + : undefined; + + const queryPlan = this.planner.buildQueryPlan( + operation, + buildQueryPlanOptions + ); let formattedQueryPlan: string | null; try { formattedQueryPlan = prettyFormatQueryPlan(queryPlan); diff --git a/federation-2/router-bridge/js-src/plan_worker.ts b/federation-2/router-bridge/js-src/plan_worker.ts index e94ad796f..4c2c2b20b 100644 --- a/federation-2/router-bridge/js-src/plan_worker.ts +++ b/federation-2/router-bridge/js-src/plan_worker.ts @@ -3,6 +3,7 @@ import { ASTNode, Source, SourceLocation, ExecutionResult } from "graphql"; import { BridgeQueryPlanner, ExecutionResultWithUsageReporting, + PlanOptions, QueryPlanResult, } from "./plan"; import { QueryPlannerConfigExt } from "./types"; @@ -45,6 +46,7 @@ interface PlanEvent { query: string; operationName?: string; schemaId: number; + options?: PlanOptions; } interface ApiSchemaEvent { kind: PlannerEventKind.ApiSchema; @@ -258,7 +260,7 @@ async function run() { case PlannerEventKind.Plan: const planResult = planners .get(event.schemaId) - .plan(event.query, event.operationName); + .plan(event.query, event.operationName, event.options); await send({ id, payload: planResult }); break; case PlannerEventKind.ApiSchema: diff --git a/federation-2/router-bridge/src/planner.rs b/federation-2/router-bridge/src/planner.rs index 64e08f64b..38732427f 100644 --- a/federation-2/router-bridge/src/planner.rs +++ b/federation-2/router-bridge/src/planner.rs @@ -1,7 +1,6 @@ /*! * Instantiate a QueryPlanner from a schema, and perform query planning */ - use std::collections::HashMap; use std::fmt::Debug; use std::fmt::Display; @@ -530,12 +529,14 @@ where &self, query: String, operation_name: Option, + options: PlanOptions, ) -> Result, crate::error::Error> { self.worker .request(PlanCmd::Plan { query, operation_name, schema_id: self.schema_id, + options, }) .await } @@ -608,6 +609,14 @@ where } } +/// Options for planning a query +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Hash, Default)] +#[serde(rename_all = "camelCase")] +pub struct PlanOptions { + /// Which labels to override during query planning + pub override_conditions: Vec, +} + #[derive(Serialize, Debug, Clone, PartialEq, Eq, Hash)] #[serde(tag = "kind")] enum PlanCmd { @@ -622,6 +631,7 @@ enum PlanCmd { query: String, operation_name: Option, schema_id: u64, + options: PlanOptions, }, #[serde(rename_all = "camelCase")] ApiSchema { schema_id: u64 }, @@ -769,6 +779,7 @@ mod tests { include_str!("testdata/unsupported_feature_for_execution.graphql"); const UNSUPPORTED_FEATURE_FOR_SECURITY: &str = include_str!("testdata/unsupported_feature_for_security.graphql"); + const PROGRESSIVE_OVERRIDE: &str = include_str!("testdata/progressive_override.graphql"); #[tokio::test] async fn anonymous_query_works() { @@ -778,7 +789,7 @@ mod tests { .unwrap(); let payload = planner - .plan(QUERY.to_string(), None) + .plan(QUERY.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -789,6 +800,42 @@ mod tests { }); } + // This test runs the same query twice, but provides an override label on + // the first run. The resulting snapshots should show the difference in the + // query plans. + #[tokio::test] + async fn progressive_override() { + let query = "{ t { a } }"; + let planner = Planner::::new( + PROGRESSIVE_OVERRIDE.to_string(), + QueryPlannerConfig::default(), + ) + .await + .unwrap(); + + let payload1 = planner + .plan( + query.to_string(), + None, + PlanOptions { + override_conditions: vec!["foo".to_string()], + }, + ) + .await + .unwrap() + .into_result() + .unwrap(); + insta::assert_snapshot!(serde_json::to_string_pretty(&payload1.data).unwrap()); + + let payload2 = planner + .plan(query.to_string(), None, PlanOptions::default()) + .await + .unwrap() + .into_result() + .unwrap(); + insta::assert_snapshot!(serde_json::to_string_pretty(&payload2.data).unwrap()); + } + #[tokio::test] async fn named_query_works() { let planner = @@ -797,7 +844,7 @@ mod tests { .unwrap(); let payload = planner - .plan(NAMED_QUERY.to_string(), None) + .plan(NAMED_QUERY.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -819,6 +866,7 @@ mod tests { .plan( MULTIPLE_QUERIES.to_string(), Some("MyFirstName".to_string()), + PlanOptions::default(), ) .await .unwrap() @@ -841,6 +889,7 @@ mod tests { .plan( NAMED_QUERY.to_string(), Some("MyFirstAndLastName".to_string()), + PlanOptions::default(), ) .await .unwrap() @@ -859,7 +908,11 @@ mod tests { .unwrap(); let payload = planner - .plan(QUERY_REUSE_QUERY_FRAGMENTS.to_string(), None) + .plan( + QUERY_REUSE_QUERY_FRAGMENTS.to_string(), + None, + PlanOptions::default(), + ) .await .unwrap() .into_result() @@ -880,7 +933,11 @@ mod tests { .unwrap(); let payload = planner - .plan(QUERY_REUSE_QUERY_FRAGMENTS.to_string(), None) + .plan( + QUERY_REUSE_QUERY_FRAGMENTS.to_string(), + None, + PlanOptions::default(), + ) .await .unwrap() .into_result() @@ -901,7 +958,11 @@ mod tests { .unwrap(); let payload = planner - .plan(QUERY_REUSE_QUERY_FRAGMENTS.to_string(), None) + .plan( + QUERY_REUSE_QUERY_FRAGMENTS.to_string(), + None, + PlanOptions::default(), + ) .await .unwrap() .into_result() @@ -917,7 +978,11 @@ mod tests { .unwrap(); let payload = planner - .plan("this query will definitely not parse".to_string(), None) + .plan( + "this query will definitely not parse".to_string(), + None, + PlanOptions::default(), + ) .await .unwrap() .into_result() @@ -955,6 +1020,7 @@ mod tests { query { me { id ...thatUserFragment1 } }" .to_string(), None, + PlanOptions::default(), ) .await .unwrap() @@ -982,6 +1048,7 @@ mod tests { .plan( QUERY.to_string(), Some("ThisOperationNameDoesntExist".to_string()), + PlanOptions::default(), ) .await .unwrap() @@ -1006,7 +1073,7 @@ mod tests { .unwrap(); let payload = planner - .plan(MULTIPLE_QUERIES.to_string(), None) + .plan(MULTIPLE_QUERIES.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -1030,7 +1097,11 @@ mod tests { .unwrap(); let payload = planner - .plan(MULTIPLE_ANONYMOUS_QUERIES.to_string(), None) + .plan( + MULTIPLE_ANONYMOUS_QUERIES.to_string(), + None, + PlanOptions::default(), + ) .await .unwrap() .into_result() @@ -1054,7 +1125,7 @@ mod tests { .unwrap(); let payload = planner - .plan(NO_OPERATION.to_string(), None) + .plan(NO_OPERATION.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -1274,7 +1345,10 @@ mod tests { .await .unwrap(); - let actual = planner.plan(query, operation_name).await.unwrap(); + let actual = planner + .plan(query, operation_name, PlanOptions::default()) + .await + .unwrap(); assert_eq!(expected_errors, actual.errors.unwrap()); } @@ -1287,14 +1361,14 @@ mod tests { .unwrap(); let query_1_response = planner - .plan(QUERY.to_string(), None) + .plan(QUERY.to_string(), None, PlanOptions::default()) .await .unwrap() .data .unwrap(); let query_2_response = planner - .plan(QUERY2.to_string(), None) + .plan(QUERY2.to_string(), None, PlanOptions::default()) .await .unwrap() .data @@ -1302,9 +1376,15 @@ mod tests { let all_futures = stream::iter((0..1000).map(|i| { let (query, fut) = if i % 2 == 0 { - (QUERY, planner.plan(QUERY.to_string(), None)) + ( + QUERY, + planner.plan(QUERY.to_string(), None, PlanOptions::default()), + ) } else { - (QUERY2, planner.plan(QUERY2.to_string(), None)) + ( + QUERY2, + planner.plan(QUERY2.to_string(), None, PlanOptions::default()), + ) }; async move { (query, fut.await.unwrap()) } @@ -1618,7 +1698,7 @@ ofType { .await .unwrap(); let query_plan1 = planner - .plan(query.to_string(), None) + .plan(query.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -1633,7 +1713,7 @@ ofType { .await .unwrap(); let query_plan2 = updated_planner - .plan(query.to_string(), None) + .plan(query.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -1660,7 +1740,7 @@ ofType { assert_eq!( query_plan2.data, updated_planner - .plan(query.to_string(), None) + .plan(query.to_string(), None, PlanOptions::default()) .await .unwrap() .into_result() @@ -1973,6 +2053,7 @@ feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: SECURITY but i }"# .to_string(), None, + PlanOptions::default(), ) .await .unwrap() @@ -2042,7 +2123,8 @@ feature https://specs.apollo.dev/unsupported-feature/v0.1 is for: SECURITY but i .plan( "query { currentUser { activeOrganization { id suborga { id ...@defer { nonNullId } } } } }" .to_string(), - None + None, + PlanOptions::default(), ) .await .unwrap() diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override-2.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override-2.snap new file mode 100644 index 000000000..00faf5eb2 --- /dev/null +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override-2.snap @@ -0,0 +1,52 @@ +--- +source: router-bridge/src/planner.rs +assertion_line: 833 +expression: "serde_json::to_string_pretty(&payload2.data).unwrap()" +--- +{ + "queryPlan": { + "kind": "QueryPlan", + "node": { + "kind": "Sequence", + "nodes": [ + { + "kind": "Fetch", + "serviceName": "Subgraph1", + "variableUsages": [], + "operation": "{t{__typename k}}", + "operationKind": "query" + }, + { + "kind": "Flatten", + "path": [ + "t" + ], + "node": { + "kind": "Fetch", + "serviceName": "Subgraph2", + "requires": [ + { + "kind": "InlineFragment", + "typeCondition": "T", + "selections": [ + { + "kind": "Field", + "name": "__typename" + }, + { + "kind": "Field", + "name": "k" + } + ] + } + ], + "variableUsages": [], + "operation": "query($representations:[_Any!]!){_entities(representations:$representations){...on T{a}}}", + "operationKind": "query" + } + } + ] + } + }, + "formattedQueryPlan": "QueryPlan {\n Sequence {\n Fetch(service: \"Subgraph1\") {\n {\n t {\n __typename\n k\n }\n }\n },\n Flatten(path: \"t\") {\n Fetch(service: \"Subgraph2\") {\n {\n ... on T {\n __typename\n k\n }\n } =>\n {\n ... on T {\n a\n }\n }\n },\n },\n },\n}" +} diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override.snap new file mode 100644 index 000000000..e551cc732 --- /dev/null +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__progressive_override.snap @@ -0,0 +1,18 @@ +--- +source: router-bridge/src/planner.rs +assertion_line: 825 +expression: "serde_json::to_string_pretty(&payload1.data).unwrap()" +--- +{ + "queryPlan": { + "kind": "QueryPlan", + "node": { + "kind": "Fetch", + "serviceName": "Subgraph1", + "variableUsages": [], + "operation": "{t{a}}", + "operationKind": "query" + } + }, + "formattedQueryPlan": "QueryPlan {\n Fetch(service: \"Subgraph1\") {\n {\n t {\n a\n }\n }\n },\n}" +} diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-2.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-2.snap index ccc4705e9..d2ad6c798 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-2.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-2.snap @@ -1,6 +1,6 @@ --- source: router-bridge/src/planner.rs -assertion_line: 1696 +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-3.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-3.snap index 02c4b7ac2..96203dc85 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-3.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-3.snap @@ -1,6 +1,6 @@ --- source: router-bridge/src/planner.rs -assertion_line: 1696 +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-4.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-4.snap index f650f97fc..9eb81a51c 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-4.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-4.snap @@ -1,6 +1,6 @@ --- source: router-bridge/src/planner.rs -assertion_line: 1696 +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-5.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-5.snap index 21cfce7ba..dce4e82f2 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-5.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-5.snap @@ -1,5 +1,6 @@ --- source: router-bridge/src/planner.rs +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-6.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-6.snap index 8e4e60769..75a65b583 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-6.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs-6.snap @@ -1,5 +1,6 @@ --- source: router-bridge/src/planner.rs +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs.snap b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs.snap index 07ea2c62a..2c43ff44b 100644 --- a/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs.snap +++ b/federation-2/router-bridge/src/snapshots/router_bridge__planner__tests__subgraphs.snap @@ -1,6 +1,6 @@ --- source: router-bridge/src/planner.rs -assertion_line: 1696 +assertion_line: 1784 expression: schema --- directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA diff --git a/federation-2/router-bridge/src/testdata/progressive_override.graphql b/federation-2/router-bridge/src/testdata/progressive_override.graphql new file mode 100644 index 000000000..51d51867f --- /dev/null +++ b/federation-2/router-bridge/src/testdata/progressive_override.graphql @@ -0,0 +1,57 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION) +{ + query: Query +} + +directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE + +directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION + +directive @join__graph(name: String!, url: String!) on ENUM_VALUE + +directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE + +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + +directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + +scalar join__FieldSet + +enum join__Graph { + SUBGRAPH1 @join__graph(name: "Subgraph1", url: "https://Subgraph1") + SUBGRAPH2 @join__graph(name: "Subgraph2", url: "https://Subgraph2") +} + +scalar link__Import + +enum link__Purpose { + """ + \`SECURITY\` features provide metadata necessary to securely resolve fields. + """ + SECURITY + + """ + \`EXECUTION\` features provide metadata necessary for operation execution. + """ + EXECUTION +} + +type Query + @join__type(graph: SUBGRAPH1) + @join__type(graph: SUBGRAPH2) +{ + t: T @join__field(graph: SUBGRAPH1) +} + +type T + @join__type(graph: SUBGRAPH1, key: "k") + @join__type(graph: SUBGRAPH2, key: "k") +{ + k: ID + a: Int @join__field(graph: SUBGRAPH1, override: "Subgraph2", overrideLabel: "foo") @join__field(graph: SUBGRAPH2, overrideLabel: "foo") + b: Int @join__field(graph: SUBGRAPH2) +} \ No newline at end of file diff --git a/xtask/src/utils.rs b/xtask/src/utils.rs index 34c61fd7a..576e32ad8 100644 --- a/xtask/src/utils.rs +++ b/xtask/src/utils.rs @@ -2,7 +2,7 @@ use anyhow::{anyhow, Result}; use camino::Utf8PathBuf; use lazy_static::lazy_static; -use std::{convert::From, env, str}; +use std::{env, str}; const MANIFEST_DIR: &str = env!("CARGO_MANIFEST_DIR");