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

Reply via email to