You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by jb...@apache.org on 2019/06/19 16:54:32 UTC

[activemq-artemis] branch master updated: ARTEMIS-2381 making setBrokerURL public as requested by Camel Devs

This is an automated email from the ASF dual-hosted git repository.

jbertram pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git


The following commit(s) were added to refs/heads/master by this push:
     new 9a32286  ARTEMIS-2381 making setBrokerURL public as requested by Camel Devs
     new ff8db1d  This closes #2713
9a32286 is described below

commit 9a32286883c71647bb67b09e6b13fbefce75b0ea
Author: Clebert Suconic <cl...@apache.org>
AuthorDate: Tue Jun 18 10:10:52 2019 -0400

    ARTEMIS-2381 making setBrokerURL public as requested by Camel Devs
---
 .../jms/client/ActiveMQConnectionFactory.java      | 28 ++++++++---
 .../activemq/artemis/jms/client/ChangeURLTest.java | 58 ++++++++++++++++++++++
 .../unit/ra/ConnectionFactoryPropertiesTest.java   |  1 +
 3 files changed, 81 insertions(+), 6 deletions(-)

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 ac0f8eb..238f064 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 0000000..fda397f
--- /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 4f0885f..1291929 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");