Skip to content

Commit

Permalink
fix: connectors, pqs, and __typename
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lennyburdette committed Feb 12, 2025
1 parent 4a8feb5 commit 19dbf04
Show file tree
Hide file tree
Showing 8 changed files with 299 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
),
Expand Down Expand Up @@ -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
}
}"###
Expand Down Expand Up @@ -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
}
}"###
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
}
}
}
}
]
Original file line number Diff line number Diff line change
@@ -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}"
}
]
}
54 changes: 54 additions & 0 deletions apollo-router/tests/samples/enterprise/connectors-pqs/plan.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
)
}

0 comments on commit 19dbf04

Please sign in to comment.