From d635d2426ee78e958d23e9dcb7f844c688d4c292 Mon Sep 17 00:00:00 2001 From: Grant Birchmeier Date: Mon, 16 Sep 2024 17:44:45 -0500 Subject: [PATCH] Make Message.ToString() not change object state issue #883 New function ConstructString now does the BodyLength/CheckSum update that ToString used to do --- Examples/JsonToFix/Program.cs | 2 +- Examples/TradeClient/TradeClientApp.cs | 4 +- QuickFIXn/Message/FieldMap.cs | 10 +++ QuickFIXn/Message/Group.cs | 4 + QuickFIXn/Message/Header.cs | 10 +++ QuickFIXn/Message/Message.cs | 21 ++++- QuickFIXn/Message/Trailer.cs | 10 +++ QuickFIXn/Session.cs | 6 +- RELEASE_NOTES.md | 2 + UnitTests/DataDictionaryTests.cs | 2 +- UnitTests/MessageTests.cs | 108 +++++++++++-------------- UnitTests/SessionDynamicTest.cs | 2 +- UnitTests/SessionTest.cs | 18 ++--- 13 files changed, 119 insertions(+), 80 deletions(-) diff --git a/Examples/JsonToFix/Program.cs b/Examples/JsonToFix/Program.cs index 38655445f..11775be98 100644 --- a/Examples/JsonToFix/Program.cs +++ b/Examples/JsonToFix/Program.cs @@ -11,7 +11,7 @@ static void JsonMsgToFix(string json, QuickFix.DataDictionary.DataDictionary ses { var msg = new Message(); msg.FromJson(json, true, sessionDataDictionary, appDataDictionary, msgFactory); - Console.WriteLine(msg.ToString()); + Console.WriteLine(msg.ConstructString()); } static void JsonToFix(string fname, QuickFix.DataDictionary.DataDictionary sessionDataDictionary, QuickFix.DataDictionary.DataDictionary appDataDictionary) diff --git a/Examples/TradeClient/TradeClientApp.cs b/Examples/TradeClient/TradeClientApp.cs index 8f1a3a4d0..5b9ffa50c 100644 --- a/Examples/TradeClient/TradeClientApp.cs +++ b/Examples/TradeClient/TradeClientApp.cs @@ -27,7 +27,7 @@ public void ToAdmin(Message message, SessionID sessionID) { } public void FromApp(Message message, SessionID sessionID) { - Console.WriteLine("IN: " + message.ToString()); + Console.WriteLine("IN: " + message.ConstructString()); try { Crack(message, sessionID); @@ -57,7 +57,7 @@ public void ToApp(Message message, SessionID sessionID) { } Console.WriteLine(); - Console.WriteLine("OUT: " + message.ToString()); + Console.WriteLine("OUT: " + message.ConstructString()); } #endregion diff --git a/QuickFIXn/Message/FieldMap.cs b/QuickFIXn/Message/FieldMap.cs index 6c286d8e3..12cc75cc9 100644 --- a/QuickFIXn/Message/FieldMap.cs +++ b/QuickFIXn/Message/FieldMap.cs @@ -579,11 +579,21 @@ public int CalculateLength() return total; } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// public virtual string CalculateString() { return CalculateString(new StringBuilder(), FieldOrder); } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// + /// + /// public virtual string CalculateString(StringBuilder sb, int[] preFields) { HashSet groupCounterTags = new HashSet(_groups.Keys); diff --git a/QuickFIXn/Message/Group.cs b/QuickFIXn/Message/Group.cs index 16e71d351..3e7387ed7 100644 --- a/QuickFIXn/Message/Group.cs +++ b/QuickFIXn/Message/Group.cs @@ -63,6 +63,10 @@ public virtual Group Clone() /// public int Delim { get; } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// public override string CalculateString() { return base.CalculateString(new StringBuilder(), FieldOrder ?? new[] { Delim }); } diff --git a/QuickFIXn/Message/Header.cs b/QuickFIXn/Message/Header.cs index ebfe308fc..5df0661f8 100644 --- a/QuickFIXn/Message/Header.cs +++ b/QuickFIXn/Message/Header.cs @@ -15,10 +15,20 @@ public Header(Header src) : base(src) { } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// public override string CalculateString() { return base.CalculateString(new StringBuilder(), HEADER_FIELD_ORDER); } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// + /// + /// public override string CalculateString(StringBuilder sb, int[] preFields) { return base.CalculateString(sb, HEADER_FIELD_ORDER); } diff --git a/QuickFIXn/Message/Message.cs b/QuickFIXn/Message/Message.cs index 5d22087b0..3112afdf8 100644 --- a/QuickFIXn/Message/Message.cs +++ b/QuickFIXn/Message/Message.cs @@ -793,10 +793,15 @@ public SessionID GetSessionID(Message m) m.Header.GetString(Tags.TargetCompID)); } - private Object lock_ToString = new Object(); - public override string ToString() + private Object lock_ConstructString = new Object(); + /// + /// Update BodyLength in Header, update CheckSum in Trailer, and return a FIX string. + /// (This function changes the object state!) + /// + /// + public string ConstructString() { - lock (lock_ToString) + lock (lock_ConstructString) { Header.SetField(new BodyLength(BodyLength()), true); Trailer.SetField(new CheckSum(Fields.Converters.CheckSumConverter.Convert(CheckSum())), true); @@ -805,6 +810,16 @@ public override string ToString() } } + /// + /// Create a FIX-style string from the message. + /// This does NOT add/update BodyLength or CheckSum to the message header, or otherwise change the object state. + /// (Use to compute and add/update BodyLength & CheckSum.) + /// + /// + public override string ToString() { + return Header.CalculateString() + CalculateString() + Trailer.CalculateString(); + } + protected int BodyLength() { return Header.CalculateLength() + CalculateLength() + Trailer.CalculateLength(); diff --git a/QuickFIXn/Message/Trailer.cs b/QuickFIXn/Message/Trailer.cs index 4db07dcea..d4fadfb6f 100644 --- a/QuickFIXn/Message/Trailer.cs +++ b/QuickFIXn/Message/Trailer.cs @@ -15,10 +15,20 @@ public Trailer(Trailer src) : base(src) { } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// public override string CalculateString() { return base.CalculateString(new StringBuilder(), TRAILER_FIELD_ORDER); } + /// + /// Creates a FIX (ish) string representation of this FieldMap (does not change the object state) + /// + /// + /// + /// public override string CalculateString(StringBuilder sb, int[] preFields) { return base.CalculateString(sb, TRAILER_FIELD_ORDER); } diff --git a/QuickFIXn/Session.cs b/QuickFIXn/Session.cs index a359ea31c..53cd34765 100755 --- a/QuickFIXn/Session.cs +++ b/QuickFIXn/Session.cs @@ -789,7 +789,7 @@ protected void NextResendRequest(Message resendReq) { GenerateSequenceReset(resendReq, begin, msgSeqNum); } - Send(msg.ToString()); + Send(msg.ConstructString()); begin = 0; } current = msgSeqNum + 1; @@ -1517,7 +1517,7 @@ protected bool NextQueued(SeqNumType num) } else { - NextMessage(msg.ToString()); + NextMessage(msg.ConstructString()); } return true; } @@ -1567,7 +1567,7 @@ protected bool SendRaw(Message message, SeqNumType seqNum) } } - string messageString = message.ToString(); + string messageString = message.ConstructString(); if (0 == seqNum) Persist(message, messageString); return Send(messageString); diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 7bcd9f4b2..19239dd20 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -22,6 +22,8 @@ What's New * #878 - corrections to tag 45 "Side" in various DDs (gbirchmeier) - most people won't notice, easy fix if they do * fix typo in FIX50 and FIX50SP1: `CROSS_SHORT_EXXMPT` fixed to `CROSS_SHORT_EXEMPT` * correction in FIX41 and FIX42: `D` to `UNDISCLOSED` +* #863 - Change Message.ToString() to not alter object state anymore. (gbirchmeier) + Use new function Message.ConstructString() if you need BodyLength/CheckSum to be updated. **Non-breaking changes** * #864 - when multiple threads race to init DefaultMessageFactory, diff --git a/UnitTests/DataDictionaryTests.cs b/UnitTests/DataDictionaryTests.cs index 40446c5b6..1c013feb8 100644 --- a/UnitTests/DataDictionaryTests.cs +++ b/UnitTests/DataDictionaryTests.cs @@ -318,7 +318,7 @@ public void CheckGroupCountTest() //verify that FromString didn't correct the counter //HEY YOU, READ THIS NOW: if these fail, first check if MessageTests::FromString_DoNotCorrectCounter() passes Assert.AreEqual("386=3", n.NoTradingSessions.toStringField()); - StringAssert.Contains("386=3", n.ToString()); + StringAssert.Contains("386=3", n.ConstructString()); Assert.Throws(delegate { dd.CheckGroupCount(n.NoTradingSessions, n, "D"); }); } diff --git a/UnitTests/MessageTests.cs b/UnitTests/MessageTests.cs index 315f75ae8..34bc41241 100644 --- a/UnitTests/MessageTests.cs +++ b/UnitTests/MessageTests.cs @@ -5,6 +5,7 @@ using QuickFix; using QuickFix.Fields; using UnitTests.TestHelpers; +using Message = QuickFix.Message; namespace UnitTests { @@ -185,7 +186,7 @@ public void FromString_DoNotCorrectCounter() n.FromString(s, true, dd, dd, _defaultMsgFactory); Assert.AreEqual("386=3", n.NoTradingSessions.toStringField()); - StringAssert.Contains("386=3", n.ToString()); //should not be "corrected" to 2! + StringAssert.Contains("386=3", n.ConstructString()); //should not be "corrected" to 2! } [Test] @@ -209,52 +210,7 @@ public void FromString_GroupDelimiterIssue() } [Test] - public void ToStringTest() - { - string str1 = "8=FIX.4.2\x01" + "9=55\x01" + "35=0\x01" + "34=3\x01" + "49=TW\x01" + - "52=20000426-12:05:06\x01" + "56=ISLD\x01" + "1=acct123\x01" + "10=123\x01"; - Message msg = new Message(); - try - { - msg.FromString(str1, true, null, null, _defaultMsgFactory); - } - catch (InvalidMessage e) - { - Assert.Fail("Unexpected exception (InvalidMessage): " + e.Message); - } - StringField f1 = new StringField(8); - StringField f2 = new StringField(9); - StringField f3 = new StringField(35); - StringField f4 = new StringField(34); - StringField f5 = new StringField(49); - StringField f6 = new StringField(52); - StringField f7 = new StringField(56); - StringField f8 = new StringField(10); - StringField f9 = new StringField(1); - msg.Header.GetField(f1); - msg.Header.GetField(f2); - msg.Header.GetField(f3); - msg.Header.GetField(f4); - msg.Header.GetField(f5); - msg.Header.GetField(f6); - msg.Header.GetField(f7); - msg.GetField(f9); - msg.Trailer.GetField(f8); - Assert.That(f1.Obj, Is.EqualTo("FIX.4.2")); - Assert.That(f2.Obj, Is.EqualTo("55")); - Assert.That(f3.Obj, Is.EqualTo("0")); - Assert.That(f4.Obj, Is.EqualTo("3")); - Assert.That(f5.Obj, Is.EqualTo("TW")); - Assert.That(f6.Obj, Is.EqualTo("20000426-12:05:06")); - Assert.That(f7.Obj, Is.EqualTo("ISLD")); - Assert.That(f8.Obj, Is.EqualTo("123")); - Assert.That(f9.Obj, Is.EqualTo("acct123")); - string raw = msg.ToString(); - Assert.That(raw, Is.EqualTo(str1)); - } - - [Test] - public void ToStringFieldOrder() + public void ConstructStringFieldOrder() { Message msg = new Message(); msg.Header.SetField(new QuickFix.Fields.MsgType("A")); @@ -263,7 +219,7 @@ public void ToStringFieldOrder() msg.Header.SetField(new QuickFix.Fields.TargetCompID("TARGET")); msg.Header.SetField(new QuickFix.Fields.MsgSeqNum(42)); string expect = "8=FIX.4.2\x01" + "9=31\x01" + "35=A\x01" + "34=42\x01" + "49=SENDER\x01" + "56=TARGET\x01" + "10=200\x01"; - Assert.That(msg.ToString(), Is.EqualTo(expect)); + Assert.That(msg.ConstructString(), Is.EqualTo(expect)); } [Test] @@ -525,7 +481,7 @@ public void RepeatingGroup() group2.EncodedText = new QuickFix.Fields.EncodedText("bbb"); news.AddGroup(group2); - string raw = news.ToString(); + string raw = news.ConstructString(); string nul = "\x01"; StringAssert.Contains( @@ -547,7 +503,7 @@ public void RepeatingGroup_ReuseObject() group.Text = new QuickFix.Fields.Text("line2"); news.AddGroup(group); - string raw = news.ToString(); + string raw = news.ConstructString(); string nul = "\x01"; StringAssert.Contains( @@ -676,7 +632,7 @@ public void RepeatingGroup_DelimiterFieldFirst() symGroup.SecurityID = new SecurityID("secid2"); msg.AddGroup(symGroup); - string msgString = msg.ToString(); + string msgString = msg.ConstructString(); string expected = String.Join(Message.SOH, new string[] { "146=2", "55=FOO1", "48=secid1", "55=FOO2", "48=secid2" }); StringAssert.Contains(expected, msgString); @@ -710,7 +666,7 @@ public void RepeatingGroup_FieldOrder() symGroup.SecurityIDSource = new SecurityIDSource("src2"); msg.AddGroup(symGroup); - string msgString = msg.ToString(); + string msgString = msg.ConstructString(); string expected = String.Join(Message.SOH, new string[] { "146=2", "55=FOO1", "65=sfx1", "48=secid1", "22=src1", "55=FOO2", "65=sfx2", "48=secid2", "22=src2", @@ -720,12 +676,12 @@ public void RepeatingGroup_FieldOrder() } [Test] - public void ToString_FIX50() + public void ConstructString_FIX50() { QuickFix.FIX50.News msg = new QuickFix.FIX50.News(); msg.Headline = new Headline("FOO"); - StringAssert.StartsWith("8=FIXT.1.1" + Message.SOH, msg.ToString()); + StringAssert.StartsWith("8=FIXT.1.1" + Message.SOH, msg.ConstructString()); } [Test] @@ -753,7 +709,7 @@ public void RepeatingGroup_SubgroupCounterTagAppearsOnlyOnce() ci.AddGroup(noParty); - string msgString = ci.ToString(); + string msgString = ci.ConstructString(); string expected = String.Join(Message.SOH, new string[] { "909=CollateralInquiry", // top-level fields (non-header) "453=1", //NoPartyIDs @@ -896,7 +852,7 @@ public void SendDateOnlyTimeOnlyConvertProblem() grp.QuoteEntryID = new QuoteEntryID("15467902"); msg.AddGroup(grp); - string msgString = msg.ToString(); + string msgString = msg.ConstructString(); string expected = String.Join(Message.SOH, new string[] { "35=W", "22=4", "48=BE0932900518", "55=[N/A]", "262=1b145288-9c9a-4911-a084-7341c69d3e6b", "762=EURO_EUR", "268=2", "269=0", "270=97.625", "15=EUR", "271=1246000", "272=20121024", "273=07:30:47", "276=I", "282=BEARGB21XXX", "299=15478575", @@ -971,13 +927,13 @@ public void issue95() msg.FromString(msgStr, false, dd, dd, _defaultMsgFactory); // make sure no fields were dropped in parsing - Assert.AreEqual(msgStr.Length, msg.ToString().Length); + Assert.AreEqual(msgStr.Length, msg.ConstructString().Length); // make sure repeating groups are not rearranged // 1 level deep - StringAssert.Contains(String.Join(Message.SOH, new string[] { "55=ABC", "65=CD", "48=securityid", "22=1" }), msg.ToString()); + StringAssert.Contains(String.Join(Message.SOH, new string[] { "55=ABC", "65=CD", "48=securityid", "22=1" }), msg.ConstructString()); // 2 levels deep - StringAssert.Contains(String.Join(Message.SOH, new string[] { "311=underlyingsymbol", "312=WI", "309=underlyingsecurityid", "305=1" }), msg.ToString()); + StringAssert.Contains(String.Join(Message.SOH, new string[] { "311=underlyingsymbol", "312=WI", "309=underlyingsecurityid", "305=1" }), msg.ConstructString()); } [Test] @@ -1009,7 +965,7 @@ public void ChecksumIsLastFieldOfTrailer() msg.Trailer.SetField(new Signature("woot")); msg.Trailer.SetField(new SignatureLength(4)); - string foo = msg.ToString().Replace(Message.SOH, '|'); + string foo = msg.ConstructString().Replace(Message.SOH, '|'); StringAssert.EndsWith("|10=099|", foo); } @@ -1101,5 +1057,37 @@ public void JsonNestedRepeatingGroupParseGroupTest() Assert.That(noPartySubGrp.GetString(Tags.PartySubIDType), Is.EqualTo("4")); Assert.That(noPartySubGrp.GetString(31338), Is.EqualTo("custom group field")); } + + private QuickFix.FIX44.News CreateStringResultInput() { + QuickFix.FIX44.News msg = new(); + msg.Header.SetField(new BeginString(QuickFix.FixValues.BeginString.FIX44)); + msg.Header.SetField(new MsgType("B")); + msg.SetField(new Headline("myHeadline")); + + QuickFix.FIX44.News.LinesOfTextGroup grp1 = new() { Text = new Text("line1") }; + QuickFix.FIX44.News.LinesOfTextGroup grp2 = new() { Text = new Text("line2") }; + msg.AddGroup(grp1); + msg.AddGroup(grp2); + return msg; + } + + [Test] + public void ToStringTest() { + QuickFix.FIX44.News msg = CreateStringResultInput(); + // ToString() does not add BodyLength or CheckSum -- it does not change object state + string expected = "8=FIX.4.4|35=B|148=myHeadline|33=2|58=line1|58=line2|"; + Assert.AreEqual(expected, msg.ToString().Replace(Message.SOH, '|')); + } + + [Test] + public void ConstructStringTest() { + QuickFix.FIX44.News msg = CreateStringResultInput(); + // ConstructString() adds BodyLength and CheckSum + string expected = "8=FIX.4.4|9=43|35=B|148=myHeadline|33=2|58=line1|58=line2|10=161|"; + Assert.AreEqual(expected, msg.ConstructString().Replace(Message.SOH, '|')); + + // the object state is changed + Assert.AreEqual(expected, msg.ToString().Replace(Message.SOH, '|')); + } } } diff --git a/UnitTests/SessionDynamicTest.cs b/UnitTests/SessionDynamicTest.cs index 5953560ad..1c405a194 100644 --- a/UnitTests/SessionDynamicTest.cs +++ b/UnitTests/SessionDynamicTest.cs @@ -348,7 +348,7 @@ void SendLogon(Socket s, string senderCompID) msg.Header.SetField(new QuickFix.Fields.SendingTime(System.DateTime.UtcNow)); msg.SetField(new QuickFix.Fields.HeartBtInt(300)); // Simple logon message - s.Send(CharEncoding.DefaultEncoding.GetBytes(msg.ToString())); + s.Send(CharEncoding.DefaultEncoding.GetBytes(msg.ConstructString())); } void ClearLogs() diff --git a/UnitTests/SessionTest.cs b/UnitTests/SessionTest.cs index cda66f587..d40383b8a 100755 --- a/UnitTests/SessionTest.cs +++ b/UnitTests/SessionTest.cs @@ -64,7 +64,7 @@ public void DumpMsgLookup() Console.WriteLine(String.Format(" {0}: count {1}", key, msgLookup[key].Count)); foreach (QuickFix.Message m in msgLookup[key]) { - Console.WriteLine(" - " + m.ToString()); + Console.WriteLine(" - " + m.ConstructString()); } } } @@ -230,7 +230,7 @@ private void SendLogon(QuickFix.Message msg) msg.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++)); msg.Header.SetField(new QuickFix.Fields.SendingTime(System.DateTime.UtcNow)); msg.SetField(new QuickFix.Fields.HeartBtInt(1)); - session.Next(msg.ToString()); + session.Next(msg.ConstructString()); } public bool SENT_SEQUENCE_RESET() @@ -347,7 +347,7 @@ public void SendNOSMessage() order.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID)); order.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++)); - session.Next(order.ToString()); + session.Next(order.ConstructString()); } public void SendResendRequest(SeqNumType begin, SeqNumType end) @@ -370,7 +370,7 @@ private void SendTheMessage(QuickFix.Message msg) msg.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID)); msg.Header.SetField(new QuickFix.Fields.MsgSeqNum(seqNum++)); - session.Next(msg.ToString()); + session.Next(msg.ConstructString()); } [Test] @@ -721,10 +721,10 @@ public void TestMaxMessagesInResendRequest() reset.Header.SetField(new QuickFix.Fields.MsgSeqNum(2)); reset.SetField(new QuickFix.Fields.NewSeqNo(2501)); - session.Next(reset.ToString()); + session.Next(reset.ConstructString()); order.Header.SetField(new QuickFix.Fields.MsgSeqNum(2501)); - session.Next(order.ToString()); + session.Next(order.ConstructString()); // Should have triggered next resend (2502->5001), check this //Console.WriteLine(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count); @@ -736,10 +736,10 @@ public void TestMaxMessagesInResendRequest() // Jump forward to the end of the resend chunk with a fillgap reset message reset.Header.SetField(new QuickFix.Fields.MsgSeqNum(2502)); reset.SetField(new QuickFix.Fields.NewSeqNo(5001)); - session.Next(reset.ToString()); + session.Next(reset.ConstructString()); order.Header.SetField(new QuickFix.Fields.MsgSeqNum(5001)); - session.Next(order.ToString()); // Triggers next resend (5002->5005) + session.Next(order.ConstructString()); // Triggers next resend (5002->5005) //Console.WriteLine(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count); Assert.That(responder.msgLookup[QuickFix.Fields.MsgType.RESENDREQUEST].Count == 1); @@ -939,7 +939,7 @@ public void TestApplicationExtension() order.Header.SetField(new QuickFix.Fields.SenderCompID(sessionID.TargetCompID)); order.Header.SetField(new QuickFix.Fields.MsgSeqNum(2)); - session.Next(order.ToString()); + session.Next(order.ConstructString()); Assert.That(mockApp.InterceptedMessageTypes.Count, Is.EqualTo(2)); Assert.True(mockApp.InterceptedMessageTypes.Contains(QuickFix.Fields.MsgType.LOGON));