Preserve ${...} placeholders when removing/modifying networkConnectors in RuntimeConfigurationPlugin

activemq-runtime-config/src/main/java/org/apache/activemq/plugin/NetworkConnectorProcessor.java

@@ -16,6 +16,7 @@
  */
 package org.apache.activemq.plugin;
 
+import java.util.Properties;
 import java.util.TreeMap;
 
 import org.apache.activemq.network.DiscoveryNetworkConnector;
@@ -63,15 +64,23 @@ public void remove(Object o) {
     }
 
     private boolean configMatch(DtoNetworkConnector dto, NetworkConnector candidate) {
-        TreeMap<String, String> dtoProps = new TreeMap<String, String>();
+        Properties dtoProps = new Properties();
         IntrospectionSupport.getProperties(dto, dtoProps, null);
+        // the live candidate has its ${foo} placeholders already resolved, so resolve them on
+        // the dto side too before comparing — otherwise a connector declared with placeholders
+        // can never be matched for removal/modification
+        if (plugin.placeHolderUtil != null) {
+            plugin.placeHolderUtil.filter(dtoProps);
+        }
 
         TreeMap<String, String> candidateProps = new TreeMap<String, String>();
         IntrospectionSupport.getProperties(candidate, candidateProps, null);
 
         // every dto prop must be present in the candidate
-        for (String key : dtoProps.keySet()) {
-            if (!candidateProps.containsKey(key) || !candidateProps.get(key).equals(dtoProps.get(key))) {
+        for (Object keyObj : dtoProps.keySet()) {
+            String key = (String) keyObj;
+            String dtoValue = (String) dtoProps.get(key);
+            if (!candidateProps.containsKey(key) || !candidateProps.get(key).equals(dtoValue)) {
                 return false;
             }
         }

activemq-runtime-config/src/test/java/org/apache/activemq/SpringBeanTest.java

@@ -16,9 +16,12 @@
  */
 package org.apache.activemq;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import javax.management.ObjectName;
 import org.apache.activemq.network.DiscoveryNetworkConnector;
+import org.apache.activemq.network.NetworkConnector;
 import org.apache.activemq.plugin.RuntimeConfigurationBroker;
 import org.apache.activemq.plugin.jmx.RuntimeConfigurationViewMBean;
 import org.apache.activemq.util.IntrospectionSupport;
@@ -144,6 +147,49 @@ public void testAddPropertyRefFromFileAsList() throws Exception {
 
     }
 
+    @Test
+    public void testRemovePropertyRefNetworkConnector() throws Exception {
+
+        System.setProperty("network.uri", "static:(tcp://localhost:8888)");
+        System.setProperty("network.user", "guest");
+        System.setProperty("network.password", "secret");
+
+        final String brokerConfig = "SpringPropertyRemoveTest-broker";
+        applyNewConfig(brokerConfig, "emptyUpdatableConfig1000-spring-property-two-nc");
+        startBroker(brokerConfig);
+        assertTrue("broker alive", brokerService.isStarted());
+        assertEquals("two network connectors", 2, brokerService.getNetworkConnectors().size());
+
+        for (DiscoveryNetworkConnector nc : asDiscoveryConnectors(brokerService.getNetworkConnectors())) {
+            assertEquals("uri resolved", System.getProperty("network.uri"), nc.getUri().toASCIIString());
+            assertEquals("userName resolved", System.getProperty("network.user"), nc.getUserName());
+            assertEquals("password resolved", System.getProperty("network.password"), nc.getPassword());
+        }
+
+        applyNewConfig(brokerConfig, "emptyUpdatableConfig1000-spring-property-one-nc", SLEEP);
+
+        assertTrue("one network connector remains", Wait.waitFor(new Wait.Condition() {
+            @Override
+            public boolean isSatisified() throws Exception {
+                return 1 == brokerService.getNetworkConnectors().size();
+            }
+        }));
+
+        DiscoveryNetworkConnector remaining = (DiscoveryNetworkConnector) brokerService.getNetworkConnectors().get(0);
+        assertEquals("kept the one named 'one'", "one", remaining.getName());
+        assertEquals("uri still resolved", System.getProperty("network.uri"), remaining.getUri().toASCIIString());
+        assertEquals("userName still resolved", System.getProperty("network.user"), remaining.getUserName());
+        assertEquals("password still resolved", System.getProperty("network.password"), remaining.getPassword());
+    }
+
+    private static List<DiscoveryNetworkConnector> asDiscoveryConnectors(List<NetworkConnector> connectors) {
+        List<DiscoveryNetworkConnector> result = new ArrayList<>(connectors.size());
+        for (NetworkConnector nc : connectors) {
+            result.add((DiscoveryNetworkConnector) nc);
+        }
+        return result;
+    }
+
     @Test
     public void testAddPropertyRefFromFileAndBeanFactory() throws Exception {
 

activemq-runtime-config/src/test/resources/org/apache/activemq/emptyUpdatableConfig1000-spring-property-one-nc.xml

@@ -0,0 +1,38 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    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.
+-->
+<beans
+  xmlns="http://www.springframework.org/schema/beans"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
+  http://activemq.apache.org/schema/core http://activemq.apache.org/schema/core/activemq-core.xsd">
+
+  <bean class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer" />
+
+  <broker xmlns="http://activemq.apache.org/schema/core" start="false" persistent="false" >
+    <managementContext>
+      <managementContext createConnector="false"/>
+    </managementContext>
+    <plugins>
+      <runtimeConfigurationPlugin checkPeriod="1000" />
+    </plugins>
+
+    <networkConnectors>
+      <networkConnector uri="${network.uri}" userName="${network.user}" password="${network.password}" networkTTL="1" name="one" />
+    </networkConnectors>
+  </broker>
+</beans>

activemq-runtime-config/src/test/resources/org/apache/activemq/emptyUpdatableConfig1000-spring-property-two-nc.xml

@@ -0,0 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    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.
+-->
+<beans
+  xmlns="http://www.springframework.org/schema/beans"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd
+  http://activemq.apache.org/schema/core http://activemq.apache.org/schema/core/activemq-core.xsd">
+
+  <bean class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer" />
+
+  <broker xmlns="http://activemq.apache.org/schema/core" start="false" persistent="false" >
+    <managementContext>
+      <managementContext createConnector="false"/>
+    </managementContext>
+    <plugins>
+      <runtimeConfigurationPlugin checkPeriod="1000" />
+    </plugins>
+
+    <networkConnectors>
+      <networkConnector uri="${network.uri}" userName="${network.user}" password="${network.password}" networkTTL="1" name="one" />
+      <networkConnector uri="${network.uri}" userName="${network.user}" password="${network.password}" networkTTL="1" name="two" />
+    </networkConnectors>
+  </broker>
+</beans>