Replace most ChangeAccess queries with searches
Use the secondary index rather than depending on SQL indexes, which
will not be available in a notedb world. For the most part this change
is pretty straightforward, as the interface for InternalChangeQuery is
almost the same as for ChangeAccess. It also provides a bit more
functionality, such as limiting the results.
By default, all of these queries should bypass visibility checks,
since things like the submit queue necessarily need to see everything.
Even if it is properly running as InternalUser and that is hard-coded
to have visibility, it is simpler to just bypass the checks entirely.
The biggest complication comes from converting callers that previously
opened a new ReviewDb in a try/finally block and called a ChangeAccess
method directly from that. In the InternalChangeQuery model,
Provider<ReviewDb> needs to be injected, so we need to set the
ThreadLocalRequestContext with the manually-opened DB. To simplify
this code, add an AutoCloseable RequestContext implementation.
To save future schema churn, this change does not include a schema
change to drop the indexes.
Change-Id: I99de8a2cf2aba01971059b89df33b1676cd8546e
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index c1f8de3..5b39f48 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -40,6 +40,7 @@
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.ApprovalsUtil;
+import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.PutConfig;
import com.google.gson.reflect.TypeToken;
@@ -73,6 +74,8 @@
@Inject
private ApprovalsUtil approvalsUtil;
+ @Inject
+ private ChangeIndexer indexer;
@Before
public void setUp() throws Exception {
@@ -166,6 +169,7 @@
PatchSetApproval.LabelId.SUBMIT),
(short) 1,
new Timestamp(System.currentTimeMillis()))));
+ indexer.index(db, c);
}
private void submit(String changeId, int expectedStatus) throws IOException {
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ChangeAccess.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ChangeAccess.java
index ec46638..c497819 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ChangeAccess.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/server/ChangeAccess.java
@@ -14,9 +14,7 @@
package com.google.gerrit.reviewdb.server;
-import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gwtorm.server.Access;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.PrimaryKey;
@@ -35,26 +33,6 @@
ResultSet<Change> byKeyRange(Change.Key reva, Change.Key revb)
throws OrmException;
- @Query("WHERE dest = ? AND changeKey = ?")
- ResultSet<Change> byBranchKey(Branch.NameKey p, Change.Key key)
- throws OrmException;
-
- @Query("WHERE dest.projectName = ?")
- ResultSet<Change> byProject(Project.NameKey p) throws OrmException;
-
- @Query("WHERE dest = ? AND status = '" + Change.STATUS_SUBMITTED
- + "' ORDER BY lastUpdatedOn")
- ResultSet<Change> submitted(Branch.NameKey dest) throws OrmException;
-
- @Query("WHERE status = '" + Change.STATUS_SUBMITTED + "'")
- ResultSet<Change> allSubmitted() throws OrmException;
-
- @Query("WHERE open = true AND dest.projectName = ?")
- ResultSet<Change> byProjectOpenAll(Project.NameKey p) throws OrmException;
-
- @Query("WHERE open = true AND dest = ?")
- ResultSet<Change> byBranchOpenAll(Branch.NameKey p) throws OrmException;
-
@Query
ResultSet<Change> all() throws OrmException;
}
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
index 59e6934..b9216a7 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_generic.sql
@@ -69,18 +69,6 @@
-- *********************************************************************
-- ChangeAccess
--- covers: submitted, allSubmitted
-CREATE INDEX changes_submitted
-ON changes (status, dest_project_name, dest_branch_name, last_updated_on);
-
--- covers: byProjectOpenAll
-CREATE INDEX changes_byProjectOpen
-ON changes (open, dest_project_name, last_updated_on);
-
--- covers: byProject
-CREATE INDEX changes_byProject
-ON changes (dest_project_name);
-
CREATE INDEX changes_key
ON changes (change_key);
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
index de86ff6..48836ac 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_maxdb.sql
@@ -76,21 +76,6 @@
-- *********************************************************************
-- ChangeAccess
--- covers: submitted, allSubmitted
-CREATE INDEX changes_submitted
-ON changes (status, dest_project_name, dest_branch_name, last_updated_on)
-#
-
--- covers: byProjectOpenPrev, byProjectOpenNext
-CREATE INDEX changes_byProjectOpen
-ON changes (open, dest_project_name, last_updated_on);
-#
-
--- covers: byProject
-CREATE INDEX changes_byProject
-ON changes (dest_project_name)
-#
-
CREATE INDEX changes_key
ON changes (change_key)
#
diff --git a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
index ef50f24..95fc4bd 100644
--- a/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
+++ b/gerrit-reviewdb/src/main/resources/com/google/gerrit/reviewdb/server/index_postgres.sql
@@ -117,20 +117,6 @@
-- *********************************************************************
-- ChangeAccess
--- covers: submitted, allSubmitted
-CREATE INDEX changes_submitted
-ON changes (dest_project_name, dest_branch_name, last_updated_on)
-WHERE status = 's';
-
--- covers: byProjectOpenAll
-CREATE INDEX changes_byProjectOpen
-ON changes (dest_project_name, last_updated_on)
-WHERE open = 'Y';
-
--- covers: byProject
-CREATE INDEX changes_byProject
-ON changes (dest_project_name);
-
CREATE INDEX changes_key
ON changes (change_key);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
index b3c4709..f2fefbe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server;
import static com.google.gerrit.server.change.PatchSetInserter.ValidatePolicy.RECEIVE_COMMITS;
+import static com.google.gerrit.server.query.change.ChangeData.asChanges;
import com.google.common.base.Optional;
import com.google.common.base.Strings;
@@ -42,6 +43,7 @@
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.ssh.SshInfo;
import com.google.gerrit.server.util.IdGenerator;
import com.google.gerrit.server.util.MagicBranch;
@@ -182,6 +184,7 @@
private final Provider<CurrentUser> userProvider;
private final CommitValidators.Factory commitValidatorsFactory;
private final Provider<ReviewDb> db;
+ private final Provider<InternalChangeQuery> queryProvider;
private final RevertedSender.Factory revertedSenderFactory;
private final ChangeInserter.Factory changeInserterFactory;
private final PatchSetInserter.Factory patchSetInserterFactory;
@@ -193,6 +196,7 @@
ChangeUtil(Provider<CurrentUser> userProvider,
CommitValidators.Factory commitValidatorsFactory,
Provider<ReviewDb> db,
+ Provider<InternalChangeQuery> queryProvider,
RevertedSender.Factory revertedSenderFactory,
ChangeInserter.Factory changeInserterFactory,
PatchSetInserter.Factory patchSetInserterFactory,
@@ -202,6 +206,7 @@
this.userProvider = userProvider;
this.commitValidatorsFactory = commitValidatorsFactory;
this.db = db;
+ this.queryProvider = queryProvider;
this.revertedSenderFactory = revertedSenderFactory;
this.changeInserterFactory = changeInserterFactory;
this.patchSetInserterFactory = patchSetInserterFactory;
@@ -532,9 +537,9 @@
// Try change triplet
Optional<ChangeTriplet> triplet = ChangeTriplet.parse(id);
if (triplet.isPresent()) {
- return db.get().changes().byBranchKey(
+ return asChanges(queryProvider.get().byBranchKey(
triplet.get().branch(),
- triplet.get().id()).toList();
+ triplet.get().id()));
}
throw new ResourceNotFoundException(id);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ChangeIdHandler.java b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
index 9c3d052..1cbab8a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/args4j/ChangeIdHandler.java
@@ -17,9 +17,11 @@
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.kohsuke.args4j.CmdLineException;
@@ -30,17 +32,16 @@
import org.kohsuke.args4j.spi.Setter;
public class ChangeIdHandler extends OptionHandler<Change.Id> {
-
- @Inject
- private ReviewDb db;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
public ChangeIdHandler(
- final ReviewDb db,
+ // TODO(dborowitz): Not sure whether this is injectable here.
+ Provider<InternalChangeQuery> queryProvider,
@Assisted final CmdLineParser parser, @Assisted final OptionDef option,
@Assisted final Setter<Change.Id> setter) {
super(parser, option, setter);
- this.db = db;
+ this.queryProvider = queryProvider;
}
@Override
@@ -58,8 +59,8 @@
final Project.NameKey project = new Project.NameKey(tokens[0]);
final Branch.NameKey branch =
new Branch.NameKey(project, "refs/heads/" + tokens[1]);
- for (final Change change : db.changes().byBranchKey(branch, key)) {
- setter.addValue(change.getId());
+ for (final ChangeData cd : queryProvider.get().byBranchKey(branch, key)) {
+ setter.addValue(cd.getId());
return 1;
}
} catch (IllegalArgumentException e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
index 9cb1128..374cde3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CherryPickChange.java
@@ -39,6 +39,8 @@
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.RefControl;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.ssh.NoSshInfo;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -67,6 +69,7 @@
public class CherryPickChange {
private final Provider<ReviewDb> db;
+ private final Provider<InternalChangeQuery> queryProvider;
private final GitRepositoryManager gitManager;
private final TimeZone serverTimeZone;
private final Provider<CurrentUser> currentUser;
@@ -76,15 +79,17 @@
final MergeUtil.Factory mergeUtilFactory;
@Inject
- CherryPickChange(final Provider<ReviewDb> db,
- @GerritPersonIdent final PersonIdent myIdent,
- final GitRepositoryManager gitManager,
- final Provider<CurrentUser> currentUser,
- final CommitValidators.Factory commitValidatorsFactory,
- final ChangeInserter.Factory changeInserterFactory,
- final PatchSetInserter.Factory patchSetInserterFactory,
- final MergeUtil.Factory mergeUtilFactory) {
+ CherryPickChange(Provider<ReviewDb> db,
+ Provider<InternalChangeQuery> queryProvider,
+ @GerritPersonIdent PersonIdent myIdent,
+ GitRepositoryManager gitManager,
+ Provider<CurrentUser> currentUser,
+ CommitValidators.Factory commitValidatorsFactory,
+ ChangeInserter.Factory changeInserterFactory,
+ PatchSetInserter.Factory patchSetInserterFactory,
+ MergeUtil.Factory mergeUtilFactory) {
this.db = db;
+ this.queryProvider = queryProvider;
this.gitManager = gitManager;
this.serverTimeZone = myIdent.getTimeZone();
this.currentUser = currentUser;
@@ -163,11 +168,11 @@
changeKey = new Change.Key("I" + computedChangeId.name());
}
- List<Change> destChanges =
- db.get().changes()
- .byBranchKey(new Branch.NameKey(project, destRef.getName()),
- changeKey).toList();
-
+ Branch.NameKey newDest =
+ new Branch.NameKey(change.getProject(), destRef.getName());
+ List<ChangeData> destChanges = queryProvider.get()
+ .setLimit(2)
+ .byBranchKey(newDest, changeKey);
if (destChanges.size() > 1) {
throw new InvalidChangeOperationException("Several changes with key "
+ changeKey + " reside on the same branch. "
@@ -175,7 +180,7 @@
} else if (destChanges.size() == 1) {
// The change key exists on the destination branch. The cherry pick
// will be added as a new patch set.
- return insertPatchSet(git, revWalk, destChanges.get(0),
+ return insertPatchSet(git, revWalk, destChanges.get(0).change(),
cherryPickCommit, refControl, identifiedUser);
} else {
// Change key not found on destination branch. We can create a new
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
index 6056e5f..e69e268 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/GetRelated.java
@@ -32,6 +32,8 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
@@ -64,11 +66,15 @@
private final GitRepositoryManager gitMgr;
private final Provider<ReviewDb> dbProvider;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
- GetRelated(GitRepositoryManager gitMgr, Provider<ReviewDb> db) {
+ GetRelated(GitRepositoryManager gitMgr,
+ Provider<ReviewDb> db,
+ Provider<InternalChangeQuery> queryProvider) {
this.gitMgr = gitMgr;
this.dbProvider = db;
+ this.queryProvider = queryProvider;
}
@Override
@@ -92,8 +98,8 @@
private List<ChangeAndCommit> walk(RevisionResource rsrc, RevWalk rw, Ref ref)
throws OrmException, IOException {
- Map<Change.Id, Change> changes = allOpenChanges(rsrc);
- Map<PatchSet.Id, PatchSet> patchSets = allPatchSets(rsrc, changes.keySet());
+ Map<Change.Id, ChangeData> changes = allOpenChanges(rsrc);
+ Map<PatchSet.Id, PatchSet> patchSets = allPatchSets(rsrc, changes.values());
Map<String, PatchSet> commits = Maps.newHashMap();
for (PatchSet p : patchSets.values()) {
@@ -119,7 +125,7 @@
PatchSet p = commits.get(c.name());
Change g = null;
if (p != null) {
- g = changes.get(p.getId().getParentKey());
+ g = changes.get(p.getId().getParentKey()).change();
added.add(p.getId().getParentKey());
} else {
// check if there is a merged or abandoned change for this commit
@@ -148,25 +154,18 @@
return list;
}
- private Map<Change.Id, Change> allOpenChanges(RevisionResource rsrc)
+ private Map<Change.Id, ChangeData> allOpenChanges(RevisionResource rsrc)
throws OrmException {
- ReviewDb db = dbProvider.get();
- return db.changes().toMap(
- db.changes().byBranchOpenAll(rsrc.getChange().getDest()));
+ return ChangeData.asMap(
+ queryProvider.get().byBranchOpen(rsrc.getChange().getDest()));
}
private Map<PatchSet.Id, PatchSet> allPatchSets(RevisionResource rsrc,
- Collection<Change.Id> ids) throws OrmException {
- int n = ids.size();
- ReviewDb db = dbProvider.get();
- List<ResultSet<PatchSet>> t = Lists.newArrayListWithCapacity(n);
- for (Change.Id id : ids) {
- t.add(db.patchSets().byChange(id));
- }
-
- Map<PatchSet.Id, PatchSet> r = Maps.newHashMapWithExpectedSize(n * 2);
- for (ResultSet<PatchSet> rs : t) {
- for (PatchSet p : rs) {
+ Collection<ChangeData> cds) throws OrmException {
+ Map<PatchSet.Id, PatchSet> r =
+ Maps.newHashMapWithExpectedSize(cds.size() * 2);
+ for (ChangeData cd : cds) {
+ for (PatchSet p : cd.patches()) {
r.put(p.getId(), p);
}
}
@@ -178,7 +177,7 @@
}
private List<ChangeAndCommit> children(RevisionResource rsrc, RevWalk rw,
- Map<Change.Id, Change> changes, Map<PatchSet.Id, PatchSet> patchSets,
+ Map<Change.Id, ChangeData> changes, Map<PatchSet.Id, PatchSet> patchSets,
Set<Change.Id> added)
throws OrmException, IOException {
// children is a map of parent commit name to PatchSet built on it.
@@ -205,9 +204,9 @@
}
for (Map.Entry<Change.Id, PatchSet.Id> e : matches.entrySet()) {
- Change change = changes.get(e.getKey());
+ ChangeData cd = changes.get(e.getKey());
PatchSet ps = patchSets.get(e.getValue());
- if (change == null || ps == null || !seenChange.add(e.getKey())) {
+ if (cd == null || ps == null || !seenChange.add(e.getKey())) {
continue;
}
@@ -218,7 +217,7 @@
q.addFirst(ps.getRevision().get());
if (added.add(ps.getId().getParentKey())) {
rw.parseBody(c);
- graph.add(new ChangeAndCommit(change, ps, c));
+ graph.add(new ChangeAndCommit(cd.change(), ps, c));
}
}
}
@@ -228,13 +227,15 @@
}
private boolean isVisible(ProjectControl projectCtl,
- Map<Change.Id, Change> changes,
+ Map<Change.Id, ChangeData> changes,
Map<PatchSet.Id, PatchSet> patchSets,
PatchSet.Id psId) throws OrmException {
- Change c = changes.get(psId.getParentKey());
+ ChangeData cd = changes.get(psId.getParentKey());
PatchSet ps = patchSets.get(psId);
- if (c != null && ps != null) {
- ChangeControl ctl = projectCtl.controlFor(c);
+ if (cd != null && ps != null) {
+ // Related changes are in the same project, so reuse the existing
+ // ProjectControl.
+ ChangeControl ctl = projectCtl.controlFor(cd.change());
return ctl.isVisible(dbProvider.get())
&& ctl.isPatchVisible(ps, dbProvider.get());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCache.java
index eb4acfc..d1c968f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCache.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeCache.java
@@ -20,11 +20,13 @@
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.cache.CacheModule;
-import com.google.gwtorm.server.SchemaFactory;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Module;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
@@ -79,20 +81,21 @@
}
static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
- private final SchemaFactory<ReviewDb> schema;
+ private final OneOffRequestContext requestContext;
+ private final Provider<InternalChangeQuery> queryProvider;
@Inject
- Loader(SchemaFactory<ReviewDb> schema) {
- this.schema = schema;
+ Loader(OneOffRequestContext requestContext,
+ Provider<InternalChangeQuery> queryProvider) {
+ this.requestContext = requestContext;
+ this.queryProvider = queryProvider;
}
@Override
public List<Change> load(Project.NameKey key) throws Exception {
- final ReviewDb db = schema.open();
- try {
- return Collections.unmodifiableList(db.changes().byProject(key).toList());
- } finally {
- db.close();
+ try (AutoCloseable ctx = requestContext.open()) {
+ return Collections.unmodifiableList(
+ ChangeData.asChanges(queryProvider.get().byProject(key)));
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
index 6c2092b..ba38f8f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
@@ -65,11 +65,13 @@
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.util.RequestScopePropagator;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -148,6 +150,7 @@
private final MergeValidators.Factory mergeValidatorsFactory;
private final PatchSetInfoFactory patchSetInfoFactory;
private final ProjectCache projectCache;
+ private final Provider<InternalChangeQuery> queryProvider;
private final RequestScopePropagator requestScopePropagator;
private final SchemaFactory<ReviewDb> schemaFactory;
private final SubmitStrategyFactory submitStrategyFactory;
@@ -191,6 +194,7 @@
MergeValidators.Factory mergeValidatorsFactory,
PatchSetInfoFactory patchSetInfoFactory,
ProjectCache projectCache,
+ Provider<InternalChangeQuery> queryProvider,
RequestScopePropagator requestScopePropagator,
SchemaFactory<ReviewDb> schemaFactory,
SubmitStrategyFactory submitStrategyFactory,
@@ -216,6 +220,7 @@
this.mergeValidatorsFactory = mergeValidatorsFactory;
this.patchSetInfoFactory = patchSetInfoFactory;
this.projectCache = projectCache;
+ this.queryProvider = queryProvider;
this.requestScopePropagator = requestScopePropagator;
this.schemaFactory = schemaFactory;
this.submitStrategyFactory = submitStrategyFactory;
@@ -256,7 +261,7 @@
boolean reopen = false;
ListMultimap<SubmitType, Change> toSubmit =
- validateChangeList(db.changes().submitted(destBranch).toList());
+ validateChangeList(queryProvider.get().submitted(destBranch));
ListMultimap<SubmitType, CodeReviewCommit> toMergeNextTurn =
ArrayListMultimap.create();
List<CodeReviewCommit> potentiallyStillSubmittableOnNextRun =
@@ -444,9 +449,14 @@
branchTip = null;
branchUpdate.setExpectedOldObjectId(ObjectId.zeroId());
} else {
- for (Change c : db.changes().submitted(destBranch).toList()) {
- setNew(c, message(c, "Your change could not be merged, "
- + "because the destination branch does not exist anymore."));
+ for (ChangeData cd : queryProvider.get().submitted(destBranch)) {
+ try {
+ Change c = cd.change();
+ setNew(c, message(c, "Your change could not be merged, "
+ + "because the destination branch does not exist anymore."));
+ } catch (OrmException e) {
+ log.error("Error setting change new", e);
+ }
}
}
return branchUpdate;
@@ -481,7 +491,7 @@
}
private ListMultimap<SubmitType, Change> validateChangeList(
- List<Change> submitted) throws MergeException {
+ List<ChangeData> submitted) throws MergeException {
ListMultimap<SubmitType, Change> toSubmit = ArrayListMultimap.create();
Map<String, Ref> allRefs;
@@ -496,15 +506,16 @@
tips.add(r.getObjectId());
}
- for (Change chg : submitted) {
+ for (ChangeData cd : submitted) {
ChangeControl ctl;
+ Change chg;
try {
- ctl = changeControlFactory.controlFor(chg,
- identifiedUserFactory.create(chg.getOwner()));
- } catch (NoSuchChangeException e) {
+ ctl = cd.changeControl();
+ chg = cd.change();
+ } catch (OrmException e) {
throw new MergeException("Failed to validate changes", e);
}
- Change.Id changeId = chg.getId();
+ Change.Id changeId = cd.getId();
if (chg.currentPatchSetId() == null) {
logError("Missing current patch set on change " + changeId);
commits.put(changeId, CodeReviewCommit.noPatchSet(ctl));
@@ -514,7 +525,7 @@
PatchSet ps;
try {
- ps = db.patchSets().get(chg.currentPatchSetId());
+ ps = cd.currentPatchSet();
} catch (OrmException e) {
throw new MergeException("Cannot query the database", e);
}
@@ -565,6 +576,7 @@
continue;
}
+ // TODO(dborowitz): Consider putting ChangeData in CodeReviewCommit.
commit.setControl(ctl);
commit.setPatchsetId(ps.getId());
commits.put(changeId, commit);
@@ -733,7 +745,7 @@
private void updateChangeStatus(List<Change> submitted)
throws NoSuchChangeException {
- logDebug("Updating change status for {} changes", submitted);
+ logDebug("Updating change status for {} changes", submitted.size());
for (Change c : submitted) {
CodeReviewCommit commit = commits.get(c.getId());
CommitMergeStatus s = commit != null ? commit.getStatusCode() : null;
@@ -747,7 +759,8 @@
}
String txt = s.getMessage();
- logDebug("Status of change {} on {}: {}", c.getId(), c.getDest(), s);
+ logDebug("Status of change {} ({}) on {}: {}", c.getId(), commit.name(),
+ c.getDest(), s);
try {
switch (s) {
@@ -1207,9 +1220,9 @@
Exception err = null;
try {
openSchema();
- for (Change c
- : db.changes().byProjectOpenAll(destBranch.getParentKey())) {
- abandonOneChange(c);
+ for (ChangeData cd
+ : queryProvider.get().byProjectOpen(destBranch.getParentKey())) {
+ abandonOneChange(cd.change());
}
db.close();
db = null;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index 0606dcc..02f8643 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -272,6 +272,7 @@
private final IdentifiedUser currentUser;
private final ReviewDb db;
+ private final Provider<InternalChangeQuery> queryProvider;
private final ChangeData.Factory changeDataFactory;
private final ChangeUpdate.Factory updateFactory;
private final SchemaFactory<ReviewDb> schemaFactory;
@@ -377,6 +378,7 @@
final NotesMigration notesMigration) throws IOException {
this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
this.db = db;
+ this.queryProvider = queryProvider;
this.changeDataFactory = changeDataFactory;
this.updateFactory = updateFactory;
this.schemaFactory = schemaFactory;
@@ -1523,7 +1525,7 @@
return;
}
- List<Change> changes = p.destChanges.toList();
+ List<ChangeData> changes = p.destChanges;
if (changes.size() > 1) {
// WTF, multiple changes in this project have the same key?
// Since the commit is new, the user should recreate it with
@@ -1538,7 +1540,8 @@
if (changes.size() == 1) {
// Schedule as a replacement to this one matching change.
//
- if (requestReplace(magicBranch.cmd, false, changes.get(0), p.commit)) {
+ if (requestReplace(
+ magicBranch.cmd, false, changes.get(0).change(), p.commit)) {
continue;
} else {
newChanges = Collections.emptyList();
@@ -1602,12 +1605,12 @@
private class ChangeLookup {
final RevCommit commit;
final Change.Key changeKey;
- final ResultSet<Change> destChanges;
+ final List<ChangeData> destChanges;
ChangeLookup(RevCommit c, Change.Key key) throws OrmException {
commit = c;
changeKey = key;
- destChanges = db.changes().byBranchKey(magicBranch.dest, key);
+ destChanges = queryProvider.get().byBranchKey(magicBranch.dest, key);
}
}
@@ -2353,7 +2356,7 @@
closeProgress.update(1);
if (closedChange != null) {
if (byKey == null) {
- byKey = openChangesByKey(branch);
+ byKey = openChangesByBranch(branch);
}
byKey.remove(closedChange);
}
@@ -2361,7 +2364,7 @@
for (final String changeId : c.getFooterLines(CHANGE_ID)) {
if (byKey == null) {
- byKey = openChangesByKey(branch);
+ byKey = openChangesByBranch(branch);
}
final Change onto = byKey.get(new Change.Key(changeId.trim()));
@@ -2436,11 +2439,11 @@
return change.getKey();
}
- private Map<Change.Key, Change> openChangesByKey(Branch.NameKey branch)
+ private Map<Change.Key, Change> openChangesByBranch(Branch.NameKey branch)
throws OrmException {
final Map<Change.Key, Change> r = new HashMap<>();
- for (Change c : db.changes().byBranchOpenAll(branch)) {
- r.put(c.getKey(), c);
+ for (ChangeData cd : queryProvider.get().byBranchOpen(branch)) {
+ r.put(cd.change().getKey(), cd.change());
}
return r;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
index 5580f20..7095552 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
@@ -101,8 +101,10 @@
final int limit = 32;
try {
Set<PatchSet.Id> toGet = Sets.newHashSetWithExpectedSize(limit);
- for (ChangeData cd :
- queryProvider.get().setLimit(limit).byProjectOpen(projectName)) {
+ for (ChangeData cd : queryProvider.get()
+ .enforceVisibility(true)
+ .setLimit(limit)
+ .byProjectOpen(projectName)) {
PatchSet.Id id = cd.change().currentPatchSetId();
if (id != null) {
toGet.add(id);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReloadSubmitQueueOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReloadSubmitQueueOp.java
index 266ce1a..734512f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReloadSubmitQueueOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReloadSubmitQueueOp.java
@@ -15,11 +15,12 @@
package com.google.gerrit.server.git;
import com.google.gerrit.reviewdb.client.Branch;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import com.google.inject.Singleton;
import org.slf4j.Logger;
@@ -32,35 +33,39 @@
private static final Logger log =
LoggerFactory.getLogger(ReloadSubmitQueueOp.class);
- private final SchemaFactory<ReviewDb> schema;
+ private final OneOffRequestContext requestContext;
+ private final Provider<InternalChangeQuery> queryProvider;
private final MergeQueue mergeQueue;
@Inject
- ReloadSubmitQueueOp(final WorkQueue wq, final SchemaFactory<ReviewDb> sf,
- final MergeQueue mq) {
+ ReloadSubmitQueueOp(
+ OneOffRequestContext rc,
+ WorkQueue wq,
+ Provider<InternalChangeQuery> qp,
+ MergeQueue mq) {
super(wq);
- schema = sf;
+ requestContext = rc;
+ queryProvider = qp;
mergeQueue = mq;
}
@Override
public void run() {
- final HashSet<Branch.NameKey> pending = new HashSet<>();
- try {
- final ReviewDb c = schema.open();
- try {
- for (final Change change : c.changes().allSubmitted()) {
- pending.add(change.getDest());
+ try (AutoCloseable ctx = requestContext.open()) {
+ HashSet<Branch.NameKey> pending = new HashSet<>();
+ for (ChangeData cd : queryProvider.get().allSubmitted()) {
+ try {
+ pending.add(cd.change().getDest());
+ } catch (OrmException e) {
+ log.error("Error reading submitted change", e);
}
- } finally {
- c.close();
}
- } catch (OrmException e) {
- log.error("Cannot reload MergeQueue", e);
- }
- for (final Branch.NameKey branch : pending) {
- mergeQueue.schedule(branch);
+ for (Branch.NameKey branch : pending) {
+ mergeQueue.schedule(branch);
+ }
+ } catch (Exception e) {
+ log.error("Cannot reload MergeQueue", e);
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
index 6bcd92d..982f897 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java
@@ -14,28 +14,27 @@
package com.google.gerrit.server.index;
+import static com.google.gerrit.server.query.change.ChangeData.asChanges;
+
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.AsyncFunction;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
-import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.QueueProvider.QueueType;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.server.util.RequestContext;
-import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import com.google.inject.util.Providers;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -48,24 +47,21 @@
private static final Logger log = LoggerFactory
.getLogger(ReindexAfterUpdate.class);
- private final ThreadLocalRequestContext tl;
- private final SchemaFactory<ReviewDb> schemaFactory;
- private final IdentifiedUser.GenericFactory userFactory;
+ private final OneOffRequestContext requestContext;
+ private final Provider<InternalChangeQuery> queryProvider;
private final ChangeIndexer.Factory indexerFactory;
private final IndexCollection indexes;
private final ListeningExecutorService executor;
@Inject
ReindexAfterUpdate(
- ThreadLocalRequestContext tl,
- SchemaFactory<ReviewDb> schemaFactory,
- IdentifiedUser.GenericFactory userFactory,
+ OneOffRequestContext requestContext,
+ Provider<InternalChangeQuery> queryProvider,
ChangeIndexer.Factory indexerFactory,
IndexCollection indexes,
@IndexExecutor(QueueType.BATCH) ListeningExecutorService executor) {
- this.tl = tl;
- this.schemaFactory = schemaFactory;
- this.userFactory = userFactory;
+ this.requestContext = requestContext;
+ this.queryProvider = queryProvider;
this.indexerFactory = indexerFactory;
this.indexes = indexes;
this.executor = executor;
@@ -81,8 +77,7 @@
List<ListenableFuture<Void>> result =
Lists.newArrayListWithCapacity(changes.size());
for (Change c : changes) {
- result.add(executor.submit(
- new Index(event, c.getOwner(), c.getId())));
+ result.add(executor.submit(new Index(event, c.getId())));
}
return Futures.allAsList(result);
}
@@ -90,7 +85,6 @@
}
private abstract class Task<V> implements Callable<V> {
- protected ReviewDb db;
protected Event event;
protected Task(Event event) {
@@ -99,20 +93,15 @@
@Override
public final V call() throws Exception {
- try {
- db = schemaFactory.open();
- return impl();
+ try (ManualRequestContext ctx = requestContext.open()) {
+ return impl(ctx);
} catch (Exception e) {
log.error("Failed to reindex changes after " + event, e);
throw e;
- } finally {
- if (db != null) {
- db.close();
- }
}
}
- protected abstract V impl() throws Exception;
+ protected abstract V impl(RequestContext ctx) throws Exception;
}
private class GetChanges extends Task<List<Change>> {
@@ -121,49 +110,33 @@
}
@Override
- protected List<Change> impl() throws OrmException {
+ protected List<Change> impl(RequestContext ctx) throws OrmException {
String ref = event.getRefName();
Project.NameKey project = new Project.NameKey(event.getProjectName());
if (ref.equals(RefNames.REFS_CONFIG)) {
- return db.changes().byProjectOpenAll(project).toList();
+ return asChanges(queryProvider.get().byProjectOpen(project));
} else {
- return db.changes().byBranchOpenAll(new Branch.NameKey(project, ref))
- .toList();
+ return asChanges(queryProvider.get().byBranchOpen(
+ new Branch.NameKey(project, ref)));
}
}
}
private class Index extends Task<Void> {
- private final Account.Id user;
private final Change.Id id;
- Index(Event event, Account.Id user, Change.Id id) {
+ Index(Event event, Change.Id id) {
super(event);
- this.user = user;
this.id = id;
}
@Override
- protected Void impl() throws OrmException, IOException {
- RequestContext context = new RequestContext() {
- @Override
- public CurrentUser getCurrentUser() {
- return userFactory.create(user);
- }
-
- @Override
- public Provider<ReviewDb> getReviewDbProvider() {
- return Providers.of(db);
- }
- };
- RequestContext old = tl.setContext(context);
- try {
- Change c = db.changes().get(id);
- indexerFactory.create(executor, indexes).index(db, c);
- return null;
- } finally {
- tl.setContext(old);
- }
+ protected Void impl(RequestContext ctx) throws OrmException, IOException {
+ // Reload change, as some time may have passed since GetChanges.
+ ReviewDb db = ctx.getReviewDbProvider().get();
+ Change c = db.changes().get(id);
+ indexerFactory.create(executor, indexes).index(db, c);
+ return null;
}
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java
index 8b029dd..d5c34bc 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/SiteIndexer.java
@@ -21,6 +21,7 @@
import com.google.common.base.Stopwatch;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
@@ -31,6 +32,7 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -39,6 +41,7 @@
import com.google.gerrit.server.git.MultiProgressMonitor.Task;
import com.google.gerrit.server.patch.PatchListLoader;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
@@ -65,9 +68,11 @@
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintWriter;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -237,7 +242,7 @@
repo = repoManager.openRepository(project);
Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL);
db = schemaFactory.open();
- for (Change c : db.changes().byProject(project)) {
+ for (Change c : scanChanges(db, repo)) {
Ref r = refs.get(c.currentPatchSetId().toRefName());
if (r != null) {
byId.put(r.getObjectId(), changeDataFactory.create(db, c));
@@ -268,6 +273,26 @@
};
}
+ private static List<Change> scanChanges(ReviewDb db, Repository repo)
+ throws OrmException, IOException {
+ Map<String, Ref> refs =
+ repo.getRefDatabase().getRefs(RefNames.REFS_CHANGES);
+ Set<Change.Id> ids = new LinkedHashSet<>();
+ for (Ref r : refs.values()) {
+ Change.Id id = Change.Id.fromRef(r.getName());
+ if (id != null) {
+ ids.add(id);
+ }
+ }
+ List<Change> changes = new ArrayList<>(ids.size());
+ // A batch size of N may overload get(Iterable), so use something smaller,
+ // but still >1.
+ for (List<Change.Id> batch : Iterables.partition(ids, 30)) {
+ Iterables.addAll(changes, db.changes().get(batch));
+ }
+ return changes;
+ }
+
private static class ProjectIndexer implements Callable<Void> {
private final ChangeIndexer indexer;
private final ThreeWayMergeStrategy mergeStrategy;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
index 162a97d..9a714f8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/DeleteBranch.java
@@ -25,6 +25,7 @@
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.DeleteBranch.Input;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet;
import com.google.inject.Inject;
@@ -48,16 +49,19 @@
private final Provider<IdentifiedUser> identifiedUser;
private final GitRepositoryManager repoManager;
private final Provider<ReviewDb> dbProvider;
+ private final Provider<InternalChangeQuery> queryProvider;
private final GitReferenceUpdated referenceUpdated;
private final ChangeHooks hooks;
@Inject
DeleteBranch(Provider<IdentifiedUser> identifiedUser,
GitRepositoryManager repoManager, Provider<ReviewDb> dbProvider,
+ Provider<InternalChangeQuery> queryProvider,
GitReferenceUpdated referenceUpdated, ChangeHooks hooks) {
this.identifiedUser = identifiedUser;
this.repoManager = repoManager;
this.dbProvider = dbProvider;
+ this.queryProvider = queryProvider;
this.referenceUpdated = referenceUpdated;
this.hooks = hooks;
}
@@ -68,8 +72,8 @@
if (!rsrc.getControl().controlForRef(rsrc.getBranchKey()).canDelete()) {
throw new AuthException("Cannot delete branch");
}
- if (dbProvider.get().changes().byBranchOpenAll(rsrc.getBranchKey())
- .iterator().hasNext()) {
+ if (!queryProvider.get().setLimit(1)
+ .byBranchOpen(rsrc.getBranchKey()).isEmpty()) {
throw new ResourceConflictException("branch " + rsrc.getBranchKey()
+ " has open changes");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
index dcb4924..b3a0a9a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java
@@ -75,6 +75,24 @@
import java.util.Map;
public class ChangeData {
+ public static List<Change> asChanges(List<ChangeData> changeDatas)
+ throws OrmException {
+ List<Change> result = new ArrayList<>(changeDatas.size());
+ for (ChangeData cd : changeDatas) {
+ result.add(cd.change());
+ }
+ return result;
+ }
+
+ public static Map<Change.Id, ChangeData> asMap(List<ChangeData> changes) {
+ Map<Change.Id, ChangeData> result =
+ Maps.newHashMapWithExpectedSize(changes.size());
+ for (ChangeData cd : changes) {
+ result.put(cd.getId(), cd);
+ }
+ return result;
+ }
+
public static void ensureChangeLoaded(Iterable<ChangeData> changes)
throws OrmException {
Map<Change.Id, ChangeData> missing = Maps.newHashMap();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
index 40ae816..7a0609a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/InternalChangeQuery.java
@@ -17,6 +17,8 @@
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.Branch;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.query.Predicate;
import com.google.gerrit.server.query.QueryParseException;
@@ -25,17 +27,36 @@
import java.util.List;
-/** Execute a single query over changes, for use by Gerrit internals. */
+/**
+ * Execute a single query over changes, for use by Gerrit internals.
+ * <p>
+ * By default, visibility of returned changes is not enforced (unlike in {@link
+ * QueryProcessor}). The methods in this class are not typically used by
+ * user-facing paths, but rather by internal callers that need to process all
+ * matching results.
+ */
public class InternalChangeQuery {
- private static Predicate<ChangeData> project(Project.NameKey projectName) {
- return new ProjectPredicate(projectName.get());
+ private static Predicate<ChangeData> ref(Branch.NameKey branch) {
+ return new RefPredicate(branch.get());
+ }
+
+ private static Predicate<ChangeData> change(Change.Key key) {
+ return new ChangeIdPredicate(key.get());
+ }
+
+ private static Predicate<ChangeData> project(Project.NameKey project) {
+ return new ProjectPredicate(project.get());
+ }
+
+ private static Predicate<ChangeData> status(Change.Status status) {
+ return new ChangeStatusPredicate(status);
}
private final QueryProcessor qp;
@Inject
InternalChangeQuery(QueryProcessor queryProcessor) {
- qp = queryProcessor;
+ qp = queryProcessor.enforceVisibility(false);
}
public InternalChangeQuery setLimit(int n) {
@@ -43,9 +64,47 @@
return this;
}
- public List<ChangeData> byProjectOpen(Project.NameKey projectName)
+ public InternalChangeQuery enforceVisibility(boolean enforce) {
+ qp.enforceVisibility(enforce);
+ return this;
+ }
+
+
+ public List<ChangeData> byBranchKey(Branch.NameKey branch, Change.Key key)
throws OrmException {
- return query(and(project(projectName), open()));
+ return query(and(
+ ref(branch),
+ project(branch.getParentKey()),
+ change(key)));
+ }
+
+ public List<ChangeData> byProject(Project.NameKey project)
+ throws OrmException {
+ return query(project(project));
+ }
+
+ public List<ChangeData> submitted(Branch.NameKey branch) throws OrmException {
+ return query(and(
+ ref(branch),
+ project(branch.getParentKey()),
+ status(Change.Status.SUBMITTED)));
+ }
+
+ public List<ChangeData> allSubmitted() throws OrmException {
+ return query(status(Change.Status.SUBMITTED));
+ }
+
+ public List<ChangeData> byBranchOpen(Branch.NameKey branch)
+ throws OrmException {
+ return query(and(
+ ref(branch),
+ project(branch.getParentKey()),
+ open()));
+ }
+
+ public List<ChangeData> byProjectOpen(Project.NameKey project)
+ throws OrmException {
+ return query(and(project(project), open()));
}
private List<ChangeData> query(Predicate<ChangeData> p) throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/ManualRequestContext.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/ManualRequestContext.java
new file mode 100644
index 0000000..0ef4a18
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/ManualRequestContext.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2015 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.util;
+
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
+
+/**
+ * Closeable version of a {@link RequestContext} with manually-specified
+ * providers.
+ */
+public class ManualRequestContext implements RequestContext, AutoCloseable {
+ private final CurrentUser user;
+ private final Provider<ReviewDb> db;
+ private final ThreadLocalRequestContext requestContext;
+ private final RequestContext old;
+
+ ManualRequestContext(CurrentUser user, SchemaFactory<ReviewDb> schemaFactory,
+ ThreadLocalRequestContext requestContext) throws OrmException {
+ this.user = user;
+ this.db = Providers.of(schemaFactory.open());
+ this.requestContext = requestContext;
+ old = requestContext.setContext(this);
+ }
+
+ @Override
+ public CurrentUser getCurrentUser() {
+ return user;
+ }
+
+ @Override
+ public Provider<ReviewDb> getReviewDbProvider() {
+ return db;
+ }
+
+ @Override
+ public void close() {
+ requestContext.setContext(old);
+ db.get().close();
+ }
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/util/OneOffRequestContext.java b/gerrit-server/src/main/java/com/google/gerrit/server/util/OneOffRequestContext.java
new file mode 100644
index 0000000..6feb182
--- /dev/null
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/util/OneOffRequestContext.java
@@ -0,0 +1,51 @@
+// Copyright (C) 2015 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.util;
+
+import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.InternalUser;
+import com.google.gwtorm.server.OrmException;
+import com.google.gwtorm.server.SchemaFactory;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/**
+ * Helper to create one-off request contexts.
+ * <p>
+ * Each call to {@link #open()} opens a new {@link ReviewDb}, so this class
+ * should only be used in a bounded try/finally block.
+ * <p>
+ * The user in the request context is {@link InternalUser}.
+ */
+@Singleton
+public class OneOffRequestContext {
+ private final InternalUser.Factory userFactory;
+ private final SchemaFactory<ReviewDb> schemaFactory;
+ private final ThreadLocalRequestContext requestContext;
+
+ @Inject
+ OneOffRequestContext(InternalUser.Factory userFactory,
+ SchemaFactory<ReviewDb> schemaFactory,
+ ThreadLocalRequestContext requestContext) {
+ this.userFactory = userFactory;
+ this.schemaFactory = schemaFactory;
+ this.requestContext = requestContext;
+ }
+
+ public ManualRequestContext open() throws OrmException {
+ return new ManualRequestContext(userFactory.create(),
+ schemaFactory, requestContext);
+ }
+}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
index c59381e..e75c1c8 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java
@@ -21,6 +21,7 @@
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.ChangesCollection;
import com.google.gerrit.server.change.DeleteReviewer;
@@ -29,7 +30,6 @@
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ProjectControl;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException;
