Merge branch 'main' into develop

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java

@@ -42,6 +42,8 @@ import org.eclipse.jgit.transport.RefSpec;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import sonia.scm.ConcurrentModificationException;
+import sonia.scm.ContextEntry;
 import sonia.scm.repository.GitWorkingCopyFactory;
 import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.Person;
@@ -62,6 +64,7 @@ import static java.util.Optional.of;
 import static java.util.stream.Collectors.toList;
 import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.NON_EXISTING;
 import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.OK;
+import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_NONFASTFORWARD;
 import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.UP_TO_DATE;
 import static sonia.scm.ContextEntry.ContextBuilder.entity;
 import static sonia.scm.NotFoundException.notFound;
@@ -247,13 +250,21 @@ class AbstractGitCommand {
     }
 
     void push(String... refSpecs) {
+      push(false, refSpecs);
+    }
+
+    void forcePush(String... refSpecs) {
+      push(true, refSpecs);
+    }
+
+    private void push(boolean force, String... refSpecs) {
       logger.trace("Pushing mirror result to repository {} with refspec '{}'", repository, refSpecs);
       try {
         Iterable<PushResult> pushResults =
           clone
             .push()
             .setRefSpecs(stream(refSpecs).map(RefSpec::new).collect(toList()))
-            .setForce(true)
+            .setForce(force)
             .call();
         Iterator<PushResult> pushResultIterator = pushResults.iterator();
         if (!pushResultIterator.hasNext()) {
@@ -269,8 +280,13 @@ class AbstractGitCommand {
           .filter(remoteRefUpdate -> !ACCEPTED_UPDATE_STATUS.contains(remoteRefUpdate.getStatus()))
           .findAny()
           .ifPresent(remoteRefUpdate -> {
-            logger.info("message for unexpected push result {} for remote {}: {}", remoteRefUpdate.getStatus(), remoteRefUpdate.getRemoteName(), pushResult.getMessages());
-            throw forMessage(repository, pushResult.getMessages());
+            if (remoteRefUpdate.getStatus() == REJECTED_NONFASTFORWARD) {
+              logger.debug("non fast-forward change detected; probably the remote {} has been changed during the modification: {}", remoteRefUpdate.getRemoteName(), pushResult.getMessages());
+              throw new ConcurrentModificationException(ContextEntry.ContextBuilder.entity("Branch", remoteRefUpdate.getRemoteName()).in(repository).build());
+            } else {
+              logger.info("message for unexpected push result {} for remote {}: {}", remoteRefUpdate.getStatus(), remoteRefUpdate.getRemoteName(), pushResult.getMessages());
+              throw forMessage(repository, pushResult.getMessages());
+            }
           });
       } catch (GitAPIException e) {
         throw new InternalRepositoryException(repository, "could not push changes into central repository", e);

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMirrorCommand.java

@@ -219,7 +219,7 @@ public class GitMirrorCommand extends AbstractGitCommand implements MirrorComman
       defaultBranchSelector.newDefaultBranch().ifPresent(this::setNewDefaultBranch);
 
       String[] pushRefSpecs = generatePushRefSpecs().toArray(new String[0]);
-      push(pushRefSpecs);
+      forcePush(pushRefSpecs);
       ResultType finalResult = lfsUpdateResult.hasFailures()? FAILED: result;
       return new MirrorCommandResult(finalResult, mirrorLog, stopwatch.stop().elapsed(), lfsUpdateResult);
     }

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

@@ -0,0 +1,132 @@
+/*
+ * 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.repository.spi;
+
+import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
+import org.awaitility.Awaitility;
+import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import sonia.scm.ConcurrentModificationException;
+import sonia.scm.repository.work.NoneCachingWorkingCopyPool;
+import sonia.scm.repository.work.WorkdirProvider;
+
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+
+public class AbstractGitCommandTest extends AbstractGitCommandTestBase {
+
+  @Rule
+  public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
+  @Rule
+  public TemporaryFolder cloneFolder = new TemporaryFolder();
+
+  private final CountDownLatch createdWorkingCopyLatch = new CountDownLatch(1);
+  private final CountDownLatch createdConflictingCommitLatch = new CountDownLatch(1);
+  private boolean gotConflict = false;
+
+  @Override
+  protected String getZippedRepositoryResource() {
+    return "sonia/scm/repository/spi/scm-git-spi-test.zip";
+  }
+
+  @Test
+  public void shouldNotOverwriteConcurrentCommit() throws GitAPIException, IOException, InterruptedException {
+    GitContext context = createContext();
+    SimpleGitCommand command = new SimpleGitCommand(context);
+    Repository repo = context.open();
+
+    new Thread(() -> {
+      try {
+        command.doSomething();
+      } catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+    }).start();
+
+    Git clone = Git
+      .cloneRepository()
+      .setURI(repo.getDirectory().toString())
+      .setDirectory(cloneFolder.newFolder())
+      .call();
+
+    // somehow we have to wait here, though we don't know why
+    Thread.sleep(1000);
+
+    createdWorkingCopyLatch.await();
+    createCommit(clone);
+    clone.push().call();
+    createdConflictingCommitLatch.countDown();
+
+    Awaitility.await().until(() -> gotConflict);
+  }
+
+  private class SimpleGitCommand extends AbstractGitCommand {
+
+    SimpleGitCommand(GitContext context) {
+      super(context);
+    }
+
+    void doSomething() {
+      inClone(
+        git -> new Worker(context, repository, git),
+        new SimpleGitWorkingCopyFactory(new NoneCachingWorkingCopyPool(new WorkdirProvider(repositoryLocationResolver)), new SimpleMeterRegistry()),
+        "master"
+      );
+    }
+  }
+
+  private synchronized void createCommit(Git git) throws GitAPIException {
+    git.commit().setMessage("Add new commit").call();
+  }
+
+  private class Worker extends AbstractGitCommand.GitCloneWorker<Void> {
+
+    private final Git git;
+
+    private Worker(GitContext context, sonia.scm.repository.Repository repository, Git git) {
+      super(git, context, repository);
+      this.git = git;
+    }
+
+    @Override
+    Void run() {
+      try {
+        createCommit(git);
+        createdWorkingCopyLatch.countDown();
+        createdConflictingCommitLatch.await();
+        push("master");
+      } catch (ConcurrentModificationException e) {
+        gotConflict = true;
+      } catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+      return null;
+    }
+  }
+}