activemq
activemq copied to clipboard
apache
WIP: [AMQ-8320] Add JMS 2.0 JMSProducer deliveryDelay support
Seems like the same code as the old PR, so same comment from before:
Since message objects can be reused/resent, and might already have an existing value for JMSDeliveryTime, it should always be setting the JMSDeliveryTime value on send even if there isnt a delay, to ensure the JMSDeliveryTime value on the message after send reflects behaviour of that send and not some previous send or receive.
That doesnt mean it has to transit a delivery time on the wire...ideally it wouldnt if there is no delay being requested for this send...just that after send the local message object should report the delivery time for that send. Elsewhere I added an internal message method to set the value and note whether it is for transmission.
It doesnt feel like it should be setting a message property at all, only a header. I don't expect the TCK tests which check additional properties arent being added to the messages are using delivery-delay at the time..but they could. I know the older 'message property extension' scheduling stuff uses one, but its not clear to me this needs to. They could just be treated as separate mechanisms, which they kind of are already as you have to choose to respect the new value on the producer or the old one on the message. Theres no reason the broker couldnt do the same, and this new mechanism essentially ignore that old prop.
It also now occurs to me it may not handle foreign providers Message properly since it seemingly doesnt call setJMSDeliveryTime at all on send (unless thats being done elsewhere already...though doesnt seem obvious it could know the value to use).
Although the PR title makes it clear that it does, it also seems weird this change only covers the JMSProducer API related code, and misses out the MessageProducer related API, when delivery delay support applies equally to both (and the underlying send code is used for both).