opendev/gerrit
Code Issues Proposed changes

Remove ReviewDb args from merge code

Browse Source 
Change-Id: If1b42cdf66b4001eec753174ba13204f8bb846a3
This commit is contained in:
Dave Borowitz
2018-12-14 14:45:25 -08:00
parent 495e5c7b36
commit 712fa897b6
14 changed files with 22 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
java/com/google/gerrit/server/git/receive/ReceiveCommits.java
View File

@@ -683,7 +683,7 @@ class ReceiveCommits {
// Update superproject gitlinks if required.
if (!branches.isEmpty()) {
try (MergeOpRepoManager orm = ormProvider.get()) {
orm.setContext(db, TimeUtil.nowTs(), user);
orm.setContext(TimeUtil.nowTs(), user);
SubmoduleOp op = subOpFactory.create(branches, orm);
op.updateSuperProjects();
} catch (SubmoduleException e) {
@@ -2507,7 +2507,7 @@ class ReceiveCommits {
logger.atFine().log(
"Processing submit with tip change %s (%s)", tipChange.getId(), magicBranch.cmd.getNewId());
try (MergeOp op = mergeOpProvider.get()) {
op.merge(db, tipChange, user, false, new SubmitInput(), false);
op.merge(tipChange, user, false, new SubmitInput(), false);
}
}

7
java/com/google/gerrit/server/restapi/change/GetRevisionActions.java
View File

@@ -19,7 +19,6 @@ import com.google.common.hash.Hashing;
import com.google.gerrit.extensions.common.ActionInfo;
import com.google.gerrit.extensions.restapi.ETagView;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ActionJson;
import com.google.gerrit.server.change.ChangeResource;
@@ -42,19 +41,16 @@ import org.eclipse.jgit.lib.Config;
public class GetRevisionActions implements ETagView<RevisionResource> {
private final ActionJson delegate;
private final Config config;
private final Provider<ReviewDb> dbProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
private final ChangeResource.Factory changeResourceFactory;
@Inject
GetRevisionActions(
ActionJson delegate,
Provider<ReviewDb> dbProvider,
Provider<MergeSuperSet> mergeSuperSet,
ChangeResource.Factory changeResourceFactory,
@GerritServerConfig Config config) {
this.delegate = delegate;
this.dbProvider = dbProvider;
this.mergeSuperSet = mergeSuperSet;
this.changeResourceFactory = changeResourceFactory;
this.config = config;
@@ -72,8 +68,7 @@ public class GetRevisionActions implements ETagView<RevisionResource> {
try {
rsrc.getChangeResource().prepareETag(h, user);
h.putBoolean(MergeSuperSet.wholeTopicEnabled(config));
ReviewDb db = dbProvider.get();
ChangeSet cs = mergeSuperSet.get().completeChangeSet(db, rsrc.getChange(), user);
ChangeSet cs = mergeSuperSet.get().completeChangeSet(rsrc.getChange(), user);
for (ChangeData cd : cs.changes()) {
changeResourceFactory.create(cd.notes(), user).prepareETag(h, user);
}

6
java/com/google/gerrit/server/restapi/change/ListRobotComments.java
View File

@@ -18,7 +18,6 @@ import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.RobotComment;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -31,14 +30,11 @@ import java.util.Map;
@Singleton
public class ListRobotComments implements RestReadView<RevisionResource> {
protected final Provider<ReviewDb> db;
protected final Provider<CommentJson> commentJson;
protected final CommentsUtil commentsUtil;
@Inject
ListRobotComments(
Provider<ReviewDb> db, Provider<CommentJson> commentJson, CommentsUtil commentsUtil) {
this.db = db;
ListRobotComments(Provider<CommentJson> commentJson, CommentsUtil commentsUtil) {
this.commentJson = commentJson;
this.commentsUtil = commentsUtil;
}

7
java/com/google/gerrit/server/restapi/change/PreviewSubmit.java
View File

@@ -26,7 +26,6 @@ import com.google.gerrit.extensions.restapi.RestReadView;
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.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.change.ArchiveFormat;
@@ -61,7 +60,6 @@ import org.kohsuke.args4j.Option;
public class PreviewSubmit implements RestReadView<RevisionResource> {
private static final int MAX_DEFAULT_BUNDLE_SIZE = 100 * 1024 * 1024;
private final Provider<ReviewDb> dbProvider;
private final Provider<MergeOp> mergeOpProvider;
private final AllowedFormats allowedFormats;
private int maxBundleSize;
@@ -74,11 +72,9 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
@Inject
PreviewSubmit(
Provider<ReviewDb> dbProvider,
Provider<MergeOp> mergeOpProvider,
AllowedFormats allowedFormats,
@GerritServerConfig Config cfg) {
this.dbProvider = dbProvider;
this.mergeOpProvider = mergeOpProvider;
this.allowedFormats = allowedFormats;
this.maxBundleSize = cfg.getInt("download", "maxBundleSize", MAX_DEFAULT_BUNDLE_SIZE);
@@ -116,14 +112,13 @@ public class PreviewSubmit implements RestReadView<RevisionResource> {
private BinaryResult getBundles(RevisionResource rsrc, ArchiveFormat f)
throws OrmException, RestApiException, UpdateException, IOException, ConfigInvalidException,
PermissionBackendException {
ReviewDb db = dbProvider.get();
IdentifiedUser caller = rsrc.getUser().asIdentifiedUser();
Change change = rsrc.getChange();
@SuppressWarnings("resource") // Returned BinaryResult takes ownership and handles closing.
MergeOp op = mergeOpProvider.get();
try {
op.merge(db, change, caller, false, new SubmitInput(), true);
op.merge(change, caller, false, new SubmitInput(), true);
BinaryResult bin = new SubmitPreviewResult(op, f, maxBundleSize);
bin.disableGzip()
.setContentType(f.getMimeType())

10
java/com/google/gerrit/server/restapi/change/Submit.java
View File

@@ -36,7 +36,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RevId;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -107,7 +106,6 @@ public class Submit
}
}
private final Provider<ReviewDb> dbProvider;
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
@@ -128,7 +126,6 @@ public class Submit
@Inject
Submit(
Provider<ReviewDb> dbProvider,
GitRepositoryManager repoManager,
PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
@@ -140,7 +137,6 @@ public class Submit
Provider<InternalChangeQuery> queryProvider,
PatchSetUtil psUtil,
ProjectCache projectCache) {
this.dbProvider = dbProvider;
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
@@ -212,8 +208,7 @@ public class Submit
}
try (MergeOp op = mergeOpProvider.get()) {
ReviewDb db = dbProvider.get();
op.merge(db, change, submitter, true, input, false);
op.merge(change, submitter, true, input, false);
try {
change = changeNotesFactory.createChecked(change.getProject(), change.getId()).getChange();
} catch (NoSuchChangeException e) {
@@ -321,7 +316,6 @@ public class Submit
throw new OrmRuntimeException("Could not determine problems for the change", e);
}
ReviewDb db = dbProvider.get();
ChangeData cd = changeDataFactory.create(resource.getNotes());
try {
MergeOp.checkSubmitRule(cd, false);
@@ -334,7 +328,7 @@ public class Submit
ChangeSet cs;
try {
cs = mergeSuperSet.get().completeChangeSet(db, cd.change(), resource.getUser());
cs = mergeSuperSet.get().completeChangeSet(cd.change(), resource.getUser());
} catch (OrmException | IOException | PermissionBackendException e) {
throw new OrmRuntimeException(
"Could not determine complete set of changes to be submitted", e);

7
java/com/google/gerrit/server/restapi/change/SubmittedTogether.java
View File

@@ -28,7 +28,6 @@ import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.change.ChangeJson;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.WalkSorter;
@@ -62,7 +61,6 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
Comparator.comparing(ChangeData::project).thenComparing(cd -> cd.getId().id, reverseOrder());
private final ChangeJson.Factory json;
private final Provider<ReviewDb> dbProvider;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeSuperSet> mergeSuperSet;
private final Provider<WalkSorter> sorter;
@@ -89,12 +87,10 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
@Inject
SubmittedTogether(
ChangeJson.Factory json,
Provider<ReviewDb> dbProvider,
Provider<InternalChangeQuery> queryProvider,
Provider<MergeSuperSet> mergeSuperSet,
Provider<WalkSorter> sorter) {
this.json = json;
this.dbProvider = dbProvider;
this.queryProvider = queryProvider;
this.mergeSuperSet = mergeSuperSet;
this.sorter = sorter;
@@ -129,8 +125,7 @@ public class SubmittedTogether implements RestReadView<ChangeResource> {
int hidden;
if (c.getStatus().isOpen()) {
ChangeSet cs =
mergeSuperSet.get().completeChangeSet(dbProvider.get(), c, resource.getUser());
ChangeSet cs = mergeSuperSet.get().completeChangeSet(c, resource.getUser());
cds = ensureRequiredDataIsLoaded(cs.changes().asList());
hidden = cs.nonVisibleChanges().size();
} else if (c.getStatus().asChangeStatus() == ChangeStatus.MERGED) {

3
java/com/google/gerrit/server/submit/LocalMergeSuperSetComputation.java
View File

@@ -28,7 +28,6 @@ import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -109,7 +108,7 @@ public class LocalMergeSuperSetComputation implements MergeSuperSetComputation {
@Override
public ChangeSet completeWithoutTopic(
ReviewDb db, MergeOpRepoManager orm, ChangeSet changeSet, CurrentUser user)
MergeOpRepoManager orm, ChangeSet changeSet, CurrentUser user)
throws OrmException, IOException, PermissionBackendException {
Collection<ChangeData> visibleChanges = new ArrayList<>();
Collection<ChangeData> nonVisibleChanges = new ArrayList<>();

10
java/com/google/gerrit/server/submit/MergeOp.java
View File

@@ -52,7 +52,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
@@ -239,7 +238,6 @@ public class MergeOp implements AutoCloseable {
private MergeOpRepoManager orm;
private CommitStatus commitStatus;
private ReviewDb db;
private SubmitInput submitInput;
private ListMultimap<RecipientType, Account.Id> accountsToNotify;
private Set<Project.NameKey> allProjects;
@@ -427,7 +425,6 @@ public class MergeOp implements AutoCloseable {
* topic or via superproject subscriptions. All affected changes are integrated using the projects
* integration strategy.
*
* @param db the review database.
* @param change the change to be merged.
* @param caller the identity of the caller
* @param checkSubmitRules whether the prolog submit rules should be evaluated
@@ -438,7 +435,6 @@ public class MergeOp implements AutoCloseable {
* @throws IOException an error occurred reading from NoteDb.
*/
public void merge(
ReviewDb db,
Change change,
IdentifiedUser caller,
boolean checkSubmitRules,
@@ -451,7 +447,6 @@ public class MergeOp implements AutoCloseable {
this.dryrun = dryrun;
this.caller = caller;
this.ts = TimeUtil.nowTs();
this.db = db;
this.submissionId = new RequestId(change.getId().toString());
try (TraceContext traceContext =
@@ -461,7 +456,7 @@ public class MergeOp implements AutoCloseable {
logger.atFine().log("Beginning integration of %s", change);
try {
ChangeSet indexBackedChangeSet =
mergeSuperSet.setMergeOpRepoManager(orm).completeChangeSet(db, change, caller);
mergeSuperSet.setMergeOpRepoManager(orm).completeChangeSet(change, caller);
checkState(
indexBackedChangeSet.ids().contains(change.getId()),
"change %s missing from %s",
@@ -536,7 +531,7 @@ public class MergeOp implements AutoCloseable {
orm.close();
}
orm = ormProvider.get();
orm.setContext(db, ts, caller);
orm.setContext(ts, caller);
}
private ChangeSet reloadChanges(ChangeSet changeSet) {
@@ -669,7 +664,6 @@ public class MergeOp implements AutoCloseable {
SubmitStrategy strategy =
submitStrategyFactory.create(
submitting.submitType(),
db,
or.rw,
or.canMergeFlag,
getAlreadyAccepted(or, ob.oldTip),

7
java/com/google/gerrit/server/submit/MergeOpRepoManager.java
View File

@@ -20,7 +20,6 @@ import com.google.common.collect.Maps;
import com.google.gerrit.reviewdb.client.Branch;
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.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -105,7 +104,7 @@ public class MergeOpRepoManager implements AutoCloseable {
}
public BatchUpdate getUpdate() {
checkState(db != null, "call setContext before getUpdate");
checkState(caller != null, "call setContext before getUpdate");
if (update == null) {
update =
batchUpdateFactory
@@ -157,7 +156,6 @@ public class MergeOpRepoManager implements AutoCloseable {
private final GitRepositoryManager repoManager;
private final ProjectCache projectCache;
private ReviewDb db;
private Timestamp ts;
private IdentifiedUser caller;
@@ -175,8 +173,7 @@ public class MergeOpRepoManager implements AutoCloseable {
openRepos = new HashMap<>();
}
public void setContext(ReviewDb db, Timestamp ts, IdentifiedUser caller) {
this.db = db;
public void setContext(Timestamp ts, IdentifiedUser caller) {
this.ts = ts;
this.caller = caller;
}

17
java/com/google/gerrit/server/submit/MergeSuperSet.java
View File

@@ -22,7 +22,6 @@ import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.logging.TraceContext;
@@ -93,7 +92,7 @@ public class MergeSuperSet {
return this;
}
public ChangeSet completeChangeSet(ReviewDb db, Change change, CurrentUser user)
public ChangeSet completeChangeSet(Change change, CurrentUser user)
throws IOException, OrmException, PermissionBackendException {
try {
if (orm == null) {
@@ -120,10 +119,10 @@ public class MergeSuperSet {
ChangeSet changeSet = new ChangeSet(cd, visible);
if (wholeTopicEnabled(cfg)) {
return completeChangeSetIncludingTopics(db, changeSet, user);
return completeChangeSetIncludingTopics(changeSet, user);
}
try (TraceContext traceContext = PluginContext.newTrace(mergeSuperSetComputation)) {
return mergeSuperSetComputation.get().completeWithoutTopic(db, orm, changeSet, user);
return mergeSuperSetComputation.get().completeWithoutTopic(orm, changeSet, user);
}
} finally {
if (closeOrm && orm != null) {
@@ -137,9 +136,8 @@ public class MergeSuperSet {
* Completes {@code changeSet} with any additional changes from its topics
*
* <p>{@link #completeChangeSetIncludingTopics} calls this repeatedly, alternating with {@link
* MergeSuperSetComputation#completeWithoutTopic(ReviewDb, MergeOpRepoManager, ChangeSet,
* CurrentUser)}, to discover what additional changes should be submitted with a change until the
* set stops growing.
* MergeSuperSetComputation#completeWithoutTopic(MergeOpRepoManager, ChangeSet, CurrentUser)}, to
* discover what additional changes should be submitted with a change until the set stops growing.
*
* <p>{@code topicsSeen} and {@code visibleTopicsSeen} keep track of topics already explored to
* avoid wasted work.
@@ -182,8 +180,7 @@ public class MergeSuperSet {
return new ChangeSet(visibleChanges, nonVisibleChanges);
}
private ChangeSet completeChangeSetIncludingTopics(
ReviewDb db, ChangeSet changeSet, CurrentUser user)
private ChangeSet completeChangeSetIncludingTopics(ChangeSet changeSet, CurrentUser user)
throws IOException, OrmException, PermissionBackendException {
Set<String> topicsSeen = new HashSet<>();
Set<String> visibleTopicsSeen = new HashSet<>();
@@ -196,7 +193,7 @@ public class MergeSuperSet {
do {
oldSeen = seen;
try (TraceContext traceContext = PluginContext.newTrace(mergeSuperSetComputation)) {
changeSet = mergeSuperSetComputation.get().completeWithoutTopic(db, orm, changeSet, user);
changeSet = mergeSuperSetComputation.get().completeWithoutTopic(orm, changeSet, user);
}
changeSet = topicClosure(changeSet, user, topicsSeen, visibleTopicsSeen);
seen = topicsSeen.size() + visibleTopicsSeen.size();

5
java/com/google/gerrit/server/submit/MergeSuperSetComputation.java
View File

@@ -15,7 +15,6 @@
package com.google.gerrit.server.submit;
import com.google.gerrit.extensions.annotations.ExtensionPoint;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gwtorm.server.OrmException;
@@ -41,13 +40,11 @@ public interface MergeSuperSetComputation {
* <p>This method is invoked iteratively while new changes to be submitted together are discovered
* by expanding the topics of the changes. This method must not do any topic expansion on its own.
*
* @param db {@link ReviewDb} instance
* @param orm {@link MergeOpRepoManager} that should be used to access repositories
* @param changeSet A set of changes for which it is known that they should be submitted together
* @param user The user for which the visibility checks should be performed
* @return the completed set of changes that should be submitted together
*/
ChangeSet completeWithoutTopic(
ReviewDb db, MergeOpRepoManager orm, ChangeSet changeSet, CurrentUser user)
ChangeSet completeWithoutTopic(MergeOpRepoManager orm, ChangeSet changeSet, CurrentUser user)
throws OrmException, IOException, PermissionBackendException;
}

5
java/com/google/gerrit/server/submit/SubmitStrategy.java
View File

@@ -25,7 +25,6 @@ import com.google.gerrit.extensions.config.FactoryModule;
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.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.GerritPersonIdent;
@@ -91,7 +90,6 @@ public abstract class SubmitStrategy {
IdentifiedUser caller,
MergeTip mergeTip,
RevFlag canMergeFlag,
ReviewDb db,
Set<RevCommit> alreadyAccepted,
Set<CodeReviewCommit> incoming,
RequestId submissionId,
@@ -124,7 +122,6 @@ public abstract class SubmitStrategy {
final IdentifiedUser caller;
final MergeTip mergeTip;
final RevFlag canMergeFlag;
final ReviewDb db;
final Set<RevCommit> alreadyAccepted;
final RequestId submissionId;
final SubmitType submitType;
@@ -163,7 +160,6 @@ public abstract class SubmitStrategy {
@Assisted IdentifiedUser caller,
@Assisted MergeTip mergeTip,
@Assisted RevFlag canMergeFlag,
@Assisted ReviewDb db,
@Assisted Set<RevCommit> alreadyAccepted,
@Assisted Set<CodeReviewCommit> incoming,
@Assisted RequestId submissionId,
@@ -194,7 +190,6 @@ public abstract class SubmitStrategy {
this.caller = caller;
this.mergeTip = mergeTip;
this.canMergeFlag = canMergeFlag;
this.db = db;
this.alreadyAccepted = alreadyAccepted;
this.submissionId = submissionId;
this.submitType = submitType;

3
java/com/google/gerrit/server/submit/SubmitStrategyFactory.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.CodeReviewCommit;
import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
@@ -48,7 +47,6 @@ public class SubmitStrategyFactory {
public SubmitStrategy create(
SubmitType submitType,
ReviewDb db,
CodeReviewRevWalk rw,
RevFlag canMergeFlag,
Set<RevCommit> alreadyAccepted,
@@ -72,7 +70,6 @@ public class SubmitStrategyFactory {
caller,
mergeTip,
canMergeFlag,
db,
alreadyAccepted,
incoming,
submissionId,