diff --git a/gradle/changelog/fix_pack_parser_error b/gradle/changelog/fix_pack_parser_error new file mode 100644 index 0000000000..92496aa311 --- /dev/null +++ b/gradle/changelog/fix_pack_parser_error @@ -0,0 +1,2 @@ +- type: bugfix + description: Sporadic error in reading git pack files ([#1518](https://github.com/scm-manager/scm-manager/issues/1518)) diff --git a/scm-plugins/scm-git-plugin/src/main/java/org/eclipse/jgit/transport/ScmTransportProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/org/eclipse/jgit/transport/ScmTransportProtocol.java index 61c3013f36..9fcfa69314 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/org/eclipse/jgit/transport/ScmTransportProtocol.java +++ b/scm-plugins/scm-git-plugin/src/main/java/org/eclipse/jgit/transport/ScmTransportProtocol.java @@ -36,8 +36,8 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.RepositoryCache; import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; -import sonia.scm.repository.spi.HookEventFacade; import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitHookEventFacade; import sonia.scm.web.GitReceiveHook; import java.io.File; @@ -54,7 +54,7 @@ public class ScmTransportProtocol extends TransportProtocol { private static final Set SCHEMES = ImmutableSet.of(NAME); private Provider converterFactory; - private Provider hookEventFacadeProvider; + private Provider hookEventFacadeProvider; private Provider repositoryHandlerProvider; public ScmTransportProtocol() { @@ -63,7 +63,7 @@ public class ScmTransportProtocol extends TransportProtocol { @Inject public ScmTransportProtocol( Provider converterFactory, - Provider hookEventFacadeProvider, + Provider hookEventFacadeProvider, Provider repositoryHandlerProvider) { this.converterFactory = converterFactory; this.hookEventFacadeProvider = hookEventFacadeProvider; @@ -110,11 +110,11 @@ public class ScmTransportProtocol extends TransportProtocol { private final GitChangesetConverterFactory converterFactory; private final GitRepositoryHandler handler; - private final HookEventFacade hookEventFacade; + private final GitHookEventFacade hookEventFacade; public TransportLocalWithHooks( GitChangesetConverterFactory converterFactory, - HookEventFacade hookEventFacade, + GitHookEventFacade hookEventFacade, GitRepositoryHandler handler, Repository local, URIish uri, File gitDir) { super(local, uri, gitDir); diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java index 028e91009d..18840670c6 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/BaseReceivePackFactory.java @@ -31,8 +31,8 @@ import org.eclipse.jgit.transport.resolver.ServiceNotAuthorizedException; import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; -import sonia.scm.repository.spi.HookEventFacade; import sonia.scm.web.CollectingPackParserListener; +import sonia.scm.web.GitHookEventFacade; import sonia.scm.web.GitReceiveHook; public abstract class BaseReceivePackFactory implements ReceivePackFactory { @@ -40,7 +40,7 @@ public abstract class BaseReceivePackFactory implements ReceivePackFactory private final GitRepositoryHandler handler; private final GitReceiveHook hook; - protected BaseReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + protected BaseReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) { this.handler = handler; this.hook = new GitReceiveHook(converterFactory, hookEventFacade, handler); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java index 7902c1cef1..9dbc7a5a71 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/GitCommandProtocol.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.protocolcommand.git; import org.eclipse.jgit.lib.Repository; @@ -39,6 +39,7 @@ import sonia.scm.protocolcommand.CommandContext; import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.protocolcommand.ScmCommandProtocol; import sonia.scm.repository.RepositoryPermissions; +import sonia.scm.web.GitHookEventFacade; import javax.inject.Inject; import java.io.IOException; @@ -48,19 +49,20 @@ public class GitCommandProtocol implements ScmCommandProtocol { private static final Logger LOG = LoggerFactory.getLogger(GitCommandProtocol.class); - private ScmUploadPackFactory uploadPackFactory; - private ScmReceivePackFactory receivePackFactory; + private final ScmUploadPackFactory uploadPackFactory; + private final ScmReceivePackFactory receivePackFactory; + private final GitHookEventFacade gitHookEventFacade; @Inject - public GitCommandProtocol(ScmUploadPackFactory uploadPackFactory, ScmReceivePackFactory receivePackFactory) { + public GitCommandProtocol(ScmUploadPackFactory uploadPackFactory, ScmReceivePackFactory receivePackFactory, GitHookEventFacade gitHookEventFacade) { this.uploadPackFactory = uploadPackFactory; this.receivePackFactory = receivePackFactory; + this.gitHookEventFacade = gitHookEventFacade; } @Override public void handle(CommandContext commandContext, RepositoryContext repositoryContext) throws IOException { String subCommand = commandContext.getArgs()[0]; - if (RemoteConfig.DEFAULT_UPLOAD_PACK.equals(subCommand)) { LOG.trace("got upload pack"); upload(commandContext, repositoryContext); @@ -79,6 +81,8 @@ public class GitCommandProtocol implements ScmCommandProtocol { receivePack.receive(commandContext.getInputStream(), commandContext.getOutputStream(), commandContext.getErrorStream()); } catch (ServiceNotEnabledException | ServiceNotAuthorizedException e) { throw new IOException("error creating receive pack for ssh", e); + } finally { + gitHookEventFacade.firePending(); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java index a999a57d4d..8fe5e5ce3b 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/protocolcommand/git/ScmReceivePackFactory.java @@ -30,12 +30,12 @@ import org.eclipse.jgit.transport.ReceivePack; import sonia.scm.protocolcommand.RepositoryContext; import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; -import sonia.scm.repository.spi.HookEventFacade; +import sonia.scm.web.GitHookEventFacade; public class ScmReceivePackFactory extends BaseReceivePackFactory { @Inject - public ScmReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + public ScmReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) { super(converterFactory, handler, hookEventFacade); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHookContextProvider.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHookContextProvider.java index ce61b85a57..1b03a87406 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHookContextProvider.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitHookContextProvider.java @@ -123,6 +123,10 @@ public class GitHookContextProvider extends HookContextProvider return SUPPORTED_FEATURES; } + public String getRepositoryId() { + return repositoryId; + } + private final GitHookChangesetProvider changesetProvider; private final List receiveCommands; private final ReceivePack receivePack; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitHookEventFacade.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitHookEventFacade.java new file mode 100644 index 0000000000..1c86ce1072 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitHookEventFacade.java @@ -0,0 +1,136 @@ +/* + * MIT License + * + * Copyright (c) 2020-present Cloudogu GmbH and Contributors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package sonia.scm.web; + +import com.google.common.util.concurrent.ThreadFactoryBuilder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import sonia.scm.repository.RepositoryHookType; +import sonia.scm.repository.spi.GitHookContextProvider; +import sonia.scm.repository.spi.HookEventFacade; + +import javax.annotation.Nonnull; +import javax.inject.Inject; +import java.io.Closeable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +/** + * This class is used to delay the firing of post commit hooks, so that they are not + * executed while the internal git processing has not finished. Without this, this can + * lead to conflicting access to pack files which results in 'Short compressed stream' + * errors (see https://github.com/scm-manager/scm-manager/pull/1518). + *
+ * The delay is handled either by "caching" the hook data in a thread local, where it + * is fetched from when {@link #firePending()} is called, or in case the trigger is fired + * due to changes made with internal git file push (from a workdir to the central + * repository) by detecting the internal thread used by JGit and joining another thread + * where the pending push is triggered. + */ +public class GitHookEventFacade implements Closeable { + + private static final Logger LOG = LoggerFactory.getLogger(GitHookEventFacade.class); + + private static final ThreadLocal PENDING_POST_HOOK = new ThreadLocal<>(); + + private final HookEventFacade hookEventFacade; + private final ExecutorService internalThreadHookHandler; + + @Inject + public GitHookEventFacade(HookEventFacade hookEventFacade) { + this.hookEventFacade = hookEventFacade; + this.internalThreadHookHandler = createInternalThreadHookHandlerPool(); + } + + public void fire(RepositoryHookType type, GitHookContextProvider context) { + switch (type) { + case PRE_RECEIVE: + doFire(type, context); + break; + case POST_RECEIVE: + Thread thread = Thread.currentThread(); + if ("JGit-Receive-Pack".equals(thread.getName())) { + // this thread name is used in the JGit class org.eclipse.jgit.transport.InternalPushConnection + LOG.debug("handling internal git thread for post receive hook"); + handleGitInternalThread(context, thread); + } else { + LOG.debug("register post receive hook for repository id {} in thread {}", context.getRepositoryId(), thread); + PENDING_POST_HOOK.set(context); + } + break; + default: + throw new IllegalArgumentException("unknown hook type: " + type); + } + } + + public void firePending() { + try { + LOG.debug("fire pending post receive hooks in thread {}", Thread.currentThread()); + doFire(RepositoryHookType.POST_RECEIVE, PENDING_POST_HOOK.get()); + } finally { + clean(); + } + } + + private void handleGitInternalThread(GitHookContextProvider context, Thread internalJGitThread) { + internalThreadHookHandler.submit(() -> { + try { + internalJGitThread.join(); + } catch (InterruptedException e) { + LOG.debug("got interrupted in internal git thread for repository id {}", context.getRepositoryId(), e); + } finally { + LOG.debug("internal git thread ended for repository id {}", context.getRepositoryId()); + doFire(RepositoryHookType.POST_RECEIVE, context); + } + }); + } + + public void clean() { + PENDING_POST_HOOK.remove(); + } + + private void doFire(RepositoryHookType type, GitHookContextProvider context) { + if (context != null) { + LOG.debug("firing {} hook for repository {} in Thread {}", type, context.getRepositoryId(), Thread.currentThread()); + hookEventFacade.handle(context.getRepositoryId()).fireHookEvent(type, context); + } else { + LOG.debug("No context found for event type {} in Thread {}", type, Thread.currentThread()); + } + } + + @Nonnull + private ExecutorService createInternalThreadHookHandlerPool() { + return Executors.newCachedThreadPool( + new ThreadFactoryBuilder() + .setNameFormat("GitInternalThreadHookHandler-%d") + .build() + ); + } + + @Override + public void close() { + internalThreadHookHandler.shutdown(); + } +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java index a093b5cda3..9cae0611f8 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java @@ -38,7 +38,6 @@ import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.RepositoryHookType; import sonia.scm.repository.spi.GitHookContextProvider; -import sonia.scm.repository.spi.HookEventFacade; import java.io.IOException; import java.util.Collection; @@ -57,17 +56,11 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook private static final Logger logger = LoggerFactory.getLogger(GitReceiveHook.class); - //~--- constructors --------------------------------------------------------- + private final GitRepositoryHandler handler; + private final GitChangesetConverterFactory converterFactory; + private final GitHookEventFacade hookEventFacade; - /** - * Constructs ... - * - * - * - * @param hookEventFacade - * @param handler - */ - public GitReceiveHook(GitChangesetConverterFactory converterFactory, HookEventFacade hookEventFacade, + public GitReceiveHook(GitChangesetConverterFactory converterFactory, GitHookEventFacade hookEventFacade, GitRepositoryHandler handler) { this.converterFactory = converterFactory; @@ -75,15 +68,6 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook this.handler = handler; } - //~--- methods -------------------------------------------------------------- - - /** - * Method description - * - * - * @param rpack - * @param receiveCommands - */ @Override public void onPostReceive(ReceivePack rpack, Collection receiveCommands) @@ -91,14 +75,6 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook onReceive(rpack, receiveCommands, RepositoryHookType.POST_RECEIVE); } - /** - * Method description - * - * - * - * @param rpack - * @param receiveCommands - */ @Override public void onPreReceive(ReceivePack rpack, Collection receiveCommands) @@ -106,14 +82,6 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook onReceive(rpack, receiveCommands, RepositoryHookType.PRE_RECEIVE); } - /** - * Method description - * - * - * @param rpack - * @param receiveCommands - * @param type - */ private void handleReceiveCommands(ReceivePack rpack, List receiveCommands, RepositoryHookType type) { @@ -126,8 +94,7 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook GitHookContextProvider context = new GitHookContextProvider(converterFactory, rpack, receiveCommands, repository, repositoryId); - hookEventFacade.handle(repositoryId).fireHookEvent(type, context); - + hookEventFacade.fire(type, context); } catch (Exception ex) { @@ -183,13 +150,4 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook StoredConfig gitConfig = repository.getConfig(); return handler.getRepositoryId(gitConfig); } - - //~--- fields --------------------------------------------------------------- - - /** Field description */ - private GitRepositoryHandler handler; - - private final GitChangesetConverterFactory converterFactory; - /** Field description */ - private HookEventFacade hookEventFacade; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java index dc18189da9..49cfed592d 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceivePackFactory.java @@ -36,7 +36,6 @@ import org.eclipse.jgit.transport.resolver.ServiceNotEnabledException; import sonia.scm.protocolcommand.git.BaseReceivePackFactory; import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; -import sonia.scm.repository.spi.HookEventFacade; import javax.servlet.http.HttpServletRequest; @@ -54,7 +53,7 @@ public class GitReceivePackFactory extends BaseReceivePackFactory wrapped; @Inject - public GitReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, HookEventFacade hookEventFacade) { + public GitReceivePackFactory(GitChangesetConverterFactory converterFactory, GitRepositoryHandler handler, GitHookEventFacade hookEventFacade) { super(converterFactory, handler, hookEventFacade); this.wrapped = new DefaultReceivePackFactory(); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/ScmGitServlet.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/ScmGitServlet.java index a063127b77..d4236e45b8 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/ScmGitServlet.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/ScmGitServlet.java @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web; import com.google.common.annotations.VisibleForTesting; @@ -68,27 +68,17 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet //~--- constructors --------------------------------------------------------- - /** - * Constructs ... - * - * - * - * @param repositoryResolver - * @param receivePackFactory - * @param repositoryViewer - * @param repositoryRequestListenerUtil - * @param lfsServletFactory - */ @Inject public ScmGitServlet(GitRepositoryResolver repositoryResolver, GitReceivePackFactory receivePackFactory, GitRepositoryViewer repositoryViewer, RepositoryRequestListenerUtil repositoryRequestListenerUtil, - LfsServletFactory lfsServletFactory) + LfsServletFactory lfsServletFactory, GitHookEventFacade gitHookEventFacade) { this.repositoryViewer = repositoryViewer; this.repositoryRequestListenerUtil = repositoryRequestListenerUtil; this.lfsServletFactory = lfsServletFactory; + this.gitHookEventFacade = gitHookEventFacade; setRepositoryResolver(repositoryResolver); setReceivePackFactory(receivePackFactory); @@ -109,7 +99,7 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet @Override public void service(HttpServletRequest request, HttpServletResponse response, Repository repository) throws ServletException, IOException - { + { String repoPath = repository.getNamespace() + "/" + repository.getName(); logger.trace("handle git repository at {}", repoPath); if (isLfsBatchApiRequest(request, repoPath)) { @@ -123,17 +113,22 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet } else if (isRegularGitAPIRequest(request)) { logger.trace("handle regular git request"); // continue with the regular git Backend - handleRegularGitRequest(request, response, repository); + try { + handleRegularGitRequest(request, response, repository); + gitHookEventFacade.firePending(); + } finally { + gitHookEventFacade.clean(); + } } else { logger.trace("handle browser request"); handleBrowserRequest(request, response, repository); } } - + private boolean isRegularGitAPIRequest(HttpServletRequest request) { return REGEX_GITHTTPBACKEND.matcher(HttpUtil.getStrippedURI(request)).matches(); } - + private void handleGitLfsRequest(HttpServlet servlet, HttpServletRequest request, HttpServletResponse response, Repository repository) throws ServletException, IOException { if (repositoryRequestListenerUtil.callListeners(request, response, repository)) { servlet.service(request, response); @@ -141,7 +136,7 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet logger.debug("request aborted by repository request listener"); } } - + private void handleRegularGitRequest(HttpServletRequest request, HttpServletResponse response, Repository repository) throws ServletException, IOException { if (repositoryRequestListenerUtil.callListeners(request, response, repository)) { super.service(request, response); @@ -149,7 +144,7 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet logger.debug("request aborted by repository request listener"); } } - + /** * This method renders basic information about the repository into the response. The result is meant to be viewed by @@ -245,4 +240,6 @@ public class ScmGitServlet extends GitServlet implements ScmProviderHttpServlet private final GitRepositoryViewer repositoryViewer; private final LfsServletFactory lfsServletFactory; + + private final GitHookEventFacade gitHookEventFacade; } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/BindTransportProtocolRule.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/BindTransportProtocolRule.java index f859efecae..d949267e69 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/BindTransportProtocolRule.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/BindTransportProtocolRule.java @@ -27,27 +27,29 @@ package sonia.scm.repository.spi; import org.eclipse.jgit.transport.ScmTransportProtocol; import org.eclipse.jgit.transport.Transport; import org.junit.rules.ExternalResource; -import sonia.scm.repository.GitChangesetConverterFactory; import sonia.scm.repository.GitRepositoryHandler; import sonia.scm.repository.GitTestHelper; import sonia.scm.repository.PreProcessorUtil; import sonia.scm.repository.RepositoryManager; import sonia.scm.repository.api.HookContextFactory; +import sonia.scm.web.GitHookEventFacade; import static com.google.inject.util.Providers.of; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class BindTransportProtocolRule extends ExternalResource { +class BindTransportProtocolRule extends ExternalResource { private ScmTransportProtocol scmTransportProtocol; + RepositoryManager repositoryManager = mock(RepositoryManager.class); + GitHookEventFacade hookEventFacade; + @Override protected void before() { HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class)); - RepositoryManager repositoryManager = mock(RepositoryManager.class); - HookEventFacade hookEventFacade = new HookEventFacade(of(repositoryManager), hookContextFactory); + hookEventFacade = new GitHookEventFacade(new HookEventFacade(of(repositoryManager), hookContextFactory)); GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class); scmTransportProtocol = new ScmTransportProtocol(of(GitTestHelper.createConverterFactory()), of(hookEventFacade), of(gitRepositoryHandler)); @@ -60,5 +62,6 @@ public class BindTransportProtocolRule extends ExternalResource { @Override protected void after() { Transport.unregister(scmTransportProtocol); + hookEventFacade.close(); } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java index e4e09c5bd5..8048308783 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java @@ -36,12 +36,15 @@ import sonia.scm.ConcurrentModificationException; import sonia.scm.NotFoundException; import sonia.scm.repository.GitTestHelper; import sonia.scm.repository.Person; +import sonia.scm.repository.RepositoryHookType; import java.io.File; import java.io.IOException; import java.nio.file.Files; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.Mockito.verify; public class GitModifyCommandTest extends GitModifyCommandTestBase { @@ -327,4 +330,21 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase { assertThat(lastCommit.getRawGpgSignature()).isNullOrEmpty(); } } + + @Test + public void shouldTriggerPostCommitHook() throws IOException { + File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile(); + + GitModifyCommand command = createCommand(); + + ModifyCommandRequest request = new ModifyCommandRequest(); + request.setCommitMessage("test commit"); + request.addRequest(new ModifyCommandRequest.CreateFileRequest("new_file", newFile, false)); + request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det")); + + command.execute(request); + + verify(transportProtocolRule.repositoryManager).fireHookEvent(argThat(argument -> argument.getType() == RepositoryHookType.PRE_RECEIVE)); + verify(transportProtocolRule.repositoryManager).fireHookEvent(argThat(argument -> argument.getType() == RepositoryHookType.POST_RECEIVE)); + } } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkingCopyFactoryTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkingCopyFactoryTest.java index 93c47a7474..8f2a47796f 100644 --- a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkingCopyFactoryTest.java +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/SimpleGitWorkingCopyFactoryTest.java @@ -30,6 +30,7 @@ import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.ScmTransportProtocol; import org.eclipse.jgit.transport.Transport; import org.eclipse.jgit.transport.URIish; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -42,6 +43,7 @@ import sonia.scm.repository.api.HookContextFactory; import sonia.scm.repository.work.NoneCachingWorkingCopyPool; import sonia.scm.repository.work.WorkdirProvider; import sonia.scm.repository.work.WorkingCopy; +import sonia.scm.web.GitHookEventFacade; import java.io.File; import java.io.IOException; @@ -60,17 +62,23 @@ public class SimpleGitWorkingCopyFactoryTest extends AbstractGitCommandTestBase // keep this so that it will not be garbage collected (Transport keeps this in a week reference) private ScmTransportProtocol proto; private WorkdirProvider workdirProvider; + GitHookEventFacade hookEventFacade; @Before public void bindScmProtocol() throws IOException { HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class)); - HookEventFacade hookEventFacade = new HookEventFacade(of(mock(RepositoryManager.class)), hookContextFactory); + hookEventFacade = new GitHookEventFacade(new HookEventFacade(of(mock(RepositoryManager.class)), hookContextFactory)); GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class); proto = new ScmTransportProtocol(of(GitTestHelper.createConverterFactory()), of(hookEventFacade), of(gitRepositoryHandler)); Transport.register(proto); workdirProvider = new WorkdirProvider(temporaryFolder.newFolder(), repositoryLocationResolver, false); } + @After + public void close() { + hookEventFacade.close(); + } + @Test public void emptyPoolShouldCreateNewWorkdir() { SimpleGitWorkingCopyFactory factory = new SimpleGitWorkingCopyFactory(new NoneCachingWorkingCopyPool(workdirProvider));