GEODE-6704: optimize getAllGatewaySenderIds (apache#3518)

* added unit tests for AbstractRegion.getAllGatewaySenderIds

* optimized AbstractRegion.allGatewaySenderIds to do no object creations

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

@@ -23,6 +23,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -190,6 +191,9 @@ public abstract class AbstractRegion implements InternalRegion, AttributesMutato
 
   private Set<String> visibleAsyncEventQueueIds;
 
+  /**
+   * This set is always unmodifiable.
+   */
   private Set<String> allGatewaySenderIds;
 
   protected boolean enableSubscriptionConflation;
@@ -703,7 +707,7 @@ Set<String> getVisibleAsyncEventQueueIds() {
 
   @Override
   public Set<String> getAllGatewaySenderIds() {
-    return Collections.unmodifiableSet(this.allGatewaySenderIds);
+    return this.allGatewaySenderIds;
   }
 
   /**
@@ -997,12 +1001,11 @@ private void setAllGatewaySenderIds() {
     if (getGatewaySenderIds().isEmpty() && getAsyncEventQueueIds().isEmpty()) {
       this.allGatewaySenderIds = Collections.emptySet(); // fix for bug 45774
     }
-    Set<String> tmp = new CopyOnWriteArraySet<String>();
-    tmp.addAll(this.getGatewaySenderIds());
+    Set<String> tmp = new HashSet<String>(this.getGatewaySenderIds());
     for (String asyncQueueId : this.getAsyncEventQueueIds()) {
       tmp.add(AsyncEventQueueImpl.getSenderIdFromAsyncEventQueueId(asyncQueueId));
     }
-    this.allGatewaySenderIds = tmp;
+    this.allGatewaySenderIds = Collections.unmodifiableSet(tmp);
   }
 
   private void initializeVisibleAsyncEventQueueIds(InternalRegionArguments internalRegionArgs) {

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

@@ -15,13 +15,26 @@
 package org.apache.geode.internal.cache;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import java.util.Set;
 
 import org.junit.Test;
 
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.DiskWriteAttributes;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.ExpirationAttributes;
 import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionAttributes;
+import org.apache.geode.cache.asyncqueue.internal.AsyncEventQueueImpl;
+import org.apache.geode.internal.cache.LocalRegion.RegionMapConstructor;
 import org.apache.geode.internal.cache.extension.ExtensionPoint;
 import org.apache.geode.internal.cache.extension.SimpleExtensionPoint;
+import org.apache.geode.test.fake.Fakes;
 
 /**
  * Unit tests for {@link AbstractRegion}.
@@ -38,10 +51,70 @@ public class AbstractRegionJUnitTest {
    * rest.
    */
   @Test
-
   public void extensionPointIsSimpleExtensionPointByDefault() {
     AbstractRegion region = spy(AbstractRegion.class);
     ExtensionPoint<Region<?, ?>> extensionPoint = region.getExtensionPoint();
     assertThat(extensionPoint).isNotNull().isInstanceOf(SimpleExtensionPoint.class);
   }
+
+  @Test
+  public void getAllGatewaySenderIdsReturnsEmptySet() {
+    AbstractRegion region = createTestableAbstractRegion();
+
+    Set<String> result = region.getAllGatewaySenderIds();
+
+    assertThat(result).isEmpty();
+  }
+
+  @Test
+  public void getAllGatewaySenderIdsIncludesAsyncEventQueueId() {
+    AbstractRegion region = createTestableAbstractRegion();
+    region.addAsyncEventQueueId("asyncQueueId", false);
+    String asyncQueueId = AsyncEventQueueImpl.getSenderIdFromAsyncEventQueueId("asyncQueueId");
+
+    Set<String> result = region.getAllGatewaySenderIds();
+
+    assertThat(result).containsExactlyInAnyOrder(asyncQueueId);
+  }
+
+  @Test
+  public void getAllGatewaySenderIdsIncludesGatewaySenderIds() {
+    AbstractRegion region = createTestableAbstractRegion();
+    region.addGatewaySenderId("gatewaySenderId");
+
+    Set<String> result = region.getAllGatewaySenderIds();
+
+    assertThat(result).containsExactlyInAnyOrder("gatewaySenderId");
+  }
+
+  @Test
+  public void getAllGatewaySenderIdsIncludesBothGatewaySenderIdsAndAsyncQueueIds() {
+    AbstractRegion region = createTestableAbstractRegion();
+    region.addGatewaySenderId("gatewaySenderId");
+    region.addAsyncEventQueueId("asyncQueueId", false);
+    String asyncQueueId = AsyncEventQueueImpl.getSenderIdFromAsyncEventQueueId("asyncQueueId");
+
+    Set<String> result = region.getAllGatewaySenderIds();
+
+    assertThat(result).containsExactlyInAnyOrder("gatewaySenderId", asyncQueueId);
+  }
+
+  private AbstractRegion createTestableAbstractRegion() {
+    RegionAttributes regionAttributes = mock(RegionAttributes.class);
+    when(regionAttributes.getDataPolicy()).thenReturn(DataPolicy.DEFAULT);
+    EvictionAttributes evictionAttributes = mock(EvictionAttributes.class);
+    when(evictionAttributes.getAction()).thenReturn(EvictionAction.NONE);
+    when(regionAttributes.getEvictionAttributes()).thenReturn(evictionAttributes);
+    ExpirationAttributes expirationAttributes = mock(ExpirationAttributes.class);
+    when(regionAttributes.getRegionTimeToLive()).thenReturn(expirationAttributes);
+    when(regionAttributes.getRegionIdleTimeout()).thenReturn(expirationAttributes);
+    when(regionAttributes.getEntryTimeToLive()).thenReturn(expirationAttributes);
+    when(regionAttributes.getEntryIdleTimeout()).thenReturn(expirationAttributes);
+    DiskWriteAttributes diskWriteAttributes = mock(DiskWriteAttributes.class);
+    when(regionAttributes.getDiskWriteAttributes()).thenReturn(diskWriteAttributes);
+    RegionMapConstructor regionMapConstructor = mock(RegionMapConstructor.class);
+    AbstractRegion region = new LocalRegion("regionName", regionAttributes, null, Fakes.cache(),
+        new InternalRegionArguments(), null, regionMapConstructor, null, null, null);
+    return region;
+  }
 }