Skip to content

Commit 2780c56

Browse files
ROVER-312 Removes bug when no given config
At the moment we assume that not being able to get the "supergraph" key from the config still means the config actually exists. This is not true in general, as the case raised in the bug ticket proves. This improves the handling of that case by expanding the match statements which removes lots of calls to `get_mut` and `unwrap` that were causing this issue in the first place. Also adds a unit test to ensure we don't end up in this situation again.
1 parent b3bddae commit 2780c56

File tree

1 file changed

+50
-19
lines changed

1 file changed

+50
-19
lines changed

src/command/dev/router/hot_reload.rs

+50-19
Original file line numberDiff line numberDiff line change
@@ -75,28 +75,44 @@ impl HotReloadConfig {
7575
.map_err(|err| HotReloadError::Config { err: err.into() })?
7676
.to_string();
7777

78-
// Try and get the supergraph stanza
79-
match config.get_mut("supergraph") {
80-
None => {
81-
// If it doesn't exist then we need to build the mapping, and give it the
82-
// only key we're interested in, which is listen.
83-
let mut listen_mapping = Mapping::new();
84-
listen_mapping.insert(
85-
Value::String("listen".into()),
86-
Value::String(processed_address),
87-
);
88-
config.as_mapping_mut().unwrap().insert(
78+
let mut listen_mapping = Mapping::new();
79+
listen_mapping.insert(
80+
Value::String("listen".into()),
81+
Value::String(processed_address.clone()),
82+
);
83+
84+
let config = match config {
85+
Value::Mapping(mut config_mapping) => {
86+
match config_mapping.get_mut("supergraph") {
87+
Some(Value::Mapping(supergraph_mapping)) => {
88+
// If it does exist then we can just overwrite the existing value
89+
// of listen with what we've worked out
90+
supergraph_mapping.insert(
91+
Value::String("listen".into()),
92+
Value::String(processed_address),
93+
);
94+
}
95+
_ => {
96+
// If it doesn't exist then we need to build the mapping, and give it the
97+
// only key we're interested in, which is listen.
98+
config_mapping.insert(
99+
Value::String("supergraph".into()),
100+
Value::Mapping(listen_mapping),
101+
);
102+
}
103+
}
104+
config_mapping
105+
}
106+
// If config's not a mapping, then we don't have any config at all, so we
107+
// need to build the simplest thing we can, which is just a mapping from
108+
// supergraph to listen to the value we've computed
109+
_ => {
110+
let mut result = Mapping::new();
111+
result.insert(
89112
Value::String("supergraph".into()),
90113
Value::Mapping(listen_mapping),
91114
);
92-
}
93-
Some(supergraph_mapping) => {
94-
// If it does exist then we can just overwrite the existing value
95-
// of listen with what we've worked out
96-
supergraph_mapping.as_mapping_mut().unwrap().insert(
97-
Value::String("listen".into()),
98-
Value::String(processed_address),
99-
);
115+
result
100116
}
101117
};
102118

@@ -378,4 +394,19 @@ headers:
378394
.unwrap()
379395
});
380396
}
397+
398+
#[rstest]
399+
fn default_state_no_config() {
400+
let address = RouterAddress::new(
401+
Some(RouterHost::Default("127.0.0.1".parse().unwrap())),
402+
Some(RouterPort::Default(4000)),
403+
);
404+
let overrides = HotReloadConfigOverrides::new(address);
405+
let hot_reload_config = HotReloadConfig::new("".into(), Some(overrides));
406+
assert_that!(hot_reload_config).is_ok().matches(|config| {
407+
let value: Value = serde_yaml::from_str(&config.content).unwrap();
408+
println!("{config}");
409+
value.get("supergraph").unwrap().get("listen").unwrap() == "127.0.0.1:4000"
410+
});
411+
}
381412
}

0 commit comments

Comments
 (0)