Don't pass ReviewDb to ChangeData static methods

Assume that all of the ChangeDatas passed to these methods are from
the same context and therefore use the same database handle, so pick
one to use from the passed-in ChangeDatas.

Change-Id: Ic37cba1bd3fbfb987857b86fb67aaaa6c9aaba04
This commit is contained in:
Dave Borowitz
2013-12-23 10:31:26 -08:00
parent cfd54d6e20
commit d7ba1805cc
6 changed files with 21 additions and 31 deletions

View File

@@ -229,16 +229,16 @@ public class ChangeJson {
throws OrmException { throws OrmException {
accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS)); accountLoader = accountLoaderFactory.create(has(DETAILED_ACCOUNTS));
Iterable<ChangeData> all = Iterables.concat(in); Iterable<ChangeData> all = Iterables.concat(in);
ChangeData.ensureChangeLoaded(db, all); ChangeData.ensureChangeLoaded(all);
if (has(ALL_REVISIONS)) { if (has(ALL_REVISIONS)) {
ChangeData.ensureAllPatchSetsLoaded(db, all); ChangeData.ensureAllPatchSetsLoaded(all);
} else { } else {
ChangeData.ensureCurrentPatchSetLoaded(db, all); ChangeData.ensureCurrentPatchSetLoaded(all);
} }
if (has(REVIEWED)) { if (has(REVIEWED)) {
ensureReviewedLoaded(all); ensureReviewedLoaded(all);
} }
ChangeData.ensureCurrentApprovalsLoaded(db, all); ChangeData.ensureCurrentApprovalsLoaded(all);
List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size()); List<List<ChangeInfo>> res = Lists.newArrayListWithCapacity(in.size());
Map<Change.Id, ChangeInfo> out = Maps.newHashMap(); Map<Change.Id, ChangeInfo> out = Maps.newHashMap();

View File

@@ -19,7 +19,6 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status; import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.query.AndPredicate; import com.google.gerrit.server.query.AndPredicate;
import com.google.gerrit.server.query.NotPredicate; import com.google.gerrit.server.query.NotPredicate;
import com.google.gerrit.server.query.OrPredicate; import com.google.gerrit.server.query.OrPredicate;
@@ -33,7 +32,6 @@ import com.google.gerrit.server.query.change.ChangeQueryRewriter;
import com.google.gerrit.server.query.change.ChangeStatusPredicate; import com.google.gerrit.server.query.change.ChangeStatusPredicate;
import com.google.gerrit.server.query.change.OrSource; import com.google.gerrit.server.query.change.OrSource;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.BitSet; import java.util.BitSet;
import java.util.EnumSet; import java.util.EnumSet;
@@ -122,15 +120,12 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
} }
private final IndexCollection indexes; private final IndexCollection indexes;
private final Provider<ReviewDb> db;
private final BasicChangeRewrites basicRewrites; private final BasicChangeRewrites basicRewrites;
@Inject @Inject
IndexRewriteImpl(IndexCollection indexes, IndexRewriteImpl(IndexCollection indexes,
Provider<ReviewDb> db,
BasicChangeRewrites basicRewrites) { BasicChangeRewrites basicRewrites) {
this.indexes = indexes; this.indexes = indexes;
this.db = db;
this.basicRewrites = basicRewrites; this.basicRewrites = basicRewrites;
} }
@@ -250,7 +245,7 @@ public class IndexRewriteImpl implements ChangeQueryRewriter {
Predicate<ChangeData> in, Predicate<ChangeData> in,
List<Predicate<ChangeData>> all) { List<Predicate<ChangeData>> all) {
if (in instanceof AndPredicate) { if (in instanceof AndPredicate) {
return new AndSource(db, all); return new AndSource(all);
} else if (in instanceof OrPredicate) { } else if (in instanceof OrPredicate) {
return new OrSource(all); return new OrSource(all);
} }

View File

@@ -19,14 +19,12 @@ import com.google.common.base.Throwables;
import com.google.common.collect.FluentIterable; import com.google.common.collect.FluentIterable;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.query.AndPredicate; import com.google.gerrit.server.query.AndPredicate;
import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.Predicate;
import com.google.gwtorm.server.ListResultSet; import com.google.gwtorm.server.ListResultSet;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.OrmRuntimeException; import com.google.gwtorm.server.OrmRuntimeException;
import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.ResultSet;
import com.google.inject.Provider;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
@@ -73,13 +71,10 @@ public class AndSource extends AndPredicate<ChangeData>
return r; return r;
} }
private final Provider<ReviewDb> db;
private int cardinality = -1; private int cardinality = -1;
public AndSource(Provider<ReviewDb> db, public AndSource(Collection<? extends Predicate<ChangeData>> that) {
Collection<? extends Predicate<ChangeData>> that) {
super(sort(that)); super(sort(that));
this.db = db;
} }
@Override @Override
@@ -151,7 +146,7 @@ public class AndSource extends AndPredicate<ChangeData>
public List<ChangeData> apply(List<ChangeData> buffer) { public List<ChangeData> apply(List<ChangeData> buffer) {
if (loadChange) { if (loadChange) {
try { try {
ChangeData.ensureChangeLoaded(db, buffer); ChangeData.ensureChangeLoaded(buffer);
} catch (OrmException e) { } catch (OrmException e) {
throw new OrmRuntimeException(e); throw new OrmRuntimeException(e);
} }

View File

@@ -45,7 +45,6 @@ import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.ResultSet;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;
@@ -67,8 +66,8 @@ import java.util.Map;
import java.util.Set; import java.util.Set;
public class ChangeData { public class ChangeData {
public static void ensureChangeLoaded(Provider<ReviewDb> db, public static void ensureChangeLoaded(Iterable<ChangeData> changes)
Iterable<ChangeData> changes) throws OrmException { throws OrmException {
Map<Change.Id, ChangeData> missing = Maps.newHashMap(); Map<Change.Id, ChangeData> missing = Maps.newHashMap();
for (ChangeData cd : changes) { for (ChangeData cd : changes) {
if (cd.change == null) { if (cd.change == null) {
@@ -76,21 +75,22 @@ public class ChangeData {
} }
} }
if (!missing.isEmpty()) { if (!missing.isEmpty()) {
for (Change change : db.get().changes().get(missing.keySet())) { ReviewDb db = missing.values().iterator().next().db;
for (Change change : db.changes().get(missing.keySet())) {
missing.get(change.getId()).change = change; missing.get(change.getId()).change = change;
} }
} }
} }
public static void ensureAllPatchSetsLoaded(Provider<ReviewDb> db, public static void ensureAllPatchSetsLoaded(Iterable<ChangeData> changes)
Iterable<ChangeData> changes) throws OrmException { throws OrmException {
for (ChangeData cd : changes) { for (ChangeData cd : changes) {
cd.patches(); cd.patches();
} }
} }
public static void ensureCurrentPatchSetLoaded(Provider<ReviewDb> db, public static void ensureCurrentPatchSetLoaded(Iterable<ChangeData> changes)
Iterable<ChangeData> changes) throws OrmException { throws OrmException {
Map<PatchSet.Id, ChangeData> missing = Maps.newHashMap(); Map<PatchSet.Id, ChangeData> missing = Maps.newHashMap();
for (ChangeData cd : changes) { for (ChangeData cd : changes) {
if (cd.currentPatchSet == null && cd.patches == null) { if (cd.currentPatchSet == null && cd.patches == null) {
@@ -98,7 +98,8 @@ public class ChangeData {
} }
} }
if (!missing.isEmpty()) { if (!missing.isEmpty()) {
for (PatchSet ps : db.get().patchSets().get(missing.keySet())) { ReviewDb db = missing.values().iterator().next().db;
for (PatchSet ps : db.patchSets().get(missing.keySet())) {
ChangeData cd = missing.get(ps.getId()); ChangeData cd = missing.get(ps.getId());
cd.currentPatchSet = ps; cd.currentPatchSet = ps;
if (cd.limitedIds == null) { if (cd.limitedIds == null) {
@@ -108,12 +109,12 @@ public class ChangeData {
} }
} }
public static void ensureCurrentApprovalsLoaded(Provider<ReviewDb> db, public static void ensureCurrentApprovalsLoaded(Iterable<ChangeData> changes)
Iterable<ChangeData> changes) throws OrmException { throws OrmException {
List<ResultSet<PatchSetApproval>> pending = Lists.newArrayList(); List<ResultSet<PatchSetApproval>> pending = Lists.newArrayList();
for (ChangeData cd : changes) { for (ChangeData cd : changes) {
if (cd.currentApprovals == null && cd.limitedApprovals == null) { if (cd.currentApprovals == null && cd.limitedApprovals == null) {
pending.add(db.get().patchSetApprovals() pending.add(cd.db.patchSetApprovals()
.byPatchSet(cd.change().currentPatchSetId())); .byPatchSet(cd.change().currentPatchSetId()));
} }
} }

View File

@@ -253,7 +253,7 @@ public class QueryProcessor {
} }
// Don't trust QueryRewriter to have left the visible predicate. // Don't trust QueryRewriter to have left the visible predicate.
AndSource a = new AndSource(db, ImmutableList.of(s, visibleToMe)); AndSource a = new AndSource(ImmutableList.of(s, visibleToMe));
limits.add(limit(q)); limits.add(limit(q));
sources.add(a); sources.add(a);
} }

View File

@@ -54,7 +54,6 @@ public class IndexRewriteTest {
queryBuilder = new FakeQueryBuilder(indexes); queryBuilder = new FakeQueryBuilder(indexes);
rewrite = new IndexRewriteImpl( rewrite = new IndexRewriteImpl(
indexes, indexes,
null,
new BasicChangeRewrites(null, indexes)); new BasicChangeRewrites(null, indexes));
} }