You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ep...@apache.org on 2023/06/22 11:59:27 UTC

[solr] branch main updated: SOLR-16820: Allow semVer style collection names (amongst others) (#1688)

This is an automated email from the ASF dual-hosted git repository.

epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 5dc56ac4735 SOLR-16820: Allow semVer style collection names (amongst others) (#1688)
5dc56ac4735 is described below

commit 5dc56ac473557d04edcfa7af4a9966135daf94ad
Author: Will White <wi...@gmail.com>
AuthorDate: Thu Jun 22 12:59:20 2023 +0100

    SOLR-16820: Allow semVer style collection names (amongst others) (#1688)
    
    The previous collection name validation didn't match the validation
    used in the CreateCollectionAPI, so it was possible to create a
    collection name (e.g. `foobar-1.2.3`) that wouldn't pass validation
    at this stage via the PackageTool deploy/undeploy commands.
    ---------
    
    Co-authored-by: Eric Pugh <ep...@opensourceconnections.com>
    Co-authored-by: Kevin Risden <kr...@apache.org>
---
 solr/CHANGES.txt                                      |  2 ++
 .../org/apache/solr/packagemanager/PackageUtils.java  | 13 +++----------
 solr/packaging/test/test_packages.bats                | 19 +++++++++++++++++++
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0db629d08f5..e496d71f78e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -176,6 +176,8 @@ Improvements
   then uses the weights to decide the optimal strategy for placing new replicas or balancing existing replicas.
   (Houston Putman, Tomás Fernández Löbbe, Jason Gerlowski, Radu Gheorghe)
 
+* SOLR-16820: Align allowed collection names and the validation of them in the CreateCollectionAPI and the PackageTool. (Will White via Eric Pugh)
+
 Optimizations
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
index c813f400a37..97379a3a114 100644
--- a/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
+++ b/solr/core/src/java/org/apache/solr/packagemanager/PackageUtils.java
@@ -16,6 +16,8 @@
  */
 package org.apache.solr.packagemanager;
 
+import static org.apache.solr.client.solrj.util.SolrIdentifierValidator.validateCollectionName;
+
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.jayway.jsonpath.Configuration;
 import com.jayway.jsonpath.spi.json.JacksonJsonProvider;
@@ -268,17 +270,8 @@ public class PackageUtils {
   }
 
   public static String[] validateCollections(String collections[]) {
-    String collectionNameRegex = "^[a-zA-Z0-9_-]*$";
     for (String c : collections) {
-      if (c.matches(collectionNameRegex) == false) {
-        throw new SolrException(
-            ErrorCode.BAD_REQUEST,
-            "Invalid collection name: "
-                + c
-                + ". Didn't match the pattern: '"
-                + collectionNameRegex
-                + "'");
-      }
+      validateCollectionName(c);
     }
     return collections;
   }
diff --git a/solr/packaging/test/test_packages.bats b/solr/packaging/test/test_packages.bats
index dc2ef88c618..6d7359bb7fd 100644
--- a/solr/packaging/test/test_packages.bats
+++ b/solr/packaging/test/test_packages.bats
@@ -46,3 +46,22 @@ teardown() {
   run solr package list-available
   assert_output --partial "Available packages:"
 }
+
+@test "deploying and undeploying of packages" {
+  run solr start -c -Denable.packages=true
+
+  solr create_collection -c foo-1.2
+
+  # Deploy package - the package doesn't need to exist before the collection validation kicks in
+  run solr package deploy PACKAGE_NAME -collections foo-1.2
+  # assert_output --partial "Deployment successful"
+  refute_output --partial "Invalid collection"
+  
+  # Until PACKAGE_NAME refers to an actual installable package, this is as far as we get.
+  assert_output --partial "Package instance doesn't exist: PACKAGE_NAME:null"
+
+  # Undeploy package
+  run solr package undeploy PACKAGE_NAME -collections foo-1.2
+  refute_output --partial "Invalid collection"
+  assert_output --partial "Package PACKAGE_NAME not deployed on collection foo-1.2"
+}