From dd1478fd1c40ceca4382eda011c3f093ba75dfe5 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 20 Mar 2014 17:27:12 +0100 Subject: [PATCH] SONAR-5056 Fix issue on move up/down --- .../org/sonar/api/server/debt/DebtModel.java | 10 ++++-- .../internal/DefaultDebtCharacteristic.java | 7 ++++ .../sonar/server/debt/DebtModelLookup.java | 2 +- .../server/debt/DebtModelOperations.java | 21 +++++++++--- .../sonar/server/debt/DebtModelService.java | 6 ++-- .../server/debt/DebtModelLookupTest.java | 4 +-- .../server/debt/DebtModelOperationsTest.java | 32 ++++++++++++------- .../server/debt/DebtModelServiceTest.java | 8 ++--- 8 files changed, 63 insertions(+), 27 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/DebtModel.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/DebtModel.java index e2bd93cba6c..cfc8f323cba 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/DebtModel.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/DebtModel.java @@ -29,10 +29,16 @@ import java.util.List; */ public interface DebtModel extends ServerComponent { + /** + * @return all characteristics + */ + List allCharacteristics(); + + /** + * @return only characteristics of highest level + */ List characteristics(); - List rootCharacteristics(); - DebtCharacteristic characteristicById(int id); } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtCharacteristic.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtCharacteristic.java index 9c531cdaa24..0ecf1a5bf3e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtCharacteristic.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/debt/internal/DefaultDebtCharacteristic.java @@ -20,6 +20,8 @@ package org.sonar.api.server.debt.internal; +import org.apache.commons.lang.builder.ToStringBuilder; +import org.apache.commons.lang.builder.ToStringStyle; import org.sonar.api.server.debt.DebtCharacteristic; import javax.annotation.CheckForNull; @@ -109,4 +111,9 @@ public class DefaultDebtCharacteristic implements DebtCharacteristic { return this; } + @Override + public String toString() { + return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); + } + } diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java index aa79358d0ad..da97cadd0d3 100644 --- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java +++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java @@ -47,7 +47,7 @@ public class DebtModelLookup implements ServerComponent { return toCharacteristics(dao.selectEnabledRootCharacteristics()); } - public List characteristics() { + public List allCharacteristics() { return toCharacteristics(dao.selectEnabledCharacteristics()); } 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 871f777a894..4b7c6004746 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 @@ -21,6 +21,8 @@ package org.sonar.server.debt; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; import org.apache.ibatis.session.SqlSession; import org.sonar.api.ServerComponent; import org.sonar.api.server.debt.DebtCharacteristic; @@ -126,14 +128,25 @@ public class DebtModelOperations implements ServerComponent { SqlSession session = mybatis.openSession(); try { - CharacteristicDto dto = findCharacteristic(characteristicId, session); + final CharacteristicDto dto = findCharacteristic(characteristicId, session); int currentOrder = dto.getOrder(); - CharacteristicDto dtoToSwitchOrderWith = moveUpOrDown ? dao.selectPrevious(currentOrder, session) : dao.selectNext(currentOrder, session); - // Do nothing when characteristic is already to the new location - if (dtoToSwitchOrderWith == null) { + // characteristics should be order by 'order' + List rootCharacteristics = dao.selectEnabledRootCharacteristics(session); + int currentPosition = Iterables.indexOf(rootCharacteristics, new Predicate() { + @Override + public boolean apply(CharacteristicDto input) { + return input.getKey().equals(dto.getKey()); + } + }); + Integer nextMove = moveUpOrDown ? (currentPosition > 0 ? currentPosition - 1 : null) : (currentPosition < rootCharacteristics.size()-1 ? currentPosition + 1 : null); + + // Do nothing when characteristic is already to the good location + if (nextMove == null) { return toCharacteristic(dto); } + + CharacteristicDto dtoToSwitchOrderWith = Iterables.get(rootCharacteristics, nextMove); int nextOrder = dtoToSwitchOrderWith.getOrder(); dtoToSwitchOrderWith.setOrder(currentOrder); dtoToSwitchOrderWith.setUpdatedAt(new Date(system2.now())); 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 c21e1d16b8a..483f4fa5c28 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 @@ -44,12 +44,12 @@ public class DebtModelService implements DebtModel { this.debtModelRestore = debtModelRestore; } - public List rootCharacteristics() { + public List characteristics() { return debtModelLookup.rootCharacteristics(); } - public List characteristics() { - return debtModelLookup.characteristics(); + public List allCharacteristics() { + return debtModelLookup.allCharacteristics(); } @CheckForNull diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java index 6c72ca00e31..eedc79f38af 100644 --- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java +++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java @@ -60,9 +60,9 @@ public class DebtModelLookupTest { } @Test - public void find_characteristics() { + public void find_all_characteristics() { when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(characteristicDto)); - assertThat(service.characteristics()).hasSize(1); + assertThat(service.allCharacteristics()).hasSize(1); } @Test 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 cc8400499a1..f3ab22fe6b3 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 @@ -108,7 +108,7 @@ public class DebtModelOperationsTest { }).when(dao).insert(any(CharacteristicDto.class), any(SqlSession.class)); when(mybatis.openSession()).thenReturn(session); - service = new DebtModelOperations(mybatis, dao, ruleDao,system2); + service = new DebtModelOperations(mybatis, dao, ruleDao, system2); } @Test @@ -234,8 +234,11 @@ public class DebtModelOperationsTest { @Test public void move_up() { - when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setOrder(2)); - when(dao.selectPrevious(2, session)).thenReturn(new CharacteristicDto().setId(2).setOrder(1)); + when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2)); + when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList( + new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(1), + new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2) + )); DebtCharacteristic result = service.moveUp(10); @@ -251,9 +254,11 @@ public class DebtModelOperationsTest { @Test public void do_nothing_when_move_up_and_already_on_top() { - CharacteristicDto dto = new CharacteristicDto().setId(10).setOrder(1); - when(dao.selectById(10, session)).thenReturn(dto); - when(dao.selectPrevious(1, session)).thenReturn(null); + when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(1)); + when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList( + new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(1), + new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(2) + )); service.moveUp(10); @@ -262,8 +267,11 @@ public class DebtModelOperationsTest { @Test public void move_down() { - when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setOrder(2)); - when(dao.selectNext(2, session)).thenReturn(new CharacteristicDto().setId(2).setOrder(3)); + when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2)); + when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList( + new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2), + new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(3) + )); DebtCharacteristic result = service.moveDown(10); @@ -279,9 +287,11 @@ public class DebtModelOperationsTest { @Test public void do_nothing_when_move_down_and_already_on_bottom() { - CharacteristicDto dto = new CharacteristicDto().setId(10).setOrder(5); - when(dao.selectById(10, session)).thenReturn(dto); - when(dao.selectNext(5, session)).thenReturn(null); + when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2)); + when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList( + new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(1), + new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2) + )); service.moveDown(10); 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 e02bb0133a3..9c044e6508a 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 @@ -48,14 +48,14 @@ public class DebtModelServiceTest { @Test public void find_root_characteristics() { - service.rootCharacteristics(); + service.characteristics(); verify(debtModelLookup).rootCharacteristics(); } @Test - public void find_characteristics() { - service.characteristics(); - verify(debtModelLookup).characteristics(); + public void find_all_characteristics() { + service.allCharacteristics(); + verify(debtModelLookup).allCharacteristics(); } @Test -- 2.39.5