[
https://issues.apache.org/jira/browse/ARTEMIS-4212?focusedWorklogId=854143&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-854143
]
ASF GitHub Bot logged work on ARTEMIS-4212:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 31/Mar/23 10:25
Start Date: 31/Mar/23 10:25
Worklog Time Spent: 10m
Work Description: gemmellr commented on code in PR #4421:
URL: https://github.com/apache/activemq-artemis/pull/4421#discussion_r1154207382
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/AutoCreateResult.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.api.core;
+
+public enum AutoCreateResult {
+
+ EXISTED, CREATED, UPDATED, NOT_FOUND;
+
+ public byte getType() {
+ switch (this) {
+ case EXISTED:
+ return 0;
+ case CREATED:
+ return 1;
+ case UPDATED:
+ return 2;
+ case NOT_FOUND:
+ return 3;
+ default:
+ return -1;
+ }
+ }
+
+ public static AutoCreateResult getType(byte type) {
+ switch (type) {
+ case 0:
+ return EXISTED;
+ case 1:
+ return CREATED;
+ case 2:
+ return UPDATED;
+ case 3:
+ return NOT_FOUND;
+ default:
+ return null;
Review Comment:
Perhaps just throw, e.g IllegalArgumentException?
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/AddressQueryImpl.java:
##########
@@ -133,4 +141,14 @@ public Integer getDefaultConsumersBeforeDispatch() {
public Long getDefaultDelayBeforeDispatch() {
return defaultDelayBeforeDispatch;
}
+
+ @Override
+ public boolean isSupportsMulticast() {
+ return supportsMulticast;
+ }
+
+ @Override
+ public boolean isSupportsAnycast() {
+ return supportsAnycast;
+ }
Review Comment:
The fields are object Boolean but the return is primitive boolean, so these
methods will NPE if the values passed into the constructor are ever null, and
it appears there are cases thats true. The constructor, fields, and/or methods
should be updated to handle this.
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
##########
@@ -1742,49 +1743,80 @@ public Transaction getCurrentTransaction() {
return tx;
}
-
@Override
- public boolean checkAutoCreate(SimpleString address, RoutingType
routingType) throws Exception {
- boolean result;
- SimpleString unPrefixedAddress = removePrefix(address);
+ public AutoCreateResult checkAutoCreate(QueueConfiguration queueConfig)
throws Exception {
+ AutoCreateResult result;
+ SimpleString unPrefixedAddress = removePrefix(queueConfig.getAddress());
AddressSettings addressSettings =
server.getAddressSettingsRepository().getMatch(unPrefixedAddress.toString());
- if (routingType == RoutingType.MULTICAST) {
- if (server.getAddressInfo(unPrefixedAddress) == null) {
- if (addressSettings.isAutoCreateAddresses()) {
+ /*
+ *
+ */
+ if (queueConfig.getRoutingType() == null) {
+ return AutoCreateResult.EXISTED;
+ }
Review Comment:
No comment? :) Also, hmmm...(I see thats basically what the old code did
also)
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/multiprotocol/JMSMismatchedRoutingTypeTest.java:
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.tests.integration.jms.multiprotocol;
+
+import javax.jms.Connection;
+import javax.jms.MessageProducer;
+import javax.jms.Queue;
+import javax.jms.Session;
+import javax.jms.Topic;
+
+import org.apache.activemq.artemis.api.core.QueueConfiguration;
+import org.apache.activemq.artemis.api.core.RoutingType;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
+import org.apache.activemq.artemis.core.server.impl.AddressInfo;
+import org.apache.activemq.artemis.tests.util.RandomUtil;
+import org.junit.Test;
+
+public class JMSMismatchedRoutingTypeTest extends
MultiprotocolJMSClientTestSupport {
+
+ protected final String ANYCAST_ADDRESS = RandomUtil.randomString();
+ protected final String MULTICAST_ADDRESS = RandomUtil.randomString();
+
+ @Override
+ protected boolean isAutoCreateAddresses() {
+ return false;
+ }
+
+ @Override
+ protected boolean isAutoCreateQueues() {
+ return false;
+ }
+
+ @Override
+ protected void createAddressAndQueues(ActiveMQServer server) throws
Exception {
+ server.addAddressInfo(new
AddressInfo(SimpleString.toSimpleString(ANYCAST_ADDRESS), RoutingType.ANYCAST));
+ server.createQueue(new
QueueConfiguration(RandomUtil.randomString()).setAddress(ANYCAST_ADDRESS).setRoutingType(RoutingType.ANYCAST));
+
+ server.addAddressInfo(new
AddressInfo(SimpleString.toSimpleString(MULTICAST_ADDRESS),
RoutingType.MULTICAST));
+ server.createQueue(new
QueueConfiguration(RandomUtil.randomString()).setAddress(MULTICAST_ADDRESS).setRoutingType(RoutingType.MULTICAST));
+ }
+
+ @Test
+ public void testSendingMulticastToAnycastAMQP() throws Exception {
+ internalTestSendingMulticastToAnycast(AMQPConnection);
+ }
+
+ @Test
+ public void testSendingMulticastToAnycastCore() throws Exception {
+ internalTestSendingMulticastToAnycast(CoreConnection);
+ }
+
+ @Test
+ public void testSendingMulticastToAnycastOpenWire() throws Exception {
+ internalTestSendingMulticastToAnycast(OpenWireConnection);
+ }
+
+ private void internalTestSendingMulticastToAnycast(ConnectionSupplier
connectionSupplier) throws Exception {
+ Connection connection = connectionSupplier.createConnection();
+ Session s = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ Topic topic = s.createTopic(ANYCAST_ADDRESS);
+ try {
+ MessageProducer producer = s.createProducer(topic);
+ producer.send(s.createMessage());
+ fail("Sending message here should fail!");
+ } catch (Exception e) {
+ e.printStackTrace();
Review Comment:
Maybe just a simple log message instead of the full stack? Could also debug
log with the exception if desired (here and below)? The test output is long
enough without another bunch of stacktraces in it for things expected to happen.
##########
artemis-commons/src/main/java/org/apache/activemq/artemis/api/core/AutoCreateResult.java:
##########
@@ -0,0 +1,52 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.api.core;
+
+public enum AutoCreateResult {
+
+ EXISTED, CREATED, UPDATED, NOT_FOUND;
+
+ public byte getType() {
+ switch (this) {
+ case EXISTED:
+ return 0;
+ case CREATED:
+ return 1;
+ case UPDATED:
+ return 2;
+ case NOT_FOUND:
+ return 3;
+ default:
+ return -1;
+ }
+ }
+
+ public static AutoCreateResult getType(byte type) {
Review Comment:
Perhaps valueOf / create / etc, avoid the method overload for methods
actually going in different directions (to/from byte)?
EDIT: actually these methods dont seem to be used or tested at all...could
they just go?
##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java:
##########
@@ -1270,11 +1251,7 @@ public Response processAddProducer(ProducerInfo info)
throws Exception {
ActiveMQDestination destination = info.getDestination();
if (destination != null &&
!AdvisorySupport.isAdvisoryTopic(destination)) {
- if (destination.isQueue()) {
- OpenWireConnection.this.validateDestination(destination);
Review Comment:
Removing this call seems like it will leave the validateDestination method
unused, remove if so?
##########
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java:
##########
@@ -899,36 +895,21 @@ public boolean sendCommand(final Command command) {
}
public void addDestination(DestinationInfo info) throws Exception {
- boolean created = false;
+ boolean created;
ActiveMQDestination dest = info.getDestination();
if (!protocolManager.isSupportAdvisory() &&
AdvisorySupport.isAdvisoryTopic(dest)) {
return;
}
SimpleString qName = SimpleString.toSimpleString(dest.getPhysicalName());
- if (server.locateQueue(qName) == null) {
- AddressSettings addressSettings =
server.getAddressSettingsRepository().getMatch(dest.getPhysicalName());
- if (dest.isQueue() && (addressSettings.isAutoCreateQueues() ||
dest.isTemporary())) {
- try {
- internalSession.createQueue(new
QueueConfiguration(qName).setRoutingType(RoutingType.ANYCAST).setDurable(!dest.isTemporary()).setTemporary(dest.isTemporary()).setAutoCreated(!dest.isTemporary()));
- created = true;
- } catch (ActiveMQQueueExistsException exists) {
- // The queue may have been created by another thread in the
mean time. Catch and do nothing.
- }
- } else if (dest.isTopic() && (addressSettings.isAutoCreateAddresses()
|| dest.isTemporary())) {
- try {
- AddressInfo addressInfo = new AddressInfo(qName,
RoutingType.MULTICAST);
- if (AdvisorySupport.isAdvisoryTopic(dest) &&
protocolManager.isSuppressInternalManagementObjects()) {
- addressInfo.setInternal(true);
- }
- if (internalSession.getAddress(addressInfo.getName()) == null) {
- internalSession.createAddress(addressInfo,
!dest.isTemporary());
- created = true;
- }
- } catch (ActiveMQAddressExistsException exists) {
- // The address may have been created by another thread in the
mean time. Catch and do nothing.
- }
+
+ if (internalSession.checkAutoCreate(new
QueueConfiguration(qName).setRoutingType(dest.isQueue() ? RoutingType.ANYCAST :
RoutingType.MULTICAST).setTemporary(dest.isTemporary())) ==
AutoCreateResult.CREATED) {
+ created = true;
+ if (AdvisorySupport.isAdvisoryTopic(dest) &&
protocolManager.isSuppressInternalManagementObjects()) {
+ internalSession.getAddress(qName).setInternal(true);
}
+ } else {
+ throw
ActiveMQMessageBundle.BUNDLE.noSuchQueue(SimpleString.toSimpleString(dest.getPhysicalName()));
Review Comment:
The perhaps-now-unused validateDestination method was previously only
called, and so only threw this 'noSuchQueue' exception, for Queue destinations.
This seems like it will now do it for both destination types, in which case the
message might seem misleading some of the time?
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/SessionBindingQueryResponseMessage_V5.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.artemis.core.protocol.core.impl.wireformat;
+
+import java.util.List;
+
+import org.apache.activemq.artemis.api.core.ActiveMQBuffer;
+import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.utils.BufferHelper;
+
+public class SessionBindingQueryResponseMessage_V5 extends
SessionBindingQueryResponseMessage_V4 {
+
+ protected Boolean supportsMulticast;
+
+ protected Boolean supportsAnycast;
+
+ public SessionBindingQueryResponseMessage_V5(final boolean exists,
+ final List<SimpleString>
queueNames,
+ final boolean autoCreateQueues,
+ final boolean
autoCreateAddresses,
+ final boolean
defaultPurgeOnNoConsumers,
+ final int defaultMaxConsumers,
+ final Boolean defaultExclusive,
+ final Boolean defaultLastValue,
+ final SimpleString
defaultLastValueKey,
+ final Boolean
defaultNonDestructive,
+ final Integer
defaultConsumersBeforeDispatch,
+ final Long
defaultDelayBeforeDispatch,
+ final Boolean
supportsMulticast,
+ final Boolean supportsAnycast)
{
+ super(SESS_BINDINGQUERY_RESP_V5);
+
+ this.exists = exists;
+
+ this.queueNames = queueNames;
+
+ this.autoCreateQueues = autoCreateQueues;
+
+ this.autoCreateAddresses = autoCreateAddresses;
+
+ this.defaultPurgeOnNoConsumers = defaultPurgeOnNoConsumers;
+
+ this.defaultMaxConsumers = defaultMaxConsumers;
+
+ this.defaultExclusive = defaultExclusive;
+
+ this.defaultLastValue = defaultLastValue;
+
+ this.defaultLastValueKey = defaultLastValueKey;
+
+ this.defaultNonDestructive = defaultNonDestructive;
+
+ this.defaultConsumersBeforeDispatch = defaultConsumersBeforeDispatch;
+
+ this.defaultDelayBeforeDispatch = defaultDelayBeforeDispatch;
+
+ this.supportsMulticast = supportsMulticast;
+
+ this.supportsAnycast = supportsAnycast;
+ }
+
+ public SessionBindingQueryResponseMessage_V5() {
+ super(SESS_BINDINGQUERY_RESP_V5);
+ }
+
+ public Boolean isSupportsMulticast() {
+ return supportsMulticast;
+ }
+
+ public Boolean isSupportsAnycast() {
+ return supportsAnycast;
+ }
+
+ @Override
+ public void encodeRest(final ActiveMQBuffer buffer) {
+ super.encodeRest(buffer);
+ BufferHelper.writeNullableBoolean(buffer, supportsMulticast);
+ BufferHelper.writeNullableBoolean(buffer, supportsAnycast);
+ }
+
+ @Override
+ public void decodeRest(final ActiveMQBuffer buffer) {
+ super.decodeRest(buffer);
+ if (buffer.readableBytes() > 0) {
+ supportsMulticast = BufferHelper.readNullableBoolean(buffer);
+ supportsAnycast = BufferHelper.readNullableBoolean(buffer);
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = super.hashCode();
+ result = prime * result + (supportsMulticast == null ? 0 :
supportsMulticast ? 1231 : 1237);
+ result = prime * result + (supportsAnycast == null ? 0 : supportsAnycast
? 1231 : 1237);
+ return result;
+ }
+
+ @Override
+ protected String getPacketString() {
+ StringBuffer buff = new StringBuffer(super.getPacketString());
+ buff.append(", supportsMulticast=" + supportsMulticast);
+ buff.append(", supportsAnycast=" + supportsAnycast);
+ return buff.toString();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (!super.equals(obj))
+ return false;
+ if (!(obj instanceof SessionBindingQueryResponseMessage_V5))
+ return false;
+ SessionBindingQueryResponseMessage_V5 other =
(SessionBindingQueryResponseMessage_V5) obj;
+ if (supportsMulticast == null) {
+ if (other.supportsMulticast != null)
+ return false;
+ } else if (!supportsMulticast.equals(other.supportsMulticast))
+ return false;
Review Comment:
I dont like brace'less if's generally so never really use them, but I'd not
considered this even more horrible _mix_ was possible in a single flow hehe :S
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
##########
@@ -1742,49 +1743,80 @@ public Transaction getCurrentTransaction() {
return tx;
}
-
@Override
- public boolean checkAutoCreate(SimpleString address, RoutingType
routingType) throws Exception {
- boolean result;
- SimpleString unPrefixedAddress = removePrefix(address);
+ public AutoCreateResult checkAutoCreate(QueueConfiguration queueConfig)
throws Exception {
+ AutoCreateResult result;
+ SimpleString unPrefixedAddress = removePrefix(queueConfig.getAddress());
AddressSettings addressSettings =
server.getAddressSettingsRepository().getMatch(unPrefixedAddress.toString());
- if (routingType == RoutingType.MULTICAST) {
- if (server.getAddressInfo(unPrefixedAddress) == null) {
- if (addressSettings.isAutoCreateAddresses()) {
+ /*
+ *
+ */
+ if (queueConfig.getRoutingType() == null) {
+ return AutoCreateResult.EXISTED;
+ }
+
+ // No matter what routingType is used the address must exist already or
be automatically created
+ AddressInfo addressInfo = server.getAddressInfo(unPrefixedAddress);
+ if (addressInfo == null) {
+ // the address doesn't exist
+ if (addressSettings.isAutoCreateAddresses() ||
queueConfig.isTemporary()) {
+ // try to create the address if possible
+ try {
+ createAddress(queueConfig.getAddress(),
queueConfig.getRoutingType(), true).setTemporary(queueConfig.isTemporary());
+ } catch (ActiveMQAddressExistsException e) {
+ // The address may have been created by another thread in the
mean time. Catch and do nothing.
+ }
+ result = AutoCreateResult.CREATED;
+ } else {
+ // if the address doesn't exist and can't be autocreated then
return false immediately
+ return AutoCreateResult.NOT_FOUND;
+ }
+ } else {
+ // the address exists
+ if
(addressInfo.getRoutingTypes().contains(queueConfig.getRoutingType())) {
+ // the existing address supports the requested routingType
+ result = AutoCreateResult.EXISTED;
+ } else {
+ // the existing address doesn't support the requested routingType
+ if (addressSettings.isAutoCreateAddresses() ||
queueConfig.isTemporary()) {
+ // try to update the address with the new routingType if
possible
try {
- createAddress(address, routingType, true);
+
createAddress(addressInfo.addRoutingType(queueConfig.getRoutingType()), true);
} catch (ActiveMQAddressExistsException e) {
// The address may have been created by another thread in
the mean time. Catch and do nothing.
}
- result = true;
+ result = AutoCreateResult.UPDATED;
} else {
- result = false;
+ // if the address exists but doesn't support the requested
routingType and can't be updated with the new routingType then return false
immediately
+ return AutoCreateResult.NOT_FOUND;
}
- } else {
- result = true;
}
- } else if (routingType == RoutingType.ANYCAST) {
+ }
+
+ if (queueConfig.getRoutingType() == RoutingType.ANYCAST) {
if (server.locateQueue(unPrefixedAddress) == null) {
- Bindings bindings =
server.getPostOffice().lookupBindingsForAddress(address);
+ // the queue doesn't exist
+ Bindings bindings =
server.getPostOffice().lookupBindingsForAddress(queueConfig.getAddress());
if (bindings != null) {
// this means the address has another queue with a different
name, which is fine, we just ignore it on this case
- result = true;
- } else if (addressSettings.isAutoCreateQueues()) {
+ result = AutoCreateResult.EXISTED;
+ } else if (addressSettings.isAutoCreateQueues() ||
queueConfig.isTemporary()) {
+ // try to create the queue
try {
- createQueue(new
QueueConfiguration(address).setRoutingType(routingType).setAutoCreated(true));
+ createQueue(queueConfig.setAutoCreated(true));
Review Comment:
manipulating the passed in QueueConfig feels a little icky. If doing so,
should the method doc that callers of it shouldnt reuse them?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
##########
@@ -1742,49 +1743,80 @@ public Transaction getCurrentTransaction() {
return tx;
}
-
@Override
- public boolean checkAutoCreate(SimpleString address, RoutingType
routingType) throws Exception {
- boolean result;
- SimpleString unPrefixedAddress = removePrefix(address);
+ public AutoCreateResult checkAutoCreate(QueueConfiguration queueConfig)
throws Exception {
+ AutoCreateResult result;
+ SimpleString unPrefixedAddress = removePrefix(queueConfig.getAddress());
AddressSettings addressSettings =
server.getAddressSettingsRepository().getMatch(unPrefixedAddress.toString());
- if (routingType == RoutingType.MULTICAST) {
- if (server.getAddressInfo(unPrefixedAddress) == null) {
- if (addressSettings.isAutoCreateAddresses()) {
+ /*
+ *
+ */
+ if (queueConfig.getRoutingType() == null) {
+ return AutoCreateResult.EXISTED;
+ }
+
+ // No matter what routingType is used the address must exist already or
be automatically created
+ AddressInfo addressInfo = server.getAddressInfo(unPrefixedAddress);
+ if (addressInfo == null) {
+ // the address doesn't exist
+ if (addressSettings.isAutoCreateAddresses() ||
queueConfig.isTemporary()) {
+ // try to create the address if possible
+ try {
+ createAddress(queueConfig.getAddress(),
queueConfig.getRoutingType(), true).setTemporary(queueConfig.isTemporary());
+ } catch (ActiveMQAddressExistsException e) {
+ // The address may have been created by another thread in the
mean time. Catch and do nothing.
+ }
+ result = AutoCreateResult.CREATED;
+ } else {
+ // if the address doesn't exist and can't be autocreated then
return false immediately
+ return AutoCreateResult.NOT_FOUND;
+ }
+ } else {
+ // the address exists
+ if
(addressInfo.getRoutingTypes().contains(queueConfig.getRoutingType())) {
+ // the existing address supports the requested routingType
+ result = AutoCreateResult.EXISTED;
+ } else {
+ // the existing address doesn't support the requested routingType
+ if (addressSettings.isAutoCreateAddresses() ||
queueConfig.isTemporary()) {
+ // try to update the address with the new routingType if
possible
try {
- createAddress(address, routingType, true);
+
createAddress(addressInfo.addRoutingType(queueConfig.getRoutingType()), true);
} catch (ActiveMQAddressExistsException e) {
// The address may have been created by another thread in
the mean time. Catch and do nothing.
}
- result = true;
+ result = AutoCreateResult.UPDATED;
} else {
- result = false;
+ // if the address exists but doesn't support the requested
routingType and can't be updated with the new routingType then return false
immediately
Review Comment:
Comment is inaccurate
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ServerSessionImpl.java:
##########
@@ -1742,49 +1743,80 @@ public Transaction getCurrentTransaction() {
return tx;
}
-
@Override
- public boolean checkAutoCreate(SimpleString address, RoutingType
routingType) throws Exception {
- boolean result;
- SimpleString unPrefixedAddress = removePrefix(address);
+ public AutoCreateResult checkAutoCreate(QueueConfiguration queueConfig)
throws Exception {
+ AutoCreateResult result;
+ SimpleString unPrefixedAddress = removePrefix(queueConfig.getAddress());
AddressSettings addressSettings =
server.getAddressSettingsRepository().getMatch(unPrefixedAddress.toString());
- if (routingType == RoutingType.MULTICAST) {
- if (server.getAddressInfo(unPrefixedAddress) == null) {
- if (addressSettings.isAutoCreateAddresses()) {
+ /*
+ *
+ */
+ if (queueConfig.getRoutingType() == null) {
+ return AutoCreateResult.EXISTED;
+ }
+
+ // No matter what routingType is used the address must exist already or
be automatically created
+ AddressInfo addressInfo = server.getAddressInfo(unPrefixedAddress);
+ if (addressInfo == null) {
+ // the address doesn't exist
+ if (addressSettings.isAutoCreateAddresses() ||
queueConfig.isTemporary()) {
+ // try to create the address if possible
+ try {
+ createAddress(queueConfig.getAddress(),
queueConfig.getRoutingType(), true).setTemporary(queueConfig.isTemporary());
+ } catch (ActiveMQAddressExistsException e) {
+ // The address may have been created by another thread in the
mean time. Catch and do nothing.
+ }
+ result = AutoCreateResult.CREATED;
+ } else {
+ // if the address doesn't exist and can't be autocreated then
return false immediately
Review Comment:
Comment is inaccurate
Issue Time Tracking
-------------------
Worklog Id: (was: 854143)
Time Spent: 0.5h (was: 20m)
> Unexpected Behavior when Routing Type of Destinations Doesn't Match Clients
> ---------------------------------------------------------------------------
>
> Key: ARTEMIS-4212
> URL: https://issues.apache.org/jira/browse/ARTEMIS-4212
> Project: ActiveMQ Artemis
> Issue Type: Improvement
> Reporter: Justin Bertram
> Assignee: Justin Bertram
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> When the routing type of an address (and associated queue) does not match the
> routing type of a client producer, the resultant behavior is a bit unexpected.
> Expected Behavior:
> If a client sends a message to an address / queue with the same name, but a
> different routing type, the expected behavior would be to throw some sort of
> InvalidDestinationException (if auto-create is not enabled), or to create the
> matching address and queue with the appropriate routing type. The routing
> count on the existing address (with non-matching routing type) should remain
> unchanged.
> Actual Behavior:
> When sending, for example, to a predefined anycast address and queue from a
> multiccast (Topic) producer, the routed count on the address is incremented,
> but the message count on the matching queue is not. No indication is given at
> the client end that the messages failed to get routed - they are silently
> dropped.
> This is reproducible using a qpid / proton queue producer to send to a
> multicast address or using a topic producer to send to an anycast address,
> e.g.:
> 1. Create a a broker, setting auto-create-queues and auto-create addresses to
> "false" for the catch-all address-setting
> 2. Start the broker and create a an address and matching queue with a ANYCAST
> routing type
> 3. Send 1000 messages to the broker using the same queue name but mismatched
> routing type:
> {code}
> ./artemis producer --url amqp://localhost:61616 --user admin --password admin
> --destination topic://{QUEUE NAME} --protocol amqp
> {code}
> No error is emitted and the routing count is incremented by 1000 at the
> address level, but remains unchanged at the destination level.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
