You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2017/11/17 15:08:52 UTC

[geode] branch develop updated: GEODE-1897: Ensure that eviction-object-sizer also implements Declarable (#1068)

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

jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new c1db889  GEODE-1897: Ensure that eviction-object-sizer also implements Declarable (#1068)
c1db889 is described below

commit c1db88961a33a12dbad86962262e9a4201a787e5
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Fri Nov 17 07:08:50 2017 -0800

    GEODE-1897: Ensure that eviction-object-sizer also implements Declarable (#1068)
    
    - Minor acceptance test updates
---
 .../internal/cli/commands/CreateRegionCommand.java | 12 +++------
 .../cli/functions/RegionCreateFunction.java        |  7 ++++++
 .../management/internal/cli/i18n/CliStrings.java   |  5 +++-
 .../cli/commands/CreateRegionCommandDUnitTest.java | 18 +++++++-------
 .../CreateRegionCommandIntegrationTest.java        | 29 +++++++++++++++++++++-
 .../cli/commands/CreateRegionCommandTest.java      |  9 -------
 .../cli/commands/TestObjectSizerNotDeclarable.java | 25 +++++++++++++++++++
 7 files changed, 76 insertions(+), 29 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
index a4b2f72..5eeed3f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommand.java
@@ -678,15 +678,9 @@ public class CreateRegionCommand implements GfshCommand {
             .createUserErrorResult(CliStrings.CREATE_REGION__MSG__MISSING_EVICTION_ACTION);
       }
 
-      if (evictionSizer != null) {
-        if (maxEntry != null) {
-          return ResultBuilder.createUserErrorResult(
-              CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_AND_ENTRY_COUNT);
-        }
-        if (maxMemory == null) {
-          return ResultBuilder.createUserErrorResult(
-              CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_WITHOUT_MAX_MEMORY);
-        }
+      if (evictionSizer != null && maxEntry != null) {
+        return ResultBuilder.createUserErrorResult(
+            CliStrings.CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_AND_ENTRY_COUNT);
       }
 
       if (evictionAction != null
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
index 55344d4..1c7baef 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/RegionCreateFunction.java
@@ -24,6 +24,7 @@ import org.apache.geode.cache.CacheListener;
 import org.apache.geode.cache.CacheLoader;
 import org.apache.geode.cache.CacheWriter;
 import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.Declarable;
 import org.apache.geode.cache.EvictionAttributes;
 import org.apache.geode.cache.PartitionAttributes;
 import org.apache.geode.cache.PartitionAttributesFactory;
@@ -36,6 +37,7 @@ import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.cache.execute.Function;
 import org.apache.geode.cache.execute.FunctionContext;
 import org.apache.geode.cache.execute.ResultSender;
+import org.apache.geode.cache.util.ObjectSizer;
 import org.apache.geode.compression.Compressor;
 import org.apache.geode.internal.ClassPathLoader;
 import org.apache.geode.internal.InternalEntity;
@@ -202,6 +204,11 @@ public class RegionCreateFunction implements Function, InternalEntity {
 
     EvictionAttributes evictionAttributes = regionCreateArgs.getEvictionAttributes();
     if (evictionAttributes != null) {
+      ObjectSizer sizer = evictionAttributes.getObjectSizer();
+      if (sizer != null && !(sizer instanceof Declarable)) {
+        throw new IllegalArgumentException(
+            CliStrings.CREATE_REGION__MSG__OBJECT_SIZER_MUST_BE_OBJECTSIZER_AND_DECLARABLE);
+      }
       factory.setEvictionAttributes(evictionAttributes);
     }
 
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 0e27fba..cf99d0f 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -869,7 +869,7 @@ public class CliStrings {
       "Activates LRU eviction based on the region's memory usage specified by this value.";
   public static final String CREATE_REGION__EVICTION_ENTRY_COUNT = "eviction-entry-count";
   public static final String CREATE_REGION__EVICTION_ENTRY_COUNT__HELP =
-      "Activates LRU eviction based on the region's entry count specified by this value.";
+      "Activates LRU eviction based on the region's entry count specified by this value (in megabytes).";
   public static final String CREATE_REGION__EVICTION_OBJECT_SIZER = "eviction-object-sizer";
   public static final String CREATE_REGION__EVICTION_OBJECT_SIZER__HELP =
       "A custom class which implements ObjectSizer in order to perform max memory eviction.";
@@ -1068,6 +1068,9 @@ public class CliStrings {
   public static final String CREATE_REGION__MSG__INVALID_EVICTION_OBJECT_SIZER_WITHOUT_MAX_MEMORY =
       "eviction-object-sizer cannot be specified without eviction-max-memory";
 
+  public static final String CREATE_REGION__MSG__OBJECT_SIZER_MUST_BE_OBJECTSIZER_AND_DECLARABLE =
+      "eviction-object-sizer must implement both ObjectSizer and Declarable interfaces";
+
   /* debug command */
   public static final String DEBUG = "debug";
   public static final String DEBUG__HELP = "Enable/Disable debugging output in GFSH.";
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
index b9f6513..01b5a7a 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandDUnitTest.java
@@ -19,7 +19,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import java.io.File;
 
-import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
@@ -42,14 +43,13 @@ import org.apache.geode.test.junit.rules.serializable.SerializableTestName;
 @Category(DistributedTest.class)
 public class CreateRegionCommandDUnitTest {
 
-  MemberVM locator;
-  MemberVM server;
+  private static MemberVM locator, server;
 
-  @Rule
-  public LocatorServerStartupRule lsRule = new LocatorServerStartupRule();
+  @ClassRule
+  public static LocatorServerStartupRule lsRule = new LocatorServerStartupRule();
 
-  @Rule
-  public GfshCommandRule gfsh = new GfshCommandRule();
+  @ClassRule
+  public static GfshCommandRule gfsh = new GfshCommandRule();
 
   @Rule
   public TestName testName = new SerializableTestName();
@@ -57,8 +57,8 @@ public class CreateRegionCommandDUnitTest {
   @Rule
   public TemporaryFolder tmpDir = new TemporaryFolder();
 
-  @Before
-  public void before() throws Exception {
+  @BeforeClass
+  public static void before() throws Exception {
     locator = lsRule.startLocatorVM(0);
     server = lsRule.startServerVM(1, locator.getPort());
 
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
index 28dffab..de80f75 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandIntegrationTest.java
@@ -510,6 +510,24 @@ public class CreateRegionCommandIntegrationTest {
   }
 
   @Test
+  public void testEvictionAttributesForLRUHeapWithObjectSizer() throws Exception {
+    gfsh.executeAndAssertThat(
+        "create region --name=FOO --type=REPLICATE --eviction-action=local-destroy --eviction-object-sizer="
+            + TestObjectSizer.class.getName())
+        .statusIsSuccess();
+
+    Region foo = server.getCache().getRegion("/FOO");
+    assertThat(foo.getAttributes().getEvictionAttributes().getAction())
+        .isEqualTo(EvictionAction.LOCAL_DESTROY);
+    assertThat(foo.getAttributes().getEvictionAttributes().getAlgorithm())
+        .isEqualTo(EvictionAlgorithm.LRU_HEAP);
+    assertThat(foo.getAttributes().getEvictionAttributes().getObjectSizer().getClass().getName())
+        .isEqualTo(TestObjectSizer.class.getName());
+
+    gfsh.executeAndAssertThat("destroy region --name=/FOO").statusIsSuccess();
+  }
+
+  @Test
   public void testEvictionAttributesForLRUEntry() throws Exception {
     gfsh.executeAndAssertThat(
         "create region --name=FOO --type=REPLICATE --eviction-entry-count=1001 --eviction-action=overflow-to-disk")
@@ -542,7 +560,7 @@ public class CreateRegionCommandIntegrationTest {
   }
 
   @Test
-  public void testEvictionAttributesForSizer() throws Exception {
+  public void testEvictionAttributesForObjectSizer() throws Exception {
     gfsh.executeAndAssertThat(
         "create region --name=FOO --type=REPLICATE --eviction-max-memory=1001 --eviction-action=overflow-to-disk --eviction-object-sizer="
             + TestObjectSizer.class.getName())
@@ -558,4 +576,13 @@ public class CreateRegionCommandIntegrationTest {
 
     gfsh.executeAndAssertThat("destroy region --name=/FOO").statusIsSuccess();
   }
+
+  @Test
+  public void testEvictionAttributesForNonDeclarableObjectSizer() throws Exception {
+    gfsh.executeAndAssertThat(
+        "create region --name=FOO --type=REPLICATE --eviction-max-memory=1001 --eviction-action=overflow-to-disk --eviction-object-sizer="
+            + TestObjectSizerNotDeclarable.class.getName())
+        .statusIsError().containsOutput(
+            "eviction-object-sizer must implement both ObjectSizer and Declarable interfaces");
+  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
index d320386..4b360d4 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/CreateRegionCommandTest.java
@@ -125,15 +125,6 @@ public class CreateRegionCommandTest {
   }
 
   @Test
-  public void invalidEvictionSizerWithoutMemory() throws Exception {
-    CommandResult result = parser.executeCommandWithInstance(command,
-        "create region --name=region --type=REPLICATE --eviction-object-sizer=abc --eviction-action=local-destroy");
-    assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
-    assertThat(result.getContent().toString())
-        .contains("eviction-object-sizer cannot be specified without eviction-max-memory");
-  }
-
-  @Test
   public void templateRegionAttributesNotAvailable() throws Exception {
     doReturn(null).when(command).getRegionAttributes(eq(cache), any());
     doReturn(Collections.emptySet()).when(command).findMembers(any(), any());
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java
new file mode 100644
index 0000000..b16be55
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/TestObjectSizerNotDeclarable.java
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.cli.commands;
+
+import org.apache.geode.cache.util.ObjectSizer;
+
+public class TestObjectSizerNotDeclarable implements ObjectSizer {
+  @Override
+  public int sizeof(Object o) {
+    return 77;
+  }
+}

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].