connamara / quickfixn

QuickFIX/n implements the FIX protocol on .NET.
http://quickfixn.org
Other
469 stars 557 forks source link

MessageFactory group-creator can't handle DDs where a group-counter tag appears in two different places #495

Open gbirchmeier opened 5 years ago

gbirchmeier commented 5 years ago

Ran into this while generating source for TT's DD.

The DD contains the NoSides tag in two different places (it denotes a top-level group, and a nested group inside of a different group).

In their DD:

        <message name='TradeCaptureReport' msgcat='app' msgtype='AE'>
            ...
-->         <group name='NoSides' required='N'>
                <group name='NoPartyIDs' required='N'>
                    <field name='PartyID' required='N' />
                    <field name='PartyRole' required='N' />
                    <field name='PartyRoleQualifier' required='N' />
                    <field name='PartyIDSource' required='N' />
                </group>
                <field name='OrderID' required='N' />
                <field name='OrderIDGUID' required='N' />
                <field name='Account' required='N' />
                <field name='Side' required='N' />
                <field name='AllocQty' required='N' />
                <field name='AllocPositionEffect' required='N' />
            </group>
            <group name='NoTCRLegs' required='N'>
                <field name='LegLastPx' required='N' />
                <field name='LegLastQty' required='N' />
-->             <group name='NoSides' required='N'>
                    <field name='OrderID' required='N' />
                    <field name='OrderIDGUID' required='N' />
                    <field name='Account' required='N' />
                    <field name='Side' required='N' />
                    <field name='AllocQty' required='N' />
                    <field name='AllocPositionEffect' required='N' />
                </group>
            </group>

That's totally legit as far as I know. However, the generator generates this (in Messages\FIX44\MessageFactory.cs):

            public Group Create(string beginString, string msgType, int correspondingFieldID)
            {
                ...
                if (QuickFix.FIX44.TradeCaptureReport.MsgType.Equals(msgType))
                {
                    switch (correspondingFieldID)
                    {
                        case QuickFix.Fields.Tags.NoSecurityAltID: return new QuickFix.FIX44.TradeCaptureReport.NoSecurityAltIDGroup();
                        case QuickFix.Fields.Tags.NoLegs: return new QuickFix.FIX44.TradeCaptureReport.NoLegsGroup();
                        case QuickFix.Fields.Tags.NoLegSecurityAltID: return new QuickFix.FIX44.TradeCaptureReport.NoLegsGroup.NoLegSecurityAltIDGroup();
-->                     case QuickFix.Fields.Tags.NoSides: return new QuickFix.FIX44.TradeCaptureReport.NoSidesGroup();
                        case QuickFix.Fields.Tags.NoPartyIDs: return new QuickFix.FIX44.TradeCaptureReport.NoSidesGroup.NoPartyIDsGroup();
                        case QuickFix.Fields.Tags.NoTCRLegs: return new QuickFix.FIX44.TradeCaptureReport.NoTCRLegsGroup();
-->                     case QuickFix.Fields.Tags.NoSides: return new QuickFix.FIX44.TradeCaptureReport.NoTCRLegsGroup.NoSidesGroup();
                    }
                }

That function's interface is defined in QuickFix\IMessageFactory.cs. Obviously it's a mistake to think that msgtype and group-counter-tag alone are enough to identify the Group.

Luckily, the only user of this function is QuickFix\Message\Message.cs -> SetGroup(...), so changing it should not be that disruptive.

So, the tasks to solve are:

gbirchmeier commented 5 years ago

Please see #528 for some further code analysis.

gbirchmeier commented 5 years ago

Per this excellent StackOverflow answer by Ciaran McHale, this situation isn't actually FIX compliant.

I may still attempt to fix it anyway, as I've now seen 2 counterparties do it. But it is not a priority, and a better first course of action is to ask your counterparty to correct their implementation.

SilverZippo commented 10 months ago

I hope you are going to fix this soon. Whilst this is not strictly FIX compliant to use the same group tag in different parts of a message, it does happen to be occur in Refinitv's (formally Thomposn Reuters) TRTN data dictionary, and I can't see them agreeing to change it given the large number of firms which have must have been using it for years. It is also handled perfectly well in quickfixJ, and I was hoping to port some exisitng java code to c#, but because of this issue, I am going to have to give up for the time being.

miro-penchev commented 7 months ago

We have the same problem with one of Bloomberg interfaces, I doubt they gonna consider making changes of their side. Looks like C++ implementation of QuickFIX doesnt have such issue.