merge with 2.0.0-m3

scm-plugins/scm-git-plugin/src/main/java/org/eclipse/jgit/transport/ScmTransportProtocol.java

@@ -48,6 +48,7 @@ import org.eclipse.jgit.lib.RepositoryCache;
 
 import sonia.scm.repository.GitRepositoryHandler;
 import sonia.scm.repository.spi.HookEventFacade;
+import sonia.scm.web.CollectingPackParserListener;
 import sonia.scm.web.GitReceiveHook;
 
 //~--- JDK imports ------------------------------------------------------------
@@ -64,10 +65,10 @@ public class ScmTransportProtocol extends TransportProtocol
 {
 
   /** Field description */
-  private static final String NAME = "scm";
+  public static final String NAME = "scm";
 
   /** Field description */
-  private static final Set<String> SCHEMES = ImmutableSet.of("scm");
+  private static final Set<String> SCHEMES = ImmutableSet.of(NAME);
 
   //~--- constructors ---------------------------------------------------------
 
@@ -234,6 +235,8 @@ public class ScmTransportProtocol extends TransportProtocol
 
         pack.setPreReceiveHook(hook);
         pack.setPostReceiveHook(hook);
+
+        CollectingPackParserListener.set(pack);
       }
 
       return pack;

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

@@ -4,8 +4,10 @@ import com.google.common.base.Strings;
 import org.apache.shiro.SecurityUtils;
 import org.apache.shiro.subject.Subject;
 import org.eclipse.jgit.api.Git;
+import org.eclipse.jgit.api.MergeCommand.FastForwardMode;
 import org.eclipse.jgit.api.MergeResult;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.api.errors.RefNotFoundException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.merge.MergeStrategy;
@@ -15,6 +17,7 @@ import org.slf4j.LoggerFactory;
 import sonia.scm.repository.GitWorkdirFactory;
 import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.Person;
+import sonia.scm.repository.RepositoryPermissions;
 import sonia.scm.repository.api.MergeCommandResult;
 import sonia.scm.repository.api.MergeDryRunCommandResult;
 import sonia.scm.user.User;
@@ -22,6 +25,9 @@ import sonia.scm.user.User;
 import java.io.IOException;
 import java.text.MessageFormat;
 
+import static sonia.scm.ContextEntry.ContextBuilder.entity;
+import static sonia.scm.NotFoundException.notFound;
+
 public class GitMergeCommand extends AbstractGitCommand implements MergeCommand {
 
   private static final Logger logger = LoggerFactory.getLogger(GitMergeCommand.class);
@@ -40,6 +46,8 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
 
   @Override
   public MergeCommandResult merge(MergeCommandRequest request) {
+    RepositoryPermissions.push(context.getRepository().getId()).check();
+
     try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context)) {
       Repository repository = workingCopy.get();
       logger.debug("cloned repository to folder {}", repository.getWorkTree());
@@ -88,20 +96,43 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       }
     }
 
-    private void checkOutTargetBranch() {
+    private void checkOutTargetBranch() throws IOException {
       try {
         clone.checkout().setName(target).call();
+      } catch (RefNotFoundException e) {
+        logger.trace("could not checkout target branch {} for merge directly; trying to create local branch", target, e);
+        checkOutTargetAsNewLocalBranch();
       } catch (GitAPIException e) {
         throw new InternalRepositoryException(context.getRepository(), "could not checkout target branch for merge: " + target, e);
       }
     }
 
+    private void checkOutTargetAsNewLocalBranch() throws IOException {
+      try {
+        ObjectId targetRevision = resolveRevision(target);
+        if (targetRevision == null) {
+          throw notFound(entity("revision", target).in(context.getRepository()));
+        }
+        clone.checkout().setStartPoint(targetRevision.getName()).setName(target).setCreateBranch(true).call();
+      } catch (RefNotFoundException e) {
+        logger.debug("could not checkout target branch {} for merge as local branch", target, e);
+        throw notFound(entity("revision", target).in(context.getRepository()));
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not checkout target branch for merge as local branch: " + target, e);
+      }
+    }
+
     private MergeResult doMergeInClone() throws IOException {
       MergeResult result;
       try {
+        ObjectId sourceRevision = resolveRevision(toMerge);
+        if (sourceRevision == null) {
+          throw notFound(entity("revision", toMerge).in(context.getRepository()));
+        }
         result = clone.merge()
+          .setFastForward(FastForwardMode.NO_FF)
           .setCommit(false) // we want to set the author manually
-          .include(toMerge, resolveRevision(toMerge))
+          .include(toMerge, sourceRevision)
           .call();
       } catch (GitAPIException e) {
         throw new InternalRepositoryException(context.getRepository(), "could not merge branch " + toMerge + " into " + target, e);
@@ -113,10 +144,12 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       logger.debug("merged branch {} into {}", toMerge, target);
       Person authorToUse = determineAuthor();
       try {
-        clone.commit()
-          .setAuthor(authorToUse.getName(), authorToUse.getMail())
-          .setMessage(MessageFormat.format(determineMessageTemplate(), toMerge, target))
-          .call();
+        if (!clone.status().call().isClean()) {
+          clone.commit()
+            .setAuthor(authorToUse.getName(), authorToUse.getMail())
+            .setMessage(MessageFormat.format(determineMessageTemplate(), toMerge, target))
+            .call();
+        }
       } catch (GitAPIException e) {
         throw new InternalRepositoryException(context.getRepository(), "could not commit merge between branch " + toMerge + " and " + target, e);
       }
@@ -147,7 +180,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       try {
         clone.push().call();
       } catch (GitAPIException e) {
-        throw new InternalRepositoryException(context.getRepository(), "could not push merged branch " + toMerge + " to origin", e);
+        throw new InternalRepositoryException(context.getRepository(), "could not push merged branch " + target + " to origin", e);
       }
       logger.debug("pushed merged branch {}", target);
     }

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

@@ -3,6 +3,7 @@ package sonia.scm.repository.spi;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ScmTransportProtocol;
 import org.eclipse.jgit.util.FileUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -45,12 +46,16 @@ public class SimpleGitWorkdirFactory implements GitWorkdirFactory {
 
   protected Repository cloneRepository(File bareRepository, File target) throws GitAPIException {
     return Git.cloneRepository()
-      .setURI(bareRepository.getAbsolutePath())
+      .setURI(createScmTransportProtocolUri(bareRepository))
       .setDirectory(target)
       .call()
       .getRepository();
   }
 
+  private String createScmTransportProtocolUri(File bareRepository) {
+    return ScmTransportProtocol.NAME + "://" + bareRepository.getAbsolutePath();
+  }
+
   private void close(Repository repository) {
     repository.close();
     try {

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

@@ -6,20 +6,33 @@ import org.apache.shiro.subject.SimplePrincipalCollection;
 import org.apache.shiro.subject.Subject;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.transport.ScmTransportProtocol;
+import org.eclipse.jgit.transport.Transport;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import sonia.scm.repository.GitRepositoryHandler;
 import sonia.scm.repository.Person;
+import sonia.scm.repository.PreProcessorUtil;
+import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.api.HookContextFactory;
 import sonia.scm.repository.api.MergeCommandResult;
 import sonia.scm.user.User;
 
 import java.io.IOException;
 
+import static com.google.inject.util.Providers.of;
 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.when;
 
-@SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini")
+@SubjectAware(configuration = "classpath:sonia/scm/configuration/shiro.ini", username = "admin", password = "secret")
 public class GitMergeCommandTest extends AbstractGitCommandTestBase {
 
   private static final String REALM = "AdminRealm";
@@ -27,6 +40,27 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase {
   @Rule
   public ShiroRule shiro = new ShiroRule();
 
+  private ScmTransportProtocol scmTransportProtocol;
+
+  @Before
+  public void bindScmProtocol() {
+    HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class));
+    RepositoryManager repositoryManager = mock(RepositoryManager.class);
+    HookEventFacade hookEventFacade = new HookEventFacade(of(repositoryManager), hookContextFactory);
+    GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class);
+    scmTransportProtocol = new ScmTransportProtocol(of(hookEventFacade), of(gitRepositoryHandler));
+
+    Transport.register(scmTransportProtocol);
+
+    when(gitRepositoryHandler.getRepositoryId(any())).thenReturn("1");
+    when(repositoryManager.get("1")).thenReturn(new sonia.scm.repository.Repository());
+  }
+
+  @After
+  public void unregisterScmProtocol() {
+    Transport.unregister(scmTransportProtocol);
+  }
+
   @Test
   public void shouldDetectMergeableBranches() {
     GitMergeCommand command = createCommand();
@@ -77,6 +111,30 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase {
     assertThat(new String(contentOfFileB)).isEqualTo("b\ncontent from branch\n");
   }
 
+  @Test
+  public void shouldNotMergeTwice() throws IOException, GitAPIException {
+    GitMergeCommand command = createCommand();
+    MergeCommandRequest request = new MergeCommandRequest();
+    request.setTargetBranch("master");
+    request.setBranchToMerge("mergeable");
+    request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
+
+    MergeCommandResult mergeCommandResult = command.merge(request);
+
+    assertThat(mergeCommandResult.isSuccess()).isTrue();
+
+    Repository repository = createContext().open();
+    ObjectId firstMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId();
+
+    MergeCommandResult secondMergeCommandResult = command.merge(request);
+
+    assertThat(secondMergeCommandResult.isSuccess()).isTrue();
+
+    ObjectId secondMergeCommit = new Git(repository).log().add(repository.resolve("master")).setMaxCount(1).call().iterator().next().getId();
+
+    assertThat(secondMergeCommit).isEqualTo(firstMergeCommit);
+  }
+
   @Test
   public void shouldUseConfiguredCommitMessageTemplate() throws IOException, GitAPIException {
     GitMergeCommand command = createCommand();
@@ -111,11 +169,14 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase {
   }
 
   @Test
-  @SubjectAware(username = "admin", password = "secret")
   public void shouldTakeAuthorFromSubjectIfNotSet() throws IOException, GitAPIException {
+    SimplePrincipalCollection principals = new SimplePrincipalCollection();
+    principals.add("admin", REALM);
+    principals.add( new User("dirk", "Dirk Gently", "dirk@holistic.det"), REALM);
     shiro.setSubject(
       new Subject.Builder()
-        .principals(new SimplePrincipalCollection(new User("dirk", "Dirk Gently", "dirk@holistic.det"), REALM))
+        .principals(principals)
+        .authenticated(true)
         .buildSubject());
     GitMergeCommand command = createCommand();
     MergeCommandRequest request = new MergeCommandRequest();
@@ -133,6 +194,32 @@ public class GitMergeCommandTest extends AbstractGitCommandTestBase {
     assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det");
   }
 
+  @Test
+  public void shouldMergeIntoNotDefaultBranch() throws IOException, GitAPIException {
+    GitMergeCommand command = createCommand();
+    MergeCommandRequest request = new MergeCommandRequest();
+    request.setAuthor(new Person("Dirk Gently", "dirk@holistic.det"));
+    request.setTargetBranch("mergeable");
+    request.setBranchToMerge("master");
+
+    MergeCommandResult mergeCommandResult = command.merge(request);
+
+    Repository repository = createContext().open();
+    assertThat(mergeCommandResult.isSuccess()).isTrue();
+
+    Iterable<RevCommit> commits = new Git(repository).log().add(repository.resolve("mergeable")).setMaxCount(1).call();
+    RevCommit mergeCommit = commits.iterator().next();
+    PersonIdent mergeAuthor = mergeCommit.getAuthorIdent();
+    String message = mergeCommit.getFullMessage();
+    assertThat(mergeAuthor.getName()).isEqualTo("Dirk Gently");
+    assertThat(mergeAuthor.getEmailAddress()).isEqualTo("dirk@holistic.det");
+    assertThat(message).contains("master", "mergeable");
+    // We expect the merge result of file b.txt here by looking up the sha hash of its content.
+    // If the file is missing (aka not merged correctly) this will throw a MissingObjectException:
+    byte[] contentOfFileB = repository.open(repository.resolve("9513e9c76e73f3e562fd8e4c909d0607113c77c6")).getBytes();
+    assertThat(new String(contentOfFileB)).isEqualTo("b\ncontent from branch\n");
+  }
+
   private GitMergeCommand createCommand() {
     return new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory());
   }

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

@@ -2,14 +2,23 @@ package sonia.scm.repository.spi;
 
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ScmTransportProtocol;
+import org.eclipse.jgit.transport.Transport;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
+import sonia.scm.repository.GitRepositoryHandler;
+import sonia.scm.repository.PreProcessorUtil;
+import sonia.scm.repository.RepositoryManager;
+import sonia.scm.repository.api.HookContextFactory;
 
 import java.io.File;
 import java.io.IOException;
 
+import static com.google.inject.util.Providers.of;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 
@@ -18,6 +27,14 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase {
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
 
+  @Before
+  public void bindScmProtocol() {
+    HookContextFactory hookContextFactory = new HookContextFactory(mock(PreProcessorUtil.class));
+    HookEventFacade hookEventFacade = new HookEventFacade(of(mock(RepositoryManager.class)), hookContextFactory);
+    GitRepositoryHandler gitRepositoryHandler = mock(GitRepositoryHandler.class);
+    Transport.register(new ScmTransportProtocol(of(hookEventFacade), of(gitRepositoryHandler)));
+  }
+
   @Test
   public void emptyPoolShouldCreateNewWorkdir() throws IOException {
     SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder());