gemmellr commented on code in PR #4956:
URL: https://github.com/apache/activemq-artemis/pull/4956#discussion_r1625678167
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/ParameterisedAddress.java:
##########
@@ -106,4 +107,46 @@ public static boolean isParameterised(SimpleString
address) {
return URISupport.containsQuery(address);
}
+ public static SimpleString extractAddress(SimpleString address) {
+ return SimpleString.toSimpleString(extractAddress(address.toString()));
+ }
+
+ /**
+ * Given an address string, extract only the query portion if the address is
+ * parameterized, otherwise return an empty {@link Map}.
+ *
+ * @param address
+ * The address to operate on.
+ *
+ * @return a {@link Map} containing the parameters associated with the
given address.
+ */
+ @SuppressWarnings("unchecked")
+ public static Map<String, String> extractParameters(String address) {
+ final int index = address.indexOf('?');
+
+ if (index == -1) {
+ return Collections.EMPTY_MAP;
+ } else {
+ return parseQuery(address);
+ }
+ }
+
+ /**
+ * Given an address string, extract only the address portion if the address
is
+ * parameterized, otherwise just return the provided address.
+ *
+ * @param address
+ * The address to operate on.
+ *
+ * @return the original address minus any appended parameters.
+ */
+ public static String extractAddress(String address) {
+ final int index = address.indexOf('?');
+
+ if (index == -1) {
+ return address;
+ } else {
+ return address.substring(0, index);
+ }
+ }
Review Comment:
The methods cant handle a null but dont say that and the 'otherwise return
provided' bit might imply otherwise. Should either say so or handle it.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/DefaultSenderController.java:
##########
@@ -409,16 +419,38 @@ public Consumer init(ProtonServerSenderContext
senderContext) throws Exception {
// have not honored what it asked for.
source.setFilter(supportedFilters.isEmpty() ? null : supportedFilters);
- boolean browseOnly = !multicast && source.getDistributionMode() != null
&& source.getDistributionMode().equals(COPY);
+ final boolean browseOnly = !multicast && source.getDistributionMode() !=
null && source.getDistributionMode().equals(COPY);
+ final Number consumerPriority =
getReceiverPriority(protonSender.getRemoteProperties(),
extractConsumerPriority(addressParameters));
+
+ // Any new parameters used should be extracted from the values parsed
from the address to avoid this log message.
+ if (!addressParameters.isEmpty()) {
+ final String unusedParametersMessage = ""
+ + " Not all specified address options were applicable to the
created server consumer."
+ + " Check the options are spelled correctly."
+ + " Unused parameters=[" + addressParameters + "].";
+
+ logger.debug(unusedParametersMessage);
+ }
+
+ return sessionSPI.createSender(senderContext, queue, multicast ? null :
selector, browseOnly, consumerPriority);
+ }
+
+ private static Number extractConsumerPriority(Map<String, String>
addressParameters) {
+ if (addressParameters != null && !addressParameters.isEmpty() ) {
+ final String priorityString =
addressParameters.remove(QueueConfiguration.CONSUMER_PRIORITY);
+ if (priorityString != null) {
+ return Integer.valueOf(priorityString);
+ }
+ }
- return (Consumer) sessionSPI.createSender(senderContext, queue,
multicast ? null : selector, browseOnly);
+ return null;
}
@Override
public void close() throws Exception {
Source source = (Source) protonSender.getSource();
- if (source != null && source.getAddress() != null && multicast) {
- SimpleString queueName =
SimpleString.toSimpleString(source.getAddress());
+ if (source != null && getSourceAddress(source) != null && multicast) {
Review Comment:
Given that getSourceAddress(source) handles null Source values, would seem
like this could be put in a variable here rather than potentially doing the
extraction twice as it is.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/DefaultSenderController.java:
##########
@@ -207,6 +212,10 @@ public Consumer init(ProtonServerSenderContext
senderContext) throws Exception {
}
source.setAddress(queue.toString());
} else {
+ final String sourceAddress =
ParameterisedAddress.extractAddress(source.getAddress());
+
+ addressParameters =
ParameterisedAddress.extractParameters(source.getAddress());
Review Comment:
The method cant handle being passed a null, but its possible this value is
null.
I'd guess that might even explain the unit test failure (from an exception
type having changed, to an internal exception)
--
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
