Merge pull request connamara#956 from hvanderwal-mylette/bugfix/955_FileLogClear

Bugfix 955 FileLog.Clear() does not clear the log files, and causes any further logging to throw exceptions

Author: gbirchmeier

QuickFIXn/Logger/FileLog.cs

@@ -90,6 +90,9 @@ private void EnsureEventLogInit(bool append = true)
 
     #region Log Members
 
+    /// <summary>
+    /// Delete log content and leave ready for next log message
+    /// </summary>
     public void Clear()
     {
         lock (_sync)
@@ -99,6 +102,9 @@ public void Clear()
             _messageLog?.Dispose();
             _eventLog?.Dispose();
 
+            _messageLog = null;
+            _eventLog = null;
+
             EnsureMessageLogInit(append: false);
             EnsureEventLogInit(append: false);
         }

RELEASE_NOTES.md

@@ -44,6 +44,7 @@ What's New
 * #951 - fix: restore Session disconnect during SocketInitiatorThread.Read exception (gbirchmeier/trevor-bush)
 * #963 - fix: concurrency bug with NonSessionLog on Windows (gbirchmeier)
 * #195 - fix: Acceptor.Stop() then restart now works (huwmjenkins/vyourtchenko)
+* #956 - fix: FileLog.Clear() broke in 1.13, now fixed again (hansw96/gbirchmeier)
 
 ### v1.13.1
 * backport #951 to 1.13

UnitTests/Logger/FileLogTests.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
 using NUnit.Framework;
 using QuickFix.Logger;
 
@@ -9,6 +10,13 @@ public class FileLogTests
 {
     private FileLog? _log;
 
+    private readonly QuickFix.SessionID _defaultSessionId = new ("FIX.4.2", "SENDERCOMP", "TARGETCOMP");
+    private readonly string _logDirectory = Path.Combine(TestContext.CurrentContext.TestDirectory, "log");
+    private readonly string _eventLogFilePath = Path.Combine(TestContext.CurrentContext.TestDirectory, "log",
+        "FIX.4.2-SENDERCOMP-TARGETCOMP.event.current.log");
+    private readonly string _messagesLogFilePath = Path.Combine(TestContext.CurrentContext.TestDirectory, "log",
+            "FIX.4.2-SENDERCOMP-TARGETCOMP.messages.current.log");
+
     [SetUp]
     public void Setup()
     { }
@@ -40,60 +48,83 @@ public void TestPrefixForSubsAndLocation()
         Assert.That(FileLog.Prefix(sessionIdWithSubsNoLocation), Is.EqualTo("FIX.4.2-SENDERCOMP_SENDERSUB-TARGETCOMP_TARGETSUB"));
     }
 
-    [Test]
-    public void TestGeneratedFileName()
+    private FileLogFactory CreateTestFactory()
     {
-        var logDirectory = Path.Combine(TestContext.CurrentContext.TestDirectory, "log");
+        if (Directory.Exists(_logDirectory))
+            Directory.Delete(_logDirectory, true);
 
-        if (Directory.Exists(logDirectory))
-            Directory.Delete(logDirectory, true);
-
-        QuickFix.SessionID sessionId = new QuickFix.SessionID("FIX.4.2", "SENDERCOMP", "TARGETCOMP");
         QuickFix.SessionSettings settings = new QuickFix.SessionSettings();
-
         QuickFix.SettingsDictionary config = new QuickFix.SettingsDictionary();
         config.SetString(QuickFix.SessionSettings.CONNECTION_TYPE, "initiator");
-        config.SetString(QuickFix.SessionSettings.FILE_LOG_PATH, logDirectory);
-
-        settings.Set(sessionId, config);
+        config.SetString(QuickFix.SessionSettings.FILE_LOG_PATH, _logDirectory);
 
-        string expectedEventLogFilePath = Path.Combine(logDirectory, "FIX.4.2-SENDERCOMP-TARGETCOMP.event.current.log");
-        string expectedMessagesLogFilePath = Path.Combine(logDirectory, "FIX.4.2-SENDERCOMP-TARGETCOMP.messages.current.log");
+        settings.Set(_defaultSessionId, config);
+        return new FileLogFactory(settings);
+    }
 
-        FileLogFactory factory = new FileLogFactory(settings);
-        _log = (FileLog)factory.Create(sessionId);
+    [Test]
+    public void TestGeneratedFileName()
+    {
+        FileLogFactory factory = CreateTestFactory();
+        _log = (FileLog)factory.Create(_defaultSessionId);
 
-        Assert.That(!File.Exists(expectedEventLogFilePath));
-        Assert.That(!File.Exists(expectedMessagesLogFilePath));
+        // Log files aren't created before first log statement
+        Assert.That(!File.Exists(_eventLogFilePath));
+        Assert.That(!File.Exists(_messagesLogFilePath));
 
         _log.OnEvent("some event");
 
-        Assert.That(File.Exists(expectedEventLogFilePath));
-        Assert.That(!File.Exists(expectedMessagesLogFilePath));
+        // The event file exists now
+        Assert.That(File.Exists(_eventLogFilePath));
+        Assert.That(!File.Exists(_messagesLogFilePath));
 
         _log.OnIncoming("some incoming");
         _log.OnOutgoing("some outgoing");
 
-        Assert.That(File.Exists(expectedEventLogFilePath));
-        Assert.That(File.Exists(expectedMessagesLogFilePath));
+        // The message file exists now
+        Assert.That(File.Exists(_eventLogFilePath));
+        Assert.That(File.Exists(_messagesLogFilePath));
 
         // cleanup (don't delete log unless success)
         _log.Dispose();
         _log = null;
-        Directory.Delete(logDirectory, true);
+        Directory.Delete(_logDirectory, true);
     }
 
     [Test]
     public void TestThrowsIfNoConfig()
     {
-        QuickFix.SessionID sessionId = new QuickFix.SessionID("FIX.4.2", "SENDERCOMP", "TARGETCOMP");
         QuickFix.SettingsDictionary config = new QuickFix.SettingsDictionary();
         config.SetString(QuickFix.SessionSettings.CONNECTION_TYPE, "initiator");
         QuickFix.SessionSettings settings = new QuickFix.SessionSettings();
-        settings.Set(sessionId, config);
+        settings.Set(_defaultSessionId, config);
 
         FileLogFactory factory = new FileLogFactory(settings);
+        var ex = Assert.Throws<QuickFix.ConfigError>(delegate { factory.Create(_defaultSessionId); });
+        Assert.That(ex!.Message, Does.Contain("Configuration failed: No value for key: FileLogPath"));
+    }
 
-        Assert.Throws<QuickFix.ConfigError>(delegate { factory.Create(sessionId); });
+    [Test]
+    public void TestClear()
+    {
+        FileLogFactory factory = CreateTestFactory();
+        _log = (FileLog)factory.Create(_defaultSessionId);
+
+        _log.OnEvent("some event");
+        _log.OnIncoming("some incoming");
+
+        // Clear() should delete the contents of the log files...
+        _log.Clear();
+        Assert.That(new FileInfo(_eventLogFilePath).Length, Is.EqualTo(0));
+        Assert.That(new FileInfo(_messagesLogFilePath).Length, Is.EqualTo(0));
+
+        // ... and leave them ready for more logging
+        Assert.DoesNotThrow(delegate { _log.OnEvent("another event after clear"); });
+        Assert.DoesNotThrow(delegate { _log.OnIncoming("another incoming after clear"); });
+
+        // cleanup (don't delete log unless success)
+        _log.Dispose();
+        _log = null;
+        Directory.Delete(_logDirectory, true);
     }
 }