diff --git a/cpp/src/tests/api/interface/database_interface_test.h b/cpp/src/tests/api/interface/database_interface_test.h index 2cadb6ae..a4ed1e65 100644 --- a/cpp/src/tests/api/interface/database_interface_test.h +++ b/cpp/src/tests/api/interface/database_interface_test.h @@ -964,31 +964,17 @@ TEST_P(DatabaseInterfaceTest, // Plugin entries are unordered. std::stringstream expectedContent; - if (content.find(blankDifferentEsm) < content.find(blankEsm)) { - expectedContent << "plugins:" << endl - << " - name: '" << blankDifferentEsm << "'" << endl - << " dirty:" << endl - << " - crc: 0x7D22F9DF" << endl - << " util: 'TES4Edit'" << endl - << " udr: 4" << endl - << " - name: '" << blankEsm << "'" << endl - << " tag:" << endl - << " - Actors.ACBS" << endl - << " - Actors.AIData" << endl - << " - -C.Water"; - } else { - expectedContent << "plugins:" << endl - << " - name: '" << blankEsm << "'" << endl - << " tag:" << endl - << " - Actors.ACBS" << endl - << " - Actors.AIData" << endl - << " - -C.Water" << endl - << " - name: '" << blankDifferentEsm << "'" << endl - << " dirty:" << endl - << " - crc: 0x7D22F9DF" << endl - << " util: 'TES4Edit'" << endl - << " udr: 4"; - } + expectedContent << "plugins:" << endl + << " - name: '" << blankEsm << "'" << endl + << " tag:" << endl + << " - Actors.ACBS" << endl + << " - Actors.AIData" << endl + << " - -C.Water" << endl + << " - name: '" << blankDifferentEsm << "'" << endl + << " dirty:" << endl + << " - crc: 0x7D22F9DF" << endl + << " util: 'TES4Edit'" << endl + << " udr: 4"; EXPECT_EQ(expectedContent.str(), content); } diff --git a/src/database/mod.rs b/src/database/mod.rs index c293cc09..d1732871 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -131,7 +131,7 @@ impl Database { let mut doc = MetadataDocument::default(); - for plugin in self.masterlist.plugins_iter() { + for plugin in self.masterlist.ordered_plugins_iter() { let mut minimal_plugin = PluginMetadata::with_same_name(plugin); minimal_plugin.set_tags(plugin.tags().to_vec()); minimal_plugin.set_dirty_info(plugin.dirty_info().to_vec()); @@ -725,7 +725,7 @@ plugins: use super::*; #[test] - fn should_only_write_plugin_bash_tags_and_dirty_info() { + fn should_write_plugins_in_insertion_order_with_only_bash_tags_and_dirty_info() { let fixture = Fixture::new(GameType::Oblivion); let mut database = fixture.database(); let output_path = fixture.inner.local_path.join("minimal.yaml"); @@ -741,20 +741,7 @@ plugins: let content = std::fs::read_to_string(output_path).unwrap(); // Plugin entries are unordered. - let expected_content = if content.find(BLANK_DIFFERENT_ESM) < content.find(BLANK_ESM) { - "plugins: - - name: 'Blank - Different.esm' - dirty: - - crc: 0x7D22F9DF - util: 'TES4Edit' - udr: 4 - - name: 'Blank.esm' - tag: - - Actors.ACBS - - Actors.AIData - - -C.Water" - } else { - "plugins: + let expected_content = "plugins: - name: 'Blank.esm' tag: - Actors.ACBS @@ -764,8 +751,7 @@ plugins: dirty: - crc: 0x7D22F9DF util: 'TES4Edit' - udr: 4" - }; + udr: 4"; assert_eq!(expected_content, content); } diff --git a/src/metadata/metadata_document.rs b/src/metadata/metadata_document.rs index 9dd6dd31..fd7f29f5 100644 --- a/src/metadata/metadata_document.rs +++ b/src/metadata/metadata_document.rs @@ -1,6 +1,7 @@ use std::{ collections::{HashMap, HashSet}, path::Path, + sync::Arc, }; use saphyr::{LoadableYamlNode, MarkedYaml, YamlData}; @@ -29,8 +30,9 @@ pub(crate) struct MetadataDocument { bash_tags: Vec, groups: Vec, messages: Vec, - plugins: HashMap, + plugins: HashMap, PluginMetadata>, regex_plugins: Vec, + ordered_plugin_names: Vec>, } impl MetadataDocument { @@ -142,23 +144,25 @@ impl MetadataDocument { .into()); }; - let mut plugins: HashMap = HashMap::new(); - let mut regex_plugins: Vec = Vec::new(); + let mut plugins = HashMap::new(); + let mut regex_plugins = Vec::new(); + let mut ordered_plugin_names = Vec::new(); for plugin_yaml in get_slice_value(&doc, "plugins", YamlObjectType::MetadataDocument)? { let plugin = PluginMetadata::try_from_yaml(plugin_yaml)?; + let filename = Arc::new(Filename::new(plugin.name().to_owned())); + if plugin.is_regex_plugin() { regex_plugins.push(plugin); - } else { - let filename = Filename::new(plugin.name().to_owned()); - if let Some(old) = plugins.insert(filename, plugin) { - return Err(ParseMetadataError::duplicate_entry( - plugin_yaml.span.start, - old.name().to_owned(), - YamlObjectType::PluginMetadata, - ) - .into()); - } + } else if let Some(old) = plugins.insert(Arc::clone(&filename), plugin) { + return Err(ParseMetadataError::duplicate_entry( + plugin_yaml.span.start, + old.name().to_owned(), + YamlObjectType::PluginMetadata, + ) + .into()); } + + ordered_plugin_names.push(filename); } let messages = get_slice_value(&doc, "globals", YamlObjectType::MetadataDocument)? @@ -208,6 +212,7 @@ impl MetadataDocument { self.plugins = plugins; self.regex_plugins = regex_plugins; + self.ordered_plugin_names = ordered_plugin_names; self.messages = messages; self.bash_tags = bash_tags; self.groups = groups; @@ -249,7 +254,7 @@ impl MetadataDocument { emitter.begin_array(); - for plugin in self.plugins_iter() { + for plugin in self.ordered_plugins_iter() { if !plugin.has_name_only() { plugin.emit_yaml(&mut emitter); } @@ -283,8 +288,14 @@ impl MetadataDocument { &self.messages } - pub(crate) fn plugins_iter(&self) -> impl Iterator { - self.plugins.values().chain(self.regex_plugins.iter()) + pub(crate) fn ordered_plugins_iter(&self) -> impl Iterator { + self.ordered_plugin_names.iter().filter_map(|f| { + self.plugins.get(f).or_else(|| { + self.regex_plugins + .iter() + .find(|r| r.name() == f.as_ref().as_str()) + }) + }) } pub(crate) fn find_plugin( @@ -332,24 +343,38 @@ impl MetadataDocument { } pub(crate) fn set_plugin_metadata(&mut self, plugin_metadata: PluginMetadata) { + let filename = Arc::new(Filename::new(plugin_metadata.name().to_owned())); + if plugin_metadata.is_regex_plugin() { self.regex_plugins.push(plugin_metadata); + self.ordered_plugin_names.push(filename); } else { - self.plugins.insert( - Filename::new(plugin_metadata.name().to_owned()), - plugin_metadata, - ); + let old_value = self.plugins.insert(Arc::clone(&filename), plugin_metadata); + if old_value.is_none() { + self.ordered_plugin_names.push(filename); + } } } pub(crate) fn remove_plugin_metadata(&mut self, plugin_name: &str) { - let removed = self.plugins.remove(&Filename::new(plugin_name.to_owned())); + let filename = Filename::new(plugin_name.to_owned()); + let mut was_removed = self.plugins.remove(&filename).is_some(); // Only remove regex plugins if no specific plugin was removed, because // they're mutually exclusive. - if removed.is_none() { - self.regex_plugins - .retain(|p| !unicase::eq(p.name(), plugin_name)); + if !was_removed { + self.regex_plugins.retain(|p| { + let equal = unicase::eq(p.name(), plugin_name); + if equal { + was_removed = true; + } + !equal + }); + } + + if was_removed { + self.ordered_plugin_names + .retain(|f| f.as_ref() != &filename); } } @@ -359,6 +384,7 @@ impl MetadataDocument { self.messages.clear(); self.plugins.clear(); self.regex_plugins.clear(); + self.ordered_plugin_names.clear(); } } @@ -370,6 +396,7 @@ impl std::default::Default for MetadataDocument { messages: Vec::default(), plugins: HashMap::default(), regex_plugins: Vec::default(), + ordered_plugin_names: Vec::default(), } } } @@ -649,14 +676,17 @@ plugins: let mut metadata_list = MetadataDocument::default(); metadata_list.load(&path).unwrap(); - let plugin_names: Vec<_> = metadata_list - .plugins_iter() - .map(PluginMetadata::name) - .collect(); - assert!(plugin_names.contains(&"Blank.esm")); - assert!(plugin_names.contains(&"Blank.esp")); - assert!(plugin_names.contains(&"Blank.+\\.esp")); - assert!(plugin_names.contains(&"Blank.+(Different)?.*\\.esp")); + let mut plugin_names_iter = metadata_list + .ordered_plugins_iter() + .map(PluginMetadata::name); + assert_eq!("Blank.esm", plugin_names_iter.next().unwrap()); + assert_eq!("Blank.+\\.esp", plugin_names_iter.next().unwrap()); + assert_eq!( + "Blank.+(Different)?.*\\.esp", + plugin_names_iter.next().unwrap() + ); + assert_eq!("Blank.esp", plugin_names_iter.next().unwrap()); + assert_eq!(None, plugin_names_iter.next()); assert_eq!(&["C.Climate", "Relev"], metadata_list.bash_tags()); @@ -1003,14 +1033,18 @@ plugins: metadata.load_from_str(METADATA_LIST_YAML).unwrap(); assert!(!metadata.messages().is_empty()); - assert!(metadata.plugins_iter().next().is_some()); assert!(!metadata.bash_tags().is_empty()); + assert!(!metadata.plugins.is_empty()); + assert!(!metadata.regex_plugins.is_empty()); + assert!(!metadata.ordered_plugin_names.is_empty()); metadata.clear(); assert!(metadata.messages().is_empty()); - assert!(metadata.plugins_iter().next().is_none()); assert!(metadata.bash_tags().is_empty()); + assert!(metadata.plugins.is_empty()); + assert!(metadata.regex_plugins.is_empty()); + assert!(metadata.ordered_plugin_names.is_empty()); } #[test] @@ -1049,7 +1083,7 @@ plugins: } #[test] - fn add_plugin_should_store_specific_plugin_metadata() { + fn set_plugin_metadata_should_store_specific_plugin_metadata() { let mut metadata = MetadataDocument::default(); let name = "Blank.esp"; @@ -1064,7 +1098,33 @@ plugins: } #[test] - fn add_plugin_should_store_given_regex_plugin_metadata() { + fn set_plugin_metadata_should_append_the_specific_plugin_filename_to_ordered_plugin_names_if_it_is_new() + { + let mut metadata = MetadataDocument::default(); + metadata.load_from_str(METADATA_LIST_YAML).unwrap(); + + let name = "Blank - Other.esp"; + metadata.set_plugin_metadata(PluginMetadata::new(name).unwrap()); + + assert_eq!(name, metadata.ordered_plugin_names.last().unwrap().as_str()); + } + + #[test] + fn set_plugin_metadata_should_not_append_the_specific_plugin_filename_to_ordered_plugin_names_if_it_is_not_new() + { + let mut metadata = MetadataDocument::default(); + metadata.load_from_str(METADATA_LIST_YAML).unwrap(); + + let ordered_plugin_names = metadata.ordered_plugin_names.clone(); + + let name = "Blank.esp"; + metadata.set_plugin_metadata(PluginMetadata::new(name).unwrap()); + + assert_eq!(ordered_plugin_names, metadata.ordered_plugin_names); + } + + #[test] + fn set_plugin_metadata_should_store_given_regex_plugin_metadata() { let mut metadata = MetadataDocument::default(); let mut plugin = PluginMetadata::new(".+Dependent\\.esp").unwrap(); @@ -1078,6 +1138,17 @@ plugins: assert_eq!("group1", plugin.group().unwrap()); } + #[test] + fn set_plugin_metadata_should_append_the_regex_plugin_filename_to_ordered_plugin_names() { + let mut metadata = MetadataDocument::default(); + metadata.load_from_str(METADATA_LIST_YAML).unwrap(); + + let name = ".+Dependent\\.esp"; + metadata.set_plugin_metadata(PluginMetadata::new(name).unwrap()); + + assert_eq!(name, metadata.ordered_plugin_names.last().unwrap().as_str()); + } + #[test] fn remove_plugin_metadata_should_remove_the_given_plugin_specific_metadata() { let mut metadata = MetadataDocument::default(); @@ -1089,6 +1160,12 @@ plugins: metadata.remove_plugin_metadata(name); assert!(metadata.find_plugin(name).unwrap().is_none()); + + assert!( + !metadata + .ordered_plugin_names + .contains(&Arc::new(Filename::new(name.to_owned()))) + ); } #[test] @@ -1112,6 +1189,12 @@ plugins: metadata.remove_plugin_metadata(regex_name); assert!(metadata.find_plugin(name).unwrap().is_none()); + + assert!( + !metadata + .ordered_plugin_names + .contains(&Arc::new(Filename::new(name.to_owned()))) + ); } #[test] @@ -1135,6 +1218,12 @@ plugins: metadata.remove_plugin_metadata("blank.*\\.esp"); assert!(metadata.find_plugin(name).unwrap().is_none()); + + assert!( + !metadata + .ordered_plugin_names + .contains(&Arc::new(Filename::new(name.to_owned()))) + ); } #[test] @@ -1145,13 +1234,15 @@ plugins: let name = "Blank.+\\.esp"; assert!(metadata.find_plugin(name).unwrap().is_some()); - metadata.remove_plugin_metadata(name); - - assert!(metadata.find_plugin(name).unwrap().is_some()); - metadata.remove_plugin_metadata("Blank - Different.esp"); assert!(metadata.find_plugin(name).unwrap().is_some()); + + assert!( + metadata + .ordered_plugin_names + .contains(&Arc::new(Filename::new(name.to_owned()))) + ); } }