feat(console): add user-friendly CLI commands for queues, topics, broker, and network connectors(#1816). - #1966
feat(console): add user-friendly CLI commands for queues, topics, broker, and network connectors(#1816).#1966anmol-saxena-14 wants to merge 2 commits into
Conversation
… and network connectors.
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days. |
|
This PR is not stale, it is still under review by reviewer. |
jbonofre
left a comment
jbonofre
left a comment
There was a problem hiding this comment.
Thanks for the contribution, this is a nice addition and follows the existing AbstractJmxCommand / ServiceLoader pattern cleanly. The tests spinning up real embedded brokers and verifying side effects via JMX proxies are much appreciated. A few comments inline — the main one to address before merge is the negative tests that can pass silently. The rest are cleanups (duplication, dead code) and are non-blocking.
| } | ||
|
|
||
| @Test(timeout = 30000) | ||
| public void testAddMissingUriThrows() throws Exception { |
There was a problem hiding this comment.
This test can pass silently: the try/catch has no fail() after execute("add"), so if the IllegalArgumentException is never thrown the assertion in the catch block simply doesn't run and the test still passes. It would keep passing even if the validation were removed. Please add a fail("expected IllegalArgumentException") after the execute(...) call, or switch to assertThrows(IllegalArgumentException.class, () -> execute("add")).
| } | ||
|
|
||
| @Test(timeout = 30000) | ||
| public void testRemoveMissingNameThrows() throws Exception { |
There was a problem hiding this comment.
Same issue as testAddMissingUriThrows: no fail() after execute("remove"), so this passes even if no exception is thrown. Use assertThrows(...) or add a fail(...) after the call.
| } | ||
|
|
||
| @Test(timeout = 30000) | ||
| public void testProduceMissingBody() throws Exception { |
There was a problem hiding this comment.
Same silent-pass problem here: the try/catch around execute("produce", "Q1") has no fail(), so the test passes whether or not the exception is thrown. Please use assertThrows(IllegalArgumentException.class, ...) or add a fail(...).
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private BrokerViewMBean getBrokerMBean() throws Exception { |
There was a problem hiding this comment.
getBrokerMBean() and dashes() are duplicated verbatim across all four new command classes (QueuesCommand, TopicsCommand, BrokerCommand, NetworkConnectorsCommand). Consider lifting a shared helper (e.g. getBrokerViewMBean()) into AbstractJmxCommand so all commands reuse it. Non-blocking, but it would avoid the copy-paste drift.
| } | ||
| } | ||
|
|
||
| private static String dashes(int count) { |
There was a problem hiding this comment.
dashes(int) can just be "-".repeat(count) (Java 11+ is available here), which would also let this duplicated helper disappear once the shared base is in place.
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private void purgeQueue(String queueName) throws Exception { |
There was a problem hiding this comment.
purge, info, browse, produce, pause, and resume all repeat the same ~6-line block: query by destinationName, bail with a not-found message if empty, otherwise build a QueueViewMBean proxy from the first result. Extracting a private QueueViewMBean lookupQueue(String name) helper (returning null and printing the not-found message on a miss) would remove most of the repetition and centralize the not-found behavior.
| */ | ||
| package org.apache.activemq.console; | ||
|
|
||
| import static org.junit.Assert.assertFalse; |
There was a problem hiding this comment.
assertFalse is imported but never used in this test — please remove the unused import.
| return out.toString(); | ||
| } | ||
|
|
||
| private TopicViewMBean getTopicProxy(String topicName) throws Exception { |
There was a problem hiding this comment.
getTopicProxy(...) is never called anywhere in this test class (unlike getQueueProxy in QueuesCommandTest, which is used). Please remove this dead helper.
2 participants
Adds four new action-oriented CLI commands as described in Issue#1816, making it straightforward to manage a running broker from the shell(and from AI agents such as Claude Code) without requiring JMX tooling.
New commands
activemq queueslist | create | delete | purge | info | browse | produce | pause | resumeactivemq topicslist | create | delete | infoactivemq brokerinfo | connectors [list|add|remove] | scheduleractivemq network-connectorslist | add | removeEach command is registered via the existing ServiceLoader SPI
(META-INF/services/...Command)so no existing code is modified to discover them.The consume action is intentionally not implemented in the new commands. Receiving and acknowledging messages requires an active JMS connection,which is outside the scope of the JMX-based CLI facade. The existing
activemq consumercommand covers that use case.Tests : 37 new unit tests across
BrokerCommandTest,NetworkConnectorsCommandTest,QueuesCommandTest, andTopicsCommandTestcovering all actions and error-handling paths.