[GitHub] activemq-artemis issue #2418: ARTEMIS-2142 Fix ServerJMSMessage
Github user gemmellr commented on the issue:
I tried the tests and they worked, but it occurred to me that though changes might be needed in general to fix whats currently stopping the tests passing, just making them pass is likely masking another issue that is the actual problem.
I had a look what one of the tests is actually doing with JMSXGroupSeq given it worked before and didnt now. The answer seems to be nothing at all, which is somewhat expected given its not testing that. As such the failure to handle JMSXGroupSeq properly isnt the main issue, the fact its present at all would be. If nothing set it then it shouldn't be present. With this change in place I can see the received AMQP message in one of the previously-failing tests does have a group sequence value (of 0) populated when it seemingly should not.
More of an aside, I'm not hugely fond of 'getValidatedUserId' as a name since its not clear anything has actually validated it. The various setters added to AMQPMessage perhaps also open up scope for protocol violations (since they are all immutable properties).