Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 0 additions & 1 deletion Frontmatter.Test/DecorationOnlyKeyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class DecorationOnlyKeyTests
public void ClearCaches()
{
ClearCache(typeof(NameStandardizer), "PropertyNameCache");
ClearCache(typeof(PropertyMerger), "PropertyMergeCache");
}

private static void ClearCache(Type type, string fieldName)
Expand Down
87 changes: 87 additions & 0 deletions Frontmatter.Test/MergeStrategyIsolationTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Copyright (c) 2023-2026 ktsu-dev contributors

namespace ktsu.Frontmatter.Test;

using System.Collections.Generic;

using Microsoft.VisualStudio.TestTools.UnitTesting;

/// <summary>
/// Regression tests for #130: the canonical name a key merges to depends on the strategy and on
/// the other keys in the same document, so one call must not decide it for a later one.
/// </summary>
/// <remarks>
/// These tests deliberately clear nothing between calls. The bug lived in process-wide state, and a
/// test that resets that state cannot see it.
/// </remarks>
[TestClass]
public class MergeStrategyIsolationTests
{
[TestMethod]
public void ConservativeAfterAggressive_KeepsKeyThatOnlyAggressiveMerges()
{
static Dictionary<string, object> Document() => new()
{
["title"] = "T",
["custom_title"] = "C",
};

Dictionary<string, object> aggressive = PropertyMerger.MergeSimilarProperties(Document(), FrontmatterMergeStrategy.Aggressive);
Assert.IsFalse(aggressive.ContainsKey("custom_title"), "Aggressive should merge custom_title into title");

Dictionary<string, object> conservative = PropertyMerger.MergeSimilarProperties(Document(), FrontmatterMergeStrategy.Conservative);

Assert.AreEqual("T", conservative["title"]);
Assert.IsTrue(conservative.ContainsKey("custom_title"), "Conservative only applies predefined mappings, so custom_title must survive");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
Assert.AreEqual("C", conservative["custom_title"]);
}

[TestMethod]
public void AggressiveAfterConservative_StillMergesKeys()
{
static Dictionary<string, object> Document() => new()
{
["summary"] = "S",
["page_summary"] = "P",
};

Dictionary<string, object> conservative = PropertyMerger.MergeSimilarProperties(Document(), FrontmatterMergeStrategy.Conservative);
Assert.HasCount(2, conservative);

Dictionary<string, object> aggressive = PropertyMerger.MergeSimilarProperties(Document(), FrontmatterMergeStrategy.Aggressive);

Assert.HasCount(1, aggressive);
}

[TestMethod]
public void KeyAloneInALaterDocument_IsNotRenamedByAnEarlierDocument()
{
PropertyMerger.MergeSimilarProperties(new Dictionary<string, object>
{
["blurb"] = "B",
["page_blurb"] = "P",
}, FrontmatterMergeStrategy.Aggressive);

Dictionary<string, object> later = PropertyMerger.MergeSimilarProperties(new Dictionary<string, object>
{
["page_blurb"] = "Only",
}, FrontmatterMergeStrategy.Aggressive);

Assert.IsTrue(later.ContainsKey("page_blurb"), "With no other key to merge with, page_blurb keeps its name");
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
Assert.AreEqual("Only", later["page_blurb"]);
}

[TestMethod]
public void CombineFrontmatter_ConservativeAfterAggressive_KeepsCustomTitle()
{
const string first = "---\ntitle: T\ncustom_title: C\n---\nBody1\n";
const string second = "---\ntitle: T\ncustom_title: C\n---\nBody2\n";

Frontmatter.CombineFrontmatter(first, FrontmatterNaming.AsIs, FrontmatterOrder.AsIs, FrontmatterMergeStrategy.Aggressive);
string result = Frontmatter.CombineFrontmatter(second, FrontmatterNaming.AsIs, FrontmatterOrder.AsIs, FrontmatterMergeStrategy.Conservative);

Dictionary<string, object>? frontmatter = Frontmatter.ExtractFrontmatter(result);
Assert.IsNotNull(frontmatter);
Assert.AreEqual("C", frontmatter["custom_title"]);
}
}
18 changes: 0 additions & 18 deletions Frontmatter.Test/PropertyMergerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
namespace ktsu.Frontmatter.Test;

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Reflection;

using Microsoft.VisualStudio.TestTools.UnitTesting;

Expand All @@ -24,22 +22,6 @@ public class PropertyMergerTests
private static readonly string[] valueArray = ["tag1", "tag2"];
private static readonly string[] valueArray0 = ["tag2", "tag3"];

[TestInitialize]
public void ClearPropertyMergerCache()
{
// Clear the static cache between tests using reflection
FieldInfo? cacheField = typeof(PropertyMerger).GetField("PropertyMergeCache",
BindingFlags.NonPublic | BindingFlags.Static);

if (cacheField != null)
{
ConcurrentDictionary<string, string>? cache = cacheField.GetValue(null) as ConcurrentDictionary<string, string>;
cache?.Clear();
}

Console.WriteLine("Cache cleared");
}

[TestMethod]
public void MergeSimilarProperties_NoStrategy_DoesntMergeAnything()
{
Expand Down
1 change: 0 additions & 1 deletion Frontmatter.Test/SingleCharacterKeyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ public class SingleCharacterKeyTests
public void ClearCaches()
{
ClearCache(typeof(NameStandardizer), "PropertyNameCache");
ClearCache(typeof(PropertyMerger), "PropertyMergeCache");
}

private static void ClearCache(Type type, string fieldName)
Expand Down
24 changes: 4 additions & 20 deletions Frontmatter/PropertyMerger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace ktsu.Frontmatter;

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;

Expand All @@ -11,11 +10,6 @@ namespace ktsu.Frontmatter;
/// </summary>
internal static class PropertyMerger
{
/// <summary>
/// Cache for property merge mappings
/// </summary>
private static readonly ConcurrentDictionary<string, string> PropertyMergeCache = new();

/// <summary>
/// Merges properties that capture redundant information based on the specified strategy.
/// </summary>
Expand Down Expand Up @@ -55,18 +49,14 @@ internal static Dictionary<string, object> MergeSimilarProperties(Dictionary<str
return mergedFrontmatter;
}

// Not cached: the canonical name depends on the strategy and, for Aggressive and Maximum, on the
// other keys in the same document. A cache keyed on the property name alone let whichever call
// reached a key first decide its fate for every later call in the process.
private static string GetCanonicalName(string key, FrontmatterMergeStrategy strategy, string[] frontmatterKeys)
{
// Try to find in cache first
if (PropertyMergeCache.TryGetValue(key, out string? cachedName))
{
return cachedName;
}

// For None strategy or keys with special characters, preserve the original key
if (strategy == FrontmatterMergeStrategy.None || key.Any(c => !char.IsLetterOrDigit(c) && c != '_' && c != '-'))
{
PropertyMergeCache.TryAdd(key, key);
return key;
}

Expand All @@ -80,13 +70,7 @@ private static string GetCanonicalName(string key, FrontmatterMergeStrategy stra
};

// If no mapping was found, preserve the original key
if (string.IsNullOrEmpty(canonicalName) || canonicalName == key)
{
canonicalName = key;
}

PropertyMergeCache.TryAdd(key, canonicalName);
return canonicalName;
return string.IsNullOrEmpty(canonicalName) ? key : canonicalName;
}

private static string GetConservativeCanonicalName(string key) =>
Expand Down
Loading