Remove ChangeQueryBuilder dependency from InternalChangeQuery
ChangeQueryBuilder is a kitchen sink with all sorts of logic that it's probably not a great idea to depend on from code deep in Gerrit internals, which is what InternalChangeQuery is for. It just needs access to a couple of very simple predicates. Simple enough, in fact, that their constructors should just be called manually, which is fine since they're still in the same package. A similar story applies to QueryProcessor; although IsVisibleToPredicate is slightly more complicated than ProjectPredicate, it still doesn't require the whole suite of ChangeQueryBuilder.Arguments. Change-Id: I17180749b66347cc576d9cbf4e07d37c03728625
This commit is contained in:
		@@ -15,6 +15,7 @@
 | 
			
		||||
package com.google.gerrit.server.query.change;
 | 
			
		||||
 | 
			
		||||
import static com.google.gerrit.server.query.Predicate.and;
 | 
			
		||||
import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.reviewdb.client.Project;
 | 
			
		||||
import com.google.gerrit.server.query.Predicate;
 | 
			
		||||
@@ -26,14 +27,15 @@ import java.util.List;
 | 
			
		||||
 | 
			
		||||
/** Execute a single query over changes, for use by Gerrit internals. */
 | 
			
		||||
public class InternalChangeQuery {
 | 
			
		||||
  private static Predicate<ChangeData> project(Project.NameKey projectName) {
 | 
			
		||||
    return new ProjectPredicate(projectName.get());
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private final QueryProcessor qp;
 | 
			
		||||
  private final ChangeQueryBuilder qb;
 | 
			
		||||
 | 
			
		||||
  @Inject
 | 
			
		||||
  InternalChangeQuery(QueryProcessor queryProcessor,
 | 
			
		||||
      ChangeQueryBuilder queryBuilder) {
 | 
			
		||||
  InternalChangeQuery(QueryProcessor queryProcessor) {
 | 
			
		||||
    qp = queryProcessor;
 | 
			
		||||
    qb = queryBuilder;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public InternalChangeQuery setLimit(int n) {
 | 
			
		||||
@@ -41,9 +43,9 @@ public class InternalChangeQuery {
 | 
			
		||||
    return this;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public List<ChangeData> byProjectOpen(Project.NameKey project)
 | 
			
		||||
  public List<ChangeData> byProjectOpen(Project.NameKey projectName)
 | 
			
		||||
      throws OrmException {
 | 
			
		||||
    return query(and(qb.project(project.get()), qb.status_open()));
 | 
			
		||||
    return query(and(project(projectName), open()));
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
 | 
			
		||||
 
 | 
			
		||||
@@ -15,12 +15,15 @@
 | 
			
		||||
package com.google.gerrit.server.query.change;
 | 
			
		||||
 | 
			
		||||
import static com.google.common.base.Preconditions.checkState;
 | 
			
		||||
import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open;
 | 
			
		||||
 | 
			
		||||
import com.google.common.collect.ImmutableList;
 | 
			
		||||
import com.google.common.collect.Ordering;
 | 
			
		||||
import com.google.gerrit.common.data.GlobalCapability;
 | 
			
		||||
import com.google.gerrit.reviewdb.server.ReviewDb;
 | 
			
		||||
import com.google.gerrit.server.CurrentUser;
 | 
			
		||||
import com.google.gerrit.server.index.IndexPredicate;
 | 
			
		||||
import com.google.gerrit.server.project.ChangeControl;
 | 
			
		||||
import com.google.gerrit.server.query.Predicate;
 | 
			
		||||
import com.google.gerrit.server.query.QueryParseException;
 | 
			
		||||
import com.google.gwtorm.server.OrmException;
 | 
			
		||||
@@ -32,21 +35,24 @@ import java.util.ArrayList;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
 | 
			
		||||
public class QueryProcessor {
 | 
			
		||||
  private final ChangeQueryBuilder queryBuilder;
 | 
			
		||||
  private final ChangeQueryRewriter queryRewriter;
 | 
			
		||||
  private final Provider<ReviewDb> db;
 | 
			
		||||
  private final Provider<CurrentUser> userProvider;
 | 
			
		||||
  private final ChangeControl.GenericFactory changeControlFactory;
 | 
			
		||||
  private final ChangeQueryRewriter queryRewriter;
 | 
			
		||||
 | 
			
		||||
  private int limitFromCaller;
 | 
			
		||||
  private int start;
 | 
			
		||||
  private boolean enforceVisibility = true;
 | 
			
		||||
 | 
			
		||||
  @Inject
 | 
			
		||||
  QueryProcessor(ChangeQueryBuilder queryBuilder,
 | 
			
		||||
  QueryProcessor(Provider<ReviewDb> db,
 | 
			
		||||
      Provider<CurrentUser> userProvider,
 | 
			
		||||
      ChangeControl.GenericFactory changeControlFactory,
 | 
			
		||||
      ChangeQueryRewriter queryRewriter) {
 | 
			
		||||
    this.queryBuilder = queryBuilder;
 | 
			
		||||
    this.queryRewriter = queryRewriter;
 | 
			
		||||
    this.db = db;
 | 
			
		||||
    this.userProvider = userProvider;
 | 
			
		||||
    this.changeControlFactory = changeControlFactory;
 | 
			
		||||
    this.queryRewriter = queryRewriter;
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  public QueryProcessor enforceVisibility(boolean enforce) {
 | 
			
		||||
@@ -104,7 +110,7 @@ public class QueryProcessor {
 | 
			
		||||
      List<Predicate<ChangeData>> queries)
 | 
			
		||||
      throws OrmException, QueryParseException {
 | 
			
		||||
    Predicate<ChangeData> visibleToMe = enforceVisibility
 | 
			
		||||
        ? queryBuilder.is_visible()
 | 
			
		||||
        ? new IsVisibleToPredicate(db, changeControlFactory, userProvider.get())
 | 
			
		||||
        : null;
 | 
			
		||||
    int cnt = queries.size();
 | 
			
		||||
 | 
			
		||||
@@ -121,7 +127,7 @@ public class QueryProcessor {
 | 
			
		||||
      // ask for one more result from the query.
 | 
			
		||||
      Predicate<ChangeData> s = queryRewriter.rewrite(q, start, limit + 1);
 | 
			
		||||
      if (!(s instanceof ChangeDataSource)) {
 | 
			
		||||
        q = Predicate.and(queryBuilder.status_open(), q);
 | 
			
		||||
        q = Predicate.and(open(), q);
 | 
			
		||||
        s = queryRewriter.rewrite(q, start, limit);
 | 
			
		||||
      }
 | 
			
		||||
      if (!(s instanceof ChangeDataSource)) {
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user