From 2453e7a7682d22ff7ca206b8ac45283d4dd1a497 Mon Sep 17 00:00:00 2001 From: Erikson Tung Date: Thu, 25 Jun 2020 13:42:16 -0700 Subject: [PATCH 1/2] migration-helpers: AddSettingsMigration handle more than one setting Adds new migration helper `AddSettingsMigration`. `AddSettingsMigration` handles migrating more than one setting. Original `AddSettingMigration` now wraps `AddSettingsMigration` and is now deprecated --- sources/api/migration/README.md | 4 +- .../src/common_migrations.rs | 46 ++++++++++++++----- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/sources/api/migration/README.md b/sources/api/migration/README.md index 9a4b2d94329..1b117c4c310 100644 --- a/sources/api/migration/README.md +++ b/sources/api/migration/README.md @@ -156,7 +156,7 @@ If we're confident no user configuration has changed, or if the user wants to di Say we add a new open-source application, like rngd. We can add data regarding settings, services, and configuration-files of the application to our data model. -In this case, we can use the existing helper `AddSettingMigration`. +In this case, we can use the existing helper `AddSettingsMigration`. It doesn't need to do anything on upgrade, because the new key will be populated by its default value. On downgrade, it removes the setting, so that the old data store model doesn't see an unexpected key and reject the data. @@ -164,7 +164,7 @@ On downgrade, it removes the setting, so that the old data store model doesn't s If we upgrade an important application, its available and required settings may change. This means we'd have to update the data model to include any new or changed settings, and we'd write migrations to transform data from the old settings to the new. -This can likely be handled by existing helpers `AddSettingMigration`, `RemoveSettingMigration`, `ReplaceStringMigration`, and `ReplaceTemplateMigration`. +This can likely be handled by existing helpers `AddSettingsMigration`, `RemoveSettingMigration`, `ReplaceStringMigration`, and `ReplaceTemplateMigration`. ### Data store implementation change diff --git a/sources/api/migration/migration-helpers/src/common_migrations.rs b/sources/api/migration/migration-helpers/src/common_migrations.rs index 3675c5ead3c..7c202bee7eb 100644 --- a/sources/api/migration/migration-helpers/src/common_migrations.rs +++ b/sources/api/migration/migration-helpers/src/common_migrations.rs @@ -4,26 +4,31 @@ use serde::Serialize; use snafu::{OptionExt, ResultExt}; use std::collections::HashMap; -/// We use this migration when we add a setting and want to make sure it's removed before we go -/// back to old versions that don't understand it. -pub struct AddSettingMigration(pub &'static str); +/// We use this migration when we add settings and want to make sure they're removed before we go +/// back to old versions that don't understand them. +pub struct AddSettingsMigration<'a> (pub &'a [&'static str]); -impl Migration for AddSettingMigration { - /// New versions must either have a default for the setting or generate it; we don't need to +impl Migration for AddSettingsMigration<'_> { + /// New versions must either have a default for the settings or generate them; we don't need to /// do anything. fn forward(&mut self, input: MigrationData) -> Result { - println!("AddSettingMigration({}) has no work to do on upgrade.", self.0); + println!( + "AddSettingsMigration({:?}) has no work to do on upgrade.", + self.0 + ); Ok(input) } - /// Older versions don't know about the setting; we remove it so that old versions don't see - /// it and fail deserialization. (The setting must be defaulted or generated in new versions, + /// Older versions don't know about the settings; we remove them so that old versions don't see + /// them and fail deserialization. (The settings must be defaulted or generated in new versions, /// and safe to remove.) fn backward(&mut self, mut input: MigrationData) -> Result { - if let Some(data) = input.data.remove(self.0) { - println!("Removed {}, which was set to '{}'", self.0, data); - } else { - println!("Found no {} to remove", self.0); + for setting in self.0 { + if let Some(data) = input.data.remove(*setting) { + println!("Removed {}, which was set to '{}'", setting, data); + } else { + println!("Found no {} to remove", setting); + } } Ok(input) } @@ -31,6 +36,23 @@ impl Migration for AddSettingMigration { // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= +/// Similar to the above, this migration is for when we add a single setting. +/// We are retaining this migration helper in case there are migrations already using it. +#[deprecated(note = "Please use `AddSettingsMigration` instead")] +pub struct AddSettingMigration(pub &'static str); + +impl Migration for AddSettingMigration { + fn forward(&mut self, input: MigrationData) -> Result { + AddSettingsMigration(&[self.0]).forward(input) + } + + fn backward(&mut self, input: MigrationData) -> Result { + AddSettingsMigration(&[self.0]).backward(input) + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + /// We use this migration when we remove a setting from the model, so the new version doesn't see /// it and error. pub struct RemoveSettingMigration(pub &'static str); From cff78691a709e41e1a87cc957cdcdbd31490996a Mon Sep 17 00:00:00 2001 From: Erikson Tung Date: Thu, 25 Jun 2020 17:05:41 -0700 Subject: [PATCH 2/2] migration-helpers: `RemoveSettingsMigration` to remove multiple settings at once Adds new migration helper `RemoveSettingsMigration` to remove multiple settings at once. Original `RemoveSettingMigration` now wraps `RemoveSettingsMigration` and is now deprecated. --- sources/api/migration/README.md | 2 +- .../src/common_migrations.rs | 48 ++++++++++++++----- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/sources/api/migration/README.md b/sources/api/migration/README.md index 1b117c4c310..ad60383fed3 100644 --- a/sources/api/migration/README.md +++ b/sources/api/migration/README.md @@ -164,7 +164,7 @@ On downgrade, it removes the setting, so that the old data store model doesn't s If we upgrade an important application, its available and required settings may change. This means we'd have to update the data model to include any new or changed settings, and we'd write migrations to transform data from the old settings to the new. -This can likely be handled by existing helpers `AddSettingsMigration`, `RemoveSettingMigration`, `ReplaceStringMigration`, and `ReplaceTemplateMigration`. +This can likely be handled by existing helpers `AddSettingsMigration`, `RemoveSettingsMigration`, `ReplaceStringMigration`, and `ReplaceTemplateMigration`. ### Data store implementation change diff --git a/sources/api/migration/migration-helpers/src/common_migrations.rs b/sources/api/migration/migration-helpers/src/common_migrations.rs index 7c202bee7eb..f8cbb01239e 100644 --- a/sources/api/migration/migration-helpers/src/common_migrations.rs +++ b/sources/api/migration/migration-helpers/src/common_migrations.rs @@ -6,7 +6,7 @@ use std::collections::HashMap; /// We use this migration when we add settings and want to make sure they're removed before we go /// back to old versions that don't understand them. -pub struct AddSettingsMigration<'a> (pub &'a [&'static str]); +pub struct AddSettingsMigration<'a>(pub &'a [&'static str]); impl Migration for AddSettingsMigration<'_> { /// New versions must either have a default for the settings or generate them; we don't need to @@ -53,33 +53,55 @@ impl Migration for AddSettingMigration { // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= -/// We use this migration when we remove a setting from the model, so the new version doesn't see -/// it and error. -pub struct RemoveSettingMigration(pub &'static str); +/// We use this migration when we remove settings from the model, so the new version doesn't see +/// them and error. +pub struct RemoveSettingsMigration<'a>(pub &'a [&'static str]); -impl Migration for RemoveSettingMigration { - /// Newer versions don't know about the setting; we remove it so that new versions don't see - /// it and fail deserialization. (The setting must be defaulted or generated in old versions, +impl Migration for RemoveSettingsMigration<'_> { + /// Newer versions don't know about the settings; we remove them so that new versions don't see + /// them and fail deserialization. (The settings must be defaulted or generated in old versions, /// and safe to remove.) fn forward(&mut self, mut input: MigrationData) -> Result { - if let Some(data) = input.data.remove(self.0) { - println!("Removed {}, which was set to '{}'", self.0, data); - } else { - println!("Found no {} to remove", self.0); + for setting in self.0 { + if let Some(data) = input.data.remove(*setting) { + println!("Removed {}, which was set to '{}'", setting, data); + } else { + println!("Found no {} to remove", setting); + } } Ok(input) } - /// Old versions must either have a default for the setting or generate it; we don't need to + /// Old versions must either have a default for the settings or generate it; we don't need to /// do anything. fn backward(&mut self, input: MigrationData) -> Result { - println!("RemoveSettingMigration({}) has no work to do on downgrade.", self.0); + println!( + "RemoveSettingsMigration({:?}) has no work to do on downgrade.", + self.0 + ); Ok(input) } } // =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= +/// Similar to the above, this migration is for when we need to remove a single setting. +/// We are retaining this migration helper in case there are migrations already using it. +#[deprecated(note = "Please use `RemoveSettingsMigration` instead")] +pub struct RemoveSettingMigration(pub &'static str); + +impl Migration for RemoveSettingMigration { + fn forward(&mut self, input: MigrationData) -> Result { + RemoveSettingsMigration(&[self.0]).forward(input) + } + + fn backward(&mut self, input: MigrationData) -> Result { + RemoveSettingsMigration(&[self.0]).backward(input) + } +} + +// =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= =^..^= + /// We use this migration when we replace a setting's old string value with a new string value. pub struct ReplaceStringMigration { pub setting: &'static str,