From 19dbf04a3c2ca6595afd0aed5cfc77eedbe48c09 Mon Sep 17 00:00:00 2001 From: Lenny Burdette Date: Tue, 11 Feb 2025 21:36:18 -0500 Subject: [PATCH] fix: connectors, pqs, and __typename i'm not sure why, but __typename selections appear in certain fetch node operations only for PQs. our current __typename handling would emit `__typename: $("_Entity")` which is never correct (_Entity is a union), so i added an extra guard against this. --- .../connect/json_selection/selection_set.rs | 38 ++++----- .../enterprise/connectors-pqs/README.md | 1 + .../connectors-pqs/configuration.yaml | 21 +++++ .../connectors-pqs/http_snapshots.json | 51 ++++++++++++ .../enterprise/connectors-pqs/manifest.json | 12 +++ .../enterprise/connectors-pqs/plan.json | 54 ++++++++++++ .../connectors-pqs/supergraph.graphql | 83 +++++++++++++++++++ .../enterprise/connectors-pqs/supergraph.yaml | 63 ++++++++++++++ 8 files changed, 299 insertions(+), 24 deletions(-) create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/README.md create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/configuration.yaml create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/http_snapshots.json create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/manifest.json create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/plan.json create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.graphql create mode 100644 apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.yaml diff --git a/apollo-federation/src/sources/connect/json_selection/selection_set.rs b/apollo-federation/src/sources/connect/json_selection/selection_set.rs index 5721e704c8..f9814a6cc2 100644 --- a/apollo-federation/src/sources/connect/json_selection/selection_set.rs +++ b/apollo-federation/src/sources/connect/json_selection/selection_set.rs @@ -19,15 +19,14 @@ use apollo_compiler::executable::Field; use apollo_compiler::executable::Selection; use apollo_compiler::executable::SelectionSet; +use apollo_compiler::name; use apollo_compiler::ExecutableDocument; use apollo_compiler::Node; use multimap::MultiMap; -use super::known_var::KnownVariable; use super::lit_expr::LitExpr; use super::location::Ranged; use super::location::WithRange; -use super::parser::MethodArgs; use super::parser::PathList; use crate::sources::connect::json_selection::Alias; use crate::sources::connect::json_selection::NamedSelection; @@ -61,31 +60,22 @@ impl SubSelection { // When the operation contains __typename, it might be used to complete // an entity reference (e.g. `__typename id`) for a subsequent fetch. - // This encodes the typename selection as `__typename: $->echo("Product")` + // This encodes the typename selection as `__typename: $("Product")` + // + // NOTE: For reasons I don't understand, persisted queries may contain + // `__typename` for `_entities` queries. We never want to emit + // `__typename: "_Entity"`, so we'll guard against that case. // // TODO: this must change before we support interfaces and unions // because it will emit the abstract type's name which is invalid. - if field_map.contains_key("__typename") { + if field_map.contains_key("__typename") && selection_set.ty != name!(_Entity) { new_selections.push(NamedSelection::Path { alias: Some(Alias::new("__typename")), path: PathSelection { path: WithRange::new( - PathList::Var( - WithRange::new(KnownVariable::Dollar, None), - WithRange::new( - PathList::Method( - WithRange::new("echo".to_string(), None), - Some(MethodArgs { - args: vec![WithRange::new( - LitExpr::String(selection_set.ty.to_string()), - None, - )], - ..Default::default() - }), - WithRange::new(PathList::Empty, None), - ), - None, - ), + PathList::Expr( + WithRange::new(LitExpr::String(selection_set.ty.to_string()), None), + WithRange::new(PathList::Empty, None), ), None, ), @@ -575,10 +565,10 @@ mod tests { assert_eq!( transformed.to_string(), r###"$.result { - __typename: $->echo("T") + __typename: $("T") id author: { - __typename: $->echo("A") + __typename: $("A") id: authorId } }"### @@ -655,11 +645,11 @@ mod tests { r###"reviews: result { id product: { - __typename: $->echo("Product") + __typename: $("Product") upc: product_upc } author: { - __typename: $->echo("User") + __typename: $("User") id: author_id } }"### diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/README.md b/apollo-router/tests/samples/enterprise/connectors-pqs/README.md new file mode 100644 index 0000000000..bd45538baf --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/README.md @@ -0,0 +1 @@ +This is a regression test for a problem that occurs when using PQs and connectors. See apollo-federation/src/sources/connect/json_selection/selection_set.rs diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/configuration.yaml b/apollo-router/tests/samples/enterprise/connectors-pqs/configuration.yaml new file mode 100644 index 0000000000..8e3e6352fd --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/configuration.yaml @@ -0,0 +1,21 @@ +include_subgraph_errors: + all: true + +preview_connectors: + subgraphs: + connectors: + sources: + one: + override_url: http://localhost:4001 + +telemetry: + exporters: + logging: + stdout: + format: text + +persisted_queries: + enabled: true + log_unknown: true + local_manifests: + - tests/samples/enterprise/connectors-pqs/manifest.json diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/http_snapshots.json b/apollo-router/tests/samples/enterprise/connectors-pqs/http_snapshots.json new file mode 100644 index 0000000000..f2f28d710a --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/http_snapshots.json @@ -0,0 +1,51 @@ +[ + { + "request": { + "method": "GET", + "path": "api/search?query=Boston", + "body": null + }, + "response": { + "status": 200, + "headers": { + "content-type": [ + "application/json; charset=utf-8" + ] + }, + "body": { + "Locations": [ + { + "Coords": { + "Lat": "1.1", + "Lon": "2.2" + } + } + ] + } + } + }, + { + "request": { + "method": "GET", + "path": "weather/1.1,2.2", + "body": null + }, + "response": { + "status": 200, + "headers": { + "content-type": [ + "application/json; charset=utf-8" + ] + }, + "body": { + "currentConditions": { + "snow": 1.1, + "temp": 2.2, + "windspeed": 3.3, + "conditions": "cold", + "snowdepth": 4.4 + } + } + } + } +] \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/manifest.json b/apollo-router/tests/samples/enterprise/connectors-pqs/manifest.json new file mode 100644 index 0000000000..daa02b5a05 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/manifest.json @@ -0,0 +1,12 @@ +{ + "format": "apollo-persisted-query-manifest", + "version": 1, + "operations": [ + { + "id": "87b3393750af706fde5c3bbdf37533012f4d4eeecb82f621b596c34249428153", + "name": "getWeatherData", + "type": "query", + "body": "query getWeatherData($search: String!) {\n geoByAddress(search: $search) {\n lat\n long\n weather {\n conditions\n forecastSnowFall\n temperature\n windSpeed\n currentSnowDepth\n __typename\n }\n __typename\n }\n}" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/plan.json b/apollo-router/tests/samples/enterprise/connectors-pqs/plan.json new file mode 100644 index 0000000000..1c62249226 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/plan.json @@ -0,0 +1,54 @@ +{ + "enterprise": true, + "redis": false, + "snapshot": true, + "actions": [ + { + "type": "Start", + "schema_path": "./supergraph.graphql", + "configuration_path": "./configuration.yaml", + "subgraphs": { + "one": { + "snapshot": { + "path": "./http_snapshots.json", + "base_url": "http://localhost:4001" + } + } + } + }, + { + "type": "Request", + "request": { + "variables": { + "search": "Boston" + }, + "extensions": { + "persistedQuery": { + "version": 1, + "sha256Hash": "87b3393750af706fde5c3bbdf37533012f4d4eeecb82f621b596c34249428153" + } + } + }, + "expected_response": { + "data": { + "geoByAddress": { + "lat": "1.1", + "long": "2.2", + "weather": { + "conditions": "cold", + "forecastSnowFall": 1.1, + "temperature": 2.2, + "windSpeed": 3.3, + "currentSnowDepth": 4.4, + "__typename": "Weather" + }, + "__typename": "Geo" + } + } + } + }, + { + "type": "Stop" + } + ] +} \ No newline at end of file diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.graphql b/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.graphql new file mode 100644 index 0000000000..aec7d5414b --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.graphql @@ -0,0 +1,83 @@ +schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION) + @link(url: "https://specs.apollo.dev/connect/v0.1", for: EXECUTION) + @join__directive(graphs: [CONNECTORS], name: "link", args: {url: "https://specs.apollo.dev/connect/v0.1", import: ["@connect", "@source"]}) + @join__directive(graphs: [CONNECTORS], name: "source", args: {name: "one", http: {baseURL: "http://localhost:4001"}}) +{ + query: Query +} + +directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION + +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, contextArguments: [join__ContextArgument!]) 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 + +type Geo + @join__type(graph: CONNECTORS) +{ + lat: String! + long: String! + weather: Weather +} + +input join__ContextArgument { + name: String! + type: String! + context: String! + selection: join__FieldValue! +} + +scalar join__DirectiveArguments + +scalar join__FieldSet + +scalar join__FieldValue + +enum join__Graph { + CONNECTORS @join__graph(name: "connectors", url: "none") +} + +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: CONNECTORS) +{ + geoByAddress(search: String!): Geo @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "one", http: {GET: "/api/search?query={$args.search}"}, selection: "lat: Locations->first.Coords.Lat\nlong: Locations->first.Coords.Lon\nweather: {\n lat: Locations->first.Coords.Lat\n long: Locations->first.Coords.Lon\n}"}) + getWeatherData(lat: String!, long: String!): Weather @join__directive(graphs: [CONNECTORS], name: "connect", args: {source: "one", http: {GET: "/weather/{$args.lat},{$args.long}"}, selection: "lat: $args.lat\nlong: $args.long\n$.currentConditions {\n forecastSnowFall: snow\n temperature: temp\n windSpeed: windspeed\n conditions\n currentSnowDepth: snowdepth\n}", entity: true}) +} + +type Weather + @join__type(graph: CONNECTORS, key: "lat long", resolvable: false) +{ + lat: String! + long: String! + temperature: Float + windSpeed: Float + conditions: String + forecastSnowFall: Float + currentSnowDepth: Float +} diff --git a/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.yaml b/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.yaml new file mode 100644 index 0000000000..7059969fa1 --- /dev/null +++ b/apollo-router/tests/samples/enterprise/connectors-pqs/supergraph.yaml @@ -0,0 +1,63 @@ +federation_version: =2.10.0-preview.6 +subgraphs: + connectors: + routing_url: none + schema: + sdl: | + extend schema + @link(url: "https://specs.apollo.dev/federation/v2.10", import: ["@key"]) + @link( + url: "https://specs.apollo.dev/connect/v0.1" + import: ["@connect", "@source"] + ) + @source(name: "one", http: { baseURL: "http://localhost:4001" }) + + type Geo { + lat: String! + long: String! + weather: Weather + } + + type Weather @key(fields: "lat long", resolvable: false) { + lat: String! + long: String! + temperature: Float + windSpeed: Float + conditions: String + forecastSnowFall: Float + currentSnowDepth: Float + } + + type Query { + geoByAddress(search: String!): Geo + @connect( + source: "one" + http: { GET: "/api/search?query={$args.search}" } + selection: """ + lat: Locations->first.Coords.Lat + long: Locations->first.Coords.Lon + weather: { + lat: Locations->first.Coords.Lat + long: Locations->first.Coords.Lon + } + """ + ) + + getWeatherData (lat: String!, long: String!) : Weather + @connect( + source: "one" + http: { GET: "/weather/{$args.lat},{$args.long}" } + selection: """ + lat: $args.lat + long: $args.long + $.currentConditions { + forecastSnowFall: snow + temperature: temp + windSpeed: windspeed + conditions + currentSnowDepth: snowdepth + } + """ + entity: true + ) + }