diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java index 4fe6def655..ab38562e4e 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/IndexVersionCheck.java @@ -36,8 +36,8 @@ import java.util.Map; public class IndexVersionCheck implements LifecycleListener { public static final Map SCHEMA_VERSIONS = ImmutableMap.of( - "changes_open", ChangeField.SCHEMA_VERSION, - "changes_closed", ChangeField.SCHEMA_VERSION); + LuceneChangeIndex.CHANGES_OPEN, ChangeField.SCHEMA_VERSION, + LuceneChangeIndex.CHANGES_CLOSED, ChangeField.SCHEMA_VERSION); public static File gerritIndexConfig(SitePaths sitePaths) { return new File(sitePaths.index_dir, "gerrit_index.config"); diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index ce54b35af0..f67a76f861 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -15,13 +15,18 @@ package com.google.gerrit.lucene; import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_CHANGE; +import static com.google.gerrit.server.query.change.IndexRewriteImpl.CLOSED_STATUSES; +import static com.google.gerrit.server.query.change.IndexRewriteImpl.OPEN_STATUSES; import static org.apache.lucene.search.BooleanClause.Occur.MUST; import static org.apache.lucene.search.BooleanClause.Occur.MUST_NOT; import static org.apache.lucene.search.BooleanClause.Occur.SHOULD; import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.config.SitePaths; import com.google.gerrit.server.index.ChangeField; import com.google.gerrit.server.index.ChangeIndex; import com.google.gerrit.server.index.FieldDef; @@ -35,38 +40,33 @@ import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeDataSource; +import com.google.gerrit.server.query.change.IndexRewriteImpl; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; +import com.google.inject.Inject; +import com.google.inject.Singleton; -import org.apache.lucene.analysis.standard.StandardAnalyzer; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.document.IntField; import org.apache.lucene.document.StringField; import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.index.IndexWriterConfig; -import org.apache.lucene.index.IndexWriterConfig.OpenMode; import org.apache.lucene.index.Term; import org.apache.lucene.search.BooleanClause; import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; -import org.apache.lucene.search.ScoreDoc; import org.apache.lucene.search.SearcherManager; import org.apache.lucene.search.TermQuery; -import org.apache.lucene.store.Directory; -import org.apache.lucene.store.FSDirectory; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.NumericUtils; import org.apache.lucene.util.Version; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.io.File; import java.io.IOException; import java.util.Collections; import java.util.Iterator; import java.util.List; +import java.util.Set; /** * Secondary index implementation using Apache Lucene. @@ -76,81 +76,86 @@ import java.util.List; * though there may be some lag between a committed write and it showing up to * other threads' searchers. */ -public class LuceneChangeIndex implements ChangeIndex { - private static final Logger log = - LoggerFactory.getLogger(LuceneChangeIndex.class); - +@Singleton +public class LuceneChangeIndex implements ChangeIndex, LifecycleListener { public static final Version LUCENE_VERSION = Version.LUCENE_43; + public static final String CHANGES_OPEN = "changes_open"; + public static final String CHANGES_CLOSED = "changes_closed"; private final FillArgs fillArgs; - private final Directory dir; - private final IndexWriter writer; - private final SearcherManager searcherManager; + private final SubIndex openIndex; + private final SubIndex closedIndex; - LuceneChangeIndex(File file, FillArgs fillArgs) throws IOException { + @Inject + LuceneChangeIndex(SitePaths sitePaths, FillArgs fillArgs) throws IOException { this.fillArgs = fillArgs; - dir = FSDirectory.open(file); - IndexWriterConfig writerConfig = - new IndexWriterConfig(LUCENE_VERSION, new StandardAnalyzer(LUCENE_VERSION)); - writerConfig.setOpenMode(OpenMode.CREATE_OR_APPEND); - writer = new IndexWriter(dir, writerConfig); - searcherManager = new SearcherManager(writer, true, null); + openIndex = new SubIndex(new File(sitePaths.index_dir, CHANGES_OPEN)); + closedIndex = new SubIndex(new File(sitePaths.index_dir, CHANGES_CLOSED)); } - void close() { - try { - searcherManager.close(); - } catch (IOException e) { - log.warn("error closing Lucene searcher", e); - } - try { - writer.close(true); - } catch (IOException e) { - log.warn("error closing Lucene writer", e); - } - try { - dir.close(); - } catch (IOException e) { - log.warn("error closing Lucene directory", e); - } + @Override + public void start() { + // Do nothing. + } + + @Override + public void stop() { + openIndex.close(); + closedIndex.close(); } @Override public void insert(ChangeData cd) throws IOException { - writer.addDocument(toDocument(cd)); - commit(); + Term id = idTerm(cd); + Document doc = toDocument(cd); + if (cd.getChange().getStatus().isOpen()) { + closedIndex.delete(id); + openIndex.insert(doc); + } else { + openIndex.delete(id); + closedIndex.insert(doc); + } } @Override public void replace(ChangeData cd) throws IOException { - writer.updateDocument(intTerm(FIELD_CHANGE, cd.getId().get()), - toDocument(cd)); - commit(); + Term id = idTerm(cd); + Document doc = toDocument(cd); + if (cd.getChange().getStatus().isOpen()) { + closedIndex.delete(id); + openIndex.replace(id, doc); + } else { + openIndex.delete(id); + closedIndex.replace(id, doc); + } } @Override public void delete(ChangeData cd) throws IOException { - writer.deleteDocuments(intTerm(FIELD_CHANGE, cd.getId().get())); - commit(); + Term id = idTerm(cd); + if (cd.getChange().getStatus().isOpen()) { + openIndex.delete(id); + } else { + closedIndex.delete(id); + } } @Override public ChangeDataSource getSource(Predicate p) throws QueryParseException { - return new QuerySource(toQuery(p)); + Set statuses = IndexRewriteImpl.getPossibleStatus(p); + List indexes = Lists.newArrayListWithCapacity(2); + if (!Sets.intersection(statuses, OPEN_STATUSES).isEmpty()) { + indexes.add(openIndex); + } + if (!Sets.intersection(statuses, CLOSED_STATUSES).isEmpty()) { + indexes.add(closedIndex); + } + return new QuerySource(indexes, toQuery(p)); } - public Directory getDirectory() { - return dir; - } - - public IndexWriter getWriter() { - return writer; - } - - private void commit() throws IOException { - writer.commit(); - searcherManager.maybeRefresh(); + private Term idTerm(ChangeData cd) { + return intTerm(FIELD_CHANGE, cd.getId().get()); } private Query toQuery(Predicate p) throws QueryParseException { @@ -214,9 +219,11 @@ public class LuceneChangeIndex implements ChangeIndex { // TODO(dborowitz): Push limit down from predicate tree. private static final int LIMIT = 1000; + private final List indexes; private final Query query; - public QuerySource(Query query) { + public QuerySource(List indexes, Query query) { + this.indexes = indexes; this.query = query; } @@ -233,36 +240,28 @@ public class LuceneChangeIndex implements ChangeIndex { @Override public ResultSet read() throws OrmException { try { - IndexSearcher searcher = searcherManager.acquire(); - try { - ScoreDoc[] docs = searcher.search(query, LIMIT).scoreDocs; - List result = Lists.newArrayListWithCapacity(docs.length); - for (ScoreDoc sd : docs) { - Document doc = searcher.doc(sd.doc); - Number v = doc.getField(FIELD_CHANGE).numericValue(); - result.add(new ChangeData(new Change.Id(v.intValue()))); - } - final List r = Collections.unmodifiableList(result); - - return new ResultSet() { - @Override - public Iterator iterator() { - return r.iterator(); - } - - @Override - public List toList() { - return r; - } - - @Override - public void close() { - // Do nothing. - } - }; - } finally { - searcherManager.release(searcher); + List result = + Lists.newArrayListWithExpectedSize(2 * getCardinality()); + for (SubIndex index : indexes) { + result.addAll(index.search(query, LIMIT)); } + final List r = Collections.unmodifiableList(result); + return new ResultSet() { + @Override + public Iterator iterator() { + return r.iterator(); + } + + @Override + public List toList() { + return r; + } + + @Override + public void close() { + // Do nothing. + } + }; } catch (IOException e) { throw new OrmException(e); } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java deleted file mode 100644 index 1681914edf..0000000000 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndexManager.java +++ /dev/null @@ -1,70 +0,0 @@ -// Copyright (C) 2013 The Android Open Source Project -// -// Licensed 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.package com.google.gerrit.server.git; - -package com.google.gerrit.lucene; - -import com.google.common.base.Throwables; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.gerrit.extensions.events.LifecycleListener; -import com.google.gerrit.server.config.SitePaths; -import com.google.gerrit.server.index.ChangeIndex; -import com.google.gerrit.server.index.FieldDef.FillArgs; -import com.google.inject.Inject; -import com.google.inject.Singleton; - -import java.io.File; -import java.io.IOException; -import java.util.concurrent.ExecutionException; - -@Singleton -class LuceneChangeIndexManager implements ChangeIndex.Manager, - LifecycleListener { - private final LoadingCache indexes; - - @Inject - LuceneChangeIndexManager(final SitePaths sitePaths, final FillArgs fillArgs) { - indexes = CacheBuilder.newBuilder().build( - new CacheLoader() { - @Override - public LuceneChangeIndex load(String key) throws IOException { - return new LuceneChangeIndex( - new File(sitePaths.index_dir, key), fillArgs); - } - }); - } - - @Override - public void start() { - // Do nothing. - } - - @Override - public void stop() { - for (LuceneChangeIndex index : indexes.asMap().values()) { - index.close(); - } - } - - @Override - public LuceneChangeIndex get(String name) throws IOException { - try { - return indexes.get(name); - } catch (ExecutionException e) { - Throwables.propagateIfInstanceOf(e.getCause(), IOException.class); - throw new IOException(e); - } - } -} diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java index bb92728888..440d874e64 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneIndexModule.java @@ -34,8 +34,8 @@ public class LuceneIndexModule extends LifecycleModule { @Override protected void configure() { install(new IndexModule(threads)); - bind(ChangeIndex.Manager.class).to(LuceneChangeIndexManager.class); - listener().to(LuceneChangeIndexManager.class); + bind(ChangeIndex.class).to(LuceneChangeIndex.class); + listener().to(LuceneChangeIndex.class); if (checkVersion) { listener().to(IndexVersionCheck.class); } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java new file mode 100644 index 0000000000..0a48519e14 --- /dev/null +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/SubIndex.java @@ -0,0 +1,115 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed 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.package com.google.gerrit.server.git; + +package com.google.gerrit.lucene; + +import static com.google.gerrit.lucene.LuceneChangeIndex.LUCENE_VERSION; +import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_CHANGE; + +import com.google.common.collect.Lists; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.query.change.ChangeData; + +import org.apache.lucene.analysis.standard.StandardAnalyzer; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.IndexWriterConfig.OpenMode; +import org.apache.lucene.index.Term; +import org.apache.lucene.search.IndexSearcher; +import org.apache.lucene.search.Query; +import org.apache.lucene.search.ScoreDoc; +import org.apache.lucene.search.SearcherManager; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +/** Piece of the change index that is implemented as a separate Lucene index. */ +class SubIndex { + private static final Logger log = + LoggerFactory.getLogger(LuceneChangeIndex.class); + + private final Directory dir; + private final IndexWriter writer; + private final SearcherManager searcherManager; + + SubIndex(File file) throws IOException { + dir = FSDirectory.open(file); + IndexWriterConfig writerConfig = + new IndexWriterConfig(LUCENE_VERSION, new StandardAnalyzer(LUCENE_VERSION)); + writerConfig.setOpenMode(OpenMode.CREATE_OR_APPEND); + writer = new IndexWriter(dir, writerConfig); + searcherManager = new SearcherManager(writer, true, null); + } + + void close() { + try { + searcherManager.close(); + } catch (IOException e) { + log.warn("error closing Lucene searcher", e); + } + try { + writer.close(true); + } catch (IOException e) { + log.warn("error closing Lucene writer", e); + } + try { + dir.close(); + } catch (IOException e) { + log.warn("error closing Lucene directory", e); + } + } + + void insert(Document doc) throws IOException { + writer.addDocument(doc); + commit(); + } + + void replace(Term term, Document doc) throws IOException { + writer.updateDocument(term, doc); + commit(); + } + + void delete(Term term) throws IOException { + writer.deleteDocuments(term); + commit(); + } + + List search(Query query, int limit) throws IOException { + IndexSearcher searcher = searcherManager.acquire(); + try { + ScoreDoc[] docs = searcher.search(query, limit).scoreDocs; + List result = Lists.newArrayListWithCapacity(docs.length); + for (ScoreDoc sd : docs) { + Document doc = searcher.doc(sd.doc); + Number v = doc.getField(FIELD_CHANGE).numericValue(); + result.add(new ChangeData(new Change.Id(v.intValue()))); + } + return Collections.unmodifiableList(result); + } finally { + searcherManager.release(searcher); + } + } + + private void commit() throws IOException { + writer.commit(); + searcherManager.maybeRefresh(); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java index da2e4515d3..aabe77130d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndex.java @@ -32,38 +32,29 @@ import java.io.IOException; * appropriate. */ public interface ChangeIndex { - public static interface Manager { - /** Instance indicating secondary index is disabled. */ - public static final Manager DISABLED = new Manager() { - @Override - public ChangeIndex get(String name) throws IOException { - return new ChangeIndex() { - @Override - public void insert(ChangeData cd) throws IOException { - // Do nothing. - } + /** Instance indicating secondary index is disabled. */ + public static final ChangeIndex DISABLED = new ChangeIndex() { + @Override + public void insert(ChangeData cd) throws IOException { + // Do nothing. + } - @Override - public void replace(ChangeData cd) throws IOException { - // Do nothing. - } + @Override + public void replace(ChangeData cd) throws IOException { + // Do nothing. + } - @Override - public void delete(ChangeData cd) throws IOException { - // Do nothing. - } + @Override + public void delete(ChangeData cd) throws IOException { + // Do nothing. + } - @Override - public ChangeDataSource getSource(Predicate p) - throws QueryParseException { - throw new UnsupportedOperationException(); - } - }; - } - }; - - ChangeIndex get(String name) throws IOException; - } + @Override + public ChangeDataSource getSource(Predicate p) + throws QueryParseException { + throw new UnsupportedOperationException(); + } + }; /** * Insert a change document into the index. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java index 7a9e869572..2e4dee0d67 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexerImpl.java @@ -37,15 +37,13 @@ public class ChangeIndexerImpl implements ChangeIndexer { LoggerFactory.getLogger(ChangeIndexerImpl.class); private final ListeningScheduledExecutorService executor; - private final ChangeIndex openIndex; - private final ChangeIndex closedIndex; + private final ChangeIndex index; @Inject ChangeIndexerImpl(@IndexExecutor ListeningScheduledExecutorService executor, - ChangeIndex.Manager indexManager) throws IOException { + ChangeIndex index) throws IOException { this.executor = executor; - this.openIndex = indexManager.get("changes_open"); - this.closedIndex = indexManager.get("changes_closed"); + this.index = index; } @Override @@ -74,13 +72,7 @@ public class ChangeIndexerImpl implements ChangeIndexer { public void run() { ChangeData cd = new ChangeData(change); try { - if (change.getStatus().isOpen()) { - closedIndex.delete(cd); - openIndex.replace(cd); - } else { - openIndex.delete(cd); - closedIndex.replace(cd); - } + index.replace(cd); } catch (IOException e) { log.error("Error indexing change", e); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java index fb7fbe643f..1c4ffa82c8 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/NoIndexModule.java @@ -24,7 +24,7 @@ public class NoIndexModule extends AbstractModule { @Override protected void configure() { - bind(ChangeIndex.Manager.class).toInstance(ChangeIndex.Manager.DISABLED); + bind(ChangeIndex.class).toInstance(ChangeIndex.DISABLED); bind(ChangeIndexer.class).toInstance(ChangeIndexer.DISABLED); bind(IndexRewrite.class).toInstance(IndexRewrite.DISABLED); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java index f635da6cd7..1cddf82ed5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/PredicateWrapper.java @@ -14,9 +14,6 @@ package com.google.gerrit.server.index; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.gerrit.server.query.change.ChangeData; @@ -25,9 +22,6 @@ import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; /** * Wrapper combining an {@link IndexPredicate} together with a @@ -40,68 +34,27 @@ import java.util.List; public class PredicateWrapper extends Predicate implements ChangeDataSource { private final Predicate pred; - private final List sources; + private final ChangeDataSource source; - public PredicateWrapper(Predicate pred, ChangeIndex index) + public PredicateWrapper(ChangeIndex index, Predicate pred) throws QueryParseException { - this(pred, ImmutableList.of(index)); - } - - public PredicateWrapper(Predicate pred, - Collection indexes) throws QueryParseException { this.pred = pred; - sources = Lists.newArrayListWithCapacity(indexes.size()); - for (ChangeIndex index : indexes) { - sources.add(index.getSource(pred)); - } + this.source = index.getSource(pred); } @Override public int getCardinality() { - int n = 0; - for (ChangeDataSource source : sources) { - n += source.getCardinality(); - } - return n; + return source.getCardinality(); } @Override public boolean hasChange() { - for (ChangeDataSource source : sources) { - if (!source.hasChange()) { - return false; - } - } - return true; + return source.hasChange(); } @Override public ResultSet read() throws OrmException { - final List> results = - Lists.newArrayListWithCapacity(sources.size()); - for (ChangeDataSource source : sources) { - results.add(source.read()); - } - return new ResultSet() { - @Override - public Iterator iterator() { - // TODO(dborowitz): May return duplicates since moving a document - // between indexes is not atomic. - return Iterables.concat(results).iterator(); - } - - @Override - public List toList() { - return Collections.unmodifiableList(Lists.newArrayList(iterator())); - } - - @Override - public void close() { - for (ResultSet rs : results) { - rs.close(); - } - } - }; + return source.read(); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index d220b83ba7..69207ee19c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -114,7 +114,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final PatchListCache patchListCache; final GitRepositoryManager repoManager; final ProjectCache projectCache; - final ChangeIndex.Manager indexManager; + final ChangeIndex index; @Inject Arguments(Provider dbProvider, @@ -128,7 +128,7 @@ public class ChangeQueryBuilder extends QueryBuilder { PatchListCache patchListCache, GitRepositoryManager repoManager, ProjectCache projectCache, - ChangeIndex.Manager indexManager) { + ChangeIndex index) { this.dbProvider = dbProvider; this.rewriter = rewriter; this.userFactory = userFactory; @@ -140,7 +140,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.patchListCache = patchListCache; this.repoManager = repoManager; this.projectCache = projectCache; - this.indexManager = indexManager; + this.index = index; } } @@ -298,7 +298,7 @@ public class ChangeQueryBuilder extends QueryBuilder { if (file.startsWith("^")) { throw error("regular expression not permitted here: file:" + file); } - if (args.indexManager == ChangeIndex.Manager.DISABLED) { + if (args.index == ChangeIndex.DISABLED) { throw error("secondary index must be enabled for file:" + file); } return new EqualsFilePredicate(args.dbProvider, args.patchListCache, file); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java index 48f28013e0..66670545e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IndexRewriteImpl.java @@ -14,7 +14,6 @@ package com.google.gerrit.server.query.change; -import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.collect.Sets; import com.google.gerrit.reviewdb.client.Change; @@ -28,7 +27,6 @@ import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; import com.google.inject.Inject; -import java.io.IOException; import java.util.BitSet; import java.util.EnumSet; import java.util.List; @@ -36,8 +34,11 @@ import java.util.Set; /** Rewriter that pushes boolean logic into the secondary index. */ public class IndexRewriteImpl implements IndexRewrite { - private static final Set OPEN_STATUSES; - private static final Set CLOSED_STATUSES; + /** Set of all open change statuses. */ + public static final Set OPEN_STATUSES; + + /** Set of all closed change statuses. */ + public static final Set CLOSED_STATUSES; static { EnumSet open = EnumSet.noneOf(Change.Status.class); @@ -53,18 +54,44 @@ public class IndexRewriteImpl implements IndexRewrite { CLOSED_STATUSES = Sets.immutableEnumSet(closed); } - private final ChangeIndex openIndex; - private final ChangeIndex closedIndex; - - @Inject - IndexRewriteImpl(ChangeIndex.Manager indexManager) throws IOException { - this(indexManager.get("changes_open"), indexManager.get("changes_closed")); + /** + * Get the set of statuses that changes matching the given predicate may have. + * + * @param in predicate + * @return the maximal set of statuses that any changes matching the input + * predicates may have, based on examining boolean and + * {@link ChangeStatusPredicate}s. + */ + public static EnumSet getPossibleStatus(Predicate in) { + if (in instanceof ChangeStatusPredicate) { + return EnumSet.of(((ChangeStatusPredicate) in).getStatus()); + } else if (in.getClass() == NotPredicate.class) { + return EnumSet.complementOf(getPossibleStatus(in.getChild(0))); + } else if (in.getClass() == OrPredicate.class) { + EnumSet s = EnumSet.noneOf(Change.Status.class); + for (int i = 0; i < in.getChildCount(); i++) { + s.addAll(getPossibleStatus(in.getChild(i))); + } + return s; + } else if (in.getClass() == AndPredicate.class) { + EnumSet s = EnumSet.allOf(Change.Status.class); + for (int i = 0; i < in.getChildCount(); i++) { + s.retainAll(getPossibleStatus(in.getChild(i))); + } + return s; + } else if (in.getChildCount() == 0) { + return EnumSet.allOf(Change.Status.class); + } else { + throw new IllegalStateException( + "Invalid predicate type in change index query: " + in.getClass()); + } } - @VisibleForTesting - IndexRewriteImpl(ChangeIndex openIndex, ChangeIndex closedIndex) { - this.openIndex = openIndex; - this.closedIndex = closedIndex; + private final ChangeIndex index; + + @Inject + IndexRewriteImpl(ChangeIndex index) { + this.index = index; } @Override @@ -181,43 +208,9 @@ public class IndexRewriteImpl implements IndexRewrite { } } - @VisibleForTesting - static EnumSet getPossibleStatus(Predicate in) { - if (in instanceof ChangeStatusPredicate) { - return EnumSet.of(((ChangeStatusPredicate) in).getStatus()); - } else if (in.getClass() == NotPredicate.class) { - return EnumSet.complementOf(getPossibleStatus(in.getChild(0))); - } else if (in.getClass() == OrPredicate.class) { - EnumSet s = EnumSet.noneOf(Change.Status.class); - for (int i = 0; i < in.getChildCount(); i++) { - s.addAll(getPossibleStatus(in.getChild(i))); - } - return s; - } else if (in.getClass() == AndPredicate.class) { - EnumSet s = EnumSet.allOf(Change.Status.class); - for (int i = 0; i < in.getChildCount(); i++) { - s.retainAll(getPossibleStatus(in.getChild(i))); - } - return s; - } else if (in.getChildCount() == 0) { - return EnumSet.allOf(Change.Status.class); - } else { - throw new IllegalStateException( - "Invalid predicate type in change index query: " + in.getClass()); - } - } - private PredicateWrapper wrap(Predicate p) { try { - Set possibleStatus = getPossibleStatus(p); - List indexes = Lists.newArrayListWithCapacity(2); - if (!Sets.intersection(possibleStatus, OPEN_STATUSES).isEmpty()) { - indexes.add(openIndex); - } - if (!Sets.intersection(possibleStatus, CLOSED_STATUSES).isEmpty()) { - indexes.add(closedIndex); - } - return new PredicateWrapper(p, indexes); + return new PredicateWrapper(index, p); } catch (QueryParseException e) { throw new IllegalStateException( "Failed to convert " + p + " to index predicate", e); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java index 877d3d621b..d7f1a608d5 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/query/change/IndexRewriteTest.java @@ -79,22 +79,19 @@ public class IndexRewriteTest extends TestCase { } } - private DummyIndex openIndex; - private DummyIndex closedIndex; + private DummyIndex index; private ChangeQueryBuilder queryBuilder; private IndexRewrite rewrite; @Override public void setUp() throws Exception { super.setUp(); - openIndex = new DummyIndex(); - closedIndex = new DummyIndex(); + index = new DummyIndex(); queryBuilder = new ChangeQueryBuilder( new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, null, null, null, null, null, null), null); - - rewrite = new IndexRewriteImpl(openIndex, closedIndex); + rewrite = new IndexRewriteImpl(index); } public void testIndexPredicate() throws Exception { @@ -208,7 +205,7 @@ public class IndexRewriteTest extends TestCase { private PredicateWrapper wrap(Predicate p) throws QueryParseException { - return new PredicateWrapper(p, openIndex); + return new PredicateWrapper(index, p); } private Set status(String query) throws QueryParseException {