SONAR-5056 When restoring a model, deletion of characteristics should update all...

diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java
index 5dd59cbffa6f87749eebadaedd4dc604eedb97cc..b52a8062a47c6323beb4b9857c2154efe708e41b 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelBackup.java
@@ -251,7 +251,7 @@ public class DebtModelBackup implements ServerComponent {
       // Disable no more existing characteristics
       for (CharacteristicDto sourceCharacteristic : sourceCharacteristics) {
         if (targetModel.characteristicByKey(sourceCharacteristic.getKey()) == null) {
-          debtModelOperations.disableCharacteristic(sourceCharacteristic, updateDate, session);
+          debtModelOperations.delete(sourceCharacteristic, updateDate, session);
         }
       }
     }

diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
index 46066deb3c8c5269a59436cdc98173a512ee72df..8d703c0adcb1710e6d276a7407993121d763fd10 100644 (file)
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
@@ -192,8 +192,20 @@ public class DebtModelOperations implements ServerComponent {
     Date updateDate = new Date(system2.now());
     SqlSession session = mybatis.openBatchSession();
     try {
+      delete(findCharacteristic(characteristicId, session), updateDate, session);
+      session.commit();
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
 
-      CharacteristicDto characteristicOrSubCharacteristic = findCharacteristic(characteristicId, session);
+  /**
+   * Disabled a characteristic or a sub characteristic.
+   * If it has already been disabled, do nothing (for instance when call on a list of characteristics and sub-characteristics in random order)
+   */
+  public void delete(CharacteristicDto characteristicOrSubCharacteristic, Date updateDate, SqlSession session) {
+    // Do nothing is the characteristic is already disabled
+    if (characteristicOrSubCharacteristic.isEnabled()) {
       // When root characteristic, browse sub characteristics and disable rule debt on each sub characteristic then disable it
       if (characteristicOrSubCharacteristic.getParentId() == null) {
         List<CharacteristicDto> subCharacteristics = dao.selectCharacteristicsByParentId(characteristicOrSubCharacteristic.getId(), session);
@@ -205,23 +217,20 @@ public class DebtModelOperations implements ServerComponent {
         // When sub characteristic, disable rule debt on the sub characteristic then disable it
         disableSubCharacteristic(characteristicOrSubCharacteristic, updateDate, session);
       }
-      session.commit();
-    } finally {
-      MyBatis.closeQuietly(session);
     }
   }
 
-  public void disableCharacteristic(CharacteristicDto characteristic, Date updateDate, SqlSession session) {
-    characteristic.setEnabled(false);
-    characteristic.setUpdatedAt(updateDate);
-    dao.update(characteristic, session);
-  }
-
   private void disableSubCharacteristic(CharacteristicDto subCharacteristic, Date updateDate, SqlSession session) {
     disableRulesDebt(ruleDao.selectBySubCharacteristicId(subCharacteristic.getId(), session), subCharacteristic.getId(), updateDate, session);
     disableCharacteristic(subCharacteristic, updateDate, session);
   }
 
+  private void disableCharacteristic(CharacteristicDto characteristic, Date updateDate, SqlSession session) {
+    characteristic.setEnabled(false);
+    characteristic.setUpdatedAt(updateDate);
+    dao.update(characteristic, session);
+  }
+
   public void disableRulesDebt(List<RuleDto> ruleDtos, Integer subCharacteristicId, Date updateDate, SqlSession session) {
     for (RuleDto ruleDto : ruleDtos) {
       if (subCharacteristicId.equals(ruleDto.getSubCharacteristicId())) {

diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java
index 3e7eb82333cf4a1f0f946fb7faaa3c2075e2387a..cab7dab8aa16fdd9946945dc41ce3f0ea15e134c 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelBackupTest.java
@@ -314,8 +314,8 @@ public class DebtModelBackupTest {
 
     debtModelBackup.restoreCharacteristics(new DebtModel(), true, now, session);
 
-    verify(debtModelOperations).disableCharacteristic(dto1, now, session);
-    verify(debtModelOperations).disableCharacteristic(dto2, now, session);
+    verify(debtModelOperations).delete(dto1, now, session);
+    verify(debtModelOperations).delete(dto2, now, session);
   }
 
   @Test
@@ -401,7 +401,7 @@ public class DebtModelBackupTest {
 
     debtModelBackup.restore("java");
 
-    verify(debtModelOperations, never()).disableCharacteristic(any(CharacteristicDto.class), eq(now), eq(session));
+    verify(debtModelOperations, never()).delete(any(CharacteristicDto.class), eq(now), eq(session));
     verify(session).commit();
   }
 
@@ -627,7 +627,7 @@ public class DebtModelBackupTest {
 
     debtModelBackup.restoreFromXml("<xml/>", "java");
 
-    verify(debtModelOperations, never()).disableCharacteristic(any(CharacteristicDto.class), eq(now), eq(session));
+    verify(debtModelOperations, never()).delete(any(CharacteristicDto.class), eq(now), eq(session));
     verify(session).commit();
   }
 

diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
index d57638f2a00cc099b514a9c68ea3698de05cd0d3..41a8077417ad3ec4857a2046955858e629853a5f 100644 (file)
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
@@ -460,4 +460,21 @@ public class DebtModelOperationsTest {
     verify(ruleRegistry).reindex(ruleCaptor.getAllValues(), batchSession);
   }
 
+  @Test
+  public void not_delete_already_disabled_characteristic() {
+    BatchSession batchSession = mock(BatchSession.class);
+    when(mybatis.openBatchSession()).thenReturn(batchSession);
+
+    when(dao.selectById(1, batchSession)).thenReturn(new CharacteristicDto()
+      .setId(1)
+      .setKey("MEMORY_EFFICIENCY")
+      .setName("Memory use")
+      .setOrder(2)
+      .setEnabled(false));
+
+    service.delete(1);
+
+    verify(dao, never()).update(any(CharacteristicDto.class), eq(batchSession));
+  }
+
 }