From 81f90209f35e03e41484d61728c3ce9cdaf73c0d Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 16:07:23 +0200 Subject: [PATCH 01/11] indent and logging --- .../java/sonia/scm/web/GitReceiveHook.java | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) 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 e41495afe2..fed98e2d5b 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 @@ -249,11 +249,20 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook { for (ReceiveCommand rc : receiveCommands) { - if (((RepositoryHookType.PRE_RECEIVE == type) - && (rc.getResult() - == ReceiveCommand.Result.NOT_ATTEMPTED)) || ((RepositoryHookType - .POST_RECEIVE == type) && (rc.getResult() - == ReceiveCommand.Result.OK))) + if (logger.isTraceEnabled()) + { + //J- + logger.trace("receive command, type={}, ref={}, result={}", + new Object[] { + rc.getType(), + rc.getRefName(), + rc.getResult() + } + ); + //J+ + } + + if (isReceiveable(rc, type)) { ObjectId newId = rc.getNewId(); ObjectId oldId = null; @@ -345,6 +354,25 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook return id; } + /** + * Method description + * + * + * @param rc + * @param type + * + * @return + */ + private boolean isReceiveable(ReceiveCommand rc, RepositoryHookType type) + { + //J- + return ((RepositoryHookType.PRE_RECEIVE == type) && + (rc.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED)) || + ((RepositoryHookType.POST_RECEIVE == type) && + (rc.getResult() == ReceiveCommand.Result.OK)); + //J+ + } + /** * Method description * From 7baede3eb16e35a5ebdb5026546fe2ce9ffa41d2 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 17:00:44 +0200 Subject: [PATCH 02/11] improve trace logging for git repository hooks --- .../java/sonia/scm/web/GitReceiveHook.java | 57 ++++++++++++++----- 1 file changed, 44 insertions(+), 13 deletions(-) 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 fed98e2d5b..60d8aa99af 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 @@ -148,6 +148,11 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook File repositoryDirectory, File hook, ObjectId oldId, ObjectId newId, String refName) { + if (logger.isDebugEnabled()) + { + logger.debug("execute file hook '{}' in directoy '{}'"); + } + final Command cmd = new SimpleCommand(hook.getAbsolutePath(), getId(oldId), getId(newId), Util.nonNull(refName)); @@ -247,29 +252,43 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook private void onReceive(ReceivePack rpack, Collection receiveCommands, RepositoryHookType type) { + if ( logger.isTraceEnabled() ) + { + logger.trace("received git hook, type={}", type); + } for (ReceiveCommand rc : receiveCommands) { - if (logger.isTraceEnabled()) - { - //J- - logger.trace("receive command, type={}, ref={}, result={}", - new Object[] { - rc.getType(), - rc.getRefName(), - rc.getResult() - } - ); - //J+ - } - 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(); ObjectId oldId = null; if (isUpdateCommand(rc)) { oldId = rc.getOldId(); + + if (logger.isTraceEnabled()) + { + logger.trace("handle update receive command from commit '{}' to '{}'", + oldId.getName(), newId.getName()); + } + } + else if (logger.isTraceEnabled()) + { + logger.trace("handle receive command for commit '{}'", newId.getName()); } File directory = rpack.getRepository().getDirectory(); @@ -297,6 +316,18 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook fireHookEvent(rpack, rc, directory, oldId, newId, type); } + else if (logger.isTraceEnabled()) + { + //J- + logger.trace("skip receive command, type={}, ref={}, result={}", + new Object[] { + rc.getType(), + rc.getRefName(), + rc.getResult() + } + ); + //J+ + } } } From 2eb629054501bfa96a76553f1d8de8f1d366ba00 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 18:56:45 +0200 Subject: [PATCH 03/11] improve trace logging for git repository hooks --- .../repository/GitRepositoryHookEvent.java | 33 +++++++++----- .../java/sonia/scm/repository/GitUtil.java | 43 ++++++++++++++++--- .../java/sonia/scm/web/GitReceiveHook.java | 37 +++++----------- 3 files changed, 72 insertions(+), 41 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java index 3aa4344f50..76fba12042 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java @@ -36,7 +36,6 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- 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; @@ -75,18 +74,34 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent * * @param directory * @param ref + * @param refName * @param newId * @param oldId * @param type */ - public GitRepositoryHookEvent(File directory, Ref ref, ObjectId newId, + public GitRepositoryHookEvent(File directory, String refName, ObjectId newId, ObjectId oldId, RepositoryHookType type) { this.directory = directory; - this.defaultBranch = GitUtil.getBranch(ref); + this.branch = GitUtil.getBranch(refName); 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 ---------------------------------------------------------- @@ -158,16 +173,14 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent { Changeset changeset = converter.createChangeset(commit); - if (changeset.getBranches().isEmpty() - && Util.isNotEmpty(defaultBranch)) + if (changeset.getBranches().isEmpty() && Util.isNotEmpty(branch)) { if (logger.isTraceEnabled()) { - logger.trace("set branch to current default branch {}", - defaultBranch); + logger.trace("set branch to current default branch {}", branch); } - changeset.getBranches().add(defaultBranch); + changeset.getBranches().add(branch); } result.add(changeset); @@ -192,10 +205,10 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent //~--- fields --------------------------------------------------------------- /** Field description */ - private List changesets; + private String branch; /** Field description */ - private String defaultBranch; + private List changesets; /** Field description */ private File directory; diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java index c65536217a..895b93bbca 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitUtil.java @@ -226,14 +226,27 @@ public class GitUtil if (ref != null) { + branch = getBranch(ref.getName()); + } - String name = ref.getName(); + return branch; + } - if (name.startsWith(PREFIX_HEADS)) - { - branch = name.substring(PREFIX_HEADS.length()); - } + /** + * Method description + * + * + * @param name + * + * @return + */ + public static String getBranch(String name) + { + String branch = null; + if (Util.isNotEmpty(name) && name.startsWith(PREFIX_HEADS)) + { + branch = name.substring(PREFIX_HEADS.length()); } return branch; @@ -329,6 +342,26 @@ public class GitUtil return date; } + /** + * Method description + * + * + * @param objectId + * + * @return + */ + public static String getId(ObjectId objectId) + { + String id = Util.EMPTY_STRING; + + if (objectId != null) + { + id = objectId.name(); + } + + return id; + } + /** * Method description * 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 60d8aa99af..1003055ddb 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 @@ -49,6 +49,7 @@ import sonia.scm.io.CommandResult; import sonia.scm.io.SimpleCommand; import sonia.scm.repository.GitRepositoryHandler; 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; @@ -153,8 +154,9 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook logger.debug("execute file hook '{}' in directoy '{}'"); } - final Command cmd = new SimpleCommand(hook.getAbsolutePath(), getId(oldId), - getId(newId), Util.nonNull(refName)); + final Command cmd = new SimpleCommand(hook.getAbsolutePath(), + GitUtil.getId(oldId), GitUtil.getId(newId), + Util.nonNull(refName)); // issue-99 cmd.setWorkDirectory(repositoryDirectory); @@ -221,7 +223,7 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook String repositoryName = RepositoryUtil.getRepositoryName(handler, directory); GitRepositoryHookEvent e = new GitRepositoryHookEvent(directory, - rc.getRef(), newId, oldId, type); + rc.getRefName(), newId, oldId, type); repositoryManager.fireHookEvent(GitRepositoryHandler.TYPE_NAME, repositoryName, e); @@ -252,10 +254,11 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook private void onReceive(ReceivePack rpack, Collection receiveCommands, RepositoryHookType type) { - if ( logger.isTraceEnabled() ) + if (logger.isTraceEnabled()) { logger.trace("received git hook, type={}", type); } + for (ReceiveCommand rc : receiveCommands) { if (isReceiveable(rc, type)) @@ -282,13 +285,15 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook if (logger.isTraceEnabled()) { - logger.trace("handle update receive command from commit '{}' to '{}'", + logger.trace( + "handle update receive command from commit '{}' to '{}'", oldId.getName(), newId.getName()); } } else if (logger.isTraceEnabled()) { - logger.trace("handle receive command for commit '{}'", newId.getName()); + logger.trace("handle receive command for commit '{}'", + newId.getName()); } File directory = rpack.getRepository().getDirectory(); @@ -365,26 +370,6 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook return IOUtil.getScript(baseFile); } - /** - * Method description - * - * - * @param objectId - * - * @return - */ - private String getId(ObjectId objectId) - { - String id = Util.EMPTY_STRING; - - if (objectId != null) - { - id = objectId.name(); - } - - return id; - } - /** * Method description * From fd84153a603c94feaa2fefe7146c3a6d56aa5343 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 19:01:39 +0200 Subject: [PATCH 04/11] do not fire hook event, if no new id is specified --- .../repository/GitRepositoryHookEvent.java | 81 ++++++++-------- .../java/sonia/scm/web/GitReceiveHook.java | 96 ++++++++++++------- 2 files changed, 99 insertions(+), 78 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java index 76fba12042..7ced4f92f4 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java @@ -35,6 +35,8 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.collect.Lists; + import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevSort; @@ -145,58 +147,55 @@ public class GitRepositoryHookEvent extends AbstractRepositoryHookEvent */ private List fetchChangesets() { - List result = new ArrayList(); + List result = Lists.newArrayList(); + GitChangesetConverter converter = null; + RevWalk walk = null; + org.eclipse.jgit.lib.Repository repository = null; - if (newId != null) + try { - GitChangesetConverter converter = null; - RevWalk walk = null; - org.eclipse.jgit.lib.Repository repository = null; + 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)); - try + if (oldId != null) { - 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)); + walk.markUninteresting(walk.parseCommit(oldId)); + } - if (oldId != null) + RevCommit commit = walk.next(); + + while (commit != null) + { + Changeset changeset = converter.createChangeset(commit); + + if (changeset.getBranches().isEmpty() && Util.isNotEmpty(branch)) { - 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()) { - if (logger.isTraceEnabled()) - { - logger.trace("set branch to current default branch {}", branch); - } - - changeset.getBranches().add(branch); + logger.trace("set branch to current default branch {}", branch); } - result.add(changeset); - commit = walk.next(); + 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); - } + } + catch (IOException ex) + { + logger.error("could not fetch changesets", ex); + } + finally + { + IOUtil.close(converter); + GitUtil.release(walk); + GitUtil.close(repository); + } return result; 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 1003055ddb..8f40c7a221 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 @@ -277,49 +277,16 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook } ObjectId newId = rc.getNewId(); - ObjectId oldId = null; - if (isUpdateCommand(rc)) + if (newId != null) { - oldId = rc.getOldId(); - - if (logger.isTraceEnabled()) - { - logger.trace( - "handle update receive command from commit '{}' to '{}'", - oldId.getName(), newId.getName()); - } + onReceive(rpack, rc, newId, type); } - else if (logger.isTraceEnabled()) + else if (logger.isWarnEnabled()) { - logger.trace("handle receive command for commit '{}'", - newId.getName()); + logger.warn("received hook event without new id"); } - File directory = rpack.getRepository().getDirectory(); - String scriptName = null; - - if (type == RepositoryHookType.POST_RECEIVE) - { - scriptName = FILE_HOOK_POST_RECEIVE; - } - else if (type == RepositoryHookType.PRE_RECEIVE) - { - scriptName = FILE_HOOK_PRE_RECEIVE; - } - - if (scriptName != null) - { - File hookScript = getHookScript(directory, scriptName); - - if (hookScript != null) - { - executeFileHook(rpack, rc, directory, hookScript, oldId, newId, - rc.getRefName()); - } - } - - fireHookEvent(rpack, rc, directory, oldId, newId, type); } else if (logger.isTraceEnabled()) { @@ -336,6 +303,61 @@ public class GitReceiveHook implements PreReceiveHook, PostReceiveHook } } + /** + * Method description + * + * + * @param rpack + * @param rc + * @param newId + * @param type + */ + private void onReceive(ReceivePack rpack, ReceiveCommand rc, ObjectId newId, + RepositoryHookType type) + { + ObjectId oldId = null; + + if (isUpdateCommand(rc)) + { + oldId = rc.getOldId(); + + if (logger.isTraceEnabled()) + { + logger.trace("handle update receive command from commit '{}' to '{}'", + oldId.getName(), newId.getName()); + } + } + else if (logger.isTraceEnabled()) + { + logger.trace("handle receive command for commit '{}'", newId.getName()); + } + + File directory = rpack.getRepository().getDirectory(); + String scriptName = null; + + if (type == RepositoryHookType.POST_RECEIVE) + { + scriptName = FILE_HOOK_POST_RECEIVE; + } + else if (type == RepositoryHookType.PRE_RECEIVE) + { + scriptName = FILE_HOOK_PRE_RECEIVE; + } + + if (scriptName != null) + { + File hookScript = getHookScript(directory, scriptName); + + if (hookScript != null) + { + executeFileHook(rpack, rc, directory, hookScript, oldId, newId, + rc.getRefName()); + } + } + + fireHookEvent(rpack, rc, directory, oldId, newId, type); + } + /** * Method description * From 85d9e4d592ee079b3bc098625218cdb57d4c43f1 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 19:13:02 +0200 Subject: [PATCH 05/11] remove unused import --- .../main/java/sonia/scm/repository/GitRepositoryHookEvent.java | 1 - 1 file changed, 1 deletion(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java index 7ced4f92f4..af997462dd 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java @@ -53,7 +53,6 @@ import sonia.scm.util.Util; import java.io.File; import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; import java.util.List; From e34d95f394a6476d8af93bddef4591b76405dff6 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 13 Sep 2012 19:34:04 +0200 Subject: [PATCH 06/11] use getName method for logging git ids --- .../java/sonia/scm/repository/GitChangesetConverter.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java index 28e862d850..fb14003e64 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java @@ -269,7 +269,8 @@ public class GitChangesetConverter implements Closeable { if (logger.isTraceEnabled()) { - logger.trace("no parent tree at position 0 for commit {}", commit); + logger.trace("no parent tree at position 0 for commit {}", + commit.getName()); } treeWalk.addTree(new EmptyTreeIterator()); @@ -279,7 +280,7 @@ public class GitChangesetConverter implements Closeable { if (logger.isTraceEnabled()) { - logger.trace("no parent available for commit {}", commit); + logger.trace("no parent available for commit {}", commit.getName()); } treeWalk.addTree(new EmptyTreeIterator()); From c6f24063df3199446e0689c205e146dab46db0e2 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 19 Sep 2012 19:11:06 +0200 Subject: [PATCH 07/11] improve git hook handling --- .../repository/GitHookChangesetCollector.java | 237 ++++++++++++++++++ .../repository/GitRepositoryHookEvent.java | 137 ++-------- .../java/sonia/scm/web/GitReceiveHook.java | 183 ++++++-------- 3 files changed, 334 insertions(+), 223 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java new file mode 100644 index 0000000000..3edbbc7fc0 --- /dev/null +++ b/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 receiveCommands) + { + this.rpack = rpack; + this.receiveCommands = receiveCommands; + } + + //~--- methods -------------------------------------------------------------- + + /** + * Method description + * + * + * @return + */ + public List collectChangesets() + { + List 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 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 getExistingObjects(ReceivePack rpack) + { + List existingObjects = Lists.newArrayList(); + + if (existingObjects == null) + { + Map refs = rpack.getRepository().getAllRefs(); + + for (Ref r : refs.values()) + { + existingObjects.add(r.getObjectId()); + } + } + + return existingObjects; + } + + //~--- fields --------------------------------------------------------------- + + /** Field description */ + private List receiveCommands; + + /** Field description */ + private ReceivePack rpack; +} diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java index af997462dd..ebcc4fefad 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitRepositoryHookEvent.java +++ b/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.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; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.eclipse.jgit.transport.ReceivePack; //~--- 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 ref - * @param refName - * @param newId - * @param oldId + * @param rpack + * @param receiveCommands * @param type */ - public GitRepositoryHookEvent(File directory, String refName, ObjectId newId, - ObjectId oldId, RepositoryHookType type) + public GitRepositoryHookEvent(ReceivePack rpack, + List receiveCommands, RepositoryHookType type) { - this.directory = directory; - this.branch = GitUtil.getBranch(refName); - this.newId = newId; - this.oldId = oldId; + this.rpack = rpack; + this.receiveCommands = receiveCommands; 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 getChangesets() + public List 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 fetchChangesets() - { - List 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 changesets; /** Field description */ - private File directory; + private List receiveCommands; /** Field description */ - private ObjectId newId; - - /** Field description */ - private ObjectId oldId; + private ReceivePack rpack; /** Field description */ private RepositoryHookType type; 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 8f40c7a221..2696a03846 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 @@ -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 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 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 receiveCommands, RepositoryHookType type) + { + if (logger.isTraceEnabled()) + { + logger.trace("received git hook, type={}", type); + } + + List 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"); + } } /** From ca8aaddddce2c3ee032fc2a8850f6d4e484aca24 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Sep 2012 11:00:42 +0200 Subject: [PATCH 08/11] use revwalk from rpack for changeset converter --- .../java/sonia/scm/repository/GitHookChangesetCollector.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java index 3edbbc7fc0..b62848fb5a 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java @@ -98,13 +98,14 @@ public class GitHookChangesetCollector org.eclipse.jgit.lib.Repository repository = rpack.getRepository(); - GitChangesetConverter converter = null; RevWalk walk = null; + + GitChangesetConverter converter = null; try { - converter = new GitChangesetConverter(repository, GitUtil.ID_LENGTH); walk = rpack.getRevWalk(); + converter = new GitChangesetConverter(repository, walk, GitUtil.ID_LENGTH); for (ReceiveCommand rc : receiveCommands) { From 7404791b8d5381b2d84c47449efc8db107708ff8 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Sep 2012 11:08:12 +0200 Subject: [PATCH 09/11] parse commit body to avoid npe --- .../java/sonia/scm/repository/GitHookChangesetCollector.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java index b62848fb5a..0d93ca5b1c 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java @@ -161,6 +161,8 @@ public class GitHookChangesetCollector while (commit != null) { + // parse commit body to avoid npe + walk.parseBody(commit); Changeset changeset = converter.createChangeset(commit); List branches = changeset.getBranches(); From 7afa2b0193a28efacfc6ceb89d8bd97cf03e0bdc Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 20 Sep 2012 11:37:38 +0200 Subject: [PATCH 10/11] use always the branch informations from the receivecommand --- .../scm/repository/GitChangesetConverter.java | 55 +++++++++++++------ .../repository/GitHookChangesetCollector.java | 26 +++------ 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java index fb14003e64..2599238bf9 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitChangesetConverter.java @@ -35,6 +35,7 @@ package sonia.scm.repository; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.collect.Lists; import com.google.common.collect.Multimap; import org.eclipse.jgit.diff.DiffEntry; @@ -134,7 +135,6 @@ public class GitChangesetConverter implements Closeable * Method description * * - * * @param commit * * @return @@ -142,6 +142,42 @@ public class GitChangesetConverter implements Closeable * @throws IOException */ public Changeset createChangeset(RevCommit commit) throws IOException + { + List branches = Lists.newArrayList(); + Set refs = repository.getAllRefsByPeeledObjectId().get(commit.getId()); + + if (Util.isNotEmpty(refs)) + { + + for (Ref ref : refs) + { + String branch = GitUtil.getBranch(ref); + + if (branch != null) + { + branches.add(branch); + } + } + + } + + return createChangeset(commit, branches); + } + + /** + * Method description + * + * + * + * @param commit + * @param branches + * + * @return + * + * @throws IOException + */ + public Changeset createChangeset(RevCommit commit, List branches) + throws IOException { String id = commit.getId().abbreviate(idLength).name(); List parentList = null; @@ -183,22 +219,7 @@ public class GitChangesetConverter implements Closeable changeset.getTags().addAll(tagCollection); } - Set refs = repository.getAllRefsByPeeledObjectId().get(commit.getId()); - - if (Util.isNotEmpty(refs)) - { - - for (Ref ref : refs) - { - String branch = GitUtil.getBranch(ref); - - if (branch != null) - { - changeset.getBranches().add(branch); - } - } - - } + changeset.setBranches(branches); return changeset; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java index 0d93ca5b1c..f23e2fd917 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitHookChangesetCollector.java @@ -99,13 +99,14 @@ public class GitHookChangesetCollector org.eclipse.jgit.lib.Repository repository = rpack.getRepository(); RevWalk walk = null; - + GitChangesetConverter converter = null; try { walk = rpack.getRevWalk(); - converter = new GitChangesetConverter(repository, walk, GitUtil.ID_LENGTH); + converter = new GitChangesetConverter(repository, walk, + GitUtil.ID_LENGTH); for (ReceiveCommand rc : receiveCommands) { @@ -159,28 +160,19 @@ public class GitHookChangesetCollector RevCommit commit = walk.next(); + List branches = Lists.newArrayList(branch); + while (commit != null) { + // parse commit body to avoid npe walk.parseBody(commit); - Changeset changeset = converter.createChangeset(commit); - List branches = changeset.getBranches(); + Changeset changeset = converter.createChangeset(commit, branches); - if (branches.isEmpty()) + if (logger.isTraceEnabled()) { - if (logger.isTraceEnabled()) - { - //J- - logger.trace( - "missing branch informations for {}, set default branch {}", - changeset.getId(), - branch - ); - //J+ - } - - branches.add(branch); + logger.trace("retrive commit {} for hook", changeset.getId()); } changesets.add(changeset); From 3abacd8998f046b98e372b0374a7abdbadc64717 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Fri, 21 Sep 2012 13:11:06 +0200 Subject: [PATCH 11/11] close branch issue-242