ROVER-245 Remove FullyResolvedSubgraphs

This structure was not quite right for what we were looking for
and instead is better served as a BTreeMap. In addition
we make sure we keep passing around the `routing_url` so composition
should continue to work without having to use the `localhost` hack.

src/command/lsp/mod.rs

@@ -1,3 +1,4 @@
+use futures::StreamExt;
 mod errors;
 
 use crate::command::lsp::errors::SupergraphConfigLazyResolutionError;
@@ -27,7 +28,6 @@ use apollo_federation_types::config::FederationVersion;
 use apollo_language_server::{ApolloLanguageServer, Config};
 use camino::Utf8PathBuf;
 use clap::Parser;
-use futures::StreamExt;
 use rover_client::blocking::StudioClient;
 use serde::Serialize;
 use std::collections::HashMap;
@@ -207,7 +207,7 @@ async fn start_composition(
             .setup_supergraph_config_watcher(lazily_resolved_supergraph_config.clone())
             .setup_composition_watcher(
                 lazily_resolved_supergraph_config
-                    .fully_resolve_subgraphs(&client_config, &studio_client)
+                    .extract_subgraphs_as_sdls(&client_config, &studio_client)
                     .await
                     .map_err(ResolveSupergraphConfigError::ResolveSubgraphs)?,
                 supergraph_binary,

src/command/supergraph/compose/do_compose.rs

@@ -42,9 +42,7 @@ use crate::{
         supergraph::{
             binary::{OutputTarget, SupergraphBinary},
             config::{
-                full::{
-                    FullyResolvedSubgraph, FullyResolvedSubgraphs, FullyResolvedSupergraphConfig,
-                },
+                full::{FullyResolvedSubgraph, FullyResolvedSupergraphConfig},
                 resolver::SupergraphConfigResolver,
                 unresolved::UnresolvedSupergraphConfig,
             },

src/composition/runner/mod.rs

@@ -3,51 +3,46 @@
 
 #![warn(missing_docs)]
 
-use std::{collections::BTreeMap, env::current_dir, fmt::Debug, io::stdin};
-
-//use std::{env::current_dir, fs::File, process::Command, str};
-
-use anyhow::anyhow;
-use apollo_federation_types::config::{FederationVersion, SupergraphConfig};
-use buildstructor::Builder;
-use camino::Utf8PathBuf;
-use futures::stream::{BoxStream, StreamExt};
-use rover_client::shared::GraphRef;
-use rover_std::warnln;
-use tempfile::tempdir;
-
+use self::state::SetupSubgraphWatchers;
+use crate::command::supergraph::compose::CompositionOutput;
+use crate::composition::supergraph::config::full::FullyResolvedSubgraph;
+use crate::composition::supergraph::config::resolver::SupergraphConfigResolver;
+use crate::composition::supergraph::install::InstallSupergraph;
+use crate::options::LicenseAccepter;
+use crate::utils::effect::exec::TokioCommand;
+use crate::utils::effect::install::InstallBinary;
+use crate::utils::effect::read_file::FsReadFile;
+use crate::utils::effect::write_file::FsWriteFile;
+use crate::utils::parsers::FileDescriptorType;
 use crate::{
-    command::supergraph::compose::CompositionOutput,
-    composition::{
-        supergraph::install::InstallSupergraph,
-        watchers::watcher::{file::FileWatcher, supergraph_config::SupergraphConfigWatcher},
+    composition::watchers::watcher::{
+        file::FileWatcher, supergraph_config::SupergraphConfigWatcher,
     },
-    options::{LicenseAccepter, ProfileOpt},
+    options::ProfileOpt,
     subtask::{Subtask, SubtaskRunStream, SubtaskRunUnit},
     utils::{
         client::StudioClientConfig,
-        effect::{
-            exec::{ExecCommand, TokioCommand},
-            install::InstallBinary,
-            read_file::{FsReadFile, ReadFile},
-            write_file::{FsWriteFile, WriteFile},
-        },
-        parsers::FileDescriptorType,
+        effect::{exec::ExecCommand, read_file::ReadFile, write_file::WriteFile},
     },
     RoverError, RoverResult,
 };
-
-use self::state::SetupSubgraphWatchers;
+use anyhow::anyhow;
+use apollo_federation_types::config::{FederationVersion, SupergraphConfig};
+use buildstructor::Builder;
+use camino::Utf8PathBuf;
+use futures::stream::{BoxStream, StreamExt};
+use rover_client::shared::GraphRef;
+use rover_std::warnln;
+use std::env::current_dir;
+use std::io::stdin;
+use std::{collections::BTreeMap, fmt::Debug};
+use tempfile::tempdir;
 
 use super::{
     events::CompositionEvent,
     supergraph::{
         binary::{OutputTarget, SupergraphBinary},
-        config::{
-            full::FullyResolvedSubgraphs,
-            lazy::{LazilyResolvedSubgraph, LazilyResolvedSupergraphConfig},
-            resolver::SupergraphConfigResolver,
-        },
+        config::lazy::{LazilyResolvedSubgraph, LazilyResolvedSupergraphConfig},
     },
     watchers::{composition::CompositionWatcher, subgraphs::SubgraphWatchers},
 };
@@ -259,7 +254,7 @@ impl Runner<state::SetupCompositionWatcher> {
     #[allow(clippy::too_many_arguments)]
     pub fn setup_composition_watcher<ReadF, ExecC, WriteF>(
         self,
-        subgraphs: FullyResolvedSubgraphs,
+        subgraphs: BTreeMap<String, FullyResolvedSubgraph>,
         supergraph_binary: SupergraphBinary,
         exec_command: ExecC,
         read_file: ReadF,

src/composition/supergraph/config/full/mod.rs

@@ -5,9 +5,7 @@
 //! the supergraph binary expects.
 
 mod subgraph;
-mod subgraphs;
 mod supergraph;
 
 pub use subgraph::*;
-pub use subgraphs::*;
 pub use supergraph::*;

src/composition/supergraph/config/full/supergraph.rs

@@ -6,6 +6,7 @@ use derive_getters::Getters;
 use futures::{stream, StreamExt, TryFutureExt};
 use itertools::Itertools;
 
+use super::FullyResolvedSubgraph;
 use crate::{
     composition::supergraph::config::{
         error::ResolveSubgraphError, resolver::ResolveSupergraphConfigError,
@@ -14,8 +15,6 @@ use crate::{
     utils::effect::{fetch_remote_subgraph::FetchRemoteSubgraph, introspect::IntrospectSubgraph},
 };
 
-use super::FullyResolvedSubgraph;
-
 /// Represents a [`SupergraphConfig`] that has a known [`FederationVersion`] and
 /// its subgraph [`SchemaSource`]s reduced to [`SchemaSource::Sdl`]
 #[derive(Clone, Debug, Eq, PartialEq, Getters)]

src/composition/supergraph/config/lazy/supergraph.rs

@@ -7,7 +7,7 @@ use futures::{stream, StreamExt};
 use itertools::Itertools;
 
 use super::LazilyResolvedSubgraph;
-use crate::composition::supergraph::config::full::{FullyResolvedSubgraph, FullyResolvedSubgraphs};
+use crate::composition::supergraph::config::full::FullyResolvedSubgraph;
 use crate::composition::supergraph::config::{
     error::ResolveSubgraphError, unresolved::UnresolvedSupergraphConfig,
 };
@@ -58,17 +58,13 @@ impl LazilyResolvedSupergraphConfig {
         }
     }
 
-    /// Fully resolves a [`LazilyResolvedSupergraphConfig`] into a [`FullyResolvedSubgraphs`]
-    /// by retrieving all the schemas as strings that represent the SDL.
-    ///
-    /// This is a one-way conversion by design as FullyResolvedSubgraphs are just SDL representations
-    /// of whatever source was used to produce them. As such it's impossible to go backwards once
-    /// you are left with the bare SDL representation of the subgraph.
-    pub async fn fully_resolve_subgraphs(
+    /// Fully resolves a [`LazilyResolvedSupergraphConfig`] into a [`BTreeMap<String, FullyResolvedSubgraph>`]
+    /// by retrieving all the schemas as strings
+    pub async fn extract_subgraphs_as_sdls(
         self,
         introspect_subgraph_impl: &impl IntrospectSubgraph,
         fetch_remote_subgraph_impl: &impl FetchRemoteSubgraph,
-    ) -> Result<FullyResolvedSubgraphs, Vec<ResolveSubgraphError>> {
+    ) -> Result<BTreeMap<String, FullyResolvedSubgraph>, Vec<ResolveSubgraphError>> {
         let subgraphs = stream::iter(self.subgraphs.into_iter().map(
             |(name, lazily_resolved_subgraph)| async {
                 let result = FullyResolvedSubgraph::fully_resolve(
@@ -89,7 +85,7 @@ impl LazilyResolvedSupergraphConfig {
             Vec<ResolveSubgraphError>,
         ) = subgraphs.into_iter().partition_result();
         if errors.is_empty() {
-            Ok(subgraphs.into())
+            Ok(subgraphs.into_iter().collect())
         } else {
             Err(errors)
         }

src/composition/watchers/composition.rs

@@ -4,18 +4,17 @@ use buildstructor::Builder;
 use camino::Utf8PathBuf;
 use futures::stream::BoxStream;
 use rover_std::errln;
+use std::collections::BTreeMap;
 use tap::TapFallible;
 use tokio::{sync::mpsc::UnboundedSender, task::AbortHandle};
 use tokio_stream::StreamExt;
 
+use crate::composition::supergraph::config::full::FullyResolvedSubgraph;
 use crate::composition::{CompositionSubgraphAdded, CompositionSubgraphRemoved};
 use crate::{
     composition::{
         events::CompositionEvent,
-        supergraph::{
-            binary::{OutputTarget, SupergraphBinary},
-            config::full::FullyResolvedSubgraphs,
-        },
+        supergraph::binary::{OutputTarget, SupergraphBinary},
         watchers::subgraphs::SubgraphEvent,
     },
     subtask::SubtaskHandleStream,
@@ -24,7 +23,7 @@ use crate::{
 
 #[derive(Builder, Debug)]
 pub struct CompositionWatcher<ReadF, ExecC, WriteF> {
-    subgraphs: FullyResolvedSubgraphs,
+    subgraphs: BTreeMap<String, FullyResolvedSubgraph>,
     supergraph_binary: SupergraphBinary,
     output_target: OutputTarget,
     exec_command: ExecC,
@@ -56,8 +55,16 @@ where
                         SubgraphEvent::SubgraphChanged(subgraph_schema_changed) => {
                             let name = subgraph_schema_changed.name();
                             let sdl = subgraph_schema_changed.sdl();
+                            let routing_url = subgraph_schema_changed.routing_url();
                             if subgraphs
-                                .upsert_subgraph(name.to_string(), sdl.to_string())
+                                .insert(
+                                    name.clone(),
+                                    FullyResolvedSubgraph::new(
+                                        sdl.clone(),
+                                        routing_url.clone(),
+                                        None,
+                                    ),
+                                )
                                 .is_none()
                             {
                                 let _ = sender
@@ -72,7 +79,7 @@ where
                         }
                         SubgraphEvent::SubgraphRemoved(subgraph_removed) => {
                             let name = subgraph_removed.name();
-                            subgraphs.remove_subgraph(name);
+                            subgraphs.remove(name);
                             let _ = sender
                                 .send(CompositionEvent::SubgraphRemoved(
                                     CompositionSubgraphRemoved { name: name.clone() },
@@ -81,7 +88,7 @@ where
                         }
                     }
 
-                    let supergraph_config = SupergraphConfig::from(subgraphs.clone());
+                    let supergraph_config = convert_to_supergraph_config(subgraphs.clone());
                     let supergraph_config_yaml = serde_yaml::to_string(&supergraph_config);
 
                     let supergraph_config_yaml = match supergraph_config_yaml {
@@ -137,6 +144,15 @@ where
     }
 }
 
+fn convert_to_supergraph_config(
+    _subgraphs: BTreeMap<String, FullyResolvedSubgraph>,
+) -> SupergraphConfig {
+    SupergraphConfig::new(
+        _subgraphs.into_iter().map(|(k, v)| (k, v.into())).collect(),
+        None,
+    )
+}
+
 #[cfg(test)]
 mod tests {
     use std::{
@@ -161,18 +177,18 @@ mod tests {
 
     use super::CompositionWatcher;
     use crate::composition::CompositionSubgraphAdded;
+    use crate::subtask::SubtaskRunStream;
     use crate::{
         composition::{
             events::CompositionEvent,
             supergraph::{
                 binary::{OutputTarget, SupergraphBinary},
-                config::full::FullyResolvedSubgraphs,
                 version::SupergraphVersion,
             },
             test::{default_composition_json, default_composition_success},
             watchers::subgraphs::{SubgraphEvent, SubgraphSchemaChanged},
         },
-        subtask::{Subtask, SubtaskRunStream},
+        subtask::Subtask,
         utils::effect::{
             exec::MockExecCommand, read_file::MockReadFile, write_file::MockWriteFile,
         },
@@ -190,7 +206,7 @@ mod tests {
         let temp_dir = assert_fs::TempDir::new()?;
         let temp_dir_path = Utf8PathBuf::from_path_buf(temp_dir.to_path_buf()).unwrap();
 
-        let subgraphs = FullyResolvedSubgraphs::new(BTreeMap::new());
+        let subgraphs = BTreeMap::new();
         let supergraph_version = SupergraphVersion::new(Version::from_str("2.8.0").unwrap());
 
         let supergraph_binary = SupergraphBinary::builder()
@@ -251,7 +267,11 @@ mod tests {
             .build();
 
         let subgraph_change_events: BoxStream<SubgraphEvent> = once(async {
-            SubgraphEvent::SubgraphChanged(SubgraphSchemaChanged::new(subgraph_name, subgraph_sdl))
+            SubgraphEvent::SubgraphChanged(SubgraphSchemaChanged::new(
+                subgraph_name,
+                subgraph_sdl,
+                None,
+            ))
         })
         .boxed();
         let (mut composition_messages, composition_subtask) = Subtask::new(composition_handler);