GEODE-10106: Use local ref to queueConnection. (#7740)

* Using queueConnection local ref for multiple if checks
* As it is a volatile variable, the value may become null mid checks.

(cherry picked from commit b16dafa)

Author: nabarunnag

geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java

@@ -862,8 +862,7 @@ public void recoverPrimary(Set<ServerLocation> excludedServers) {
       return;
     }
     final boolean isDebugEnabled = logger.isDebugEnabled();
-    if (queueConnections != null && queueConnections.getPrimary() != null
-        && !queueConnections.getPrimary().isDestroyed()) {
+    if (!isPrimaryRecoveryNeeded(queueConnections)) {
       if (isDebugEnabled) {
         logger.debug("Primary recovery not needed");
       }
@@ -966,6 +965,16 @@ public void recoverPrimary(Set<ServerLocation> excludedServers) {
     }
   }
 
+  static boolean isPrimaryRecoveryNeeded(final ConnectionList queueConnectionList) {
+    if (queueConnectionList != null) {
+      final Connection primaryConnection = queueConnectionList.getPrimary();
+      if (primaryConnection != null) {
+        return primaryConnection.isDestroyed();
+      }
+    }
+    return true;
+  }
+
   private QueueConnectionImpl initializeQueueConnection(Connection connection, boolean isPrimary,
       ClientUpdater failedUpdater) {
     QueueConnectionImpl queueConnection = null;

geode-core/src/test/java/org/apache/geode/cache/client/internal/QueueManagerImplTest.java

@@ -23,13 +23,19 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.InOrder;
 
+import org.apache.geode.distributed.internal.ServerLocation;
+
 public class QueueManagerImplTest {
   private final InternalPool pool = mock(InternalPool.class, RETURNS_DEEP_STUBS);
   private final Endpoint endpoint = mock(Endpoint.class);
@@ -39,6 +45,9 @@ public class QueueManagerImplTest {
   private final ClientUpdater clientUpdater = mock(ClientUpdater.class);
   private QueueManagerImpl queueManager;
 
+  private final QueueManagerImpl.ConnectionList queueConnections = mock(
+      QueueManagerImpl.ConnectionList.class);
+
   @Before
   public void setup() {
     queueManager = new QueueManagerImpl(pool, null, null, null, 1, 1, null, null);
@@ -53,6 +62,34 @@ public void setup() {
     when(backupEndpoint.isClosed()).thenReturn(false);
   }
 
+  @Test
+  public void whenPrimaryIsNotDestroyedThenPrimaryRecoveryIsNotNeeded() {
+    assertThat(queueManager.addToConnectionList((primary), true)).isTrue();
+    when(primary.isDestroyed()).thenReturn(false);
+    when(pool.getPoolOrCacheCancelInProgress()).thenReturn(null);
+    Set<ServerLocation> excludedServers = new HashSet<>();
+    queueManager.recoverPrimary(excludedServers);
+    verify(pool, times(2)).getPoolOrCacheCancelInProgress();
+  }
+
+  @Test
+  public void whenPrimaryIsNullThenPrimaryRecoveryIsNeeded() {
+    assertThat(QueueManagerImpl.isPrimaryRecoveryNeeded(null)).isTrue();
+  }
+
+  @Test
+  public void whenConnectionListIsNullThenPrimaryRecoveryIsNeeded() {
+    when(queueConnections.getPrimary()).thenReturn(null);
+    assertThat(QueueManagerImpl.isPrimaryRecoveryNeeded(queueConnections)).isTrue();
+  }
+
+  @Test
+  public void whenPrimaryIsDestroyedThenPrimaryRecoveryIsNeeded() {
+    when(queueConnections.getPrimary()).thenReturn(primary);
+    when(primary.isDestroyed()).thenReturn(true);
+    assertThat(QueueManagerImpl.isPrimaryRecoveryNeeded(queueConnections)).isTrue();
+  }
+
   @Test
   public void addNoClientUpdaterConnectionToConnectionListReturnsFalse() {
     when(primary.getUpdater()).thenReturn(null);