This is an automated email from the ASF dual-hosted git repository.

onichols pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.12 by this push:
     new b16851e  GEODE-7664: calling RegionConfigRealizer.exists methods 
doesn't need … (#4932) (#6569)
b16851e is described below

commit b16851ec28a80c5c6ee2eac9bb586df78f832882
Author: Jens Deppe <[email protected]>
AuthorDate: Tue Jun 8 09:28:00 2021 -0700

    GEODE-7664: calling RegionConfigRealizer.exists methods doesn't need … 
(#4932) (#6569)
    
    (cherry picked from commit 63c681d217bdcc3d6ed0150f977f18461be1a785)
    
    Co-authored-by: Jinmei Liao <[email protected]>
---
 .../realizers/RegionConfigRealizer.java            | 18 ++++++++++++++-
 .../realizers/RegionConfigRealizerTest.java        | 27 ++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizer.java
 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizer.java
index cf10bc5..aeffdaf 100644
--- 
a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizer.java
+++ 
b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizer.java
@@ -308,6 +308,22 @@ public class RegionConfigRealizer
     return info;
   }
 
+  /**
+   * the default implementation will have the extra work of getting the 
runtime info which is
+   * unnecessary. It will also have some concurrency issue if the region is 
being destroyed.
+   */
+  @Override
+  public boolean exists(Region config, InternalCache cache) {
+    org.apache.geode.cache.Region<Object, Object> region = cache.getRegion("/" 
+ config.getName());
+    if (region == null) {
+      return false;
+    }
+
+    if (region.isDestroyed()) {
+      return false;
+    }
+    return true;
+  }
 
   @Override
   public RealizationResult update(Region config, InternalCache cache) {
@@ -325,7 +341,7 @@ public class RegionConfigRealizer
     try {
       region.destroyRegion();
     } catch (RegionDestroyedException dex) {
-      // Probably happened as a distirbuted op but it still reflects our 
current desired action
+      // Probably happened as a distributed op but it still reflects our 
current desired action
       // which is why it can be ignored here.
       return new RealizationResult().setMessage("Region does not exist.");
     }
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
index d6953f4..a99cc1b 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/configuration/realizers/RegionConfigRealizerTest.java
@@ -42,6 +42,7 @@ public class RegionConfigRealizerTest {
   RegionConfigRealizer realizer;
   RegionConfigValidator validator;
   Region config;
+  org.apache.geode.cache.Region region;
 
   @Before
   public void setup() {
@@ -52,6 +53,7 @@ public class RegionConfigRealizerTest {
     realizer = new RegionConfigRealizer();
     config = new Region();
     config.setName("test");
+    region = mock(org.apache.geode.cache.Region.class);
   }
 
   @Test
@@ -113,4 +115,29 @@ public class RegionConfigRealizerTest {
     verify(regionFactory, never()).setDiskStoreName(any());
     verify(regionFactory).setDataPolicy(DataPolicy.REPLICATE);
   }
+
+  @Test
+  public void regionDoesNotExistIfNotInCache() {
+    config.setName("test");
+    when(cache.getRegion("/test")).thenReturn(null);
+    assertThat(realizer.exists(config, cache)).isFalse();
+  }
+
+
+  @Test
+  public void regionDoesNotExistIfDestroyed() {
+    when(cache.getRegion("/test")).thenReturn(region);
+    when(region.isDestroyed()).thenReturn(true);
+    assertThat(realizer.exists(config, cache)).isFalse();
+  }
+
+  @Test
+  public void regionExistsDoesNotGetRuntimeInfo() {
+    config.setName("test");
+    when(cache.getRegion("/test")).thenReturn(region);
+    when(region.isDestroyed()).thenReturn(false);
+    boolean exists = realizer.exists(config, cache);
+    assertThat(exists).isTrue();
+    verify(region, never()).size();
+  }
 }

Reply via email to