diff --git a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java index ac0f8ebf8c..238f064413 100644 --- a/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java +++ b/artemis-jms-client/src/main/java/org/apache/activemq/artemis/jms/client/ActiveMQConnectionFactory.java @@ -216,10 +216,24 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio } public ActiveMQConnectionFactory(String brokerURL) { - setBrokerURL(brokerURL); + try { + setBrokerURL(brokerURL); + } catch (Throwable e) { + throw new IllegalStateException(e); + } } - private void setBrokerURL(String brokerURL) { + /** Warning: This method will not clear any previous properties. + * Say, you set the user on a first call. + * Now you just change the brokerURI on a second call without passing the user. + * The previous filled user will be already set, and nothing will clear it out. + * + * Also: you cannot use this method after the connection factory is made readOnly. + * Which happens after you create a first connection. */ + public void setBrokerURL(String brokerURL) throws JMSException { + if (readOnly) { + throw new javax.jms.IllegalStateException("You cannot use setBrokerURL after the connection factory has been used"); + } ConnectionFactoryParser cfParser = new ConnectionFactoryParser(); try { URI uri = cfParser.expandURI(brokerURL); @@ -398,15 +412,17 @@ public class ActiveMQConnectionFactory extends JNDIStorable implements Connectio if (url == null || url.isEmpty()) { url = props.getProperty("brokerURL"); } - if (url != null && url.length() > 0) { - setBrokerURL(url); - } + if (url == null || url.isEmpty()) { throw new IllegalArgumentException(Context.PROVIDER_URL + " or " + "brokerURL is required"); } try { + if (url != null && url.length() > 0) { + setBrokerURL(url); + } + BeanSupport.setProperties(this, props); - } catch (IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { + } catch (JMSException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) { throw new RuntimeException(e); } } diff --git a/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jms/client/ChangeURLTest.java b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jms/client/ChangeURLTest.java new file mode 100644 index 0000000000..fda397f36c --- /dev/null +++ b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/jms/client/ChangeURLTest.java @@ -0,0 +1,58 @@ +/* + * 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.jms.client; + +import javax.jms.JMSException; + +import org.junit.Assert; +import org.junit.Test; + +public class ChangeURLTest { + + @Test + public void testChangeURL() throws Exception { + ActiveMQConnectionFactory factory = new ActiveMQConnectionFactory("tcp://localhost:61616?user=nono"); + Assert.assertEquals("nono", factory.getUser()); + + factory.setBrokerURL("tcp://localhost:61616?user=changed"); + Assert.assertEquals("changed", factory.getUser()); + + boolean failed = false; + try { + factory.createConnection(); + } catch (Throwable expected) { + // there is no broker running, this is expected and somewhat required to fail, however readOnly should be set to true here + // so the next assertion is the important one. + // The goal here is to make connectionFactory.readOnly=true + failed = true; + } + + Assert.assertTrue("failure expected", failed); + + failed = false; + + try { + factory.setBrokerURL("tcp://localhost:61618?user=changed"); + } catch (JMSException ex) { + failed = true; + } + + Assert.assertTrue("failure expected", failed); + } + +} diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ConnectionFactoryPropertiesTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ConnectionFactoryPropertiesTest.java index 4f0885f5ba..1291929967 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ConnectionFactoryPropertiesTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ConnectionFactoryPropertiesTest.java @@ -42,6 +42,7 @@ public class ConnectionFactoryPropertiesTest extends ActiveMQTestBase { static { UNSUPPORTED_CF_PROPERTIES = new TreeSet<>(); UNSUPPORTED_CF_PROPERTIES.add("discoveryGroupName"); + UNSUPPORTED_CF_PROPERTIES.add("brokerURL"); UNSUPPORTED_CF_PROPERTIES.add("incomingInterceptorList"); UNSUPPORTED_CF_PROPERTIES.add("outgoingInterceptorList"); UNSUPPORTED_CF_PROPERTIES.add("user");