diff --git a/Frontmatter.Test/EquivalentKeyMergeTests.cs b/Frontmatter.Test/EquivalentKeyMergeTests.cs new file mode 100644 index 0000000..83d2fc5 --- /dev/null +++ b/Frontmatter.Test/EquivalentKeyMergeTests.cs @@ -0,0 +1,68 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Frontmatter.Test; + +using System.Collections.Generic; +using System.Linq; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Regression tests for keys that normalize to the same name, such as related and +/// page_related. Each key used to be mapped to the other key's name, so the two were +/// swapped rather than merged. +/// +[TestClass] +public class EquivalentKeyMergeTests +{ + private static readonly string[] ListA = ["a"]; + private static readonly string[] ListB = ["b"]; + + [TestMethod] + [DataRow(FrontmatterMergeStrategy.Aggressive, false)] + [DataRow(FrontmatterMergeStrategy.Aggressive, true)] + [DataRow(FrontmatterMergeStrategy.Maximum, false)] + [DataRow(FrontmatterMergeStrategy.Maximum, true)] + public void EquivalentListKeys_MergeUnderOneNameWithBothValues(FrontmatterMergeStrategy strategy, bool prefixedFirst) + { + Dictionary frontmatter = prefixedFirst + ? new() { ["page_related"] = ListB, ["related"] = ListA } + : new() { ["related"] = ListA, ["page_related"] = ListB }; + + Dictionary result = PropertyMerger.MergeSimilarProperties(frontmatter, strategy); + + Assert.HasCount(1, result); + Assert.IsTrue(result.TryGetValue("related", out object? related), "The unprefixed key should name the merged property"); + CollectionAssert.AreEquivalent(new object[] { "a", "b" }, ((IEnumerable)related).ToArray()); + } + + [TestMethod] + [DataRow(FrontmatterMergeStrategy.Aggressive, false)] + [DataRow(FrontmatterMergeStrategy.Aggressive, true)] + [DataRow(FrontmatterMergeStrategy.Maximum, false)] + [DataRow(FrontmatterMergeStrategy.Maximum, true)] + public void EquivalentScalarKeys_MergeIntoOneProperty(FrontmatterMergeStrategy strategy, bool prefixedFirst) + { + Dictionary frontmatter = prefixedFirst + ? new() { ["page_related"] = "y", ["related"] = "x" } + : new() { ["related"] = "x", ["page_related"] = "y" }; + + Dictionary result = PropertyMerger.MergeSimilarProperties(frontmatter, strategy); + + Assert.HasCount(1, result, "Two scalars that normalize alike should merge into one property"); + } + + [TestMethod] + public void CombineFrontmatter_Aggressive_MergesRelatedListsInsteadOfSwappingThem() + { + const string input = "---\nrelated:\n- a\npage_related:\n- b\n---\nBody\n"; + + string result = Frontmatter.CombineFrontmatter(input, FrontmatterNaming.AsIs, FrontmatterOrder.AsIs, FrontmatterMergeStrategy.Aggressive); + + Dictionary? frontmatter = Frontmatter.ExtractFrontmatter(result); + Assert.IsNotNull(frontmatter); + Assert.HasCount(1, frontmatter); + Assert.IsTrue(frontmatter.TryGetValue("related", out object? related), "The merged list should be written under related"); + CollectionAssert.AreEquivalent(new object[] { "a", "b" }, ((IEnumerable)related).ToArray()); + } +} diff --git a/Frontmatter/PropertyMerger.cs b/Frontmatter/PropertyMerger.cs index 323059a..baa50d2 100644 --- a/Frontmatter/PropertyMerger.cs +++ b/Frontmatter/PropertyMerger.cs @@ -3,6 +3,7 @@ namespace ktsu.Frontmatter; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; /// @@ -40,9 +41,14 @@ internal static Dictionary MergeSimilarProperties(Dictionary group in propertyMappings.GroupBy(x => x.Value, x => x.Key)) { - string canonicalKey = group.Key; List originalKeys = [.. group]; + // A key with nothing to merge with keeps its own name, so no key is ever renamed onto + // another key's name without the two actually being merged. + string canonicalKey = originalKeys.Count == 1 && frontmatter.ContainsKey(group.Key) && group.Key != originalKeys[0] + ? originalKeys[0] + : group.Key; + MergePropertyGroup(frontmatter, mergedFrontmatter, canonicalKey, originalKeys); } @@ -203,19 +209,9 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) } // Look for exact matches after normalization - foreach (string existingKey in existingKeys) + if (TryFindEquivalenceClassName(key, normalizedKey, existingKeys, out string? className)) { - if (existingKey == key) - { - continue; - } - - string normalizedExisting = NormalizePropertyName(existingKey); - if (string.Equals(normalizedKey, normalizedExisting, StringComparison.OrdinalIgnoreCase)) - { - // If the existing key is a known property, use its canonical name - return PropertyMappings.All.TryGetValue(existingKey, out string? knownName) ? knownName : existingKey; - } + return className; } // Look for partial matches @@ -235,6 +231,13 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) if (normalizedKey.Contains(normalizedExisting2, StringComparison.OrdinalIgnoreCase) || normalizedExisting2.Contains(normalizedKey, StringComparison.OrdinalIgnoreCase)) { + // Both keys of a pair must agree on one name, or each would be renamed to the other. + string preferred = PreferredName([key, existingKey]); + if (preferred == key) + { + continue; + } + // If the existing key is a known property, use its canonical name return PropertyMappings.All.TryGetValue(existingKey, out string? knownName2) ? knownName2 : existingKey; } @@ -243,6 +246,46 @@ private static string FindBasicCanonicalName(string key, string[] existingKeys) return key; } + /// + /// Finds the one name shared by every key that normalizes the same as . + /// + /// The key to analyze. + /// The normalized form of . + /// All existing keys in the frontmatter. + /// The name every key in the class maps to. + /// True if at least one other key normalizes the same as . + private static bool TryFindEquivalenceClassName(string key, string normalizedKey, string[] existingKeys, [NotNullWhen(true)] out string? className) + { + List equivalenceClass = [key]; + equivalenceClass.AddRange(existingKeys.Where(existingKey => existingKey != key && + string.Equals(normalizedKey, NormalizePropertyName(existingKey), StringComparison.OrdinalIgnoreCase))); + + if (equivalenceClass.Count == 1) + { + className = null; + return false; + } + + // Every key in the class must reach the same name, whichever of them is asking. Mapping each + // key to some other key instead sent related to page_related and page_related to related, + // which swapped their values rather than merging them. + string preferred = PreferredName(equivalenceClass); + className = PropertyMappings.All.TryGetValue(preferred, out string? knownName) ? knownName : preferred; + return true; + } + + /// + /// Picks one name from a set of keys, independently of their order: a known property first, + /// then the shortest key, with any tie broken ordinally. + /// + /// The keys to choose between. + /// The preferred key. + private static string PreferredName(IEnumerable keys) => keys + .OrderBy(k => PropertyMappings.All.ContainsKey(k) ? 0 : 1) + .ThenBy(k => k.Length) + .ThenBy(k => k, StringComparer.Ordinal) + .First(); + /// /// Attempts to find a canonical name for a property key using semantic analysis. /// @@ -258,6 +301,13 @@ private static string FindSemanticCanonicalName(string key, string[] existingKey return basicMatch; } + // The key is the name its equivalence class merges under, so it must not move elsewhere. + string normalizedKey = NormalizePropertyName(key); + if (normalizedKey.Length > 0 && TryFindEquivalenceClassName(key, normalizedKey, existingKeys, out _)) + { + return key; + } + // Then try more aggressive matching using word similarity string[] keyWords = PropertyNameNormalizer.NormalizeToWords(key);