Add LuceneQuerySource to AbstractLuceneIndex

This commit is a no-op change that reduces duplicate code in the Lucene indices by creating a shared implementation of QuerySource. Change-Id: I5aa921af8c0611ed086ad82a6221ae8d013c1c9c
2017-11-16 09:58:18 +01:00
parent 4485711ffe
commit 7e3ea658f2
6 changed files with 109 additions and 275 deletions
--- a/java/com/google/gerrit/index/QueryOptions.java
+++ b/java/com/google/gerrit/index/QueryOptions.java
@@ -20,6 +20,7 @@ import com.google.auto.value.AutoValue;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.primitives.Ints;
 import java.util.Set;
+import java.util.function.Function;

@AutoValue
 public abstract class QueryOptions {
@@ -53,4 +54,8 @@ public abstract class QueryOptions {
  public QueryOptions withStart(int newStart) {
    return create(config(), newStart, limit(), fields());
  }
+
+  public QueryOptions filterFields(Function<QueryOptions, Set<String>> filter) {
+    return create(config(), start(), limit(), filter.apply(this));
+  }
 }
--- a/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
+++ b/java/com/google/gerrit/lucene/AbstractLuceneIndex.java
@@ -31,13 +31,21 @@ import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.gerrit.index.FieldDef;
 import com.google.gerrit.index.FieldType;
 import com.google.gerrit.index.Index;
+import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.Schema.Values;
+import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.index.IndexUtils;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.ResultSet;
 import java.io.IOException;
 import java.sql.Timestamp;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -48,6 +56,7 @@ import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.function.Function;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.Field.Store;
@@ -62,9 +71,13 @@ import org.apache.lucene.index.Term;
 import org.apache.lucene.index.TrackingIndexWriter;
 import org.apache.lucene.search.ControlledRealTimeReopenThread;
 import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.Query;
 import org.apache.lucene.search.ReferenceManager;
 import org.apache.lucene.search.ReferenceManager.RefreshListener;
+import org.apache.lucene.search.ScoreDoc;
 import org.apache.lucene.search.SearcherFactory;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TopFieldDocs;
 import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.slf4j.Logger;
@@ -300,6 +313,8 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
    return result;
  }

+  protected abstract V fromDocument(Document doc);
+
  void add(Document doc, Values<V> values) {
    String name = values.getField().getName();
    FieldType<?> type = values.getField().getType();
@@ -445,4 +460,76 @@ public abstract class AbstractLuceneIndex<K, V> implements Index<K, V> {
  public Schema<V> getSchema() {
    return schema;
  }
+
+  protected class LuceneQuerySource implements DataSource<V> {
+    private final QueryOptions opts;
+    private final Query query;
+    private final Sort sort;
+
+    LuceneQuerySource(QueryOptions opts, Query query, Sort sort) {
+      this.opts = opts;
+      this.query = query;
+      this.sort = sort;
+    }
+
+    @Override
+    public int getCardinality() {
+      return 10;
+    }
+
+    @Override
+    public ResultSet<V> read() throws OrmException {
+      return readImpl((doc) -> fromDocument(doc));
+    }
+
+    @Override
+    public ResultSet<FieldBundle> readRaw() throws OrmException {
+      return readImpl(AbstractLuceneIndex.this::toFieldBundle);
+    }
+
+    private <T> ResultSet<T> readImpl(Function<Document, T> mapper) throws OrmException {
+      IndexSearcher searcher = null;
+      try {
+        searcher = acquire();
+        int realLimit = opts.start() + opts.limit();
+        TopFieldDocs docs = searcher.search(query, realLimit, sort);
+        List<T> result = new ArrayList<>(docs.scoreDocs.length);
+        for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
+          ScoreDoc sd = docs.scoreDocs[i];
+          Document doc = searcher.doc(sd.doc, opts.fields());
+          T mapperResult = mapper.apply(doc);
+          if (mapperResult != null) {
+            result.add(mapperResult);
+          }
+        }
+        final List<T> r = Collections.unmodifiableList(result);
+        return new ResultSet<T>() {
+          @Override
+          public Iterator<T> iterator() {
+            return r.iterator();
+          }
+
+          @Override
+          public List<T> toList() {
+            return r;
+          }
+
+          @Override
+          public void close() {
+            // Do nothing.
+          }
+        };
+      } catch (IOException e) {
+        throw new OrmException(e);
+      } finally {
+        if (searcher != null) {
+          try {
+            release(searcher);
+          } catch (IOException e) {
+            log.warn("cannot release Lucene searcher", e);
+          }
+        }
+      }
+    }
+  }
 }
--- a/java/com/google/gerrit/lucene/ChangeSubIndex.java
+++ b/java/com/google/gerrit/lucene/ChangeSubIndex.java
@@ -105,4 +105,9 @@ public class ChangeSubIndex extends AbstractLuceneIndex<Change.Id, ChangeData>
    }
    super.add(doc, values);
  }
+
+  @Override
+  protected ChangeData fromDocument(Document doc) {
+    throw new UnsupportedOperationException("don't use ChangeSubIndex directly");
+  }
 }
--- a/java/com/google/gerrit/lucene/LuceneAccountIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneAccountIndex.java
@@ -19,7 +19,6 @@ import static com.google.gerrit.server.index.account.AccountField.ID;
 import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.DataSource;
-import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.Account;
@@ -29,39 +28,24 @@ import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.index.IndexUtils;
 import com.google.gerrit.server.index.account.AccountIndex;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
 import java.util.concurrent.ExecutionException;
-import java.util.function.Function;
 import org.apache.lucene.document.Document;
 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.SearcherFactory;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
-import org.apache.lucene.search.TopFieldDocs;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.store.RAMDirectory;
 import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;

 public class LuceneAccountIndex extends AbstractLuceneIndex<Account.Id, AccountState>
    implements AccountIndex {
-  private static final Logger log = LoggerFactory.getLogger(LuceneAccountIndex.class);
-
  private static final String ACCOUNTS = "accounts";

  private static final String ID_SORT_FIELD = sortFieldName(ID);
@@ -129,88 +113,14 @@ public class LuceneAccountIndex extends AbstractLuceneIndex<Account.Id, AccountS
  @Override
  public DataSource<AccountState> getSource(Predicate<AccountState> p, QueryOptions opts)
      throws QueryParseException {
-    return new QuerySource(
-        opts,
+    return new LuceneQuerySource(
+        opts.filterFields(IndexUtils::accountFields),
        queryBuilder.toQuery(p),
        new Sort(new SortField(ID_SORT_FIELD, SortField.Type.LONG, true)));
  }

-  private class QuerySource implements DataSource<AccountState> {
-    private final QueryOptions opts;
-    private final Query query;
-    private final Sort sort;
-
-    private QuerySource(QueryOptions opts, Query query, Sort sort) {
-      this.opts = opts;
-      this.query = query;
-      this.sort = sort;
-    }
-
-    @Override
-    public int getCardinality() {
-      // TODO(dborowitz): In contrast to the comment in
-      // LuceneChangeIndex.QuerySource#getCardinality, at this point I actually
-      // think we might just want to remove getCardinality.
-      return 10;
-    }
-
-    @Override
-    public ResultSet<AccountState> read() throws OrmException {
-      return readImpl(LuceneAccountIndex.this::toAccountState);
-    }
-
-    @Override
-    public ResultSet<FieldBundle> readRaw() throws OrmException {
-      return readImpl(LuceneAccountIndex.this::toFieldBundle);
-    }
-
-    private <T> ResultSet<T> readImpl(Function<Document, T> mapper) throws OrmException {
-      IndexSearcher searcher = null;
-      try {
-        searcher = acquire();
-        int realLimit = opts.start() + opts.limit();
-        TopFieldDocs docs = searcher.search(query, realLimit, sort);
-        List<T> result = new ArrayList<>(docs.scoreDocs.length);
-        for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
-          ScoreDoc sd = docs.scoreDocs[i];
-          Document doc = searcher.doc(sd.doc, IndexUtils.accountFields(opts));
-          T mapperResult = mapper.apply(doc);
-          if (mapperResult != null) {
-            result.add(mapperResult);
-          }
-        }
-        final List<T> r = Collections.unmodifiableList(result);
-        return new ResultSet<T>() {
-          @Override
-          public Iterator<T> iterator() {
-            return r.iterator();
-          }
-
-          @Override
-          public List<T> toList() {
-            return r;
-          }
-
-          @Override
-          public void close() {
-            // Do nothing.
-          }
-        };
-      } catch (IOException e) {
-        throw new OrmException(e);
-      } finally {
-        if (searcher != null) {
-          try {
-            release(searcher);
-          } catch (IOException e) {
-            log.warn("cannot release Lucene searcher", e);
-          }
-        }
-      }
-    }
-  }
-
-  private AccountState toAccountState(Document doc) {
+  @Override
+  protected AccountState fromDocument(Document doc) {
    Account.Id id = new Account.Id(doc.getField(ID.getName()).numericValue().intValue());
    // Use the AccountCache rather than depending on any stored fields in the
    // document (of which there shouldn't be any). The most expensive part to
--- a/java/com/google/gerrit/lucene/LuceneGroupIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneGroupIndex.java
@@ -19,7 +19,6 @@ import static com.google.gerrit.server.index.group.GroupField.UUID;
 import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.DataSource;
-import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -29,38 +28,24 @@ import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.group.InternalGroup;
 import com.google.gerrit.server.index.IndexUtils;
 import com.google.gerrit.server.index.group.GroupIndex;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
 import java.util.concurrent.ExecutionException;
-import java.util.function.Function;
 import org.apache.lucene.document.Document;
 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.SearcherFactory;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
-import org.apache.lucene.search.TopFieldDocs;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.store.RAMDirectory;
 import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;

 public class LuceneGroupIndex extends AbstractLuceneIndex<AccountGroup.UUID, InternalGroup>
    implements GroupIndex {
-  private static final Logger log = LoggerFactory.getLogger(LuceneGroupIndex.class);

  private static final String GROUPS = "groups";

@@ -129,85 +114,14 @@ public class LuceneGroupIndex extends AbstractLuceneIndex<AccountGroup.UUID, Int
  @Override
  public DataSource<InternalGroup> getSource(Predicate<InternalGroup> p, QueryOptions opts)
      throws QueryParseException {
-    return new QuerySource(
-        opts,
+    return new LuceneQuerySource(
+        opts.filterFields(IndexUtils::groupFields),
        queryBuilder.toQuery(p),
        new Sort(new SortField(UUID_SORT_FIELD, SortField.Type.STRING, false)));
  }

-  private class QuerySource implements DataSource<InternalGroup> {
-    private final QueryOptions opts;
-    private final Query query;
-    private final Sort sort;
-
-    private QuerySource(QueryOptions opts, Query query, Sort sort) {
-      this.opts = opts;
-      this.query = query;
-      this.sort = sort;
-    }
-
-    @Override
-    public int getCardinality() {
-      return 10;
-    }
-
-    @Override
-    public ResultSet<InternalGroup> read() throws OrmException {
-      return readImpl(LuceneGroupIndex.this::toInternalGroup);
-    }
-
-    @Override
-    public ResultSet<FieldBundle> readRaw() throws OrmException {
-      return readImpl(LuceneGroupIndex.this::toFieldBundle);
-    }
-
-    private <T> ResultSet<T> readImpl(Function<Document, T> mapper) throws OrmException {
-      IndexSearcher searcher = null;
-      try {
-        searcher = acquire();
-        int realLimit = opts.start() + opts.limit();
-        TopFieldDocs docs = searcher.search(query, realLimit, sort);
-        List<T> result = new ArrayList<>(docs.scoreDocs.length);
-        for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
-          ScoreDoc sd = docs.scoreDocs[i];
-          Document doc = searcher.doc(sd.doc, IndexUtils.groupFields(opts));
-          T mapperResult = mapper.apply(doc);
-          if (mapperResult != null) {
-            result.add(mapperResult);
-          }
-        }
-        final List<T> r = Collections.unmodifiableList(result);
-        return new ResultSet<T>() {
-          @Override
-          public Iterator<T> iterator() {
-            return r.iterator();
-          }
-
-          @Override
-          public List<T> toList() {
-            return r;
-          }
-
-          @Override
-          public void close() {
-            // Do nothing.
-          }
-        };
-      } catch (IOException e) {
-        throw new OrmException(e);
-      } finally {
-        if (searcher != null) {
-          try {
-            release(searcher);
-          } catch (IOException e) {
-            log.warn("cannot release Lucene searcher", e);
-          }
-        }
-      }
-    }
-  }
-
-  private InternalGroup toInternalGroup(Document doc) {
+  @Override
+  protected InternalGroup fromDocument(Document doc) {
    AccountGroup.UUID uuid = new AccountGroup.UUID(doc.getField(UUID.getName()).stringValue());
    // Use the GroupCache rather than depending on any stored fields in the
    // document (of which there shouldn't be any).
--- a/java/com/google/gerrit/lucene/LuceneProjectIndex.java
+++ b/java/com/google/gerrit/lucene/LuceneProjectIndex.java
@@ -19,7 +19,6 @@ import static com.google.gerrit.server.index.project.ProjectField.NAME;
 import com.google.gerrit.index.QueryOptions;
 import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.DataSource;
-import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
 import com.google.gerrit.reviewdb.client.Project;
@@ -29,39 +28,24 @@ import com.google.gerrit.server.index.IndexUtils;
 import com.google.gerrit.server.index.project.ProjectIndex;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectData;
-import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
 import java.io.IOException;
 import java.nio.file.Path;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
 import java.util.concurrent.ExecutionException;
-import java.util.function.Function;
 import org.apache.lucene.document.Document;
 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.SearcherFactory;
 import org.apache.lucene.search.Sort;
 import org.apache.lucene.search.SortField;
-import org.apache.lucene.search.TopFieldDocs;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.store.RAMDirectory;
 import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;

 public class LuceneProjectIndex extends AbstractLuceneIndex<Project.NameKey, ProjectData>
    implements ProjectIndex {
-  private static final Logger log = LoggerFactory.getLogger(LuceneProjectIndex.class);
-
  private static final String PROJECTS = "projects";

  private static final String NAME_SORT_FIELD = sortFieldName(NAME);
@@ -129,85 +113,14 @@ public class LuceneProjectIndex extends AbstractLuceneIndex<Project.NameKey, Pro
  @Override
  public DataSource<ProjectData> getSource(Predicate<ProjectData> p, QueryOptions opts)
      throws QueryParseException {
-    return new QuerySource(
-        opts,
+    return new LuceneQuerySource(
+        opts.filterFields(IndexUtils::projectFields),
        queryBuilder.toQuery(p),
        new Sort(new SortField(NAME_SORT_FIELD, SortField.Type.STRING, false)));
  }

-  private class QuerySource implements DataSource<ProjectData> {
-    private final QueryOptions opts;
-    private final Query query;
-    private final Sort sort;
-
-    private QuerySource(QueryOptions opts, Query query, Sort sort) {
-      this.opts = opts;
-      this.query = query;
-      this.sort = sort;
-    }
-
-    @Override
-    public int getCardinality() {
-      return 10;
-    }
-
-    @Override
-    public ResultSet<ProjectData> read() throws OrmException {
-      return readImpl(LuceneProjectIndex.this::toProjectData);
-    }
-
-    @Override
-    public ResultSet<FieldBundle> readRaw() throws OrmException {
-      return readImpl(LuceneProjectIndex.this::toFieldBundle);
-    }
-
-    private <T> ResultSet<T> readImpl(Function<Document, T> mapper) throws OrmException {
-      IndexSearcher searcher = null;
-      try {
-        searcher = acquire();
-        int realLimit = opts.start() + opts.limit();
-        TopFieldDocs docs = searcher.search(query, realLimit, sort);
-        List<T> result = new ArrayList<>(docs.scoreDocs.length);
-        for (int i = opts.start(); i < docs.scoreDocs.length; i++) {
-          ScoreDoc sd = docs.scoreDocs[i];
-          Document doc = searcher.doc(sd.doc, IndexUtils.projectFields(opts));
-          T mapperResult = mapper.apply(doc);
-          if (mapperResult != null) {
-            result.add(mapperResult);
-          }
-        }
-        final List<T> r = Collections.unmodifiableList(result);
-        return new ResultSet<T>() {
-          @Override
-          public Iterator<T> iterator() {
-            return r.iterator();
-          }
-
-          @Override
-          public List<T> toList() {
-            return r;
-          }
-
-          @Override
-          public void close() {
-            // Do nothing.
-          }
-        };
-      } catch (IOException e) {
-        throw new OrmException(e);
-      } finally {
-        if (searcher != null) {
-          try {
-            release(searcher);
-          } catch (IOException e) {
-            log.warn("cannot release Lucene searcher", e);
-          }
-        }
-      }
-    }
-  }
-
-  private ProjectData toProjectData(Document doc) {
+  @Override
+  protected ProjectData fromDocument(Document doc) {
    Project.NameKey nameKey = new Project.NameKey(doc.getField(NAME.getName()).stringValue());
    return projectCache.get().get(nameKey).toProjectData();
  }