Merge pull request #732 from benjarobin/fix-mr-filter-by-label

Fix filtering by label never matches anything. Fixes #647.

src/main/java/com/dabsquared/gitlabjenkins/gitlab/hook/model/MergeRequestHook.java

@@ -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() {
+    public List<MergeRequestLabel> getLabels() {
-		return assignee;
+        return labels;
-	}
+    }
-    
-	public void setAssignee(User assignee) {
-		this.assignee = assignee;
-	}
 
-	@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();
     }
 }

src/main/java/com/dabsquared/gitlabjenkins/gitlab/hook/model/MergeRequestLabel.java

@@ -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();
+    }
+}

src/main/java/com/dabsquared/gitlabjenkins/gitlab/hook/model/MergeRequestObjectAttributes.java

@@ -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();
     }
 }

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

@@ -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)
+            && isNotSkipWorkInProgressMergeRequest(objectAttributes)) {
-            && mergeRequestLabelFilter.isMergeRequestAllowed(hook.getObjectAttributes().getLabels())) {
+
-            super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
+            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) {

src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/note/NoteHookTriggerHandlerImpl.java

@@ -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())
+        if (isValidTriggerPhrase(hook.getObjectAttributes().getNote())) {
-            && mergeRequestLabelFilter.isMergeRequestAllowed(hook.getMergeRequest().getLabels())) {
             super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
         }
     }