Fix filtering by label never matches anything #647

This commit is contained in:
Benjamin Robin 2018-03-24 13:24:53 +01:00
parent e022274e90
commit a8ce097f27
5 changed files with 218 additions and 32 deletions

View File

@ -6,6 +6,8 @@ import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
import java.util.List;
/**
* @author Robin Müller
*/
@ -16,6 +18,7 @@ public class MergeRequestHook extends WebHook {
private User assignee;
private Project project;
private MergeRequestObjectAttributes objectAttributes;
private List<MergeRequestLabel> labels;
public User getUser() {
return user;
@ -25,6 +28,14 @@ public class MergeRequestHook extends WebHook {
this.user = user;
}
public User getAssignee() {
return assignee;
}
public void setAssignee(User assignee) {
this.assignee = assignee;
}
public Project getProject() {
return project;
}
@ -41,15 +52,15 @@ public class MergeRequestHook extends WebHook {
this.objectAttributes = objectAttributes;
}
public User getAssignee() {
return assignee;
}
public void setAssignee(User assignee) {
this.assignee = assignee;
}
public List<MergeRequestLabel> getLabels() {
return labels;
}
@Override
public void setLabels(List<MergeRequestLabel> labels) {
this.labels = labels;
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
@ -63,6 +74,7 @@ public class MergeRequestHook extends WebHook {
.append(assignee, that.assignee)
.append(project, that.project)
.append(objectAttributes, that.objectAttributes)
.append(labels, that.labels)
.isEquals();
}
@ -73,6 +85,7 @@ public class MergeRequestHook extends WebHook {
.append(assignee)
.append(project)
.append(objectAttributes)
.append(labels)
.toHashCode();
}
@ -83,6 +96,7 @@ public class MergeRequestHook extends WebHook {
.append("assignee", assignee)
.append("project", project)
.append("objectAttributes", objectAttributes)
.append("labels", labels)
.toString();
}
}

View File

@ -0,0 +1,173 @@
package com.dabsquared.gitlabjenkins.gitlab.hook.model;
import net.karneim.pojobuilder.GeneratePojoBuilder;
import org.apache.commons.lang.builder.EqualsBuilder;
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;
import java.util.Date;
/**
* @author Benjamin ROBIN
*/
@GeneratePojoBuilder(intoPackage = "*.builder.generated", withFactoryMethod = "*")
public class MergeRequestLabel {
/*
"id": 206,
"title": "API",
"color": "#ffffff",
"project_id": 14,
"created_at": "2013-12-03T17:15:43Z",
"updated_at": "2013-12-03T17:15:43Z",
"template": false,
"description": "API related issues",
"type": "ProjectLabel",
"group_id": 41
*/
private Integer id;
private String title;
private String color;
private Integer projectId;
private Date createdAt;
private Date updatedAt;
private Boolean template;
private String description;
private String type;
private Integer groupId;
public Integer getId() {
return id;
}
public void setId(Integer id) {
this.id = id;
}
public String getTitle() {
return title;
}
public void setTitle(String title) {
this.title = title;
}
public String getColor() {
return color;
}
public void setColor(String color) {
this.color = color;
}
public Integer getProjectId() {
return projectId;
}
public void setProjectId(Integer projectId) {
this.projectId = projectId;
}
public Date getCreatedAt() {
return createdAt;
}
public void setCreatedAt(Date createdAt) {
this.createdAt = createdAt;
}
public Date getUpdatedAt() {
return updatedAt;
}
public void setUpdatedAt(Date updatedAt) {
this.updatedAt = updatedAt;
}
public Boolean getTemplate() {
return template;
}
public void setTemplate(Boolean template) {
this.template = template;
}
public String getDescription() {
return description;
}
public void setDescription(String description) {
this.description = description;
}
public String getType() {
return type;
}
public void setType(String type) {
this.type = type;
}
public Integer getGroupId() {
return groupId;
}
public void setGroupId(Integer groupId) {
this.groupId = groupId;
}
@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
MergeRequestLabel that = (MergeRequestLabel) o;
return new EqualsBuilder()
.append(id, that.id)
.append(title, that.title)
.append(color, that.color)
.append(projectId, that.projectId)
.append(createdAt, that.createdAt)
.append(updatedAt, that.updatedAt)
.append(template, that.template)
.append(description, that.description)
.append(type, that.type)
.append(groupId, that.groupId)
.isEquals();
}
@Override
public int hashCode() {
return new HashCodeBuilder(17, 37)
.append(id)
.append(title)
.append(color)
.append(projectId)
.append(createdAt)
.append(updatedAt)
.append(template)
.append(description)
.append(type)
.append(groupId)
.toHashCode();
}
@Override
public String toString() {
return new ToStringBuilder(this)
.append("id", id)
.append("title", title)
.append("color", color)
.append("projectId", projectId)
.append("createdAt", createdAt)
.append("updatedAt", updatedAt)
.append("template", template)
.append("description", description)
.append("type", type)
.append("groupId", groupId)
.toString();
}
}

View File

@ -34,7 +34,6 @@ public class MergeRequestObjectAttributes {
private String url;
private Action action;
private Boolean workInProgress;
private List<String> labels;
public Integer getId() {
return id;
@ -196,14 +195,6 @@ public class MergeRequestObjectAttributes {
this.workInProgress = workInProgress;
}
public List<String> getLabels() {
return labels;
}
public void setLabels(List<String> labels) {
this.labels = labels;
}
@Override
public boolean equals(Object o) {
if (this == o) {
@ -234,7 +225,6 @@ public class MergeRequestObjectAttributes {
.append(url, that.url)
.append(action, that.action)
.append(workInProgress, that.workInProgress)
.append(labels, that.labels)
.isEquals();
}
@ -261,7 +251,6 @@ public class MergeRequestObjectAttributes {
.append(url)
.append(action)
.append(workInProgress)
.append(labels)
.toHashCode();
}
@ -288,7 +277,6 @@ public class MergeRequestObjectAttributes {
.append("url", url)
.append("action", action)
.append("workInProgress", workInProgress)
.append("labels", labels)
.toString();
}
}

View File

@ -5,6 +5,7 @@ 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.MergeRequestLabel;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.State;
import com.dabsquared.gitlabjenkins.trigger.exception.NoRevisionToBuildException;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilter;
@ -18,6 +19,8 @@ import hudson.plugins.git.RevisionParameterAction;
import org.apache.commons.lang.StringUtils;
import java.util.Collection;
import java.util.ArrayList;
import java.util.List;
import java.util.EnumSet;
import java.util.logging.Level;
import java.util.logging.Logger;
@ -40,7 +43,7 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, boolean skipWorkInProgressMergeRequest) {
this(allowedStates, EnumSet.allOf(Action.class),skipWorkInProgressMergeRequest);
}
MergeRequestHookTriggerHandlerImpl(Collection<State> allowedStates, Collection<Action> allowedActions, boolean skipWorkInProgressMergeRequest) {
this.allowedStates = allowedStates;
this.allowedActions = allowedActions;
@ -52,9 +55,18 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
if (isAllowedByConfig(objectAttributes)
&& isLastCommitNotYetBuild(job, hook)
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)
&& mergeRequestLabelFilter.isMergeRequestAllowed(hook.getObjectAttributes().getLabels())) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)) {
List<String> labelsNames = new ArrayList<>();
if (hook.getLabels() != null) {
for (MergeRequestLabel label : hook.getLabels()) {
labelsNames.add(label.getTitle());
}
}
if (mergeRequestLabelFilter.isMergeRequestAllowed(labelsNames)) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}
}
@ -137,11 +149,11 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
private boolean isLastCommitNotYetBuild(Job<?, ?> project, MergeRequestHook hook) {
MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
if (objectAttributes.getAction() == Action.approved) {
if (objectAttributes != null && objectAttributes.getAction() == Action.approved) {
LOGGER.log(Level.FINEST, "Skipping LastCommitNotYetBuild check for approve action");
return true;
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())) {
@ -158,10 +170,10 @@ class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<M
}
private boolean isAllowedByConfig(MergeRequestObjectAttributes objectAttributes) {
return allowedStates.contains(objectAttributes.getState())
return allowedStates.contains(objectAttributes.getState())
&& allowedActions.contains(objectAttributes.getAction());
}
}
private boolean isNotSkipWorkInProgressMergeRequest(MergeRequestObjectAttributes objectAttributes) {
Boolean workInProgress = objectAttributes.getWorkInProgress();
if (skipWorkInProgressMergeRequest && workInProgress != null && workInProgress) {

View File

@ -32,8 +32,7 @@ class NoteHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<NoteHook>
@Override
public void handle(Job<?, ?> job, NoteHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
if (isValidTriggerPhrase(hook.getObjectAttributes().getNote())
&& mergeRequestLabelFilter.isMergeRequestAllowed(hook.getMergeRequest().getLabels())) {
if (isValidTriggerPhrase(hook.getObjectAttributes().getNote())) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}