Camel
  1. Camel
  2. CAMEL-5979

Camel-Quickfix dynamic SenderSubId/TargetSubId issue

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.10.3
    • Fix Version/s: 2.9.7, 2.10.5, 2.11.0
    • Component/s: camel-quickfix
    • Labels:
      None
    • Environment:

      QuickFix/J version 1.5 on JDK 1.6.x

    • Estimated Complexity:
      Unknown

      Description

      I am facing an issue with camel-quickfix component for the following scenario.

      We have a FIX message coming in to the Quickfix/J engine i.e. logon request with only the senderID and TargetID populated and the engine successfully processes the logon request based on the configuration. The subsequent requests for Single Order (Tag # 35 = D) the message received contains the optional values SenderSubID and TargetSubID populated, the message is picked up by the camel component without any issue but its only when a reply has to sent to the sender the component ends up throwing a error i.e. IllegalStateException : Unknown Session... I looked into the code https://svn.apache.org/repos/asf/camel/trunk/components/camel-quickfix/src/main/java/org/apache/camel/component/quickfixj/QuickfixjConsumer.java and found the issue. The session ID is being reconstructed from the in value being set into the exchange object, which as per API used would construct the session containing SenderID,SenderSubID,TargetID and TargetSubID which is getting compared to the session set during the logon process which only contains the SenderID and TargetID and hence doesn't matches and returns a null. On receiving the null value the code ends up throwing the IllegalStateException. Looking at the QuickFixJConsumer code i do understand the importance of reconstructing the SessionID object from the Message set as Out in Exchange but from what i read about various client implementations on using FIX the above scenario mentioned also holds good. For now I have gone ahead and patched the QuickFixJConsumer code for my project by picking up the session id set in Exchange object rather than recreating it using the MessageUtils. But i feel in a longer run a elegant solution has to be put in place rather than user of camel patching the code up for their use
      In case you need any further details do let me know..

        Activity

        Hide
        Claus Ibsen added a comment -

        Please spend more time to format the description so its readable.

        Nobody bother to read it and work on the ticket if you dont take the time to make it understandable.

        Show
        Claus Ibsen added a comment - Please spend more time to format the description so its readable. Nobody bother to read it and work on the ticket if you dont take the time to make it understandable.
        Hide
        Yogesh Rao added a comment -

        Will try to work on the description but have a issue with the priority reduced to minor. Can you let me why it has been changed. As i see it camel-quickfix right now is not supporting one of the popularly used scenario in FIX domain.

        Show
        Yogesh Rao added a comment - Will try to work on the description but have a issue with the priority reduced to minor. Can you let me why it has been changed. As i see it camel-quickfix right now is not supporting one of the popularly used scenario in FIX domain.
        Hide
        Christian Müller added a comment -
        Show
        Christian Müller added a comment - See also the discussion on the users mailing list: http://camel.465427.n5.nabble.com/Camel-QuickFix-Component-issue-td5725622.html
        Hide
        Christian Müller added a comment -

        A few questions:

        • In trunk, the SessionID is recreated with the in message and not the out message. Could you check whether this issue is still there?
        • Can you share your code? How do you solved your issue?
        • Can we create the SessionID before we send the exchange to the processor? Than we can store it in the exchange for later usage.
        Show
        Christian Müller added a comment - A few questions: In trunk, the SessionID is recreated with the in message and not the out message. Could you check whether this issue is still there? Can you share your code? How do you solved your issue? Can we create the SessionID before we send the exchange to the processor? Than we can store it in the exchange for later usage.
        Hide
        Yogesh Rao added a comment -

        Hello Christian,

        Answer to your comments

        1. Yes the issue persists, It was my bad that i mentioned in the description that the SessionID is recreated from out Message whereas i should have written it is being constructed from in message.
        2. Yes the only change i have done to the trunk code is instead of
        SessionID messageSessionID = MessageUtils.getReverseSessionID(exchange.getIn().getBody(quickfix.Message.class));
        I have used is the already created session which is set as part of header, for which the code change is
        SessionID messageSessionID = exchange.getIn().getHeader("SessionID",SessionID.class);
        This helps me pick up the same session from which the request had come in.
        3. I think that is already being done. Its just that in the code we trying to recreate a new sessionID instead of using the already set SessionID found as part of the header.

        I will modify my description to reflect "in message" instead of "out message" for tracking purposes. I am also trying to think would there be a scenario wherein the already written code would hold good then perhaps we might have to toggle the SessionID creation based on some property/switch.

        Thank you!

        Show
        Yogesh Rao added a comment - Hello Christian, Answer to your comments 1. Yes the issue persists, It was my bad that i mentioned in the description that the SessionID is recreated from out Message whereas i should have written it is being constructed from in message. 2. Yes the only change i have done to the trunk code is instead of SessionID messageSessionID = MessageUtils.getReverseSessionID(exchange.getIn().getBody(quickfix.Message.class)); I have used is the already created session which is set as part of header, for which the code change is SessionID messageSessionID = exchange.getIn().getHeader("SessionID",SessionID.class); This helps me pick up the same session from which the request had come in. 3. I think that is already being done. Its just that in the code we trying to recreate a new sessionID instead of using the already set SessionID found as part of the header. I will modify my description to reflect "in message" instead of "out message" for tracking purposes. I am also trying to think would there be a scenario wherein the already written code would hold good then perhaps we might have to toggle the SessionID creation based on some property/switch. Thank you!
        Hide
        Yogesh Rao added a comment -

        Christian,

        Let me know if my fix is fine.. i can go ahead and check in the changes in camel's svn.

        Show
        Yogesh Rao added a comment - Christian, Let me know if my fix is fine.. i can go ahead and check in the changes in camel's svn.
        Hide
        Christian Müller added a comment -

        Yogesh Rao: Can you attach your path to this ticket so that we can review and apply it?

        Show
        Christian Müller added a comment - Yogesh Rao : Can you attach your path to this ticket so that we can review and apply it?
        Hide
        Yogesh Rao added a comment -

        patch for picking up the session id already set as header instead of recreating it from Message object

        Show
        Yogesh Rao added a comment - patch for picking up the session id already set as header instead of recreating it from Message object
        Hide
        Yogesh Rao added a comment -

        christian,

        I am assuming you were asking for a patch instead of path.. Please find the patch attached.

        Show
        Yogesh Rao added a comment - christian, I am assuming you were asking for a patch instead of path.. Please find the patch attached.
        Hide
        Yogesh Rao added a comment -

        Christian,

        Any updates on the patch attached ?

        Show
        Yogesh Rao added a comment - Christian, Any updates on the patch attached ?
        Hide
        Yogesh Rao added a comment -

        Christian,

        We are waiting for a fix, is the attached patch any good? please let us know at earliest.

        Thank you!

        Show
        Yogesh Rao added a comment - Christian, We are waiting for a fix, is the attached patch any good? please let us know at earliest. Thank you!
        Hide
        Christian Müller added a comment -

        If I apply this patch, two tests failed:

        mvn clean install -Psourcecheck
        ...
        processInOutExchange(org.apache.camel.component.quickfixj.QuickfixjConsumerTest)  Time elapsed: 0.268 sec  <<< FAILURE!
        ...
        setExceptionOnInOutExchange(org.apache.camel.component.quickfixj.QuickfixjConsumerTest)  Time elapsed: 0.006 sec  <<< FAILURE!
        ...
        Tests run: 42, Failures: 2, Errors: 0, Skipped: 0
        

        Could you please check this.

        Show
        Christian Müller added a comment - If I apply this patch, two tests failed: mvn clean install -Psourcecheck ... processInOutExchange(org.apache.camel.component.quickfixj.QuickfixjConsumerTest) Time elapsed: 0.268 sec <<< FAILURE! ... setExceptionOnInOutExchange(org.apache.camel.component.quickfixj.QuickfixjConsumerTest) Time elapsed: 0.006 sec <<< FAILURE! ... Tests run: 42, Failures: 2, Errors: 0, Skipped: 0 Could you please check this.
        Hide
        Christian Müller added a comment -

        It's now fixed in trunk (r1455750), camel-2.10.x (r1455752) and camel-2.9.y (r1455760, r1455762).
        Could you please verify whether the fix works for you?

        And thanks again for the patch...

        Show
        Christian Müller added a comment - It's now fixed in trunk (r1455750), camel-2.10.x (r1455752) and camel-2.9.y (r1455760, r1455762). Could you please verify whether the fix works for you? And thanks again for the patch...
        Hide
        Yogesh Rao added a comment -

        I have been using the patch in my code long time ago and the code fix has been working for us. We had tampered the camel-quickfix jar for us, now will upgrade the version of camel to check if it works.. This would take time but I am sure should work.

        Thank you for the fix...

        Cheers!
        -Yogesh

        Show
        Yogesh Rao added a comment - I have been using the patch in my code long time ago and the code fix has been working for us. We had tampered the camel-quickfix jar for us, now will upgrade the version of camel to check if it works.. This would take time but I am sure should work. Thank you for the fix... Cheers! -Yogesh

          People

          • Assignee:
            Christian Müller
            Reporter:
            Yogesh Rao
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development