activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From gemmellr <>
Subject [GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Date Wed, 07 Nov 2018 17:05:54 GMT
Github user gemmellr commented on the issue:
    > 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.


View raw message