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].