git.net

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage


Github user gemmellr commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2418
  
    > Re seq not set, its an int field so its tech always set if you request it, just should default 0 if not set.
    
    JMSXGroupSeq is set explicitly by the application. If it isn't set by the application, then the property doesn't exist, the same as any other int property. It shouldn't be in the property names and we shouldn't be setting group-sequence to 0 on the wire when nothing has actually set JMSXGroupSeq.
    
    > Re the name this is the name in the message interface that already existed for that.
    
    Oh, right. Yes, according to the changelog adding it, it deliberately wasn't implemented in the AMQPMessage, which makes sense because per my earlier comment using it would be a protocol violation given it is an immutable part of the message. So that bit should be unwound I think.
    
    > If i was just to fix the test, it would have been a one liner null check , and i didnt want to just fix the test (would have been less work for me too if i did that). But the fact is as i mentioned above finding a few things that better to fix it all up a bit, thus why its a bit more involved change
    
    I think a one-liner fix would probably have been better, keeping the general improvement separate/later. It makes it easier to see the actual issue and fix is, and makes backporting bug fixes easier without extra changes either being in the way or tagging along (not that that actually applies here specifically, but generally I mean).
    
    As I've said though, it doesnt seem that either actually addresses the apparent main issue though, of group sequence values being present in messages a sequence was never set on.


---