[geode] branch develop updated: GEODE-4696: Refactored validateCommandParameters (#1535)

[nn...@apache.org]

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

nnag 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 f2997f7  GEODE-4696: Refactored validateCommandParameters (#1535)
f2997f7 is described below

commit f2997f7e9072f4eeceab4831c374fd01e4813371
Author: Nabarun Nag <na...@users.noreply.github.com>
AuthorDate: Mon Mar 5 15:01:02 2018 -0800

    GEODE-4696: Refactored validateCommandParameters (#1535)
    
            * Enum completedly moved to a new class
    	* Changed the name to CreateLuceneCommandParametersValidator
    	* Additional tests added to validate the Lucene index name.
---
 .../CreateLuceneCommandParametersValidator.java    | 53 +++++++++++++++++++
 .../cache/lucene/internal/LuceneServiceImpl.java   | 39 --------------
 .../cli/functions/LuceneCreateIndexFunction.java   |  8 +--
 .../sanctioned-geode-lucene-serializables.txt      |  1 -
 .../internal/ValidateCommandParametersTest.java    | 61 ++++++++++++++++++----
 5 files changed, 109 insertions(+), 53 deletions(-)

diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/CreateLuceneCommandParametersValidator.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/CreateLuceneCommandParametersValidator.java
new file mode 100644
index 0000000..af0fe51
--- /dev/null
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/CreateLuceneCommandParametersValidator.java
@@ -0,0 +1,53 @@
+/*
+ * 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.cache.lucene.internal;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.geode.internal.i18n.LocalizedStrings;
+
+public class CreateLuceneCommandParametersValidator {
+  public static void validateRegionName(String name) {
+    validateNameNotEmptyOrNull(name);
+    String msg =
+        "Region names may only be alphanumeric, must not begin with double-underscores, but can contain hyphens, underscores, or forward slashes: ";
+    Matcher matcher = Pattern.compile("[aA-zZ0-9-_./]+").matcher(name);
+    if (name.startsWith("__") || name.startsWith("/__") || !matcher.matches()) {
+      throw new IllegalArgumentException(msg + name);
+    }
+  }
+
+  public static void validateLuceneIndexName(String name) {
+    validateNameNotEmptyOrNull(name);
+    String msg =
+        "Index names may only be alphanumeric, must not begin with double-underscores, but can contain hyphens or underscores: ";
+    Matcher matcher = Pattern.compile("[aA-zZ0-9-_.]+").matcher(name);
+    if (name.startsWith("__") || !matcher.matches()) {
+      throw new IllegalArgumentException(msg + name);
+    }
+  }
+
+  private static void validateNameNotEmptyOrNull(String nameProvided) {
+    if (nameProvided == null) {
+      throw new IllegalArgumentException(
+          LocalizedStrings.LocalRegion_NAME_CANNOT_BE_NULL.toLocalizedString());
+    }
+    if (nameProvided.isEmpty()) {
+      throw new IllegalArgumentException(
+          LocalizedStrings.LocalRegion_NAME_CANNOT_BE_EMPTY.toLocalizedString());
+    }
+  }
+}
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
index 2ac0df4..02f477a 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
@@ -175,45 +175,6 @@ public class LuceneServiceImpl implements InternalLuceneService {
     return getUniqueIndexName(indexName, regionPath) + regionSuffix;
   }
 
-  public enum validateCommandParameters {
-    REGION_PATH, INDEX_NAME;
-
-    public void validateName(String name) {
-      if (name == null) {
-        throw new IllegalArgumentException(
-            LocalizedStrings.LocalRegion_NAME_CANNOT_BE_NULL.toLocalizedString());
-      }
-      if (name.isEmpty()) {
-        throw new IllegalArgumentException(
-            LocalizedStrings.LocalRegion_NAME_CANNOT_BE_EMPTY.toLocalizedString());
-      }
-
-      boolean iae = false;
-      String msg =
-          " names may only be alphanumeric, must not begin with double-underscores, but can contain hyphens";
-      Matcher matcher = null;
-      switch (this) {
-        case REGION_PATH:
-          matcher = Pattern.compile("[aA-zZ0-9-_./]+").matcher(name);
-          msg = "Region" + msg + ", underscores, or forward slashes: ";
-          iae = name.startsWith("__") || name.startsWith("/__") || !matcher.matches();
-          break;
-        case INDEX_NAME:
-          matcher = Pattern.compile("[aA-zZ0-9-_.]+").matcher(name);
-          msg = "Index" + msg + " or underscores: ";
-          iae = name.startsWith("__") || !matcher.matches();
-          break;
-        default:
-          throw new IllegalArgumentException("Illegal option for validateName function");
-      }
-
-      // Ensure the region only contains valid characters
-      if (iae) {
-        throw new IllegalArgumentException(msg + name);
-      }
-    }
-  }
-
   public void createIndex(String indexName, String regionPath, Map<String, Analyzer> fieldAnalyzers,
       LuceneSerializer serializer, boolean allowOnExistingRegion) {
     if (fieldAnalyzers == null || fieldAnalyzers.isEmpty()) {
diff --git a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java
index a2e2f32..5d2bd3f 100644
--- a/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java
+++ b/geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/cli/functions/LuceneCreateIndexFunction.java
@@ -15,8 +15,8 @@
 
 package org.apache.geode.cache.lucene.internal.cli.functions;
 
-import static org.apache.geode.cache.lucene.internal.LuceneServiceImpl.validateCommandParameters.INDEX_NAME;
-import static org.apache.geode.cache.lucene.internal.LuceneServiceImpl.validateCommandParameters.REGION_PATH;
+import static org.apache.geode.cache.lucene.internal.CreateLuceneCommandParametersValidator.validateLuceneIndexName;
+import static org.apache.geode.cache.lucene.internal.CreateLuceneCommandParametersValidator.validateRegionName;
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.lucene.analysis.Analyzer;
@@ -73,7 +73,7 @@ public class LuceneCreateIndexFunction implements InternalFunction {
       memberId = cache.getDistributedSystem().getDistributedMember().getId();
       LuceneService service = LuceneServiceProvider.get(cache);
 
-      INDEX_NAME.validateName(indexName);
+      validateLuceneIndexName(indexName);
 
       String[] fields = indexInfo.getSearchableFieldNames();
       String[] analyzerName = indexInfo.getFieldAnalyzers();
@@ -99,7 +99,7 @@ public class LuceneCreateIndexFunction implements InternalFunction {
       }
 
       XmlEntity xmlEntity = null;
-      REGION_PATH.validateName(regionPath);
+      validateRegionName(regionPath);
       if (LuceneServiceImpl.LUCENE_REINDEX) {
         indexFactory.create(indexName, regionPath, true);
         if (cache.getRegion(regionPath) != null) {
diff --git a/geode-lucene/src/main/resources/org/apache/geode/internal/sanctioned-geode-lucene-serializables.txt b/geode-lucene/src/main/resources/org/apache/geode/internal/sanctioned-geode-lucene-serializables.txt
index a15e2d8..a13c06b 100755
--- a/geode-lucene/src/main/resources/org/apache/geode/internal/sanctioned-geode-lucene-serializables.txt
+++ b/geode-lucene/src/main/resources/org/apache/geode/internal/sanctioned-geode-lucene-serializables.txt
@@ -2,7 +2,6 @@ org/apache/geode/cache/lucene/LuceneIndexDestroyedException,false,indexName:java
 org/apache/geode/cache/lucene/LuceneIndexExistsException,false,indexName:java/lang/String,regionPath:java/lang/String
 org/apache/geode/cache/lucene/LuceneIndexNotFoundException,false,indexName:java/lang/String,regionPath:java/lang/String
 org/apache/geode/cache/lucene/LuceneQueryException,false
-org/apache/geode/cache/lucene/internal/LuceneServiceImpl$validateCommandParameters,false
 org/apache/geode/cache/lucene/internal/cli/LuceneDestroyIndexInfo,false,definedDestroyOnly:boolean
 org/apache/geode/cache/lucene/internal/cli/LuceneFunctionSerializable,false,indexName:java/lang/String,regionPath:java/lang/String
 org/apache/geode/cache/lucene/internal/cli/LuceneIndexDetails,true,1,fieldAnalyzers:java/util/Map,indexStats:java/util/Map,initialized:boolean,searchableFieldNames:java/lang/String[],serializer:java/lang/String,serverName:java/lang/String
diff --git a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/ValidateCommandParametersTest.java b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/ValidateCommandParametersTest.java
index 958c72f..fd644c3 100644
--- a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/ValidateCommandParametersTest.java
+++ b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/ValidateCommandParametersTest.java
@@ -15,29 +15,72 @@
 
 package org.apache.geode.cache.lucene.internal;
 
+import static org.apache.geode.cache.lucene.internal.CreateLuceneCommandParametersValidator.validateLuceneIndexName;
+import static org.apache.geode.cache.lucene.internal.CreateLuceneCommandParametersValidator.validateRegionName;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import org.apache.geode.cache.lucene.internal.LuceneServiceImpl.validateCommandParameters;
 import org.apache.geode.test.junit.categories.UnitTest;
 
 @Category(UnitTest.class)
 public class ValidateCommandParametersTest {
 
   @Test
-  public void validateRegionName() throws Exception {
-    validateCommandParameters region = validateCommandParameters.REGION_PATH;
-    region.validateName("/test");
-    region.validateName("test");
-    assertThatThrownBy(() -> region.validateName("__#@T"))
+  public void validateVariousVariationsOfRegionName() throws Exception {
+    validateRegionName("/test");
+    validateRegionName("test");
+    validateRegionName("te/st");
+    validateRegionName("te-st");
+    validateRegionName("_test");
+    validateRegionName("/_test");
+    validateRegionName("/_tes/t");
+    assertThatThrownBy(() -> validateRegionName("/__test"))
         .isInstanceOf(IllegalArgumentException.class);
-    assertThatThrownBy(() -> region.validateName("/__#@T"))
+    assertThatThrownBy(() -> validateRegionName("__#@T"))
         .isInstanceOf(IllegalArgumentException.class);
-    assertThatThrownBy(() -> region.validateName("__"))
+    assertThatThrownBy(() -> validateRegionName("__#@T"))
         .isInstanceOf(IllegalArgumentException.class);
-    assertThatThrownBy(() -> region.validateName("/__"))
+    assertThatThrownBy(() -> validateRegionName("/__#@T"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName("__")).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName("/__"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName("")).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName(null)).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName(" ")).isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateRegionName("@#$%"))
+        .isInstanceOf(IllegalArgumentException.class);
+  }
+
+  @Test
+  public void validateVariousVariationsOfIndexName() throws Exception {
+    assertThatThrownBy(() -> validateLuceneIndexName("/test"))
+        .isInstanceOf(IllegalArgumentException.class);
+    validateLuceneIndexName("test");
+    validateLuceneIndexName("_test");
+    validateLuceneIndexName("te-st");
+    validateLuceneIndexName("_te-st");
+    assertThatThrownBy(() -> validateLuceneIndexName("te/st"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("__test"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("__#@T"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("/__#@T"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("__"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("/__"))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName(""))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName(null))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName(" "))
+        .isInstanceOf(IllegalArgumentException.class);
+    assertThatThrownBy(() -> validateLuceneIndexName("@#$%"))
         .isInstanceOf(IllegalArgumentException.class);
   }
 }

-- 
To stop receiving notification emails like this one, please contact
nnag@apache.org.