Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migration-helpers: ReplaceListMigration -> ReplaceListsMigration #1262

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 99 additions & 76 deletions sources/api/migration/migration-helpers/src/common_migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,97 +295,111 @@ impl Migration for ReplaceStringMigration {

// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^=

/// We use this migration when we replace a setting that contains a list of string values.
/// We use this migration when we need to replace settings that contain lists of string values;
/// for example, when a release changes the list of configuration-files associated with a service.
// String is the only type we use today, and handling multiple value types is more complicated than
// we need at the moment. Allowing &[serde_json::Value] seems nice, but it would allow arbitrary
// data transformations that the API model would then fail to load.
pub struct ReplaceListMigration {

pub struct ListReplacement {
pub setting: &'static str,
pub old_vals: &'static [&'static str],
pub new_vals: &'static [&'static str],
}

impl Migration for ReplaceListMigration {
pub struct ReplaceListsMigration(pub Vec<ListReplacement>);

impl Migration for ReplaceListsMigration {
fn forward(&mut self, mut input: MigrationData) -> Result<MigrationData> {
if let Some(data) = input.data.get_mut(self.setting) {
match data {
serde_json::Value::Array(data) => {
// We only handle string lists; convert each value to a str we can compare.
let list: Vec<&str> = data
.iter()
.map(|v| v.as_str())
.collect::<Option<Vec<&str>>>()
.with_context(|| error::ReplaceListContents {
setting: self.setting,
data: data.clone(),
})?;

if list == self.old_vals {
// Convert back to the original type so we can store it.
*data = self.new_vals.iter().map(|s| (*s).into()).collect();
for replacement in &self.0 {
if let Some(data) = input.data.get_mut(replacement.setting) {
match data {
serde_json::Value::Array(data) => {
// We only handle string lists; convert each value to a str we can compare.
let list: Vec<&str> = data
.iter()
.map(|v| v.as_str())
.collect::<Option<Vec<&str>>>()
.with_context(|| error::ReplaceListContents {
setting: replacement.setting,
data: data.clone(),
})?;

if list == replacement.old_vals {
// Convert back to the original type so we can store it.
*data = replacement.new_vals.iter().map(|s| (*s).into()).collect();
println!(
"Changed value of '{}' from {:?} to {:?} on upgrade",
replacement.setting, replacement.old_vals, replacement.new_vals
);
} else {
println!(
"'{}' is not set to {:?}, leaving alone",
replacement.setting, list
);
}
}
_ => {
println!(
"Changed value of '{}' from {:?} to {:?} on upgrade",
self.setting, self.old_vals, self.new_vals
"'{}' is set to non-list value '{}'; ReplaceListsMigration only handles lists",
replacement.setting, data
);
} else {
println!("'{}' is not set to {:?}, leaving alone", self.setting, list);
}
}
_ => {
println!(
"'{}' is set to non-list value '{}'; ReplaceListMigration only handles lists",
self.setting, data
);
}
} else {
println!("Found no '{}' to change on upgrade", replacement.setting);
}
} else {
println!("Found no '{}' to change on upgrade", self.setting);
}
Ok(input)
}

fn backward(&mut self, mut input: MigrationData) -> Result<MigrationData> {
if let Some(data) = input.data.get_mut(self.setting) {
match data {
serde_json::Value::Array(data) => {
// We only handle string lists; convert each value to a str we can compare.
let list: Vec<&str> = data
.iter()
.map(|v| v.as_str())
.collect::<Option<Vec<&str>>>()
.with_context(|| error::ReplaceListContents {
setting: self.setting,
data: data.clone(),
})?;

if list == self.new_vals {
// Convert back to the original type so we can store it.
*data = self.old_vals.iter().map(|s| (*s).into()).collect();
println!(
"Changed value of '{}' from {:?} to {:?} on downgrade",
self.setting, self.new_vals, self.old_vals
);
} else {
println!("'{}' is not set to {:?}, leaving alone", self.setting, list);
for replacement in &self.0 {
if let Some(data) = input.data.get_mut(replacement.setting) {
match data {
serde_json::Value::Array(data) => {
// We only handle string lists; convert each value to a str we can compare.
let list: Vec<&str> = data
.iter()
.map(|v| v.as_str())
.collect::<Option<Vec<&str>>>()
.with_context(|| error::ReplaceListContents {
setting: replacement.setting,
data: data.clone(),
})?;

if list == replacement.new_vals {
// Convert back to the original type so we can store it.
*data = replacement.old_vals.iter().map(|s| (*s).into()).collect();
println!(
"Changed value of '{}' from {:?} to {:?} on downgrade",
replacement.setting, replacement.new_vals, replacement.old_vals
);
} else {
println!(
"'{}' is not set to {:?}, leaving alone",
replacement.setting, list
);
}
}
}
_ => {
println!(
"'{}' is set to non-list value '{}'; ReplaceListMigration only handles lists",
self.setting, data
_ => {
println!(
"'{}' is set to non-list value '{}'; ReplaceListsMigration only handles lists",
replacement.setting, data
);
}
}
} else {
println!("Found no '{}' to change on downgrade", replacement.setting);
}
} else {
println!("Found no '{}' to change on downgrade", self.setting);
}
Ok(input)
}
}

#[cfg(test)]
mod test_replace_list {
use super::ReplaceListMigration;
use super::{ListReplacement, ReplaceListsMigration};
use crate::{Migration, MigrationData};
use maplit::hashmap;
use std::collections::HashMap;
Expand All @@ -398,11 +412,11 @@ mod test_replace_list {
},
metadata: HashMap::new(),
};
let result = ReplaceListMigration {
let result = ReplaceListsMigration(vec![ListReplacement {
setting: "hi",
old_vals: &["there"],
new_vals: &["sup"],
}
}])
.forward(data)
.unwrap();
assert_eq!(
Expand All @@ -421,11 +435,11 @@ mod test_replace_list {
},
metadata: HashMap::new(),
};
let result = ReplaceListMigration {
let result = ReplaceListsMigration(vec![ListReplacement {
setting: "hi",
old_vals: &["sup"],
new_vals: &["there"],
}
}])
.backward(data)
.unwrap();
assert_eq!(
Expand All @@ -441,21 +455,30 @@ mod test_replace_list {
let data = MigrationData {
data: hashmap! {
"hi".into() => vec!["there", "you"].into(),
"hi2".into() => vec!["hey", "listen"].into(),
"ignored".into() => vec!["no", "change"].into(),
},
metadata: HashMap::new(),
};
let result = ReplaceListMigration {
setting: "hi",
old_vals: &["there", "you"],
new_vals: &["sup", "hey"],
}
let result = ReplaceListsMigration(vec![
ListReplacement {
setting: "hi",
old_vals: &["there", "you"],
new_vals: &["sup", "hey"],
},
ListReplacement {
setting: "hi2",
old_vals: &["hey", "listen"],
new_vals: &["look", "watch out"],
},
])
.forward(data)
.unwrap();
assert_eq!(
result.data,
hashmap! {
"hi".into() => vec!["sup", "hey"].into(),
"hi2".into() => vec!["look", "watch out"].into(),
"ignored".into() => vec!["no", "change"].into(),
}
);
Expand All @@ -470,11 +493,11 @@ mod test_replace_list {
},
metadata: HashMap::new(),
};
let result = ReplaceListMigration {
let result = ReplaceListsMigration(vec![ListReplacement {
setting: "hi",
old_vals: &["there"],
new_vals: &["sup", "hey"],
}
}])
.forward(data)
.unwrap();
// No change
Expand All @@ -495,11 +518,11 @@ mod test_replace_list {
},
metadata: HashMap::new(),
};
let result = ReplaceListMigration {
let result = ReplaceListsMigration(vec![ListReplacement {
setting: "hi",
old_vals: &["there"],
new_vals: &["sup", "hey"],
}
}])
.forward(data)
.unwrap();
// No change
Expand All @@ -519,11 +542,11 @@ mod test_replace_list {
},
metadata: HashMap::new(),
};
ReplaceListMigration {
ReplaceListsMigration(vec![ListReplacement {
setting: "hi",
old_vals: &["there"],
new_vals: &["sup", "hey"],
}
}])
.forward(data)
.unwrap_err();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
#![deny(rust_2018_idioms)]

use migration_helpers::common_migrations::ReplaceListMigration;
use migration_helpers::common_migrations::{ListReplacement, ReplaceListsMigration};
use migration_helpers::{migrate, Result};
use std::process;

/// We changed corndog to use subcommands so it can handle different kernel settings without having
/// to apply them all every time.
fn run() -> Result<()> {
migrate(ReplaceListMigration {
migrate(ReplaceListsMigration(vec![ListReplacement {
setting: "services.sysctl.restart-commands",
old_vals: &["/usr/bin/corndog"],
new_vals: &["/usr/bin/corndog sysctl"],
})
}]))
}

// Returning a Result from main makes it print a Debug representation of the error, but with Snafu
Expand Down