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 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: gitbox-unsubscr...@activemq.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org For additional commands, e-mail: gitbox-h...@activemq.apache.org For further information, visit: https://activemq.apache.org/contact