[GitHub] [geode] mkevo commented on a change in pull request #5159: GEODE-7956: correct legal region names
mkevo commented on a change in pull request #5159: URL: https://github.com/apache/geode/pull/5159#discussion_r433411605 ## File path: geode-docs/basic_config/data_regions/region_naming.html.md.erb ## @@ -24,7 +24,7 @@ follow these region naming guidelines. - Characters permitted in region names are alphanumeric characters (`ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789`), -period (`.`), underscore (`_`), square brackets (`[, ]`) and hyphen (`-`). +period (`.`), underscore (`_`), square brackets (`[, ]`), hyphen (`-`), caret (`^`) and backqoute (```). Review comment: Done. Thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mkevo commented on a change in pull request #5159: GEODE-7956: correct legal region names
mkevo commented on a change in pull request #5159: URL: https://github.com/apache/geode/pull/5159#discussion_r433090252 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java ## @@ -24,7 +24,7 @@ public class RegionNameValidation { - private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+"); + private static final Pattern NAME_PATTERN = Pattern.compile("[a-zA-Z\\[\\]0-9-_.]+"); Review comment: Yes, I agree. I also think that aA-zZ is the same as a-zA-Z. The new regex will be **[a-zA-Z0-9-_.^`\[\]\\]+**. Is it ok with you? This more clear to see what is included. I will also add separate tests for all of these characters. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mkevo commented on a change in pull request #5159: GEODE-7956: correct legal region names
mkevo commented on a change in pull request #5159: URL: https://github.com/apache/geode/pull/5159#discussion_r433077472 ## File path: geode-core/src/main/java/org/apache/geode/internal/cache/RegionNameValidation.java ## @@ -24,7 +24,7 @@ public class RegionNameValidation { - private static final Pattern NAME_PATTERN = Pattern.compile("[aA-zZ0-9-_.]+"); + private static final Pattern NAME_PATTERN = Pattern.compile("[a-zA-Z\\[\\]0-9-_.]+"); Review comment: I didn't find RFC for this, but old pattern include this characters and you can see it if you add logger for names in test `startingWithMatchingCharactersAreOk` in RegionNameValidationTest where it validates all characters which is allowed by this pattern. From ASCII table A is 65 and z is 122, and this part **A-z** include all characters between these two values, and this characters(^, `, [, ], and backslash) are between 91 and 96. Old pattern **[aA-zZ0-9-_.]+** is the same as **[A-z0-9-_.]+**.\ So we can go with change pattern to **[A-z0-9-_.]+** and be sure that if someone is using it we don't break them. And also document all charactes which is allowed. Do you agree with that? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mkevo commented on a change in pull request #5159: GEODE-7956: correct legal region names
mkevo commented on a change in pull request #5159: URL: https://github.com/apache/geode/pull/5159#discussion_r430875852 ## File path: geode-docs/basic_config/data_regions/region_naming.html.md.erb ## @@ -24,7 +24,7 @@ follow these region naming guidelines. - Permitted characters within region names are alphanumeric characters (`ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789`), -the underscore character (`_`), and the hyphen character (`-`). +the dot character (`.`), the underscore character (`_`), the square brackets characters (`[, ]`) and the hyphen character (`-`). Review comment: Done! Thanks @davebarnes97! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org