diff --git a/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java b/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java index 63d497c69..c264eb9ae 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java @@ -198,6 +198,7 @@ protected CauseData retrieveCauseData(MergeRequestHook hook) { .withSourceBranch(hook.getObjectAttributes().getSourceBranch()) .withUserName( hook.getObjectAttributes().getLastCommit().getAuthor().getName()) + .withUserUsername(hook.getUser().getUsername()) .withUserEmail( hook.getObjectAttributes().getLastCommit().getAuthor().getEmail()) .withSourceRepoHomepage(hook.getObjectAttributes().getSource().getHomepage()) @@ -360,10 +361,10 @@ private boolean isForcedByAddedLabel(MergeRequestHook hook) { changedLabels.getPrevious() != null ? changedLabels.getPrevious() : emptyList(); return current.stream() - .filter(currentLabel -> !previous.stream() - .anyMatch(previousLabel -> Objects.equals(currentLabel.getId(), previousLabel.getId()))) - .map(label -> label.getTitle()) - .anyMatch(label -> labelsThatForcesBuildIfAdded.contains(label)); + .filter(currentLabel -> previous.stream() + .noneMatch(previousLabel -> Objects.equals(currentLabel.getId(), previousLabel.getId()))) + .map(MergeRequestLabel::getTitle) + .anyMatch(labelsThatForcesBuildIfAdded::contains); } private boolean isNotSkipWorkInProgressMergeRequest(MergeRequestObjectAttributes objectAttributes) { diff --git a/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImpl.java b/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImpl.java index 15737f3e4..d2ec0b912 100644 --- a/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImpl.java +++ b/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImpl.java @@ -72,6 +72,7 @@ protected CauseData retrieveCauseData(NoteHook hook) { .withBranch(hook.getMergeRequest().getSourceBranch()) .withSourceBranch(hook.getMergeRequest().getSourceBranch()) .withUserName(hook.getMergeRequest().getLastCommit().getAuthor().getName()) + .withUserUsername(hook.getUser().getUsername()) .withUserEmail( hook.getMergeRequest().getLastCommit().getAuthor().getEmail()) .withSourceRepoHomepage(hook.getMergeRequest().getSource().getHomepage()) diff --git a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/PendingBuildsHandlerTest.java b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/PendingBuildsHandlerTest.java index a083f93e2..035dabdf2 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/PendingBuildsHandlerTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/PendingBuildsHandlerTest.java @@ -22,7 +22,6 @@ import com.dabsquared.gitlabjenkins.gitlab.hook.model.Repository; import com.dabsquared.gitlabjenkins.gitlab.hook.model.State; import com.dabsquared.gitlabjenkins.gitlab.hook.model.User; -import com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.CommitBuilder; import com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.MergeRequestHookBuilder; import com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.ProjectBuilder; import com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.PushHookBuilder; @@ -36,7 +35,7 @@ import hudson.model.Queue; import java.io.File; import java.io.IOException; -import java.util.Arrays; +import java.util.Collections; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.junit.After; import org.junit.Before; @@ -76,7 +75,7 @@ public void clearQueue() { @Test public void projectCanBeConfiguredToSendPendingBuildStatusWhenTriggered() throws IOException { - Project project = + Project project = freestyleProject("freestyleProject1", new GitLabCommitStatusPublisher(GITLAB_BUILD_NAME, false)); GitLabPushTrigger gitLabPushTrigger = gitLabPushTrigger(project); @@ -118,7 +117,7 @@ public void workflowJobCanConfiguredToSendToPendingBuildStatusWhenTriggered() th @Test public void queuedMergeRequestBuildsCanBeCancelledOnMergeRequestUpdate() throws IOException { - Project project = freestyleProject("project1", new GitLabCommitStatusPublisher(GITLAB_BUILD_NAME, false)); + Project project = freestyleProject("project1", new GitLabCommitStatusPublisher(GITLAB_BUILD_NAME, false)); GitLabPushTrigger gitLabPushTrigger = gitLabPushTrigger(project); gitLabPushTrigger.setCancelPendingBuildsOnUpdate(true); @@ -153,7 +152,7 @@ public void queuedMergeRequestBuildsCanBeCancelledOnMergeRequestUpdate() throws assertThat(jenkins.getInstance().getQueue().getItems().length, is(3)); } - private GitLabPushTrigger gitLabPushTrigger(Project project) throws IOException { + private GitLabPushTrigger gitLabPushTrigger(Project project) throws IOException { GitLabPushTrigger gitLabPushTrigger = gitLabPushTrigger(); project.addTrigger(gitLabPushTrigger); gitLabPushTrigger.start(project, true); @@ -212,6 +211,13 @@ private MergeRequestHook mergeRequestHook(int projectId, String branch, String c .withProject(ProjectBuilder.project() .withWebUrl("https://gitlab.org/test.git") .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(); } @@ -230,15 +236,17 @@ private PushHook pushHook(int projectId, String branch, String commitId) { .withAfter(commitId) .withRepository(new Repository()) .withProject(ProjectBuilder.project().withNamespace("namespace").build()) - .withCommits(Arrays.asList( - CommitBuilder.commit().withId(commitId).withAuthor(user).build())) + .withCommits(Collections.singletonList( + commit().withId(commitId).withAuthor(user).build())) .withRepository(repository) .withObjectKind("push") - .withUserName("username") + .withUserName("User") + .withUserUsername("username") + .withUserEmail("user@gitlab.com") .build(); } - private Project freestyleProject(String name, GitLabCommitStatusPublisher gitLabCommitStatusPublisher) + private Project freestyleProject(String name, GitLabCommitStatusPublisher gitLabCommitStatusPublisher) throws IOException { FreeStyleProject project = jenkins.createFreeStyleProject(name); project.setQuietPeriod(5000); @@ -248,7 +256,7 @@ private Project freestyleProject(String name, GitLabCommitStatusPublisher gitLab } private WorkflowJob workflowJob() throws IOException { - ItemGroup itemGroup = mock(ItemGroup.class); + ItemGroup itemGroup = mock(ItemGroup.class); when(itemGroup.getFullName()).thenReturn("parent"); when(itemGroup.getUrlChildPrefix()).thenReturn("prefix"); diff --git a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImplTest.java b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImplTest.java index 31211539c..e0ee4e1ba 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImplTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImplTest.java @@ -27,6 +27,7 @@ import hudson.util.OneShotEvent; import java.io.IOException; import java.util.Arrays; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; @@ -468,6 +469,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen .withProject(project() .withWebUrl("https://gitlab.org/test.git") .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)), @@ -491,6 +499,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen .withProject(project() .withWebUrl("https://gitlab.org/test.git") .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)), @@ -597,6 +612,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen .withProject(project() .withWebUrl("https://gitlab.org/test.git") .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)), @@ -622,7 +644,7 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen }); project.setQuietPeriod(0); MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl( - Arrays.asList(State.opened, State.reopened), Arrays.asList(Action.approved), false, false, false); + Arrays.asList(State.opened, State.reopened), List.of(Action.approved), false, false, false); mergeRequestHookTriggerHandler.handle( project, mergeRequestHook() @@ -633,6 +655,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen .withId("testid") .build()) .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)), diff --git a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImplTest.java b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImplTest.java index fc30ac565..5fd2f3271 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImplTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImplTest.java @@ -159,6 +159,13 @@ public boolean perform(AbstractBuild build, Launcher launcher, BuildListen .withWebUrl("https://gitlab.org/test.git") .build()) .build()) + .withUser(user().withId(1) + .withName("User") + .withUsername("user") + .withEmail("user@gitlab.com") + .withAvatarUrl( + "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon") + .build()) .build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)), diff --git a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java index 3289086d9..8d6bda593 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/MergeRequestBuildActionTest.java @@ -1,6 +1,7 @@ package com.dabsquared.gitlabjenkins.webhook.build; import static com.dabsquared.gitlabjenkins.cause.CauseDataBuilder.causeData; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -103,10 +104,14 @@ public void build() throws IOException { testProject.addTrigger(mockTrigger); executeMergeRequestAction(testProject, getJson("MergeRequestEvent.json")); } finally { - ArgumentCaptor pushHookArgumentCaptor = ArgumentCaptor.forClass(MergeRequestHook.class); - verify(mockTrigger).onPost(pushHookArgumentCaptor.capture()); - assertThat(pushHookArgumentCaptor.getValue().getProject(), is(notNullValue())); - assertThat(pushHookArgumentCaptor.getValue().getProject().getWebUrl(), is(notNullValue())); + ArgumentCaptor mergeRequestHookArgumentCaptor = + ArgumentCaptor.forClass(MergeRequestHook.class); + verify(mockTrigger).onPost(mergeRequestHookArgumentCaptor.capture()); + assertThat(mergeRequestHookArgumentCaptor.getValue().getProject(), is(notNullValue())); + assertThat(mergeRequestHookArgumentCaptor.getValue().getProject().getWebUrl(), is(notNullValue())); + assertThat(mergeRequestHookArgumentCaptor.getValue().getUser(), is(notNullValue())); + assertThat(mergeRequestHookArgumentCaptor.getValue().getUser().getName(), containsString("Administrator")); + assertThat(mergeRequestHookArgumentCaptor.getValue().getUser().getUsername(), containsString("root")); } } diff --git a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java index 03a97c03d..23e15e116 100644 --- a/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java +++ b/src/test/java/com/dabsquared/gitlabjenkins/webhook/build/NoteBuildActionTest.java @@ -1,6 +1,10 @@ package com.dabsquared.gitlabjenkins.webhook.build; import static com.dabsquared.gitlabjenkins.cause.CauseDataBuilder.causeData; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.verify; @@ -14,6 +18,8 @@ import hudson.model.queue.QueueTaskFuture; import hudson.plugins.git.GitSCM; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Objects; import java.util.concurrent.ExecutionException; import org.apache.commons.io.IOUtils; import org.eclipse.jgit.api.Git; @@ -28,6 +34,7 @@ import org.jvnet.hudson.test.JenkinsRule; import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.StaplerResponse; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.junit.MockitoJUnitRunner; @@ -68,13 +75,19 @@ public void setup() throws Exception { @Test public void build() throws IOException { - FreeStyleProject testProject = jenkins.createFreeStyleProject(); - testProject.addTrigger(trigger); - - exception.expect(HttpResponses.HttpResponseException.class); - new NoteBuildAction(testProject, getJson("NoteEvent.json"), null).execute(response); - - verify(trigger).onPost(any(NoteHook.class)); + try { + FreeStyleProject testProject = jenkins.createFreeStyleProject(); + testProject.addTrigger(trigger); + + exception.expect(HttpResponses.HttpResponseException.class); + new NoteBuildAction(testProject, getJson("NoteEvent.json"), null).execute(response); + } finally { + ArgumentCaptor noteHookArgumentCaptor = ArgumentCaptor.forClass(NoteHook.class); + verify(trigger).onPost(noteHookArgumentCaptor.capture()); + assertThat(noteHookArgumentCaptor.getValue().getUser(), is(notNullValue())); + assertThat(noteHookArgumentCaptor.getValue().getUser().getName(), containsString("Administrator")); + assertThat(noteHookArgumentCaptor.getValue().getUser().getUsername(), containsString("root")); + } } @Test @@ -84,7 +97,9 @@ public void build_alreadyBuiltMR_alreadyBuiltMR() throws IOException, ExecutionE testProject.setScm(new GitSCM(gitRepoUrl)); QueueTaskFuture future = testProject.scheduleBuild2( 0, new ParametersAction(new StringParameterValue("gitlabTargetBranch", "master"))); - future.get(); + if (future != null) { + future.get(); + } exception.expect(HttpResponses.HttpResponseException.class); new NoteBuildAction(testProject, getJson("NoteEvent_alreadyBuiltMR.json"), null).execute(response); @@ -134,6 +149,7 @@ public void build_alreadyBuiltMR_differentTargetBranch() } private String getJson(String name) throws IOException { - return IOUtils.toString(getClass().getResourceAsStream(name)).replace("${commitSha1}", commitSha1); + return IOUtils.toString(Objects.requireNonNull(getClass().getResourceAsStream(name)), StandardCharsets.UTF_8) + .replace("${commitSha1}", commitSha1); } }