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 <[email protected]>
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
['"[email protected]" <[email protected]>'].

Reply via email to