diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 95200a1b08f..f00de53631a 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -93,6 +93,11 @@ New Features * SOLR-12304: The MoreLikeThisComponent now supports the mlt.interestingTerms parameter. Previously this option was unique to the MLT handler. (Alessandro Benedetti via David Smiley) +Bug Fixes +---------------------- + +* SOLR-13475: Null Pointer Exception when querying collection through collection alias. (Jörn Franke, ab) + Other Changes ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java index 57be84fc725..8d0d45cd5a2 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateAliasCmd.java @@ -85,6 +85,9 @@ public class CreateAliasCmd extends AliasCmd { private void callCreatePlainAlias(ZkNodeProps message, String aliasName, ZkStateReader zkStateReader) { final List canonicalCollectionList = parseCollectionsParameter(message.get("collections")); + if (canonicalCollectionList.isEmpty()) { + throw new SolrException(BAD_REQUEST, "'collections' parameter doesn't contain any collection names."); + } final String canonicalCollectionsString = StrUtils.join(canonicalCollectionList, ','); validateAllCollectionsExistAndNoDuplicates(canonicalCollectionList, zkStateReader); zkStateReader.aliasesManager @@ -101,6 +104,7 @@ public class CreateAliasCmd extends AliasCmd { if (colls instanceof List) return (List) colls; return StrUtils.splitSmart(colls.toString(), ",", true).stream() .map(String::trim) + .filter(s -> !s.isEmpty()) .collect(Collectors.toList()); } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java index a71cb5370d1..dfd5a218750 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java @@ -329,8 +329,10 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd } } - // create an alias pointing to the new collection - ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(alias, collectionName)); + // create an alias pointing to the new collection, if different from the collectionName + if (!alias.equals(collectionName)) { + ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(alias, collectionName)); + } } catch (SolrException ex) { throw ex; diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java index d5215f7b9ca..2054258bbeb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java @@ -25,6 +25,7 @@ import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonParams.NAME; import java.lang.invoke.MethodHandles; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -75,7 +76,7 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd zkStateReader.aliasesManager.update(); // aliases may have been stale; get latest from ZK } - String aliasReference = checkAliasReference(zkStateReader, extCollection); + List aliasReferences = checkAliasReference(zkStateReader, extCollection); Aliases aliases = zkStateReader.getAliases(); String collection = aliases.resolveSimpleAlias(extCollection); @@ -135,9 +136,14 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd // wait for a while until we don't see the collection zkStateReader.waitForState(collection, 60, TimeUnit.SECONDS, (liveNodes, collectionState) -> collectionState == null); - // we can delete any remaining unique alias - if (aliasReference != null) { - ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> a.cloneWithCollectionAlias(aliasReference, null)); + // we can delete any remaining unique aliases + if (!aliasReferences.isEmpty()) { + ocmh.zkStateReader.aliasesManager.applyModificationAndExportToZk(a -> { + for (String alias : aliasReferences) { + a = a.cloneWithCollectionAlias(alias, null); + } + return a; + }); } // TimeOut timeout = new TimeOut(60, TimeUnit.SECONDS, timeSource); @@ -178,23 +184,29 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd } } - // it's ok if a collection is referenced either by none or exactly by a single alias. - // This method returns the single alias to delete, if present, or null - private String checkAliasReference(ZkStateReader zkStateReader, String extCollection) throws Exception { - List aliases = referencedByAlias(extCollection, zkStateReader.getAliases()); - if (aliases.size() > 1) { + // This method returns the single collection aliases to delete, if present, or null + private List checkAliasReference(ZkStateReader zkStateReader, String extCollection) throws Exception { + Aliases aliases = zkStateReader.getAliases(); + List aliasesRefs = referencedByAlias(extCollection, aliases); + List aliasesToDelete = new ArrayList<>(); + if (aliasesRefs.size() > 0) { zkStateReader.aliasesManager.update(); // aliases may have been stale; get latest from ZK - aliases = referencedByAlias(extCollection, zkStateReader.getAliases()); - if (aliases.size() > 1) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, - "Collection : " + extCollection + " is part of aliases: " + aliases + ", remove or modify the aliases before removing this collection."); + aliases = zkStateReader.getAliases(); + aliasesRefs = referencedByAlias(extCollection, aliases); + if (aliasesRefs.size() > 0) { + for (String alias : aliasesRefs) { + // for back-compat in 8.x we don't automatically remove other + // aliases that point only to this collection + if (!extCollection.equals(alias)) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, + "Collection : " + extCollection + " is part of aliases: " + aliasesRefs + ", remove or modify the aliases before removing this collection."); + } else { + aliasesToDelete.add(alias); + } + } } } - if (!aliases.isEmpty()) { - return aliases.get(0); - } else { - return null; - } + return aliasesToDelete; } public static List referencedByAlias(String extCollection, Aliases aliases) throws IllegalArgumentException { diff --git a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java index 2ee0cfd21df..a64ed7a4931 100644 --- a/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/AliasIntegrationTest.java @@ -543,9 +543,19 @@ public class AliasIntegrationTest extends SolrCloudTestCase { .commit(cluster.getSolrClient(), "collection2"); /////////////// + // make sure there's only one level of alias + CollectionAdminRequest.deleteAlias("collection1").process(cluster.getSolrClient()); CollectionAdminRequest.createAlias("testalias1", "collection1").process(cluster.getSolrClient()); - // ensure that the alias has been registered + // verify proper resolution on the server-side + ZkStateReader zkStateReader = cluster.getSolrClient().getZkStateReader(); + zkStateReader.aliasesManager.update(); + Aliases aliases = zkStateReader.getAliases(); + List collections = aliases.resolveAliases("testalias1"); + assertEquals(collections.toString(), 1, collections.size()); + assertTrue(collections.contains("collection1")); + + // ensure that the alias is visible in the API assertEquals("collection1", new CollectionAdminRequest.ListAliases().process(cluster.getSolrClient()).getAliases().get("testalias1")); diff --git a/solr/solr-ref-guide/src/collections-api.adoc b/solr/solr-ref-guide/src/collections-api.adoc index 1c39a1bb097..af7f63b527f 100644 --- a/solr/solr-ref-guide/src/collections-api.adoc +++ b/solr/solr-ref-guide/src/collections-api.adoc @@ -121,8 +121,8 @@ The name of the collection with which all replicas of this collection must be co See <> for more details. `alias`:: -Starting with version 8.1 when a collection is created additionally an alias (by default with the same name) is created -that points to this collection. This parameter allows changing the name of this alias, effectively combining +Starting with version 8.1 when a collection is created additionally an alias can be created +that points to this collection. This parameter allows specifying the name of this alias, effectively combining this operation with <> Collections are first created in read-write mode but can be put in `readOnly` diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java index 6f97c252523..f1fa3f1365a 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java @@ -248,11 +248,15 @@ public class Aliases { for (int i = 0; i < level1.size(); i++) { String level1Alias = level1.get(i); List level2 = collectionAliasListMap.get(level1Alias); - if (level2 == null && uniqueResult != null) { - uniqueResult.add(level1Alias); + if (level2 == null) { + // will copy all level1alias-es so far on lazy init + if (uniqueResult != null) { + uniqueResult.add(level1Alias); + } } else { if (uniqueResult == null) { // lazy init uniqueResult = new LinkedHashSet<>(level1.size()); + // add all level1Alias-es so far uniqueResult.addAll(level1.subList(0, i)); } uniqueResult.addAll(level2); @@ -294,6 +298,7 @@ public class Aliases { entry.setValue(Collections.unmodifiableList(list)); } } + newColAliases.entrySet().removeIf(entry -> entry.getValue().isEmpty()); } else { newColProperties = this.collectionAliasProperties;// no changes // java representation is a list, so split before adding to maintain consistency