From 0d43c9230e6f70849028cb81ff056531eb68e918 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Tue, 23 Apr 2013 01:25:40 +0200 Subject: [PATCH] SONAR-3755 complete issue workflow --- .../org/sonar/plugins/core/CorePlugin.java | 10 +- ...rator.java => IssueTrackingDecorator.java} | 54 +++---- ...t.java => IssueTrackingDecoratorTest.java} | 36 ++--- .../sonar/batch/issue/DefaultIssuable.java | 10 +- .../batch/issue/DeprecatedViolations.java | 1 + .../batch/issue/InitialOpenIssuesSensor.java | 8 +- .../batch/issue/InitialOpenIssuesStack.java | 12 +- .../sonar/batch/issue/IssuableFactory.java | 15 +- ...rator.java => IssueCountersDecorator.java} | 7 +- .../org/sonar/batch/issue/IssuePersister.java | 3 + .../sonar/batch/issue/ScanIssueChanges.java | 61 -------- .../{ModuleIssues.java => ScanIssues.java} | 34 ++++- .../sonar/batch/scan/ModuleScanContainer.java | 118 +++++++-------- .../batch/scan/ProjectScanContainer.java | 14 +- .../batch/issue/IssuableFactoryTest.java | 10 +- ...t.java => IssueCountersDecoratorTest.java} | 8 +- .../batch/issue/ScanIssueChangesTest.java | 72 --------- ...uleIssuesTest.java => ScanIssuesTest.java} | 69 ++++++--- .../org/sonar/core/issue/DefaultIssue.java | 2 +- .../core/issue/workflow/IssueWorkflow.java | 10 +- .../core/issue/workflow/SetResolution.java | 3 + .../org/sonar/core/issue/workflow/State.java | 3 +- .../issue/workflow/IssueWorkflowTest.java | 138 ++++++++++++++++++ .../issue/workflow/SetResolutionTest.java | 45 ++++++ .../sonar/core/issue/workflow/StateTest.java | 60 ++++++++ .../core/issue/workflow/TransitionTest.java | 18 +++ .../java/org/sonar/api/issue/IssueChange.java | 10 +- .../org/sonar/api/issue/IssueChangeTest.java | 4 +- .../server/issue/DefaultJRubyIssues.java | 7 +- .../server/issue/ServerIssueChanges.java | 11 +- .../org/sonar/server/platform/Platform.java | 4 + .../wsclient/issue/DefaultIssueClient.java | 4 +- .../org/sonar/wsclient/issue/IssueChange.java | 13 +- .../org/sonar/wsclient/issue/IssueClient.java | 4 +- .../issue/DefaultIssueClientTest.java | 6 +- .../sonar/wsclient/issue/IssueChangeTest.java | 6 +- 36 files changed, 538 insertions(+), 352 deletions(-) rename plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/{IssuesWorkflowDecorator.java => IssueTrackingDecorator.java} (77%) rename plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/{IssuesWorkflowDecoratorTest.java => IssueTrackingDecoratorTest.java} (84%) rename sonar-batch/src/main/java/org/sonar/batch/issue/{IssuesDecorator.java => IssueCountersDecorator.java} (97%) delete mode 100644 sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java rename sonar-batch/src/main/java/org/sonar/batch/issue/{ModuleIssues.java => ScanIssues.java} (70%) rename sonar-batch/src/test/java/org/sonar/batch/issue/{IssuesDecoratorTest.java => IssueCountersDecoratorTest.java} (98%) delete mode 100644 sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java rename sonar-batch/src/test/java/org/sonar/batch/issue/{ModuleIssuesTest.java => ScanIssuesTest.java} (68%) create mode 100644 sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java create mode 100644 sonar-core/src/test/java/org/sonar/core/issue/workflow/SetResolutionTest.java create mode 100644 sonar-core/src/test/java/org/sonar/core/issue/workflow/StateTest.java diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java index a405fed81eb..0c2ebda5334 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java @@ -25,7 +25,6 @@ import org.sonar.api.*; import org.sonar.api.checks.NoSonarFilter; import org.sonar.api.notifications.NotificationDispatcherMetadata; import org.sonar.api.resources.Java; -import org.sonar.batch.issue.*; import org.sonar.core.timemachine.Periods; import org.sonar.plugins.core.batch.IndexProjectPostJob; import org.sonar.plugins.core.charts.DistributionAreaChart; @@ -34,7 +33,7 @@ import org.sonar.plugins.core.charts.XradarChart; import org.sonar.plugins.core.colorizers.JavaColorizerFormat; import org.sonar.plugins.core.dashboards.*; import org.sonar.plugins.core.issue.IssueTracking; -import org.sonar.plugins.core.issue.IssuesWorkflowDecorator; +import org.sonar.plugins.core.issue.IssueTrackingDecorator; import org.sonar.plugins.core.measurefilters.MyFavouritesFilter; import org.sonar.plugins.core.measurefilters.ProjectFilter; import org.sonar.plugins.core.notifications.alerts.NewAlerts; @@ -411,14 +410,9 @@ public final class CorePlugin extends SonarPlugin { CheckAlertThresholds.class, GenerateAlertEvents.class, ViolationsDecorator.class, - IssuesWorkflowDecorator.class, - InitialOpenIssuesSensor.class, - InitialOpenIssuesStack.class, - IssuesDecorator.class, + IssueTrackingDecorator.class, WeightedViolationsDecorator.class, - WeightedIssuesDecorator.class, ViolationsDensityDecorator.class, - IssuesDensityDecorator.class, LineCoverageDecorator.class, CoverageDecorator.class, BranchCoverageDecorator.class, diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java similarity index 77% rename from plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java rename to plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 9215aa79717..3729d79f9a1 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -21,7 +21,7 @@ package org.sonar.plugins.core.issue; import com.google.common.base.Function; import com.google.common.collect.Collections2; -import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; import com.google.common.collect.Sets; import org.sonar.api.batch.Decorator; import org.sonar.api.batch.DecoratorBarriers; @@ -33,29 +33,27 @@ import org.sonar.api.resources.Resource; import org.sonar.api.resources.ResourceUtils; import org.sonar.api.resources.Scopes; import org.sonar.batch.issue.InitialOpenIssuesStack; -import org.sonar.batch.issue.ModuleIssues; +import org.sonar.batch.issue.ScanIssues; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueDto; import javax.annotation.Nullable; - +import java.util.ArrayList; import java.util.Collection; import java.util.Date; import java.util.Set; -import static com.google.common.collect.Lists.newArrayList; - @DependedUpon(DecoratorBarriers.END_OF_ISSUES_UPDATES) -public class IssuesWorkflowDecorator implements Decorator { +public class IssueTrackingDecorator implements Decorator { - private final ModuleIssues moduleIssues; + private final ScanIssues scanIssues; private final InitialOpenIssuesStack initialOpenIssuesStack; - private final IssueTracking issueTracking; + private final IssueTracking tracking; - public IssuesWorkflowDecorator(ModuleIssues moduleIssues, InitialOpenIssuesStack initialOpenIssuesStack, IssueTracking issueTracking) { - this.moduleIssues = moduleIssues; + public IssueTrackingDecorator(ScanIssues scanIssues, InitialOpenIssuesStack initialOpenIssuesStack, IssueTracking tracking) { + this.scanIssues = scanIssues; this.initialOpenIssuesStack = initialOpenIssuesStack; - this.issueTracking = issueTracking; + this.tracking = tracking; } public boolean shouldExecuteOnProject(Project project) { @@ -64,14 +62,12 @@ public class IssuesWorkflowDecorator implements Decorator { public void decorate(Resource resource, DecoratorContext context) { if (isComponentSupported(resource)) { - Collection newIssues = moduleIssues.issues(resource.getEffectiveKey()); + Collection newIssues = new ArrayList(scanIssues.issues(resource.getEffectiveKey())); Collection openIssues = initialOpenIssuesStack.selectAndRemove(resource.getId()); + tracking.track(resource, openIssues, newIssues); + updateIssues(newIssues); - Collection newDefaultIssues = toDefaultIssues(newIssues); - issueTracking.track(resource, openIssues, newDefaultIssues); - updateIssues(newDefaultIssues); - - Set issueKeys = Sets.newHashSet(Collections2.transform(newIssues, new IssueToKeyfunction())); + Set issueKeys = Sets.newHashSet(Collections2.transform(newIssues, new IssueToKeyFunction())); for (IssueDto openIssue : openIssues) { addManualIssuesAndCloseResolvedOnes(openIssue); closeResolvedStandardIssues(openIssue, issueKeys); @@ -87,7 +83,7 @@ public class IssuesWorkflowDecorator implements Decorator { private void updateIssues(Collection newIssues) { for (DefaultIssue issue : newIssues) { - moduleIssues.addOrUpdate(issue); + scanIssues.addOrUpdate(issue); } } @@ -97,7 +93,7 @@ public class IssuesWorkflowDecorator implements Decorator { if (Issue.STATUS_RESOLVED.equals(issue.status())) { close(issue); } - moduleIssues.addOrUpdate(issue); + scanIssues.addOrUpdate(issue); } } @@ -113,7 +109,7 @@ public class IssuesWorkflowDecorator implements Decorator { issue.setResolution(openIssue.getResolution()); issue.setStatus(openIssue.getStatus()); issue.setUpdatedAt(getLoadedDate()); - moduleIssues.addOrUpdate(issue); + scanIssues.addOrUpdate(issue); } } @@ -142,7 +138,7 @@ public class IssuesWorkflowDecorator implements Decorator { private void closeAndSave(IssueDto openIssue) { DefaultIssue issue = openIssue.toDefaultIssue(); close(issue); - moduleIssues.addOrUpdate(issue); + scanIssues.addOrUpdate(issue); } private void reopenAndSave(IssueDto openIssue) { @@ -150,30 +146,20 @@ public class IssuesWorkflowDecorator implements Decorator { issue.setStatus(Issue.STATUS_REOPENED); issue.setResolution(null); issue.setUpdatedAt(getLoadedDate()); - moduleIssues.addOrUpdate(issue); - } - - - private Collection toDefaultIssues(Collection issues) { - return newArrayList(Iterables.transform(issues, new Function() { - @Override - public DefaultIssue apply(Issue issue) { - return (DefaultIssue) issue; - } - })); + scanIssues.addOrUpdate(issue); } private boolean isComponentSupported(Resource resource) { return Scopes.isHigherThanOrEquals(resource.getScope(), Scopes.FILE); } - private static final class IssueToKeyfunction implements Function { + private static final class IssueToKeyFunction implements Function { public String apply(@Nullable Issue issue) { return (issue != null ? issue.key() : null); } } - private Date getLoadedDate(){ + private Date getLoadedDate() { return initialOpenIssuesStack.getLoadedDate(); } } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java similarity index 84% rename from plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java rename to plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index 71e6c52ffd1..3f521a59c6d 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -28,7 +28,7 @@ import org.sonar.api.resources.Project; import org.sonar.api.resources.Qualifiers; import org.sonar.api.resources.Resource; import org.sonar.batch.issue.InitialOpenIssuesStack; -import org.sonar.batch.issue.ModuleIssues; +import org.sonar.batch.issue.ScanIssues; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueDto; import org.sonar.core.persistence.AbstractDaoTestCase; @@ -45,18 +45,18 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.*; -public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { +public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { - IssuesWorkflowDecorator decorator; - ModuleIssues moduleIssues = mock(ModuleIssues.class); + IssueTrackingDecorator decorator; + ScanIssues scanIssues = mock(ScanIssues.class); InitialOpenIssuesStack initialOpenIssuesStack = mock(InitialOpenIssuesStack.class); - IssueTracking issueTracking = mock(IssueTracking.class); + IssueTracking tracking = mock(IssueTracking.class); Date loadedDate = new Date(); @Before public void init() { when(initialOpenIssuesStack.getLoadedDate()).thenReturn(loadedDate); - decorator = new IssuesWorkflowDecorator(moduleIssues, initialOpenIssuesStack, issueTracking); + decorator = new IssueTrackingDecorator(scanIssues, initialOpenIssuesStack, tracking); } @Test @@ -67,7 +67,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { } @Test - public void should_execute_on_project_not_if_past_inspection() { + public void should_execute_on_project_not_if_past_scan() { Project project = mock(Project.class); when(project.isLatestAnalysis()).thenReturn(false); assertFalse(decorator.shouldExecuteOnProject(project)); @@ -75,14 +75,14 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { @Test public void should_close_resolved_issue() { - when(moduleIssues.issues(anyString())).thenReturn(Collections.emptyList()); + when(scanIssues.issues(anyString())).thenReturn(Collections.emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( new IssueDto().setKey("100").setRuleId(10).setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); - verify(moduleIssues).addOrUpdate(argument.capture()); + verify(scanIssues).addOrUpdate(argument.capture()); assertThat(argument.getValue().status()).isEqualTo(Issue.STATUS_CLOSED); assertThat(argument.getValue().updatedAt()).isEqualTo(loadedDate); assertThat(argument.getValue().closedAt()).isEqualTo(loadedDate); @@ -90,14 +90,14 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { @Test public void should_close_resolved_manual_issue() { - when(moduleIssues.issues(anyString())).thenReturn(Collections.emptyList()); + when(scanIssues.issues(anyString())).thenReturn(Collections.emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( new IssueDto().setKey("100").setRuleId(1).setManualIssue(true).setStatus(Issue.STATUS_RESOLVED).setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); - verify(moduleIssues).addOrUpdate(argument.capture()); + verify(scanIssues).addOrUpdate(argument.capture()); assertThat(argument.getValue().status()).isEqualTo(Issue.STATUS_CLOSED); assertThat(argument.getValue().updatedAt()).isEqualTo(loadedDate); assertThat(argument.getValue().closedAt()).isEqualTo(loadedDate); @@ -105,8 +105,8 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { @Test public void should_reopen_unresolved_issue() { - when(moduleIssues.issues(anyString())).thenReturn(Lists.newArrayList( - new DefaultIssue().setKey("100"))); + when(scanIssues.issues(anyString())).thenReturn(Lists.newArrayList( + new DefaultIssue().setKey("100"))); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( new IssueDto().setKey("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FIXED) .setRuleKey_unit_test_only("squid", "AvoidCycle"))); @@ -114,7 +114,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { decorator.decorate(mock(Resource.class), null); ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); - verify(moduleIssues, times(2)).addOrUpdate(argument.capture()); + verify(scanIssues, times(2)).addOrUpdate(argument.capture()); List capturedDefaultIssues = argument.getAllValues(); // First call is done when updating issues after calling issue tracking and we don't care @@ -126,7 +126,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { @Test public void should_keep_false_positive_issue() { - when(moduleIssues.issues(anyString())).thenReturn(Lists.newArrayList( + when(scanIssues.issues(anyString())).thenReturn(Lists.newArrayList( new DefaultIssue().setKey("100"))); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( new IssueDto().setKey("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE) @@ -135,7 +135,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { decorator.decorate(mock(Resource.class), null); ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); - verify(moduleIssues, times(2)).addOrUpdate(argument.capture()); + verify(scanIssues, times(2)).addOrUpdate(argument.capture()); List capturedDefaultIssues = argument.getAllValues(); // First call is done when updating issues after calling issue tracking and we don't care @@ -147,7 +147,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { @Test public void should_close_remaining_open_issue_on_root_project() { - when(moduleIssues.issues(anyString())).thenReturn(Collections.emptyList()); + when(scanIssues.issues(anyString())).thenReturn(Collections.emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(Collections.emptyList()); when(initialOpenIssuesStack.getAllIssues()).thenReturn(newArrayList(new IssueDto().setKey("100").setRuleId(1).setRuleKey_unit_test_only("squid", "AvoidCycle"))); @@ -157,7 +157,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { decorator.decorate(resource, null); ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); - verify(moduleIssues).addOrUpdate(argument.capture()); + verify(scanIssues).addOrUpdate(argument.capture()); assertThat(argument.getValue().status()).isEqualTo(Issue.STATUS_CLOSED); assertThat(argument.getValue().updatedAt()).isEqualTo(loadedDate); assertThat(argument.getValue().closedAt()).isEqualTo(loadedDate); diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java index 71d2651d825..9010ca3d965 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java @@ -31,22 +31,22 @@ import java.util.Collection; */ public class DefaultIssuable implements Issuable { - private final ModuleIssues moduleIssues; + private final ScanIssues scanIssues; private final Component component; - DefaultIssuable(Component component, ModuleIssues moduleIssues) { + DefaultIssuable(Component component, ScanIssues scanIssues) { this.component = component; - this.moduleIssues = moduleIssues; + this.scanIssues = scanIssues; } @Override public IssueBuilder newIssue() { - return new DefaultIssueBuilder(moduleIssues, component.key()); + return new DefaultIssueBuilder(scanIssues, component.key()); } @Override public Collection issues() { - return moduleIssues.issues(component.key()); + return scanIssues.issues(component.key()); } @Override diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java index 585bbbcde05..359354b66b5 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java @@ -56,6 +56,7 @@ public class DeprecatedViolations implements BatchComponent { .setCost(violation.getCost()) .setLine(violation.getLineId()) .setDescription(violation.getMessage()) + .setResolution(Issue.RESOLUTION_OPEN) .setStatus(Issue.STATUS_OPEN) .setSeverity(violation.getSeverity() != null ? violation.getSeverity().name() : null); diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesSensor.java b/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesSensor.java index a9e3a70857e..0d58db7e089 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesSensor.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesSensor.java @@ -17,7 +17,6 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - package org.sonar.batch.issue; import org.sonar.api.batch.Sensor; @@ -29,6 +28,9 @@ import org.sonar.core.issue.IssueDto; import java.util.Date; import java.util.List; +/** + * Load all the issues referenced during the previous scan. + */ public class InitialOpenIssuesSensor implements Sensor { private final InitialOpenIssuesStack initialOpenIssuesStack; @@ -47,8 +49,8 @@ public class InitialOpenIssuesSensor implements Sensor { @Override public void analyse(Project project, SensorContext context) { Date loadingDate = new Date(); - List issuesDto = issueDao.selectOpenIssues(project.getId()); - initialOpenIssuesStack.setIssues(issuesDto, loadingDate); + List dtos = issueDao.selectOpenIssues(project.getId()); + initialOpenIssuesStack.setIssues(dtos, loadingDate); } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesStack.java b/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesStack.java index f99cf9ab119..ada44a2aa0d 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesStack.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/InitialOpenIssuesStack.java @@ -23,7 +23,7 @@ package org.sonar.batch.issue; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ListMultimap; -import org.sonar.api.BatchExtension; +import org.sonar.api.BatchComponent; import org.sonar.core.issue.IssueDto; import java.util.Collection; @@ -31,9 +31,9 @@ import java.util.Collections; import java.util.Date; import java.util.List; -public class InitialOpenIssuesStack implements BatchExtension { +public class InitialOpenIssuesStack implements BatchComponent { - private ListMultimap issuesByResourceId; + private final ListMultimap issuesByResourceId; private Date loadedDate; @@ -41,14 +41,14 @@ public class InitialOpenIssuesStack implements BatchExtension { issuesByResourceId = ArrayListMultimap.create(); } - public void setIssues(List issues, Date loadedDate){ + public void setIssues(List issues, Date loadedDate) { this.loadedDate = loadedDate; for (IssueDto issueDto : issues) { issuesByResourceId.put(issueDto.getResourceId(), issueDto); } } - public List selectAndRemove(final Integer resourceId){ + public List selectAndRemove(final Integer resourceId) { List foundIssuesDto = issuesByResourceId.get(resourceId); if (!foundIssuesDto.isEmpty()) { List issuesDto = ImmutableList.copyOf(foundIssuesDto); @@ -59,7 +59,7 @@ public class InitialOpenIssuesStack implements BatchExtension { } } - public Collection getAllIssues(){ + public Collection getAllIssues() { return issuesByResourceId.values(); } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java index 7d39e28e91e..8ad43865f77 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java @@ -27,13 +27,17 @@ import org.sonar.core.component.ResourceComponent; import javax.annotation.CheckForNull; +/** + * Create the perspective {@link Issuable} on components. + * @since 3.6 + */ public class IssuableFactory extends PerspectiveBuilder { - private final ModuleIssues moduleIssues; + private final ScanIssues scanIssues; - public IssuableFactory(ModuleIssues moduleIssues) { + public IssuableFactory(ScanIssues scanIssues) { super(Issuable.class); - this.moduleIssues = moduleIssues; + this.scanIssues = scanIssues; } @CheckForNull @@ -43,9 +47,6 @@ public class IssuableFactory extends PerspectiveBuilder { if (component instanceof ResourceComponent) { supported = Scopes.isHigherThanOrEquals(((ResourceComponent) component).scope(), Scopes.FILE); } - if (supported) { - return new DefaultIssuable(component, moduleIssues); - } - return null; + return supported ? new DefaultIssuable(component, scanIssues) : null; } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssueCountersDecorator.java similarity index 97% rename from sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java rename to sonar-batch/src/main/java/org/sonar/batch/issue/IssueCountersDecorator.java index 3cf77072595..93ed6bba741 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssueCountersDecorator.java @@ -41,16 +41,17 @@ import java.util.*; import static com.google.common.collect.Lists.newArrayList; /** + * Computes metrics related to number of issues. * @since 3.6 */ @DependsUpon(DecoratorBarriers.END_OF_ISSUES_UPDATES) -public class IssuesDecorator implements Decorator { +public class IssueCountersDecorator implements Decorator { private final ResourcePerspectives perspectives; private final RuleFinder rulefinder; private final TimeMachineConfiguration timeMachineConfiguration; - public IssuesDecorator(ResourcePerspectives perspectives, RuleFinder rulefinder, TimeMachineConfiguration timeMachineConfiguration) { + public IssueCountersDecorator(ResourcePerspectives perspectives, RuleFinder rulefinder, TimeMachineConfiguration timeMachineConfiguration) { this.perspectives = perspectives; this.rulefinder = rulefinder; this.timeMachineConfiguration = timeMachineConfiguration; @@ -62,7 +63,7 @@ public class IssuesDecorator implements Decorator { @DependedUpon public List generatesIssuesMetrics() { - return newArrayList( + return ImmutableList.of( CoreMetrics.ISSUES, CoreMetrics.BLOCKER_ISSUES, CoreMetrics.CRITICAL_ISSUES, diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java index d29d00f2982..f8d51fd14b5 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java @@ -34,6 +34,9 @@ import java.util.Map; import static com.google.common.collect.Lists.newArrayList; +/** + * Executed at the end of project scan, when all the modules are completed. + */ public class IssuePersister implements ScanPersister { private final IssueDao dao; diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java deleted file mode 100644 index 0d1ce0991bb..00000000000 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2013 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.batch.issue; - -import org.sonar.api.issue.Issue; -import org.sonar.api.issue.IssueChange; -import org.sonar.api.issue.IssueChanges; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.workflow.IssueWorkflow; - -import java.util.Date; - -public class ScanIssueChanges implements IssueChanges { - - private final IssueCache cache; - private final IssueWorkflow workflow; - - public ScanIssueChanges(IssueCache cache, IssueWorkflow workflow) { - this.cache = cache; - this.workflow = workflow; - } - - @Override - public Issue apply(Issue issue, IssueChange change) { - if (!change.hasChanges()) { - return issue; - } - DefaultIssue reloaded = reload(issue); - workflow.apply(reloaded, change); - reloaded.setUpdatedAt(new Date()); - cache.addOrUpdate(reloaded); - // TODO keep history of changes - return reloaded; - } - - - private DefaultIssue reload(Issue issue) { - DefaultIssue reloaded = (DefaultIssue) cache.componentIssue(issue.componentKey(), issue.key()); - if (reloaded == null) { - throw new IllegalStateException("Bad API usage. Unregistered issues can't be changed."); - } - return reloaded; - } -} diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java similarity index 70% rename from sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java rename to sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java index 35a032b05a0..e96440f94be 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ModuleIssues.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java @@ -21,34 +21,57 @@ package org.sonar.batch.issue; import com.google.common.base.Preconditions; import com.google.common.base.Strings; -import org.sonar.api.BatchComponent; import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueChange; +import org.sonar.api.issue.IssueChanges; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.rules.ActiveRule; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.OnIssueCreation; +import org.sonar.core.issue.workflow.IssueWorkflow; import java.util.Collection; import java.util.UUID; -public class ModuleIssues implements OnIssueCreation, BatchComponent { +public class ScanIssues implements IssueChanges, OnIssueCreation { private final RulesProfile qProfile; private final IssueCache cache; private final Project project; + private final IssueWorkflow workflow; - public ModuleIssues(RulesProfile qProfile, IssueCache cache, Project project) { + public ScanIssues(RulesProfile qProfile, IssueCache cache, Project project, IssueWorkflow workflow) { this.qProfile = qProfile; this.cache = cache; this.project = project; + this.workflow = workflow; + } + + @Override + public Issue apply(Issue issue, IssueChange change) { + if (!change.hasChanges()) { + return issue; + } + DefaultIssue reloaded = reload(issue); + workflow.change(reloaded, change); + cache.addOrUpdate(reloaded); + return reloaded; + } + + private DefaultIssue reload(Issue issue) { + DefaultIssue reloaded = (DefaultIssue) cache.componentIssue(issue.componentKey(), issue.key()); + if (reloaded == null) { + throw new IllegalStateException("Bad API usage. Unregistered issues can't be changed."); + } + return reloaded; } public Collection issues(String componentKey) { return cache.componentIssues(componentKey); } - public ModuleIssues addOrUpdate(DefaultIssue issue) { + public ScanIssues addOrUpdate(DefaultIssue issue) { Preconditions.checkState(!Strings.isNullOrEmpty(issue.key()), "Missing issue key"); cache.addOrUpdate(issue); return this; @@ -61,16 +84,15 @@ public class ModuleIssues implements OnIssueCreation, BatchComponent { // rule does not exist or is not enabled -> ignore the issue return; } - String key = UUID.randomUUID().toString(); Preconditions.checkState(!Strings.isNullOrEmpty(key), "Fail to generate issue key"); issue.setKey(key); issue.setCreatedAt(project.getAnalysisDate()); + issue.setResolution(Issue.RESOLUTION_OPEN); issue.setStatus(Issue.STATUS_OPEN); if (issue.severity() == null) { issue.setSeverity(activeRule.getSeverity().name()); } - cache.addOrUpdate(issue); } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleScanContainer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleScanContainer.java index 7aec05b91dc..5e535c18edf 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleScanContainer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ModuleScanContainer.java @@ -19,8 +19,6 @@ */ package org.sonar.batch.scan; -import org.sonar.batch.bootstrap.ExtensionMatcher; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.BatchExtension; @@ -32,33 +30,21 @@ import org.sonar.api.resources.Languages; import org.sonar.api.resources.Project; import org.sonar.api.scan.filesystem.FileExclusions; import org.sonar.api.scan.filesystem.PathResolver; -import org.sonar.batch.DefaultProjectClasspath; -import org.sonar.batch.DefaultSensorContext; -import org.sonar.batch.DefaultTimeMachine; -import org.sonar.batch.ProfileProvider; -import org.sonar.batch.ProjectTree; -import org.sonar.batch.ResourceFilters; -import org.sonar.batch.ViolationFilters; +import org.sonar.batch.*; import org.sonar.batch.bootstrap.BatchExtensionDictionnary; import org.sonar.batch.bootstrap.ExtensionInstaller; +import org.sonar.batch.bootstrap.ExtensionMatcher; import org.sonar.batch.bootstrap.ExtensionUtils; import org.sonar.batch.components.TimeMachineConfiguration; import org.sonar.batch.events.EventBus; import org.sonar.batch.index.DefaultIndex; import org.sonar.batch.index.ResourcePersister; -import org.sonar.batch.issue.IssuableFactory; -import org.sonar.batch.issue.ModuleIssues; +import org.sonar.batch.issue.*; import org.sonar.batch.local.DryRunExporter; import org.sonar.batch.phases.PhaseExecutor; import org.sonar.batch.phases.PhasesTimeProfiler; -import org.sonar.batch.scan.filesystem.DeprecatedFileSystemAdapter; -import org.sonar.batch.scan.filesystem.ExclusionFilters; -import org.sonar.batch.scan.filesystem.FileSystemLogger; -import org.sonar.batch.scan.filesystem.LanguageFilters; -import org.sonar.batch.scan.filesystem.ModuleFileSystemProvider; -import org.sonar.batch.scan.source.SymbolPerspectiveBuilder; +import org.sonar.batch.scan.filesystem.*; import org.sonar.core.component.ScanPerspectives; -import org.sonar.batch.scan.source.HighlightableBuilder; public class ModuleScanContainer extends ComponentContainer { private static final Logger LOG = LoggerFactory.getLogger(ModuleScanContainer.class); @@ -79,53 +65,59 @@ public class ModuleScanContainer extends ComponentContainer { private void addCoreComponents() { ProjectDefinition moduleDefinition = getComponentByType(ProjectTree.class).getProjectDefinition(module); add( - moduleDefinition, - module.getConfiguration(), - module, - ModuleSettings.class); + moduleDefinition, + module.getConfiguration(), + module, + ModuleSettings.class); // hack to initialize commons-configuration before ExtensionProviders getComponentByType(ModuleSettings.class); add( - EventBus.class, - PhaseExecutor.class, - PhasesTimeProfiler.class, - UnsupportedProperties.class, - PhaseExecutor.getPhaseClasses(), - moduleDefinition.getContainerExtensions(), - - // TODO move outside project, but not possible yet because of dependency of project settings (cf plsql) - Languages.class, - - // file system - PathResolver.class, - FileExclusions.class, - LanguageFilters.class, - ExclusionFilters.class, - DefaultProjectClasspath.class, - new ModuleFileSystemProvider(), - DeprecatedFileSystemAdapter.class, - FileSystemLogger.class, - - // the Snapshot component will be removed when asynchronous measures are improved (required for AsynchronousMeasureSensor) - getComponentByType(ResourcePersister.class).getSnapshot(module), - - TimeMachineConfiguration.class, - org.sonar.api.database.daos.MeasuresDao.class, - DefaultSensorContext.class, - BatchExtensionDictionnary.class, - DefaultTimeMachine.class, - ViolationFilters.class, - ResourceFilters.class, - DryRunExporter.class, - new ProfileProvider(), - - ModuleIssues.class, - IssuableFactory.class, - - ScanPerspectives.class - ); + EventBus.class, + PhaseExecutor.class, + PhasesTimeProfiler.class, + UnsupportedProperties.class, + PhaseExecutor.getPhaseClasses(), + moduleDefinition.getContainerExtensions(), + + // TODO move outside project, but not possible yet because of dependency of project settings (cf plsql) + Languages.class, + + // file system + PathResolver.class, + FileExclusions.class, + LanguageFilters.class, + ExclusionFilters.class, + DefaultProjectClasspath.class, + new ModuleFileSystemProvider(), + DeprecatedFileSystemAdapter.class, + FileSystemLogger.class, + + // the Snapshot component will be removed when asynchronous measures are improved (required for AsynchronousMeasureSensor) + getComponentByType(ResourcePersister.class).getSnapshot(module), + + TimeMachineConfiguration.class, + org.sonar.api.database.daos.MeasuresDao.class, + DefaultSensorContext.class, + BatchExtensionDictionnary.class, + DefaultTimeMachine.class, + ViolationFilters.class, + ResourceFilters.class, + DryRunExporter.class, + new ProfileProvider(), + + // issues + ScanIssues.class, + IssuableFactory.class, + IssueCountersDecorator.class, + WeightedIssuesDecorator.class, + IssuesDensityDecorator.class, + InitialOpenIssuesSensor.class, + InitialOpenIssuesStack.class, + + ScanPerspectives.class + ); } private void addExtensions() { @@ -147,9 +139,9 @@ public class ModuleScanContainer extends ComponentContainer { protected void doAfterStart() { DefaultIndex index = getComponentByType(DefaultIndex.class); index.setCurrentProject(module, - getComponentByType(ResourceFilters.class), - getComponentByType(ViolationFilters.class), - getComponentByType(RulesProfile.class)); + getComponentByType(ResourceFilters.class), + getComponentByType(ViolationFilters.class), + getComponentByType(RulesProfile.class)); getComponentByType(PhaseExecutor.class).execute(module); } diff --git a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java index 6e8727ee754..f965cf0d69b 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java +++ b/sonar-batch/src/main/java/org/sonar/batch/scan/ProjectScanContainer.java @@ -34,10 +34,7 @@ import org.sonar.batch.bootstrap.ExtensionMatcher; import org.sonar.batch.bootstrap.ExtensionUtils; import org.sonar.batch.bootstrap.MetricProvider; import org.sonar.batch.index.*; -import org.sonar.batch.issue.DeprecatedViolations; -import org.sonar.batch.issue.IssueCache; -import org.sonar.batch.issue.IssuePersister; -import org.sonar.batch.issue.ScanIssueChanges; +import org.sonar.batch.issue.*; import org.sonar.batch.phases.GraphPersister; import org.sonar.batch.profiling.PhasesSumUpTimeProfiler; import org.sonar.batch.scan.maven.FakeMavenPluginExecutor; @@ -45,6 +42,7 @@ import org.sonar.batch.scan.maven.MavenPluginExecutor; import org.sonar.batch.scan.source.HighlightableBuilder; import org.sonar.batch.scan.source.SymbolPerspectiveBuilder; import org.sonar.core.component.ScanGraph; +import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.core.notification.DefaultNotificationManager; import org.sonar.core.test.TestPlanBuilder; import org.sonar.core.test.TestPlanPerspectiveLoader; @@ -89,10 +87,15 @@ public class ProjectScanContainer extends ComponentContainer { SnapshotCache.class, ComponentDataCache.class, ComponentDataPersister.class, - ScanIssueChanges.class, + + // issues + IssueWorkflow.class, + ScanIssues.class, DeprecatedViolations.class, IssueCache.class, IssuePersister.class, + + // tests TestPlanPerspectiveLoader.class, TestablePerspectiveLoader.class, TestPlanBuilder.class, @@ -100,6 +103,7 @@ public class ProjectScanContainer extends ComponentContainer { ScanGraph.create(), GraphPersister.class, + // lang HighlightableBuilder.class, SymbolPerspectiveBuilder.class ); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java index 21c39da2215..16358a6546f 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java @@ -33,11 +33,11 @@ import static org.mockito.Mockito.mock; public class IssuableFactoryTest { - ModuleIssues moduleIssues = mock(ModuleIssues.class); + ScanIssues issueFactory = mock(ScanIssues.class); @Test public void file_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(moduleIssues); + IssuableFactory factory = new IssuableFactory(issueFactory); Component component = new ResourceComponent(new File("foo/bar.c")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -48,7 +48,7 @@ public class IssuableFactoryTest { @Test public void project_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(moduleIssues); + IssuableFactory factory = new IssuableFactory(issueFactory); Component component = new ResourceComponent(new Project("Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -59,7 +59,7 @@ public class IssuableFactoryTest { @Test public void java_file_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(moduleIssues); + IssuableFactory factory = new IssuableFactory(issueFactory); Component component = new ResourceComponent(new JavaFile("bar.Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -70,7 +70,7 @@ public class IssuableFactoryTest { @Test public void java_class_should_not_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(moduleIssues); + IssuableFactory factory = new IssuableFactory(issueFactory); Component component = new ResourceComponent(JavaClass.create("bar", "Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssueCountersDecoratorTest.java similarity index 98% rename from sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java rename to sonar-batch/src/test/java/org/sonar/batch/issue/IssueCountersDecoratorTest.java index 6be9173c240..c78f98f74e0 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssueCountersDecoratorTest.java @@ -53,9 +53,9 @@ import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.*; -public class IssuesDecoratorTest { +public class IssueCountersDecoratorTest { - private IssuesDecorator decorator; + private IssueCountersDecorator decorator; private TimeMachineConfiguration timeMachineConfiguration; private RuleFinder rulefinder; private Issuable issuable; @@ -107,7 +107,7 @@ public class IssuesDecoratorTest { issuable = mock(Issuable.class); ResourcePerspectives perspectives = mock(ResourcePerspectives.class); when(perspectives.as(Issuable.class, resource)).thenReturn(issuable); - decorator = new IssuesDecorator(perspectives, rulefinder, timeMachineConfiguration); + decorator = new IssueCountersDecorator(perspectives, rulefinder, timeMachineConfiguration); } @Test @@ -130,7 +130,7 @@ public class IssuesDecoratorTest { public void should_do_nothing_when_issuable_is_null() { ResourcePerspectives perspectives = mock(ResourcePerspectives.class); when(perspectives.as(Issuable.class, resource)).thenReturn(null); - IssuesDecorator decorator = new IssuesDecorator(perspectives, rulefinder, timeMachineConfiguration); + IssueCountersDecorator decorator = new IssueCountersDecorator(perspectives, rulefinder, timeMachineConfiguration); decorator.decorate(resource, context); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java deleted file mode 100644 index c833c9c1e8a..00000000000 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java +++ /dev/null @@ -1,72 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2013 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.batch.issue; - -import org.junit.Test; -import org.sonar.api.issue.Issue; -import org.sonar.api.issue.IssueChange; -import org.sonar.api.rule.Severity; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.workflow.IssueWorkflow; - -import static org.fest.assertions.Assertions.assertThat; -import static org.fest.assertions.Fail.fail; -import static org.mockito.Mockito.*; - -public class ScanIssueChangesTest { - - IssueCache cache = mock(IssueCache.class); - IssueWorkflow workflow = mock(IssueWorkflow.class); - ScanIssueChanges changes = new ScanIssueChanges(cache, workflow); - - @Test - public void should_ignore_empty_change() throws Exception { - Issue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); - when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(issue); - Issue changed = changes.apply(issue, IssueChange.create()); - verifyZeroInteractions(cache); - assertThat(changed).isSameAs(issue); - assertThat(changed.updatedAt()).isNull(); - } - - @Test - public void unknown_issue_is_a_bad_api_usage() throws Exception { - Issue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); - when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(null); - try { - changes.apply(issue, IssueChange.create().setLine(200)); - fail(); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("Bad API usage. Unregistered issues can't be changed."); - } - } - - @Test - public void should_change_fields() throws Exception { - DefaultIssue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); - when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(issue); - - IssueChange change = IssueChange.create().setTransition("resolve"); - changes.apply(issue, change); - - verify(cache).addOrUpdate(issue); - verify(workflow).apply(issue, change); - } -} diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java similarity index 68% rename from sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java rename to sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java index f14390cb3fd..41bd1aafb0f 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ModuleIssuesTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java @@ -17,13 +17,12 @@ * along with this program; if not, write to the Free Software Foundation, * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */ - package org.sonar.batch.issue; -import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueChange; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.rule.RuleKey; @@ -32,32 +31,25 @@ import org.sonar.api.rules.ActiveRule; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RulePriority; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.workflow.IssueWorkflow; import java.util.Date; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Matchers.anyString; +import static org.fest.assertions.Fail.fail; import static org.mockito.Mockito.*; -public class ModuleIssuesTest { - - private ModuleIssues moduleIssues; - private RulesProfile qProfile; - private IssueCache cache; - private Project project; +public class ScanIssuesTest { - @Before - public void before() throws Exception { - qProfile = mock(RulesProfile.class); - cache = mock(IssueCache.class); - project = mock(Project.class); - - moduleIssues = new ModuleIssues(qProfile, cache, project); - } + IssueCache cache = mock(IssueCache.class); + IssueWorkflow workflow = mock(IssueWorkflow.class); + RulesProfile qProfile = mock(RulesProfile.class); + Project project = mock(Project.class); + ScanIssues scanIssues = new ScanIssues(qProfile, cache, project, workflow); @Test public void should_get_issues() throws Exception { - moduleIssues.issues("key"); + scanIssues.issues("key"); verify(cache).componentIssues("key"); } @@ -66,7 +58,7 @@ public class ModuleIssuesTest { when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(null); DefaultIssue issue = new DefaultIssue().setRuleKey(RuleKey.of("squid", "AvoidCycle")); - moduleIssues.onIssueCreation(issue); + scanIssues.onIssueCreation(issue); verifyZeroInteractions(cache); } @@ -78,7 +70,7 @@ public class ModuleIssuesTest { when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(activeRule); DefaultIssue issue = new DefaultIssue().setRuleKey(RuleKey.of("squid", "AvoidCycle")); - moduleIssues.onIssueCreation(issue); + scanIssues.onIssueCreation(issue); verifyZeroInteractions(cache); } @@ -95,7 +87,7 @@ public class ModuleIssuesTest { when(project.getAnalysisDate()).thenReturn(analysisDate); DefaultIssue issue = new DefaultIssue().setRuleKey(RuleKey.of("squid", "AvoidCycle")).setSeverity(Severity.CRITICAL); - moduleIssues.onIssueCreation(issue); + scanIssues.onIssueCreation(issue); ArgumentCaptor argument = ArgumentCaptor.forClass(Issue.class); verify(cache).addOrUpdate(argument.capture()); @@ -117,7 +109,7 @@ public class ModuleIssuesTest { when(project.getAnalysisDate()).thenReturn(analysisDate); DefaultIssue issue = new DefaultIssue().setRuleKey(RuleKey.of("squid", "AvoidCycle")).setSeverity(null); - moduleIssues.onIssueCreation(issue); + scanIssues.onIssueCreation(issue); ArgumentCaptor argument = ArgumentCaptor.forClass(Issue.class); verify(cache).addOrUpdate(argument.capture()); @@ -127,4 +119,37 @@ public class ModuleIssuesTest { assertThat(argument.getValue().createdAt()).isEqualTo(analysisDate); } + @Test + public void should_ignore_empty_change() throws Exception { + Issue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); + when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(issue); + Issue changed = scanIssues.apply(issue, IssueChange.create()); + verifyZeroInteractions(cache); + assertThat(changed).isSameAs(issue); + assertThat(changed.updatedAt()).isNull(); + } + + @Test + public void unknown_issue_is_a_bad_api_usage() throws Exception { + Issue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); + when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(null); + try { + scanIssues.apply(issue, IssueChange.create().setLine(200)); + fail(); + } catch (IllegalStateException e) { + assertThat(e).hasMessage("Bad API usage. Unregistered issues can't be changed."); + } + } + + @Test + public void should_change_fields() throws Exception { + DefaultIssue issue = new DefaultIssue().setComponentKey("org/struts/Action.java").setKey("ABCDE"); + when(cache.componentIssue("org/struts/Action.java", "ABCDE")).thenReturn(issue); + + IssueChange change = IssueChange.create().setTransition("resolve"); + scanIssues.apply(issue, change); + + verify(cache).addOrUpdate(issue); + verify(workflow).change(issue, change); + } } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index 4b623eade82..0fe18b0261e 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -40,7 +40,7 @@ import java.util.Set; public class DefaultIssue implements Issue, Serializable { - private static final Set RESOLUTIONS = ImmutableSet.of(RESOLUTION_FALSE_POSITIVE, RESOLUTION_FIXED); + private static final Set RESOLUTIONS = ImmutableSet.of(RESOLUTION_OPEN, RESOLUTION_FALSE_POSITIVE, RESOLUTION_FIXED); private static final Set STATUSES = ImmutableSet.of(STATUS_OPEN, STATUS_CLOSED, STATUS_REOPENED, STATUS_RESOLVED); private String key; diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java index a263651c563..455fba67ec3 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java @@ -27,6 +27,7 @@ import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueChange; import org.sonar.core.issue.DefaultIssue; +import java.util.Date; import java.util.List; import java.util.Map; @@ -89,7 +90,7 @@ public class IssueWorkflow implements BatchComponent, ServerComponent, Startable return machine.state(issue.status()).outTransitions(issue); } - public boolean apply(DefaultIssue issue, IssueChange change) { + public boolean change(DefaultIssue issue, IssueChange change) { if (change.hasChanges()) { if (change.description() != null) { issue.setDescription(change.description()); @@ -118,13 +119,18 @@ public class IssueWorkflow implements BatchComponent, ServerComponent, Startable if (change.transition() != null) { move(issue, change.transition()); } + issue.setUpdatedAt(new Date()); return true; } return false; } - public void move(DefaultIssue issue, String transition) { + private void move(DefaultIssue issue, String transition) { State state = machine.state(issue.status()); state.move(issue, transition); } + + StateMachine machine() { + return machine; + } } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetResolution.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetResolution.java index 2ed3dc0fa27..c18bbc5493f 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetResolution.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/SetResolution.java @@ -19,12 +19,15 @@ */ package org.sonar.core.issue.workflow; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import org.sonar.core.issue.DefaultIssue; class SetResolution implements Function { private final String resolution; SetResolution(String resolution) { + Preconditions.checkArgument(!Strings.isNullOrEmpty(resolution), "Resolution must be set"); this.resolution = resolution; } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/State.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/State.java index 8f636df6f68..79e8b914c39 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/State.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/State.java @@ -47,7 +47,8 @@ public class State { Set keys = Sets.newHashSet(); for (Transition transition : transitions) { if (keys.contains(transition.key())) { - throw new IllegalArgumentException("Transition " + transition.key() + " is declared several times from the originating state " + stateKey); + throw new IllegalArgumentException("Transition '" + transition.key() + + "' is declared several times from the originating state '" + stateKey + "'"); } keys.add(transition.key()); } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java new file mode 100644 index 00000000000..46c4ba0b3df --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/IssueWorkflowTest.java @@ -0,0 +1,138 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.issue.workflow; + +import com.google.common.base.Function; +import com.google.common.collect.Collections2; +import org.junit.Test; +import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueChange; +import org.sonar.core.issue.DefaultIssue; + +import javax.annotation.Nullable; +import java.util.Collection; +import java.util.List; + +import static org.fest.assertions.Assertions.assertThat; + +public class IssueWorkflowTest { + IssueWorkflow workflow = new IssueWorkflow(); + + @Test + public void should_init_state_machine() throws Exception { + assertThat(workflow.machine()).isNull(); + workflow.start(); + assertThat(workflow.machine()).isNotNull(); + assertThat(workflow.machine().state(Issue.STATUS_OPEN)).isNotNull(); + assertThat(workflow.machine().state(Issue.STATUS_CLOSED)).isNotNull(); + assertThat(workflow.machine().state(Issue.STATUS_REOPENED)).isNotNull(); + assertThat(workflow.machine().state(Issue.STATUS_RESOLVED)).isNotNull(); + workflow.stop(); + } + + @Test + public void should_list_available_transitions() throws Exception { + workflow.start(); + + DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_OPEN); + List transitions = workflow.availableTransitions(issue); + assertThat(transitions).hasSize(3); + assertThat(keys(transitions)).containsOnly("close", "falsepositive", "resolve"); + } + + @Test + public void should_not_change_anything() throws Exception { + workflow.start(); + + DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_OPEN); + workflow.change(issue, IssueChange.create()); + + assertThat(issue.updatedAt()).isNull(); + } + + @Test + public void should_set_fields() throws Exception { + workflow.start(); + + DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_OPEN); + IssueChange change = IssueChange.create() + .setAssignee("arthur") + .setAttribute("JIRA", "FOO-1234") + .setCost(4.2) + .setLine(123) + .setDescription("the desc") + .setSeverity("BLOCKER"); + workflow.change(issue, change); + + assertThat(issue.updatedAt()).isNotNull(); + assertThat(issue.assignee()).isEqualTo("arthur"); + assertThat(issue.attribute("JIRA")).isEqualTo("FOO-1234"); + assertThat(issue.cost()).isEqualTo(4.2); + assertThat(issue.line()).isEqualTo(123); + assertThat(issue.description()).isEqualTo("the desc"); + assertThat(issue.severity()).isEqualTo("BLOCKER"); + } + + @Test + public void should_change_only_fields_with_new_values() throws Exception { + workflow.start(); + + DefaultIssue issue = new DefaultIssue() + .setStatus(Issue.STATUS_OPEN) + .setAssignee("karadoc") + .setAttribute("YOUTRACK", "ABC") + .setCost(3.4); + IssueChange change = IssueChange.create() + .setAttribute("JIRA", "FOO-1234") + .setLine(123) + .setSeverity("BLOCKER"); + workflow.change(issue, change); + + assertThat(issue.updatedAt()).isNotNull(); + assertThat(issue.assignee()).isEqualTo("karadoc"); + assertThat(issue.attribute("JIRA")).isEqualTo("FOO-1234"); + assertThat(issue.attribute("YOUTRACK")).isEqualTo("ABC"); + assertThat(issue.cost()).isEqualTo(3.4); + assertThat(issue.line()).isEqualTo(123); + assertThat(issue.severity()).isEqualTo("BLOCKER"); + } + + @Test + public void should_change_issue_state() throws Exception { + workflow.start(); + + DefaultIssue issue = new DefaultIssue().setStatus(Issue.STATUS_OPEN); + IssueChange change = IssueChange.create().setTransition("close"); + workflow.change(issue, change); + + assertThat(issue.updatedAt()).isNotNull(); + assertThat(issue.resolution()).isEqualTo(Issue.RESOLUTION_FIXED); + assertThat(issue.status()).isEqualTo(Issue.STATUS_CLOSED); + } + + private Collection keys(List transitions) { + return Collections2.transform(transitions, new Function() { + @Override + public String apply(@Nullable Transition transition) { + return transition.key(); + } + }); + } +} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetResolutionTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetResolutionTest.java new file mode 100644 index 00000000000..19e7c17cd3f --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/SetResolutionTest.java @@ -0,0 +1,45 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.issue.workflow; + +import org.junit.Test; +import org.sonar.api.issue.Issue; +import org.sonar.core.issue.DefaultIssue; + +import static org.fest.assertions.Assertions.assertThat; + +public class SetResolutionTest { + @Test + public void should_set_resolution() throws Exception { + DefaultIssue issue = new DefaultIssue(); + SetResolution function = new SetResolution(Issue.RESOLUTION_FIXED); + function.execute(issue); + assertThat(issue.resolution()).isEqualTo(Issue.RESOLUTION_FIXED); + } + + @Test + public void resolution_should_not_be_empty() throws Exception { + try { + new SetResolution(""); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Resolution must be set"); + } + } +} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/StateTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/StateTest.java new file mode 100644 index 00000000000..b354cf50736 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/StateTest.java @@ -0,0 +1,60 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.core.issue.workflow; + +import org.junit.Test; + +import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; + +public class StateTest { + + Transition t1 = Transition.builder("close").from("OPEN").to("CLOSED").build(); + + @Test + public void key_should_be_set() throws Exception { + try { + new State("", new Transition[0]); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("State key must be set"); + } + } + + @Test + public void key_should_be_upper_case() throws Exception { + try { + new State("close", new Transition[0]); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("State key must be upper-case"); + } + } + + @Test + public void no_duplicated_out_transitions() throws Exception { + try { + new State("CLOSE", new Transition[]{t1, t1}); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Transition 'close' is declared several times from the originating state 'CLOSE'"); + } + } +} diff --git a/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java b/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java index c3ab3e52bbc..6298954e094 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/workflow/TransitionTest.java @@ -25,6 +25,7 @@ import org.sonar.core.issue.DefaultIssue; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class TransitionTest { @@ -110,4 +111,21 @@ public class TransitionTest { verify(function1).execute(issue); verify(function2).execute(issue); } + + @Test + public void should_verify_conditions() throws Exception { + DefaultIssue issue = new DefaultIssue(); + Transition transition = Transition.builder("close") + .from("OPEN").to("CLOSED") + .conditions(condition1, condition2) + .build(); + + when(condition1.matches(issue)).thenReturn(true); + when(condition2.matches(issue)).thenReturn(false); + assertThat(transition.supports(issue)).isFalse(); + + when(condition1.matches(issue)).thenReturn(true); + when(condition2.matches(issue)).thenReturn(true); + assertThat(transition.supports(issue)).isTrue(); + } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java index 4db95704a2b..85454d0d9bf 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java @@ -31,7 +31,7 @@ import java.util.Map; public class IssueChange { private String severity = null; private String comment = null; - private String commentLogin = null; + private String login = null; private Boolean manualSeverity = null; private String description = null; private boolean lineChanged = false; @@ -66,8 +66,8 @@ public class IssueChange { return this; } - public IssueChange setCommentLogin(String s) { - this.commentLogin = s; + public IssueChange setLogin(String s) { + this.login = s; return this; } @@ -125,8 +125,8 @@ public class IssueChange { return comment; } - public String commentLogin() { - return commentLogin; + public String login() { + return login; } public Boolean manualSeverity() { diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java index d39f7c89fb4..4d8458d7ce8 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java @@ -109,9 +109,9 @@ public class IssueChangeTest { @Test public void should_add_comment() { IssueChange change = IssueChange.create(); - change.setComment("foo").setCommentLogin("perceval"); + change.setComment("foo").setLogin("perceval"); assertThat(change.comment()).isEqualTo("foo"); - assertThat(change.commentLogin()).isEqualTo("perceval"); + assertThat(change.login()).isEqualTo("perceval"); assertThat(change.hasChanges()).isTrue(); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java b/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java index 3e672dd5387..f1d7e07121b 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java @@ -89,6 +89,7 @@ public class DefaultJRubyIssues implements JRubyIssues { IssueChange change = IssueChange.create(); if (props.containsKey("newSeverity")) { change.setSeverity((String) props.get("newSeverity")); + change.setManualSeverity(true); } if (props.containsKey("newDesc")) { change.setDescription((String) props.get("newDesc")); @@ -108,8 +109,10 @@ public class DefaultJRubyIssues implements JRubyIssues { if (props.containsKey("newTitle")) { change.setTitle((String) props.get("newTitle")); } - - // TODO set attribute + comment + if (props.containsKey("comment")) { + change.setComment((String) props.get("comment")); + } + // TODO set attribute and login return change; } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java index a997eb93a3d..f5fad23757d 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java @@ -19,6 +19,7 @@ */ package org.sonar.server.issue; +import org.sonar.api.ServerComponent; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueChange; import org.sonar.api.web.UserRole; @@ -26,6 +27,7 @@ import org.sonar.core.issue.UpdateIssueFields; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueDao; import org.sonar.core.issue.IssueDto; +import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.core.user.AuthorizationDao; import javax.annotation.Nullable; @@ -34,16 +36,19 @@ import java.util.Arrays; /** * @since 3.6 */ -public class ServerIssueChanges { +public class ServerIssueChanges implements ServerComponent { + private final IssueWorkflow workflow; private final IssueDao issueDao; private final AuthorizationDao authorizationDao; - public ServerIssueChanges(IssueDao issueDao, AuthorizationDao authorizationDao) { + public ServerIssueChanges(IssueWorkflow workflow, IssueDao issueDao, AuthorizationDao authorizationDao) { + this.workflow = workflow; this.issueDao = issueDao; this.authorizationDao = authorizationDao; } + public Issue change(String issueKey, IssueChange change, @Nullable Integer userId) { if (userId == null) { // must be logged @@ -59,7 +64,7 @@ public class ServerIssueChanges { } DefaultIssue issue = dto.toDefaultIssue(); if (change.hasChanges()) { - UpdateIssueFields.apply(issue, change); + workflow.change(issue, change); issueDao.update(Arrays.asList(IssueDto.toDto(issue, dto.getResourceId(), dto.getRuleId()))); } return issue; diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index 135ee866847..ecbd17a2f4d 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -40,6 +40,7 @@ import org.sonar.core.config.Logback; import org.sonar.core.i18n.GwtI18n; import org.sonar.core.i18n.I18nManager; import org.sonar.core.i18n.RuleI18nManager; +import org.sonar.core.issue.workflow.IssueWorkflow; import org.sonar.server.issue.ServerIssueFinder; import org.sonar.core.measure.MeasureFilterEngine; import org.sonar.core.measure.MeasureFilterExecutor; @@ -235,6 +236,9 @@ public final class Platform { servicesContainer.addSingleton(DryRunDatabaseFactory.class); servicesContainer.addSingleton(DefaultResourcePermissions.class); servicesContainer.addSingleton(Periods.class); + + // issues + servicesContainer.addSingleton(IssueWorkflow.class); servicesContainer.addSingleton(ServerIssueChanges.class); servicesContainer.addSingleton(ServerIssueFinder.class); servicesContainer.addSingleton(DefaultJRubyIssues.class); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java index 90940313998..a44cff6d0ba 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/DefaultIssueClient.java @@ -51,7 +51,7 @@ public class DefaultIssueClient implements IssueClient { } @Override - public void apply(String issueKey, IssueChange change) { + public void change(String issueKey, IssueChange change) { if (!change.urlParams().isEmpty()) { Map queryParams = new LinkedHashMap(change.urlParams()); queryParams.put("key", issueKey); @@ -72,7 +72,7 @@ public class DefaultIssueClient implements IssueClient { @Override public void comment(String issueKey, String comment) { - apply(issueKey, IssueChange.create().comment(comment)); + change(issueKey, IssueChange.create().comment(comment)); } } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueChange.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueChange.java index 319736e8016..c4f2e163115 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueChange.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueChange.java @@ -51,7 +51,7 @@ public class IssueChange { * Add a comment */ public IssueChange comment(String s) { - params.put("newComment", s); + params.put("comment", s); return this; } @@ -60,8 +60,13 @@ public class IssueChange { return this; } - public IssueChange resolution(String s) { - params.put("newResolution", s); + public IssueChange line(Integer i) { + params.put("newLine", i); + return this; + } + + public IssueChange transition(String s) { + params.put("transition", s); return this; } @@ -71,7 +76,7 @@ public class IssueChange { } public IssueChange description(String s) { - params.put("newDescription", s); + params.put("newDesc", s); return this; } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java index 3258de32d3e..23463f884e7 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/issue/IssueClient.java @@ -26,12 +26,12 @@ public interface IssueClient { Issues find(IssueQuery query); - void apply(String issueKey, IssueChange change); + void change(String issueKey, IssueChange change); void create(NewIssue issue); /** - * Shortcut for {@code #apply(issueKey, IssueChange.create().comment(comment)} + * Shortcut for {@code #change(issueKey, IssueChange.create().comment(comment)} */ void comment(String issueKey, String comment); } diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/DefaultIssueClientTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/DefaultIssueClientTest.java index f056867fe62..f234952cae7 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/DefaultIssueClientTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/DefaultIssueClientTest.java @@ -64,9 +64,9 @@ public class DefaultIssueClientTest { HttpRequestFactory requestFactory = new HttpRequestFactory(httpServer.url(), null, null); IssueClient client = new DefaultIssueClient(requestFactory); - client.apply("ABCDE", IssueChange.create().severity("BLOCKER").comment("because!")); + client.change("ABCDE", IssueChange.create().severity("BLOCKER").comment("because!")); - assertThat(httpServer.requestedPath()).isEqualTo("/api/issues/change?newSeverity=BLOCKER&newComment=because!&key=ABCDE"); + assertThat(httpServer.requestedPath()).isEqualTo("/api/issues/change?newSeverity=BLOCKER&comment=because!&key=ABCDE"); } @Test @@ -74,7 +74,7 @@ public class DefaultIssueClientTest { HttpRequestFactory requestFactory = new HttpRequestFactory(httpServer.url(), null, null); IssueClient client = new DefaultIssueClient(requestFactory); - client.apply("ABCDE", IssueChange.create()); + client.change("ABCDE", IssueChange.create()); assertThat(httpServer.requestedPath()).isNull(); } diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueChangeTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueChangeTest.java index 0305b205088..a373a91c4f9 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueChangeTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/issue/IssueChangeTest.java @@ -39,15 +39,15 @@ public class IssueChangeTest { .attribute("JIRA", "FOO-1234") .attribute("LINK", null) .cost(4.2) - .resolution("FIXED") + .transition("resolve") .severity("BLOCKER"); assertThat(change.urlParams()).hasSize(7).includes( - entry("newComment", "this is a comment"), + entry("comment", "this is a comment"), entry("newAssignee", "lancelot"), entry("newAttr[JIRA]", "FOO-1234"), entry("newAttr[LINK]", ""), entry("newCost", 4.2), - entry("newResolution", "FIXED"), + entry("transition", "resolve"), entry("newSeverity", "BLOCKER") ); } -- 2.39.5