Merge pull request #830 from gbirchmeier/xdebuglog

replace ClientThreadHandler "Debug" logs with NonSessionLog

QuickFIXn/AbstractInitiator.cs

@@ -21,12 +21,10 @@ public abstract class AbstractInitiator : IInitiator
  private readonly SessionFactory _sessionFactory;
  private Thread? _thread;
 
- #region Properties
+ protected readonly NonSessionLog _nonSessionLog;
 
  public bool IsStopped { get; private set; } = true;
 
- #endregion
-
  protected AbstractInitiator(
  IApplication app,
  IMessageStoreFactory storeFactory,
@@ -38,6 +36,7 @@ protected AbstractInitiator(
  var logFactory = logFactoryNullable ?? new NullLogFactory();
  var msgFactory = messageFactoryNullable ?? new DefaultMessageFactory();
  _sessionFactory = new SessionFactory(app, storeFactory, logFactory, msgFactory);
+ _nonSessionLog = new NonSessionLog(logFactory);
 
  HashSet<SessionID> definedSessions = _settings.GetSessions();
  if (0 == definedSessions.Count)

QuickFIXn/AcceptorSocketDescriptor.cs

@@ -1,32 +1,28 @@
 #nullable enable
 using System.Collections.Generic;
 using System.Net;
+using QuickFix.Logger;
 
 namespace QuickFix
 {
  internal class AcceptorSocketDescriptor
  {
- #region Properties
-
  public ThreadedSocketReactor SocketReactor { get; }
-
  public IPEndPoint Address { get; }
 
- #endregion
-
- #region Private Members
-
  private readonly Dictionary<SessionID, Session> _acceptedSessions = new ();
 
- #endregion
-
- public AcceptorSocketDescriptor(IPEndPoint socketEndPoint, SocketSettings socketSettings, QuickFix.SettingsDictionary sessionDict)
+ public AcceptorSocketDescriptor(
+ IPEndPoint socketEndPoint,
+ SocketSettings socketSettings,
+ SettingsDictionary sessionDict,
+ NonSessionLog nonSessionLog)
  {
  Address = socketEndPoint;
- SocketReactor = new ThreadedSocketReactor(Address, socketSettings, sessionDict, this);
+ SocketReactor = new ThreadedSocketReactor(Address, socketSettings, sessionDict, this, nonSessionLog);
  }
 
- public void AcceptSession(Session session)
+ internal void AcceptSession(Session session)
  {
  lock (_acceptedSessions)
  {
@@ -39,15 +35,15 @@ public void AcceptSession(Session session)
  /// </summary>
  /// <param name="sessionId">ID of session to be removed</param>
  /// <returns>true if session removed, false if not found</returns>
- public bool RemoveSession(SessionID sessionId)
+ internal bool RemoveSession(SessionID sessionId)
  {
  lock (_acceptedSessions)
  {
  return _acceptedSessions.Remove(sessionId);
  }
  }
 
- public Dictionary<SessionID, Session> GetAcceptedSessions()
+ internal Dictionary<SessionID, Session> GetAcceptedSessions()
  {
  lock (_acceptedSessions)
  {

QuickFIXn/ClientHandlerThread.cs

@@ -31,23 +31,16 @@ public ExitedEventArgs(ClientHandlerThread clientHandlerThread)
  private Thread? _thread = null;
  private volatile bool _isShutdownRequested = false;
  private readonly SocketReader _socketReader;
- private readonly FileLog _log;
-
- internal ClientHandlerThread(TcpClient tcpClient, long clientId, QuickFix.SettingsDictionary settingsDict,
- SocketSettings socketSettings, AcceptorSocketDescriptor? acceptorDescriptor)
- {
- string debugLogFilePath = "log";
- if (settingsDict.Has(SessionSettings.DEBUG_FILE_LOG_PATH))
- debugLogFilePath = settingsDict.GetString(SessionSettings.DEBUG_FILE_LOG_PATH);
- else if (settingsDict.Has(SessionSettings.FILE_LOG_PATH))
- debugLogFilePath = settingsDict.GetString(SessionSettings.FILE_LOG_PATH);
-
- // FIXME - do something more flexible than hardcoding a filelog
- _log = new FileLog(debugLogFilePath, new SessionID(
- "ClientHandlerThread", clientId.ToString(), "Debug-" + Guid.NewGuid()));
 
+ internal ClientHandlerThread(
+ TcpClient tcpClient,
+ long clientId,
+ SocketSettings socketSettings,
+ AcceptorSocketDescriptor? acceptorDescriptor,
+ NonSessionLog nonSessionLog
+ ) {
  Id = clientId;
- _socketReader = new SocketReader(tcpClient, socketSettings, this, acceptorDescriptor);
+ _socketReader = new SocketReader(tcpClient, socketSettings, this, acceptorDescriptor, nonSessionLog);
  }
 
  public void Start()
@@ -58,7 +51,7 @@ public void Start()
 
  public void Shutdown(string reason)
  {
- Log("shutdown requested: " + reason);
+ // TODO - need the reason param?
  _isShutdownRequested = true;
  }
 
@@ -85,29 +78,13 @@ private void Run()
  }
  }
 
- Log("shutdown");
  OnExited();
  }
 
  private void OnExited() {
  Exited?.Invoke(this, new ExitedEventArgs(this));
  }
 
- /// FIXME do real logging
- public void Log(string s)
- {
- _log.OnEvent(s);
- }
-
- /// <summary>
- /// Provide StreamReader with access to the log
- /// </summary>
- /// <returns></returns>
- internal ILog GetLog()
- {
- return _log;
- }
-
  #region Responder Members
 
  public bool Send(string data)
@@ -136,7 +113,6 @@ protected virtual void Dispose(bool disposing)
  if (disposing)
  {
  _socketReader.Dispose();
- _log.Dispose();
  }
  _disposed = true;
  }

QuickFIXn/IInitiator.cs

@@ -48,7 +48,7 @@ public interface IInitiator : IDisposable
  /// <param name="sessionID">ID of session to be added</param>
  /// <param name="dict">session settings</param>
  /// <returns>true if session added successfully, false if session already exists or is not an initiator</returns>
- bool AddSession(SessionID sessionID, QuickFix.SettingsDictionary dict);
+ bool AddSession(SessionID sessionID, SettingsDictionary dict);
 
  /// <summary>
  /// Remove an existing session after initiator has been started

QuickFIXn/Logger/CompositeLogFactory.cs

@@ -4,7 +4,8 @@
 namespace QuickFix.Logger;
 
 /// <summary>
-/// Allows multiple log factories to be used with QuickFIX/N. For example, you could log events to the console and also log all events and messages to a file.
+/// Allows multiple log factories to be used with QuickFIX/N.
+/// For example, you could log events to the console and also log all events and messages to a file.
 /// </summary>
 public class CompositeLogFactory : ILogFactory
 {
@@ -24,4 +25,8 @@ public ILog Create(SessionID sessionID)
  {
  return new CompositeLog(_factories.Select(f => f.Create(sessionID)).ToArray());
  }
+
+ public ILog CreateNonSessionLog() {
+ return new CompositeLog(_factories.Select(f => f.Create(new SessionID("Non", "Session", "Log"))).ToArray());
+ }
 }

QuickFIXn/Logger/FileLogFactory.cs

@@ -9,22 +9,24 @@ public class FileLogFactory : ILogFactory
 {
  private readonly SessionSettings _settings;
 
- #region LogFactory Members
-
  public FileLogFactory(SessionSettings settings)
  {
  _settings = settings;
  }
 
  /// <summary>
- /// Creates a file-based message store
+ /// Creates a file-based message log
  /// </summary>
- /// <param name="sessionId">session ID for the message store</param>
+ /// <param name="sessionId">session ID for the message log</param>
  /// <returns></returns>
  public ILog Create(SessionID sessionId)
  {
  return new FileLog(_settings.Get(sessionId).GetString(SessionSettings.FILE_LOG_PATH), sessionId);
  }
 
- #endregion
+ public ILog CreateNonSessionLog() {
+ return new FileLog(
+ _settings.Get().GetString(SessionSettings.FILE_LOG_PATH),
+ new SessionID("Non", "Session", "Log"));
+ }
 }

QuickFIXn/Logger/ILogFactory.cs

@@ -3,14 +3,25 @@
 namespace QuickFix.Logger;
 
 /// <summary>
-/// Used by a session to create a log implementation
+/// Creates a log instance
 /// </summary>
 public interface ILogFactory
 {
  /// <summary>
- /// Create a log implementation
+ /// Create a log instance for a session
  /// </summary>
  /// <param name="sessionId">session ID usually used for configuration access</param>
  /// <returns></returns>
  ILog Create(SessionID sessionId);
+
+ /// <summary>
+ /// Create a log instance that is not tied to a session.
+ /// This log will
+ /// (1) only be used for messages that cannot be linked to a session
+ /// (2) only have its OnEvent() method called
+ /// (3) only be created when a message is logged (to avoid empty log files)
+ /// Messages are written to this log only on rare occasions. It's possible you may never see it created.
+ /// </summary>
+ /// <returns></returns>
+ ILog CreateNonSessionLog();
 }

QuickFIXn/Logger/NonSessionLog.cs

@@ -0,0 +1,28 @@
+#nullable enable
+using System;
+
+namespace QuickFix.Logger;
+
+/// <summary>
+/// A logger that can be used when the calling logic cannot identify a session (which is rare).
+/// Does not create a file until first write.
+/// </summary>
+public class NonSessionLog {
+
+ private readonly ILogFactory _logFactory;
+ private ILog? _log;
+
+ private readonly object _sync = new();
+
+ internal NonSessionLog(ILogFactory logFactory) {
+ _logFactory = logFactory;
+ }
+
+ internal void OnEvent(string s) {
+ lock (_sync) {
+ _log ??= _logFactory.CreateNonSessionLog();
+ }
+ _log.OnEvent(s);
+ }
+}
+

QuickFIXn/Logger/NullLogFactory.cs

@@ -10,4 +10,9 @@ public ILog Create(SessionID _x)
  {
  return new NullLog();
  }
+
+ public ILog CreateNonSessionLog()
+ {
+ return new NullLog();
+ }
 }

QuickFIXn/Logger/ScreenLogFactory.cs

@@ -28,8 +28,6 @@ public ScreenLogFactory(bool logIncoming, bool logOutgoing, bool logEvent)
  _settings = new SessionSettings();
  }
 
- #region LogFactory Members
-
  public ILog Create(SessionID sessionId) {
  bool logIncoming = _logIncoming;
  bool logOutgoing = _logOutgoing;
@@ -47,5 +45,7 @@ public ILog Create(SessionID sessionId) {
  return new ScreenLog(logIncoming, logOutgoing, logEvent);
  }
 
- #endregion
+ public ILog CreateNonSessionLog() {
+ return new ScreenLog(true, true, true);
+ }
 }

QuickFIXn/Session.cs

@@ -396,7 +396,7 @@ public void Disconnect(string reason)
  }
  else
  {
- Log.OnEvent("Session {SessionID} already disconnected: {reason}");
+ Log.OnEvent($"Session {SessionID} already disconnected: {reason}");
  }
 
  if (_state.ReceivedLogon || _state.SentLogon)

QuickFIXn/SessionFactory.cs

@@ -36,7 +36,7 @@ public SessionFactory(
  _messageFactory = messageFactory ?? new DefaultMessageFactory();
  }
 
- private static bool DetectIfInitiator(QuickFix.SettingsDictionary settings)
+ private static bool DetectIfInitiator(SettingsDictionary settings)
  {
  switch (settings.GetString(SessionSettings.CONNECTION_TYPE))
  {
@@ -46,7 +46,7 @@ private static bool DetectIfInitiator(QuickFix.SettingsDictionary settings)
  throw new ConfigError("Invalid ConnectionType");
  }
 
- public Session Create(SessionID sessionId, QuickFix.SettingsDictionary settings)
+ public Session Create(SessionID sessionId, SettingsDictionary settings)
  {
  bool isInitiator = SessionFactory.DetectIfInitiator(settings);
 
@@ -161,7 +161,7 @@ public Session Create(SessionID sessionId, QuickFix.SettingsDictionary settings)
  return session;
  }
 
- protected DataDictionary.DataDictionary CreateDataDictionary(SessionID sessionId, QuickFix.SettingsDictionary settings, string settingsKey, string beginString)
+ protected DataDictionary.DataDictionary CreateDataDictionary(SessionID sessionId, SettingsDictionary settings, string settingsKey, string beginString)
  {
  string path;
  if (settings.Has(settingsKey))

QuickFIXn/SessionID.cs

@@ -11,20 +11,12 @@ namespace QuickFix
  /// </summary>
  public class SessionID
  {
- #region Properties
-
  public string BeginString { get; }
-
  public string SenderCompID { get; }
-
  public string SenderSubID { get; }
-
  public string SenderLocationID { get; }
-
  public string TargetCompID { get; }
-
  public string TargetSubID { get; }
-
  public string TargetLocationID { get; }
 
  /// <summary>
@@ -39,17 +31,11 @@ public class SessionID
  /// </summary>
  public bool IsFIXT { get; }
 
- #endregion
-
- #region Public Members
+ // TODO just make the values nullable, jeez
  public const string NOT_SET = "";
- #endregion
 
- #region Private Members
  private readonly string _id;
 
- #endregion
-
  public SessionID(string beginString, string senderCompId, string senderSubId, string senderLocationId, string targetCompId, string targetSubId, string targetLocationId, string? sessionQualifier = NOT_SET)
  {
  BeginString = beginString ?? throw new ArgumentNullException(nameof(beginString));