Enforce non-negative delivery delay in strictCompliance mode#1809
Enforce non-negative delivery delay in strictCompliance mode#1809pradeep85841 wants to merge 2 commits intoapache:mainfrom
Conversation
pradeep85841
commented
Mar 19, 2026
- Implemented setDeliveryDelay and getDeliveryDelay in ActiveMQMessageProducer
- Added Jakarta 3.1 compliance check to reject negative delays when strictCompliance is enabled
- Added unit tests to verify both strict enforcement and legacy behaviour
|
| private MessageTransformer transformer; | ||
| private MemoryUsage producerWindow; | ||
|
|
||
| protected long deliveryDelay = 0; |
There was a problem hiding this comment.
deliveryDelay is not used in send().
It means that it's not effective.
To be effective, the delay needs to be applied during message dispatch (so either passing it through to the session send, or setting AMQ_SCHEDULED_DELAY on the message).
Also ActiveMQProducer.setDeliveryDelay() (for JMS 2.0 JMSProducer) is not updated and still throws UnsupportedOperationException.
I think it's better to have the PR atomic and consistent.
|
|
||
| // Jakarta 3.1 Compliance Guard | ||
| if (deliveryDelay < 0 && session.connection.isStrictCompliance()) { | ||
| throw new jakarta.jms.MessageFormatException("Delivery delay cannot be negative when strictCompliance is enabled."); |
There was a problem hiding this comment.
This is wrong imho.
Jakarta Messaging 3.1 spec (MessageProducer.setDeliveryDelay) doesn't mandate MessageFormatException for invalid arguments.
MessageFormatException is for message body/property type conversion issues (like in setBody).
An IllegalArgumentException or JMSException (generic) would be more appropriate.
The JMS Spec says nothing about negative delays being illegal, the behavior is undefined and it's up to us to decide the validation logic.
| */ | ||
|
|
||
| @Override | ||
| public void setDeliveryDelay(long deliveryDelay) throws JMSException { |
There was a problem hiding this comment.
Rather than defining here, why not updated ActiveMQMessageProducerSupport ?
| private MessageTransformer transformer; | ||
| private MemoryUsage producerWindow; | ||
|
|
||
| protected long deliveryDelay = 0; |
There was a problem hiding this comment.
Do we need really protected here ?
As we have getter/setter access, we can go with private and using mock in the test, right ?
|
Updates to DeliveryDelay work should be applied to the pre-existing WIP PR: [#1157] |