diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java index 4474b6ba8821..e16750f82b58 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java +++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueManagerImpl.java @@ -862,8 +862,7 @@ public void recoverPrimary(Set 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 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; diff --git a/geode-core/src/test/java/org/apache/geode/cache/client/internal/QueueManagerImplTest.java b/geode-core/src/test/java/org/apache/geode/cache/client/internal/QueueManagerImplTest.java index 264a83417132..75cab9697860 100644 --- a/geode-core/src/test/java/org/apache/geode/cache/client/internal/QueueManagerImplTest.java +++ b/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 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);