From 056c4f9a2c4ca4b47d750d59f738aab27313b8fd Mon Sep 17 00:00:00 2001 From: Jonathan Rainer Date: Mon, 10 Feb 2025 09:22:31 +0000 Subject: [PATCH] ROVER-316 Upgrade to `apollo-language-server` 0.4.0 to resolve spurious EOF errors (#2389) As per title, this upgrades us to using `apollo-language-server` v0.4.0 (and by extension `apollo-federation-types` v0.15.2), to resolve the issues we see with EOF errors being thrown when we first open files. This patches round the issues with the removal of Camino, by adding conversions where necessary. --- Cargo.lock | 83 ++++++++++--------- Cargo.toml | 6 +- deny.toml | 8 +- .../supergraph/config/error/subgraph.rs | 8 +- .../supergraph/config/full/subgraph/file.rs | 5 +- .../supergraph/config/full/subgraph/mod.rs | 20 ++--- .../supergraph/config/lazy/subgraph.rs | 14 ++-- src/composition/supergraph/config/scenario.rs | 7 +- .../supergraph/config/unresolved/subgraph.rs | 9 +- .../config/unresolved/supergraph.rs | 4 +- src/composition/watchers/watcher/file.rs | 21 +++-- src/composition/watchers/watcher/subgraph.rs | 23 ++--- src/utils/supergraph_config.rs | 8 +- 13 files changed, 113 insertions(+), 103 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9cb5c279b..51b7460ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -155,15 +155,15 @@ checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04" [[package]] name = "apollo-compiler" -version = "1.0.0-beta.24" +version = "1.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "71153ad85c85f7aa63f0e0a5868912c220bb48e4c764556f5841d37fc17b0103" +checksum = "615145d8c4b53ea4e6c15261adbb71cefad3a6834c4a12158f89a484f0769fb2" dependencies = [ "ahash 0.8.11", "apollo-parser 0.8.4", - "ariadne 0.4.1", + "ariadne", "indexmap 2.7.1", - "rowan 0.15.16", + "rowan 0.16.1", "serde", "serde_json_bytes", "thiserror 1.0.69", @@ -172,18 +172,6 @@ dependencies = [ "uuid", ] -[[package]] -name = "apollo-composition" -version = "0.1.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01d82be69fce8ac07544017fe8cd52befc61d4548d3233bd1757984d56c53804" -dependencies = [ - "apollo-compiler", - "apollo-federation", - "apollo-federation-types", - "either", -] - [[package]] name = "apollo-encoder" version = "0.8.0" @@ -196,17 +184,17 @@ dependencies = [ [[package]] name = "apollo-federation" -version = "2.0.0-preview.1" +version = "2.0.0-preview.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c87a5a622646520e2e7df15ed33de1558e9d6eaaa7834eeafd02a8378cd73d2" +checksum = "0fb3caac3f7ca459562c22a95d9a880f45e1f73ecb50827215884cf814845dbb" dependencies = [ "apollo-compiler", "derive_more", "either", - "http 0.2.12", + "hashbrown 0.15.2", + "http 1.2.0", "indexmap 2.7.1", "itertools 0.13.0", - "lazy_static", "line-col", "multimap", "nom", @@ -217,6 +205,7 @@ dependencies = [ "serde", "serde_json", "serde_json_bytes", + "shape", "strum", "strum_macros", "thiserror 1.0.69", @@ -227,11 +216,11 @@ dependencies = [ [[package]] name = "apollo-federation-types" -version = "0.14.1" +version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "80e256949c5f8ebcb6aee00063cb56aaa89cbcfc29cd45fa83519eb9a3290cfe" +checksum = "bc933111fa54f7fb3002c4833d766b4806ac017edf06d4be54b73b8d90d2141c" dependencies = [ - "camino", + "apollo-compiler", "log", "semver", "serde", @@ -244,12 +233,11 @@ dependencies = [ [[package]] name = "apollo-language-server" -version = "0.3.5" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3aadcacf564bfe5c3a0e1444bd893942001e616e1d1e44670ddd04e50117af15" +checksum = "49e6c00b39694b5f0a4cbdc913a0c1516571abe7f25315e03a315d229c378fd1" dependencies = [ "apollo-compiler", - "apollo-composition", "apollo-federation", "apollo-federation-types", "apollo-parser 0.8.4", @@ -307,23 +295,13 @@ version = "1.7.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69f7f8c3906b62b754cd5326047894316021dcfe5a194c8ea52bdd94934a3457" -[[package]] -name = "ariadne" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "44055e597c674aef7cb903b2b9f6e4cba1277ed0d2d61dae7cd52d7ffa81f8e2" -dependencies = [ - "concolor", - "unicode-width 0.1.14", - "yansi", -] - [[package]] name = "ariadne" version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "31beedec3ce83ae6da3a79592b3d8d7afd146a5b15bb9bb940279aced60faa89" dependencies = [ + "concolor", "unicode-width 0.1.14", "yansi", ] @@ -1152,7 +1130,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c" dependencies = [ "lazy_static", - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] @@ -2031,6 +2009,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a0d2fde1f7b3d48b8395d5f2de76c18a528bd6a9cdde438df747bfcba3e05d6f" + [[package]] name = "foreign-types" version = "0.3.2" @@ -2445,6 +2429,11 @@ name = "hashbrown" version = "0.15.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bf151400ff0baff5465007dd2f3e717f3fe502074ca563069ce3a6629d07b289" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] [[package]] name = "headers" @@ -4930,7 +4919,7 @@ dependencies = [ "apollo-encoder", "apollo-federation-types", "apollo-parser 0.8.4", - "ariadne 0.5.0", + "ariadne", "backoff", "buildstructor", "camino", @@ -5576,6 +5565,18 @@ dependencies = [ "digest", ] +[[package]] +name = "shape" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "27e114fc498dd1dcc20b12a9571a7b9b78396760e44b7a9024afa3a04ce26763" +dependencies = [ + "apollo-compiler", + "indexmap 2.7.1", + "serde_json", + "serde_json_bytes", +] + [[package]] name = "sharded-slab" version = "0.1.7" @@ -5718,7 +5719,7 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "03c3c6b7927ffe7ecaa769ee0e3994da3b8cafc8f444578982c83ecb161af917" dependencies = [ - "heck 0.4.1", + "heck 0.5.0", "proc-macro2", "quote", "syn 2.0.98", @@ -6997,7 +6998,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index c51903e70..c94ef922a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,9 +69,9 @@ apollo-parser = "0.8" apollo-encoder = "0.8" # https://github.com/apollographql/federation-rs -apollo-federation-types = "0.14.1" +apollo-federation-types = "0.15.2" -apollo-language-server = { version = "0.3.5", default-features = false, features = ["tokio"] } +apollo-language-server = { version = "0.4.0", default-features = false, features = ["tokio"] } # crates.io dependencies anyhow = "1" @@ -88,7 +88,7 @@ buildstructor = "0.5.4" bytes = "1.8.0" cargo_metadata = "0.18" calm_io = "0.1" -camino = "1" +camino = { version = "1", features = ["serde1"] } clap = "4" chrono = "0.4" ci_info = "0.14" diff --git a/deny.toml b/deny.toml index bfcdb758c..2bd50d7a5 100644 --- a/deny.toml +++ b/deny.toml @@ -52,8 +52,8 @@ allow = [ "ISC", "MIT", "MPL-2.0", - "Unicode-DFS-2016", - "Unicode-3.0" + "Unicode-3.0", + "Zlib" ] confidence-threshold = 0.8 private = { ignore = true } @@ -78,10 +78,6 @@ allow = ["Elastic-2.0"] name = "apollo-federation" allow = ["Elastic-2.0"] -[[licenses.exceptions]] -name = "apollo-composition" -allow = ["Elastic-2.0"] - # This section is considered when running `cargo deny check bans`. # More documentation about the 'bans' section can be found here: # https://embarkstudios.github.io/cargo-deny/checks/bans/cfg.html diff --git a/src/composition/supergraph/config/error/subgraph.rs b/src/composition/supergraph/config/error/subgraph.rs index 30eab52f0..4e2fcbcb3 100644 --- a/src/composition/supergraph/config/error/subgraph.rs +++ b/src/composition/supergraph/config/error/subgraph.rs @@ -1,4 +1,3 @@ -use std::path::PathBuf; use std::sync::Arc; use camino::Utf8PathBuf; @@ -16,9 +15,9 @@ pub enum ResolveSubgraphError { /// Supplied path to the supergraph config file supergraph_config_path: Utf8PathBuf, /// Supplied path to the subgraph schema file - path: PathBuf, + path: Utf8PathBuf, /// The result of joining the paths together, that caused the failure - joined_path: PathBuf, + joined_path: Utf8PathBuf, /// The source error source: Arc, }, @@ -95,4 +94,7 @@ pub enum ResolveSubgraphError { /// Error encountered if we can't parse a URL properly in the introspection case #[error(transparent)] ParsingSubgraphUrlError(#[from] url::ParseError), + /// Error encountered if we can't parse a PathBuf into a Utf8PathBuf + #[error(transparent)] + ParsingUt8FilePathError(#[from] camino::FromPathBufError), } diff --git a/src/composition/supergraph/config/full/subgraph/file.rs b/src/composition/supergraph/config/full/subgraph/file.rs index 182528cd5..5b58fb545 100644 --- a/src/composition/supergraph/config/full/subgraph/file.rs +++ b/src/composition/supergraph/config/full/subgraph/file.rs @@ -10,9 +10,8 @@ use rover_std::Fs; use tower::Service; use super::FullyResolvedSubgraph; -use crate::composition::supergraph::config::{ - error::ResolveSubgraphError, unresolved::UnresolvedSubgraph, -}; +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::unresolved::UnresolvedSubgraph; /// Service that resolves a file-based subgraph #[derive(Clone, Builder)] diff --git a/src/composition/supergraph/config/full/subgraph/mod.rs b/src/composition/supergraph/config/full/subgraph/mod.rs index 012e74d16..d1340e776 100644 --- a/src/composition/supergraph/config/full/subgraph/mod.rs +++ b/src/composition/supergraph/config/full/subgraph/mod.rs @@ -7,21 +7,19 @@ use buildstructor::buildstructor; use camino::Utf8PathBuf; use derive_getters::Getters; use rover_client::shared::GraphRef; -use tower::{service_fn, util::BoxCloneService, Service, ServiceExt}; +use tower::util::BoxCloneService; +use tower::{service_fn, Service, ServiceExt}; pub mod file; pub mod introspect; pub mod remote; -use self::{ - file::ResolveFileSubgraph, - introspect::{MakeResolveIntrospectSubgraphRequest, ResolveIntrospectSubgraphFactory}, - remote::ResolveRemoteSubgraph, -}; -use crate::composition::supergraph::config::{ - error::ResolveSubgraphError, resolver::fetch_remote_subgraph::FetchRemoteSubgraphFactory, - unresolved::UnresolvedSubgraph, -}; +use self::file::ResolveFileSubgraph; +use self::introspect::{MakeResolveIntrospectSubgraphRequest, ResolveIntrospectSubgraphFactory}; +use self::remote::ResolveRemoteSubgraph; +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::resolver::fetch_remote_subgraph::FetchRemoteSubgraphFactory; +use crate::composition::supergraph::config::unresolved::UnresolvedSubgraph; /// Alias for a [`tower::Service`] that fully resolves a subgraph pub type FullyResolveSubgraphService = @@ -70,7 +68,7 @@ impl FullyResolvedSubgraph { SchemaSource::File { file } => { let service = ResolveFileSubgraph::builder() .supergraph_config_root(supergraph_config_root) - .path(file.clone()) + .path(Utf8PathBuf::try_from(file)?) .unresolved_subgraph(unresolved_subgraph.clone()) .build(); Ok(service.boxed_clone()) diff --git a/src/composition/supergraph/config/lazy/subgraph.rs b/src/composition/supergraph/config/lazy/subgraph.rs index 8fbf1e60d..6816062ce 100644 --- a/src/composition/supergraph/config/lazy/subgraph.rs +++ b/src/composition/supergraph/config/lazy/subgraph.rs @@ -3,9 +3,8 @@ use buildstructor::Builder; use camino::Utf8PathBuf; use derive_getters::Getters; -use crate::composition::supergraph::config::{ - error::ResolveSubgraphError, unresolved::UnresolvedSubgraph, -}; +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::unresolved::UnresolvedSubgraph; /// A subgraph config that has had its file paths validated and /// confirmed to be relative to a supergraph config file @@ -25,11 +24,16 @@ impl LazilyResolvedSubgraph { ) -> Result { match unresolved_subgraph.schema() { SchemaSource::File { file } => { - let file = unresolved_subgraph.resolve_file_path(supergraph_config_root, file)?; + let file = unresolved_subgraph.resolve_file_path( + &supergraph_config_root.clone(), + &Utf8PathBuf::try_from(file.clone())?, + )?; Ok(LazilyResolvedSubgraph { name: unresolved_subgraph.name().to_string(), routing_url: unresolved_subgraph.routing_url().clone(), - schema: SchemaSource::File { file }, + schema: SchemaSource::File { + file: file.into_std_path_buf(), + }, }) } _ => Ok(LazilyResolvedSubgraph { diff --git a/src/composition/supergraph/config/scenario.rs b/src/composition/supergraph/config/scenario.rs index e72541e50..56b60859e 100644 --- a/src/composition/supergraph/config/scenario.rs +++ b/src/composition/supergraph/config/scenario.rs @@ -1,4 +1,7 @@ -use std::{collections::HashMap, io::Write, path::Path, str::FromStr}; +use std::collections::HashMap; +use std::io::Write; +use std::path::Path; +use std::str::FromStr; use anyhow::Result; use apollo_federation_types::config::{SchemaSource, SubgraphConfig}; @@ -234,7 +237,7 @@ pub fn file_subgraph_scenario( subgraph_name, SubgraphConfig { schema: SchemaSource::File { - file: schema_file_path, + file: schema_file_path.into_std_path_buf(), }, routing_url: Some(routing_url), }, diff --git a/src/composition/supergraph/config/unresolved/subgraph.rs b/src/composition/supergraph/config/unresolved/subgraph.rs index 5dbe53e1e..f543e2e9b 100644 --- a/src/composition/supergraph/config/unresolved/subgraph.rs +++ b/src/composition/supergraph/config/unresolved/subgraph.rs @@ -4,9 +4,8 @@ use apollo_federation_types::config::{SchemaSource, SubgraphConfig}; use camino::Utf8PathBuf; use derive_getters::Getters; -use crate::composition::supergraph::config::{ - error::ResolveSubgraphError, lazy::LazilyResolvedSubgraph, -}; +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::lazy::LazilyResolvedSubgraph; /// Represents a `SubgraphConfig` that needs to be resolved, either fully or lazily #[derive(Clone, Debug, Getters)] @@ -39,8 +38,8 @@ impl UnresolvedSubgraph { Err(err) => Err(ResolveSubgraphError::FileNotFound { subgraph_name: self.name.to_string(), supergraph_config_path: root.clone(), - path: path.as_std_path().to_path_buf(), - joined_path: joined_path.as_std_path().to_path_buf(), + path: path.clone(), + joined_path, source: Arc::new(err), }), } diff --git a/src/composition/supergraph/config/unresolved/supergraph.rs b/src/composition/supergraph/config/unresolved/supergraph.rs index 67429a8be..311567ba4 100644 --- a/src/composition/supergraph/config/unresolved/supergraph.rs +++ b/src/composition/supergraph/config/unresolved/supergraph.rs @@ -507,7 +507,7 @@ mod tests { .schema(file_subgraph_scenario.sdl.clone()) .name(file_subgraph_name.to_string()) .schema_source(SchemaSource::File { - file: file_subgraph_scenario.schema_file_path.clone(), + file: file_subgraph_scenario.schema_file_path.into_std_path_buf(), }) .build(), ), @@ -903,7 +903,7 @@ mod tests { .schema(SchemaSource::File { file: supergraph_config_root_dir_path .join(file_subgraph_scenario.schema_file_path) - .canonicalize_utf8()?, + .canonicalize()?, }) .name(file_subgraph_name.clone()) .routing_url(file_subgraph_scenario.routing_url) diff --git a/src/composition/watchers/watcher/file.rs b/src/composition/watchers/watcher/file.rs index a9582d696..aed55b94f 100644 --- a/src/composition/watchers/watcher/file.rs +++ b/src/composition/watchers/watcher/file.rs @@ -1,6 +1,7 @@ use camino::Utf8PathBuf; use derive_getters::Getters; -use futures::{stream::BoxStream, StreamExt, TryFutureExt}; +use futures::stream::BoxStream; +use futures::{StreamExt, TryFutureExt}; use rover_std::{errln, Fs, RoverStdError}; use tap::TapFallible; use tokio::sync::mpsc::unbounded_channel; @@ -8,9 +9,9 @@ use tokio_stream::wrappers::UnboundedReceiverStream; use tokio_util::sync::CancellationToken; use tower::{Service, ServiceExt}; -use crate::composition::supergraph::config::{ - error::ResolveSubgraphError, - full::{FullyResolveSubgraphService, FullyResolvedSubgraph}, +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::full::{ + FullyResolveSubgraphService, FullyResolvedSubgraph, }; /// File watcher specifically for files related to composition @@ -152,7 +153,9 @@ impl SubgraphFileWatcher { #[cfg(test)] mod tests { - use std::{fs::OpenOptions, io::Write, time::Duration}; + use std::fs::OpenOptions; + use std::io::Write; + use std::time::Duration; use apollo_federation_types::config::{SchemaSource, SubgraphConfig}; use speculoos::prelude::*; @@ -179,7 +182,9 @@ mod tests { .unresolved_subgraph(UnresolvedSubgraph::new( subgraph_name.to_string(), SubgraphConfig { - schema: SchemaSource::File { file: path.clone() }, + schema: SchemaSource::File { + file: path.clone().into_std_path_buf(), + }, routing_url: Some(routing_url.to_string()), }, )) @@ -220,7 +225,9 @@ mod tests { .name(subgraph_name.to_string()) .routing_url(routing_url.to_string()) .schema(sdl.to_string()) - .schema_source(SchemaSource::File { file: path }) + .schema_source(SchemaSource::File { + file: path.into_std_path_buf(), + }) .build(); assert_that!(&output) .is_ok() diff --git a/src/composition/watchers/watcher/subgraph.rs b/src/composition/watchers/watcher/subgraph.rs index 9a0dfcf6a..85a0e1fbd 100644 --- a/src/composition/watchers/watcher/subgraph.rs +++ b/src/composition/watchers/watcher/subgraph.rs @@ -1,7 +1,9 @@ use std::time::Duration; use apollo_federation_types::config::SchemaSource; -use futures::{stream::BoxStream, StreamExt}; +use camino::Utf8PathBuf; +use futures::stream::BoxStream; +use futures::StreamExt; use rover_client::operations::subgraph::introspect::SubgraphIntrospectError; use rover_std::{infoln, RoverStdError}; use tap::TapFallible; @@ -9,15 +11,14 @@ use tokio::sync::mpsc::UnboundedSender; use tokio_util::sync::CancellationToken; use tower::{Service, ServiceExt}; -use super::{file::SubgraphFileWatcher, introspection::SubgraphIntrospection}; -use crate::{ - composition::supergraph::config::{ - error::ResolveSubgraphError, - full::{FullyResolveSubgraphService, FullyResolvedSubgraph}, - lazy::LazilyResolvedSubgraph, - }, - subtask::SubtaskHandleUnit, +use super::file::SubgraphFileWatcher; +use super::introspection::SubgraphIntrospection; +use crate::composition::supergraph::config::error::ResolveSubgraphError; +use crate::composition::supergraph::config::full::{ + FullyResolveSubgraphService, FullyResolvedSubgraph, }; +use crate::composition::supergraph::config::lazy::LazilyResolvedSubgraph; +use crate::subtask::SubtaskHandleUnit; #[derive(thiserror::Error, Debug)] pub enum SubgraphFetchError { @@ -84,10 +85,10 @@ impl SubgraphWatcher { // directives from introspection (but not when the source is a file) match subgraph.schema() { SchemaSource::File { file } => { - infoln!("Watching {} for changes", file.as_std_path().display()); + infoln!("Watching {} for changes", file.display()); Self { watcher: SubgraphWatcherKind::File(SubgraphFileWatcher::new( - file.clone(), + Utf8PathBuf::try_from(file.clone()).unwrap(), resolver, )), } diff --git a/src/utils/supergraph_config.rs b/src/utils/supergraph_config.rs index 43d1b3687..8e1da9502 100644 --- a/src/utils/supergraph_config.rs +++ b/src/utils/supergraph_config.rs @@ -147,6 +147,7 @@ fn correctly_resolve_paths( .map( |(subgraph_name, subgraph_config)| match subgraph_config.schema { SchemaSource::File { file } => { + let file = Utf8PathBuf::try_from(file)?; let potential_canonical_file = root_to_resolve_from.join(&file); match potential_canonical_file.canonicalize_utf8() { Ok(canonical_file_name) => Ok(( @@ -154,7 +155,7 @@ fn correctly_resolve_paths( SubgraphConfig { routing_url: subgraph_config.routing_url, schema: SchemaSource::File { - file: canonical_file_name, + file: canonical_file_name.into_std_path_buf(), }, }, )), @@ -517,9 +518,7 @@ mod test_get_supergraph_config { .zip([schema_path.canonicalize().unwrap()]) { match subgraph_config.schema { - SchemaSource::File { file } => { - assert_that!(file.as_std_path()).is_equal_to(b.as_path()) - } + SchemaSource::File { file } => assert_that!(file).is_equal_to(b), _ => panic!("Incorrect schema source found"), } } @@ -799,6 +798,7 @@ pub(crate) async fn resolve_supergraph_yaml( let cloned_subgraph_name = subgraph_name.to_string(); let result = match &subgraph_data.schema { SchemaSource::File { file } => { + let file = Utf8PathBuf::try_from(file.clone())?; let relative_schema_path = match unresolved_supergraph_yaml { FileDescriptorType::File(config_path) => match config_path.parent() { Some(parent) => {