diff options
9 files changed, 263 insertions, 35 deletions
diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDao.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDao.java index 42ef216b738..0699b48354c 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDao.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicDao.java @@ -39,7 +39,6 @@ public class CharacteristicDao implements BatchComponent, ServerComponent { /** * @return enabled root characteristics and characteristics - * */ public List<CharacteristicDto> selectEnabledCharacteristics() { SqlSession session = mybatis.openSession(); @@ -56,7 +55,6 @@ public class CharacteristicDao implements BatchComponent, ServerComponent { /** * @return all characteristics - * */ public List<CharacteristicDto> selectCharacteristics() { SqlSession session = mybatis.openSession(); @@ -76,14 +74,20 @@ public class CharacteristicDao implements BatchComponent, ServerComponent { */ public List<CharacteristicDto> selectEnabledRootCharacteristics() { SqlSession session = mybatis.openSession(); - CharacteristicMapper mapper = session.getMapper(CharacteristicMapper.class); try { - return mapper.selectEnabledRootCharacteristics(); + return selectEnabledRootCharacteristics(session); } finally { MyBatis.closeQuietly(session); } } + /** + * @return only enabled root characteristics, order by order + */ + public List<CharacteristicDto> selectEnabledRootCharacteristics(SqlSession session) { + return session.getMapper(CharacteristicMapper.class).selectEnabledRootCharacteristics(); + } + @CheckForNull public CharacteristicDto selectByKey(String key) { SqlSession session = mybatis.openSession(); @@ -106,6 +110,35 @@ public class CharacteristicDao implements BatchComponent, ServerComponent { } } + @CheckForNull + public CharacteristicDto selectByName(String name) { + SqlSession session = mybatis.openSession(); + try { + return selectByName(name, session); + } finally { + MyBatis.closeQuietly(session); + } + } + + @CheckForNull + public CharacteristicDto selectByName(String name, SqlSession session) { + return session.getMapper(CharacteristicMapper.class).selectByName(name); + } + + public int selectMaxCharacteristicOrder() { + SqlSession session = mybatis.openSession(); + try { + return selectMaxCharacteristicOrder(session); + } finally { + MyBatis.closeQuietly(session); + } + } + + public int selectMaxCharacteristicOrder(SqlSession session) { + Integer result = session.getMapper(CharacteristicMapper.class).selectMaxCharacteristicOrder(); + return result != null ? result : 0; + } + public void insert(CharacteristicDto dto, SqlSession session) { session.getMapper(CharacteristicMapper.class).insert(dto); } diff --git a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicMapper.java b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicMapper.java index 8557c307c3e..b63e89ad456 100644 --- a/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/technicaldebt/db/CharacteristicMapper.java @@ -34,6 +34,10 @@ public interface CharacteristicMapper { CharacteristicDto selectById(int id); + CharacteristicDto selectByName(String name); + + Integer selectMaxCharacteristicOrder(); + void insert(CharacteristicDto characteristic); int update(CharacteristicDto characteristic); diff --git a/sonar-core/src/main/resources/org/sonar/core/technicaldebt/db/CharacteristicMapper.xml b/sonar-core/src/main/resources/org/sonar/core/technicaldebt/db/CharacteristicMapper.xml index 4cf3ea98e7f..d598985763c 100644 --- a/sonar-core/src/main/resources/org/sonar/core/technicaldebt/db/CharacteristicMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/technicaldebt/db/CharacteristicMapper.xml @@ -56,6 +56,24 @@ </where> </select> + <select id="selectByName" parameterType="String" resultType="Characteristic"> + select <include refid="characteristicColumns"/> + from characteristics c + <where> + and c.name=#{name} + and c.enabled=${_true} + </where> + </select> + + <select id="selectMaxCharacteristicOrder" resultType="Integer"> + select max(c.characteristic_order) + from characteristics c + <where> + and c.parent_id is null + and c.enabled=${_true} + </where> + </select> + <insert id="insert" parameterType="Characteristic" keyColumn="id" useGeneratedKeys="true" keyProperty="id"> INSERT INTO characteristics (kee, name, parent_id, characteristic_order, enabled, created_at, updated_at) VALUES (#{kee}, #{name}, #{parentId}, #{characteristicOrder}, #{enabled}, current_timestamp, current_timestamp) diff --git a/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDaoTest.java b/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDaoTest.java index 1a3fa00574a..82f6dba1d01 100644 --- a/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/technicaldebt/db/CharacteristicDaoTest.java @@ -120,6 +120,15 @@ public class CharacteristicDaoTest extends AbstractDaoTestCase { } @Test + public void select_characteristic_by_name() { + setupData("shared"); + + assertThat(dao.selectByName("Portability")).isNotNull(); + assertThat(dao.selectByName("Compiler related portability")).isNotNull(); + assertThat(dao.selectByName("Unknown")).isNull(); + } + + @Test public void select_characteristic_by_id() { setupData("shared"); @@ -130,6 +139,20 @@ public class CharacteristicDaoTest extends AbstractDaoTestCase { } @Test + public void select_max_characteristic_order() { + setupData("shared"); + + assertThat(dao.selectMaxCharacteristicOrder()).isEqualTo(1); + } + + @Test + public void select_max_characteristic_order_when_characteristics_are_all_disabled() { + setupData("select_max_characteristic_order_when_characteristics_are_all_disabled"); + + assertThat(dao.selectMaxCharacteristicOrder()).isEqualTo(0); + } + + @Test public void insert_characteristic() throws Exception { CharacteristicDto dto = new CharacteristicDto() .setKey("COMPILER_RELATED_PORTABILITY") diff --git a/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/select_max_characteristic_order_when_characteristics_are_all_disabled.xml b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/select_max_characteristic_order_when_characteristics_are_all_disabled.xml new file mode 100644 index 00000000000..78e951519bd --- /dev/null +++ b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/select_max_characteristic_order_when_characteristics_are_all_disabled.xml @@ -0,0 +1,13 @@ +<dataset> + + <!-- Disabled root characteristic --> + <characteristics id="1" kee="PORTABILITY" name="Portability" parent_id="[null]" characteristic_order="1" + enabled="[false]" + created_at="2013-11-20" updated_at="2013-11-22"/> + + <!-- Disabled root characteristic --> + <characteristics id="2" kee="DISABLED_ROOT_CHARACTERISTIC" name="Disabled root characteristic" parent_id="[null]" characteristic_order="2" + enabled="[false]" + created_at="2013-11-20" updated_at="2013-11-22"/> + +</dataset> diff --git a/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/shared.xml b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/shared.xml index e8c52c0a142..499959e3c3a 100644 --- a/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/shared.xml +++ b/sonar-core/src/test/resources/org/sonar/core/technicaldebt/db/CharacteristicDaoTest/shared.xml @@ -6,7 +6,7 @@ created_at="2013-11-20" updated_at="2013-11-22"/> <!-- Characteristic --> - <characteristics id="2" kee="COMPILER_RELATED_PORTABILITY" name="Compiler related portability" parent_id="1" root_id="1" characteristic_order="[null]" + <characteristics id="2" kee="COMPILER_RELATED_PORTABILITY" name="Compiler related portability" parent_id="1" rcharacteristic_order="[null]" enabled="[true]" created_at="2013-11-20" updated_at="2013-11-22"/> @@ -16,7 +16,7 @@ created_at="2013-11-20" updated_at="2013-11-22"/> <!-- Disabled characteristic --> - <characteristics id="5" kee="DISABLED_CHARACTERISTIC" name="Disabled characteristic" parent_id="4" root_id="4" characteristic_order="[null]" + <characteristics id="5" kee="DISABLED_CHARACTERISTIC" name="Disabled characteristic" parent_id="4" characteristic_order="[null]" enabled="[false]" created_at="2013-11-20" updated_at="2013-11-22"/> 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 a5a32807add..4d314d96d5a 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 @@ -22,13 +22,19 @@ package org.sonar.server.debt; 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.core.persistence.MyBatis; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.util.Validation; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import java.util.Collection; import java.util.List; @@ -37,12 +43,15 @@ import static com.google.common.collect.Lists.newArrayList; /** * Used through ruby code <pre>Internal.debt</pre> + * Also used by SQALE plugin. */ public class DebtModelService implements DebtModel { + private final MyBatis mybatis; private final CharacteristicDao dao; - public DebtModelService(CharacteristicDao dao) { + public DebtModelService(MyBatis mybatis, CharacteristicDao dao) { + this.mybatis = mybatis; this.dao = dao; } @@ -60,6 +69,49 @@ public class DebtModelService implements DebtModel { return dto != null ? toCharacteristic(dto) : null; } + public DebtCharacteristic createCharacteristic(String name, @Nullable Integer parentId) { + SqlSession session = mybatis.openSession(); + try { + checkNotAlreadyExists(name, session); + + CharacteristicDto newCharacteristic = new CharacteristicDto() + .setKey(name.toUpperCase().replace(" ", "_")) + .setName(name) + .setEnabled(true); + + // New sub characteristic + if (parentId != null) { + CharacteristicDto parent = findCharacteristic(parentId); + if (parent.getParentId() != null) { + throw new BadRequestException("A sub characteristic can not have a sub characteristic as parent."); + } + newCharacteristic.setParentId(parent.getId()); + } else { + // New root characteristic + newCharacteristic.setOrder(dao.selectMaxCharacteristicOrder(session)+1); + } + dao.insert(newCharacteristic, session); + session.commit(); + return toCharacteristic(newCharacteristic); + } finally { + MyBatis.closeQuietly(session); + } + } + + private CharacteristicDto findCharacteristic(Integer id){ + CharacteristicDto dto = dao.selectById(id); + if (dto == null) { + throw new NotFoundException(String.format("Characteristic with id %s does not exists.", id)); + } + return dto; + } + + private void checkNotAlreadyExists(String name, SqlSession session) { + if (dao.selectByName(name, session) != null) { + throw BadRequestException.ofL10n(Validation.IS_ALREADY_USED_MESSAGE, name); + } + } + private static List<DebtCharacteristic> toCharacteristics(Collection<CharacteristicDto> dtos) { return newArrayList(Iterables.transform(dtos, new Function<CharacteristicDto, DebtCharacteristic>() { @Override diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/characteristic.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/characteristic.rb index e04b5154720..42708225ed8 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/characteristic.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/characteristic.rb @@ -30,10 +30,9 @@ class Characteristic < ActiveRecord::Base MINUTE = "mn" belongs_to :parent, :class_name => 'Characteristic', :foreign_key => 'parent_id' - belongs_to :rule - validates_uniqueness_of :name, :scope => [:enabled], :case_sensitive => false, :if => Proc.new { |c| c.rule_id.nil? && c.enabled } - validates_length_of :name, :in => 1..NAME_MAX_SIZE, :allow_blank => false, :if => Proc.new { |c| c.rule_id.nil? } + validates_uniqueness_of :name, :scope => [:enabled], :case_sensitive => false, :if => Proc.new { |c| c.enabled } + validates_length_of :name, :in => 1..NAME_MAX_SIZE, :allow_blank => false def root? parent_id.nil? @@ -48,11 +47,7 @@ class Characteristic < ActiveRecord::Base end def name(rule_name_if_empty=false) - result=read_attribute(:name) - if (result.nil? && rule_name_if_empty && rule_id) - result=rule.name - end - result + read_attribute(:name) end end 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 8a59e05b387..3277e46cf90 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 @@ -19,17 +19,26 @@ */ package org.sonar.server.debt; +import org.apache.ibatis.session.SqlSession; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; +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.core.persistence.MyBatis; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; +import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.NotFoundException; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; +import static org.fest.assertions.Fail.fail; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -38,51 +47,132 @@ public class DebtModelServiceTest { @Mock CharacteristicDao dao; + @Mock + MyBatis mybatis; + + @Mock + SqlSession session; + DebtModelService service; + CharacteristicDto rootCharacteristicDto = new CharacteristicDto() + .setId(1) + .setKey("MEMORY_EFFICIENCY") + .setName("Memory use") + .setOrder(1); + + CharacteristicDto characteristicDto = new CharacteristicDto() + .setId(2) + .setKey("EFFICIENCY") + .setName("Efficiency") + .setParentId(1); + + int currentId; + @Before public void setUp() throws Exception { - service = new DebtModelService(dao); + currentId = 10; + + // Associate an id when inserting an object to simulate the db id generator + doAnswer(new Answer() { + public Object answer(InvocationOnMock invocation) { + Object[] args = invocation.getArguments(); + CharacteristicDto dto = (CharacteristicDto) args[0]; + dto.setId(++currentId); + return null; + } + }).when(dao).insert(any(CharacteristicDto.class), any(SqlSession.class)); + + when(mybatis.openSession()).thenReturn(session); + service = new DebtModelService(mybatis, dao); } @Test public void find_root_characteristics() { - CharacteristicDto dto = new CharacteristicDto() - .setId(1) - .setKey("MEMORY_EFFICIENCY") - .setName("Memory use"); - when(dao.selectEnabledRootCharacteristics()).thenReturn(newArrayList(dto)); + when(dao.selectEnabledRootCharacteristics()).thenReturn(newArrayList(rootCharacteristicDto)); assertThat(service.rootCharacteristics()).hasSize(1); } @Test public void find_characteristics() { - CharacteristicDto dto = new CharacteristicDto() - .setId(1) - .setKey("MEMORY_EFFICIENCY") - .setName("Memory use"); - when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(dto)); + when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(rootCharacteristicDto)); assertThat(service.characteristics()).hasSize(1); } @Test public void find_characteristic_by_id() { - CharacteristicDto dto = new CharacteristicDto() - .setId(1) - .setKey("MEMORY_EFFICIENCY") - .setName("Memory use") - .setParentId(2) - .setOrder(1); - when(dao.selectById(1)).thenReturn(dto); + when(dao.selectById(1)).thenReturn(rootCharacteristicDto); DebtCharacteristic characteristic = service.characteristicById(1); assertThat(characteristic.id()).isEqualTo(1); assertThat(characteristic.key()).isEqualTo("MEMORY_EFFICIENCY"); assertThat(characteristic.name()).isEqualTo("Memory use"); - assertThat(characteristic.parentId()).isEqualTo(2); assertThat(characteristic.order()).isEqualTo(1); + assertThat(characteristic.parentId()).isNull(); + + assertThat(service.characteristicById(111)).isNull(); + } + + @Test + public void create_sub_characteristic() { + when(dao.selectById(1)).thenReturn(rootCharacteristicDto); + + DebtCharacteristic result = service.createCharacteristic("Compilation name", 1); + + assertThat(result.id()).isEqualTo(currentId); + assertThat(result.key()).isEqualTo("COMPILATION_NAME"); + assertThat(result.name()).isEqualTo("Compilation name"); + assertThat(result.parentId()).isEqualTo(1); + } + + @Test + public void fail_to_create_sub_characteristic_when_parent_id_is_not_a_root_characteristic() { + when(dao.selectById(1)).thenReturn(characteristicDto); + + try { + service.createCharacteristic("Compilation", 1); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(BadRequestException.class).hasMessage("A sub characteristic can not have a sub characteristic as parent."); + } + } + + @Test + public void fail_to_create_sub_characteristic_when_parent_does_not_exists() { + when(dao.selectById(1)).thenReturn(null); + + try { + service.createCharacteristic("Compilation", 1); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class).hasMessage("Characteristic with id 1 does not exists."); + } + } + + @Test + public void fail_to_create_sub_characteristic_when_name_already_used() { + when(dao.selectByName("Compilation", session)).thenReturn(new CharacteristicDto()); + when(dao.selectById(1)).thenReturn(rootCharacteristicDto); + + try { + service.createCharacteristic("Compilation", 1); + fail(); + } catch (BadRequestException e) { + assertThat(e.l10nKey()).isEqualTo("errors.is_already_used"); + assertThat(e.l10nParams().iterator().next()).isEqualTo("Compilation"); + } + } + + @Test + public void create_characteristic() { + when(dao.selectMaxCharacteristicOrder(session)).thenReturn(1); + + DebtCharacteristic result = service.createCharacteristic("Portability", null); - assertThat(service.characteristicById(10)).isNull(); + assertThat(result.id()).isEqualTo(currentId); + assertThat(result.key()).isEqualTo("PORTABILITY"); + assertThat(result.name()).isEqualTo("Portability"); + assertThat(result.order()).isEqualTo(2); } } |