p85947160
/
Gitee-Jenkins-Plugin
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Don't check LastCommitNotYetBuild on Approve MR action (#664) 

* Don't check LastCommitNotYetBuild on Approve MR action

* Add @DataBoundSetter
This commit is contained in:
jakub-bochenski 2017-12-11 11:59:14 +01:00 committed by Karsten Kraus
parent ab51c7f2f5
commit 316b8bebb8
5 changed files with 132 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/main/java/com/dabsquared/gitlabjenkins/GitLabPushTrigger.java
View File

@ -82,6 +82,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
private boolean triggerOnPipelineEvent = false;
private boolean triggerOnAcceptedMergeRequest = false;
private boolean triggerOnClosedMergeRequest = false;
private boolean triggerOnApprovedMergeRequest = false;
private TriggerOpenMergeRequest triggerOpenMergeRequestOnPush;
private boolean triggerOnNoteRequest = true;
private String noteRegex = "";
@ -119,7 +120,8 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
boolean setBuildDescription, boolean addNoteOnMergeRequest, boolean addCiMessage, boolean addVoteOnMergeRequest,
boolean acceptMergeRequestOnSuccess, BranchFilterType branchFilterType,
String includeBranchesSpec, String excludeBranchesSpec, String targetBranchRegex,
MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig, String secretToken, boolean triggerOnPipelineEvent) {
MergeRequestLabelFilterConfig mergeRequestLabelFilterConfig, String secretToken, boolean triggerOnPipelineEvent,
boolean triggerOnApprovedMergeRequest) {
this.triggerOnPush = triggerOnPush;
this.triggerOnMergeRequest = triggerOnMergeRequest;
this.triggerOnAcceptedMergeRequest = triggerOnAcceptedMergeRequest;
@ -141,6 +143,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
this.acceptMergeRequestOnSuccess = acceptMergeRequestOnSuccess;
this.mergeRequestLabelFilterConfig = mergeRequestLabelFilterConfig;
this.secretToken = Secret.fromString(secretToken);
this.triggerOnApprovedMergeRequest = triggerOnApprovedMergeRequest;
initializeTriggerHandler();
initializeBranchFilter();
@ -270,6 +273,11 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
public void setTriggerOnPush(boolean triggerOnPush) {
this.triggerOnPush = triggerOnPush;
}
@DataBoundSetter
public void setTriggerOnApprovedMergeRequest(boolean triggerOnApprovedMergeRequest) {
this.triggerOnApprovedMergeRequest = triggerOnApprovedMergeRequest;
}
@DataBoundSetter
public void setTriggerOnMergeRequest(boolean triggerOnMergeRequest) {
@ -421,7 +429,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> {
private void initializeTriggerHandler() {
mergeRequestHookTriggerHandler = newMergeRequestHookTriggerHandler(triggerOnMergeRequest,
triggerOnAcceptedMergeRequest, triggerOnClosedMergeRequest, triggerOpenMergeRequestOnPush,
skipWorkInProgressMergeRequest);
skipWorkInProgressMergeRequest, triggerOnApprovedMergeRequest);
noteHookTriggerHandler = newNoteHookTriggerHandler(triggerOnNoteRequest, noteRegex);
pushHookTriggerHandler = newPushHookTriggerHandler(triggerOnPush, triggerOpenMergeRequestOnPush, skipWorkInProgressMergeRequest);
pipelineTriggerHandler = newPipelineHookTriggerHandler(triggerOnPipelineEvent);

19
src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerFactory.java
View File

@ -1,10 +1,13 @@
package com.dabsquared.gitlabjenkins.trigger.handler.merge;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.Action;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.State;
import com.dabsquared.gitlabjenkins.trigger.TriggerOpenMergeRequest;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.List;
import java.util.Set;
/**
* @author Robin Müller
@ -17,15 +20,24 @@ public final class MergeRequestHookTriggerHandlerFactory {
boolean triggerOnAcceptedMergeRequest,
boolean triggerOnClosedMergeRequest,
TriggerOpenMergeRequest triggerOpenMergeRequest,
boolean skipWorkInProgressMergeRequest) {
if (triggerOnMergeRequest || triggerOnAcceptedMergeRequest || triggerOnClosedMergeRequest || triggerOpenMergeRequest != TriggerOpenMergeRequest.never) {
return new MergeRequestHookTriggerHandlerImpl(retrieveAllowedStates(triggerOnMergeRequest, triggerOnAcceptedMergeRequest, triggerOnClosedMergeRequest, triggerOpenMergeRequest),
boolean skipWorkInProgressMergeRequest,
boolean triggerOnApprovedMergeRequest) {
if (triggerOnMergeRequest || triggerOnAcceptedMergeRequest || triggerOnClosedMergeRequest || triggerOpenMergeRequest != TriggerOpenMergeRequest.never || triggerOnApprovedMergeRequest) {
return new MergeRequestHookTriggerHandlerImpl(retrieveAllowedStates(triggerOnMergeRequest, triggerOnAcceptedMergeRequest, triggerOnClosedMergeRequest, triggerOpenMergeRequest),
retrieveAllowedActions(triggerOnApprovedMergeRequest),
skipWorkInProgressMergeRequest);
} else {
return new NopMergeRequestHookTriggerHandler();
}
}
private static Set<Action> retrieveAllowedActions(boolean triggerOnApprovedMergeRequest) {
Set<Action> allowedActions = EnumSet.noneOf(Action.class);
if (triggerOnApprovedMergeRequest)
allowedActions.add(Action.approved);
return allowedActions;
}
private static List<State> retrieveAllowedStates(boolean triggerOnMergeRequest,
boolean triggerOnAcceptedMergeRequest,
boolean triggerOnClosedMergeRequest,
@ -44,6 +56,7 @@ public final class MergeRequestHookTriggerHandlerFactory {
if (triggerOpenMergeRequest != TriggerOpenMergeRequest.never) {
result.add(State.updated);
}
return result;
}
}

26
src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java
View File

@ -2,6 +2,7 @@ package com.dabsquared.gitlabjenkins.trigger.handler.merge;
import com.dabsquared.gitlabjenkins.cause.CauseData;
import com.dabsquared.gitlabjenkins.cause.GitLabWebHookCause;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.Action;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.MergeRequestHook;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.MergeRequestObjectAttributes;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.State;
@ -16,7 +17,8 @@ import hudson.plugins.git.GitSCM;
import hudson.plugins.git.RevisionParameterAction;
import org.apache.commons.lang.StringUtils;
import java.util.List;
import java.util.Collection;
import java.util.EnumSet;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -31,18 +33,24 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
private static final Logger LOGGER = Logger.getLogger(MergeRequestHookTriggerHandlerImpl.class.getName());
private final List<State> allowedStates;
private final Collection<State> allowedStates;
private final boolean skipWorkInProgressMergeRequest;
private final Collection<Action> allowedActions;
MergeRequestHookTriggerHandlerImpl(List<State> allowedStates, boolean skipWorkInProgressMergeRequest) {
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, boolean skipWorkInProgressMergeRequest) {
this(allowedStates, EnumSet.noneOf(Action.class),skipWorkInProgressMergeRequest);
}
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, Collection<Action> allowedActions, boolean skipWorkInProgressMergeRequest) {
this.allowedStates = allowedStates;
this.allowedActions = allowedActions;
this.skipWorkInProgressMergeRequest = skipWorkInProgressMergeRequest;
}
@Override
public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
if (allowedStates.contains(objectAttributes.getState())
if (isAllowedByConfig(objectAttributes)
&& isLastCommitNotYetBuild(job, hook)
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)
&& mergeRequestLabelFilter.isMergeRequestAllowed(hook.getObjectAttributes().getLabels())) {
@ -129,6 +137,11 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
private boolean isLastCommitNotYetBuild(Job<?, ?> project, MergeRequestHook hook) {
MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
if (objectAttributes.getAction() == Action.approved) {
LOGGER.log(Level.FINEST, "Skipping LastCommitNotYetBuild check for approve action");
return true;
}
if (objectAttributes != null && objectAttributes.getLastCommit() != null) {
Run<?, ?> mergeBuild = BuildUtil.getBuildBySHA1IncludingMergeBuilds(project, objectAttributes.getLastCommit().getId());
if (mergeBuild != null && StringUtils.equals(getTargetBranchFromBuild(mergeBuild), objectAttributes.getTargetBranch())) {
@ -144,6 +157,11 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
return cause == null ? null : cause.getData().getTargetBranch();
}
private boolean isAllowedByConfig(MergeRequestObjectAttributes objectAttributes) {
return allowedStates.contains(objectAttributes.getState())
|| allowedActions.contains(objectAttributes.getAction());
}
private boolean isNotSkipWorkInProgressMergeRequest(MergeRequestObjectAttributes objectAttributes) {
Boolean workInProgress = objectAttributes.getWorkInProgress();
if (skipWorkInProgressMergeRequest && workInProgress != null && workInProgress) {

3
src/main/resources/com/dabsquared/gitlabjenkins/GitLabPushTrigger/config.jelly
View File

@ -18,6 +18,9 @@
<f:entry title="Rebuild open Merge Requests" field="triggerOpenMergeRequestOnPush">
<f:select/>
</f:entry>
<f:entry title="Approved Merge Requests (EE-only)" field="triggerOnApprovedMergeRequest">
<f:checkbox default="true"/>
</f:entry>
<f:entry title="Comments" field="triggerOnNoteRequest">
<f:checkbox default="true"/>
</f:entry>

110
src/test/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImplTest.java
View File

@ -1,6 +1,8 @@
package com.dabsquared.gitlabjenkins.trigger.handler.merge;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.Action;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.State;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.MergeRequestObjectAttributesBuilder;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterFactory;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterType;
import hudson.Launcher;
@ -10,7 +12,15 @@ import hudson.model.FreeStyleProject;
import hudson.plugins.git.GitSCM;
import hudson.util.OneShotEvent;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.ConcurrentRefUpdateException;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.api.errors.NoHeadException;
import org.eclipse.jgit.api.errors.NoMessageException;
import org.eclipse.jgit.api.errors.UnmergedPathsException;
import org.eclipse.jgit.api.errors.WrongRepositoryStateException;
import org.eclipse.jgit.errors.AmbiguousObjectException;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevCommit;
@ -23,6 +33,7 @@ import org.jvnet.hudson.test.TestBuilder;
import java.io.IOException;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.concurrent.ExecutionException;
import static com.dabsquared.gitlabjenkins.gitlab.hook.model.builder.generated.CommitBuilder.commit;
@ -107,9 +118,47 @@ public class MergeRequestHookTriggerHandlerImplTest {
assertThat(buildTriggered.isSignaled(), is(false));
}
@Test
public void mergeRequest_build_when_approved() throws IOException, InterruptedException, GitAPIException, ExecutionException {
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.noneOf(State.class), EnumSet.of(Action.approved), false);
OneShotEvent buildTriggered = doHandle(mergeRequestHookTriggerHandler, Action.approved);
assertThat(buildTriggered.isSignaled(), is(true));
}
@Test
public void mergeRequest_build_only_when_approved_and_not_when_opened() throws IOException, InterruptedException, GitAPIException, ExecutionException {
mergeRequest_build_only_when_approved(Action.open);
}
@Test
public void mergeRequest_build_only_when_approved_and_not_when_updated() throws IOException, InterruptedException, GitAPIException, ExecutionException {
mergeRequest_build_only_when_approved(Action.update);
}
private void mergeRequest_build_only_when_approved(Action action)
throws GitAPIException, IOException, InterruptedException {
MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler = new MergeRequestHookTriggerHandlerImpl(EnumSet.noneOf(State.class), EnumSet.of(Action.approved), false);
OneShotEvent buildTriggered = doHandle(mergeRequestHookTriggerHandler, action);
assertThat(buildTriggered.isSignaled(), is(false));
}
private OneShotEvent doHandle(MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler, Action action) throws GitAPIException, IOException, InterruptedException {
return doHandle(mergeRequestHookTriggerHandler, defaultMergeRequestObjectAttributes().withState(State.opened).withAction(action));
}
private OneShotEvent doHandle(MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler, State state) throws GitAPIException, IOException, InterruptedException {
Git.init().setDirectory(tmp.getRoot()).call();
return doHandle(mergeRequestHookTriggerHandler, defaultMergeRequestObjectAttributes().withState(state));
}
private OneShotEvent doHandle(MergeRequestHookTriggerHandler mergeRequestHookTriggerHandler,
MergeRequestObjectAttributesBuilder objectAttributes) throws GitAPIException, IOException, NoHeadException,
NoMessageException, UnmergedPathsException, ConcurrentRefUpdateException, WrongRepositoryStateException,
AmbiguousObjectException, IncorrectObjectTypeException, MissingObjectException, InterruptedException {
Git.init().setDirectory(tmp.getRoot()).call();
tmp.newFile("test");
Git git = Git.open(tmp.getRoot());
git.add().addFilepattern("test");
@ -128,33 +177,10 @@ public class MergeRequestHookTriggerHandlerImplTest {
}
});
project.setQuietPeriod(0);
mergeRequestHookTriggerHandler.handle(project, mergeRequestHook()
.withObjectAttributes(mergeRequestObjectAttributes()
.withTargetBranch("refs/heads/" + git.nameRev().add(head).call().get(head))
.withState(state)
.withIid(1)
.withTitle("test")
.withTargetProjectId(1)
.withSourceProjectId(1)
.withSourceBranch("feature")
.withTargetBranch("master")
.withLastCommit(commit().withAuthor(user().withName("test").build()).withId(commit.getName()).build())
.withSource(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("git@gitlab.org:test.git")
.withSshUrl("git@gitlab.org:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.build())
.withTarget(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("git@gitlab.org:test.git")
.withSshUrl("git@gitlab.org:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.build())
mergeRequestHookTriggerHandler.handle(project, mergeRequestHook()
.withObjectAttributes(objectAttributes
.withTargetBranch("refs/heads/" + git.nameRev().add(head).call().get(head))
.withLastCommit(commit().withAuthor(user().withName("test").build()).withId(commit.getName()).build())
.build())
.withProject(project()
.withWebUrl("https://gitlab.org/test.git")
@ -165,6 +191,32 @@ public class MergeRequestHookTriggerHandlerImplTest {
buildTriggered.block(10000);
return buildTriggered;
}
}
private MergeRequestObjectAttributesBuilder defaultMergeRequestObjectAttributes() {
return mergeRequestObjectAttributes()
.withIid(1)
.withTitle("test")
.withTargetProjectId(1)
.withSourceProjectId(1)
.withSourceBranch("feature")
.withTargetBranch("master")
.withSource(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("git@gitlab.org:test.git")
.withSshUrl("git@gitlab.org:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.build())
.withTarget(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("git@gitlab.org:test.git")
.withSshUrl("git@gitlab.org:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.build());
}
}