Skip to content

Commit

Permalink
Merge pull request #732 from benjarobin/fix-mr-filter-by-label
Browse files Browse the repository at this point in the history
Fix filtering by label never matches anything. Fixes #647.
  • Loading branch information
omehegan authored Apr 4, 2018
2 parents 9d8f7cc + a8ce097 commit 6acfad1
Show file tree
Hide file tree
Showing 5 changed files with 219 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import org.apache.commons.lang.builder.HashCodeBuilder;
import org.apache.commons.lang.builder.ToStringBuilder;

import java.util.List;

/**
* @author Robin Müller
*/
Expand All @@ -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;
Expand All @@ -25,6 +28,14 @@ public void setUser(User user) {
this.user = user;
}

public User getAssignee() {
return assignee;
}

public void setAssignee(User assignee) {
this.assignee = assignee;
}

public Project getProject() {
return project;
}
Expand All @@ -41,15 +52,15 @@ public void setObjectAttributes(MergeRequestObjectAttributes objectAttributes) {
this.objectAttributes = objectAttributes;
}

public User getAssignee() {
return assignee;
}
public void setAssignee(User assignee) {
this.assignee = assignee;
}

@Override
public List<MergeRequestLabel> getLabels() {
return labels;
}

public void setLabels(List<MergeRequestLabel> labels) {
this.labels = labels;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
Expand All @@ -63,6 +74,7 @@ public boolean equals(Object o) {
.append(assignee, that.assignee)
.append(project, that.project)
.append(objectAttributes, that.objectAttributes)
.append(labels, that.labels)
.isEquals();
}

Expand All @@ -73,6 +85,7 @@ public int hashCode() {
.append(assignee)
.append(project)
.append(objectAttributes)
.append(labels)
.toHashCode();
}

Expand All @@ -83,6 +96,7 @@ public String toString() {
.append("assignee", assignee)
.append("project", project)
.append("objectAttributes", objectAttributes)
.append("labels", labels)
.toString();
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -196,14 +195,6 @@ public void setWorkInProgress(Boolean workInProgress) {
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) {
Expand Down Expand Up @@ -234,7 +225,6 @@ public boolean equals(Object o) {
.append(url, that.url)
.append(action, that.action)
.append(workInProgress, that.workInProgress)
.append(labels, that.labels)
.isEquals();
}

Expand All @@ -261,7 +251,6 @@ public int hashCode() {
.append(url)
.append(action)
.append(workInProgress)
.append(labels)
.toHashCode();
}

Expand All @@ -288,7 +277,6 @@ public String toString() {
.append("url", url)
.append("action", action)
.append("workInProgress", workInProgress)
.append("labels", labels)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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;
Expand All @@ -18,6 +19,8 @@
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;
Expand All @@ -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;
Expand All @@ -52,9 +55,18 @@ public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchF
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);
}
}
}

Expand Down Expand Up @@ -137,11 +149,11 @@ private String retrieveRevisionToBuild(MergeRequestHook hook) throws NoRevisionT

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())) {
Expand All @@ -158,10 +170,10 @@ private String getTargetBranchFromBuild(Run<?, ?> mergeBuild) {
}

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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 6acfad1

Please sign in to comment.