From d38435c13a85a11cd08c50fab26a7f6959fd0872 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Wed, 28 Sep 2016 11:05:00 +0200 Subject: [PATCH 1/2] fix npe when GitHookBranchProvider tries to collect a tag as branch, see issue #865 --- .../repository/api/GitHookBranchProvider.java | 41 ++++--- .../api/GitHookBranchProviderTest.java | 115 ++++++++++++++++++ 2 files changed, 136 insertions(+), 20 deletions(-) create mode 100644 scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookBranchProviderTest.java diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookBranchProvider.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookBranchProvider.java index ecc373e2b8..23b3ce0196 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookBranchProvider.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/api/GitHookBranchProvider.java @@ -33,6 +33,7 @@ package sonia.scm.repository.api; //~--- non-JDK imports -------------------------------------------------------- +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList.Builder; @@ -45,18 +46,24 @@ import sonia.scm.repository.GitUtil; import java.util.List; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + /** - * + * Collects created, modified and deleted git branches during a hook. + * * @author Sebastian Sdorra */ public class GitHookBranchProvider implements HookBranchProvider { + + private static final Logger logger = LoggerFactory.getLogger(GitHookBranchProvider.class); /** - * Constructs ... + * Constructs a new instance. * * - * @param commands + * @param commands received git commands */ public GitHookBranchProvider(List commands) { @@ -66,10 +73,14 @@ public class GitHookBranchProvider implements HookBranchProvider for (ReceiveCommand command : commands) { Type type = command.getType(); - String branch = GitUtil.getBranch(command.getRefName()); + String ref = command.getRefName(); + String branch = GitUtil.getBranch(ref); - if ((type == Type.CREATE) || (type == Type.UPDATE) - || (type == Type.UPDATE_NONFASTFORWARD)) + if (Strings.isNullOrEmpty(branch)) + { + logger.debug("ref {} is not a branch", ref); + } + else if (isCreateOrUpdate(type)) { createdOrModifiedBuilder.add(branch); } @@ -82,27 +93,19 @@ public class GitHookBranchProvider implements HookBranchProvider createdOrModified = createdOrModifiedBuilder.build(); deletedOrClosed = deletedOrClosedBuilder.build(); } + + private boolean isCreateOrUpdate(Type type){ + return type == Type.CREATE || type == Type.UPDATE || type == Type.UPDATE_NONFASTFORWARD; + } //~--- get methods ---------------------------------------------------------- - /** - * Method description - * - * - * @return - */ @Override public List getCreatedOrModified() { return createdOrModified; } - /** - * Method description - * - * - * @return - */ @Override public List getDeletedOrClosed() { @@ -111,9 +114,7 @@ public class GitHookBranchProvider implements HookBranchProvider //~--- fields --------------------------------------------------------------- - /** Field description */ private final List createdOrModified; - /** Field description */ private final List deletedOrClosed; } diff --git a/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookBranchProviderTest.java b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookBranchProviderTest.java new file mode 100644 index 0000000000..9e5e0eef36 --- /dev/null +++ b/scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/api/GitHookBranchProviderTest.java @@ -0,0 +1,115 @@ +/*** + * Copyright (c) 2015, 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. + * + * https://bitbucket.org/sdorra/scm-manager + * + */ + +package sonia.scm.repository.api; + +import com.google.common.collect.Lists; +import java.util.Arrays; +import java.util.List; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.hamcrest.Matchers; +import org.junit.Test; +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import static org.hamcrest.Matchers.*; +import org.junit.Before; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; + +/** + * Unit tests for {@link GitHookBranchProvider}. + * + * @author Sebastian Sdorra + */ +@RunWith(MockitoJUnitRunner.class) +public class GitHookBranchProviderTest { + + @Mock + private ReceiveCommand command; + + private List commands; + + /** + * Prepare mocks for upcoming test. + */ + @Before + public void setUpMocks(){ + commands = Lists.newArrayList(command); + } + + /** + * Tests {@link GitHookBranchProvider#getCreatedOrModified()}. + */ + @Test + public void testGetCreatedOrModified(){ + List types = Arrays.asList( + ReceiveCommand.Type.CREATE, ReceiveCommand.Type.UPDATE, ReceiveCommand.Type.UPDATE_NONFASTFORWARD + ); + for ( ReceiveCommand.Type type : types ){ + checkCreatedOrModified(type); + } + } + + private void checkCreatedOrModified(ReceiveCommand.Type type){ + GitHookBranchProvider provider = createGitHookBranchProvider(type, "refs/heads/hello"); + assertThat(provider.getCreatedOrModified(), Matchers.contains("hello")); + assertThat(provider.getDeletedOrClosed(), empty()); + } + + + /** + * Tests {@link GitHookBranchProvider#getDeletedOrClosed()}. + */ + @Test + public void testGetDeletedOrClosed(){ + GitHookBranchProvider provider = createGitHookBranchProvider(ReceiveCommand.Type.DELETE, "refs/heads/hello"); + assertThat(provider.getDeletedOrClosed(), Matchers.contains("hello")); + assertThat(provider.getCreatedOrModified(), empty()); + } + + /** + * Tests {@link GitHookBranchProvider} with a tag instead of a branch. + */ + @Test + public void testWithTag(){ + GitHookBranchProvider provider = createGitHookBranchProvider(ReceiveCommand.Type.CREATE, "refs/tags/1.0.0"); + assertThat(provider.getCreatedOrModified(), empty()); + assertThat(provider.getDeletedOrClosed(), empty()); + } + + private GitHookBranchProvider createGitHookBranchProvider(ReceiveCommand.Type type, String refName){ + when(command.getType()).thenReturn(type); + when(command.getRefName()).thenReturn(refName); + return new GitHookBranchProvider(commands); + } + +} \ No newline at end of file From 6a917f03a602210c094cb436a1c88c14d16e5266 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Fri, 30 Sep 2016 19:45:21 +0200 Subject: [PATCH 2/2] close branch issue-865