From a9bd7bf64f32bcd1722d4881966ad8a7ed8b5574 Mon Sep 17 00:00:00 2001 From: BRUNER Patrick Date: Wed, 29 Jul 2026 11:41:19 +0200 Subject: [PATCH 1/2] fix: read legacy XML Session File filters with Newtonsoft again Covering the PersisterXML -> Decompose path end-to-end, the gap the #666 review left open, turned up a live bug rather than the clean pass it was meant to pin. The XML writer that produced these files serialised FilterParams with Newtonsoft, which writes Color as "Black". PersisterXML.ReadFilter had since been switched to System.Text.Json, which cannot read that back: it throws, ReadFilter's catch swallows the exception, and the entry is skipped. Every filter in every legacy XML Session File was therefore dropped silently on load - FilterParamsList came back empty. Reading with Newtonsoft restores symmetry with the writer, and with the rest of the persistence stack (Persister, the columnizer and encoding converters, and FilterParams.CurrentColumnizer's own JsonConverter attribute are all Newtonsoft). A null payload is now skipped explicitly rather than throwing. The new test builds its fixture by serialising through Newtonsoft the way the writer did, instead of hand-writing JSON - a hand-written payload is a shape no LogExpert version ever wrote, and omitting Color is exactly what would have hidden this. It fails against the previous reader with zero entries parsed, and asserts Color survives alongside the [0] rule. --- .../Classes/Persister/PersisterXML.cs | 18 +++- .../SessionFileComposerTests.cs | 82 ++++++++++++++++++- 2 files changed, 95 insertions(+), 5 deletions(-) diff --git a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs index 60b122fe..1a66fd96 100644 --- a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs +++ b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs @@ -1,13 +1,14 @@ using System.Drawing; using System.Globalization; using System.Text; -using System.Text.Json; using System.Xml; using LogExpert.Core.Classes.Filter; using LogExpert.Core.Entities; using LogExpert.Core.Helpers; +using Newtonsoft.Json; + using NLog; namespace LogExpert.Core.Classes.Persister; @@ -153,7 +154,20 @@ private static List ReadFilter (XmlElement startNode) using MemoryStream stream = new(data); try { - FilterParams filterParams = JsonSerializer.Deserialize(stream); + // Newtonsoft, symmetrically with the writer that produced these + // payloads: it writes Color as "Black", which System.Text.Json cannot + // read back — it throws, the catch below swallows it, and the filter + // is lost. The rest of the persistence stack is Newtonsoft too. + using StreamReader streamReader = new(stream); + using JsonTextReader jsonReader = new(streamReader); + JsonSerializer serializer = new(); + var filterParams = serializer.Deserialize(jsonReader); + + if (filterParams == null) + { + continue; + } + filterParams.Init(); filterList.Add(filterParams); } diff --git a/src/LogExpert.Persister.Tests/SessionFileComposerTests.cs b/src/LogExpert.Persister.Tests/SessionFileComposerTests.cs index 264b59f0..b1c97af8 100644 --- a/src/LogExpert.Persister.Tests/SessionFileComposerTests.cs +++ b/src/LogExpert.Persister.Tests/SessionFileComposerTests.cs @@ -1,5 +1,7 @@ using System.Collections; +using System.Drawing; using System.Reflection; +using System.Security; using System.Text; using LogExpert.Core.Classes.Filter; @@ -267,6 +269,59 @@ private static PersistenceData CreateCarriedPersistenceData (int filterTabDepth) }; } + /// + /// Writes a Session File in the pre-JSON XML format, carrying + /// as its filter list — the shape older LogExpert versions wrote, where that list is a copy + /// of the whole Saved Filter List. Only the nodes the reader needs are present: + /// options is mandatory (the reader dereferences it unconditionally), and each + /// filter holds the base64 payload described on + /// . + /// + private string WriteLegacyXmlSessionFile (params string[] searchTexts) + { + // Distinct spreads, so an assertion on the restored entry can tell which one it came from. + var filters = string.Concat(searchTexts.Select((searchText, index) => + $"{EncodeLegacyXmlFilterParams(searchText, spreadBefore: index + 1)}")); + + var xml = $""" + + + + + + + + + + {filters} + + + """; + + var fileName = Path.Join(_testDirectory, "legacy.lxp"); + File.WriteAllText(fileName, xml, Encoding.UTF8); + + return fileName; + } + + /// + /// The payload of one legacy <params> node, produced the way the XML writer + /// produced it before it was deleted (see Persister.WriteFilter at ef33f363^): + /// Newtonsoft with default settings, base64-encoded. Serializing here rather than + /// hand-writing the JSON is the point — a hand-written literal only pins a shape we invented, + /// and would silently omit the fields that are hard to read back (notably Color, which + /// Newtonsoft writes as "Black"). + /// + private static string EncodeLegacyXmlFilterParams (string searchText, int spreadBefore) + { + var filterParams = CreateFilterParams(searchText); + filterParams.SpreadBefore = spreadBefore; + + var json = JsonConvert.SerializeObject(filterParams); + + return Convert.ToBase64String(Encoding.UTF8.GetBytes(json)); + } + #endregion #region Fixture completeness (the regression guard) @@ -371,7 +426,7 @@ public void DiskTrip_ComposePersistLoadDecompose_ReturnsEqualSnapshot () #region Window filter (issue #666) - // A Session File carries the window's OWN filter, not a copy of the global filter list. The + // A Session File carries the Window Filter, not a copy of the Saved Filter List. The // on-disk shape stays a list so existing .lxp files keep loading — new saves just write // exactly one entry into it. [Test] @@ -397,8 +452,8 @@ public void Compose_WithoutAWindowFilter_WritesAnEmptyFilterParamsList () Assert.That(SessionFileComposer.Compose(snapshot).FilterParamsList, Is.Empty); } - // The compatibility story: a pre-#666 Session File carries a copy of the whole global filter - // list, and load has always taken [0] as the window's filter. That stays true — and the + // The compatibility story: a pre-#666 Session File carries a copy of the whole Saved Filter + // List, and load has always taken [0] as the Window Filter. That stays true — and the // trailing entries, which nothing ever restored, are dropped when such a file is re-saved. [Test] public void Decompose_LegacySessionFileCarryingTheGlobalList_KeepsTheFirstEntryAndDropsTheRest () @@ -418,6 +473,27 @@ public void Decompose_LegacySessionFileCarryingTheGlobalList_KeepsTheFirstEntryA Assert.That(recomposed.FilterParamsList[0].SearchText, Is.EqualTo("the first global filter")); } + // The oldest Session Files are XML, not JSON, and are read by a different reader + // (PersisterXML) that fills the very same FilterParamsList with the old Saved Filter List. Loading + // through Persister.Load exercises the real path — JSON parse fails, the XML fallback takes + // over — so this pins that the [0] rule reaches such a file too, not just JSON ones. + [Test] + public void Decompose_LegacyXmlSessionFileOnDisk_TakesTheFirstFilterAsTheWindowFilter () + { + var sessionFileName = WriteLegacyXmlSessionFile("the first global filter", "another global filter"); + + var loaded = Core.Classes.Persister.Persister.Load(sessionFileName); + var snapshot = SessionFileComposer.Decompose(loaded); + + Assert.That(loaded.FilterParamsList, Has.Count.EqualTo(2), + "the XML fallback must actually have parsed both legacy entries — otherwise this test proves nothing"); + Assert.That(snapshot.FilterParams.SearchText, Is.EqualTo("the first global filter")); + Assert.That(snapshot.FilterParams.SpreadBefore, Is.EqualTo(1), + "the restored entry must be the first one, whole — the second carries SpreadBefore 2"); + Assert.That(snapshot.FilterParams.Color, Is.EqualTo(Color.Black), + "Color is the field the legacy payload writes as \"Black\" — losing it means the entry was not really read"); + } + [Test] public void Decompose_SessionFileWithoutFilters_LeavesTheWindowFilterUnset () { From 6c9c851dca6c3d69042673c5ef2856d0bcbc9363 Mon Sep 17 00:00:00 2001 From: BRUNER Patrick Date: Wed, 29 Jul 2026 12:54:47 +0200 Subject: [PATCH 2/2] small update --- src/LogExpert.Core/Classes/Persister/PersisterXML.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs index 1a66fd96..674f7159 100644 --- a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs +++ b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs @@ -154,10 +154,6 @@ private static List ReadFilter (XmlElement startNode) using MemoryStream stream = new(data); try { - // Newtonsoft, symmetrically with the writer that produced these - // payloads: it writes Color as "Black", which System.Text.Json cannot - // read back — it throws, the catch below swallows it, and the filter - // is lost. The rest of the persistence stack is Newtonsoft too. using StreamReader streamReader = new(stream); using JsonTextReader jsonReader = new(streamReader); JsonSerializer serializer = new();