QPID-6063: [Java Broker] Address review comments from Oleksandr Rudyy <orudyy@apache.org>

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1622162 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 25 insertions, 29 deletions

diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java
index 3bef144d2d..537c89bfc7 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHARemoteReplicationNodeImpl.java
@@ -62,7 +62,6 @@ public class BDBHARemoteReplicationNodeImpl extends AbstractConfiguredObject<BDB
 
     private final AtomicReference<State> _state;
     private final boolean _isMonitor;
-    private boolean _detached;
     private BDBHAVirtualHostNodeLogSubject _virtualHostNodeLogSubject;
     private GroupLogSubject _groupLogSubject;
     private volatile NodeRole _lastKnownRole;
@@ -254,16 +253,6 @@ public class BDBHARemoteReplicationNodeImpl extends AbstractConfiguredObject<BDB
         _state.compareAndSet(currentState, isActive ? State.ACTIVE : State.UNAVAILABLE);
     }
 
-    public boolean isDetached()
-    {
-        return _detached;
-    }
-
-    public void setDetached(boolean detached)
-    {
-        _detached = detached;
-    }
-
     @Override
     public void onValidate()
     {
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
index 5b81e6eccf..dd7f99c8d4 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
@@ -118,7 +118,6 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
     @ManagedAttributeField(afterSet="postSetPriority")
     private int _priority;
 
-
     @ManagedAttributeField(afterSet="postSetQuorumOverride")
     private int _quorumOverride;
 
@@ -867,15 +866,16 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
             BDBHARemoteReplicationNodeImpl remoteNode = getChildByName(BDBHARemoteReplicationNodeImpl.class, node.getName());
             if (remoteNode != null)
             {
-                NodeRole currentRole = remoteNode.getRole();
+                final NodeRole previousRole = remoteNode.getRole();
+                final NodeRole newRole;
                 if (nodeState == null)
                 {
-                    remoteNode.setRole(NodeRole.UNREACHABLE);
+                    newRole = NodeRole.UNREACHABLE;
+                    remoteNode.setRole(newRole);
                     remoteNode.setLastTransactionId(-1);
-                    if (!remoteNode.isDetached())
+                    if (previousRole != NodeRole.UNREACHABLE)
                     {
                         getEventLogger().message(getGroupLogSubject(), HighAvailabilityMessages.LEFT(remoteNode.getName(), remoteNode.getAddress()));
-                        remoteNode.setDetached(true);
                     }
                 }
                 else
@@ -883,13 +883,15 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
                     remoteNode.setJoinTime(nodeState.getJoinTime());
                     remoteNode.setLastTransactionId(nodeState.getCurrentTxnEndVLSN());
                     ReplicatedEnvironment.State state = nodeState.getNodeState();
-                    remoteNode.setRole(NodeRole.fromJeState(state));
-                    if (remoteNode.isDetached())
+                    newRole = NodeRole.fromJeState(state);
+                    remoteNode.setRole(newRole);
+
+                    if (previousRole == NodeRole.UNREACHABLE)
                     {
                         getEventLogger().message(getGroupLogSubject(), HighAvailabilityMessages.JOINED(remoteNode.getName(), remoteNode.getAddress()));
-                        remoteNode.setDetached(false);
                     }
-                    if (ReplicatedEnvironment.State.MASTER.name().equals(remoteNode.getRole()))
+
+                    if (NodeRole.MASTER == newRole)
                     {
                         byte[] applicationState = nodeState.getAppState();
                         Set<String> permittedNodes = ReplicatedEnvironmentFacade.convertApplicationStateBytesToPermittedNodeList(applicationState);
@@ -908,10 +910,12 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
                     }
                 }
 
-                NodeRole newRole = remoteNode.getRole();
-                if (newRole != currentRole)
+                if (newRole != previousRole)
                 {
-                    getEventLogger().message(getGroupLogSubject(), HighAvailabilityMessages.ROLE_CHANGED(remoteNode.getName(), remoteNode.getAddress(), currentRole.name(), newRole.name()));
+                    getEventLogger().message(getGroupLogSubject(), HighAvailabilityMessages.ROLE_CHANGED(remoteNode.getName(),
+                                                                                                         remoteNode.getAddress(),
+                                                                                                         previousRole.name(),
+                                                                                                         newRole.name()));
                 }
             }
         }
diff --git a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java
index 31af574ee5..7cfc28ffb2 100644
--- a/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java
+++ b/qpid/java/bdbstore/src/test/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeOperationalLoggingTest.java
@@ -24,6 +24,7 @@ import static org.mockito.Matchers.argThat;
 import static org.mockito.Mockito.*;
 
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.Map;
 
 import org.apache.qpid.server.logging.EventLogger;
@@ -264,7 +265,7 @@ public class BDBHAVirtualHostNodeOperationalLoggingTest extends QpidTestCase
         // close remote node
         node2.close();
 
-        waitForNodeDetachedField(remoteNode, true);
+        waitForRemoteNodeToAttainRole(remoteNode, EnumSet.of(NodeRole.DETACHED));
 
         // verify that remaining node issues the DETACHED operational logging for remote node
         String expectedMessage = HighAvailabilityMessages.LEFT(node2.getName(), node2.getAddress()).toString();
@@ -296,23 +297,25 @@ public class BDBHAVirtualHostNodeOperationalLoggingTest extends QpidTestCase
 
         node2.close();
 
-        waitForNodeDetachedField(remoteNode, true);
+        waitForRemoteNodeToAttainRole(remoteNode, EnumSet.of(NodeRole.DETACHED));
 
         reset(_eventLogger);
 
-        node2Attributes.put(BDBHAVirtualHostNode.PERMITTED_NODES, node1Attributes.get(BDBHAVirtualHostNode.PERMITTED_NODES));
+        node2Attributes.put(BDBHAVirtualHostNode.PERMITTED_NODES,
+                            node1Attributes.get(BDBHAVirtualHostNode.PERMITTED_NODES));
         node2 = (BDBHAVirtualHostNodeImpl)_helper.recoverHaVHN(node2.getId(), node2Attributes);
         _helper.assertNodeRole(node2, NodeRole.REPLICA, NodeRole.MASTER);
-        waitForNodeDetachedField(remoteNode, false);
+        waitForRemoteNodeToAttainRole(remoteNode, EnumSet.of(NodeRole.REPLICA, NodeRole.MASTER));
 
         final String expectedMessage = HighAvailabilityMessages.JOINED(node2.getName(), node2.getAddress()).toString();
         verify(_eventLogger).message(argThat(new LogSubjectMatcher(node1.getGroupLogSubject())),
                 argThat(new LogMessageMatcher(expectedMessage, HighAvailabilityMessages.JOINED_LOG_HIERARCHY)));
     }
 
-    private void waitForNodeDetachedField(BDBHARemoteReplicationNodeImpl remoteNode, boolean expectedDetached) throws InterruptedException {
+    private void waitForRemoteNodeToAttainRole(BDBHARemoteReplicationNode remoteNode, EnumSet<NodeRole> desiredRoles) throws Exception
+    {
         int counter = 0;
-        while (expectedDetached != remoteNode.isDetached() && counter<50)
+        while (!desiredRoles.contains(remoteNode.getRole())  && counter<50)
         {
             Thread.sleep(100);
             counter++;