Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
5 tasks
gbirchmeier opened this issue Sep 13, 2018 · 4 comments

Comments

@gbirchmeier
Copy link
Member

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:

  • figure out how QuickFix\Message\Message.cs->SetGroup(...) can pass an ancestry chain to the group-create method
  • rework IMessageFactory::Create/group to take that ancestry chain
  • rework QuickFix\DefaultMessageFactory.cs->Create/group per that interface change. It will need to create the right group based on the ancestry chain.
  • rework the generator for Messages\FIXxx\MessageFactory.cs->Create/group methods. Generated methods will need to create the right group based on that ancestry chain.
  • fix/rework all the tests that the above will break
@gbirchmeier
Copy link
Member Author

Please see #528 for some further code analysis.

@gbirchmeier
Copy link
Member Author

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
Copy link

SilverZippo commented Nov 13, 2023

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants