You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by kjduling <gi...@git.apache.org> on 2016/11/15 17:34:40 UTC
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
GitHub user kjduling opened a pull request:
https://github.com/apache/incubator-geode/pull/285
GEODE-1617: Regions can be created with a variety of characters that \u2026
\u2026are unsupported
Added comprehensive test to validate region names
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kjduling/incubator-geode feature/GEODE-1617
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-geode/pull/285.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #285
----
commit 1672018170c979a4104dce83224a3d697becfe13
Author: Kevin Duling <kd...@pivotal.io>
Date: 2016-11-15T17:33:59Z
GEODE-1617: Regions can be created with a variety of characters that are unsupported
Added comprehensive test to validate region names
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/285#discussion_r88081170
--- Diff: geode-core/src/test/java/org/apache/geode/cache/RegionNameValidationJUnitTest.java ---
@@ -0,0 +1,75 @@
+package org.apache.geode.cache;
+
+import static org.junit.Assert.fail;
+
+import org.apache.geode.internal.cache.InternalRegionArguments;
+import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Category(UnitTest.class)
+public class RegionNameValidationJUnitTest {
+ private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");
+ private static final String REGION_NAME = "MyRegion";
+
+
+ @Test
+ public void testInvalidNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ try {
+ LocalRegion.validateRegionName(null, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("", ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("FOO" + Region.SEPARATOR, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+
+ }
+
+ @Test
+ public void testExternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ validateCharacters(ira);
+ try {
+ LocalRegion.validateRegionName("__InvalidInternalRegionName", ira);
+ } catch (IllegalArgumentException ignore) {
+ }
+ }
+
+ @Test
+ public void testInternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(true);
+ LocalRegion.validateRegionName("__ValidInternalRegionName", ira);
+ }
+
+ private void validateCharacters(InternalRegionArguments ira) {
+ for (int x = 0; x < Character.MAX_VALUE; x++) {
+ String name = (char) x + REGION_NAME;
+ Matcher matcher = NAME_PATTERN.matcher(name);
+ if (matcher.matches()) {
+ LocalRegion.validateRegionName(name, ira);
+ } else {
+ try {
+ LocalRegion.validateRegionName(name, ira);
+ fail("Should have received an IllegalArgumentException");
--- End diff --
I added printing the offending character and the numeric value of it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-geode/pull/285
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #285: GEODE-1617: Regions can be created with a variet...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on the issue:
https://github.com/apache/incubator-geode/pull/285
Pushed to develop. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #285: GEODE-1617: Regions can be created with a variet...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on the issue:
https://github.com/apache/incubator-geode/pull/285
I reworded your commit message:
```
GEODE-1617: add test for region names
Added comprehensive test to validate region names
This closes #285
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
Posted by kjduling <gi...@git.apache.org>.
Github user kjduling commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/285#discussion_r88080988
--- Diff: geode-core/src/test/java/org/apache/geode/cache/RegionNameValidationJUnitTest.java ---
@@ -0,0 +1,75 @@
+package org.apache.geode.cache;
+
+import static org.junit.Assert.fail;
+
+import org.apache.geode.internal.cache.InternalRegionArguments;
+import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Category(UnitTest.class)
+public class RegionNameValidationJUnitTest {
+ private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");
+ private static final String REGION_NAME = "MyRegion";
+
+
+ @Test
+ public void testInvalidNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ try {
+ LocalRegion.validateRegionName(null, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("", ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("FOO" + Region.SEPARATOR, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+
+ }
+
+ @Test
+ public void testExternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ validateCharacters(ira);
+ try {
+ LocalRegion.validateRegionName("__InvalidInternalRegionName", ira);
--- End diff --
Yes, fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/285#discussion_r88075502
--- Diff: geode-core/src/test/java/org/apache/geode/cache/RegionNameValidationJUnitTest.java ---
@@ -0,0 +1,75 @@
+package org.apache.geode.cache;
+
+import static org.junit.Assert.fail;
+
+import org.apache.geode.internal.cache.InternalRegionArguments;
+import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Category(UnitTest.class)
+public class RegionNameValidationJUnitTest {
+ private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");
+ private static final String REGION_NAME = "MyRegion";
+
+
+ @Test
+ public void testInvalidNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ try {
+ LocalRegion.validateRegionName(null, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("", ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("FOO" + Region.SEPARATOR, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+
+ }
+
+ @Test
+ public void testExternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ validateCharacters(ira);
+ try {
+ LocalRegion.validateRegionName("__InvalidInternalRegionName", ira);
+ } catch (IllegalArgumentException ignore) {
+ }
+ }
+
+ @Test
+ public void testInternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(true);
+ LocalRegion.validateRegionName("__ValidInternalRegionName", ira);
+ }
+
+ private void validateCharacters(InternalRegionArguments ira) {
+ for (int x = 0; x < Character.MAX_VALUE; x++) {
+ String name = (char) x + REGION_NAME;
+ Matcher matcher = NAME_PATTERN.matcher(name);
+ if (matcher.matches()) {
+ LocalRegion.validateRegionName(name, ira);
+ } else {
+ try {
+ LocalRegion.validateRegionName(name, ira);
+ fail("Should have received an IllegalArgumentException");
--- End diff --
Recommend adding which character to the message so we get instant info about which character should have been illegal but was allowed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #285: GEODE-1617: Regions can be created with a...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/285#discussion_r88075036
--- Diff: geode-core/src/test/java/org/apache/geode/cache/RegionNameValidationJUnitTest.java ---
@@ -0,0 +1,75 @@
+package org.apache.geode.cache;
+
+import static org.junit.Assert.fail;
+
+import org.apache.geode.internal.cache.InternalRegionArguments;
+import org.apache.geode.internal.cache.LocalRegion;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+@Category(UnitTest.class)
+public class RegionNameValidationJUnitTest {
+ private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+");
+ private static final String REGION_NAME = "MyRegion";
+
+
+ @Test
+ public void testInvalidNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ try {
+ LocalRegion.validateRegionName(null, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("", ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+ try {
+ LocalRegion.validateRegionName("FOO" + Region.SEPARATOR, ira);
+ fail();
+ } catch (IllegalArgumentException ignore) {
+ }
+
+ }
+
+ @Test
+ public void testExternalRegionNames() {
+ InternalRegionArguments ira = new InternalRegionArguments();
+ ira.setInternalRegion(false);
+ validateCharacters(ira);
+ try {
+ LocalRegion.validateRegionName("__InvalidInternalRegionName", ira);
--- End diff --
Aren't you missing a fail() stmt here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #285: GEODE-1617: Regions can be created with a variet...
Posted by kirklund <gi...@git.apache.org>.
Github user kirklund commented on the issue:
https://github.com/apache/incubator-geode/pull/285
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---