From f487f855bb4cd283deab6c095566d9d0ec08b20f Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Mon, 9 Nov 2015 19:10:50 -0800 Subject: [PATCH] Make receive setup consistent between SSH and HTTP Configuration was different between these two transports, but should be identical. Use a common set of initialization in ReceiveCommits. Change-Id: I0b93e8bb1504018fa926dd628900b2fc9cd0dd9c --- .../gerrit/httpd/GitOverHttpServlet.java | 27 ++----------- .../server/git/LazyPostReceiveHookChain.java | 40 +++++++++++++++++++ .../gerrit/server/git/ReceiveCommits.java | 23 +++++++++-- .../google/gerrit/sshd/commands/Receive.java | 32 +-------------- 4 files changed, 66 insertions(+), 56 deletions(-) create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/git/LazyPostReceiveHookChain.java diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java index 89d62dd08a..3cee55306b 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java @@ -273,18 +273,10 @@ public class GitOverHttpServlet extends GitServlet { static class ReceiveFactory implements ReceivePackFactory { private final AsyncReceiveCommits.Factory factory; - private final TransferConfig config; - private DynamicSet receivePackInitializers; - private DynamicSet postReceiveHooks; @Inject - ReceiveFactory(AsyncReceiveCommits.Factory factory, TransferConfig config, - DynamicSet receivePackInitializers, - DynamicSet postReceiveHooks) { + ReceiveFactory(AsyncReceiveCommits.Factory factory) { this.factory = factory; - this.config = config; - this.receivePackInitializers = receivePackInitializers; - this.postReceiveHooks = postReceiveHooks; } @Override @@ -297,24 +289,13 @@ public class GitOverHttpServlet extends GitServlet { throw new ServiceNotAuthorizedException(); } - final IdentifiedUser user = pc.getUser().asIdentifiedUser(); - final ReceiveCommits rc = factory.create(pc, db).getReceiveCommits(); + ReceiveCommits rc = factory.create(pc, db).getReceiveCommits(); + rc.init(); + ReceivePack rp = rc.getReceivePack(); - rp.setRefLogIdent(user.newRefLogIdent()); - rp.setTimeout(config.getTimeout()); - rp.setMaxObjectSizeLimit(config.getMaxObjectSizeLimit()); - init(pc.getProject().getNameKey(), rp); - rp.setPostReceiveHook(PostReceiveHookChain.newChain( - Lists.newArrayList(postReceiveHooks))); req.setAttribute(ATT_RC, rc); return rp; } - - private void init(Project.NameKey project, ReceivePack rp) { - for (ReceivePackInitializer initializer : receivePackInitializers) { - initializer.init(project, rp); - } - } } static class DisabledReceiveFactory implements diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/LazyPostReceiveHookChain.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/LazyPostReceiveHookChain.java new file mode 100644 index 0000000000..ebfaae777d --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/LazyPostReceiveHookChain.java @@ -0,0 +1,40 @@ +// 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.git; + +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.inject.Inject; + +import org.eclipse.jgit.transport.PostReceiveHook; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.transport.ReceivePack; + +import java.util.Collection; + +class LazyPostReceiveHookChain implements PostReceiveHook { + private final DynamicSet hooks; + + @Inject + LazyPostReceiveHookChain(DynamicSet hooks) { + this.hooks = hooks; + } + + @Override + public void onPostReceive(ReceivePack rp, Collection commands) { + for (PostReceiveHook h : hooks) { + h.onPostReceive(rp, commands); + } + } +} 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 304ebc257e..4d2880bc59 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 @@ -65,6 +65,7 @@ import com.google.gerrit.common.data.PermissionRule; import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap.Entry; +import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Account; @@ -308,6 +309,7 @@ public class ReceiveCommits { private final SshInfo sshInfo; private final AllProjectsName allProjectsName; private final ReceiveConfig receiveConfig; + private final DynamicSet initializers; private final ChangeKindCache changeKindCache; private final BatchUpdate.Factory batchUpdateFactory; private final SetHashtagsOp.Factory hashtagsFactory; @@ -378,7 +380,10 @@ public class ReceiveCommits { final ChangeIndexer indexer, final SshInfo sshInfo, final AllProjectsName allProjectsName, - ReceiveConfig config, + ReceiveConfig receiveConfig, + TransferConfig transferConfig, + DynamicSet initializers, + Provider lazyPostReceive, @Assisted final ProjectControl projectControl, @Assisted final Repository repo, final Provider subOpProvider, @@ -420,7 +425,8 @@ public class ReceiveCommits { this.indexer = indexer; this.sshInfo = sshInfo; this.allProjectsName = allProjectsName; - this.receiveConfig = config; + this.receiveConfig = receiveConfig; + this.initializers = initializers; this.changeKindCache = changeKindCache; this.batchUpdateFactory = batchUpdateFactory; this.hashtagsFactory = hashtagsFactory; @@ -448,6 +454,10 @@ public class ReceiveCommits { rp.setAllowCreates(true); rp.setAllowDeletes(true); rp.setAllowNonFastForwards(true); + rp.setRefLogIdent(user.newRefLogIdent()); + rp.setTimeout(transferConfig.getTimeout()); + rp.setMaxObjectSizeLimit(transferConfig.getEffectiveMaxObjectSizeLimit( + projectControl.getProjectState())); rp.setCheckReceivedObjects(ps.getConfig().getCheckReceivedObjects()); rp.setRefFilter(new RefFilter() { @Override @@ -465,7 +475,7 @@ public class ReceiveCommits { }); if (!projectControl.allRefsAreVisible()) { - rp.setCheckReferencedObjectsAreReachable(config.checkReferencedObjectsAreReachable); + rp.setCheckReferencedObjectsAreReachable(receiveConfig.checkReferencedObjectsAreReachable); rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo, projectControl, db, false)); } List advHooks = new ArrayList<>(3); @@ -497,6 +507,13 @@ public class ReceiveCommits { db, queryProvider, projectControl.getProject().getNameKey())); advHooks.add(new HackPushNegotiateHook()); rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks)); + rp.setPostReceiveHook(lazyPostReceive.get()); + } + + public void init() { + for (ReceivePackInitializer i : initializers) { + i.init(projectControl.getProject().getNameKey(), rp); + } } /** Add reviewers for new (or updated) changes. */ diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java index abb788dee4..5950f91aad 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java @@ -14,15 +14,11 @@ package com.google.gerrit.sshd.commands; -import com.google.common.collect.Lists; import com.google.gerrit.common.data.Capable; -import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.AsyncReceiveCommits; import com.google.gerrit.server.git.ReceiveCommits; -import com.google.gerrit.server.git.ReceivePackInitializer; -import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.sshd.AbstractGitCommand; import com.google.gerrit.sshd.CommandMetaData; @@ -33,8 +29,6 @@ import org.eclipse.jgit.errors.UnpackException; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.transport.AdvertiseRefsHook; -import org.eclipse.jgit.transport.PostReceiveHook; -import org.eclipse.jgit.transport.PostReceiveHookChain; import org.eclipse.jgit.transport.ReceivePack; import org.kohsuke.args4j.Option; import org.slf4j.Logger; @@ -61,15 +55,6 @@ final class Receive extends AbstractGitCommand { @Inject private IdentifiedUser.GenericFactory identifiedUserFactory; - @Inject - private TransferConfig config; - - @Inject - private DynamicSet receivePackInitializers; - - @Inject - private DynamicSet postReceiveHooks; - private final Set reviewerId = new HashSet<>(); private final Set ccId = new HashSet<>(); @@ -100,17 +85,10 @@ final class Receive extends AbstractGitCommand { verifyProjectVisible("reviewer", reviewerId); verifyProjectVisible("CC", ccId); + receive.init(); receive.addReviewers(reviewerId); receive.addExtraCC(ccId); - - final ReceivePack rp = receive.getReceivePack(); - rp.setRefLogIdent(currentUser.newRefLogIdent()); - rp.setTimeout(config.getTimeout()); - rp.setMaxObjectSizeLimit(config.getEffectiveMaxObjectSizeLimit( - projectControl.getProjectState())); - init(rp); - rp.setPostReceiveHook(PostReceiveHookChain.newChain( - Lists.newArrayList(postReceiveHooks))); + ReceivePack rp = receive.getReceivePack(); try { rp.receive(in, out, err); } catch (UnpackException badStream) { @@ -177,12 +155,6 @@ final class Receive extends AbstractGitCommand { } } - private void init(ReceivePack rp) { - for (ReceivePackInitializer initializer : receivePackInitializers) { - initializer.init(projectControl.getProject().getNameKey(), rp); - } - } - private void verifyProjectVisible(final String type, final Set who) throws UnloggedFailure { for (final Account.Id id : who) {