From 628b4ed4d202bff36c8e2c94d3b10b829dbe2c93 Mon Sep 17 00:00:00 2001 From: Shai Erera Date: Sat, 11 Sep 2010 03:50:17 +0000 Subject: [PATCH] LUCENE-2636: rename to MultiCollector (trunk) git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@996060 13f79535-47bb-0310-9956-ffa450edef68 --- lucene/CHANGES.txt | 4 +- ...ningCollector.java => MultiCollector.java} | 60 +++++++++++++------ ...ectorTest.java => MultiCollectorTest.java} | 19 ++++-- 3 files changed, 58 insertions(+), 25 deletions(-) rename lucene/src/java/org/apache/lucene/search/{ChainingCollector.java => MultiCollector.java} (60%) rename lucene/src/test/org/apache/lucene/search/{ChainingCollectorTest.java => MultiCollectorTest.java} (82%) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 3f4ad2b5448..dc791925d31 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -213,8 +213,8 @@ New features to gather the hit-count per sub-clause and per document while a search is running. (Simon Willnauer, Mike McCandless) -* LUCENE-2636: Added ChainingCollector which allows chaining several Collectors - together and be passed to IndexSearcher.search methods. (Shai Erera) +* LUCENE-2636: Added MultiCollector which allows running the search with several + Collectors. (Shai Erera) * LUCENE-2504: FieldComparator.setNextReader now returns a FieldComparator instance. You can "return this", to just reuse the diff --git a/lucene/src/java/org/apache/lucene/search/ChainingCollector.java b/lucene/src/java/org/apache/lucene/search/MultiCollector.java similarity index 60% rename from lucene/src/java/org/apache/lucene/search/ChainingCollector.java rename to lucene/src/java/org/apache/lucene/search/MultiCollector.java index dfd3e3bb4fc..ee79f549b0b 100644 --- a/lucene/src/java/org/apache/lucene/search/ChainingCollector.java +++ b/lucene/src/java/org/apache/lucene/search/MultiCollector.java @@ -24,22 +24,30 @@ import org.apache.lucene.search.Collector; import org.apache.lucene.search.Scorer; /** - * A {@link Collector} which allows chaining several {@link Collector}s in order - * to process the matching documents emitted to {@link #collect(int)}. This - * collector accepts a list of {@link Collector}s in its constructor, allowing - * for some of them to be null. It optimizes away those - * null collectors, so that they are not acessed during collection - * time. - *

- * NOTE: if all the collectors passed to the constructor are null, then - * {@link IllegalArgumentException} is thrown - it is useless to run the search - * with 0 collectors. + * A {@link Collector} which allows running a search with several + * {@link Collector}s. It offers a static {@link #wrap} method which accepts a + * list of collectots and wraps them with {@link MultiCollector}, while + * filtering out the null null ones. */ -public class ChainingCollector extends Collector { +public class MultiCollector extends Collector { - private final Collector[] collectors; - - public ChainingCollector(Collector... collectors) { + /** + * Wraps a list of {@link Collector}s with a {@link MultiCollector}. This + * method works as follows: + *

+ * + * @throws IllegalArgumentException + * if either 0 collectors were input, or all collectors are + * null. + */ + public static Collector wrap(Collector... collectors) { // For the user's convenience, we allow null collectors to be passed. // However, to improve performance, these null collectors are found // and dropped from the array we save for actual collection time. @@ -52,19 +60,35 @@ public class ChainingCollector extends Collector { if (n == 0) { throw new IllegalArgumentException("At least 1 collector must not be null"); + } else if (n == 1) { + // only 1 Collector - return it. + Collector col = null; + for (Collector c : collectors) { + if (c != null) { + col = c; + break; + } + } + return col; } else if (n == collectors.length) { - // No null collectors, can use the given list as is. - this.collectors = collectors; + return new MultiCollector(collectors); } else { - this.collectors = new Collector[n]; + Collector[] colls = new Collector[n]; n = 0; for (Collector c : collectors) { if (c != null) { - this.collectors[n++] = c; + colls[n++] = c; } } + return new MultiCollector(colls); } } + + private final Collector[] collectors; + + private MultiCollector(Collector... collectors) { + this.collectors = collectors; + } @Override public boolean acceptsDocsOutOfOrder() { diff --git a/lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java b/lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java similarity index 82% rename from lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java rename to lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java index 81879c31a98..a2718eeb0f1 100644 --- a/lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java +++ b/lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java @@ -27,7 +27,7 @@ import org.apache.lucene.search.Scorer; import org.apache.lucene.util.LuceneTestCaseJ4; import org.junit.Test; -public class ChainingCollectorTest extends LuceneTestCaseJ4 { +public class MultiCollectorTest extends LuceneTestCaseJ4 { private static class DummyCollector extends Collector { @@ -63,21 +63,30 @@ public class ChainingCollectorTest extends LuceneTestCaseJ4 { public void testNullCollectors() throws Exception { // Tests that the collector rejects all null collectors. try { - new ChainingCollector(null, null); - fail("all collectors null should not be supported"); + MultiCollector.wrap(null, null); + fail("only null collectors should not be supported"); } catch (IllegalArgumentException e) { // expected } // Tests that the collector handles some null collectors well. If it // doesn't, an NPE would be thrown. - Collector c = new ChainingCollector(new DummyCollector(), null, new DummyCollector()); + Collector c = MultiCollector.wrap(new DummyCollector(), null, new DummyCollector()); + assertTrue(c instanceof MultiCollector); assertTrue(c.acceptsDocsOutOfOrder()); c.collect(1); c.setNextReader(null, 0); c.setScorer(null); } + @Test + public void testSingleCollector() throws Exception { + // Tests that if a single Collector is input, it is returned (and not MultiCollector). + DummyCollector dc = new DummyCollector(); + assertSame(dc, MultiCollector.wrap(dc)); + assertSame(dc, MultiCollector.wrap(dc, null)); + } + @Test public void testCollector() throws Exception { // Tests that the collector delegates calls to input collectors properly. @@ -85,7 +94,7 @@ public class ChainingCollectorTest extends LuceneTestCaseJ4 { // Tests that the collector handles some null collectors well. If it // doesn't, an NPE would be thrown. DummyCollector[] dcs = new DummyCollector[] { new DummyCollector(), new DummyCollector() }; - Collector c = new ChainingCollector(dcs); + Collector c = MultiCollector.wrap(dcs); assertTrue(c.acceptsDocsOutOfOrder()); c.collect(1); c.setNextReader(null, 0);