You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Anshum Gupta <an...@apache.org> on 2016/03/03 21:59:58 UTC
Re: lucene-solr git commit: SOLR-8725: Allow hyphen in shard,
collection, core, and alias names but not the first char
This broke pre-commit, I'll commit a fix.
On Thu, Mar 3, 2016 at 10:53 AM, <an...@apache.org> wrote:
> Repository: lucene-solr
> Updated Branches:
> refs/heads/master a079ff252 -> 6de2b7dbd
>
>
> SOLR-8725: Allow hyphen in shard, collection, core, and alias names but
> not the first char
>
>
> Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
> Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/6de2b7db
> Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/6de2b7db
> Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/6de2b7db
>
> Branch: refs/heads/master
> Commit: 6de2b7dbd17fc70fc9b2b053fe2628534116309b
> Parents: a079ff2
> Author: anshum <an...@apache.org>
> Authored: Wed Mar 2 16:18:42 2016 -0800
> Committer: anshum <an...@apache.org>
> Committed: Thu Mar 3 10:04:07 2016 -0800
>
> ----------------------------------------------------------------------
> solr/CHANGES.txt | 2 ++
> .../org/apache/solr/core/CoreContainer.java | 9 ++++-----
> .../solr/handler/admin/CollectionsHandler.java | 17 +++++++----------
> .../apache/solr/cloud/TestCollectionAPI.java | 8 ++++----
> .../solrj/request/CollectionAdminRequest.java | 20 ++++++++++----------
> .../client/solrj/request/CoreAdminRequest.java | 8 ++++----
> .../solrj/util/SolrIdentifierValidator.java | 16 +++++++++++++---
> .../request/TestCollectionAdminRequest.java | 8 ++++----
> .../client/solrj/request/TestCoreAdmin.java | 4 ++--
> 9 files changed, 50 insertions(+), 42 deletions(-)
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/CHANGES.txt
> ----------------------------------------------------------------------
> diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
> index a96282b..66e6bb0 100644
> --- a/solr/CHANGES.txt
> +++ b/solr/CHANGES.txt
> @@ -382,6 +382,8 @@ Other Changes
>
> * SOLR-8778: Deprecate CSVStrategy's setters, and make its pre-configured
> strategies immutable. (Steve Rowe)
>
> +* SOLR-8725: Allow hyphen in collection, core, shard, and alias name as
> the non-first character (Anshum Gupta)
> +
> ================== 5.5.1 ==================
>
> Bug Fixes
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/core/src/java/org/apache/solr/core/CoreContainer.java
> ----------------------------------------------------------------------
> diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
> b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
> index 7a55e05..9ff45ea 100644
> --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
> +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
> @@ -805,8 +805,7 @@ public class CoreContainer {
> try {
> MDCLoggingContext.setCore(core);
> if (!SolrIdentifierValidator.validateCoreName(dcore.getName())) {
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid core: " +
> dcore.getName()
> - + ". Core names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
> dcore.getName()));
> }
> if (zkSys.getZkController() != null) {
> zkSys.getZkController().preRegister(dcore);
> @@ -1010,9 +1009,9 @@ public class CoreContainer {
> }
>
> public void rename(String name, String toName) {
> - if(!SolrIdentifierValidator.validateCoreName(toName)) {
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid core: " +
> toName
> - + ". Core names must consist entirely of periods, underscores,
> and alphanumerics");
> + if (!SolrIdentifierValidator.validateCoreName(toName)) {
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
> + toName));
> }
> try (SolrCore core = getCore(name)) {
> if (core != null) {
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
> b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
> index c81e183..ce4eab2 100644
> ---
> a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
> +++
> b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
> @@ -347,14 +347,12 @@ public class CollectionsHandler extends
> RequestHandlerBase {
> verifyRuleParams(h.coreContainer, props);
> final String collectionName = (String) props.get(NAME);
> if
> (!SolrIdentifierValidator.validateCollectionName(collectionName)) {
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid
> collection: " + collectionName
> - + ". Collection names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> +
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.COLLECTION,
> collectionName));
> }
> final String shardsParam = (String) props.get(SHARDS_PROP);
> if (StringUtils.isNotEmpty(shardsParam)) {
> - log.info("Validating shards param!!!!!!!!" + shardsParam);
> verifyShardsParam(shardsParam);
> - log.info("Validating shards param!!!!!!! done" + shardsParam);
> }
> if (SYSTEM_COLL.equals(collectionName)) {
> //We must always create a .system collection with only a single
> shard
> @@ -434,8 +432,7 @@ public class CollectionsHandler extends
> RequestHandlerBase {
> throws Exception {
> final String aliasName = req.getParams().get(NAME);
> if (!SolrIdentifierValidator.validateCollectionName(aliasName)) {
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid alias:
> " + aliasName
> - + ". Aliases must consist entirely of periods, underscores,
> and alphanumerics");
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.ALIAS,
> aliasName));
> }
> return req.getParams().required().getAll(null, NAME,
> "collections");
> }
> @@ -502,8 +499,8 @@ public class CollectionsHandler extends
> RequestHandlerBase {
> ClusterState clusterState =
> handler.coreContainer.getZkController().getClusterState();
> final String newShardName = req.getParams().get(SHARD_ID_PROP);
> if (!SolrIdentifierValidator.validateShardName(newShardName)) {
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid shard:
> " + newShardName
> - + ". Shard names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
> + newShardName));
> }
> if (!ImplicitDocRouter.NAME.equals(((Map)
> clusterState.getCollection(req.getParams().get(COLLECTION_PROP)).get(DOC_ROUTER)).get(NAME)))
> throw new SolrException(ErrorCode.BAD_REQUEST, "shards can be
> added only to 'implicit' collections");
> @@ -989,8 +986,8 @@ public class CollectionsHandler extends
> RequestHandlerBase {
> private static void verifyShardsParam(String shardsParam) {
> for (String shard : shardsParam.split(",")) {
> if (!SolrIdentifierValidator.validateShardName(shard))
> - throw new SolrException(ErrorCode.BAD_REQUEST, "Invalid shard: "
> + shard
> - + ". Shard names must consist entirely of periods,
> underscores, and alphanumerics");;
> + throw new SolrException(ErrorCode.BAD_REQUEST,
> SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
> + shard));
> }
> }
>
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
> b/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
> index 353c708..b203f02 100644
> --- a/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
> +++ b/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
> @@ -649,7 +649,7 @@ public class TestCollectionAPI extends
> ReplicaPropertiesBase {
> final String errorMessage = e.getMessage();
> assertTrue(errorMessage.contains("Invalid collection"));
> assertTrue(errorMessage.contains("invalid@name
> #with$weird%characters"));
> - assertTrue(errorMessage.contains("Collection names must consist
> entirely of"));
> + assertTrue(errorMessage.contains("collection names must consist
> entirely of"));
> }
> }
> }
> @@ -672,7 +672,7 @@ public class TestCollectionAPI extends
> ReplicaPropertiesBase {
> final String errorMessage = e.getMessage();
> assertTrue(errorMessage.contains("Invalid shard"));
> assertTrue(errorMessage.contains("invalid@name
> #with$weird%characters"));
> - assertTrue(errorMessage.contains("Shard names must consist
> entirely of"));
> + assertTrue(errorMessage.contains("shard names must consist
> entirely of"));
> }
> }
> }
> @@ -693,7 +693,7 @@ public class TestCollectionAPI extends
> ReplicaPropertiesBase {
> final String errorMessage = e.getMessage();
> assertTrue(errorMessage.contains("Invalid alias"));
> assertTrue(errorMessage.contains("invalid@name
> #with$weird%characters"));
> - assertTrue(errorMessage.contains("Aliases must consist entirely
> of"));
> + assertTrue(errorMessage.contains("alias names must consist
> entirely of"));
> }
> }
> }
> @@ -726,7 +726,7 @@ public class TestCollectionAPI extends
> ReplicaPropertiesBase {
> final String errorMessage = e.getMessage();
> assertTrue(errorMessage.contains("Invalid shard"));
> assertTrue(errorMessage.contains("invalid@name
> #with$weird%characters"));
> - assertTrue(errorMessage.contains("Shard names must consist
> entirely of"));
> + assertTrue(errorMessage.contains("shard names must consist
> entirely of"));
> }
> }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
> b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
> index 768de29..9aead92 100644
> ---
> a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
> +++
> b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java
> @@ -292,15 +292,15 @@ public abstract class CollectionAdminRequest <Q
> extends CollectionAdminRequest<Q
> /**
> * Provide the name of the shards to be created, separated by commas
> *
> - * Shard names must consist entirely of periods, underscores, and
> alphanumerics. Other characters are not allowed.
> + * Shard names must consist entirely of periods, underscores,
> hyphens, and alphanumerics. Other characters are not allowed.
> *
> * @throws IllegalArgumentException if any of the shard names contain
> invalid characters.
> */
> public Create setShards(String shards) {
> for (String shard : shards.split(",")) {
> if (!SolrIdentifierValidator.validateShardName(shard)) {
> - throw new IllegalArgumentException("Invalid shard: " + shard
> - + ". Shard names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
> + shard));
> }
> }
> this.shards = shards;
> @@ -317,8 +317,8 @@ public abstract class CollectionAdminRequest <Q
> extends CollectionAdminRequest<Q
> @Override
> public Create setCollectionName(String collectionName) throws
> SolrException {
> if
> (!SolrIdentifierValidator.validateCollectionName(collectionName)) {
> - throw new IllegalArgumentException("Invalid collection: " +
> collectionName
> - + ". Collection names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.COLLECTION,
> + collectionName));
> }
> this.collection = collectionName;
> return this;
> @@ -440,15 +440,15 @@ public abstract class CollectionAdminRequest <Q
> extends CollectionAdminRequest<Q
> /**
> * Provide the name of the shard to be created.
> *
> - * Shard names must consist entirely of periods, underscores, and
> alphanumerics. Other characters are not allowed.
> + * Shard names must consist entirely of periods, underscores,
> hyphens, and alphanumerics. Other characters are not allowed.
> *
> * @throws IllegalArgumentException if the shard name contains
> invalid characters.
> */
> @Override
> public CreateShard setShardName(String shardName) {
> if (!SolrIdentifierValidator.validateShardName(shardName)) {
> - throw new IllegalArgumentException("Invalid shard: " + shardName
> - + ". Shard names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.SHARD,
> + shardName));
> }
> this.shardName = shardName;
> return this;
> @@ -641,8 +641,8 @@ public abstract class CollectionAdminRequest <Q
> extends CollectionAdminRequest<Q
> */
> public CreateAlias setAliasName(String aliasName) {
> if (!SolrIdentifierValidator.validateCollectionName(aliasName)) {
> - throw new IllegalArgumentException("Invalid alias: " + aliasName
> - + ". Aliases must consist entirely of periods, underscores,
> and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.ALIAS,
> + aliasName));
> }
> this.aliasName = aliasName;
> return this;
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java
> b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java
> index c1f986e..ab563ed 100644
> ---
> a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java
> +++
> b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CoreAdminRequest.java
> @@ -111,8 +111,8 @@ public class CoreAdminRequest extends
> SolrRequest<CoreAdminResponse> {
> @Override
> public void setCoreName(String coreName) {
> if (!SolrIdentifierValidator.validateCoreName(coreName)) {
> - throw new IllegalArgumentException("Invalid core: " + coreName
> - + ". Core names must consist entirely of periods,
> underscores, and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
> + coreName));
> }
> this.core = coreName;
> }
> @@ -560,8 +560,8 @@ public class CoreAdminRequest extends
> SolrRequest<CoreAdminResponse> {
> public static CoreAdminResponse renameCore(String coreName, String
> newName, SolrClient client )
> throws SolrServerException, IOException {
> if (!SolrIdentifierValidator.validateCoreName(newName)) {
> - throw new IllegalArgumentException("Invalid core: " + newName
> - + ". Core names must consist entirely of periods, underscores,
> and alphanumerics");
> + throw new
> IllegalArgumentException(SolrIdentifierValidator.getIdentifierMessage(SolrIdentifierValidator.IdentifierType.CORE,
> + newName));
> }
>
> CoreAdminRequest req = new CoreAdminRequest();
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrIdentifierValidator.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrIdentifierValidator.java
> b/solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrIdentifierValidator.java
> index 00d9b83..2b1f3b5 100644
> ---
> a/solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrIdentifierValidator.java
> +++
> b/solr/solrj/src/java/org/apache/solr/client/solrj/util/SolrIdentifierValidator.java
> @@ -23,11 +23,15 @@ import java.util.regex.Pattern;
> * Ensures that provided identifiers align with Solr's
> recommendations/requirements for choosing
> * collection, core, etc identifiers.
> *
> - * Identifiers are allowed to contain underscores, periods, and
> alphanumeric characters.
> + * Identifiers are allowed to contain underscores, periods, hyphens, and
> alphanumeric characters.
> */
> public class SolrIdentifierValidator {
> - final static Pattern identifierPattern =
> Pattern.compile("^[\\._A-Za-z0-9]*$");
> -
> + final static Pattern identifierPattern =
> Pattern.compile("^(?!\\-)[\\._A-Za-z0-9\\-]*$");
> +
> + public enum IdentifierType {
> + SHARD, COLLECTION, CORE, ALIAS
> + }
> +
> public static boolean validateShardName(String shardName) {
> return validateIdentifier(shardName);
> }
> @@ -46,6 +50,12 @@ public class SolrIdentifierValidator {
> }
> return true;
> }
> +
> + public static String getIdentifierMessage(IdentifierType
> identifierType, String name) {
> + return "Invalid " + identifierType.toString().toLowerCase() + ": "
> + name + ". " + identifierType.toString().toLowerCase()
> + + " names must consist entirely of periods, underscores,
> hyphens, and alphanumerics";
> +
> + }
> }
>
>
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCollectionAdminRequest.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCollectionAdminRequest.java
> b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCollectionAdminRequest.java
> index ce6a6aa..5d5c315 100644
> ---
> a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCollectionAdminRequest.java
> +++
> b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCollectionAdminRequest.java
> @@ -37,7 +37,7 @@ public class TestCollectionAdminRequest extends
> LuceneTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(exceptionMessage.contains("Invalid collection"));
> assertTrue(exceptionMessage.contains("invalid$collection@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
>
> @@ -51,7 +51,7 @@ public class TestCollectionAdminRequest extends
> LuceneTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(exceptionMessage.contains("Invalid shard"));
> assertTrue(exceptionMessage.contains("invalid$shard@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
>
> @@ -65,7 +65,7 @@ public class TestCollectionAdminRequest extends
> LuceneTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(exceptionMessage.contains("Invalid alias"));
> assertTrue(exceptionMessage.contains("invalid$alias@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
>
> @@ -79,7 +79,7 @@ public class TestCollectionAdminRequest extends
> LuceneTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(exceptionMessage.contains("Invalid shard"));
> assertTrue(exceptionMessage.contains("invalid$shard@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
> }
>
>
> http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/6de2b7db/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java
> ----------------------------------------------------------------------
> diff --git
> a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java
> b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java
> index 143d2c3..f3c3d55 100644
> ---
> a/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java
> +++
> b/solr/solrj/src/test/org/apache/solr/client/solrj/request/TestCoreAdmin.java
> @@ -171,7 +171,7 @@ public class TestCoreAdmin extends
> AbstractEmbeddedSolrServerTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(exceptionMessage.contains("Invalid core"));
> assertTrue(exceptionMessage.contains("invalid$core@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
>
> @@ -184,7 +184,7 @@ public class TestCoreAdmin extends
> AbstractEmbeddedSolrServerTestCase {
> final String exceptionMessage = e.getMessage();
> assertTrue(e.getMessage(), exceptionMessage.contains("Invalid
> core"));
> assertTrue(exceptionMessage.contains("invalid$core@name"));
> - assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, and alphanumerics"));
> + assertTrue(exceptionMessage.contains("must consist entirely of
> periods, underscores, hyphens, and alphanumerics"));
> }
> }
>
>
>