Merge branch 'moshebla-SOLR-13152' into solr-13131
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
index 4193ec0..7d73149 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CategoryRoutedAlias.java
@@ -29,6 +29,7 @@
import org.apache.solr.cloud.ZkController;
import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.Aliases;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.core.CoreContainer;
@@ -39,6 +40,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.common.SolrException.ErrorCode.BAD_REQUEST;
+
public class CategoryRoutedAlias implements RoutedAlias {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private static final String COLLECTION_INFIX = "__CRA__";
@@ -78,11 +81,13 @@
private Aliases parsedAliases; // a cached reference to the source of what we parse into parsedCollectionsDesc
private final String aliasName;
private final Map<String, String> aliasMetadata;
+ private final Integer maxCardinality;
private final Pattern mustMatch;
CategoryRoutedAlias(String aliasName, Map<String, String> aliasMetadata) {
this.aliasName = aliasName;
this.aliasMetadata = aliasMetadata;
+ this.maxCardinality = parseMaxCardinality(aliasMetadata.get(ROUTER_MAX_CARDINALITY));
final String mustMatch = this.aliasMetadata.get(ROUTER_MUST_MATCH);
this.mustMatch = mustMatch == null? null: compileMustMatch(mustMatch);
}
@@ -113,17 +118,19 @@
@Override
public void validateRouteValue(AddUpdateCommand cmd) throws SolrException {
- //Mostly this will be filled out by SOLR-13150 and SOLR-13151
- String maxCardinality = aliasMetadata.get(ROUTER_MAX_CARDINALITY);
- if(maxCardinality == null) {
- return;
- }
-
if (this.parsedAliases == null) {
updateParsedCollectionAliases(cmd.getReq().getCore().getCoreContainer().getZkController());
}
- String dataValue = String.valueOf(cmd.getSolrInputDocument().getFieldValue(getRouteField()));
+ Object fieldValue = cmd.getSolrInputDocument().getFieldValue(getRouteField());
+ // possible future enhancement: allow specification of an "unknown" category name to where we can send
+ // docs that are uncategorized.
+ if (fieldValue == null) {
+ throw new SolrException(BAD_REQUEST,"Route value is null");
+ }
+
+ String dataValue = String.valueOf(fieldValue);
+
String candidateCollectionName = buildCollectionNameFromValue(dataValue);
List<String> cols = getCollectionList(this.parsedAliases);
@@ -131,14 +138,22 @@
return;
}
- if (mustMatch != null && !mustMatch.matcher(candidateCollectionName).matches()) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Collection name " + candidateCollectionName
+ // this check will become very important for future work
+ int infix = candidateCollectionName.indexOf(COLLECTION_INFIX);
+ int valueStart = infix + COLLECTION_INFIX.length();
+ if (candidateCollectionName.substring(valueStart).contains(COLLECTION_INFIX)) {
+ throw new SolrException(BAD_REQUEST, "No portion of the route value may resolve to the 7 character sequence " +
+ "__CRA__");
+ }
+
+ if (mustMatch != null && !mustMatch.matcher(dataValue).matches()) {
+ throw new SolrException(BAD_REQUEST, "Route value " + dataValue
+ " does not match " + ROUTER_MUST_MATCH + ": " + mustMatch);
}
if (cols.stream()
- .filter(x -> !x.contains(UNINITIALIZED)).count() >= Integer.valueOf(maxCardinality)) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Max cardinality " + maxCardinality
+ .filter(x -> !x.contains(UNINITIALIZED)).count() >= maxCardinality) {
+ throw new SolrException(BAD_REQUEST, "Max cardinality " + maxCardinality
+ " reached for Category Routed Alias: " + getAliasName());
}
}
@@ -194,7 +209,7 @@
// we should see some sort of update to our aliases
if (!updateParsedCollectionAliases(coreContainer.getZkController())) { // thus we didn't make progress...
// this is not expected, even in known failure cases, but we check just in case
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+ throw new SolrException(ErrorCode.SERVER_ERROR,
"We need to create a new category routed collection but for unknown reasons were unable to do so.");
}
}
@@ -202,7 +217,16 @@
} catch (SolrException e) {
throw e;
} catch (Exception e) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+ throw new SolrException(ErrorCode.SERVER_ERROR, e);
+ }
+ }
+
+ private Integer parseMaxCardinality(String maxCardinality) {
+ try {
+ return Integer.valueOf(maxCardinality);
+ } catch (NumberFormatException e) {
+ throw new SolrException(BAD_REQUEST, ROUTER_MAX_CARDINALITY + " must be a valid Integer"
+ + ", instead got: " + maxCardinality);
}
}
@@ -210,7 +234,7 @@
try {
return Pattern.compile(mustMatch);
} catch (PatternSyntaxException e) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, ROUTER_MUST_MATCH + " must be a valid regular"
+ throw new SolrException(BAD_REQUEST, ROUTER_MUST_MATCH + " must be a valid regular"
+ " expression, instead got: " + mustMatch);
}
}
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java
index 4951003..1ae3229 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/TimeRoutedAlias.java
@@ -38,9 +38,8 @@
import java.util.TimeZone;
import java.util.concurrent.TimeUnit;
-import com.google.common.base.Objects;
-import org.apache.solr.cloud.ZkController;
import com.google.common.base.MoreObjects;
+import org.apache.solr.cloud.ZkController;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.Aliases;
import org.apache.solr.common.params.CommonParams;
diff --git a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
index fcf3232..77ecb22 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/CategoryRoutedAliasUpdateProcessorTest.java
@@ -126,9 +126,9 @@
retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
- CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, 20,
CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
- .setMaxShardsPerNode(2)).setMaxCardinality(20)
+ .setMaxShardsPerNode(2))
.process(solrClient);
addDocsAndCommit(true,
newDoc(somethingInChinese),
@@ -181,9 +181,9 @@
retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
- CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, 20,
CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
- .setMaxShardsPerNode(2)).setMaxCardinality(Integer.MAX_VALUE)
+ .setMaxShardsPerNode(2))
.process(solrClient);
// now we index a document
@@ -200,6 +200,12 @@
newDoc(SHIPS[4]));
assertInvariants(colVogon, colHoG, colStunt, colArk, colBistro);
+
+ // make sure we fail if we have no value to route on.
+ testFailedDocument(newDoc(null), "Route value is null");
+ testFailedDocument(newDoc("foo__CRA__bar"), "7 character sequence __CRA__");
+ testFailedDocument(newDoc("fóóCRAóóbar"), "7 character sequence __CRA__");
+
}
private String noSpaces(String ship) {
@@ -211,43 +217,22 @@
private String noDollar(String ship) {
return ship.replaceAll("\\$", "_");
}
- @Slow
- @Test
- public void testMissingRequiredParams() throws Exception {
- String configName = getSaferTestName();
- createConfigSet(configName);
-
- List<String> retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets();
- List<String> expectedConfigSetNames = Arrays.asList("_default", configName);
-
- // config sets leak between tests so we can't be any more specific than this on the next 2 asserts
- assertTrue("We expect at least 2 configSets",
- retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
- assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
-
- SolrException e = expectThrows(SolrException.class, () -> CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
- CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
- .setMaxShardsPerNode(2))
- .process(solrClient)
- );
- assertTrue("Create Alias should fail since not all required params were supplied", e.getMessage().contains("Not all required params were supplied"));
- }
@Slow
@Test
public void testMustMatch() throws Exception {
String configName = getSaferTestName();
createConfigSet(configName);
- final String mustMatchRegex = ".+_solr";
+ final String mustMatchRegex = "HHS\\s.+_solr";
final int maxCardinality = Integer.MAX_VALUE; // max cardinality for current test
// Start with one collection manually created (and use higher numShards & replicas than we'll use for others)
// This tests we may pre-create the collection and it's acceptable.
- final String colVogon = getAlias() + "__CRA__" + noSpaces(SHIPS[0]) + "_solr";
+ final String colVogon = getAlias() + "__CRA__" + noSpaces("HHS "+ SHIPS[0]) + "_solr";
// we expect changes ensuring a legal collection name.
- final String colHoG = getAlias() + "__CRA__" + noSpaces(SHIPS[1]) + "_solr";
+ final String colHoG = getAlias() + "__CRA__" + noSpaces("HHS "+ SHIPS[1]) + "_solr";
List<String> retrievedConfigSetNames = new ConfigSetAdminRequest.List().process(solrClient).getConfigSets();
List<String> expectedConfigSetNames = Arrays.asList("_default", configName);
@@ -257,21 +242,20 @@
retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
- CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, maxCardinality,
CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
.setMaxShardsPerNode(2))
- .setMaxCardinality(maxCardinality)
.setMustMatch(mustMatchRegex)
.process(solrClient);
// now we index a document
- addDocsAndCommit(true, newDoc(SHIPS[0] + "_solr"));
+ addDocsAndCommit(true, newDoc("HHS " + SHIPS[0] + "_solr"));
//assertDocRoutedToCol(lastDocId, col23rd);
String uninitialized = getAlias() + "__CRA__" + CategoryRoutedAlias.UNINITIALIZED;
assertInvariants(colVogon, uninitialized);
- addDocsAndCommit(true, newDoc(SHIPS[1] + "_solr"));
+ addDocsAndCommit(true, newDoc("HHS "+ SHIPS[1] + "_solr"));
assertInvariants(colVogon, colHoG);
@@ -298,10 +282,9 @@
retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
- SolrException e = expectThrows(SolrException.class, () -> CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ SolrException e = expectThrows(SolrException.class, () -> CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, maxCardinality,
CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
.setMaxShardsPerNode(2))
- .setMaxCardinality(maxCardinality)
.setMustMatch(mustMatchRegex)
.process(solrClient)
);
@@ -333,9 +316,9 @@
retrievedConfigSetNames.size() >= expectedConfigSetNames.size());
assertTrue("ConfigNames should include :" + expectedConfigSetNames, retrievedConfigSetNames.containsAll(expectedConfigSetNames));
- CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, maxCardinality,
CollectionAdminRequest.createCollection("_unused_", configName, 1, 1)
- .setMaxShardsPerNode(2)).setMaxCardinality(maxCardinality)
+ .setMaxShardsPerNode(2))
.process(solrClient);
// now we index a document
@@ -372,9 +355,9 @@
// 4 of which are leaders, and 8 of which should fail this test.
final int numShards = 1 + random().nextInt(4);
final int numReplicas = 1 + random().nextInt(3);
- CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField,
+ CollectionAdminRequest.createCategoryRoutedAlias(getAlias(), categoryField, 20,
CollectionAdminRequest.createCollection("_unused_", configName, numShards, numReplicas)
- .setMaxShardsPerNode(numReplicas)).setMaxCardinality(20)
+ .setMaxShardsPerNode(numReplicas))
.process(solrClient);
// cause some collections to be created
@@ -447,9 +430,14 @@
}
private SolrInputDocument newDoc(String routedValue) {
- return sdoc("id", Integer.toString(++lastDocId),
- categoryField, routedValue,
- intField, "0"); // always 0
+ if (routedValue != null) {
+ return sdoc("id", Integer.toString(++lastDocId),
+ categoryField, routedValue,
+ intField, "0"); // always 0
+ } else {
+ return sdoc("id", Integer.toString(++lastDocId),
+ intField, "0"); // always 0
+ }
}
@Override
@@ -477,7 +465,8 @@
final UpdateResponse resp = solrClient.add(getAlias(), sdoc);
// if we have a TolerantUpdateProcessor then we see it there)
final Object errors = resp.getResponseHeader().get("errors"); // Tolerant URP
- assertTrue(errors != null && errors.toString().contains(errorMsg));
+ assertNotNull(errors);
+ assertTrue("Expected to find " + errorMsg + " in errors: " + errors.toString(),errors.toString().contains(errorMsg));
} catch (SolrException e) {
assertTrue(e.getMessage().contains(errorMsg));
}
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 7ef507c..de7f307 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
@@ -1619,13 +1619,15 @@
*
* @param aliasName the name of the alias to create.
* @param routerField the document field to contain the timestamp to route on
+ * @param maxCardinality the maximum number of collections under this CRA
* @param createCollTemplate Holds options to create a collection. The "name" is ignored.
*/
public static CreateCategoryRoutedAlias createCategoryRoutedAlias(String aliasName,
String routerField,
+ int maxCardinality,
Create createCollTemplate) {
- return new CreateCategoryRoutedAlias(aliasName, routerField, createCollTemplate);
+ return new CreateCategoryRoutedAlias(aliasName, routerField, maxCardinality, createCollTemplate);
}
public static class CreateCategoryRoutedAlias extends AsyncCollectionAdminRequest {
@@ -1642,16 +1644,12 @@
private final Create createCollTemplate;
- public CreateCategoryRoutedAlias(String aliasName, String routerField, Create createCollTemplate) {
+ public CreateCategoryRoutedAlias(String aliasName, String routerField, int maxCardinality, Create createCollTemplate) {
super(CollectionAction.CREATEALIAS);
this.aliasName = aliasName;
this.routerField = routerField;
- this.createCollTemplate = createCollTemplate;
- }
-
- public CreateCategoryRoutedAlias setMaxCardinality(int maxCardinality) {
this.maxCardinality = maxCardinality;
- return this;
+ this.createCollTemplate = createCollTemplate;
}
public CreateCategoryRoutedAlias setMustMatch(String regex) {
@@ -1665,10 +1663,7 @@
params.add(CommonParams.NAME, aliasName);
params.add(ROUTER_TYPE_NAME, "category");
params.add(ROUTER_FIELD, routerField);
-
- if (maxCardinality != null) {
- params.add(ROUTER_MAX_CARDINALITY, maxCardinality.toString());
- }
+ params.add(ROUTER_MAX_CARDINALITY, maxCardinality.toString());
if (mustMatch != null) {
params.add(ROUTER_MUST_MATCH, mustMatch);
diff --git a/solr/solrj/src/resources/apispec/collections.Commands.json b/solr/solrj/src/resources/apispec/collections.Commands.json
index dc8e251..f24fe72 100644
--- a/solr/solrj/src/resources/apispec/collections.Commands.json
+++ b/solr/solrj/src/resources/apispec/collections.Commands.json
@@ -175,6 +175,14 @@
"autoDeleteAge": {
"type": "string",
"description": "A date math expressions yielding a time in the past. Collections covering a period of time entirely before this age will be automatically deleted."
+ },
+ "maxCardinality": {
+ "type": "integer",
+ "description": "The maximum number of categories allowed for this alias."
+ },
+ "mustMatch": {
+ "type": "string",
+ "description": "A regular expression that the value of the field specified by `router.field` must match before a corresponding collection will be created."
}
}
},
