From c1d3e8ae3cc5dc0e95161264d84b17329d1e5839 Mon Sep 17 00:00:00 2001
From: jonathanrainer <jonathan.rainer@apollographql.com>
Date: Mon, 9 Dec 2024 09:57:21 +0000
Subject: [PATCH] ROVER-245 Address PR comments and fix tests

---
 src/command/lsp/mod.rs                        | 32 ++++++++++---------
 src/command/supergraph/compose/mod.rs         |  9 +++---
 src/composition/mod.rs                        |  9 +++---
 .../watchers/watcher/supergraph_config.rs     |  9 +++---
 4 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/src/command/lsp/mod.rs b/src/command/lsp/mod.rs
index fe46f3fb2..7d48d0f6e 100644
--- a/src/command/lsp/mod.rs
+++ b/src/command/lsp/mod.rs
@@ -1,6 +1,22 @@
-use futures::StreamExt;
 mod errors;
 
+use std::collections::HashMap;
+use std::env::temp_dir;
+use std::io::stdin;
+
+use anyhow::{anyhow, Error};
+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 tower_lsp::lsp_types::{Diagnostic, Range};
+use tower_lsp::Server;
+use tracing::debug;
+use url::Url;
+
 use crate::command::lsp::errors::SupergraphConfigLazyResolutionError;
 use crate::command::lsp::errors::SupergraphConfigLazyResolutionError::PathDoesNotPointToAFile;
 use crate::composition::events::CompositionEvent;
@@ -23,20 +39,6 @@ use crate::{
     utils::{client::StudioClientConfig, parsers::FileDescriptorType},
     RoverOutput, RoverResult,
 };
-use anyhow::{anyhow, Error};
-use apollo_federation_types::config::FederationVersion;
-use apollo_language_server::{ApolloLanguageServer, Config};
-use camino::Utf8PathBuf;
-use clap::Parser;
-use rover_client::blocking::StudioClient;
-use serde::Serialize;
-use std::collections::HashMap;
-use std::env::temp_dir;
-use std::io::stdin;
-use tower_lsp::lsp_types::{Diagnostic, Range};
-use tower_lsp::Server;
-use tracing::debug;
-use url::Url;
 
 #[derive(Debug, Serialize, Parser)]
 pub struct Lsp {
diff --git a/src/command/supergraph/compose/mod.rs b/src/command/supergraph/compose/mod.rs
index 9be0d7744..3a70976a3 100644
--- a/src/command/supergraph/compose/mod.rs
+++ b/src/command/supergraph/compose/mod.rs
@@ -7,14 +7,13 @@ pub(crate) use no_compose::Compose;
 #[cfg(feature = "composition-js")]
 pub(crate) mod do_compose;
 
+use apollo_federation_types::rover::BuildHint;
 #[cfg(feature = "composition-js")]
 pub(crate) use do_compose::Compose;
 
 #[cfg(feature = "composition-js")]
 use crate::composition::CompositionSuccess;
 
-use apollo_federation_types::rover::BuildHint;
-
 #[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CompositionOutput {
     pub supergraph_sdl: String,
@@ -29,9 +28,9 @@ pub struct CompositionOutput {
 impl From<CompositionSuccess> for CompositionOutput {
     fn from(value: CompositionSuccess) -> Self {
         Self {
-            supergraph_sdl: value.supergraph_sdl().clone(),
-            hints: value.hints().to_vec(),
-            federation_version: Some(value.federation_version().to_string()),
+            supergraph_sdl: value.supergraph_sdl.clone(),
+            hints: value.hints.to_vec(),
+            federation_version: Some(value.federation_version.to_string()),
         }
     }
 }
diff --git a/src/composition/mod.rs b/src/composition/mod.rs
index e79f5a728..a66bf2073 100644
--- a/src/composition/mod.rs
+++ b/src/composition/mod.rs
@@ -6,7 +6,6 @@ use apollo_federation_types::{
     rover::{BuildErrors, BuildHint},
 };
 use camino::Utf8PathBuf;
-use derive_getters::Getters;
 
 pub mod events;
 pub mod runner;
@@ -18,11 +17,11 @@ pub mod types;
 #[cfg(feature = "composition-js")]
 mod watchers;
 
-#[derive(Getters, Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct CompositionSuccess {
-    pub(crate) supergraph_sdl: String,
-    pub(crate) hints: Vec<BuildHint>,
-    pub(crate) federation_version: FederationVersion,
+    pub supergraph_sdl: String,
+    pub hints: Vec<BuildHint>,
+    pub federation_version: FederationVersion,
 }
 
 #[derive(thiserror::Error, Debug, Eq, PartialEq)]
diff --git a/src/composition/watchers/watcher/supergraph_config.rs b/src/composition/watchers/watcher/supergraph_config.rs
index a3afb8825..a829fe659 100644
--- a/src/composition/watchers/watcher/supergraph_config.rs
+++ b/src/composition/watchers/watcher/supergraph_config.rs
@@ -7,13 +7,12 @@ use rover_std::errln;
 use tap::TapFallible;
 use tokio::{sync::mpsc::UnboundedSender, task::AbortHandle};
 
+use super::file::FileWatcher;
 use crate::{
     composition::supergraph::config::lazy::LazilyResolvedSupergraphConfig,
     subtask::SubtaskHandleUnit,
 };
 
-use super::file::FileWatcher;
-
 #[derive(Debug)]
 pub struct SupergraphConfigWatcher {
     file_watcher: FileWatcher,
@@ -98,8 +97,8 @@ impl SupergraphConfigDiff {
             .collect();
 
         // Compare the old and new subgraph names to find removals.
-        let removed_names: HashSet<String> = new_subgraph_names_and_urls
-            .difference(&old_subgraph_names_and_urls)
+        let removed_names: HashSet<String> = old_subgraph_names_and_urls
+            .difference(&new_subgraph_names_and_urls)
             .map(|(a, _)| a.clone())
             .collect();
 
@@ -138,10 +137,10 @@ impl SupergraphConfigDiff {
 
 #[cfg(test)]
 mod tests {
-    use rstest::rstest;
     use std::collections::BTreeMap;
 
     use apollo_federation_types::config::{SchemaSource, SubgraphConfig, SupergraphConfig};
+    use rstest::rstest;
 
     use super::SupergraphConfigDiff;