improve git hook handling

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

@@ -0,0 +1,237 @@
+/**
+ * Copyright (c) 2010, Sebastian Sdorra
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice,
+ *    this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ * 3. Neither the name of SCM-Manager; nor the names of its
+ *    contributors may be used to endorse or promote products derived from this
+ *    software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * http://bitbucket.org/sdorra/scm-manager
+ *
+ */
+
+
+
+package sonia.scm.repository;
+
+//~--- non-JDK imports --------------------------------------------------------
+
+import com.google.common.collect.Lists;
+
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevSort;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.eclipse.jgit.transport.ReceivePack;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import sonia.scm.util.IOUtil;
+
+//~--- JDK imports ------------------------------------------------------------
+
+import java.util.List;
+import java.util.Map;
+
+/**
+ *
+ * @author Sebastian Sdorra
+ */
+public class GitHookChangesetCollector
+{
+
+  /**
+   * the logger for GitHookChangesetCollector
+   */
+  private static final Logger logger =
+    LoggerFactory.getLogger(GitHookChangesetCollector.class);
+
+  //~--- constructors ---------------------------------------------------------
+
+  /**
+   * Constructs ...
+   *
+   *
+   * @param rpack
+   * @param receiveCommands
+   */
+  public GitHookChangesetCollector(ReceivePack rpack,
+    List<ReceiveCommand> receiveCommands)
+  {
+    this.rpack = rpack;
+    this.receiveCommands = receiveCommands;
+  }
+
+  //~--- methods --------------------------------------------------------------
+
+  /**
+   * Method description
+   *
+   *
+   * @return
+   */
+  public List<Changeset> collectChangesets()
+  {
+    List<Changeset> changesets = Lists.newArrayList();
+
+    org.eclipse.jgit.lib.Repository repository = rpack.getRepository();
+
+    GitChangesetConverter converter = null;
+    RevWalk walk = null;
+
+    try
+    {
+      converter = new GitChangesetConverter(repository, GitUtil.ID_LENGTH);
+      walk = rpack.getRevWalk();
+
+      for (ReceiveCommand rc : receiveCommands)
+      {
+        //J-
+         logger.trace("handle receive command, type={}, ref={}, result={}",
+           new Object[] {
+             rc.getType(),
+             rc.getRefName(),
+             rc.getResult()
+           }
+         );
+         //J+
+
+        ObjectId newId = rc.getNewId();
+
+        String branch = GitUtil.getBranch(rc.getRefName());
+
+        walk.reset();
+        walk.sort(RevSort.TOPO);
+        walk.sort(RevSort.REVERSE, true);
+
+        if (logger.isTraceEnabled())
+        {
+          logger.trace("mark {} as start for rev walk", newId.getName());
+        }
+
+        walk.markStart(walk.parseCommit(newId));
+
+        ObjectId oldId = rc.getOldId();
+
+        if ((oldId != null) &&!oldId.equals(ObjectId.zeroId()))
+        {
+          if (logger.isTraceEnabled())
+          {
+            logger.trace("mark {} as uninteresting for rev walk",
+              oldId.getName());
+          }
+
+          walk.markUninteresting(walk.parseCommit(oldId));
+        }
+
+        for (ObjectId id : getExistingObjects(rpack))
+        {
+          if (logger.isTraceEnabled())
+          {
+            logger.trace("mark {} as uninteresting for rev walk", id.getName());
+          }
+
+          walk.markUninteresting(walk.parseCommit(id));
+        }
+
+        RevCommit commit = walk.next();
+
+        while (commit != null)
+        {
+          Changeset changeset = converter.createChangeset(commit);
+
+          List<String> branches = changeset.getBranches();
+
+          if (branches.isEmpty())
+          {
+            if (logger.isTraceEnabled())
+            {
+              //J-
+                logger.trace(
+                  "missing branch informations for {}, set default branch {}",
+                  changeset.getId(),
+                  branch
+                );
+                //J+
+            }
+
+            branches.add(branch);
+          }
+
+          changesets.add(changeset);
+
+          commit = walk.next();
+        }
+
+      }
+
+    }
+    catch (Exception ex)
+    {
+      logger.error("could not collect changesets", ex);
+    }
+    finally
+    {
+      IOUtil.close(converter);
+    }
+
+    return changesets;
+  }
+
+  //~--- get methods ----------------------------------------------------------
+
+  /**
+   * Method description
+   *
+   *
+   * @param rpack
+   *
+   * @return
+   */
+  private List<ObjectId> getExistingObjects(ReceivePack rpack)
+  {
+    List<ObjectId> existingObjects = Lists.newArrayList();
+
+    if (existingObjects == null)
+    {
+      Map<String, Ref> refs = rpack.getRepository().getAllRefs();
+
+      for (Ref r : refs.values())
+      {
+        existingObjects.add(r.getObjectId());
+      }
+    }
+
+    return existingObjects;
+  }
+
+  //~--- fields ---------------------------------------------------------------
+
+  /** Field description */
+  private List<ReceiveCommand> receiveCommands;
+
+  /** Field description */
+  private ReceivePack rpack;
+}

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

@@ -35,25 +35,11 @@ package sonia.scm.repository;
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.collect.Lists;
+import org.eclipse.jgit.transport.ReceiveCommand;
-
+import org.eclipse.jgit.transport.ReceivePack;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevSort;
-import org.eclipse.jgit.revwalk.RevWalk;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import sonia.scm.util.IOUtil;
-import sonia.scm.util.Util;
 
 //~--- JDK imports ------------------------------------------------------------
 
-import java.io.File;
-import java.io.IOException;
-
-import java.util.Collection;
 import java.util.List;
 
 /**
@@ -63,46 +49,20 @@ import java.util.List;
 public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent
 {
 
-  /** the logger for GitRepositoryHookEvent */
-  private static final Logger logger =
-    LoggerFactory.getLogger(GitRepositoryHookEvent.class);
-
-  //~--- constructors ---------------------------------------------------------
-
   /**
    * Constructs ...
    *
    *
-   * @param directory
+   * @param rpack
-   * @param ref
+   * @param receiveCommands
-   * @param refName
-   * @param newId
-   * @param oldId
    * @param type
    */
-  public GitRepositoryHookEvent(File directory, String refName, ObjectId newId,
+  public GitRepositoryHookEvent(ReceivePack rpack,
-    ObjectId oldId, RepositoryHookType type)
+    List<ReceiveCommand> receiveCommands, RepositoryHookType type)
   {
-    this.directory = directory;
+    this.rpack = rpack;
-    this.branch = GitUtil.getBranch(refName);
+    this.receiveCommands = receiveCommands;
-    this.newId = newId;
-    this.oldId = oldId;
     this.type = type;
-
-    if (logger.isTraceEnabled())
-    {
-      //J-
-      logger.trace(
-        "create hook event for branch={}, new-id={}, old-id={} and type={}",
-        new Object[] { 
-          refName,
-          GitUtil.getId(newId),
-          GitUtil.getId(oldId), 
-          type 
-        }
-      );
-      //J+
-    }
   }
 
   //~--- get methods ----------------------------------------------------------
@@ -114,11 +74,14 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent
    * @return
    */
   @Override
-  public Collection<Changeset> getChangesets()
+  public List<Changeset> getChangesets()
   {
     if (changesets == null)
     {
-      changesets = fetchChangesets();
+      GitHookChangesetCollector collector =
+        new GitHookChangesetCollector(rpack, receiveCommands);
+
+      changesets = collector.collectChangesets();
     }
 
     return changesets;
@@ -136,86 +99,16 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent
     return type;
   }
 
-  //~--- methods --------------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @return
-   */
-  private List<Changeset> fetchChangesets()
-  {
-    List<Changeset> result = Lists.newArrayList();
-    GitChangesetConverter converter = null;
-    RevWalk walk = null;
-    org.eclipse.jgit.lib.Repository repository = null;
-
-    try
-    {
-      repository = GitUtil.open(directory);
-      converter = new GitChangesetConverter(repository, GitUtil.ID_LENGTH);
-      walk = new RevWalk(repository);
-      walk.reset();
-      walk.sort(RevSort.NONE);
-      walk.markStart(walk.parseCommit(newId));
-
-      if (oldId != null)
-      {
-        walk.markUninteresting(walk.parseCommit(oldId));
-      }
-
-      RevCommit commit = walk.next();
-
-      while (commit != null)
-      {
-        Changeset changeset = converter.createChangeset(commit);
-
-        if (changeset.getBranches().isEmpty() && Util.isNotEmpty(branch))
-        {
-          if (logger.isTraceEnabled())
-          {
-            logger.trace("set branch to current default branch {}", branch);
-          }
-
-          changeset.getBranches().add(branch);
-        }
-
-        result.add(changeset);
-        commit = walk.next();
-      }
-    }
-    catch (IOException ex)
-    {
-      logger.error("could not fetch changesets", ex);
-    }
-    finally
-    {
-      IOUtil.close(converter);
-      GitUtil.release(walk);
-      GitUtil.close(repository);
-
-    }
-
-    return result;
-  }
-
   //~--- fields ---------------------------------------------------------------
 
-  /** Field description */
-  private String branch;
-
   /** Field description */
   private List<Changeset> changesets;
 
   /** Field description */
-  private File directory;
+  private List<ReceiveCommand> receiveCommands;
 
   /** Field description */
-  private ObjectId newId;
+  private ReceivePack rpack;
-
-  /** Field description */
-  private ObjectId oldId;
 
   /** Field description */
   private RepositoryHookType type;

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitReceiveHook.java

@@ -35,7 +35,10 @@ package sonia.scm.web;
 
 //~--- non-JDK imports --------------------------------------------------------
 
+import com.google.common.collect.Lists;
+
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.PostReceiveHook;
 import org.eclipse.jgit.transport.PreReceiveHook;
 import org.eclipse.jgit.transport.ReceiveCommand;
@@ -52,7 +55,6 @@ import sonia.scm.repository.GitRepositoryHookEvent;
 import sonia.scm.repository.GitUtil;
 import sonia.scm.repository.RepositoryHookType;
 import sonia.scm.repository.RepositoryManager;
-import sonia.scm.repository.RepositoryNotFoundException;
 import sonia.scm.repository.RepositoryUtil;
 import sonia.scm.util.IOUtil;
 import sonia.scm.util.Util;
@@ -63,6 +65,8 @@ import java.io.File;
 import java.io.IOException;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
 
 /**
  *
@@ -205,104 +209,6 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook
     }
   }
 
-  /**
-   * Method description, occurred
-   *
-   * @param rpack
-   * @param rc
-   * @param directory
-   * @param oldId
-   * @param newId
-   * @param type
-   */
-  private void fireHookEvent(ReceivePack rpack, ReceiveCommand rc,
-    File directory, ObjectId oldId, ObjectId newId, RepositoryHookType type)
-  {
-    try
-    {
-      String repositoryName = RepositoryUtil.getRepositoryName(handler,
-                                directory);
-      GitRepositoryHookEvent e = new GitRepositoryHookEvent(directory,
-                                   rc.getRefName(), newId, oldId, type);
-
-      repositoryManager.fireHookEvent(GitRepositoryHandler.TYPE_NAME,
-        repositoryName, e);
-    }
-    catch (RepositoryNotFoundException ex)
-    {
-      logger.error("repository could not be found", ex);
-    }
-    catch (Exception ex)
-    {
-      if (logger.isWarnEnabled())
-      {
-        logger.warn("execption occurred during hook execution", ex);
-      }
-
-      sendError(rpack, rc, ex.getMessage());
-    }
-  }
-
-  /**
-   * Method description
-   *
-   *
-   * @param rpack
-   * @param receiveCommands
-   * @param type
-   */
-  private void onReceive(ReceivePack rpack,
-    Collection<ReceiveCommand> receiveCommands, RepositoryHookType type)
-  {
-    if (logger.isTraceEnabled())
-    {
-      logger.trace("received git hook, type={}", type);
-    }
-
-    for (ReceiveCommand rc : receiveCommands)
-    {
-      if (isReceiveable(rc, type))
-      {
-        if (logger.isTraceEnabled())
-        {
-          //J-
-          logger.trace("handle receive command, type={}, ref={}, result={}",
-            new Object[] { 
-              rc.getType(),
-              rc.getRefName(), 
-              rc.getResult() 
-            }
-          );
-          //J+
-        }
-
-        ObjectId newId = rc.getNewId();
-
-        if (newId != null)
-        {
-          onReceive(rpack, rc, newId, type);
-        }
-        else if (logger.isWarnEnabled())
-        {
-          logger.warn("received hook event without new id");
-        }
-
-      }
-      else if (logger.isTraceEnabled())
-      {
-        //J-
-        logger.trace("skip receive command, type={}, ref={}, result={}",
-          new Object[] { 
-            rc.getType(),
-            rc.getRefName(), 
-            rc.getResult() 
-          }
-        );
-        //J+
-      }
-    }
-  }
-
   /**
    * Method description
    *
@@ -312,9 +218,10 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook
    * @param newId
    * @param type
    */
-  private void onReceive(ReceivePack rpack, ReceiveCommand rc, ObjectId newId,
+  private void handleFileHooks(ReceivePack rpack, ReceiveCommand rc,
     RepositoryHookType type)
   {
+    ObjectId newId = rc.getNewId();
     ObjectId oldId = null;
 
     if (isUpdateCommand(rc))
@@ -354,8 +261,82 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook
           rc.getRefName());
       }
     }
+  }
 
-    fireHookEvent(rpack, rc, directory, oldId, newId, type);
+  /**
+   * Method description
+   *
+   *
+   * @param rpack
+   * @param receiveCommands
+   * @param type
+   */
+  private void handleReceiveCommands(ReceivePack rpack,
+    List<ReceiveCommand> receiveCommands, RepositoryHookType type)
+  {
+    try
+    {
+      Repository repository = rpack.getRepository();
+      String repositoryName = RepositoryUtil.getRepositoryName(handler,
+                                repository.getDirectory());
+
+      repositoryManager.fireHookEvent(GitRepositoryHandler.TYPE_NAME,
+        repositoryName,
+        new GitRepositoryHookEvent(rpack, receiveCommands, type));
+    }
+    catch (Exception ex)
+    {
+      logger.error("could not handle receive commands", ex);
+    }
+  }
+
+  /**
+   * Method description
+   *
+   *
+   * @param rpack
+   * @param receiveCommands
+   * @param type
+   */
+  private void onReceive(ReceivePack rpack,
+    Collection<ReceiveCommand> receiveCommands, RepositoryHookType type)
+  {
+    if (logger.isTraceEnabled())
+    {
+      logger.trace("received git hook, type={}", type);
+    }
+
+    List<ReceiveCommand> commands = Lists.newArrayList();
+
+    for (ReceiveCommand rc : receiveCommands)
+    {
+      if (isReceiveable(rc, type))
+      {
+        commands.add(rc);
+        handleFileHooks(rpack, rc, type);
+      }
+      else if (logger.isTraceEnabled())
+      {
+        //J-
+        logger.trace("skip receive command, type={}, ref={}, result={}",
+          new Object[] { 
+            rc.getType(),
+            rc.getRefName(), 
+            rc.getResult() 
+          }
+        );
+        //J+
+      }
+    }
+
+    if (!commands.isEmpty())
+    {
+      handleReceiveCommands(rpack, commands, type);
+    }
+    else if (logger.isDebugEnabled())
+    {
+      logger.debug("no receive command found to process");
+    }
   }
 
   /**