Index: lucene/CHANGES.txt =================================================================== --- lucene/CHANGES.txt (revision 1462883) +++ lucene/CHANGES.txt (working copy) @@ -185,6 +185,9 @@ IndexDeletionPolicy and InfoStream in order to make an IndexWriterConfig and its clone fully independent. (Adrien Grand) +* LUCENE-4893: Facet counts were multiplied as many times as + FacetsCollector.getFacetResults() is called. (Shai Erera) + Documentation * LUCENE-4841: Added example SimpleSortedSetFacetsExample to show how Index: lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java (revision 1462883) +++ lucene/facet/src/java/org/apache/lucene/facet/search/FacetResult.java (working copy) @@ -1,6 +1,5 @@ package org.apache.lucene.facet.search; - /* * Licensed to the Apache Software Foundation (ASF) under one or more * contributor license agreements. See the NOTICE file distributed with Index: lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java (revision 1462883) +++ lucene/facet/src/java/org/apache/lucene/facet/search/FacetsAccumulator.java (working copy) @@ -146,6 +146,14 @@ } return clps; } + + private FacetResult emptyResult(int ordinal, FacetRequest fr) { + FacetResultNode root = new FacetResultNode(); + root.ordinal = ordinal; + root.label = fr.categoryPath; + root.value = 0; + return new FacetResult(fr, root, 0); + } /** * Used by {@link FacetsCollector} to build the list of {@link FacetResult @@ -156,6 +164,14 @@ * the documents that matched the query, per-segment. */ public List accumulate(List matchingDocs) throws IOException { + if (matchingDocs.isEmpty()) { + List res = new ArrayList(); + for (FacetRequest fr : searchParams.facetRequests) { + res.add(emptyResult(taxonomyReader.getOrdinal(fr.categoryPath), fr)); + } + return res; + } + // aggregate facets per category list (usually onle one category list) FacetsAggregator aggregator = getAggregator(); for (CategoryListParams clp : getCategoryLists()) { @@ -173,12 +189,8 @@ for (FacetRequest fr : searchParams.facetRequests) { int rootOrd = taxonomyReader.getOrdinal(fr.categoryPath); if (rootOrd == TaxonomyReader.INVALID_ORDINAL) { // category does not exist - // Add empty FacetResult: - FacetResultNode root = new FacetResultNode(); - root.ordinal = TaxonomyReader.INVALID_ORDINAL; - root.label = fr.categoryPath; - root.value = 0; - res.add(new FacetResult(fr, root, 0)); + // Add empty FacetResult + res.add(emptyResult(rootOrd, fr)); continue; } CategoryListParams clp = searchParams.indexingParams.getCategoryListParams(fr.categoryPath); Index: lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java =================================================================== --- lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java (revision 1462883) +++ lucene/facet/src/java/org/apache/lucene/facet/search/FacetsCollector.java (working copy) @@ -65,6 +65,17 @@ } @Override + protected final void doSetNextReader(AtomicReaderContext context) throws IOException { + if (bits != null) { + matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, scores)); + } + bits = new FixedBitSet(context.reader().maxDoc()); + totalHits = 0; + scores = new float[64]; // some initial size + this.context = context; + } + + @Override public final boolean acceptsDocsOutOfOrder() { return false; } @@ -85,18 +96,6 @@ public final void setScorer(Scorer scorer) throws IOException { this.scorer = scorer; } - - @Override - public final void setNextReader(AtomicReaderContext context) throws IOException { - if (bits != null) { - matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, scores)); - } - bits = new FixedBitSet(context.reader().maxDoc()); - totalHits = 0; - scores = new float[64]; // some initial size - this.context = context; - } - } private final static class DocsOnlyCollector extends FacetsCollector { @@ -119,6 +118,16 @@ } @Override + protected final void doSetNextReader(AtomicReaderContext context) throws IOException { + if (bits != null) { + matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, null)); + } + bits = new FixedBitSet(context.reader().maxDoc()); + totalHits = 0; + this.context = context; + } + + @Override public final boolean acceptsDocsOutOfOrder() { return true; } @@ -131,16 +140,6 @@ @Override public final void setScorer(Scorer scorer) throws IOException {} - - @Override - public final void setNextReader(AtomicReaderContext context) throws IOException { - if (bits != null) { - matchingDocs.add(new MatchingDocs(this.context, bits, totalHits, null)); - } - bits = new FixedBitSet(context.reader().maxDoc()); - totalHits = 0; - this.context = context; - } } /** @@ -184,7 +183,7 @@ private final FacetsAccumulator accumulator; - protected final List matchingDocs = new ArrayList(); + protected List matchingDocs = null; protected FacetsCollector(FacetsAccumulator accumulator) { this.accumulator = accumulator; @@ -197,14 +196,31 @@ protected abstract void finish(); /** + * Perfoms the actual work of {@link #setNextReader(AtomicReaderContext)}. + * When this method is called, {@link #matchingDocs} is guaranteed to not be + * {@code null}. + */ + protected abstract void doSetNextReader(AtomicReaderContext context) throws IOException; + + /** * Returns a {@link FacetResult} per {@link FacetRequest} set in - * {@link FacetSearchParams}. Note that if one of the {@link FacetRequest - * requests} is for a {@link CategoryPath} that does not exist in the taxonomy, - * no matching {@link FacetResult} will be returned. + * {@link FacetSearchParams}. Note that if a {@link FacetRequest} defines a + * {@link CategoryPath} which does not exist in the taxonomy, an empty + * {@link FacetResult} will be returned for it. + * + *

+ * NOTE: you are expected to call this method only once per search. + * Calling it more than once yields empty {@link FacetResult} per + * {@link FacetRequest}. */ public final List getFacetResults() throws IOException { + if (matchingDocs == null) { + throw new IllegalStateException("either no search was executed, or you called getFacetResults more than once"); + } finish(); - return accumulator.accumulate(matchingDocs); + List result = accumulator.accumulate(matchingDocs); + matchingDocs = null; + return result; } /** @@ -212,6 +228,9 @@ * visited segment. */ public final List getMatchingDocs() { + if (matchingDocs == null) { + throw new IllegalStateException("either no search was executed, or you called getFacetResults more than once"); + } finish(); return matchingDocs; } @@ -223,7 +242,15 @@ */ public final void reset() { finish(); - matchingDocs.clear(); + matchingDocs = null; } + @Override + public final void setNextReader(AtomicReaderContext context) throws IOException { + if (matchingDocs == null) { + matchingDocs = new ArrayList(); + } + doSetNextReader(context); + } + } Index: lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java =================================================================== --- lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java (revision 1462883) +++ lucene/facet/src/test/org/apache/lucene/facet/search/TestFacetsCollector.java (working copy) @@ -230,4 +230,48 @@ IOUtils.close(taxo, taxoDir, r, indexDir); } + @Test + public void testGetFacetResultsTwice() throws Exception { + // LUCENE-4893: counts were multiplied as many times as getFacetResults was called. + Directory indexDir = newDirectory(); + Directory taxoDir = newDirectory(); + + TaxonomyWriter taxonomyWriter = new DirectoryTaxonomyWriter(taxoDir); + IndexWriter iw = new IndexWriter(indexDir, newIndexWriterConfig(TEST_VERSION_CURRENT, new MockAnalyzer(random()))); + + FacetFields facetFields = new FacetFields(taxonomyWriter); + Document doc = new Document(); + facetFields.addFields(doc, Arrays.asList(new CategoryPath("a/1", '/'), new CategoryPath("b/1", '/'))); + iw.addDocument(doc); + taxonomyWriter.close(); + iw.close(); + + DirectoryReader r = DirectoryReader.open(indexDir); + DirectoryTaxonomyReader taxo = new DirectoryTaxonomyReader(taxoDir); + + FacetSearchParams fsp = new FacetSearchParams( + new CountFacetRequest(new CategoryPath("a"), 10), + new CountFacetRequest(new CategoryPath("b"), 10)); + final FacetsAccumulator fa = random().nextBoolean() ? new FacetsAccumulator(fsp, r, taxo) : new StandardFacetsAccumulator(fsp, r, taxo); + final FacetsCollector fc = FacetsCollector.create(fa); + try { + fc.getFacetResults(); + fail("expected IllegalStateException calling getFacetResults before search"); + } catch (IllegalStateException e) { + // expected + } + + new IndexSearcher(r).search(new MatchAllDocsQuery(), fc); + + fc.getFacetResults(); + try { + fc.getFacetResults(); + fail("expected IllegalStateException calling getFacetResults more than once"); + } catch (IllegalStateException e) { + // expected + } + + IOUtils.close(taxo, taxoDir, r, indexDir); + } + }