Skip to content

Commit

Permalink
ROVER-316 Upgrade to apollo-language-server 0.4.0 to resolve spurio…
Browse files Browse the repository at this point in the history
…us 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.
  • Loading branch information
jonathanrainer authored Feb 10, 2025
1 parent 605dab0 commit 056c4f9
Show file tree
Hide file tree
Showing 13 changed files with 113 additions and 103 deletions.
83 changes: 42 additions & 41 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down
8 changes: 2 additions & 6 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/composition/supergraph/config/error/subgraph.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::path::PathBuf;
use std::sync::Arc;

use camino::Utf8PathBuf;
Expand All @@ -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<std::io::Error>,
},
Expand Down Expand Up @@ -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),
}
5 changes: 2 additions & 3 deletions src/composition/supergraph/config/full/subgraph/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
20 changes: 9 additions & 11 deletions src/composition/supergraph/config/full/subgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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())
Expand Down
14 changes: 9 additions & 5 deletions src/composition/supergraph/config/lazy/subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -25,11 +24,16 @@ impl LazilyResolvedSubgraph {
) -> Result<LazilyResolvedSubgraph, ResolveSubgraphError> {
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 {
Expand Down
7 changes: 5 additions & 2 deletions src/composition/supergraph/config/scenario.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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),
},
Expand Down
9 changes: 4 additions & 5 deletions src/composition/supergraph/config/unresolved/subgraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
}),
}
Expand Down
Loading

0 comments on commit 056c4f9

Please sign in to comment.