ROVER-245 Add new InMemory OutputTarget

At present, we only support 2 composition output targets, this adds a third, which means that the output of composition only exists in memory. This means we don't have to write/manage temporary files, and we don't compete with the LSP for usage of stdout.
apollographql · Dec 20, 2024 · 12f9c70 · 12f9c70
1 parent d772c17
commit 12f9c70
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 14 deletions.
diff --git a/src/command/lsp/mod.rs b/src/command/lsp/mod.rs
@@ -20,6 +20,7 @@ use crate::command::lsp::errors::StartCompositionError;
 use crate::command::lsp::errors::StartCompositionError::SupergraphYamlUrlConversionFailed;
 use crate::composition::events::CompositionEvent;
 use crate::composition::runner::Runner;
+use crate::composition::supergraph::binary::OutputTarget;
 use crate::composition::supergraph::config::lazy::LazilyResolvedSupergraphConfig;
 use crate::composition::supergraph::config::resolver::fetch_remote_subgraph::MakeFetchRemoteSubgraph;
 use crate::composition::supergraph::config::resolver::fetch_remote_subgraphs::MakeFetchRemoteSubgraphs;
@@ -231,6 +232,7 @@ async fn start_composition(
                 FsWriteFile::default(),
                 Utf8PathBuf::try_from(temp_dir())?,
                 true,
+                OutputTarget::InMemory,
             )
             .run();
 

diff --git a/src/composition/pipeline.rs b/src/composition/pipeline.rs
@@ -257,6 +257,7 @@ impl CompositionPipeline<state::Run> {
                 write_file,
                 output_dir,
                 compose_on_initialisation,
+                OutputTarget::Stdout,
             );
         Ok(runner)
     }

diff --git a/src/composition/runner/mod.rs b/src/composition/runner/mod.rs
@@ -20,6 +20,7 @@ use super::{
     },
     watchers::{composition::CompositionWatcher, subgraphs::SubgraphWatchers},
 };
+use crate::composition::supergraph::binary::OutputTarget;
 use crate::{
     composition::watchers::watcher::{
         file::FileWatcher, supergraph_config::SupergraphConfigWatcher,
@@ -124,6 +125,7 @@ impl Runner<state::SetupCompositionWatcher> {
         write_file: WriteF,
         temp_dir: Utf8PathBuf,
         compose_on_initialisation: bool,
+        output_target: OutputTarget,
     ) -> Runner<state::Run<ExecC, ReadF, WriteF>>
     where
         ExecC: ExecCommand + Debug + Eq + PartialEq + Send + Sync + 'static,
@@ -139,6 +141,7 @@ impl Runner<state::SetupCompositionWatcher> {
             .write_file(write_file)
             .temp_dir(temp_dir)
             .compose_on_initialisation(compose_on_initialisation)
+            .output_target(output_target)
             .build();
         Runner {
             state: state::Run {

diff --git a/src/composition/supergraph/binary.rs b/src/composition/supergraph/binary.rs
@@ -1,4 +1,5 @@
 use std::fmt::Debug;
+use std::process::Stdio;
 
 use apollo_federation_types::{
     config::FederationVersion,
@@ -9,6 +10,8 @@ use camino::Utf8PathBuf;
 use rover_std::warnln;
 use tap::TapFallible;
 
+use super::version::SupergraphVersion;
+use crate::utils::effect::exec::ExecCommandOutput;
 use crate::{
     composition::{CompositionError, CompositionSuccess},
     utils::effect::{
@@ -17,12 +20,14 @@ use crate::{
     },
 };
 
-use super::version::SupergraphVersion;
-
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum OutputTarget {
+    /// Output to a file, given by the parameter
     File(Utf8PathBuf),
+    /// Output to stdout (inappropriate for LSP usage because stdout is reserved for the LSP itself)
     Stdout,
+    /// Produce an output that only exists in memory, for passing to events
+    InMemory,
 }
 
 impl OutputTarget {
@@ -37,6 +42,7 @@ impl OutputTarget {
                 }
             }
             OutputTarget::Stdout => OutputTarget::Stdout,
+            OutputTarget::InMemory => OutputTarget::InMemory,
         }
     }
 }
@@ -89,14 +95,20 @@ impl SupergraphBinary {
         supergraph_config_path: Utf8PathBuf,
     ) -> Result<CompositionSuccess, CompositionError> {
         let args = self.prepare_compose_args(output_target, &supergraph_config_path);
+        let config = match output_target {
+            OutputTarget::File(_) | OutputTarget::Stdout => ExecCommandConfig::builder()
+                .exe(self.exe.clone())
+                .args(args)
+                .build(),
+            OutputTarget::InMemory => ExecCommandConfig::builder()
+                .exe(self.exe.clone())
+                .args(args)
+                .output(ExecCommandOutput::builder().stdout(Stdio::piped()).build())
+                .build(),
+        };
 
         let output = exec_impl
-            .exec_command(
-                ExecCommandConfig::builder()
-                    .exe(self.exe.clone())
-                    .args(args)
-                    .build(),
-            )
+            .exec_command(config)
             .await
             .tap_err(|err| tracing::error!("{:?}", err))
             .map_err(|err| CompositionError::Binary {
@@ -122,7 +134,7 @@ impl SupergraphBinary {
                         error: Box::new(err),
                     })?
             }
-            OutputTarget::Stdout => std::str::from_utf8(&output.stdout)
+            OutputTarget::Stdout | OutputTarget::InMemory => std::str::from_utf8(&output.stdout)
                 .map_err(|err| CompositionError::InvalidOutput {
                     binary: self.exe.clone(),
                     error: format!("{:?}", err),
@@ -199,6 +211,7 @@ mod tests {
     use semver::Version;
     use speculoos::prelude::*;
 
+    use super::{CompositionSuccess, OutputTarget, SupergraphBinary};
     use crate::{
         command::supergraph::compose::do_compose::SupergraphComposeOpts,
         composition::{supergraph::version::SupergraphVersion, test::default_composition_json},
@@ -208,8 +221,6 @@ mod tests {
         },
     };
 
-    use super::{CompositionSuccess, OutputTarget, SupergraphBinary};
-
     fn fed_one() -> Version {
         Version::from_str("1.0.0").unwrap()
     }

diff --git a/src/composition/watchers/composition.rs b/src/composition/watchers/composition.rs
@@ -34,6 +34,7 @@ pub struct CompositionWatcher<ExecC, ReadF, WriteF> {
     write_file: WriteF,
     temp_dir: Utf8PathBuf,
     compose_on_initialisation: bool,
+    output_target: OutputTarget,
 }
 
 impl<ExecC, ReadF, WriteF> SubtaskHandleStream for CompositionWatcher<ExecC, ReadF, WriteF>
@@ -64,7 +65,9 @@ where
                     let _ = sender
                         .send(CompositionEvent::Started)
                         .tap_err(|err| error!("{:?}", err));
-                    let output = self.run_composition(&target_file).await;
+                    let output = self
+                        .run_composition(&target_file, &self.output_target)
+                        .await;
                     match output {
                         Ok(success) => {
                             let _ = sender
@@ -126,7 +129,9 @@ where
                         .send(CompositionEvent::Started)
                         .tap_err(|err| error!("{:?}", err));
 
-                    let output = self.run_composition(&target_file).await;
+                    let output = self
+                        .run_composition(&target_file, &self.output_target)
+                        .await;
 
                     match output {
                         Ok(success) => {
@@ -189,12 +194,13 @@ where
     async fn run_composition(
         &self,
         target_file: &Utf8PathBuf,
+        output_target: &OutputTarget,
     ) -> Result<CompositionSuccess, CompositionError> {
         self.supergraph_binary
             .compose(
                 &self.exec_command,
                 &self.read_file,
-                &OutputTarget::Stdout,
+                output_target,
                 target_file.clone(),
             )
             .await
@@ -223,6 +229,7 @@ mod tests {
     use tracing_test::traced_test;
 
     use super::CompositionWatcher;
+    use crate::composition::supergraph::binary::OutputTarget;
     use crate::composition::CompositionSubgraphAdded;
     use crate::{
         composition::{
@@ -315,6 +322,7 @@ mod tests {
             .write_file(mock_write_file)
             .temp_dir(temp_dir_path)
             .compose_on_initialisation(false)
+            .output_target(OutputTarget::Stdout)
             .build();
 
         let subgraph_change_events: BoxStream<SubgraphEvent> = once(async {