From f8be8fdca66d0ed526d9d1542530b81395e86175 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Mon, 18 Nov 2013 14:12:20 +0100 Subject: [PATCH] Keep mergeability flag always up to date The mergeability flag of a change is immediately updated when the change is created or when a new patch set is uploaded. When a branch is updated the mergeability flag is asynchronously updated for all open changes that are destined for that branch. This way the mergeability flag is always up to date. With this the (new) change screen doesn't need to explicitly trigger the update of this flag whenever the screen is loaded. The corresponding call is removed. In a follow-up change we can now display the mergeability flag also in the change tables and user dashboards which wouldn't make sense if the displayed information is outdated. It is important to update the mergeability flag on a branch update in the background since the computation and even the check whether the mergeability flag needs to be recomputed is expensive. Change-Id: Id20e567e5c5d5b3d851ca5cda9bde01ac01c9967 Signed-off-by: Edwin Kempin --- Documentation/config-gerrit.txt | 7 + .../gerrit/client/change/ChangeScreen2.java | 44 +--- .../java/com/google/gerrit/pgm/Daemon.java | 2 + gerrit-server/BUCK | 1 + .../gerrit/server/change/ChangeInserter.java | 12 +- .../server/change/MergeabilityCheckQueue.java | 42 +++ .../server/change/MergeabilityChecker.java | 241 ++++++++++++++++++ .../change/MergeabilityChecksExecutor.java | 31 +++ .../MergeabilityChecksExecutorModule.java | 41 +++ .../server/change/PatchSetInserter.java | 12 +- .../server/config/GerritGlobalModule.java | 2 + .../gerrit/server/git/ReceiveCommits.java | 10 +- .../gerrit/testutil/InMemoryModule.java | 2 + .../gerrit/httpd/WebAppInitializer.java | 2 + 14 files changed, 403 insertions(+), 46 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCheckQueue.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutor.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutorModule.java diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 78d50baf8c..4f8a1988c3 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -811,6 +811,13 @@ the result. + By default this is false (test is not enabled). +[[changeMerge.threadPoolSize]]changeMerge.threadPoolSize:: ++ +Maximum size of the thread pool in which the mergeability flag of open +changes is updated. ++ +Default is 1. + [[commentlink]]Section commentlink ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Comment links are find/replace strings applied to change descriptions, diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java index 2b3a012fbd..ee695f3b69 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java @@ -20,7 +20,6 @@ import com.google.gerrit.client.actions.ActionInfo; import com.google.gerrit.client.changes.ChangeApi; import com.google.gerrit.client.changes.ChangeInfo; import com.google.gerrit.client.changes.ChangeInfo.CommitInfo; -import com.google.gerrit.client.changes.ChangeInfo.MergeableInfo; import com.google.gerrit.client.changes.ChangeInfo.MessageInfo; import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo; import com.google.gerrit.client.changes.ChangeList; @@ -608,35 +607,6 @@ public class ChangeScreen2 extends Screen { })); } - private void loadMergeable(final Change.Status status, final boolean canSubmit) { - if (Gerrit.getConfig().testChangeMerge()) { - ChangeApi.revision(changeId.get(), revision) - .view("mergeable") - .get(new AsyncCallback() { - @Override - public void onSuccess(MergeableInfo result) { - if (canSubmit) { - actions.setSubmitEnabled(result.mergeable()); - if (status == Change.Status.NEW) { - statusText.setInnerText(result.mergeable() - ? Util.C.readyToSubmit() - : Util.C.mergeConflict()); - } - } - setVisible(notMergeable, !result.mergeable()); - renderSubmitType(result.submit_type()); - } - - @Override - public void onFailure(Throwable caught) { - loadSubmitType(status, canSubmit); - } - }); - } else { - loadSubmitType(status, canSubmit); - } - } - private void loadSubmitType(final Change.Status status, final boolean canSubmit) { if (canSubmit) { actions.setSubmitEnabled(true); @@ -649,6 +619,18 @@ public class ChangeScreen2 extends Screen { .get(new AsyncCallback() { @Override public void onSuccess(NativeString result) { + if (Gerrit.getConfig().testChangeMerge()) { + if (canSubmit) { + actions.setSubmitEnabled(changeInfo.mergeable()); + if (status == Change.Status.NEW) { + statusText.setInnerText(changeInfo.mergeable() + ? Util.C.readyToSubmit() + : Util.C.mergeConflict()); + } + } + setVisible(notMergeable, !changeInfo.mergeable()); + } + renderSubmitType(result.asString()); } @@ -722,7 +704,7 @@ public class ChangeScreen2 extends Screen { history.set(commentLinkProcessor, replyAction, changeId, info); if (current) { - loadMergeable(info.status(), canSubmit); + loadSubmitType(info.status(), canSubmit); } StringBuilder sb = new StringBuilder(); diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java index aee16da557..579186e0a5 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/Daemon.java @@ -44,6 +44,7 @@ import com.google.gerrit.pgm.util.SiteProgram; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; +import com.google.gerrit.server.change.MergeabilityChecksExecutorModule; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.CanonicalWebUrlModule; @@ -301,6 +302,7 @@ public class Daemon extends SiteProgram { modules.add(new WorkQueue.Module()); modules.add(new ChangeHookRunner.Module()); modules.add(new ReceiveCommitsExecutorModule()); + modules.add(new MergeabilityChecksExecutorModule()); modules.add(new IntraLineWorkerPool.Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); modules.add(new InternalAccountDirectory.Module()); diff --git a/gerrit-server/BUCK b/gerrit-server/BUCK index 8b3f531614..d88867f2b9 100644 --- a/gerrit-server/BUCK +++ b/gerrit-server/BUCK @@ -145,6 +145,7 @@ java_test( '//gerrit-common:server', '//gerrit-extension-api:api', '//gerrit-reviewdb:server', + '//gerrit-server/src/main/prolog:common', '//lib:args4j', '//lib:easymock', '//lib:guava', diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index 77ad83ba41..cc17c71c1f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -30,7 +30,6 @@ import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; -import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.RefControl; @@ -61,7 +60,7 @@ public class ChangeInserter { private final ChangeHooks hooks; private final ApprovalsUtil approvalsUtil; private final TrackingFooters trackingFooters; - private final ChangeIndexer indexer; + private final MergeabilityChecker mergeabilityChecker; private final CreateChangeSender.Factory createChangeSenderFactory; private final RefControl refControl; @@ -83,7 +82,7 @@ public class ChangeInserter { ChangeHooks hooks, ApprovalsUtil approvalsUtil, TrackingFooters trackingFooters, - ChangeIndexer indexer, + MergeabilityChecker mergeabilityChecker, CreateChangeSender.Factory createChangeSenderFactory, @Assisted RefControl refControl, @Assisted Change change, @@ -93,7 +92,7 @@ public class ChangeInserter { this.hooks = hooks; this.approvalsUtil = approvalsUtil; this.trackingFooters = trackingFooters; - this.indexer = indexer; + this.mergeabilityChecker = mergeabilityChecker; this.createChangeSenderFactory = createChangeSenderFactory; this.refControl = refControl; this.change = change; @@ -175,7 +174,8 @@ public class ChangeInserter { db.changeMessages().insert(Collections.singleton(changeMessage)); } - CheckedFuture indexFuture = indexer.indexAsync(change); + CheckedFuture f = + mergeabilityChecker.updateAndIndexAsync(change); gitRefUpdated.fire(change.getProject(), patchSet.getRefName(), ObjectId.zeroId(), commit); @@ -196,7 +196,7 @@ public class ChangeInserter { log.error("Cannot send email for new change " + change.getId(), err); } } - indexFuture.checkedGet(); + f.checkedGet(); return change; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCheckQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCheckQueue.java new file mode 100644 index 0000000000..66b9127ddd --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityCheckQueue.java @@ -0,0 +1,42 @@ +// Copyright (C) 2013 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.change; + +import com.google.common.collect.Sets; +import com.google.gerrit.reviewdb.client.Change; + +import java.util.Collection; +import java.util.Set; + +import javax.inject.Singleton; + +@Singleton +class MergeabilityCheckQueue { + private final Set pending = Sets.newHashSet(); + + synchronized Set addAll(Collection changes) { + Set r = Sets.newLinkedHashSetWithExpectedSize(changes.size()); + for (Change c : changes) { + if (pending.add(c.getId())) { + r.add(c); + } + } + return r; + } + + synchronized void updatingMergeabilityFlag(Change change) { + pending.remove(change.getId()); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java new file mode 100644 index 0000000000..5b0d3d7534 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecker.java @@ -0,0 +1,241 @@ +// Copyright (C) 2013 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.change; + +import com.google.common.base.Function; +import com.google.common.base.Throwables; +import com.google.common.util.concurrent.AsyncFunction; +import com.google.common.util.concurrent.CheckedFuture; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.common.util.concurrent.ListeningExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.gerrit.extensions.events.GitReferenceUpdatedListener; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.reviewdb.client.Branch; +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.server.ReviewDb; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.change.Mergeable.MergeableInfo; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.WorkQueue.Executor; +import com.google.gerrit.server.index.ChangeIndexer; +import com.google.gerrit.server.project.ChangeControl; +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.ProvisionException; + +import org.eclipse.jgit.lib.Constants; + +import java.io.IOException; +import java.util.List; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; + +public class MergeabilityChecker implements GitReferenceUpdatedListener { + private final ThreadLocalRequestContext tl; + private final SchemaFactory schemaFactory; + private final ChangeControl.GenericFactory changeControlFactory; + private final IdentifiedUser.GenericFactory identifiedUserFactory; + private final Provider mergeable; + private final ChangeIndexer indexer; + private final ListeningExecutorService executor; + private final MergeabilityCheckQueue mergeabilityCheckQueue; + + @Inject + public MergeabilityChecker(ThreadLocalRequestContext tl, + SchemaFactory schemaFactory, + ChangeControl.GenericFactory changeControlFactory, + IdentifiedUser.GenericFactory identifiedUserFactory, + Provider mergeable, ChangeIndexer indexer, + @MergeabilityChecksExecutor Executor executor, + MergeabilityCheckQueue mergeabilityCheckQueue) { + this.tl = tl; + this.schemaFactory = schemaFactory; + this.changeControlFactory = changeControlFactory; + this.identifiedUserFactory = identifiedUserFactory; + this.mergeable = mergeable; + this.indexer = indexer; + this.executor = MoreExecutors.listeningDecorator(executor); + this.mergeabilityCheckQueue = mergeabilityCheckQueue; + } + + private static final Function MAPPER = + new Function() { + @Override + public IOException apply(Exception in) { + if (in instanceof IOException) { + return (IOException) in; + } else if (in instanceof ExecutionException + && in.getCause() instanceof IOException) { + return (IOException) in.getCause(); + } else { + return new IOException(in); + } + } + }; + + @Override + public void onGitReferenceUpdated(Event event) { + String ref = event.getRefName(); + if (ref.startsWith(Constants.R_HEADS) || ref.equals(GitRepositoryManager.REF_CONFIG)) { + executor.submit(new RefUpdateTask(schemaFactory, + new Project.NameKey(event.getProjectName()), ref)); + } + } + + /** + * Updates the mergeability flag of the change asynchronously. If the + * mergeability flag is updated the change is reindexed. + * + * @param change the change for which the mergeability flag should be updated + * @return CheckedFuture that updates the mergeability flag of the change and + * returns true if the mergeability flag was updated and + * the change was reindexed, and false if the + * mergeability flag was not updated and the change was not reindexed + */ + public CheckedFuture updateAsync(Change change) { + return Futures.makeChecked( + executor.submit(new ChangeUpdateTask(schemaFactory, change)), MAPPER); + } + + /** + * Updates the mergeability flag of the change asynchronously and reindexes + * the change in any case. + * + * @param change the change for which the mergeability flag should be updated + * @return CheckedFuture that updates the mergeability flag of the change and + * reindexes the change (whether the mergeability flag was updated or + * not) + */ + public CheckedFuture updateAndIndexAsync(final Change change) { + return Futures.makeChecked( + Futures.transform(updateAsync(change), + new AsyncFunction() { + @SuppressWarnings("unchecked") + @Override + public ListenableFuture apply(Boolean indexUpdated) + throws Exception { + if (!indexUpdated) { + return (ListenableFuture) indexer.indexAsync(change); + } + return Futures.immediateFuture(null); + } + }), MAPPER); + } + + public boolean update(Change change) throws IOException { + try { + return new ChangeUpdateTask(schemaFactory, change).call(); + } catch (Exception e) { + Throwables.propagateIfPossible(e); + throw MAPPER.apply(e); + } + } + + private class ChangeUpdateTask implements Callable { + private final SchemaFactory schemaFactory; + private final Change change; + + private ReviewDb reviewDb; + + ChangeUpdateTask(SchemaFactory schemaFactory, Change change) { + this.schemaFactory = schemaFactory; + this.change = change; + } + + @Override + public Boolean call() throws Exception { + mergeabilityCheckQueue.updatingMergeabilityFlag(change); + + RequestContext context = new RequestContext() { + @Override + public CurrentUser getCurrentUser() { + return identifiedUserFactory.create(change.getOwner()); + } + + @Override + public Provider getReviewDbProvider() { + return new Provider() { + @Override + public ReviewDb get() { + if (reviewDb == null) { + try { + reviewDb = schemaFactory.open(); + } catch (OrmException e) { + throw new ProvisionException("Cannot open ReviewDb", e); + } + } + return reviewDb; + } + }; + } + }; + RequestContext old = tl.setContext(context); + ReviewDb db = context.getReviewDbProvider().get(); + try { + PatchSet ps = db.patchSets().get(change.currentPatchSetId()); + MergeableInfo info = mergeable.get().apply(new RevisionResource(new ChangeResource( + changeControlFactory.controlFor(change, context.getCurrentUser())), ps)); + return change.isMergeable() != info.mergeable; + } catch (ResourceConflictException e) { + // change is closed + return false; + } finally { + tl.setContext(old); + if (reviewDb != null) { + reviewDb.close(); + reviewDb = null; + } + } + } + } + + private class RefUpdateTask implements Callable { + private final SchemaFactory schemaFactory; + private final Project.NameKey project; + private final String ref; + + RefUpdateTask(SchemaFactory schemaFactory, + Project.NameKey project, String ref) { + this.schemaFactory = schemaFactory; + this.project = project; + this.ref = ref; + } + + @Override + public Void call() throws Exception { + List openChanges; + ReviewDb db = schemaFactory.open(); + try { + openChanges = db.changes().byBranchOpenAll(new Branch.NameKey(project, ref)).toList(); + } finally { + db.close(); + } + + for (Change change : mergeabilityCheckQueue.addAll(openChanges)) { + updateAsync(change); + } + return null; + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutor.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutor.java new file mode 100644 index 0000000000..7503fe341a --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutor.java @@ -0,0 +1,31 @@ +// Copyright (C) 2013 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.change; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import com.google.gerrit.server.git.WorkQueue; +import com.google.inject.BindingAnnotation; + +import java.lang.annotation.Retention; + +/** + * Marker on the global {@link WorkQueue.Executor} used by + * {@link MergeabilityChecker}. + */ +@Retention(RUNTIME) +@BindingAnnotation +public @interface MergeabilityChecksExecutor { +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutorModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutorModule.java new file mode 100644 index 0000000000..102cfafe83 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/MergeabilityChecksExecutorModule.java @@ -0,0 +1,41 @@ +// Copyright (C) 2013 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.change; + +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.git.WorkQueue; +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; + +import org.eclipse.jgit.lib.Config; + + +/** Module providing the {@link MergeabilityChecksExecutor}. */ +public class MergeabilityChecksExecutorModule extends AbstractModule { + @Override + protected void configure() { + } + + @Provides + @Singleton + @MergeabilityChecksExecutor + public WorkQueue.Executor createMergeabilityChecksExecutor( + @GerritServerConfig Config config, + WorkQueue queues) { + int poolSize = config.getInt("changeMerge", null, "threadPoolSize", 1); + return queues.createQueue(poolSize, "MergeabilityChecks"); + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index f604e0b4e6..a1c602d4e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -37,7 +37,6 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidators; -import com.google.gerrit.server.index.ChangeIndexer; import com.google.gerrit.server.mail.ReplacePatchSetSender; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.InvalidChangeOperationException; @@ -96,7 +95,7 @@ public class PatchSetInserter { private final IdentifiedUser user; private final GitReferenceUpdated gitRefUpdated; private final CommitValidators.Factory commitValidatorsFactory; - private final ChangeIndexer indexer; + private final MergeabilityChecker mergeabilityChecker; private final ReplacePatchSetSender.Factory replacePatchSetFactory; private final MergeUtil.Factory mergeUtilFactory; @@ -122,7 +121,7 @@ public class PatchSetInserter { PatchSetInfoFactory patchSetInfoFactory, GitReferenceUpdated gitRefUpdated, CommitValidators.Factory commitValidatorsFactory, - ChangeIndexer indexer, + MergeabilityChecker mergeabilityChecker, ReplacePatchSetSender.Factory replacePatchSetFactory, MergeUtil.Factory mergeUtilFactory, @Assisted Repository git, @@ -138,7 +137,7 @@ public class PatchSetInserter { this.user = user; this.gitRefUpdated = gitRefUpdated; this.commitValidatorsFactory = commitValidatorsFactory; - this.indexer = indexer; + this.mergeabilityChecker = mergeabilityChecker; this.replacePatchSetFactory = replacePatchSetFactory; this.mergeUtilFactory = mergeUtilFactory; @@ -317,11 +316,12 @@ public class PatchSetInserter { } finally { db.rollback(); } - CheckedFuture e = indexer.indexAsync(updatedChange); + CheckedFuture f = + mergeabilityChecker.updateAndIndexAsync(change); if (runHooks) { hooks.doPatchsetCreatedHook(updatedChange, patchSet, db); } - e.checkedGet(); + f.checkedGet(); return updatedChange; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java index 4b0600a2d6..723fa82f13 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -68,6 +68,7 @@ import com.google.gerrit.server.auth.AuthBackend; import com.google.gerrit.server.auth.UniversalAuthBackend; import com.google.gerrit.server.avatar.AvatarProvider; import com.google.gerrit.server.cache.CacheRemovalListener; +import com.google.gerrit.server.change.MergeabilityChecker; import com.google.gerrit.server.documentation.QueryDocumentationExecutor; import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; @@ -244,6 +245,7 @@ public class GerritGlobalModule extends FactoryModule { DynamicSet.setOf(binder(), ProjectDeletedListener.class); DynamicSet.setOf(binder(), HeadUpdatedListener.class); DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(ChangeCache.class); + DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(MergeabilityChecker.class); DynamicSet.setOf(binder(), ChangeListener.class); DynamicSet.setOf(binder(), CommitValidationListener.class); DynamicSet.setOf(binder(), MergeValidationListener.class); 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 12daafa859..5b7b3a2d53 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 @@ -17,7 +17,6 @@ package com.google.gerrit.server.git; import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals; import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters; - import static org.eclipse.jgit.lib.Constants.R_HEADS; import static org.eclipse.jgit.lib.RefDatabase.ALL; import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED; @@ -68,6 +67,7 @@ import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.MergeabilityChecker; import com.google.gerrit.server.change.PatchSetInserter; import com.google.gerrit.server.change.PatchSetInserter.ChangeKind; import com.google.gerrit.server.change.RevisionResource; @@ -272,6 +272,7 @@ public class ReceiveCommits { private final ListeningExecutorService changeUpdateExector; private final RequestScopePropagator requestScopePropagator; private final ChangeIndexer indexer; + private final MergeabilityChecker mergeabilityChecker; private final SshInfo sshInfo; private final AllProjectsName allProjectsName; private final ReceiveConfig receiveConfig; @@ -335,6 +336,7 @@ public class ReceiveCommits { @ChangeUpdateExecutor ListeningExecutorService changeUpdateExector, final RequestScopePropagator requestScopePropagator, final ChangeIndexer indexer, + final MergeabilityChecker mergeabilityChecker, final SshInfo sshInfo, final AllProjectsName allProjectsName, ReceiveConfig config, @@ -368,6 +370,7 @@ public class ReceiveCommits { this.changeUpdateExector = changeUpdateExector; this.requestScopePropagator = requestScopePropagator; this.indexer = indexer; + this.mergeabilityChecker = mergeabilityChecker; this.sshInfo = sshInfo; this.allProjectsName = allProjectsName; this.receiveConfig = config; @@ -1915,7 +1918,8 @@ public class ReceiveCommits { if (cmd.getResult() == NOT_ATTEMPTED) { cmd.execute(rp); } - CheckedFuture indexFuture = indexer.indexAsync(change); + CheckedFuture f = + mergeabilityChecker.updateAndIndexAsync(change); gitRefUpdated.fire(project.getNameKey(), newPatchSet.getRefName(), ObjectId.zeroId(), newCommit); hooks.doPatchsetCreatedHook(change, newPatchSet, db); @@ -1949,7 +1953,7 @@ public class ReceiveCommits { return "send-email newpatchset"; } })); - indexFuture.checkedGet(); + f.checkedGet(); if (magicBranch != null && magicBranch.isSubmit()) { submit(changeCtl, newPatchSet); diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java index 111b014dbd..a8337ac509 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/InMemoryModule.java @@ -25,6 +25,7 @@ import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdentProvider; import com.google.gerrit.server.RemotePeer; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; +import com.google.gerrit.server.change.MergeabilityChecksExecutorModule; import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsNameProvider; import com.google.gerrit.server.config.AnonymousCowardName; @@ -155,6 +156,7 @@ public class InMemoryModule extends FactoryModule { install(new DefaultCacheFactory.Module()); install(new SmtpEmailSender.Module()); install(new SignedTokenEmailTokenVerifier.Module()); + install(new MergeabilityChecksExecutorModule()); IndexType indexType = null; try { diff --git a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java index 36595c014b..3920164584 100644 --- a/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java +++ b/gerrit-war/src/main/java/com/google/gerrit/httpd/WebAppInitializer.java @@ -26,6 +26,7 @@ import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory; +import com.google.gerrit.server.change.MergeabilityChecksExecutorModule; import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.CanonicalWebUrlModule; @@ -250,6 +251,7 @@ public class WebAppInitializer extends GuiceServletContextListener modules.add(new WorkQueue.Module()); modules.add(new ChangeHookRunner.Module()); modules.add(new ReceiveCommitsExecutorModule()); + modules.add(new MergeabilityChecksExecutorModule()); modules.add(new IntraLineWorkerPool.Module()); modules.add(cfgInjector.getInstance(GerritGlobalModule.class)); modules.add(new InternalAccountDirectory.Module());