svn commit: r1408415 - in /activemq/trunk: activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java

[ta...@apache.org]

Author: tabish
Date: Mon Nov 12 19:39:14 2012
New Revision: 1408415

URL: http://svn.apache.org/viewvc?rev=1408415&view=rev
Log:
fixes for: https://issues.apache.org/jira/browse/AMQ-4160

Fixed the code to properly handle the active events adds and remove. 
Disable one of the test cases because the fixed code now invalidates that test case. 

Modified:
    activemq/trunk/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java
    activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java

Modified: activemq/trunk/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java?rev=1408415&r1=1408414&r2=1408415&view=diff
==============================================================================
--- activemq/trunk/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java (original)
+++ activemq/trunk/activemq-broker/src/main/java/org/apache/activemq/network/DiscoveryNetworkConnector.java Mon Nov 12 19:39:14 2012
@@ -53,7 +53,7 @@ public class DiscoveryNetworkConnector e
 
     private DiscoveryAgent discoveryAgent;
     private Map<String, String> parameters;
-    private ConcurrentMap<URI, DiscoveryEvent> activeEvents = new ConcurrentHashMap<URI, DiscoveryEvent>();
+    private final ConcurrentMap<URI, DiscoveryEvent> activeEvents = new ConcurrentHashMap<URI, DiscoveryEvent>();
     public DiscoveryNetworkConnector() {
     }
 
@@ -72,6 +72,7 @@ public class DiscoveryNetworkConnector e
         }
     }
 
+    @Override
     public void onServiceAdd(DiscoveryEvent event) {
         // Ignore events once we start stopping.
         if (serviceSupport.isStopped() || serviceSupport.isStopping()) {
@@ -103,7 +104,10 @@ public class DiscoveryNetworkConnector e
 
             // Should we try to connect to that URI?
             if (activeEvents.putIfAbsent(uri, event) != null) {
-                LOG.debug("Discovery agent generated a duplicate onServiceAdd event for: "+uri );
+                if (LOG.isDebugEnabled()) {
+                    LOG.debug("Discovery agent generated a duplicate onServiceAdd event for: " + uri);
+                }
+                return;
             }
 
             URI connectUri = uri;
@@ -129,7 +133,7 @@ public class DiscoveryNetworkConnector e
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("Connection failure exception: " + e, e);
                     }
-                    activeEvents.remove(url);
+                    activeEvents.remove(uri);
                     return;
                 }
                 try {
@@ -140,7 +144,7 @@ public class DiscoveryNetworkConnector e
                     if (LOG.isDebugEnabled()) {
                         LOG.debug("Connection failure exception: " + e, e);
                     }
-                    activeEvents.remove(url);
+                    activeEvents.remove(uri);
                     return;
                 }
             } finally {
@@ -171,6 +175,7 @@ public class DiscoveryNetworkConnector e
         }
     }
 
+    @Override
     public void onServiceRemove(DiscoveryEvent event) {
         String url = event.getServiceName();
         if (url != null) {
@@ -183,7 +188,7 @@ public class DiscoveryNetworkConnector e
             }
 
             // Only remove bridge if this is the active discovery event for the URL.
-            if (activeEvents.remove(url, event)) {
+            if (activeEvents.remove(uri, event)) {
                 synchronized (bridges) {
                     bridges.remove(uri);
                 }
@@ -202,6 +207,7 @@ public class DiscoveryNetworkConnector e
         }
     }
 
+    @Override
     protected void handleStart() throws Exception {
         if (discoveryAgent == null) {
             throw new IllegalStateException("You must configure the 'discoveryAgent' property");
@@ -210,6 +216,7 @@ public class DiscoveryNetworkConnector e
         super.handleStart();
     }
 
+    @Override
     protected void handleStop(ServiceStopper stopper) throws Exception {
         for (Iterator<NetworkBridge> i = bridges.values().iterator(); i.hasNext();) {
             NetworkBridge bridge = i.next();
@@ -236,6 +243,7 @@ public class DiscoveryNetworkConnector e
                 super(brokerService, connectorName);
             }
 
+            @Override
             public void bridgeFailed() {
                 if (!serviceSupport.isStopped()) {
                     try {

Modified: activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java
URL: http://svn.apache.org/viewvc/activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java?rev=1408415&r1=1408414&r2=1408415&view=diff
==============================================================================
--- activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java (original)
+++ activemq/trunk/activemq-core/src/test/java/org/apache/activemq/bugs/AMQ4160Test.java Mon Nov 12 19:39:14 2012
@@ -57,7 +57,7 @@ public class AMQ4160Test extends JmsMult
      * {@link DiscoveryNetworkConnector#activeBridges()} that represents the
      * successful first bridge creation attempt.
      */
-    public void testLostActiveBridge() throws Exception {
+    public void x_testLostActiveBridge() throws Exception {
         // Start two brokers with a bridge from broker1 to broker2.
         BrokerService broker1 = createBroker(new URI(
                 "broker:(vm://broker1)/broker1?persistent=false"));