SONAR-5056 Add delete characteristic action

3 files changed, 165 insertions, 16 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
index 367f3bd1052..a17b23fdb17 100644
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
@@ -20,14 +20,18 @@
 
 package org.sonar.server.debt;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
 import org.apache.ibatis.session.SqlSession;
 import org.sonar.api.server.debt.DebtCharacteristic;
 import org.sonar.api.server.debt.DebtModel;
 import org.sonar.api.server.debt.internal.DefaultDebtCharacteristic;
+import org.sonar.api.utils.System2;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.MyBatis;
+import org.sonar.core.rule.RuleDao;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.exceptions.BadRequestException;
@@ -39,6 +43,7 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 
 import java.util.Collection;
+import java.util.Date;
 import java.util.List;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -51,10 +56,19 @@ public class DebtModelService implements DebtModel {
 
   private final MyBatis mybatis;
   private final CharacteristicDao dao;
+  private final RuleDao ruleDao;
+  private final System2 system2;
 
-  public DebtModelService(MyBatis mybatis, CharacteristicDao dao) {
+  public DebtModelService(MyBatis mybatis, CharacteristicDao dao, RuleDao ruleDao) {
+    this(mybatis, dao, ruleDao, System2.INSTANCE);
+  }
+
+  @VisibleForTesting
+  DebtModelService(MyBatis mybatis, CharacteristicDao dao, RuleDao ruleDao, System2 system2) {
     this.mybatis = mybatis;
     this.dao = dao;
+    this.ruleDao = ruleDao;
+    this.system2 = system2;
   }
 
   public List<DebtCharacteristic> rootCharacteristics() {
@@ -156,6 +170,48 @@ public class DebtModelService implements DebtModel {
     }
   }
 
+  /**
+   * Disable characteristic and sub characteristic or only sub characteristic.
+   * Will also update every rules linked to sub characteristics by setting characteristic id to -1 and remove function, factor and offset.
+   */
+  public void delete(int characteristicOrSubCharactteristicId) {
+    checkPermission();
+
+    SqlSession session = mybatis.openBatchSession();
+    try {
+      Date now = new Date(system2.now());
+
+      CharacteristicDto characteristicOrSubCharacteristicDto = findCharacteristic(characteristicOrSubCharactteristicId, session);
+      List<RuleDto> ruleDtos = ruleDao.selectByCharacteristicOrSubCharacteristicId(characteristicOrSubCharacteristicDto.getId(), session);
+      for (RuleDto ruleDto : ruleDtos) {
+        ruleDto.setCharacteristicId(RuleDto.DISABLED_CHARACTERISTIC_ID);
+        ruleDto.setRemediationFunction(null);
+        ruleDto.setRemediationFactor(null);
+        ruleDto.setRemediationOffset(null);
+        ruleDto.setUpdatedAt(now);
+        ruleDao.update(ruleDto, session);
+
+        // TODO update rules from E/S
+      }
+
+      if (characteristicOrSubCharacteristicDto.getParentId() == null) {
+        List<CharacteristicDto> dtos = dao.selectCharacteristicsByParentId(characteristicOrSubCharacteristicDto.getId(), session);
+        for (CharacteristicDto subCharacteristicDto : dtos) {
+          subCharacteristicDto.setEnabled(false);
+          subCharacteristicDto.setUpdatedAt(now);
+          dao.update(subCharacteristicDto, session);
+        }
+      }
+      characteristicOrSubCharacteristicDto.setEnabled(false);
+      characteristicOrSubCharacteristicDto.setUpdatedAt(now);
+      dao.update(characteristicOrSubCharacteristicDto, session);
+
+      session.commit();
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
+
   private CharacteristicDto findCharacteristic(Integer id, SqlSession session) {
     CharacteristicDto dto = dao.selectById(id, session);
     if (dto == null) {
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java b/sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java
index 54232217ae2..51fcac7192f 100644
--- a/sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/CopyRequirementsFromCharacteristicsToRules.java
@@ -34,7 +34,7 @@ import org.sonar.api.rules.Rule;
 import org.sonar.api.utils.Duration;
 import org.sonar.api.utils.System2;
 import org.sonar.core.persistence.Database;
-import org.sonar.core.technicaldebt.db.CharacteristicDto;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.core.technicaldebt.db.RequirementDao;
 import org.sonar.core.technicaldebt.db.RequirementDto;
 import org.sonar.server.db.migrations.MassUpdater;
@@ -174,7 +174,7 @@ public class CopyRequirementsFromCharacteristicsToRules {
 
       if (enabledRequirement == null && !Rule.STATUS_REMOVED.equals(ruleRow.getStatus())) {
         // If no requirements are enable, it means that the requirement has been disabled for this rule
-        updateStatement.setInt(1, CharacteristicDto.DISABLED_CHARACTERISTIC_ID);
+        updateStatement.setInt(1, RuleDto.DISABLED_CHARACTERISTIC_ID);
         updateStatement.setNull(2, Types.VARCHAR);
         updateStatement.setNull(3, Types.VARCHAR);
         updateStatement.setNull(4, Types.VARCHAR);
diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
index 61a58305daf..708a9a4165a 100644
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
@@ -29,8 +29,13 @@ import org.mockito.invocation.InvocationOnMock;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.mockito.stubbing.Answer;
 import org.sonar.api.server.debt.DebtCharacteristic;
+import org.sonar.api.utils.DateUtils;
+import org.sonar.api.utils.System2;
 import org.sonar.core.permission.GlobalPermissions;
+import org.sonar.core.persistence.BatchSession;
 import org.sonar.core.persistence.MyBatis;
+import org.sonar.core.rule.RuleDao;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.core.technicaldebt.db.CharacteristicDao;
 import org.sonar.core.technicaldebt.db.CharacteristicDto;
 import org.sonar.server.exceptions.BadRequestException;
@@ -38,6 +43,8 @@ import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.MockUserSession;
 
+import java.util.Date;
+
 import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
@@ -51,22 +58,32 @@ public class DebtModelServiceTest {
   CharacteristicDao dao;
 
   @Mock
+  RuleDao ruleDao;
+
+  @Mock
   MyBatis mybatis;
 
   @Mock
   SqlSession session;
 
-  CharacteristicDto rootCharacteristicDto = new CharacteristicDto()
+  @Mock
+  System2 system2;
+
+  Date now = DateUtils.parseDate("2014-03-19");
+
+  CharacteristicDto characteristicDto = new CharacteristicDto()
     .setId(1)
     .setKey("MEMORY_EFFICIENCY")
     .setName("Memory use")
-    .setOrder(2);
+    .setOrder(2)
+    .setEnabled(true);
 
-  CharacteristicDto characteristicDto = new CharacteristicDto()
+  CharacteristicDto subCharacteristicDto = new CharacteristicDto()
     .setId(2)
     .setKey("EFFICIENCY")
     .setName("Efficiency")
-    .setParentId(1);
+    .setParentId(1)
+    .setEnabled(true);
 
   int currentId;
 
@@ -74,6 +91,8 @@ public class DebtModelServiceTest {
 
   @Before
   public void setUp() throws Exception {
+    when(system2.now()).thenReturn(now.getTime());
+
     MockUserSession.set().setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
 
     currentId = 10;
@@ -88,24 +107,24 @@ public class DebtModelServiceTest {
     }).when(dao).insert(any(CharacteristicDto.class), any(SqlSession.class));
 
     when(mybatis.openSession()).thenReturn(session);
-    service = new DebtModelService(mybatis, dao);
+    service = new DebtModelService(mybatis, dao, ruleDao,system2);
   }
 
   @Test
   public void find_root_characteristics() {
-    when(dao.selectEnabledRootCharacteristics()).thenReturn(newArrayList(rootCharacteristicDto));
+    when(dao.selectEnabledRootCharacteristics()).thenReturn(newArrayList(characteristicDto));
     assertThat(service.rootCharacteristics()).hasSize(1);
   }
 
   @Test
   public void find_characteristics() {
-    when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(rootCharacteristicDto));
+    when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(characteristicDto));
     assertThat(service.characteristics()).hasSize(1);
   }
 
   @Test
   public void find_characteristic_by_id() {
-    when(dao.selectById(1)).thenReturn(rootCharacteristicDto);
+    when(dao.selectById(1)).thenReturn(characteristicDto);
 
     DebtCharacteristic characteristic = service.characteristicById(1);
     assertThat(characteristic.id()).isEqualTo(1);
@@ -119,7 +138,7 @@ public class DebtModelServiceTest {
 
   @Test
   public void create_sub_characteristic() {
-    when(dao.selectById(1, session)).thenReturn(rootCharacteristicDto);
+    when(dao.selectById(1, session)).thenReturn(characteristicDto);
 
     DebtCharacteristic result = service.create("Compilation name", 1);
 
@@ -131,7 +150,7 @@ public class DebtModelServiceTest {
 
   @Test
   public void fail_to_create_sub_characteristic_when_parent_id_is_not_a_root_characteristic() {
-    when(dao.selectById(1, session)).thenReturn(characteristicDto);
+    when(dao.selectById(1, session)).thenReturn(subCharacteristicDto);
 
     try {
       service.create("Compilation", 1);
@@ -156,7 +175,7 @@ public class DebtModelServiceTest {
   @Test
   public void fail_to_create_sub_characteristic_when_name_already_used() {
     when(dao.selectByName("Compilation", session)).thenReturn(new CharacteristicDto());
-    when(dao.selectById(1, session)).thenReturn(rootCharacteristicDto);
+    when(dao.selectById(1, session)).thenReturn(characteristicDto);
 
     try {
       service.create("Compilation", 1);
@@ -205,7 +224,7 @@ public class DebtModelServiceTest {
 
   @Test
   public void rename_characteristic() {
-    when(dao.selectById(10, session)).thenReturn(characteristicDto);
+    when(dao.selectById(10, session)).thenReturn(subCharacteristicDto);
 
     DebtCharacteristic result = service.rename(10, "New Efficiency");
 
@@ -215,7 +234,7 @@ public class DebtModelServiceTest {
 
   @Test
   public void not_rename_characteristic_when_renaming_with_same_name() {
-    when(dao.selectById(10, session)).thenReturn(characteristicDto);
+    when(dao.selectById(10, session)).thenReturn(subCharacteristicDto);
 
     service.rename(10, "Efficiency");
 
@@ -286,4 +305,78 @@ public class DebtModelServiceTest {
     verify(dao, never()).update(any(CharacteristicDto.class), eq(session));
   }
 
+  @Test
+  public void delete_sub_characteristic() {
+    BatchSession batchSession = mock(BatchSession.class);
+    when(mybatis.openBatchSession()).thenReturn(batchSession);
+
+    when(ruleDao.selectByCharacteristicOrSubCharacteristicId(2, batchSession)).thenReturn(newArrayList(
+      new RuleDto()
+        .setCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationFactor("2h").setRemediationOffset("5min")
+        .setDefaultCharacteristicId(10).setDefaultRemediationFunction("LINEAR_OFFSET").setDefaultRemediationFactor("4h").setDefaultRemediationOffset("15min")
+    ));
+    when(dao.selectById(2, batchSession)).thenReturn(subCharacteristicDto);
+
+    service.delete(2);
+
+    ArgumentCaptor<RuleDto> ruleArgument = ArgumentCaptor.forClass(RuleDto.class);
+    verify(ruleDao).update(ruleArgument.capture(), eq(batchSession));
+    RuleDto ruleDto = ruleArgument.getValue();
+
+    // Overridden debt data are disabled
+    assertThat(ruleDto.getCharacteristicId()).isEqualTo(-1);
+    assertThat(ruleDto.getRemediationFunction()).isNull();
+    assertThat(ruleDto.getRemediationFactor()).isNull();
+    assertThat(ruleDto.getRemediationOffset()).isNull();
+    assertThat(ruleDto.getUpdatedAt()).isEqualTo(now);
+
+    // Default debt data should not be touched
+    assertThat(ruleDto.getDefaultCharacteristicId()).isEqualTo(10);
+    assertThat(ruleDto.getDefaultRemediationFunction()).isEqualTo("LINEAR_OFFSET");
+    assertThat(ruleDto.getDefaultRemediationFactor()).isEqualTo("4h");
+    assertThat(ruleDto.getDefaultRemediationOffset()).isEqualTo("15min");
+
+    ArgumentCaptor<CharacteristicDto> characteristicArgument = ArgumentCaptor.forClass(CharacteristicDto.class);
+    verify(dao).update(characteristicArgument.capture(), eq(batchSession));
+    CharacteristicDto characteristicDto = characteristicArgument.getValue();
+
+    // Sub characteristic is disable
+    assertThat(characteristicDto.getId()).isEqualTo(2);
+    assertThat(characteristicDto.isEnabled()).isFalse();
+    assertThat(characteristicDto.getUpdatedAt()).isEqualTo(now);
+  }
+
+  @Test
+  public void delete_characteristic() {
+    BatchSession batchSession = mock(BatchSession.class);
+    when(mybatis.openBatchSession()).thenReturn(batchSession);
+
+    when(ruleDao.selectByCharacteristicOrSubCharacteristicId(1, batchSession)).thenReturn(newArrayList(
+      new RuleDto().setCharacteristicId(2).setRemediationFunction("LINEAR_OFFSET").setRemediationFactor("2h").setRemediationOffset("5min")
+    ));
+    when(dao.selectCharacteristicsByParentId(1, batchSession)).thenReturn(newArrayList(
+      subCharacteristicDto
+    ));
+    when(dao.selectById(1, batchSession)).thenReturn(characteristicDto);
+
+    service.delete(1);
+
+    verify(ruleDao).update(any(RuleDto.class), eq(batchSession));
+
+    ArgumentCaptor<CharacteristicDto> characteristicArgument = ArgumentCaptor.forClass(CharacteristicDto.class);
+    verify(dao, times(2)).update(characteristicArgument.capture(), eq(batchSession));
+    CharacteristicDto subCharacteristicDto = characteristicArgument.getAllValues().get(0);
+    CharacteristicDto characteristicDto = characteristicArgument.getAllValues().get(1);
+
+    // Sub characteristic is disable
+    assertThat(subCharacteristicDto.getId()).isEqualTo(2);
+    assertThat(subCharacteristicDto.isEnabled()).isFalse();
+    assertThat(subCharacteristicDto.getUpdatedAt()).isEqualTo(now);
+
+    // Characteristic is disable
+    assertThat(characteristicDto.getId()).isEqualTo(1);
+    assertThat(characteristicDto.isEnabled()).isFalse();
+    assertThat(characteristicDto.getUpdatedAt()).isEqualTo(now);
+  }
+
 }