diff options
18 files changed, 206 insertions, 97 deletions
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueHandlers.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueHandlers.java index 5193e2c4e1a..8d972013fcd 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueHandlers.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueHandlers.java @@ -24,7 +24,9 @@ import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueHandler; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.user.User; import org.sonar.core.issue.IssueUpdater; +import org.sonar.core.user.DefaultUser; import javax.annotation.Nullable; @@ -114,7 +116,13 @@ public class IssueHandlers implements BatchExtension { @Override public IssueHandler.Context assign(@Nullable String assignee) { - updater.assign(issue, assignee, changeContext); + updater.assign(issue, new DefaultUser().setLogin(assignee).setName(assignee), changeContext); + return this; + } + + @Override + public IssueHandler.Context assign(@Nullable User user) { + updater.assign(issue, user, changeContext); return this; } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java index 67dd24c8f97..1262c9edf0e 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java @@ -24,12 +24,13 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.time.DateUtils; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; +import org.sonar.api.issue.ActionPlan; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.DefaultIssueComment; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.user.User; import javax.annotation.Nullable; - import java.util.Calendar; import java.util.Date; @@ -38,12 +39,20 @@ import java.util.Date; */ public class IssueUpdater implements BatchComponent, ServerComponent { + public static final String UNUSED = ""; + public static final String SEVERITY = "severity"; + public static final String ASSIGNEE = "assignee"; + public static final String RESOLUTION = "resolution"; + public static final String STATUS = "status"; + public static final String AUTHOR = "author"; + public static final String ACTION_PLAN = "actionPlan"; + public boolean setSeverity(DefaultIssue issue, String severity, IssueChangeContext context) { if (issue.manualSeverity()) { throw new IllegalStateException("Severity can't be changed"); } if (!Objects.equal(severity, issue.severity())) { - issue.setFieldChange(context, "severity", issue.severity(), severity); + issue.setFieldChange(context, SEVERITY, issue.severity(), severity); issue.setSeverity(severity); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -60,7 +69,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setManualSeverity(DefaultIssue issue, String severity, IssueChangeContext context) { if (!issue.manualSeverity() || !Objects.equal(severity, issue.severity())) { - issue.setFieldChange(context, "severity", issue.severity(), severity); + issue.setFieldChange(context, SEVERITY, issue.severity(), severity); issue.setSeverity(severity); issue.setManualSeverity(true); issue.setUpdateDate(context.date()); @@ -71,10 +80,14 @@ public class IssueUpdater implements BatchComponent, ServerComponent { return false; } - public boolean assign(DefaultIssue issue, @Nullable String assignee, IssueChangeContext context) { - String sanitizedAssignee = StringUtils.defaultIfBlank(assignee, null); + public boolean assign(DefaultIssue issue, @Nullable User user, IssueChangeContext context) { + String sanitizedAssignee = null; + if(user != null) { + sanitizedAssignee = StringUtils.defaultIfBlank(user.login(), null); + } if (!Objects.equal(sanitizedAssignee, issue.assignee())) { - issue.setFieldChange(context, "assignee", issue.assignee(), sanitizedAssignee); + String newAssignee = user != null ? user.name() : null; + issue.setFieldChange(context, ASSIGNEE, UNUSED, newAssignee); issue.setAssignee(sanitizedAssignee); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -101,7 +114,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setResolution(DefaultIssue issue, @Nullable String resolution, IssueChangeContext context) { if (!Objects.equal(resolution, issue.resolution())) { - issue.setFieldChange(context, "resolution", issue.resolution(), resolution); + issue.setFieldChange(context, RESOLUTION, issue.resolution(), resolution); issue.setResolution(resolution); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -113,7 +126,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setStatus(DefaultIssue issue, String status, IssueChangeContext context) { if (!Objects.equal(status, issue.status())) { - issue.setFieldChange(context, "status", issue.status(), status); + issue.setFieldChange(context, STATUS, issue.status(), status); issue.setStatus(status); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -125,7 +138,7 @@ public class IssueUpdater implements BatchComponent, ServerComponent { public boolean setAuthorLogin(DefaultIssue issue, @Nullable String authorLogin, IssueChangeContext context) { if (!Objects.equal(authorLogin, issue.authorLogin())) { - issue.setFieldChange(context, "author", issue.authorLogin(), authorLogin); + issue.setFieldChange(context, AUTHOR, issue.authorLogin(), authorLogin); issue.setAuthorLogin(authorLogin); issue.setUpdateDate(context.date()); issue.setChanged(true); @@ -196,10 +209,14 @@ public class IssueUpdater implements BatchComponent, ServerComponent { return false; } - public boolean plan(DefaultIssue issue, @Nullable String actionPlanKey, IssueChangeContext context) { - String sanitizedActionPlanKey = StringUtils.defaultIfBlank(actionPlanKey, null); + public boolean plan(DefaultIssue issue, @Nullable ActionPlan actionPlan, IssueChangeContext context) { + String sanitizedActionPlanKey = null; + if(actionPlan != null) { + sanitizedActionPlanKey = StringUtils.defaultIfBlank(actionPlan.key(), null); + } if (!Objects.equal(sanitizedActionPlanKey, issue.actionPlanKey())) { - issue.setFieldChange(context, "actionPlanKey", issue.actionPlanKey(), sanitizedActionPlanKey); + String newActionPlanName = actionPlan != null ? actionPlan.name() : null; + issue.setFieldChange(context, ACTION_PLAN, UNUSED, newActionPlanName); issue.setActionPlanKey(sanitizedActionPlanKey); issue.setUpdateDate(context.date()); issue.setChanged(true); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/Function.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/Function.java index 556d5ac962a..cd43e7c3b1c 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/Function.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/Function.java @@ -20,13 +20,14 @@ package org.sonar.core.issue.workflow; import org.sonar.api.issue.Issue; +import org.sonar.api.user.User; import javax.annotation.Nullable; interface Function { interface Context { Issue issue(); - Context setAssignee(@Nullable String s); + Context setAssignee(@Nullable User user); Context setResolution(@Nullable String s); Context setCloseDate(boolean b); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/FunctionExecutor.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/FunctionExecutor.java index b5cc6b9b566..27cf7eeb5c1 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/FunctionExecutor.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/FunctionExecutor.java @@ -24,6 +24,7 @@ import org.sonar.api.ServerComponent; import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.user.User; import org.sonar.core.issue.IssueUpdater; import javax.annotation.Nullable; @@ -62,8 +63,8 @@ public class FunctionExecutor implements BatchComponent, ServerComponent { } @Override - public Function.Context setAssignee(@Nullable String s) { - updater.assign(issue, s, changeContext); + public Function.Context setAssignee(@Nullable User user) { + updater.assign(issue, user, changeContext); return this; } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetAssignee.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetAssignee.java index a9116a6313d..03db62ffbcb 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetAssignee.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetAssignee.java @@ -19,14 +19,16 @@ */ package org.sonar.core.issue.workflow; +import org.sonar.api.user.User; + import javax.annotation.Nullable; public class SetAssignee implements Function { public static final SetAssignee UNASSIGN = new SetAssignee(null); - private final String assignee; + private final User assignee; - public SetAssignee(@Nullable String assignee) { + public SetAssignee(@Nullable User assignee) { this.assignee = assignee; } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java index 63f7ac1483e..1d68e98cc39 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java @@ -20,13 +20,17 @@ package org.sonar.core.issue; import org.junit.Test; +import org.sonar.api.issue.ActionPlan; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.FieldDiffs; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.user.User; +import org.sonar.core.user.DefaultUser; import java.util.Date; import static org.fest.assertions.Assertions.assertThat; +import static org.sonar.core.issue.IssueUpdater.*; public class IssueUpdaterTest { @@ -36,13 +40,15 @@ public class IssueUpdaterTest { @Test public void should_assign() throws Exception { - boolean updated = updater.assign(issue, "emmerik", context); + User user = new DefaultUser().setLogin("emmerik").setName("Emmerik"); + + boolean updated = updater.assign(issue, user, context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); assertThat(issue.mustSendNotifications()).isTrue(); - FieldDiffs.Diff diff = issue.currentChange().get("assignee"); - assertThat(diff.oldValue()).isNull(); - assertThat(diff.newValue()).isEqualTo("emmerik"); + FieldDiffs.Diff diff = issue.currentChange().get(ASSIGNEE); + assertThat(diff.oldValue()).isEqualTo(UNUSED); + assertThat(diff.newValue()).isEqualTo("Emmerik"); } @Test @@ -52,33 +58,36 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.assignee()).isNull(); assertThat(issue.mustSendNotifications()).isTrue(); - FieldDiffs.Diff diff = issue.currentChange().get("assignee"); - assertThat(diff.oldValue()).isEqualTo("morgan"); + FieldDiffs.Diff diff = issue.currentChange().get(ASSIGNEE); + assertThat(diff.oldValue()).isEqualTo(UNUSED); assertThat(diff.newValue()).isNull(); } @Test public void should_change_assignee() throws Exception { + User user = new DefaultUser().setLogin("emmerik").setName("Emmerik"); + issue.setAssignee("morgan"); - boolean updated = updater.assign(issue, "emmerik", context); + boolean updated = updater.assign(issue, user, context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("emmerik"); assertThat(issue.mustSendNotifications()).isTrue(); - FieldDiffs.Diff diff = issue.currentChange().get("assignee"); - assertThat(diff.oldValue()).isEqualTo("morgan"); - assertThat(diff.newValue()).isEqualTo("emmerik"); + FieldDiffs.Diff diff = issue.currentChange().get(ASSIGNEE); + assertThat(diff.oldValue()).isEqualTo(UNUSED); + assertThat(diff.newValue()).isEqualTo("Emmerik"); } @Test public void should_not_change_assignee() throws Exception { + User user = new DefaultUser().setLogin("morgan").setName("Morgan"); + issue.setAssignee("morgan"); - boolean updated = updater.assign(issue, "morgan", context); + boolean updated = updater.assign(issue, user, context); assertThat(updated).isFalse(); assertThat(issue.currentChange()).isNull(); assertThat(issue.mustSendNotifications()).isFalse(); } - @Test public void should_set_severity() throws Exception { boolean updated = updater.setSeverity(issue, "BLOCKER", context); @@ -87,7 +96,7 @@ public class IssueUpdaterTest { assertThat(issue.manualSeverity()).isFalse(); assertThat(issue.mustSendNotifications()).isFalse(); - FieldDiffs.Diff diff = issue.currentChange().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get(SEVERITY); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("BLOCKER"); } @@ -100,7 +109,7 @@ public class IssueUpdaterTest { assertThat(issue.severity()).isEqualTo("BLOCKER"); assertThat(issue.mustSendNotifications()).isFalse(); - FieldDiffs.Diff diff = issue.currentChange().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get(SEVERITY); assertThat(diff.oldValue()).isEqualTo("INFO"); assertThat(diff.newValue()).isEqualTo("BLOCKER"); } @@ -113,7 +122,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.severity()).isEqualTo("MINOR"); assertThat(issue.mustSendNotifications()).isFalse(); - FieldDiffs.Diff diff = issue.currentChange().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get(SEVERITY); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); } @@ -146,7 +155,7 @@ public class IssueUpdaterTest { assertThat(issue.severity()).isEqualTo("MINOR"); assertThat(issue.manualSeverity()).isTrue(); assertThat(issue.mustSendNotifications()).isTrue(); - FieldDiffs.Diff diff = issue.currentChange().get("severity"); + FieldDiffs.Diff diff = issue.currentChange().get(SEVERITY); assertThat(diff.oldValue()).isEqualTo("BLOCKER"); assertThat(diff.newValue()).isEqualTo("MINOR"); } @@ -198,7 +207,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.resolution()).isEqualTo("OPEN"); - FieldDiffs.Diff diff = issue.currentChange().get("resolution"); + FieldDiffs.Diff diff = issue.currentChange().get(RESOLUTION); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); assertThat(issue.mustSendNotifications()).isTrue(); @@ -220,7 +229,7 @@ public class IssueUpdaterTest { assertThat(updated).isTrue(); assertThat(issue.status()).isEqualTo("OPEN"); - FieldDiffs.Diff diff = issue.currentChange().get("status"); + FieldDiffs.Diff diff = issue.currentChange().get(STATUS); assertThat(diff.oldValue()).isNull(); assertThat(diff.newValue()).isEqualTo("OPEN"); assertThat(issue.mustSendNotifications()).isTrue(); @@ -268,18 +277,62 @@ public class IssueUpdaterTest { } @Test - public void should_plan() throws Exception { - boolean updated = updater.plan(issue, "ABCD", context); + public void should_plan_with_no_existing_plan() throws Exception { + ActionPlan newActionPlan = DefaultActionPlan.create("newName"); + + boolean updated = updater.plan(issue, newActionPlan, context); assertThat(updated).isTrue(); - assertThat(issue.actionPlanKey()).isEqualTo("ABCD"); + assertThat(issue.actionPlanKey()).isEqualTo(newActionPlan.key()); - FieldDiffs.Diff diff = issue.currentChange().get("actionPlanKey"); - assertThat(diff.oldValue()).isNull(); - assertThat(diff.newValue()).isEqualTo("ABCD"); + FieldDiffs.Diff diff = issue.currentChange().get(ACTION_PLAN); + assertThat(diff.oldValue()).isEqualTo(UNUSED); + assertThat(diff.newValue()).isEqualTo("newName"); + assertThat(issue.mustSendNotifications()).isTrue(); + } + + @Test + public void should_plan_with_existing_plan() throws Exception { + issue.setActionPlanKey("formerActionPlan"); + + ActionPlan newActionPlan = DefaultActionPlan.create("newName").setKey("newKey"); + + boolean updated = updater.plan(issue, newActionPlan, context); + assertThat(updated).isTrue(); + assertThat(issue.actionPlanKey()).isEqualTo(newActionPlan.key()); + + FieldDiffs.Diff diff = issue.currentChange().get(ACTION_PLAN); + assertThat(diff.oldValue()).isEqualTo(UNUSED); + assertThat(diff.newValue()).isEqualTo("newName"); assertThat(issue.mustSendNotifications()).isTrue(); } @Test + public void should_unplan() throws Exception { + issue.setActionPlanKey("formerActionPlan"); + + boolean updated = updater.plan(issue, null, context); + assertThat(updated).isTrue(); + assertThat(issue.actionPlanKey()).isNull(); + + FieldDiffs.Diff diff = issue.currentChange().get(ACTION_PLAN); + assertThat(diff.oldValue()).isEqualTo(UNUSED); + assertThat(diff.newValue()).isNull(); + assertThat(issue.mustSendNotifications()).isTrue(); + } + + @Test + public void should_not_plan_again() throws Exception { + issue.setActionPlanKey("existingActionPlan"); + + ActionPlan newActionPlan = DefaultActionPlan.create("existingActionPlan").setKey("existingActionPlan"); + + boolean updated = updater.plan(issue, newActionPlan, context); + assertThat(updated).isFalse(); + assertThat(issue.currentChange()).isNull(); + assertThat(issue.mustSendNotifications()).isFalse(); + } + + @Test public void should_set_effort_to_fix() throws Exception { boolean updated = updater.setEffortToFix(issue, 3.14, context); assertThat(updated).isTrue(); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetAssigneeTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetAssigneeTest.java index 418cc81324a..07408cf8896 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetAssigneeTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetAssigneeTest.java @@ -21,16 +21,19 @@ package org.sonar.core.issue.workflow; import org.junit.Test; +import org.sonar.api.user.User; +import org.sonar.core.user.DefaultUser; import static org.mockito.Mockito.*; public class SetAssigneeTest { @Test public void assign() throws Exception { - SetAssignee function = new SetAssignee("eric"); + User user = new DefaultUser().setLogin("eric").setName("eric"); + SetAssignee function = new SetAssignee(user); Function.Context context = mock(Function.Context.class); function.execute(context); - verify(context, times(1)).setAssignee("eric"); + verify(context, times(1)).setAssignee(user); } @Test diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueHandler.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueHandler.java index 12f4a5c3fdd..1a59ccc8fd9 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueHandler.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueHandler.java @@ -20,6 +20,7 @@ package org.sonar.api.issue; import org.sonar.api.BatchExtension; +import org.sonar.api.user.User; import javax.annotation.Nullable; @@ -47,8 +48,14 @@ public interface IssueHandler extends BatchExtension { Context setAttribute(String key, @Nullable String value); + /** + * @deprecated since 4.0 + */ + @Deprecated Context assign(@Nullable String login); + Context assign(@Nullable User user); + Context addComment(String text); } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java index 9d954cbd77a..386de94e09e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/internal/FieldDiffs.java @@ -30,7 +30,7 @@ import java.util.Date; import java.util.Map; /** - * PLUGINS MUST NOT BE USED THIS CLASS, EXCEPT FOR UNIT TESTING. + * PLUGINS MUST NOT USE THIS CLASS, EXCEPT FOR UNIT TESTING. * * @since 3.6 */ @@ -118,13 +118,15 @@ public class FieldDiffs implements Serializable { String[] values = keyValues[1].split("\\|"); String oldValue = ""; String newValue = ""; - if (values.length > 0) { + if(values.length == 1) { + newValue = Strings.nullToEmpty(values[0]); + } else if(values.length == 2) { oldValue = Strings.nullToEmpty(values[0]); - } - if (values.length > 1) { newValue = Strings.nullToEmpty(values[1]); } diffs.setDiff(keyValues[0], oldValue, newValue); + } else { + diffs.setDiff(keyValues[0], "", ""); } } } @@ -155,11 +157,11 @@ public class FieldDiffs implements Serializable { public String toString() { //TODO escape , and | characters StringBuilder sb = new StringBuilder(); - if (oldValue != null) { - sb.append(oldValue.toString()); - } - sb.append('|'); - if (newValue != null) { + if(newValue != null) { + if(oldValue != null) { + sb.append(oldValue.toString()); + sb.append('|'); + } sb.append(newValue.toString()); } return sb.toString(); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/FieldDiffsTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/FieldDiffsTest.java index e8faaa5821d..e6f31fafb69 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/FieldDiffsTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/internal/FieldDiffsTest.java @@ -68,9 +68,9 @@ public class FieldDiffsTest { @Test public void test_toString_with_null_values() throws Exception { diffs.setDiff("severity", null, "INFO"); - diffs.setDiff("resolution", "OPEN", null); + diffs.setDiff("assignee", "user1", null); - assertThat(diffs.toString()).isEqualTo("severity=|INFO,resolution=OPEN|"); + assertThat(diffs.toString()).isEqualTo("severity=INFO,assignee="); } @Test @@ -89,7 +89,7 @@ public class FieldDiffsTest { @Test public void test_parse_empty_values() throws Exception { - diffs = FieldDiffs.parse("severity=|INFO,resolution=OPEN|"); + diffs = FieldDiffs.parse("severity=INFO,resolution="); assertThat(diffs.diffs()).hasSize(2); FieldDiffs.Diff diff = diffs.diffs().get("severity"); @@ -97,7 +97,7 @@ public class FieldDiffsTest { assertThat(diff.newValue()).isEqualTo("INFO"); diff = diffs.diffs().get("resolution"); - assertThat(diff.oldValue()).isEqualTo("OPEN"); + assertThat(diff.oldValue()).isEqualTo(""); assertThat(diff.newValue()).isEqualTo(""); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java index 61995806499..735cfed12e7 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java @@ -25,6 +25,7 @@ import org.sonar.api.ServerComponent; import org.sonar.api.issue.Issue; import org.sonar.api.issue.condition.IsUnResolved; import org.sonar.api.issue.internal.DefaultIssue; +import org.sonar.api.user.User; import org.sonar.api.user.UserFinder; import org.sonar.core.issue.IssueUpdater; import org.sonar.server.user.UserSession; @@ -58,7 +59,8 @@ public class AssignAction extends Action implements ServerComponent { @Override public boolean execute(Map<String, Object> properties, Context context) { - return issueUpdater.assign((DefaultIssue) context.issue(), assignee(properties), context.issueChangeContext()); + User user = userFinder.findByLogin(assignee(properties)); + return issueUpdater.assign((DefaultIssue) context.issue(), user, context.issueChangeContext()); } private String assignee(Map<String, Object> properties){ diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java index 809538a6f57..fa6d08b1fce 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueChangelogService.java @@ -57,6 +57,5 @@ public class IssueChangelogService implements ServerComponent { Collection<User> users = userFinder.findByLogins(logins); return new IssueChangelog(changes, users); - } } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index 13e980527af..c14ff25cf1e 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -21,6 +21,7 @@ package org.sonar.server.issue; import com.google.common.base.Strings; import org.sonar.api.ServerComponent; +import org.sonar.api.issue.ActionPlan; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.IssueQueryResult; @@ -28,6 +29,7 @@ import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; +import org.sonar.api.user.User; import org.sonar.api.user.UserFinder; import org.sonar.api.web.UserRole; import org.sonar.core.issue.IssueNotifications; @@ -42,7 +44,6 @@ import org.sonar.core.user.AuthorizationDao; import org.sonar.server.user.UserSession; import javax.annotation.Nullable; - import java.util.Arrays; import java.util.Collections; import java.util.Date; @@ -123,11 +124,15 @@ public class IssueService implements ServerComponent { verifyLoggedIn(userSession); IssueQueryResult queryResult = loadIssue(issueKey); DefaultIssue issue = (DefaultIssue) queryResult.first(); - if (assignee != null && userFinder.findByLogin(assignee) == null) { - throw new IllegalArgumentException("Unknown user: " + assignee); + User user = null; + if (!Strings.isNullOrEmpty(assignee)) { + user = userFinder.findByLogin(assignee); + if(user == null) { + throw new IllegalArgumentException("Unknown user: " + assignee); + } } IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login()); - if (issueUpdater.assign(issue, assignee, context)) { + if (issueUpdater.assign(issue, user, context)) { issueStorage.save(issue); issueNotifications.sendChanges(issue, context, queryResult); } @@ -136,14 +141,18 @@ public class IssueService implements ServerComponent { public Issue plan(String issueKey, @Nullable String actionPlanKey, UserSession userSession) { verifyLoggedIn(userSession); - if (!Strings.isNullOrEmpty(actionPlanKey) && actionPlanService.findByKey(actionPlanKey, userSession) == null) { - throw new IllegalArgumentException("Unknown action plan: " + actionPlanKey); + ActionPlan actionPlan = null; + if (!Strings.isNullOrEmpty(actionPlanKey)) { + actionPlan = actionPlanService.findByKey(actionPlanKey, userSession); + if(actionPlan == null) { + throw new IllegalArgumentException("Unknown action plan: " + actionPlanKey); + } } IssueQueryResult queryResult = loadIssue(issueKey); DefaultIssue issue = (DefaultIssue) queryResult.first(); IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.login()); - if (issueUpdater.plan(issue, actionPlanKey, context)) { + if (issueUpdater.plan(issue, actionPlan, context)) { issueStorage.save(issue); issueNotifications.sendChanges(issue, context, queryResult); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java index 27fc90746ab..b357474775e 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/PlanAction.java @@ -62,7 +62,8 @@ public class PlanAction extends Action implements ServerComponent { @Override public boolean execute(Map<String, Object> properties, Context context) { - return issueUpdater.plan((DefaultIssue) context.issue(), planKey(properties), context.issueChangeContext()); + ActionPlan actionPlan = actionPlanService.findByKey(planKey(properties), UserSession.get()); + return issueUpdater.plan((DefaultIssue) context.issue(), actionPlan, context.issueChangeContext()); } private String planKey(Map<String, Object> properties) { diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ActionPlanServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ActionPlanServiceTest.java index f27f8155f62..d49ba739016 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/ActionPlanServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/ActionPlanServiceTest.java @@ -134,13 +134,13 @@ public class ActionPlanServiceTest { IssueDto issueDto = new IssueDto().setId(100L).setStatus(Issue.STATUS_OPEN).setRuleKey_unit_test_only("squid", "s100"); when(issueDao.selectIssues(any(IssueQuery.class))).thenReturn(newArrayList(issueDto)); - when(issueUpdater.plan(any(DefaultIssue.class), eq((String) null), any(IssueChangeContext.class))).thenReturn(true); + when(issueUpdater.plan(any(DefaultIssue.class), eq((ActionPlan) null), any(IssueChangeContext.class))).thenReturn(true); ArgumentCaptor<DefaultIssue> captor = ArgumentCaptor.forClass(DefaultIssue.class); actionPlanService.delete("ABCD", userSession); verify(actionPlanDao).delete("ABCD"); verify(authorizationDao).isAuthorizedComponentId(anyLong(), anyInt(), eq(UserRole.ADMIN)); - verify(issueUpdater).plan(captor.capture(), eq((String) null), any(IssueChangeContext.class)); + verify(issueUpdater).plan(captor.capture(), eq((ActionPlan) null), any(IssueChangeContext.class)); verify(issueStorage).save(newArrayList(captor.getAllValues())); } diff --git a/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java index 3a9cec4ac86..7c5d7d43bb0 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.user.User; import org.sonar.api.user.UserFinder; import org.sonar.core.issue.IssueUpdater; import org.sonar.core.user.DefaultUser; @@ -63,8 +64,11 @@ public class AssignActionTest { Action.Context context = mock(Action.Context.class); when(context.issue()).thenReturn(issue); + User user = new DefaultUser(); + when(userFinder.findByLogin(assignee)).thenReturn(user); + action.execute(properties, context); - verify(issueUpdater).assign(eq(issue), eq(assignee), any(IssueChangeContext.class)); + verify(issueUpdater).assign(eq(issue), eq(user), any(IssueChangeContext.class)); } @Test diff --git a/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java b/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java index d1c001141c9..dce2dd5eeac 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceTest.java @@ -23,6 +23,7 @@ package org.sonar.server.issue; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.sonar.api.issue.ActionPlan; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.IssueQueryResult; @@ -31,6 +32,7 @@ import org.sonar.api.issue.internal.IssueChangeContext; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; +import org.sonar.api.user.User; import org.sonar.api.user.UserFinder; import org.sonar.api.web.UserRole; import org.sonar.core.issue.DefaultActionPlan; @@ -180,15 +182,16 @@ public class IssueServiceTest { @Test public void should_assign() { String assignee = "perceval"; + User user = new DefaultUser(); - when(userFinder.findByLogin(assignee)).thenReturn(new DefaultUser()); - when(issueUpdater.assign(eq(issue), eq(assignee), any(IssueChangeContext.class))).thenReturn(true); + when(userFinder.findByLogin(assignee)).thenReturn(user); + when(issueUpdater.assign(eq(issue), eq(user), any(IssueChangeContext.class))).thenReturn(true); Issue result = issueService.assign("ABCD", assignee, userSession); assertThat(result).isNotNull(); ArgumentCaptor<IssueChangeContext> measureCaptor = ArgumentCaptor.forClass(IssueChangeContext.class); - verify(issueUpdater).assign(eq(issue), eq(assignee), measureCaptor.capture()); + verify(issueUpdater).assign(eq(issue), eq(user), measureCaptor.capture()); verify(issueStorage).save(issue); IssueChangeContext issueChangeContext = measureCaptor.getValue(); @@ -201,13 +204,15 @@ public class IssueServiceTest { @Test public void should_not_assign() { String assignee = "perceval"; + User user = new DefaultUser(); - when(userFinder.findByLogin(assignee)).thenReturn(new DefaultUser()); - when(issueUpdater.assign(eq(issue), eq(assignee), any(IssueChangeContext.class))).thenReturn(false); + when(userFinder.findByLogin(assignee)).thenReturn(user); + when(issueUpdater.assign(eq(issue), eq(user), any(IssueChangeContext.class))).thenReturn(false); Issue result = issueService.assign("ABCD", assignee, userSession); assertThat(result).isNotNull(); - verify(issueUpdater).assign(eq(issue), eq(assignee),any(IssueChangeContext.class)); + + verify(issueUpdater).assign(eq(issue), eq(user), any(IssueChangeContext.class)); verifyZeroInteractions(issueStorage); verifyZeroInteractions(issueNotifications); } @@ -234,14 +239,16 @@ public class IssueServiceTest { public void should_plan() { String actionPlanKey = "EFGH"; - when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(new DefaultActionPlan()); - when(issueUpdater.plan(eq(issue), eq(actionPlanKey), any(IssueChangeContext.class))).thenReturn(true); + ActionPlan actionPlan = new DefaultActionPlan(); + + when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(actionPlan); + when(issueUpdater.plan(eq(issue), eq(actionPlan), any(IssueChangeContext.class))).thenReturn(true); Issue result = issueService.plan("ABCD", actionPlanKey, userSession); assertThat(result).isNotNull(); ArgumentCaptor<IssueChangeContext> measureCaptor = ArgumentCaptor.forClass(IssueChangeContext.class); - verify(issueUpdater).plan(eq(issue), eq(actionPlanKey), measureCaptor.capture()); + verify(issueUpdater).plan(eq(issue), eq(actionPlan), measureCaptor.capture()); verify(issueStorage).save(issue); IssueChangeContext issueChangeContext = measureCaptor.getValue(); @@ -255,12 +262,14 @@ public class IssueServiceTest { public void should_not_plan() { String actionPlanKey = "EFGH"; - when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(new DefaultActionPlan()); - when(issueUpdater.plan(eq(issue), eq(actionPlanKey), any(IssueChangeContext.class))).thenReturn(false); + ActionPlan actionPlan = new DefaultActionPlan(); + + when(actionPlanService.findByKey(actionPlanKey, userSession)).thenReturn(actionPlan); + when(issueUpdater.plan(eq(issue), eq(actionPlan), any(IssueChangeContext.class))).thenReturn(false); Issue result = issueService.plan("ABCD", actionPlanKey, userSession); assertThat(result).isNotNull(); - verify(issueUpdater).plan(eq(issue), eq(actionPlanKey), any(IssueChangeContext.class)); + verify(issueUpdater).plan(eq(issue), eq(actionPlan), any(IssueChangeContext.class)); verifyZeroInteractions(issueStorage); verifyZeroInteractions(issueNotifications); } diff --git a/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java b/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java index f524b7b12e6..a1a5fcded8d 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/PlanActionTest.java @@ -22,6 +22,7 @@ package org.sonar.server.issue; import org.junit.Before; import org.junit.Test; +import org.sonar.api.issue.ActionPlan; import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; @@ -61,33 +62,23 @@ public class PlanActionTest { Action.Context context = mock(Action.Context.class); when(context.issue()).thenReturn(issue); - action.execute(properties, context); - verify(issueUpdater).plan(eq(issue), eq(planKey), any(IssueChangeContext.class)); - } - - @Test - public void should_execute_on_null_action_plan(){ - Map<String, Object> properties = newHashMap(); - DefaultIssue issue = mock(DefaultIssue.class); - - Action.Context context = mock(Action.Context.class); - when(context.issue()).thenReturn(issue); + ActionPlan actionPlan = new DefaultActionPlan(); + when(actionPlanService.findByKey(eq(planKey), any(UserSession.class))).thenReturn(actionPlan); action.execute(properties, context); - verify(issueUpdater).plan(eq(issue), eq((String) null), any(IssueChangeContext.class)); + verify(issueUpdater).plan(eq(issue), eq(actionPlan), any(IssueChangeContext.class)); } @Test - public void should_execute_on_empty_action_plan(){ + public void should_execute_on_null_action_plan(){ Map<String, Object> properties = newHashMap(); - properties.put("plan", ""); DefaultIssue issue = mock(DefaultIssue.class); Action.Context context = mock(Action.Context.class); when(context.issue()).thenReturn(issue); action.execute(properties, context); - verify(issueUpdater).plan(eq(issue), eq(""), any(IssueChangeContext.class)); + verify(issueUpdater).plan(eq(issue), eq((ActionPlan) null), any(IssueChangeContext.class)); } @Test |