From 51f6214d7f9249c1a95e35467d55b797610626e8 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 2 Apr 2014 14:11:20 +0200 Subject: [PATCH] SONAR-5056 When restoring a model, deletion of characteristics should update all related rules --- .../sonar/server/debt/DebtModelBackup.java | 2 +- .../server/debt/DebtModelOperations.java | 29 ++++++++++++------- .../server/debt/DebtModelBackupTest.java | 8 ++--- .../server/debt/DebtModelOperationsTest.java | 17 +++++++++++ 4 files changed, 41 insertions(+), 15 deletions(-) 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 5dd59cbffa6..b52a8062a47 100644 --- 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 46066deb3c8..8d703c0adcb 100644 --- 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 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 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 3e7eb82333c..cab7dab8aa1 100644 --- 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("", "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 d57638f2a00..41a8077417a 100644 --- 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)); + } + } -- 2.39.5