SOLR-5300: Check that the supplied hash ranges actually cover the entire range of the shard

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1534974 13f79535-47bb-0310-9956-ffa450edef68

solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java

@@ -552,6 +552,19 @@ public class OverseerCollectionProcessor implements Runnable, ClosableThread {
                 "Specified hash range: " + r + " is not a subset of parent shard's range: " + range.toString());
           }
         }
+        List<DocRouter.Range> temp = new ArrayList<DocRouter.Range>(subRanges); // copy to preserve original order
+        Collections.sort(temp);
+        if (!range.equals(new DocRouter.Range(temp.get(0).min, temp.get(temp.size() - 1).max)))  {
+          throw new SolrException(ErrorCode.BAD_REQUEST,
+              "Specified hash ranges: " + rangesStr + " do not cover the entire range of parent shard: " + range);
+        }
+        for (int i = 1; i < temp.size(); i++) {
+          if (temp.get(i - 1).max + 1 != temp.get(i).min) {
+            throw new SolrException(ErrorCode.BAD_REQUEST,
+                "Specified hash ranges: " + rangesStr + " either overlap with each other or " +
+                    "do not cover the entire range of parent shard: " + range);
+          }
+        }
       }
     } else if (splitKey != null)  {
       if (router instanceof CompositeIdRouter) {

solr/core/src/test/org/apache/solr/cloud/ShardSplitTest.java

@@ -109,6 +109,7 @@ public class ShardSplitTest extends BasicDistributedZkTest {
   public void doTest() throws Exception {
     waitForThingsToLevelOut(15);
 
+    incompleteOrOverlappingCustomRangeTest();
     splitByUniqueKeyTest();
     splitByRouteFieldTest();
     splitByRouteKeyTest();
@@ -120,6 +121,50 @@ public class ShardSplitTest extends BasicDistributedZkTest {
 
   }
 
+  private void incompleteOrOverlappingCustomRangeTest() throws Exception  {
+    ClusterState clusterState = cloudClient.getZkStateReader().getClusterState();
+    final DocRouter router = clusterState.getCollection(AbstractDistribZkTestBase.DEFAULT_COLLECTION).getRouter();
+    Slice shard1 = clusterState.getSlice(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1);
+    DocRouter.Range shard1Range = shard1.getRange() != null ? shard1.getRange() : router.fullRange();
+
+    List<DocRouter.Range> subRanges = new ArrayList<DocRouter.Range>();
+    List<DocRouter.Range> ranges = router.partitionRange(4, shard1Range);
+
+    // test with only one range
+    subRanges.add(ranges.get(0));
+    try {
+      splitShard(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1, subRanges, null);
+      fail("Shard splitting with just one custom hash range should not succeed");
+    } catch (HttpSolrServer.RemoteSolrException e) {
+      log.info("Expected exception:", e);
+    }
+    subRanges.clear();
+
+    // test with ranges with a hole in between them
+    subRanges.add(ranges.get(3)); // order shouldn't matter
+    subRanges.add(ranges.get(0));
+    try {
+      splitShard(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1, subRanges, null);
+      fail("Shard splitting with missing hashes in between given ranges should not succeed");
+    } catch (HttpSolrServer.RemoteSolrException e) {
+      log.info("Expected exception:", e);
+    }
+    subRanges.clear();
+
+    // test with overlapping ranges
+    subRanges.add(ranges.get(0));
+    subRanges.add(ranges.get(1));
+    subRanges.add(ranges.get(2));
+    subRanges.add(new DocRouter.Range(ranges.get(3).min - 15, ranges.get(3).max));
+    try {
+      splitShard(AbstractDistribZkTestBase.DEFAULT_COLLECTION, SHARD1, subRanges, null);
+      fail("Shard splitting with overlapping ranges should not succeed");
+    } catch (HttpSolrServer.RemoteSolrException e) {
+      log.info("Expected exception:", e);
+    }
+    subRanges.clear();
+  }
+
   private void splitByUniqueKeyTest() throws Exception {
     ClusterState clusterState = cloudClient.getZkStateReader().getClusterState();
     final DocRouter router = clusterState.getCollection(AbstractDistribZkTestBase.DEFAULT_COLLECTION).getRouter();

solr/solrj/src/java/org/apache/solr/common/cloud/DocRouter.java

@@ -95,7 +95,7 @@ public abstract class DocRouter {
   // Hash ranges can't currently "wrap" - i.e. max must be greater or equal to min.
   // TODO: ranges may not be all contiguous in the future (either that or we will
   // need an extra class to model a collection of ranges)
-  public static class Range implements JSONWriter.Writable {
+  public static class Range implements JSONWriter.Writable, Comparable<Range> {
     public int min;  // inclusive
     public int max;  // inclusive
 
@@ -141,6 +141,12 @@ public abstract class DocRouter {
     public void write(JSONWriter writer) {
       writer.write(toString());
     }
+
+    @Override
+    public int compareTo(Range that) {
+      int mincomp = Integer.valueOf(this.min).compareTo(that.min);
+      return mincomp == 0 ? Integer.valueOf(this.max).compareTo(that.max) : mincomp;
+    }
   }
 
   public Range fromString(String range) {