Couple of points & questions:
* This PR is putting JMS-specific logic into the core broker where it doesn't belong.
* Is the `clientID` being returned here supposed to be the JMS client ID? The client ID on the connection object being retrieved is not the JMS client ID. The JMS client ID is set on the core session's internal meta-data.
* The JMS prefixes that are being used here to determine the are no longer valid as they were removed in 2.0.
* The test is only validating that the fields exist in the reply and not that they actually have accurate values.
* Is the new `durable` flag meant to be some kind of indication about a JMS durable subscriber? The value for `durable` being returned is for the queue to which the consumer is attached which isn't directly related to JMS durable subscriptions.
The information missing is some JMS queue information, as follows: destinationName for the name of destination e.g InQueue (not core name or address), destinationType - queue or topic, durable is the durability of consumer and clientId is JMS client ID. Could this be provided on JMSServerControlImpl?
The broker no longer makes any special distinction between JMS and non-JMS destinations. There are only core addresses, queues, and routing types.
* The name of the destination to which a JMS client sends a message should correspond to the name of a core address.
* The destination type will correspond to the address's routing type - MULTICAST for topic, ANYCAST for queue.
* The JMS client ID can be pulled from the core session's meta-data (see https://issues.apache.org/jira/browse/ARTEMIS-1769 for more info on that point).
I don't want to speak for @michaelandrepearce, but personally I find it much easier to review PRs when all the changes in the PR are directly related. This PR is about adding missing fields to some management methods not increasing code legibility. In general, increasing code legibility is a good thing, but it can be detrimental in non-obvious ways (e.g. when lots of legibility changes make it difficult to back-port fixes to maintenance branches) and IMO changes which are exclusively for improving legibility should be in their own commits. Finally, "legibility" can be subjective. You risk having your functional PR rejected by including non-functional, subjective "improvements."
Unfortunately the updated PR still suffers from importing JMS nomenclature into core management operations. The core broker doesn't have destination names or destination types. It has addresses, queues, and routing types. You should be doing something like this:
In addition to this PR, we would have to unmarshall the JSON string, change it to put back the JMS semantic and marshall it again before to send it to the management client.
This seems brittle to rely on a JSON string for this information.
@jbertram Would it be possible to enhance ActiveMQServerControl so that it returns a proper DTO containing all the information? In WildFly we can then take care of all the transformation and json serialization but we would have a proper API contract for all consumer and connection data coming from ActiveMQServerControl