fix: read legacy XML Session File filters with Newtonsoft again - #683
Merged
Conversation
added 2 commits
July 29, 2026 11:41
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.