SONAR-6236 Apply feedback from PR

7 files changed, 88 insertions, 90 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/dashboard/db/WidgetPropertyDao.java b/server/sonar-server/src/main/java/org/sonar/server/dashboard/db/WidgetPropertyDao.java
index 763d1843469..46ec46113b8 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/dashboard/db/WidgetPropertyDao.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/dashboard/db/WidgetPropertyDao.java
@@ -19,12 +19,15 @@
  */
 package org.sonar.server.dashboard.db;
 
+import com.google.common.base.Function;
 import org.sonar.core.dashboard.WidgetPropertyDto;
 import org.sonar.core.dashboard.WidgetPropertyMapper;
 import org.sonar.core.persistence.DaoComponent;
+import org.sonar.core.persistence.DaoUtils;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.MyBatis;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 
@@ -73,8 +76,14 @@ public class WidgetPropertyDao implements DaoComponent {
     return mapper(session).selectByDashboard(dashboardKey);
   }
 
-  public void deleteByWidgetIds(DbSession session, List<Long> widgetIdsWithPropertiesToDelete) {
-    mapper(session).deleteByWidgetIds(widgetIdsWithPropertiesToDelete);
+  public void deleteByWidgetIds(final DbSession session, List<Long> widgetIdsWithPropertiesToDelete) {
+    DaoUtils.executeLargeInputs(widgetIdsWithPropertiesToDelete, new Function<List<Long>, List<Void>>() {
+      @Override
+      public List<Void> apply(List<Long> input) {
+        mapper(session).deleteByWidgetIds(input);
+        return Arrays.asList();
+      }
+    });
   }
 
   private WidgetPropertyMapper mapper(DbSession session) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java
index 1d9da7f47f4..50644f9683d 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/platform/ServerComponents.java
@@ -563,7 +563,6 @@ class ServerComponents {
     pico.addSingleton(IssueFilterSerializer.class);
     pico.addSingleton(IssueFilterWs.class);
     pico.addSingleton(IssueFilterWriter.class);
-    pico.addSingleton(RegisterIssueFilters.class);
     pico.addSingleton(org.sonar.server.issue.filter.AppAction.class);
     pico.addSingleton(org.sonar.server.issue.filter.ShowAction.class);
     pico.addSingleton(org.sonar.server.issue.filter.FavoritesAction.class);
@@ -709,6 +708,7 @@ class ServerComponents {
     startupContainer.addSingleton(RegisterServletFilters.class);
     startupContainer.addSingleton(CopyRequirementsFromCharacteristicsToRules.class);
     startupContainer.addSingleton(ReportQueueCleaner.class);
+    startupContainer.addSingleton(RegisterIssueFilters.class);
     startupContainer.addSingleton(RenameIssueWidgets.class);
 
     DoPrivileged.execute(new DoPrivileged.Task() {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/startup/RegisterDashboards.java b/server/sonar-server/src/main/java/org/sonar/server/startup/RegisterDashboards.java
index 084820d0a64..f291915fdf0 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/startup/RegisterDashboards.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/startup/RegisterDashboards.java
@@ -56,6 +56,7 @@ public class RegisterDashboards implements Startable {
     this.dashboardDao = dashboardDao;
     this.activeDashboardDao = activeDashboardDao;
     this.loadedTemplateDao = loadedTemplateDao;
+    // RegisterIssueFilters must be run before this task, to be able to reference issue filters in widget properties
   }
 
   /**
diff --git a/server/sonar-server/src/main/java/org/sonar/server/startup/RenameIssueWidgets.java b/server/sonar-server/src/main/java/org/sonar/server/startup/RenameIssueWidgets.java
index 92be6491333..d63c5db20e0 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/startup/RenameIssueWidgets.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/startup/RenameIssueWidgets.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.startup;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import org.picocontainer.Startable;
 import org.sonar.api.utils.System2;
@@ -30,13 +31,22 @@ import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.MyBatis;
 import org.sonar.core.template.LoadedTemplateDto;
 import org.sonar.server.db.DbClient;
+import org.sonar.server.issue.filter.RegisterIssueFilters;
 
-import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 public class RenameIssueWidgets implements Startable {
 
-  private static final String PROJECT_ISSUE_FILTER_WIDGET_KEY = "project_issue_filter";
+  private final static String TASK_KEY = "RenameIssueWidgets";
+
+  private static final String WIDGET_FALSE_POSITIVES = "false_positive_reviews";
+  private static final String WIDGET_MY_UNRESOLVED = "my_reviews";
+  private static final String WIDGET_UNRESOLVED_BY_DEVELOPER = "reviews_per_developer";
+  private static final String WIDGET_UNRESOLVED_BY_STATUS = "unresolved_issues_statuses";
+
+  private static final String WIDGET_PROJECT_ISSUE_FILTER = "project_issue_filter";
+
   private static final String FILTER_PROPERTY = "filter";
   private static final String DISTRIBUTION_AXIS_PROPERTY = "distributionAxis";
 
@@ -44,9 +54,10 @@ public class RenameIssueWidgets implements Startable {
 
   private final System2 system;
 
-  public RenameIssueWidgets(DbClient dbClient, System2 system) {
+  public RenameIssueWidgets(DbClient dbClient, System2 system, RegisterIssueFilters startupDependency) {
     this.dbClient = dbClient;
     this.system = system;
+    // RegisterIssueFilters must be run before this task, to be able to reference issue filters in widget properties
   }
 
   @Override
@@ -54,87 +65,41 @@ public class RenameIssueWidgets implements Startable {
     DbSession session = dbClient.openSession(false);
 
     try {
-      if (dbClient.loadedTemplateDao().countByTypeAndKey(LoadedTemplateDto.ONE_SHOT_TASK_TYPE, getClass().getSimpleName()) != 0) {
+      if (dbClient.loadedTemplateDao().countByTypeAndKey(LoadedTemplateDto.ONE_SHOT_TASK_TYPE, TASK_KEY) != 0) {
         // Already done
         return;
       }
 
-      IssueFilterDto unresolvedIssues = dbClient.issueFilterDao().selectProvidedFilterByName("Unresolved Issues");
-      IssueFilterDto hiddenDebt = dbClient.issueFilterDao().selectProvidedFilterByName("False Positive and Won't Fix Issues");
-      IssueFilterDto myUnresolvedIssues = dbClient.issueFilterDao().selectProvidedFilterByName("My Unresolved Issues");
+      Map<String, IssueFilterDto> filterByWidgetKey = loadRequiredIssueFilters();
 
-      if (unresolvedIssues == null || hiddenDebt == null || myUnresolvedIssues == null) {
-        // One of the filter has been deleted, no need to do anything
-        return;
-      }
+      Map<String, String> distributionAxisByWidgetKey = ImmutableMap.of(
+        WIDGET_FALSE_POSITIVES, "resolutions",
+        WIDGET_MY_UNRESOLVED, "severities",
+        WIDGET_UNRESOLVED_BY_DEVELOPER, "assignees",
+        WIDGET_UNRESOLVED_BY_STATUS, "statuses"
+      );
 
       Loggers.get(getClass()).info("Replacing issue related widgets with issue filter widgets");
 
-      List<Long> widgetIdsWithPropertiesToDelete = Lists.newArrayList();
-      List<WidgetPropertyDto> widgetPropertiesToCreate = Lists.newArrayList();
-      Date now = system.newDate();
+      List<Long> updatedWidgetIds = Lists.newArrayList();
+      List<WidgetPropertyDto> newWidgetProperties = Lists.newArrayList();
 
       for (WidgetDto widget : dbClient.widgetDao().findAll(session)) {
-        switch (widget.getWidgetKey()) {
-          case "false_positive_reviews":
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(FILTER_PROPERTY)
-                .setTextValue(hiddenDebt.getId().toString()));
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(DISTRIBUTION_AXIS_PROPERTY)
-                .setTextValue("resolutions"));
-            updateWidget(session, widgetIdsWithPropertiesToDelete, widget);
-            break;
-          case "my_reviews":
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(FILTER_PROPERTY)
-                .setTextValue(myUnresolvedIssues.getId().toString()));
-            updateWidget(session, widgetIdsWithPropertiesToDelete, widget);
-            break;
-          case "reviews_per_developer":
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(FILTER_PROPERTY)
-                .setTextValue(unresolvedIssues.getId().toString()));
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(DISTRIBUTION_AXIS_PROPERTY)
-                .setTextValue("assignees"));
-            updateWidget(session, widgetIdsWithPropertiesToDelete, widget);
-            break;
-          case "unresolved_issues_statuses":
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(FILTER_PROPERTY)
-                .setTextValue(unresolvedIssues.getId().toString()));
-            widgetPropertiesToCreate.add(
-              new WidgetPropertyDto()
-                .setWidgetId(widget.getId())
-                .setPropertyKey(DISTRIBUTION_AXIS_PROPERTY)
-                .setTextValue("statuses"));
-            updateWidget(session, widgetIdsWithPropertiesToDelete, widget);
-            break;
-          default:
-            // Nothing to do, move along
-            break;
+        String widgetKey = widget.getWidgetKey();
+        if (filterByWidgetKey.keySet().contains(widgetKey)) {
+          newWidgetProperties.add(createFilterProperty(filterByWidgetKey.get(widgetKey), widget));
+          newWidgetProperties.add(createDistributionAxisProperty(distributionAxisByWidgetKey.get(widgetKey), widget));
+          updateWidget(session, widget);
+          updatedWidgetIds.add(widget.getId());
         }
       }
 
-      dbClient.widgetPropertyDao().deleteByWidgetIds(session, widgetIdsWithPropertiesToDelete);
-      dbClient.widgetPropertyDao().insert(session, widgetPropertiesToCreate);
+      dbClient.widgetPropertyDao().deleteByWidgetIds(session, updatedWidgetIds);
+      dbClient.widgetPropertyDao().insert(session, newWidgetProperties);
 
       dbClient.loadedTemplateDao().insert(new LoadedTemplateDto()
         .setType(LoadedTemplateDto.ONE_SHOT_TASK_TYPE)
-        .setKey(getClass().getSimpleName()), session);
+        .setKey(TASK_KEY), session);
 
       session.commit();
     } finally {
@@ -142,12 +107,41 @@ public class RenameIssueWidgets implements Startable {
     }
   }
 
-  private void updateWidget(DbSession session, List<Long> widgetIdsWithPropertiesToDelete, WidgetDto widget) {
+  protected Map<String, IssueFilterDto> loadRequiredIssueFilters() {
+    IssueFilterDto unresolvedIssues = dbClient.issueFilterDao().selectProvidedFilterByName("Unresolved Issues");
+    IssueFilterDto hiddenDebt = dbClient.issueFilterDao().selectProvidedFilterByName("False Positive and Won't Fix Issues");
+    IssueFilterDto myUnresolvedIssues = dbClient.issueFilterDao().selectProvidedFilterByName("My Unresolved Issues");
+
+    Map<String, IssueFilterDto> filterByWidgetKey = ImmutableMap.of(
+      WIDGET_FALSE_POSITIVES, hiddenDebt,
+      WIDGET_MY_UNRESOLVED, myUnresolvedIssues,
+      WIDGET_UNRESOLVED_BY_DEVELOPER, unresolvedIssues,
+      WIDGET_UNRESOLVED_BY_STATUS, unresolvedIssues
+    );
+
+    return filterByWidgetKey;
+  }
+
+  private WidgetPropertyDto createFilterProperty(IssueFilterDto issueFilter, WidgetDto widget) {
+    return createWidgetProperty(FILTER_PROPERTY, issueFilter.getId().toString(), widget);
+  }
+
+  private WidgetPropertyDto createDistributionAxisProperty(String distributionAxis, WidgetDto widget) {
+    return createWidgetProperty(DISTRIBUTION_AXIS_PROPERTY, distributionAxis, widget);
+  }
+
+  private WidgetPropertyDto createWidgetProperty(String key, String value, WidgetDto widget) {
+    return new WidgetPropertyDto()
+      .setWidgetId(widget.getId())
+      .setPropertyKey(key)
+      .setTextValue(value);
+  }
+
+  private void updateWidget(DbSession session, WidgetDto widget) {
     dbClient.widgetDao().update(session,
-      widget.setWidgetKey(PROJECT_ISSUE_FILTER_WIDGET_KEY)
+      widget.setWidgetKey(WIDGET_PROJECT_ISSUE_FILTER)
         .setUpdatedAt(system.newDate())
         .setConfigured(true));
-    widgetIdsWithPropertiesToDelete.add(widget.getId());
   }
 
   @Override
diff --git a/server/sonar-server/src/test/java/org/sonar/server/startup/RenameIssueWidgetsTest.java b/server/sonar-server/src/test/java/org/sonar/server/startup/RenameIssueWidgetsTest.java
index 92d8e2c5ca8..5e79b405e34 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/startup/RenameIssueWidgetsTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/startup/RenameIssueWidgetsTest.java
@@ -53,17 +53,12 @@ public class RenameIssueWidgetsTest {
   }
 
   @Test
-  public void should_skip_when_filter_removed() throws Exception {
-    dbTester.prepareDbUnit(this.getClass(), "empty.xml");
-
-    doStart();
-  }
-
-  @Test
   public void should_skip_when_already_executed() throws Exception {
     dbTester.prepareDbUnit(this.getClass(), "after.xml");
 
     doStart();
+
+    dbTester.assertDbUnit(this.getClass(), "after.xml", "widgets", "widget_properties", "loaded_templates");
   }
 
   private void doStart() {
@@ -80,7 +75,8 @@ public class RenameIssueWidgetsTest {
         new IssueFilterDao(dbTester.myBatis()),
         new LoadedTemplateDao(dbTester.myBatis())
       ),
-      system2);
+      system2,
+      null);
 
     task.start();
     task.stop();
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/after.xml b/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/after.xml
index ab8310df45a..2cab15849f1 100644
--- a/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/after.xml
+++ b/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/after.xml
@@ -53,7 +53,8 @@
 
   <!-- filter = 'My Unresolved Issues' -->
   <widget_properties id="5" widget_id="3" kee="filter" text_value="3"/>
-  <!-- distributionAxis is 'default', no other config -->
+  <!-- distributionAxis is 'severities' -->
+  <widget_properties id="6" widget_id="3" kee="distributionAxis" text_value="severities"/>
 
 
   <!-- 'Unresolved Issues Per Developer' - replaced by project_issue_filter w/ 'Unresolved Issues' filter -->
@@ -61,9 +62,9 @@
      column_index="1" row_index="4" configured="true" created_at="[null]" updated_at="2003-03-23 01:23:45" resource_id="[null]"/>
 
   <!-- filter = 'Unresolved Issues' -->
-  <widget_properties id="6" widget_id="4" kee="filter" text_value="1"/>
+  <widget_properties id="7" widget_id="4" kee="filter" text_value="1"/>
   <!-- distributionAxis is 'assignees' -->
-  <widget_properties id="7" widget_id="4" kee="distributionAxis" text_value="assignees"/>
+  <widget_properties id="8" widget_id="4" kee="distributionAxis" text_value="assignees"/>
 
 
   <!-- 'Unresolved Issues Per Status' - replaced by project_issue_filter w/ 'Unresolved Issues' filter -->
@@ -71,9 +72,9 @@
      column_index="1" row_index="5" configured="true" created_at="[null]" updated_at="2003-03-23 01:23:45" resource_id="[null]"/>
 
   <!-- filter = 'Unresolved Issues' -->
-  <widget_properties id="8" widget_id="5" kee="filter" text_value="1"/>
+  <widget_properties id="9" widget_id="5" kee="filter" text_value="1"/>
   <!-- distributionAxis is 'assignees' -->
-  <widget_properties id="9" widget_id="5" kee="distributionAxis" text_value="statuses"/>
+  <widget_properties id="10" widget_id="5" kee="distributionAxis" text_value="statuses"/>
 
 
   <loaded_templates id="1" template_type="ONE_SHOT_TASK" kee="RenameIssueWidgets"/>
diff --git a/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/empty.xml b/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/empty.xml
deleted file mode 100644
index 871dedcb5e9..00000000000
--- a/server/sonar-server/src/test/resources/org/sonar/server/startup/RenameIssueWidgetsTest/empty.xml
+++ /dev/null
@@ -1,3 +0,0 @@
-<dataset>
-
-</dataset>