Fix hook events for git mirror command (#1703)

Currently the mirror command implementation for git fires post receive repository hook events, that return every changeset of the repository instead of those really added by the single mirror update.

This fixes this issue by first creating a working copy, running the fetch and the update in this clone, and then pushing back the result to the central repository. This triggers the internal mechanisms used in other commands like the modify command.

The downside of this is, that we first have to create the clone, so for big mirrors a cached working copy is strongly recommended.

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/protocolcommand/git/MirrorRefFilterTest.java

@@ -1,51 +0,0 @@
-/*
- * 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.protocolcommand.git;
-
-import org.eclipse.jgit.lib.Ref;
-import org.junit.jupiter.api.Test;
-
-import java.util.HashMap;
-import java.util.Map;
-
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.entry;
-import static org.mockito.Mockito.mock;
-
-class MirrorRefFilterTest {
-
-  @Test
-  void shouldRemoveMirrorRefs() {
-    Map<String, Ref> refs = new HashMap<>();
-    Ref master = mock(Ref.class);
-    Ref mirror = mock(Ref.class);
-    refs.put("refs/heads/master", master);
-    refs.put("refs/mirror/some/other", mirror);
-
-    Map<String, Ref> filteredRefs = MirrorRefFilter.filterMirrors(refs);
-
-    assertThat(filteredRefs).containsOnly(entry("refs/heads/master", master));
-  }
-}

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMirrorCommandTest.java

@@ -24,6 +24,7 @@
 
 package sonia.scm.repository.spi;
 
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.junit.http.AppServer;
@@ -33,6 +34,7 @@ import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.revwalk.RevObject;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import sonia.scm.api.v2.resources.GitRepositoryConfigStoreProvider;
 import sonia.scm.repository.GitChangesetConverterFactory;
@@ -40,6 +42,8 @@ import sonia.scm.repository.GitConfig;
 import sonia.scm.repository.api.MirrorCommandResult;
 import sonia.scm.repository.api.MirrorFilter;
 import sonia.scm.repository.api.SimpleUsernamePasswordCredential;
+import sonia.scm.repository.work.NoneCachingWorkingCopyPool;
+import sonia.scm.repository.work.WorkdirProvider;
 import sonia.scm.security.GPG;
 import sonia.scm.store.InMemoryConfigurationStoreFactory;
 
@@ -52,10 +56,7 @@ import java.util.function.Consumer;
 
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
-import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static sonia.scm.repository.api.MirrorCommandResult.ResultType.FAILED;
 import static sonia.scm.repository.api.MirrorCommandResult.ResultType.OK;
@@ -63,10 +64,12 @@ import static sonia.scm.repository.api.MirrorCommandResult.ResultType.REJECTED_U
 
 public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
 
+  @Rule
+  public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
+
   public static final Consumer<MirrorCommandRequest> ACCEPT_ALL = r -> {
   };
   public static final Consumer<MirrorCommandRequest> REJECT_ALL = r -> r.setFilter(new DenyAllMirrorFilter());
-  private final PostReceiveRepositoryHookEventFactory postReceiveRepositoryHookEventFactory = mock(PostReceiveRepositoryHookEventFactory.class);
   private final MirrorHttpConnectionProvider mirrorHttpConnectionProvider = mock(MirrorHttpConnectionProvider.class);
   private final GPG gpg = mock(GPG.class);
   private final GitChangesetConverterFactory gitChangesetConverterFactory = new GitChangesetConverterFactory(gpg);
@@ -81,7 +84,15 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
     Git.init().setBare(true).setDirectory(clone).call();
 
     GitContext emptyContext = createMirrorContext(clone);
-    command = new GitMirrorCommand(emptyContext, postReceiveRepositoryHookEventFactory, mirrorHttpConnectionProvider, gitChangesetConverterFactory, gitTagConverter);
+    SimpleGitWorkingCopyFactory workingCopyFactory =
+      new SimpleGitWorkingCopyFactory(
+        new NoneCachingWorkingCopyPool(new WorkdirProvider(repositoryLocationResolver)), new SimpleMeterRegistry());
+    command = new GitMirrorCommand(
+      emptyContext,
+      mirrorHttpConnectionProvider,
+      gitChangesetConverterFactory,
+      gitTagConverter,
+      workingCopyFactory);
   }
 
   @Test
@@ -99,8 +110,6 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       assertThat(createdMirror.branchList().call()).isNotEmpty();
       assertThat(createdMirror.tagList().call()).isNotEmpty();
     }
-
-    verify(postReceiveRepositoryHookEventFactory).fireForFetch(any(), any());
   }
 
   @Test
@@ -133,9 +142,6 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       Optional<Ref> addedBranch = findBranch(updatedMirror, "added-branch");
       assertThat(addedBranch).isPresent();
     }
-
-    // event should be thrown two times, once for the initial clone, and once for the update
-    verify(postReceiveRepositoryHookEventFactory, times(2)).fireForFetch(any(), any());
   }
 
   @Test
@@ -203,9 +209,6 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
       Optional<Ref> addedTag = findTag(updatedMirror, "added-tag");
       assertThat(addedTag).hasValueSatisfying(ref -> assertThat(ref.getObjectId().getName()).isEqualTo("9e93d8631675a89615fac56b09209686146ff3c0"));
     }
-
-    // event should be thrown two times, once for the initial clone, and once for the update
-    verify(postReceiveRepositoryHookEventFactory, times(2)).fireForFetch(any(), any());
   }
 
   @Test
@@ -506,8 +509,6 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
           createCredential("wrong", "credentials"));
 
       assertThat(result.getResult()).isEqualTo(FAILED);
-
-      verify(postReceiveRepositoryHookEventFactory, never()).fireForFetch(any(), any());
     } finally {
       simpleHttpServer.stop();
     }
@@ -669,11 +670,13 @@ public class GitMirrorCommandTest extends AbstractGitCommandTestBase {
         return new Filter() {
           @Override
           public Result acceptTag(TagUpdate tagUpdate) {
+            tagUpdate.getTag();
             updates.tagUpdates.add(tagUpdate);
             return Result.accept();
           }
           @Override
           public Result acceptBranch(BranchUpdate branchUpdate) {
+            branchUpdate.getChangeset();
             updates.branchUpdates.add(branchUpdate);
             return Result.accept();
           }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTest.java

@@ -55,7 +55,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldCreateCommit() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -76,7 +76,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldCreateCommitOnSelectedBranch() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -97,7 +97,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldCreateNewFile() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -115,7 +115,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldCreateNewFileWhenPathStartsWithSlash() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -133,7 +133,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = AlreadyExistsException.class)
   public void shouldFailIfOverwritingExistingFileWithoutOverwriteFlag() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -147,7 +147,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = AlreadyExistsException.class)
   public void shouldFailIfPathAlreadyExistsAsAFile() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -161,7 +161,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldOverwriteExistingFileIfOverwriteFlagSet() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -179,7 +179,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldModifyExistingFile() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -197,7 +197,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = NotFoundException.class)
   public void shouldFailIfFileToModifyDoesNotExist() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -211,7 +211,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = BadRequestException.class)
   public void shouldFailIfNoChangesMade() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "b\n".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "b\n".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -225,7 +225,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = ConcurrentModificationException.class)
   public void shouldFailBranchDoesNotHaveExpectedRevision() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -281,7 +281,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = NotFoundException.class)
   public void shouldFailWithNotFoundExceptionIfBranchIsNoBranch() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "irrelevant\n".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -296,7 +296,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldSignCreatedCommit() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -317,7 +317,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldNotSignCreatedCommitIfSigningDisabled() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -338,7 +338,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test
   public void shouldTriggerPostCommitHook() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -359,7 +359,7 @@ public class GitModifyCommandTest extends GitModifyCommandTestBase {
 
   @Test(expected = ScmConstraintViolationException.class)
   public void shouldFailIfPathInGitMetadata() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "other".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "other".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommandTestBase.java

@@ -52,8 +52,6 @@ import static sonia.scm.repository.spi.GitRepositoryConfigStoreProviderTestUtil.
 @SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini", username = "admin", password = "secret")
 class GitModifyCommandTestBase extends AbstractGitCommandTestBase {
 
-  @Rule
-  public TemporaryFolder temporaryFolder = new TemporaryFolder();
   @Rule
   public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
   @Rule

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_LFSTest.java

@@ -97,7 +97,7 @@ public class GitModifyCommand_LFSTest extends GitModifyCommandTestBase {
     when(blob.getOutputStream()).thenReturn(outputStream);
     when(blob.getSize()).thenReturn((long) content.length());
 
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), content.getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), content.getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitModifyCommand_withEmptyRepositoryTest.java

@@ -43,7 +43,7 @@ public class GitModifyCommand_withEmptyRepositoryTest extends GitModifyCommandTe
 
   @Test
   public void shouldCreateNewFileInEmptyRepository() throws IOException, GitAPIException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     GitModifyCommand command = createCommand();
 
@@ -124,7 +124,7 @@ public class GitModifyCommand_withEmptyRepositoryTest extends GitModifyCommandTe
   }
 
   private ModifyCommandRequest createRequest() throws IOException {
-    File newFile = Files.write(temporaryFolder.newFile().toPath(), "new content".getBytes()).toFile();
+    File newFile = Files.write(tempFolder.newFile().toPath(), "new content".getBytes()).toFile();
 
     ModifyCommandRequest request = new ModifyCommandRequest();
     request.setCommitMessage("initial commit");