Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 995858) +++ lucene/CHANGES.txt (working copy) @@ -371,8 +371,8 @@ 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) Optimizations Index: lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java =================================================================== --- lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java (revision 995728) +++ lucene/src/test/org/apache/lucene/search/MultiCollectorTest.java (working copy) @@ -27,7 +27,7 @@ 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,7 +63,7 @@ public void testNullCollectors() throws Exception { // Tests that the collector rejects all null collectors. try { - new ChainingCollector(null, null); + MultiCollector.wrap(null, null); fail("all collectors null should not be supported"); } catch (IllegalArgumentException e) { // expected @@ -71,7 +71,8 @@ // 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); @@ -79,13 +80,21 @@ } @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. // 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); Index: lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java =================================================================== --- lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java (revision 995858) +++ lucene/src/test/org/apache/lucene/search/ChainingCollectorTest.java (working copy) @@ -1,103 +0,0 @@ -package org.apache.lucene.search; - -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import static org.junit.Assert.*; - -import java.io.IOException; - -import org.apache.lucene.index.IndexReader; -import org.apache.lucene.search.Collector; -import org.apache.lucene.search.Scorer; -import org.apache.lucene.util.LuceneTestCaseJ4; -import org.junit.Test; - -public class ChainingCollectorTest extends LuceneTestCaseJ4 { - - private static class DummyCollector extends Collector { - - boolean acceptsDocsOutOfOrderCalled = false; - boolean collectCalled = false; - boolean setNextReaderCalled = false; - boolean setScorerCalled = false; - - @Override - public boolean acceptsDocsOutOfOrder() { - acceptsDocsOutOfOrderCalled = true; - return true; - } - - @Override - public void collect(int doc) throws IOException { - collectCalled = true; - } - - @Override - public void setNextReader(IndexReader reader, int docBase) throws IOException { - setNextReaderCalled = true; - } - - @Override - public void setScorer(Scorer scorer) throws IOException { - setScorerCalled = true; - } - - } - - @Test - 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"); - } 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()); - assertTrue(c.acceptsDocsOutOfOrder()); - c.collect(1); - c.setNextReader(null, 0); - c.setScorer(null); - } - - @Test - public void testCollector() throws Exception { - // Tests that the collector delegates calls to input collectors properly. - - // 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); - assertTrue(c.acceptsDocsOutOfOrder()); - c.collect(1); - c.setNextReader(null, 0); - c.setScorer(null); - - for (DummyCollector dc : dcs) { - assertTrue(dc.acceptsDocsOutOfOrderCalled); - assertTrue(dc.collectCalled); - assertTrue(dc.setNextReaderCalled); - assertTrue(dc.setScorerCalled); - } - - } - -} Index: lucene/src/java/org/apache/lucene/search/ChainingCollector.java =================================================================== --- lucene/src/java/org/apache/lucene/search/ChainingCollector.java (revision 995858) +++ lucene/src/java/org/apache/lucene/search/ChainingCollector.java (working copy) @@ -1,100 +0,0 @@ -package org.apache.lucene.search; - -/** - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import java.io.IOException; - -import org.apache.lucene.index.IndexReader; -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. - */ -public class ChainingCollector extends Collector { - - private final Collector[] collectors; - - public ChainingCollector(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. - int n = 0; - for (Collector c : collectors) { - if (c != null) { - n++; - } - } - - if (n == 0) { - throw new IllegalArgumentException("At least 1 collector must not be null"); - } else if (n == collectors.length) { - // No null collectors, can use the given list as is. - this.collectors = collectors; - } else { - this.collectors = new Collector[n]; - n = 0; - for (Collector c : collectors) { - if (c != null) { - this.collectors[n++] = c; - } - } - } - } - - @Override - public boolean acceptsDocsOutOfOrder() { - for (Collector c : collectors) { - if (!c.acceptsDocsOutOfOrder()) { - return false; - } - } - return true; - } - - @Override - public void collect(int doc) throws IOException { - for (Collector c : collectors) { - c.collect(doc); - } - } - - @Override - public void setNextReader(IndexReader reader, int o) throws IOException { - for (Collector c : collectors) { - c.setNextReader(reader, o); - } - } - - @Override - public void setScorer(Scorer s) throws IOException { - for (Collector c : collectors) { - c.setScorer(s); - } - } - -} Index: lucene/src/java/org/apache/lucene/search/MultiCollector.java =================================================================== --- lucene/src/java/org/apache/lucene/search/MultiCollector.java (revision 995728) +++ lucene/src/java/org/apache/lucene/search/MultiCollector.java (working copy) @@ -24,22 +24,30 @@ 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,20 +60,36 @@ 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() { for (Collector c : collectors) {