From 85e7e49bcfa5fb64fd499060c279cd679c799e39 Mon Sep 17 00:00:00 2001 From: Fabrice Bellingard Date: Wed, 8 Feb 2012 10:15:56 +0100 Subject: [PATCH] SONAR-2747 Verification is now done on period1 And not necessarily 'since last analysis' period. --- .../timemachine/NewViolationsDecorator.java | 9 ++++---- .../NewViolationsDecoratorTest.java | 21 ++++-------------- .../NewViolationsEmailTemplate.java | 3 +-- .../NewViolationsEmailTemplateTest.java | 5 ++--- .../components/TimeMachineConfiguration.java | 22 ------------------- .../TimeMachineConfigurationTest.java | 20 ----------------- 6 files changed, 11 insertions(+), 69 deletions(-) diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/NewViolationsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/NewViolationsDecorator.java index 06a8bf377df..d714cada4fa 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/NewViolationsDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/NewViolationsDecorator.java @@ -209,11 +209,11 @@ public class NewViolationsDecorator implements Decorator { } protected void notifyNewViolations(Project project, DecoratorContext context) { - Integer lastAnalysisPeriodIndex = timeMachineConfiguration.getLastAnalysisPeriodIndex(); List projectPastSnapshots = timeMachineConfiguration.getProjectPastSnapshots(); - if (lastAnalysisPeriodIndex != null && projectPastSnapshots.size() >= lastAnalysisPeriodIndex) { - PastSnapshot pastSnapshot = projectPastSnapshots.get(lastAnalysisPeriodIndex - 1); - Double newViolationsCount = context.getMeasure(CoreMetrics.NEW_VIOLATIONS).getVariation(lastAnalysisPeriodIndex); + if (projectPastSnapshots.size() >= 1) { + // we always check new violations against period1 + PastSnapshot pastSnapshot = projectPastSnapshots.get(0); + Double newViolationsCount = context.getMeasure(CoreMetrics.NEW_VIOLATIONS).getVariation1(); // Do not send notification if this is the first analysis or if there's no violation if (pastSnapshot.getTargetDate() != null && newViolationsCount != null && newViolationsCount > 0) { // Maybe we should check if this is the first analysis or not? @@ -223,7 +223,6 @@ public class NewViolationsDecorator implements Decorator { .setFieldValue("projectName", project.getLongName()) .setFieldValue("projectKey", project.getKey()) .setFieldValue("projectId", String.valueOf(project.getId())) - .setFieldValue("period", lastAnalysisPeriodIndex.toString()) .setFieldValue("fromDate", dateformat.format(pastSnapshot.getTargetDate())) .setFieldValue("toDate", dateformat.format(new Date())); notificationManager.scheduleForSending(notification); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/NewViolationsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/NewViolationsDecoratorTest.java index e8695960212..a5ee668fa1c 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/NewViolationsDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/NewViolationsDecoratorTest.java @@ -32,6 +32,7 @@ import static org.mockito.Mockito.when; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; import java.util.Date; @@ -70,7 +71,7 @@ public class NewViolationsDecoratorTest { private NewViolationsDecorator decorator; private DecoratorContext context; - private Resource resource; + private Resource resource; private NotificationManager notificationManager; private Date rightNow; @@ -188,21 +189,11 @@ public class NewViolationsDecoratorTest { verify(notificationManager, never()).scheduleForSending(any(Notification.class)); } - @Test - public void shouldNotNotifyIfNoPeriodForLastAnalysis() throws Exception { - Project project = new Project("key"); - when(timeMachineConfiguration.getLastAnalysisPeriodIndex()).thenReturn(null); - - decorator.notifyNewViolations(project, context); - - verify(notificationManager, never()).scheduleForSending(any(Notification.class)); - } - @Test public void shouldNotNotifyIfNoNotEnoughPastSnapshots() throws Exception { Project project = new Project("key"); // the #setUp method adds 2 snapshots: if last period analysis is 3, then it's not enough - when(timeMachineConfiguration.getLastAnalysisPeriodIndex()).thenReturn(3); + when(timeMachineConfiguration.getProjectPastSnapshots()).thenReturn(new ArrayList()); decorator.notifyNewViolations(project, context); verify(notificationManager, never()).scheduleForSending(any(Notification.class)); @@ -211,7 +202,6 @@ public class NewViolationsDecoratorTest { @Test public void shouldNotNotifyIfNoNewViolations() throws Exception { Project project = new Project("key"); - when(timeMachineConfiguration.getLastAnalysisPeriodIndex()).thenReturn(1); Measure m = new Measure(CoreMetrics.NEW_VIOLATIONS); when(context.getMeasure(CoreMetrics.NEW_VIOLATIONS)).thenReturn(m); @@ -229,7 +219,6 @@ public class NewViolationsDecoratorTest { public void shouldNotNotifyUserIfFirstAnalysis() throws Exception { Project project = new Project("key").setName("LongName"); project.setId(45); - when(timeMachineConfiguration.getLastAnalysisPeriodIndex()).thenReturn(1); // PastSnapshot with targetDate==null means first analysis PastSnapshot pastSnapshot = new PastSnapshot("", null); when(timeMachineConfiguration.getProjectPastSnapshots()).thenReturn(Lists.newArrayList(pastSnapshot)); @@ -244,11 +233,10 @@ public class NewViolationsDecoratorTest { public void shouldNotifyUserAboutNewViolations() throws Exception { Project project = new Project("key").setName("LongName"); project.setId(45); - when(timeMachineConfiguration.getLastAnalysisPeriodIndex()).thenReturn(2); Calendar pastDate = new GregorianCalendar(2011, 10, 25); PastSnapshot pastSnapshot = new PastSnapshot("", pastDate.getTime()); when(timeMachineConfiguration.getProjectPastSnapshots()).thenReturn(Lists.newArrayList(pastSnapshot, pastSnapshot)); - Measure m = new Measure(CoreMetrics.NEW_VIOLATIONS).setVariation2(32.0); + Measure m = new Measure(CoreMetrics.NEW_VIOLATIONS).setVariation1(32.0); when(context.getMeasure(CoreMetrics.NEW_VIOLATIONS)).thenReturn(m); decorator.decorate(project, context); @@ -259,7 +247,6 @@ public class NewViolationsDecoratorTest { .setFieldValue("projectName", "LongName") .setFieldValue("projectKey", "key") .setFieldValue("projectId", "45") - .setFieldValue("period", "2") .setFieldValue("fromDate", dateformat.format(pastDate.getTime())) .setFieldValue("toDate", dateformat.format(new Date())); verify(notificationManager, times(1)).scheduleForSending(eq(notification)); diff --git a/plugins/sonar-email-notifications-plugin/src/main/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplate.java b/plugins/sonar-email-notifications-plugin/src/main/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplate.java index 20d590658b9..bfd105102d9 100644 --- a/plugins/sonar-email-notifications-plugin/src/main/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplate.java +++ b/plugins/sonar-email-notifications-plugin/src/main/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplate.java @@ -64,10 +64,9 @@ public class NewViolationsEmailTemplate extends EmailTemplate { private void appendFooter(StringBuilder sb, Notification notification) { String projectKey = notification.getFieldValue("projectKey"); - String period = notification.getFieldValue("period"); sb.append("\n") .append("See it in Sonar: ").append(configuration.getServerBaseURL()).append("/drilldown/measures/").append(projectKey) - .append("?metric=new_violations&period=").append(period).append('\n'); + .append("?metric=new_violations&period=1\n"); } } diff --git a/plugins/sonar-email-notifications-plugin/src/test/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplateTest.java b/plugins/sonar-email-notifications-plugin/src/test/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplateTest.java index c5c46712a94..4fc6c220945 100644 --- a/plugins/sonar-email-notifications-plugin/src/test/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplateTest.java +++ b/plugins/sonar-email-notifications-plugin/src/test/java/org/sonar/plugins/emailnotifications/newviolations/NewViolationsEmailTemplateTest.java @@ -57,7 +57,7 @@ public class NewViolationsEmailTemplateTest { * Project: Foo * 32 new violations on last analysis (introduced between 2012-01-02 and 2012-01-15) * - * See it in Sonar: http://nemo.sonarsource.org/drilldown/measures/org.sonar.foo:foo?metric=new_violations&period=2 + * See it in Sonar: http://nemo.sonarsource.org/drilldown/measures/org.sonar.foo:foo?metric=new_violations&period=1 * */ @Test @@ -67,7 +67,6 @@ public class NewViolationsEmailTemplateTest { .setFieldValue("projectName", "Foo") .setFieldValue("projectKey", "org.sonar.foo:foo") .setFieldValue("projectId", "45") - .setFieldValue("period", "2") .setFieldValue("fromDate", "2012-01-02") .setFieldValue("toDate", "2012-01-15"); @@ -78,7 +77,7 @@ public class NewViolationsEmailTemplateTest { "Project: Foo\n" + "32 new violations on last analysis (introduced between 2012-01-02 and 2012-01-15)\n" + "\n" + - "See it in Sonar: http://nemo.sonarsource.org/drilldown/measures/org.sonar.foo:foo?metric=new_violations&period=2\n")); + "See it in Sonar: http://nemo.sonarsource.org/drilldown/measures/org.sonar.foo:foo?metric=new_violations&period=1\n")); } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/components/TimeMachineConfiguration.java b/sonar-batch/src/main/java/org/sonar/batch/components/TimeMachineConfiguration.java index 59a07f5eaa8..b5e567ce18d 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/components/TimeMachineConfiguration.java +++ b/sonar-batch/src/main/java/org/sonar/batch/components/TimeMachineConfiguration.java @@ -112,26 +112,4 @@ public class TimeMachineConfiguration implements BatchExtension { public boolean isFileVariationEnabled() { return configuration.getBoolean("sonar.enableFileVariation", Boolean.FALSE); } - - /** - * Returns the index corresponding to the 'previous_analysis' period (which is '1' by default). - * - * @return the index of 'previous_analysis' period, or NULL is users have modified the periods and haven't set a 'previous_analysis' one. - */ - public Integer getLastAnalysisPeriodIndex() { - // period1 is the default for 'previous_analysis' - String period1 = configuration.getString(CoreProperties.TIMEMACHINE_PERIOD_PREFIX + "1"); - if (StringUtils.isBlank(period1) || CoreProperties.TIMEMACHINE_MODE_PREVIOUS_ANALYSIS.equals(period1)) { - return 1; - } - // else search for the other periods - for (int index = 2; index < 6; index++) { - if (CoreProperties.TIMEMACHINE_MODE_PREVIOUS_ANALYSIS.equals(configuration - .getString(CoreProperties.TIMEMACHINE_PERIOD_PREFIX + index))) { - return index; - } - } - // if we're here, this means that we have not found the 'previous_analysis' mode - return null; - } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/components/TimeMachineConfigurationTest.java b/sonar-batch/src/test/java/org/sonar/batch/components/TimeMachineConfigurationTest.java index 7d4be9f7802..68468e09e7e 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/components/TimeMachineConfigurationTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/components/TimeMachineConfigurationTest.java @@ -21,7 +21,6 @@ package org.sonar.batch.components; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.nullValue; import static org.mockito.Matchers.argThat; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -85,23 +84,4 @@ public class TimeMachineConfigurationTest extends AbstractDbUnitTestCase { verifyZeroInteractions(pastSnapshotFinder); } - @Test - public void shouldReturnLastAnalysisIndexIfSet() { - PropertiesConfiguration conf = new PropertiesConfiguration(); - TimeMachineConfiguration timeMachineConfiguration = new TimeMachineConfiguration(getSession(), new Project("my:project"), conf, - mock(PastSnapshotFinder.class)); - - // Nothing set, so period for 'previous_analysis' is 1 by default - assertThat(timeMachineConfiguration.getLastAnalysisPeriodIndex(), is(1)); - - // period1 has been replaced and 'previous_analysis' not set elsewhere... - conf.setProperty(CoreProperties.TIMEMACHINE_PERIOD_PREFIX + 1, "Version 1"); - conf.setProperty(CoreProperties.TIMEMACHINE_PERIOD_PREFIX + 2, "Version 2"); - assertThat(timeMachineConfiguration.getLastAnalysisPeriodIndex(), is(nullValue())); - - // 'previous_analysis' has now been set on period 4 - conf.setProperty(CoreProperties.TIMEMACHINE_PERIOD_PREFIX + 4, CoreProperties.TIMEMACHINE_MODE_PREVIOUS_ANALYSIS); - assertThat(timeMachineConfiguration.getLastAnalysisPeriodIndex(), is(4)); - } - } -- 2.39.5