jbertram commented on code in PR #5334:
URL: https://github.com/apache/activemq-artemis/pull/5334#discussion_r1878619808
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImplTest.java:
##########
@@ -99,6 +99,14 @@ public void testZeroMinExpiryDelayWhenExpirationSet() {
Mockito.verify(mockMessage).setExpiration(0L);
}
+ @Test
+ public void testZeroMinExpiryDelayWhenExpirationSetInFuture() {
+ Message mockMessage = Mockito.mock(Message.class);
+
Mockito.when(mockMessage.getExpiration()).thenReturn(System.currentTimeMillis()
+ 500_000L);
+ PostOfficeImpl.applyExpiryDelay(mockMessage, new
AddressSettings().setMinExpiryDelay(0L));
+ Mockito.verify(mockMessage).setExpiration(0L);
+ }
Review Comment:
Thinking about this more...I'm not sure a `min-expiry-delay` of `0` even
makes sense. The overall goal of this change is to enable users to never expire
incoming messages regardless of the message's existing expiration. If they want
to do that then they should set `max-expiry-delay` to `0` and leave
`expiry-delay` and `min-expiry-delay` unset (i.e. `-1`).
Setting `min-expiry-delay` to `0` is really just meaningless. A message will
not realistically have an expiration below `0` and if there's no max set (as in
this test) then anything above `0` should remain unchanged.
I'm going to clarify the Jira to make the use-case explicit.
##########
artemis-server/src/test/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImplTest.java:
##########
@@ -99,6 +99,14 @@ public void testZeroMinExpiryDelayWhenExpirationSet() {
Mockito.verify(mockMessage).setExpiration(0L);
}
+ @Test
+ public void testZeroMinExpiryDelayWhenExpirationSetInFuture() {
+ Message mockMessage = Mockito.mock(Message.class);
+
Mockito.when(mockMessage.getExpiration()).thenReturn(System.currentTimeMillis()
+ 500_000L);
+ PostOfficeImpl.applyExpiryDelay(mockMessage, new
AddressSettings().setMinExpiryDelay(0L));
+ Mockito.verify(mockMessage).setExpiration(0L);
+ }
Review Comment:
Thinking about this more...I'm not sure a `min-expiry-delay` of `0` even
makes sense. The overall goal of this change is to enable users to never expire
incoming messages regardless of the message's existing expiration. If they want
to do that then they should set `max-expiry-delay` to `0` and leave
`expiry-delay` and `min-expiry-delay` unset (i.e. `-1`).
Setting `min-expiry-delay` to `0` is really just meaningless. A message will
not realistically have an expiration below `0` and if there's no max set (as in
this test) then anything above `0` should remain unchanged.
I'm going to clarify the Jira to make the targeted use-case explicit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
