refactor: remove the possibility of duplicate mapping keys

Author: DavSanchez

agent-control/src/bin/main_config_migrate.rs

@@ -1,7 +1,7 @@
 use newrelic_agent_control::agent_control::config_repository::store::AgentControlConfigStore;
 use newrelic_agent_control::config_migrate::cli::Cli;
 use newrelic_agent_control::config_migrate::migration::agent_config_getter::AgentConfigGetter;
-use newrelic_agent_control::config_migrate::migration::config::MigrationConfig;
+use newrelic_agent_control::config_migrate::migration::config::{MappingType, MigrationConfig};
 use newrelic_agent_control::config_migrate::migration::converter::ConfigConverter;
 use newrelic_agent_control::config_migrate::migration::migrator::{ConfigMigrator, MigratorError};
 use newrelic_agent_control::config_migrate::migration::persister::legacy_config_renamer::LegacyConfigRenamer;
@@ -35,11 +35,15 @@ fn main() -> Result<(), Box<dyn Error>> {
         debug!("Checking configurations for {}", cfg.agent_type_fqn);
         match config_migrator.migrate(&cfg) {
             Ok(_) => {
-                for (_, dir_path) in cfg.dirs_map {
-                    legacy_config_renamer.rename_path(dir_path.path.as_path())?;
-                }
-                for (_, file_path) in cfg.files_map {
-                    legacy_config_renamer.rename_path(file_path.as_path())?;
+                for (_, mapping_type) in cfg.filesystem_mappings {
+                    match mapping_type {
+                        MappingType::Dir(dir_path) => {
+                            legacy_config_renamer.rename_path(dir_path.dir_path.as_path())?
+                        }
+                        MappingType::File(file_path) => {
+                            legacy_config_renamer.rename_path(file_path.as_path())?
+                        }
+                    }
                 }
                 debug!("Classic config files and paths renamed");
             }

agent-control/src/config_migrate/migration/config.rs

@@ -82,19 +82,9 @@ impl Hash for AgentTypeFieldFQN {
     }
 }
 
-pub struct FileMap {
-    pub file_path: FilePath,
-    pub agent_type_fqn: AgentTypeID,
-}
-
-pub struct DirMap {
-    pub file_path: FilePath,
-    pub agent_type_fqn: AgentTypeID,
-}
-
 #[derive(Clone, Debug, PartialEq, Deserialize)]
 pub struct DirInfo {
-    pub path: FilePath,
+    pub dir_path: FilePath,
     pub extensions: Vec<String>,
 }
 
@@ -107,9 +97,6 @@ impl DirInfo {
     }
 }
 
-pub type FilesMap = HashMap<AgentTypeFieldFQN, FilePath>;
-pub type DirsMap = HashMap<AgentTypeFieldFQN, DirInfo>;
-
 #[derive(Debug, PartialEq, Clone, Deserialize)]
 pub struct MigrationConfig {
     pub configs: Vec<MigrationAgentConfig>,
@@ -148,36 +135,35 @@ impl MigrationConfig {
 pub struct MigrationAgentConfig {
     #[serde(deserialize_with = "AgentTypeID::deserialize_fqn")]
     pub agent_type_fqn: AgentTypeID,
-    pub files_map: FilesMap,
-    pub dirs_map: DirsMap,
+    pub filesystem_mappings: HashMap<AgentTypeFieldFQN, MappingType>,
     pub next: Option<AgentTypeID>,
 }
 
-impl MigrationAgentConfig {
-    pub fn get_file(&self, fqn_to_check: AgentTypeFieldFQN) -> Option<FilePath> {
-        for (fqn, path) in self.files_map.iter() {
-            if *fqn == fqn_to_check {
-                return Some(path.clone());
-            }
-        }
-        None
-    }
+#[derive(Debug, PartialEq, Clone, Deserialize)]
+#[serde(untagged)]
+pub enum MappingType {
+    File(PathBuf),
+    Dir(DirInfo),
+}
 
-    pub fn get_dir(&self, fqn_to_check: AgentTypeFieldFQN) -> Option<DirInfo> {
-        for (fqn, dir_info) in self.dirs_map.iter() {
-            if *fqn == fqn_to_check {
-                return Some(dir_info.clone());
-            }
-        }
-        None
+impl From<DirInfo> for MappingType {
+    fn from(value: DirInfo) -> Self {
+        MappingType::Dir(value)
+    }
+}
+impl<P: Into<PathBuf>> From<P> for MappingType {
+    fn from(value: P) -> Self {
+        MappingType::File(value.into())
     }
 }
 
 #[cfg(test)]
 mod tests {
 
     use crate::agent_type::agent_type_id::AgentTypeID;
-    use crate::config_migrate::migration::config::{DirInfo, FilePath, MigrationConfig};
+    use crate::config_migrate::migration::config::{
+        DirInfo, FilePath, MappingType, MigrationConfig,
+    };
     use crate::config_migrate::migration::defaults::NEWRELIC_INFRA_AGENT_TYPE_CONFIG_MAPPING;
 
     #[test]
@@ -186,68 +172,60 @@ mod tests {
 configs:
   -
     agent_type_fqn: newrelic/com.newrelic.infrastructure:0.0.2
-    files_map:
+    filesystem_mappings:
       config_agent: /etc/newrelic-infra.yml
-    dirs_map:
       config_ohis:
-        path: /etc/newrelic-infra/integrations.d
+        dir_path: /etc/newrelic-infra/integrations.d
         extensions:
           - "yaml"
           - "yml"
       logging:
-        path: /etc/newrelic-infra/logging.d
+        dir_path: /etc/newrelic-infra/logging.d
         extensions:
           - "yaml"
           - "yml"
   -
     agent_type_fqn: newrelic/com.newrelic.another:1.0.0
-    files_map:
+    filesystem_mappings:
       config_another: /etc/another.yml
-    dirs_map:
   -
     agent_type_fqn: newrelic/com.newrelic.infrastructure:1.0.1
-    files_map:
+    filesystem_mappings:
       config_agent: /etc/newrelic-infra.yml
-    dirs_map:
       config_integrations:
-        path: /etc/newrelic-infra/integrations.d
+        dir_path: /etc/newrelic-infra/integrations.d
         extensions:
           - "yaml"
           - "yml"
-
       config_logging:
-        path: /etc/newrelic-infra/logging.d
+        dir_path: /etc/newrelic-infra/logging.d
         extensions:
           - "yaml"
           - "yml"
 
   -
     agent_type_fqn: francisco-partners/com.newrelic.another:0.0.2
-    files_map:
+    filesystem_mappings:
       config_another: /etc/another.yml
-    dirs_map:
   -
     agent_type_fqn: newrelic/com.newrelic.infrastructure:0.1.2
-    files_map:
+    filesystem_mappings:
       config_agent: /etc/newrelic-infra.yml
-    dirs_map:
       config_integrations:
-        path: /etc/newrelic-infra/integrations.d
+        dir_path: /etc/newrelic-infra/integrations.d
         extensions:
           - "yaml"
           - "yml"
-
       config_logging:
-        path: /etc/newrelic-infra/logging.d
+        dir_path: /etc/newrelic-infra/logging.d
         extensions:
           - "yaml"
           - "yml"
         
   -
     agent_type_fqn: newrelic/com.newrelic.another:0.0.1
-    files_map:
+    filesystem_mappings:
       config_another: /etc/another.yml
-    dirs_map:
 "#;
 
         let expected_fqns_in_order = [
@@ -306,7 +284,7 @@ configs: []
                 String::from("yml"),
                 String::from("otro"),
             ],
-            path: FilePath::from("some/path"),
+            dir_path: FilePath::from("some/path"),
         };
 
         assert!(dir_info.valid_filename("something.yaml"));
@@ -322,13 +300,21 @@ configs: []
         let migration_config: MigrationConfig =
             MigrationConfig::parse(NEWRELIC_INFRA_AGENT_TYPE_CONFIG_MAPPING).unwrap();
 
-        for config in migration_config.configs {
-            for dir_map in config.dirs_map {
-                assert!(dir_map.1.valid_filename("something.yaml"));
-                assert!(dir_map.1.valid_filename("something.yml"));
-                assert!(!dir_map.1.valid_filename("something.yml.sample"));
-                assert!(!dir_map.1.valid_filename("something.yaml.sample"));
-                assert!(!dir_map.1.valid_filename("something.yoml"));
+        for config in migration_config.configs.into_iter() {
+            let dir_mappings =
+                config
+                    .filesystem_mappings
+                    .into_iter()
+                    .filter_map(|(_, v)| match v {
+                        MappingType::Dir(dir) => Some(dir),
+                        _ => None,
+                    });
+            for dir_map in dir_mappings {
+                assert!(dir_map.valid_filename("something.yaml"));
+                assert!(dir_map.valid_filename("something.yml"));
+                assert!(!dir_map.valid_filename("something.yml.sample"));
+                assert!(!dir_map.valid_filename("something.yaml.sample"));
+                assert!(!dir_map.valid_filename("something.yoml"));
             }
         }
     }