From c82a64fb8ea2dc4b031dd1bb5e766c846fdbd443 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Thu, 12 Jan 2012 18:30:40 +0100 Subject: [PATCH] SONAR-3012 New widget to monitor the review activity - 5 new metrics added - Decorator implemented to compute those metrics - Widget implemented to report those metrics --- .../org/sonar/plugins/core/CorePlugin.java | 3 + .../core/sensors/ManualViolationInjector.java | 2 +- .../sensors/ReviewsMeasuresDecorator.java | 90 ++++++++++ .../widgets/reviews/ReviewsMetricsWidget.java | 40 +++++ .../widgets/reviews/reviews_metrics.html.erb | 50 ++++++ .../sensors/ReviewsMeasuresDecoratorTest.java | 162 ++++++++++++++++++ .../ReviewsMeasuresDecoratorTest/fixture.xml | 69 ++++++++ .../resources/org/sonar/l10n/core.properties | 29 ++++ .../java/org/sonar/core/review/ReviewDao.java | 28 +-- .../java/org/sonar/core/review/ReviewDto.java | 3 + .../org/sonar/core/review/ReviewMapper.java | 6 +- .../org/sonar/core/review/ReviewQuery.java | 69 ++++++-- .../org/sonar/core/review/ReviewMapper.xml | 39 +++-- .../org/sonar/core/review/ReviewDaoTest.java | 77 +++++++-- .../core/review/ReviewDaoTest/shared.xml | 46 ++++- .../org/sonar/api/measures/CoreMetrics.java | 96 +++++++++++ .../org/sonar/server/ui/DefaultPages.java | 3 +- 17 files changed, 750 insertions(+), 62 deletions(-) create mode 100644 plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecorator.java create mode 100644 plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/widgets/reviews/ReviewsMetricsWidget.java create mode 100644 plugins/sonar-core-plugin/src/main/resources/org/sonar/plugins/core/widgets/reviews/reviews_metrics.html.erb create mode 100644 plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest.java create mode 100644 plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest/fixture.xml 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 463ccb89502..24319ea90df 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 @@ -48,6 +48,7 @@ import org.sonar.plugins.core.widgets.reviews.FalsePositiveReviewsWidget; import org.sonar.plugins.core.widgets.reviews.MyReviewsWidget; import org.sonar.plugins.core.widgets.reviews.PlannedReviewsWidget; import org.sonar.plugins.core.widgets.reviews.ProjectReviewsWidget; +import org.sonar.plugins.core.widgets.reviews.ReviewsMetricsWidget; import org.sonar.plugins.core.widgets.reviews.ReviewsPerDeveloperWidget; import org.sonar.plugins.core.widgets.reviews.UnplannedReviewsWidget; @@ -248,6 +249,7 @@ public class CorePlugin extends SonarPlugin { extensions.add(PlannedReviewsWidget.class); extensions.add(UnplannedReviewsWidget.class); extensions.add(ActionPlansWidget.class); + extensions.add(ReviewsMetricsWidget.class); // dashboards extensions.add(DefaultDashboard.class); @@ -291,6 +293,7 @@ public class CorePlugin extends SonarPlugin { extensions.add(UpdateReviewsDecorator.class); extensions.add(ViolationSeverityUpdater.class); extensions.add(IndexProjectPostJob.class); + extensions.add(ReviewsMeasuresDecorator.class); // time machine extensions.add(TendencyDecorator.class); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java index 78065d011db..5a5c0d64d02 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java @@ -51,7 +51,7 @@ public class ManualViolationInjector implements Decorator { public void decorate(Resource resource, DecoratorContext context) { if (resource.getId() != null) { - ReviewQuery query = ReviewQuery.create().setManualViolation(true).setResourceId(resource.getId()).setStatus(ReviewDto.STATUS_OPEN); + ReviewQuery query = ReviewQuery.create().setManualViolation(true).setResourceId(resource.getId()).addStatus(ReviewDto.STATUS_OPEN); List reviewDtos = reviewDao.selectByQuery(query); for (ReviewDto reviewDto : reviewDtos) { if (reviewDto.getRuleId() == null) { diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecorator.java new file mode 100644 index 00000000000..706cc6fc047 --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecorator.java @@ -0,0 +1,90 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar 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. + * + * Sonar 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 Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.core.sensors; + +import org.sonar.api.batch.Decorator; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.batch.DependsUpon; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.api.resources.ResourceUtils; +import org.sonar.core.review.ReviewDao; +import org.sonar.core.review.ReviewDto; +import org.sonar.core.review.ReviewQuery; +import org.sonar.plugins.core.timemachine.ViolationTrackingDecorator; + +/** + * Decorator that creates measures related to reviews. + */ +@DependsUpon(CloseReviewsDecorator.REVIEW_LIFECYCLE_BARRIER) +public class ReviewsMeasuresDecorator implements Decorator { + + private ReviewDao reviewDao; + + public ReviewsMeasuresDecorator(ReviewDao reviewDao) { + this.reviewDao = reviewDao; + } + + public boolean shouldExecuteOnProject(Project project) { + return project.isLatestAnalysis(); + } + + @SuppressWarnings("rawtypes") + @DependsUpon + public Class dependsUponViolationTracking() { + // permanent ids of violations have been updated, so we can link them with reviews + return ViolationTrackingDecorator.class; + } + + @SuppressWarnings({"rawtypes"}) + public void decorate(Resource resource, DecoratorContext context) { + if (!ResourceUtils.isFile(resource)) { + return; + } + + // Open reviews + ReviewQuery openReviewQuery = ReviewQuery.create().setResourceId(resource.getId()).addStatus(ReviewDto.STATUS_OPEN) + .addStatus(ReviewDto.STATUS_REOPENED); + Integer openReviewsCount = reviewDao.countByQuery(openReviewQuery); + context.saveMeasure(CoreMetrics.ACTIVE_REVIEWS, openReviewsCount.doubleValue()); + + // Unassigned reviews + ReviewQuery unassignedReviewQuery = ReviewQuery.copy(openReviewQuery).setNoAssignee(); + Integer unassignedReviewsCount = reviewDao.countByQuery(unassignedReviewQuery); + context.saveMeasure(CoreMetrics.UNASSIGNED_REVIEWS, unassignedReviewsCount.doubleValue()); + + // Unplanned reviews + ReviewQuery plannedReviewQuery = ReviewQuery.copy(openReviewQuery).setPlanned(); + int plannedReviewsCount = reviewDao.countByQuery(plannedReviewQuery); + context.saveMeasure(CoreMetrics.UNPLANNED_REVIEWS, (double) (openReviewsCount - plannedReviewsCount)); + + // False positive reviews + ReviewQuery falsePositiveReviewQuery = ReviewQuery.create().setResourceId(resource.getId()) + .addResolution(ReviewDto.RESOLUTION_FALSE_POSITIVE); + Integer falsePositiveReviewsCount = reviewDao.countByQuery(falsePositiveReviewQuery); + context.saveMeasure(CoreMetrics.FALSE_POSITIVE_REVIEWS, falsePositiveReviewsCount.doubleValue()); + + // Violations without a review + int violationsCount = context.getViolations().size(); + context.saveMeasure(CoreMetrics.VIOLATIONS_WITHOUT_REVIEW, (double) (violationsCount - openReviewsCount - falsePositiveReviewsCount)); + } + +} diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/widgets/reviews/ReviewsMetricsWidget.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/widgets/reviews/ReviewsMetricsWidget.java new file mode 100644 index 00000000000..7ad4e69cd18 --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/widgets/reviews/ReviewsMetricsWidget.java @@ -0,0 +1,40 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar 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. + * + * Sonar 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 Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.core.widgets.reviews; + +import org.sonar.api.web.AbstractRubyTemplate; +import org.sonar.api.web.RubyRailsWidget; +import org.sonar.api.web.WidgetCategory; + +@WidgetCategory({ "Reviews" }) +public class ReviewsMetricsWidget extends AbstractRubyTemplate implements RubyRailsWidget { + public String getId() { + return "reviews_metrics"; + } + + public String getTitle() { + return "Reviews metrics"; + } + + @Override + protected String getTemplatePath() { + return "/org/sonar/plugins/core/widgets/reviews/reviews_metrics.html.erb"; + } +} diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/plugins/core/widgets/reviews/reviews_metrics.html.erb b/plugins/sonar-core-plugin/src/main/resources/org/sonar/plugins/core/widgets/reviews/reviews_metrics.html.erb new file mode 100644 index 00000000000..3388d4fe12e --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/plugins/core/widgets/reviews/reviews_metrics.html.erb @@ -0,0 +1,50 @@ +<% + active_reviews=measure('active_reviews') + unassigned_reviews=measure('unassigned_reviews') + unplanned_reviews=measure('unplanned_reviews') + false_positive_reviews=measure('false_positive_reviews') + violations_without_review=measure('violations_without_review') +%> + + + + + + +
+
+

<%= message('widget.reviews_metrics.active_reviews') -%>

+ <% if active_reviews %> +

+ <%= format_measure(active_reviews, :suffix => '', :url => url_for_drilldown(active_reviews)) -%> + <%= dashboard_configuration.selected_period? ? format_variation(active_reviews) : trend_icon(active_reviews) -%> +

+

+ <%= format_measure(unassigned_reviews, :suffix => message('widget.reviews_metrics.unassigned.suffix'), :url => url_for_drilldown(unassigned_reviews)) -%> + <%= dashboard_configuration.selected_period? ? format_variation(unassigned_reviews) : trend_icon(unassigned_reviews) -%> +

+

+ <%= format_measure(unplanned_reviews, :suffix => message('widget.reviews_metrics.unplanned.suffix'), :url => url_for_drilldown(unplanned_reviews)) -%> + <%= dashboard_configuration.selected_period? ? format_variation(unplanned_reviews) : trend_icon(unplanned_reviews) -%> +

+

+ <%= format_measure(false_positive_reviews, :suffix => message('widget.reviews_metrics.false_positives.suffix'), :url => url_for_drilldown(false_positive_reviews)) -%> + <%= dashboard_configuration.selected_period? ? format_variation(false_positive_reviews) : trend_icon(false_positive_reviews) -%> +

+ <% else %> + <%= message('widget.reviews_metrics.no_data') -%> + <% end %> +
+ 		+
+

<%= message('widget.reviews_metrics.unreviewed_violations') -%>

+ <% if violations_without_review %> +

+ <%= format_measure(violations_without_review, :suffix => '', :url => url_for_drilldown(violations_without_review)) -%> + <%= dashboard_configuration.selected_period? ? format_variation(violations_without_review) : trend_icon(violations_without_review) -%> +

+ <% else %> + <%= message('widget.reviews_metrics.no_data') -%> + <% end %> +
+
\ No newline at end of file diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest.java new file mode 100644 index 00000000000..b72c4b989f8 --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest.java @@ -0,0 +1,162 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar 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. + * + * Sonar 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 Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.core.sensors; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyDouble; +import static org.mockito.Matchers.argThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.junit.Test; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.Metric; +import org.sonar.api.resources.File; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.api.rules.Violation; +import org.sonar.core.review.ReviewDao; +import org.sonar.core.review.ReviewDto; +import org.sonar.core.review.ReviewQuery; +import org.sonar.plugins.core.timemachine.ViolationTrackingDecorator; + +import com.google.common.collect.Lists; + +public class ReviewsMeasuresDecoratorTest { + + @Test + public void testDependsUponViolationTracking() throws Exception { + ReviewsMeasuresDecorator decorator = new ReviewsMeasuresDecorator(null); + assertEquals(decorator.dependsUponViolationTracking(), ViolationTrackingDecorator.class); + } + + @Test + public void shouldExecuteOnProject() throws Exception { + ReviewsMeasuresDecorator decorator = new ReviewsMeasuresDecorator(null); + Project project = new Project("foo"); + project.setLatestAnalysis(true); + assertThat(decorator.shouldExecuteOnProject(project), is(true)); + } + + @Test + public void shouldDecorateOnlyFiles() throws Exception { + ReviewsMeasuresDecorator decorator = new ReviewsMeasuresDecorator(null); + DecoratorContext context = mock(DecoratorContext.class); + Resource resource = new Project("Foo"); + decorator.decorate(resource, context); + verify(context, never()).saveMeasure(any(Metric.class), anyDouble()); + } + + @Test + public void shouldComputeReviewMetrics() throws Exception { + ReviewDao reviewDao = mock(ReviewDao.class); + when(reviewDao.countByQuery(argThat(openReviewQueryMatcher()))).thenReturn(10); + when(reviewDao.countByQuery(argThat(unassignedReviewQueryMatcher()))).thenReturn(2); + when(reviewDao.countByQuery(argThat(plannedReviewQueryMatcher()))).thenReturn(3); + when(reviewDao.countByQuery(argThat(falsePositiveReviewQueryMatcher()))).thenReturn(4); + + ReviewsMeasuresDecorator decorator = new ReviewsMeasuresDecorator(reviewDao); + Resource resource = new File("foo").setId(1); + DecoratorContext context = mock(DecoratorContext.class); + List violations = mock(List.class); + when(violations.size()).thenReturn(35); + when(context.getViolations()).thenReturn(violations); + decorator.decorate(resource, context); + + verify(context).saveMeasure(CoreMetrics.ACTIVE_REVIEWS, 10d); + verify(context).saveMeasure(CoreMetrics.UNASSIGNED_REVIEWS, 2d); + verify(context).saveMeasure(CoreMetrics.UNPLANNED_REVIEWS, 7d); + verify(context).saveMeasure(CoreMetrics.FALSE_POSITIVE_REVIEWS, 4d); + verify(context).saveMeasure(CoreMetrics.VIOLATIONS_WITHOUT_REVIEW, 21d); + } + + private BaseMatcher openReviewQueryMatcher() { + return new BaseMatcher() { + public boolean matches(Object o) { + ReviewQuery query = (ReviewQuery) o; + if (query == null) { + return false; + } + return Lists.newArrayList(ReviewDto.STATUS_OPEN, ReviewDto.STATUS_REOPENED).equals(query.getStatuses()) + && query.getNoAssignee() == null && query.getPlanned() == null; + } + + public void describeTo(Description description) { + } + }; + } + + private BaseMatcher unassignedReviewQueryMatcher() { + return new BaseMatcher() { + public boolean matches(Object o) { + ReviewQuery query = (ReviewQuery) o; + if (query == null) { + return false; + } + return Lists.newArrayList(ReviewDto.STATUS_OPEN, ReviewDto.STATUS_REOPENED).equals(query.getStatuses()) + && query.getNoAssignee() == Boolean.TRUE; + } + + public void describeTo(Description description) { + } + }; + } + + private BaseMatcher plannedReviewQueryMatcher() { + return new BaseMatcher() { + public boolean matches(Object o) { + ReviewQuery query = (ReviewQuery) o; + if (query == null) { + return false; + } + return Lists.newArrayList(ReviewDto.STATUS_OPEN, ReviewDto.STATUS_REOPENED).equals(query.getStatuses()) + && query.getPlanned() == Boolean.TRUE; + } + + public void describeTo(Description description) { + } + }; + } + + private BaseMatcher falsePositiveReviewQueryMatcher() { + return new BaseMatcher() { + public boolean matches(Object o) { + ReviewQuery query = (ReviewQuery) o; + if (query == null) { + return false; + } + return Lists.newArrayList(ReviewDto.RESOLUTION_FALSE_POSITIVE).equals(query.getResolutions()); + } + + public void describeTo(Description description) { + } + }; + } +} diff --git a/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest/fixture.xml b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest/fixture.xml new file mode 100644 index 00000000000..7384c0e2e5f --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/resources/org/sonar/plugins/core/sensors/ReviewsMeasuresDecoratorTest/fixture.xml @@ -0,0 +1,69 @@ + + + + + + + + + \ No newline at end of file diff --git a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties index 5798be1507c..dbb2a5c49fc 100644 --- a/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-l10n-en-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -719,6 +719,15 @@ widget.planned_reviews.no_action_plan=No action plan widget.unplanned_reviews.name=Unplanned reviews widget.unplanned_reviews.description=Shows all the reviews of the project that are not planned yet in an action plan +widget.reviews_metrics.name=Reviews metrics +widget.reviews_metrics.description=Reports metrics about reviews +widget.reviews_metrics.no_data=No data +widget.reviews_metrics.active_reviews=Active reviews +widget.reviews_metrics.unassigned.suffix=\ unassigned +widget.reviews_metrics.unplanned.suffix=\ unplanned +widget.reviews_metrics.false_positives.suffix=\ false-positives +widget.reviews_metrics.unreviewed_violations=Unreviewed violations + #------------------------------------------------------------------------------ @@ -1542,3 +1551,23 @@ metric.business_value.description=An indication on the value of the project for metric.team_size.name=Team size metric.team_size.description=Size of the project team + +#-------------------------------------------------------------------------------------------------------------------- +# +# REVIEWS METRICS +# +#-------------------------------------------------------------------------------------------------------------------- +metric.violations_without_review.name=Unreviewed violations +metric.violations_without_review.description=Violations that have not been reviewed yet + +metric.false_positive_reviews.name=False-positive reviews +metric.false_positive_reviews.description=Active false-positive reviews + +metric.active_reviews.name=Active reviews +metric.active_reviews.description=Active open and reopened reviews + +metric.unassigned_reviews.name=Unassigned reviews +metric.unassigned_reviews.description=Active unassigned reviews + +metric.unplanned_reviews.name=Unplanned reviews +metric.unplanned_reviews.description=Active unplanned reviews diff --git a/sonar-core/src/main/java/org/sonar/core/review/ReviewDao.java b/sonar-core/src/main/java/org/sonar/core/review/ReviewDao.java index 27ea0b80764..438bb911021 100644 --- a/sonar-core/src/main/java/org/sonar/core/review/ReviewDao.java +++ b/sonar-core/src/main/java/org/sonar/core/review/ReviewDao.java @@ -19,13 +19,14 @@ */ package org.sonar.core.review; -import com.google.common.collect.Lists; +import java.util.List; + import org.apache.ibatis.session.SqlSession; import org.sonar.api.BatchComponent; import org.sonar.api.ServerComponent; import org.sonar.core.persistence.MyBatis; -import java.util.List; +import com.google.common.collect.Lists; public class ReviewDao implements BatchComponent, ServerComponent { private final MyBatis mybatis; @@ -44,29 +45,36 @@ public class ReviewDao implements BatchComponent, ServerComponent { } } - public List selectByResource(int resourceId) { + public List selectByQuery(ReviewQuery query) { SqlSession sqlSession = mybatis.openSession(); try { ReviewMapper mapper = sqlSession.getMapper(ReviewMapper.class); - return mapper.selectByResource(resourceId); + List result; + if (query.needToPartitionQuery()) { + result = Lists.newArrayList(); + for (ReviewQuery partitionedQuery : query.partition()) { + result.addAll(mapper.selectByQuery(partitionedQuery)); + } + } else { + result = mapper.selectByQuery(query); + } + return result; } finally { sqlSession.close(); } } - public List selectByQuery(ReviewQuery query) { + public Integer countByQuery(ReviewQuery query) { SqlSession sqlSession = mybatis.openSession(); try { ReviewMapper mapper = sqlSession.getMapper(ReviewMapper.class); - List result; + Integer result = 0; if (query.needToPartitionQuery()) { - result = Lists.newArrayList(); for (ReviewQuery partitionedQuery : query.partition()) { - result.addAll(mapper.selectByQuery(partitionedQuery)); + result += mapper.countByQuery(partitionedQuery); } - } else { - result = mapper.selectByQuery(query); + result = mapper.countByQuery(query); } return result; } finally { diff --git a/sonar-core/src/main/java/org/sonar/core/review/ReviewDto.java b/sonar-core/src/main/java/org/sonar/core/review/ReviewDto.java index 7f349a6b627..9fe21da9b2e 100644 --- a/sonar-core/src/main/java/org/sonar/core/review/ReviewDto.java +++ b/sonar-core/src/main/java/org/sonar/core/review/ReviewDto.java @@ -34,6 +34,9 @@ public final class ReviewDto { public static final String STATUS_RESOLVED = "RESOLVED"; public static final String STATUS_CLOSED = "CLOSED"; + public static final String RESOLUTION_FALSE_POSITIVE = "FALSE-POSITIVE"; + public static final String RESOLUTION_FIXED = "FIXED"; + private Long id; private Integer userId; private Integer assigneeId; diff --git a/sonar-core/src/main/java/org/sonar/core/review/ReviewMapper.java b/sonar-core/src/main/java/org/sonar/core/review/ReviewMapper.java index 89c7a6a84c5..1048128212f 100644 --- a/sonar-core/src/main/java/org/sonar/core/review/ReviewMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/review/ReviewMapper.java @@ -19,8 +19,6 @@ */ package org.sonar.core.review; -import org.apache.ibatis.annotations.Param; - import java.util.List; /** @@ -29,7 +27,7 @@ import java.util.List; public interface ReviewMapper { ReviewDto selectById(long id); - List selectByResource(int resourceId); - List selectByQuery(ReviewQuery query); + + Integer countByQuery(ReviewQuery query); } diff --git a/sonar-core/src/main/java/org/sonar/core/review/ReviewQuery.java b/sonar-core/src/main/java/org/sonar/core/review/ReviewQuery.java index afaa5ec369d..9481adc008e 100644 --- a/sonar-core/src/main/java/org/sonar/core/review/ReviewQuery.java +++ b/sonar-core/src/main/java/org/sonar/core/review/ReviewQuery.java @@ -19,12 +19,13 @@ */ package org.sonar.core.review; -import com.google.common.collect.Lists; -import org.sonar.core.persistence.DatabaseUtils; - import java.util.Collection; import java.util.List; +import org.sonar.core.persistence.DatabaseUtils; + +import com.google.common.collect.Lists; + /** * @since 2.13 */ @@ -35,27 +36,35 @@ public final class ReviewQuery { private Integer userId; private List violationPermanentIds; private Integer ruleId; - private String status; - private String resolution; + private List statuses; + private List resolutions; + private Boolean noAssignee; + private Boolean planned; private ReviewQuery() { } - private ReviewQuery(ReviewQuery other, List permanentIds) { + private ReviewQuery(ReviewQuery other) { this.manualViolation = other.manualViolation; this.manualSeverity = other.manualSeverity; this.resourceId = other.resourceId; this.userId = other.userId; - this.violationPermanentIds = permanentIds; + this.violationPermanentIds = other.violationPermanentIds; this.ruleId = other.ruleId; - this.status = other.status; - this.resolution = other.resolution; + this.statuses = other.statuses; + this.resolutions = other.resolutions; + this.noAssignee = other.noAssignee; + this.planned = other.planned; } public static ReviewQuery create() { return new ReviewQuery(); } + public static ReviewQuery copy(ReviewQuery reviewQuery) { + return new ReviewQuery(reviewQuery); + } + public Boolean getManualViolation() { return manualViolation; } @@ -74,12 +83,15 @@ public final class ReviewQuery { return this; } - public String getStatus() { - return status; + public List getStatuses() { + return statuses; } - public ReviewQuery setStatus(String status) { - this.status = status; + public ReviewQuery addStatus(String status) { + if (statuses == null) { + statuses = Lists.newArrayList(); + } + statuses.add(status); return this; } @@ -110,12 +122,15 @@ public final class ReviewQuery { return this; } - public String getResolution() { - return resolution; + public List getResolutions() { + return resolutions; } - public ReviewQuery setResolution(String resolution) { - this.resolution = resolution; + public ReviewQuery addResolution(String resolution) { + if (resolutions == null) { + resolutions = Lists.newArrayList(); + } + resolutions.add(resolution); return this; } @@ -128,6 +143,24 @@ public final class ReviewQuery { return this; } + public Boolean getNoAssignee() { + return noAssignee; + } + + public ReviewQuery setNoAssignee() { + this.noAssignee = Boolean.TRUE; + return this; + } + + public Boolean getPlanned() { + return planned; + } + + public ReviewQuery setPlanned() { + this.planned = Boolean.TRUE; + return this; + } + boolean needToPartitionQuery() { return violationPermanentIds != null && violationPermanentIds.size() > DatabaseUtils.MAX_IN_ELEMENTS; } @@ -136,7 +169,7 @@ public final class ReviewQuery { List> partitions = Lists.partition(violationPermanentIds, DatabaseUtils.MAX_IN_ELEMENTS); ReviewQuery[] result = new ReviewQuery[partitions.size()]; for (int index = 0; index < partitions.size(); index++) { - result[index] = new ReviewQuery(this, partitions.get(index)); + result[index] = ReviewQuery.copy(this).setViolationPermanentIds(partitions.get(index)); } return result; diff --git a/sonar-core/src/main/resources/org/sonar/core/review/ReviewMapper.xml b/sonar-core/src/main/resources/org/sonar/core/review/ReviewMapper.xml index 2f897ff6e1c..8af600daad9 100644 --- a/sonar-core/src/main/resources/org/sonar/core/review/ReviewMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/review/ReviewMapper.xml @@ -26,17 +26,10 @@ from reviews where id=#{id} - - - - + select + + + + + + diff --git a/sonar-core/src/test/java/org/sonar/core/review/ReviewDaoTest.java b/sonar-core/src/test/java/org/sonar/core/review/ReviewDaoTest.java index de1806a978d..3a307c0f253 100644 --- a/sonar-core/src/test/java/org/sonar/core/review/ReviewDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/review/ReviewDaoTest.java @@ -19,18 +19,22 @@ */ package org.sonar.core.review; -import com.google.common.collect.Lists; +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; + +import java.util.List; + import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.junit.Before; import org.junit.Test; import org.sonar.core.persistence.DaoTestCase; -import java.util.List; - -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; +import com.google.common.collect.Lists; public class ReviewDaoTest extends DaoTestCase { @@ -48,7 +52,7 @@ public class ReviewDaoTest extends DaoTestCase { ReviewDto reviewDto = dao.selectById(100L); assertThat(reviewDto.getId(), is(100L)); assertThat(reviewDto.getStatus(), is("OPEN")); - assertThat(reviewDto.getResolution(), is("RESOLVE")); + assertThat(reviewDto.getResolution(), is(nullValue())); assertThat(reviewDto.getProjectId(), is(20)); assertThat(reviewDto.getViolationPermanentId(), is(1)); assertThat(reviewDto.getSeverity(), is("BLOCKER")); @@ -66,10 +70,10 @@ public class ReviewDaoTest extends DaoTestCase { } @Test - public void shouldSelectByResource() { + public void shouldSelectByQuery() { setupData("shared"); - List reviewDtos = dao.selectByResource(400); + List reviewDtos = dao.selectByQuery(ReviewQuery.create().setResourceId(400)); assertThat(reviewDtos.size(), is(2)); for (ReviewDto reviewDto : reviewDtos) { assertThat(reviewDto.getId(), anyOf(is(100L), is(101L))); @@ -78,19 +82,62 @@ public class ReviewDaoTest extends DaoTestCase { } @Test - public void shouldSelectByQuery() { + public void shouldSelectByQueryWithStatuses() { setupData("shared"); - List reviewDtos = dao.selectByQuery(ReviewQuery.create().setResourceId(400)); + List reviewDtos = dao.selectByQuery(ReviewQuery.create().addStatus(ReviewDto.STATUS_OPEN) + .addStatus(ReviewDto.STATUS_REOPENED)); + assertThat(reviewDtos.size(), is(3)); + for (ReviewDto reviewDto : reviewDtos) { + assertThat(reviewDto.getId(), anyOf(is(100L), is(102L), is(103L))); + } + } + + @Test + public void shouldSelectByQueryWithResolutions() { + setupData("shared"); + + List reviewDtos = dao.selectByQuery(ReviewQuery.create().addResolution(ReviewDto.RESOLUTION_FALSE_POSITIVE) + .addResolution(ReviewDto.RESOLUTION_FIXED)); + assertThat(reviewDtos.size(), is(2)); + for (ReviewDto reviewDto : reviewDtos) { + assertThat(reviewDto.getId(), anyOf(is(101L), is(104L))); + } + } + + @Test + public void shouldSelectByQueryWithNoAssignee() { + setupData("shared"); + + List reviewDtos = dao.selectByQuery(ReviewQuery.create().setNoAssignee()); + assertThat(reviewDtos.size(), is(2)); + for (ReviewDto reviewDto : reviewDtos) { + assertThat(reviewDto.getId(), anyOf(is(101L), is(103L))); + } + } + + @Test + public void shouldSelectByQueryWithPlanned() { + setupData("shared"); + + List reviewDtos = dao.selectByQuery(ReviewQuery.create().setPlanned()); assertThat(reviewDtos.size(), is(2)); for (ReviewDto reviewDto : reviewDtos) { assertThat(reviewDto.getId(), anyOf(is(100L), is(101L))); - assertThat(reviewDto.getResourceId(), is(400)); } } @Test - public void shouldSelectByQuery_booleanCriteria() { + public void shouldCountByQuery() { + setupData("shared"); + + Integer count = dao.countByQuery(ReviewQuery.create().addStatus(ReviewDto.STATUS_OPEN) + .addStatus(ReviewDto.STATUS_REOPENED)); + assertThat(count, is(3)); + } + + @Test + public void shouldSelectByQueryWithBooleanCriteria() { setupData("shared"); List reviewDtos = dao.selectByQuery(ReviewQuery.create().setResourceId(400).setManualViolation(true)); @@ -111,12 +158,16 @@ public class ReviewDaoTest extends DaoTestCase { } ReviewQuery query = ReviewQuery.create().setViolationPermanentIds(permanentIds); + // test select query List reviewDtos = dao.selectByQuery(query); assertThat(reviewDtos.size(), is(3)); assertThat(reviewDtos, hasItem(new ReviewMatcherByViolationPermanentId(100))); assertThat(reviewDtos, hasItem(new ReviewMatcherByViolationPermanentId(1300))); assertThat(reviewDtos, hasItem(new ReviewMatcherByViolationPermanentId(3200))); + + // and test count query + assertThat(dao.countByQuery(query), is(3)); } static class ReviewMatcherByViolationPermanentId extends BaseMatcher { diff --git a/sonar-core/src/test/resources/org/sonar/core/review/ReviewDaoTest/shared.xml b/sonar-core/src/test/resources/org/sonar/core/review/ReviewDaoTest/shared.xml index 70c101759de..376ff96eb8c 100644 --- a/sonar-core/src/test/resources/org/sonar/core/review/ReviewDaoTest/shared.xml +++ b/sonar-core/src/test/resources/org/sonar/core/review/ReviewDaoTest/shared.xml @@ -1,17 +1,21 @@ + + + + + + + diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/measures/CoreMetrics.java b/sonar-plugin-api/src/main/java/org/sonar/api/measures/CoreMetrics.java index 824aab6bdc8..01b28da154e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/measures/CoreMetrics.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/measures/CoreMetrics.java @@ -43,6 +43,7 @@ public final class CoreMetrics { public static final String DOMAIN_DOCUMENTATION = "Documentation"; public static final String DOMAIN_RULES = "Rules"; public static final String DOMAIN_SCM = "SCM"; + public static final String DOMAIN_REVIEWS = "Reviews"; /** * @deprecated since 2.5 See http://jira.codehaus.org/browse/SONAR-2007 @@ -1398,6 +1399,101 @@ public final class CoreMetrics { .create(); + //-------------------------------------------------------------------------------------------------------------------- + // + // REVIEWS (since 2.14) + // + //-------------------------------------------------------------------------------------------------------------------- + + /** + * @since 2.14 + */ + public static final String VIOLATIONS_WITHOUT_REVIEW_KEY = "violations_without_review"; + + /** + * @since 2.14 + */ + public static final Metric VIOLATIONS_WITHOUT_REVIEW = new Metric.Builder(VIOLATIONS_WITHOUT_REVIEW_KEY, "Unreviewed violations", Metric.ValueType.INT) + .setDescription("Violations that have not been reviewed yet") + .setDirection(Metric.DIRECTION_WORST) + .setDomain(DOMAIN_REVIEWS) + .setBestValue(0.0) + .setOptimizedBestValue(true) + .setFormula(new SumChildValuesFormula(false)) + .create(); + + /** + * @since 2.14 + */ + public static final String FALSE_POSITIVE_REVIEWS_KEY = "false_positive_reviews"; + + /** + * @since 2.14 + */ + public static final Metric FALSE_POSITIVE_REVIEWS = new Metric.Builder(FALSE_POSITIVE_REVIEWS_KEY, "False-positive reviews", Metric.ValueType.INT) + .setDescription("Active false-positive reviews") + .setDirection(Metric.DIRECTION_WORST) + .setDomain(DOMAIN_REVIEWS) + .setBestValue(0.0) + .setOptimizedBestValue(true) + .setFormula(new SumChildValuesFormula(false)) + .create(); + + /** + * @since 2.14 + */ + public static final String ACTIVE_REVIEWS_KEY = "active_reviews"; + + /** + * @since 2.14 + */ + public static final Metric ACTIVE_REVIEWS = new Metric.Builder(ACTIVE_REVIEWS_KEY, "Active reviews", Metric.ValueType.INT) + .setDescription("Active open and reopened reviews") + .setDirection(Metric.DIRECTION_WORST) + .setDomain(DOMAIN_REVIEWS) + .setBestValue(0.0) + .setOptimizedBestValue(true) + .setFormula(new SumChildValuesFormula(false)) + .create(); + + /** + * @since 2.14 + */ + public static final String UNASSIGNED_REVIEWS_KEY = "unassigned_reviews"; + + /** + * @since 2.14 + */ + public static final Metric UNASSIGNED_REVIEWS = new Metric.Builder(UNASSIGNED_REVIEWS_KEY, "Unassigned reviews", Metric.ValueType.INT) + .setDescription("Active unassigned reviews") + .setDirection(Metric.DIRECTION_WORST) + .setDomain(DOMAIN_REVIEWS) + .setBestValue(0.0) + .setOptimizedBestValue(true) + .setFormula(new SumChildValuesFormula(false)) + .create(); + + /** + * @since 2.14 + */ + public static final String UNPLANNED_REVIEWS_KEY = "unplanned_reviews"; + + /** + * @since 2.14 + */ + public static final Metric UNPLANNED_REVIEWS = new Metric.Builder(UNPLANNED_REVIEWS_KEY, "Unplanned reviews", Metric.ValueType.INT) + .setDescription("Active unplanned reviews") + .setDirection(Metric.DIRECTION_WORST) + .setDomain(DOMAIN_REVIEWS) + .setBestValue(0.0) + .setOptimizedBestValue(true) + .setFormula(new SumChildValuesFormula(false)) + .create(); + + + + + //-------------------------------------------------------------------------------------------------------------------- // // OTHERS diff --git a/sonar-server/src/main/java/org/sonar/server/ui/DefaultPages.java b/sonar-server/src/main/java/org/sonar/server/ui/DefaultPages.java index 970b2d1598b..d0268838f5c 100644 --- a/sonar-server/src/main/java/org/sonar/server/ui/DefaultPages.java +++ b/sonar-server/src/main/java/org/sonar/server/ui/DefaultPages.java @@ -93,7 +93,8 @@ public final class DefaultPages { @DefaultTab(metrics = {CoreMetrics.VIOLATIONS_DENSITY_KEY, CoreMetrics.WEIGHTED_VIOLATIONS_KEY, CoreMetrics.VIOLATIONS_KEY, CoreMetrics.BLOCKER_VIOLATIONS_KEY, CoreMetrics.CRITICAL_VIOLATIONS_KEY, CoreMetrics.MAJOR_VIOLATIONS_KEY, CoreMetrics.MINOR_VIOLATIONS_KEY, CoreMetrics.INFO_VIOLATIONS_KEY, CoreMetrics.NEW_VIOLATIONS_KEY, CoreMetrics.NEW_BLOCKER_VIOLATIONS_KEY, CoreMetrics.NEW_CRITICAL_VIOLATIONS_KEY, CoreMetrics.NEW_MAJOR_VIOLATIONS_KEY, - CoreMetrics.NEW_MINOR_VIOLATIONS_KEY, CoreMetrics.NEW_INFO_VIOLATIONS_KEY}) + CoreMetrics.NEW_MINOR_VIOLATIONS_KEY, CoreMetrics.NEW_INFO_VIOLATIONS_KEY, CoreMetrics.ACTIVE_REVIEWS_KEY, CoreMetrics.UNASSIGNED_REVIEWS_KEY, + CoreMetrics.UNPLANNED_REVIEWS_KEY, CoreMetrics.FALSE_POSITIVE_REVIEWS_KEY, CoreMetrics.VIOLATIONS_WITHOUT_REVIEW_KEY}) @ResourceQualifier({Qualifiers.VIEW, Qualifiers.SUBVIEW, Qualifiers.PROJECT, Qualifiers.MODULE, Qualifiers.PACKAGE, Qualifiers.DIRECTORY, Qualifiers.FILE, Qualifiers.CLASS}) /* all exept unit tests...*/ @UserRole(UserRole.CODEVIEWER) -- 2.39.5