From c9f7811005808485f7cc2b13da45680caf3685f2 Mon Sep 17 00:00:00 2001
From: David Glasser <glasser@davidglasser.net>
Date: Thu, 5 Sep 2024 16:16:23 -0700
Subject: [PATCH] apollo-federation-types: merge routing_url in
 `SupergraphConfig::merge_subgraphs`

This function was added a few months ago in #541 and as far as I know is
only used in Rover for the `supergraph compose` and `dev` commands to
combine configuration from `--graph-ref` and
`--supergraph-config`/`--config` (and the feature does not actually work
for `supergraph compose` due to the issue fixed in
https://github.com/apollographql/rover/pull/2101).

The use case here is to let you run composition or dev against a GraphOS
graph with some local changes, and so "I just want to change the SDL and
nothing else" is a reasonable use case. This change means that if an
override specifies SDL but no routing URL for a subgraph, Rover will
continue to use the routing URL fetched from GraphOS for that subgraph,
which seems reasonable.

While this is a "backwards incompatible" change, this crate is only
intended for use by the Rover CLI and in the one case where this was
called (`rover dev --graph-ref REF --supergraph-config CONFIG`) this
seems like a clear improvement.
---
 .../src/config/supergraph.rs                  | 36 +++++++++++++++++--
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/apollo-federation-types/src/config/supergraph.rs b/apollo-federation-types/src/config/supergraph.rs
index 6e207d42a..c48f9b900 100644
--- a/apollo-federation-types/src/config/supergraph.rs
+++ b/apollo-federation-types/src/config/supergraph.rs
@@ -112,10 +112,24 @@ impl SupergraphConfig {
         self.federation_version.clone()
     }
 
-    /// Merges the subgraphs of another [`SupergraphConfig`] into this one
+    /// Merges the subgraphs of another [`SupergraphConfig`] into this one; the
+    /// other config takes precedence when there are overlaps
     pub fn merge_subgraphs(&mut self, other: &SupergraphConfig) {
-        for (key, value) in other.subgraphs.iter() {
-            self.subgraphs.insert(key.to_string(), value.clone());
+        for (key, other_subgraph) in other.subgraphs.iter() {
+            let other_subgraph = other_subgraph.clone();
+            // SubgraphConfig always has a schema. For routing_url, we take
+            // `other` if they both exist (ie, we let local configuration
+            // override)
+            let merged_subgraph = match self.subgraphs.get(key) {
+                Some(my_subgraph) => SubgraphConfig {
+                    routing_url: other_subgraph
+                        .routing_url
+                        .or(my_subgraph.routing_url.clone()),
+                    schema: other_subgraph.schema,
+                },
+                None => other_subgraph,
+            };
+            self.subgraphs.insert(key.to_string(), merged_subgraph);
         }
     }
 }
@@ -633,6 +647,10 @@ subgraphs:
     routing_url: https://people.example.com
     schema:
       file: ./good-people.graphql
+  robots:
+    routing_url: https://robots.example.com
+    schema:
+      file: ./good-robots.graphql
 "#;
         let raw_override_config = r#"---
 federation_version: 1
@@ -645,6 +663,9 @@ subgraphs:
     routing_url: https://books.example.com
     schema:
       file: ./good-books.graphql
+  robots:
+    schema:
+      file: ./better-robots.graphql
 "#;
         let mut base_config = SupergraphConfig::new_from_yaml(raw_base_config)
             .expect("Failed to parse supergraph config");
@@ -687,6 +708,15 @@ subgraphs:
                     },
                 },
             ),
+            (
+                "robots".to_string(),
+                SubgraphConfig {
+                    routing_url: Some("https://robots.example.com".to_string()),
+                    schema: SchemaSource::File {
+                        file: "./better-robots.graphql".into(),
+                    },
+                },
+            ),
         ]);
 
         assert_eq!(base_config.subgraphs, expected_subgraphs);