jbertram commented on code in PR #5291:
URL: https://github.com/apache/activemq-artemis/pull/5291#discussion_r1853985284


##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java:
##########
@@ -1298,6 +1298,18 @@ public ServerLocatorImpl 
setInitialMessagePacketSize(final int size) {
       return this;
    }
 
+   @Override
+   public int getOnMessageCloseTimeout() {
+      return config.onMessageCloseTimeout;
+   }
+
+   @Override

Review Comment:
   I changed the JavaDoc to be more clear. I didn't want to add validation as 
it wouldn't be consistent with the rest of the method and adding validation to 
all the places that could realistically use it is well beyond the scope of this 
PR.



##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/client/MessageHandlerTest.java:
##########
@@ -117,6 +118,42 @@ public void onMessage(final ClientMessage message) {
       session.close();
    }
 
+   @Test
+   public void testMessageHandlerCloseTimeout() throws Exception {
+      // create Netty acceptor so client can use new onMessageCloseTimeout URL 
parameter
+      server.getRemotingService().createAcceptor("netty", 
"tcp://127.0.0.1:61616").start();
+      final int timeout = 1000;
+      locator = 
ActiveMQClient.createServerLocator("tcp://127.0.0.1:61616?onMessageCloseTimeout="
 + timeout);
+      sf = createSessionFactory(locator);
+      ClientSession session = addClientSession(sf.createSession(false, true, 
true));
+      session.createQueue(QueueConfiguration.of(QUEUE).setDurable(false));
+      ClientProducer producer = session.createProducer(QUEUE);
+      producer.send(createTextMessage(session, "m"));
+
+      ClientConsumer consumer = session.createConsumer(QUEUE, null, false);
+
+      session.start();
+
+      CountDownLatch latch = new CountDownLatch(1);
+      consumer.setMessageHandler(message -> {
+         latch.countDown();
+         // don't just Thread.sleep() here because it will be interrupted on 
ClientConsumer.close()
+         long start = System.currentTimeMillis();
+         while (System.currentTimeMillis() - start < 2000) {
+            try {
+               Thread.sleep(100);
+            } catch (InterruptedException e) {
+               // ignore
+            }
+         }
+      });
+      latch.await();
+      long start = System.currentTimeMillis();
+      consumer.close();
+      long end = System.currentTimeMillis();
+      assertTrue( (end - start >= timeout) && (end - start <= 2000), "Closing 
consumer took " + (end - start) + "ms");

Review Comment:
   Done.



-- 
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


Reply via email to