refactor(export): read text from chunks, write the file atomically

Two changes to MessageExporter before it gets a caller.

It read SenderSource and ContentSource, the raw SeStrings. TextValue on one
holding an auto-translate phrase reaches SeStringEvaluator, which asserts it is
on the main thread and throws unconditionally when a macro resolves a global
number. An export belongs on a worker, so that would abort it partway and leave
half a file.

The plan called for resolving text in batches on the framework thread. Not
needed: Message.Sender and Message.Content are already-resolved chunk lists --
ChunkUtil turns auto-translate into text at ingest, and the full-text index reads
them exactly this way. Same strings, no evaluator, no thread affinity, and no
batching machinery.

Second, the file handling. The format was validated after the StreamWriter was
opened, so an unknown format left a zero-byte file where a previous export had
been. It is checked first now, and the write goes to a .part file that is moved
into place at the end. A crash halfway used to leave a file that opens cleanly
and is quietly incomplete -- which on the path a GDPR access request goes out on
is worse than an obvious failure.

Almost none of this is reachable from the build suite: ExportToFile takes
IEnumerable<Message>, Message needs SeString, and xUnit cannot load Dalamud.dll
-- even an empty list fails, because the runtime resolves the parameter type
before the body runs. So the format mapping is pinned there and the rest by a new
self-test, which builds probe messages with deliberately empty SeStrings: if the
exporter ever reads them again, the text comes out blank and it fails.

HellionChat/Export/MessageExporter.cs

@@ -1,6 +1,7 @@
 using System.Globalization;
 using System.Text;
 using HellionChat.Code;
+using HellionChat.Util;
 
 namespace HellionChat.Export;
 
@@ -33,7 +34,19 @@ internal static class ExportFormatExt
 }
 
 // Serializes message snapshots to Markdown, JSON, or CSV.
-// Caller handles pre-filtering except sender substring, which requires deserialized SeString.TextValue.
+//
+// Text comes from the chunk lists, never from SenderSource/ContentSource. Those
+// are raw SeStrings, and reading TextValue on one containing an auto-translate
+// phrase reaches SeStringEvaluator, which asserts it is on the main thread and
+// throws unconditionally when a macro resolves a global number. An export runs on
+// a worker, so that would abort it partway and leave half a file behind.
+//
+// The chunks are already resolved: ChunkUtil turns auto-translate into text at
+// ingest, and the full-text index reads them exactly this way. Same strings, no
+// evaluator, no thread affinity.
+//
+// The caller pre-filters by channel and date via StreamForExport; only the sender
+// substring is applied here.
 internal static class MessageExporter
 {
     internal record FilterDescription(
@@ -50,22 +63,64 @@ internal static class MessageExporter
         FilterDescription filter
     )
     {
+        // Rejected before the file is touched. The old order opened the stream
+        // first, so an unknown format left a zero-byte file where the user's
+        // previous export had been.
+        if (!Enum.IsDefined(format))
+            throw new ArgumentOutOfRangeException(nameof(format), format, null);
+
         var matching = filter.SenderSubstring is { Length: > 0 } needle
             ? messages.Where(m => MatchesSender(m, needle))
             : messages;
 
-        using var writer = new StreamWriter(path, append: false, encoding: Encoding.UTF8);
-        return format switch
+        // Written beside the target and moved into place at the end. A crash or
+        // an unplugged drive halfway through would otherwise leave a file that
+        // opens fine and is quietly incomplete -- and this is the path a GDPR
+        // access request goes out on, where "looks complete" is the dangerous
+        // failure.
+        var temp = path + ".part";
+        int written;
+        try
         {
-            ExportFormat.Markdown => WriteMarkdown(writer, matching, filter),
-            ExportFormat.Json => WriteJson(writer, matching, filter),
-            ExportFormat.Csv => WriteCsv(writer, matching, filter),
-            _ => throw new ArgumentOutOfRangeException(nameof(format), format, null),
-        };
+            using (var writer = new StreamWriter(temp, append: false, encoding: Encoding.UTF8))
+            {
+                written = format switch
+                {
+                    ExportFormat.Markdown => WriteMarkdown(writer, matching, filter),
+                    ExportFormat.Json => WriteJson(writer, matching, filter),
+                    _ => WriteCsv(writer, matching, filter),
+                };
+            }
+
+            File.Move(temp, path, overwrite: true);
+            return written;
+        }
+        catch
+        {
+            TryDeleteTemp(temp);
+            throw;
+        }
+    }
+
+    // Best effort: the export already failed, and a leftover .part file is a
+    // smaller problem than masking the original exception with an IO one.
+    private static void TryDeleteTemp(string temp)
+    {
+        try
+        {
+            if (File.Exists(temp))
+                File.Delete(temp);
+        }
+        catch (IOException) { }
+        catch (UnauthorizedAccessException) { }
     }
 
     private static bool MatchesSender(Message m, string needle) =>
-        m.SenderSource.TextValue.Contains(needle, StringComparison.OrdinalIgnoreCase);
+        SenderText(m).Contains(needle, StringComparison.OrdinalIgnoreCase);
+
+    private static string SenderText(Message m) => ChunkUtil.ToRawString(m.Sender);
+
+    private static string ContentText(Message m) => ChunkUtil.ToRawString(m.Content);
 
     private static int WriteMarkdown(
         StreamWriter w,
@@ -94,8 +149,8 @@ internal static class MessageExporter
             }
 
             var chatType = (ChatType)(ushort)m.Code.Type;
-            var sender = m.SenderSource.TextValue.Trim().Trim('<', '>', '[', ']', ':').Trim();
-            var content = m.ContentSource.TextValue;
+            var sender = SenderText(m).Trim().Trim('<', '>', '[', ']', ':').Trim();
+            var content = ContentText(m);
 
             if (string.IsNullOrEmpty(sender))
                 w.WriteLine($"**[{localDate:HH:mm}] {chatType}:** {content}");
@@ -174,8 +229,8 @@ internal static class MessageExporter
             w.Write($",\"target_kind\":{m.Code.Target}");
             w.Write($",\"receiver\":{m.Receiver}");
             w.Write($",\"content_id\":{m.ContentId}");
-            w.Write($",\"sender\":{JsonString(m.SenderSource.TextValue)}");
-            w.Write($",\"content\":{JsonString(m.ContentSource.TextValue)}");
+            w.Write($",\"sender\":{JsonString(SenderText(m))}");
+            w.Write($",\"content\":{JsonString(ContentText(m))}");
             w.Write("}");
         }
 
@@ -203,9 +258,9 @@ internal static class MessageExporter
             w.Write(',');
             w.Write(CsvString(chatType.ToString()));
             w.Write(',');
-            w.Write(CsvString(m.SenderSource.TextValue));
+            w.Write(CsvString(SenderText(m)));
             w.Write(',');
-            w.Write(CsvString(m.ContentSource.TextValue));
+            w.Write(CsvString(ContentText(m)));
             w.Write(',');
             w.Write(m.Receiver);
             w.Write(',');

HellionChat/Plugin.cs

@@ -422,6 +422,7 @@ public sealed class Plugin : IAsyncDalamudPlugin
                 await _lifecycle.LoadAsync(cancellationToken).ConfigureAwait(false);
 
             SelfTestRegistry.RegisterTestSteps([
+                new SelfTests.ExportRoundTripStep(),
                 new SelfTests.ThemeSwitchSelfTestStep(this),
                 new SelfTests.ThemeCrossfadeSelfTestStep(this),
                 new SelfTests.FontManagerCtorSmokeStep(this),

HellionChat/SelfTests/ExportRoundTripStep.cs

@@ -0,0 +1,163 @@
+using Dalamud.Bindings.ImGui;
+using Dalamud.Game.Text;
+using Dalamud.Plugin.SelfTest;
+using HellionChat.Code;
+using HellionChat.Export;
+using HellionChat.Util;
+
+namespace HellionChat.SelfTests;
+
+// v1.12.0/A2: the exporter now reads text from the chunk lists instead of the
+// raw SeStrings. That change is invisible to the build suite -- ExportToFile
+// takes IEnumerable<Message>, Message needs SeString, and xUnit cannot load
+// Dalamud.dll, so even an empty list fails before the body runs.
+//
+// So it is verified here, against real messages, with the three properties that
+// actually matter:
+//
+//   1. Text survives the round trip. If the exporter ever reads SenderSource or
+//      ContentSource again, a message built without them comes out blank.
+//   2. The format guard runs before the file is opened. It used to run after,
+//      so an unknown format left a zero-byte file where a previous export had
+//      been.
+//   3. The write is atomic. A failure partway must not leave a file that opens
+//      cleanly and is quietly incomplete -- this is the path a GDPR access
+//      request goes out on.
+internal sealed class ExportRoundTripStep : ISelfTestStep
+{
+    public string Name => "Hellion Chat - Export round trip";
+
+    public SelfTestStepResult RunStep()
+    {
+        var dir = Path.Combine(Path.GetTempPath(), $"hellionchat-selftest-{Guid.NewGuid():N}");
+        Directory.CreateDirectory(dir);
+
+        var failures = new List<string>();
+        try
+        {
+            CheckTextSurvives(dir, failures);
+            CheckUnknownFormatKeepsExistingFile(dir, failures);
+            CheckNoLeftoverPartFile(dir, failures);
+        }
+        catch (Exception e)
+        {
+            failures.Add($"threw: {e.GetType().Name}: {e.Message}");
+        }
+        finally
+        {
+            TryCleanup(dir);
+        }
+
+        foreach (var f in failures)
+            ImGui.Text(f);
+
+        SelfTestReport.Append(Name, failures.Count == 0 ? "PASS" : "FAIL", failures);
+        return failures.Count == 0 ? SelfTestStepResult.Pass : SelfTestStepResult.Fail;
+    }
+
+    private static void CheckTextSurvives(string dir, List<string> failures)
+    {
+        const string sender = "Selftest Sender";
+        const string content = "selftest content marker";
+
+        var path = Path.Combine(dir, "roundtrip.csv");
+        var written = MessageExporter.ExportToFile(
+            path,
+            ExportFormat.Csv,
+            [Probe(sender, content)],
+            new MessageExporter.FilterDescription(null, null, null, null)
+        );
+
+        if (written != 1)
+            failures.Add($"expected 1 message written, got {written}");
+
+        var text = File.ReadAllText(path);
+        if (!text.Contains(sender, StringComparison.Ordinal))
+            failures.Add("sender missing from export -- reading SeString again?");
+        if (!text.Contains(content, StringComparison.Ordinal))
+            failures.Add("content missing from export -- reading SeString again?");
+
+        // The sender filter runs inside the exporter and must see the same text.
+        var filtered = Path.Combine(dir, "filtered.csv");
+        var hits = MessageExporter.ExportToFile(
+            filtered,
+            ExportFormat.Csv,
+            [Probe(sender, content), Probe("Someone Else", "other")],
+            new MessageExporter.FilterDescription(null, null, null, "selftest sen")
+        );
+
+        if (hits != 1)
+            failures.Add($"sender filter matched {hits} messages, expected 1");
+    }
+
+    private static void CheckUnknownFormatKeepsExistingFile(string dir, List<string> failures)
+    {
+        var path = Path.Combine(dir, "previous.md");
+        File.WriteAllText(path, "an earlier export");
+
+        try
+        {
+            MessageExporter.ExportToFile(
+                path,
+                (ExportFormat)99,
+                [],
+                new MessageExporter.FilterDescription(null, null, null, null)
+            );
+            failures.Add("unknown format did not throw");
+        }
+        catch (ArgumentOutOfRangeException) { }
+
+        if (File.ReadAllText(path) != "an earlier export")
+            failures.Add("unknown format destroyed the existing file");
+    }
+
+    private static void CheckNoLeftoverPartFile(string dir, List<string> failures)
+    {
+        var path = Path.Combine(dir, "clean.json");
+        MessageExporter.ExportToFile(
+            path,
+            ExportFormat.Json,
+            [Probe("A", "b")],
+            new MessageExporter.FilterDescription(null, null, null, null)
+        );
+
+        if (!File.Exists(path))
+            failures.Add("export produced no file");
+        if (File.Exists(path + ".part"))
+            failures.Add("temporary file left behind after a successful export");
+    }
+
+    // Built with empty SeStrings on purpose: the whole point is that the text
+    // comes from the chunks.
+    private static Message Probe(string sender, string content)
+    {
+        static List<Chunk> Text(string s) =>
+            [new TextChunk(ChunkSource.Content, null, null, null, null, false, s)];
+
+        return new Message(
+            0,
+            0,
+            0,
+            new ChatCode(XivChatType.Say, 0, 0),
+            Text(sender),
+            Text(content),
+            new Dalamud.Game.Text.SeStringHandling.SeString(),
+            new Dalamud.Game.Text.SeStringHandling.SeString()
+        );
+    }
+
+    // Nothing to undo between runs: every file lives in a fresh temp directory
+    // that RunStep deletes in its own finally.
+    public void CleanUp() { }
+
+    private static void TryCleanup(string dir)
+    {
+        try
+        {
+            if (Directory.Exists(dir))
+                Directory.Delete(dir, recursive: true);
+        }
+        catch (IOException) { }
+        catch (UnauthorizedAccessException) { }
+    }
+}