From 7a0e94ba8a9dbad2747985756411118d8b49bb7f Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 3 Apr 2014 17:52:22 +0200 Subject: [PATCH] Update server DebtModel API --- .../sonar/server/debt/DebtModelBackup.java | 4 ++-- .../sonar/server/debt/DebtModelLookup.java | 6 ++++++ .../sonar/server/debt/DebtModelService.java | 5 +++++ .../server/debt/DebtModelXMLExporter.java | 21 ++++++++++--------- .../server/issue/ws/IssueShowWsHandler.java | 9 ++++---- .../sonar/server/rule/RubyRuleService.java | 6 ++++++ .../java/org/sonar/server/rule/Rules.java | 2 ++ .../app/controllers/issue_controller.rb | 11 +++++----- .../WEB-INF/app/views/issue/_rule.html.erb | 4 ++-- .../server/debt/DebtModelLookupTest.java | 18 ++++++++++++++-- .../server/debt/DebtModelOperationsTest.java | 9 ++++---- .../server/debt/DebtModelServiceTest.java | 6 ++++++ .../issue/ws/IssueShowWsHandlerTest.java | 4 ++-- .../server/rule/RubyRuleServiceTest.java | 6 ++++++ 14 files changed, 79 insertions(+), 32 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 bf0e9c7ca76..aeae487b8b9 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 @@ -408,8 +408,8 @@ public class DebtModelBackup implements ServerComponent { .setOrder(characteristic.order()) .setParentId(parentId) .setEnabled(true) - .setCreatedAt(characteristic.createdAt()) - .setUpdatedAt(characteristic.updatedAt()); + .setCreatedAt(((DefaultDebtCharacteristic) characteristic).createdAt()) + .setUpdatedAt(((DefaultDebtCharacteristic) characteristic).updatedAt()); } private static DebtCharacteristic toDebtCharacteristic(CharacteristicDto characteristic) { 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 6d74c63641c..fbfeb0855a6 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 @@ -58,6 +58,12 @@ public class DebtModelLookup implements ServerComponent { return dto != null ? toCharacteristic(dto) : null; } + @CheckForNull + public DebtCharacteristic characteristicByKey(String key) { + CharacteristicDto dto = dao.selectByKey(key); + return dto != null ? toCharacteristic(dto) : null; + } + private static List toCharacteristics(Collection dtos) { return newArrayList(Iterables.transform(dtos, new Function() { @Override 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 a3c85737a0b..db4cd78bbe4 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 @@ -58,6 +58,11 @@ public class DebtModelService implements DebtModel { return debtModelLookup.characteristicById(id); } + @CheckForNull + public DebtCharacteristic characteristicByKey(String key) { + return debtModelLookup.characteristicByKey(key); + } + public DebtCharacteristic create(String name, @Nullable Integer parentId) { return debtModelOperations.create(name, parentId); } diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelXMLExporter.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelXMLExporter.java index 8a39f48fcb2..a10081174e9 100644 --- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelXMLExporter.java +++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelXMLExporter.java @@ -30,6 +30,7 @@ import org.sonar.api.ServerComponent; import org.sonar.api.rule.RuleKey; import org.sonar.api.server.debt.DebtCharacteristic; import org.sonar.api.server.debt.DebtRemediationFunction; +import org.sonar.api.server.debt.internal.DefaultDebtCharacteristic; import org.xml.sax.InputSource; import javax.annotation.CheckForNull; @@ -93,7 +94,7 @@ public class DebtModelXMLExporter implements ServerComponent { xml.append(""); } - if (characteristic.parentId() != null) { + if (characteristic.isSub()) { List rules = rules(allRules, characteristic.key()); for (RuleDebt ruleDto : rules) { processRule(ruleDto, xml); @@ -188,19 +189,19 @@ public class DebtModelXMLExporter implements ServerComponent { public static class DebtModel { - private Multimap characteristicsByRootKey; + private Multimap characteristicsByKey; public DebtModel() { - characteristicsByRootKey = ArrayListMultimap.create(); + characteristicsByKey = ArrayListMultimap.create(); } public DebtModel addRootCharacteristic(DebtCharacteristic characteristic) { - characteristicsByRootKey.put(null, characteristic); + characteristicsByKey.put(null, characteristic); return this; } public DebtModel addSubCharacteristic(DebtCharacteristic subCharacteristic, String characteristicKey) { - characteristicsByRootKey.put(characteristicKey, subCharacteristic); + characteristicsByKey.put(characteristicKey, subCharacteristic); return this; } @@ -208,19 +209,19 @@ public class DebtModelXMLExporter implements ServerComponent { * @return root characteristics sorted by order */ public List rootCharacteristics() { - return sortByOrder(newArrayList(characteristicsByRootKey.get(null))); + return sortByOrder(newArrayList(characteristicsByKey.get(null))); } /** * @return root characteristics sorted by name */ public List subCharacteristics(String characteristicKey) { - return sortByName(newArrayList(characteristicsByRootKey.get(characteristicKey))); + return sortByName(newArrayList(characteristicsByKey.get(characteristicKey))); } @CheckForNull public DebtCharacteristic characteristicByKey(final String key) { - return Iterables.find(characteristicsByRootKey.values(), new Predicate() { + return Iterables.find(characteristicsByKey.values(), new Predicate() { @Override public boolean apply(DebtCharacteristic input) { return key.equals(input.key()); @@ -229,10 +230,10 @@ public class DebtModelXMLExporter implements ServerComponent { } public DebtCharacteristic characteristicById(final Integer id) { - return Iterables.find(characteristicsByRootKey.values(), new Predicate() { + return Iterables.find(characteristicsByKey.values(), new Predicate() { @Override public boolean apply(DebtCharacteristic input) { - return id.equals(input.id()); + return id.equals(((DefaultDebtCharacteristic) input).id()); } }); } diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java index e41a035ea08..d32cf0202af 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueShowWsHandler.java @@ -28,7 +28,7 @@ import org.sonar.api.issue.action.Action; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.FieldDiffs; 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.server.ws.Request; import org.sonar.api.server.ws.RequestHandler; import org.sonar.api.server.ws.Response; @@ -41,6 +41,7 @@ import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.issue.workflow.Transition; import org.sonar.markdown.Markdown; +import org.sonar.server.debt.DebtModelService; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.issue.ActionService; import org.sonar.server.issue.IssueChangelog; @@ -63,12 +64,12 @@ public class IssueShowWsHandler implements RequestHandler { private final IssueService issueService; private final IssueChangelogService issueChangelogService; private final ActionService actionService; - private final DebtModel debtModel; + private final DebtModelService debtModel; private final I18n i18n; private final Durations durations; public IssueShowWsHandler(IssueFinder issueFinder, IssueService issueService, IssueChangelogService issueChangelogService, ActionService actionService, - DebtModel debtModel, I18n i18n, Durations durations) { + DebtModelService debtModel, I18n i18n, Durations durations) { this.issueFinder = issueFinder; this.issueService = issueService; this.issueChangelogService = issueChangelogService; @@ -193,7 +194,7 @@ public class IssueShowWsHandler implements RequestHandler { DebtCharacteristic subCharacteristic = characteristicById(subCharacteristicId); if (subCharacteristic != null) { json.prop("subCharacteristic", subCharacteristic.name()); - DebtCharacteristic characteristic = characteristicById(subCharacteristic.parentId()); + DebtCharacteristic characteristic = characteristicById(((DefaultDebtCharacteristic) subCharacteristic).parentId()); json.prop("characteristic", characteristic != null ? characteristic.name() : null); } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java b/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java index 4ced6efd70c..6d4d445fc9f 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java @@ -28,6 +28,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.server.paging.PagedResult; import org.sonar.server.util.RubyUtils; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.util.Map; @@ -63,6 +64,11 @@ public class RubyRuleService implements ServerComponent, Startable { } } + @CheckForNull + public Rule findByKey(String ruleKey) { + return rules.findByKey(RuleKey.parse(ruleKey)); + } + public PagedResult find(Map params) { return rules.find(RuleQuery.builder() .searchQuery(Strings.emptyToNull((String) params.get("searchQuery"))) diff --git a/sonar-server/src/main/java/org/sonar/server/rule/Rules.java b/sonar-server/src/main/java/org/sonar/server/rule/Rules.java index 25405d66207..e4840358e73 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/Rules.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/Rules.java @@ -33,6 +33,7 @@ import org.sonar.server.user.UserSession; import org.sonar.server.util.RubyUtils; import org.sonar.server.util.Validation; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import java.util.List; @@ -107,6 +108,7 @@ public class Rules implements ServerExtension { ruleOperations.updateRuleTags(rule, newTags, UserSession.get()); } + @CheckForNull public Rule findByKey(RuleKey key) { return ruleRegistry.findByKey(key); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb index 43542de7520..9d37e5ac83e 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issue_controller.rb @@ -185,12 +185,11 @@ class IssueController < ApplicationController def rule verify_ajax_request require_parameters :id - rule_key = params[:id].split(':') - @rule = Rule.first(:conditions => ['plugin_name=? and plugin_rule_key=?', rule_key[0], rule_key[1]]) - characteristic_id = @rule.characteristic_id || @rule.default_characteristic_id - if characteristic_id - @characteristic = Internal.debt.characteristicById(characteristic_id) - @root_characteristic = Internal.debt.characteristicById(@characteristic.parentId()) + + @rule = Internal.rules.findByKey(params[:id]) + if @rule.debtCharacteristicKey() + @characteristic = Internal.debt.characteristicByKey(@rule.debtCharacteristicKey()) + @sub_characteristic = Internal.debt.characteristicByKey(@rule.debtSubCharacteristicKey()) end render :partial => 'issue/rule' end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb index f466660d508..9214ce50513 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_rule.html.erb @@ -17,8 +17,8 @@

<%= h @rule.plugin_name -%>:<%= h @rule.plugin_rule_key -%> <%= image_tag 'sep12.png', :class => 'spacer-right' -%> - <% if @characteristic %> - <%= @root_characteristic.name -%> > <%= @characteristic.name -%> + <% if @characteristic && @sub_characteristic %> + <%= @characteristic.name -%> > <%= @sub_characteristic.name -%> <% else %> <%= message 'issue.technical_debt_deleted' %> <% end %> 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 eedc79f38af..a0390191cc9 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 @@ -25,7 +25,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.sonar.api.server.debt.DebtCharacteristic; +import org.sonar.api.server.debt.internal.DefaultDebtCharacteristic; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; @@ -69,7 +69,7 @@ public class DebtModelLookupTest { public void find_characteristic_by_id() { when(dao.selectById(1)).thenReturn(characteristicDto); - DebtCharacteristic characteristic = service.characteristicById(1); + DefaultDebtCharacteristic characteristic = (DefaultDebtCharacteristic) service.characteristicById(1); assertThat(characteristic.id()).isEqualTo(1); assertThat(characteristic.key()).isEqualTo("MEMORY_EFFICIENCY"); assertThat(characteristic.name()).isEqualTo("Memory use"); @@ -79,4 +79,18 @@ public class DebtModelLookupTest { assertThat(service.characteristicById(111)).isNull(); } + @Test + public void find_characteristic_by_key() { + when(dao.selectByKey("MEMORY_EFFICIENCY")).thenReturn(characteristicDto); + + DefaultDebtCharacteristic characteristic = (DefaultDebtCharacteristic) service.characteristicByKey("MEMORY_EFFICIENCY"); + assertThat(characteristic.id()).isEqualTo(1); + assertThat(characteristic.key()).isEqualTo("MEMORY_EFFICIENCY"); + assertThat(characteristic.name()).isEqualTo("Memory use"); + assertThat(characteristic.order()).isEqualTo(2); + assertThat(characteristic.parentId()).isNull(); + + assertThat(service.characteristicByKey("UNKNOWN")).isNull(); + } + } 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 baf1f8bd060..8b871dce3f2 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 @@ -31,6 +31,7 @@ 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.server.debt.internal.DefaultDebtCharacteristic; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.System2; import org.sonar.core.permission.GlobalPermissions; @@ -126,7 +127,7 @@ public class DebtModelOperationsTest { public void create_sub_characteristic() { when(dao.selectById(1, session)).thenReturn(characteristicDto); - DebtCharacteristic result = service.create("Compilation name", 1); + DefaultDebtCharacteristic result = (DefaultDebtCharacteristic) service.create("Compilation name", 1); assertThat(result.id()).isEqualTo(currentId); assertThat(result.key()).isEqualTo("COMPILATION_NAME"); @@ -189,7 +190,7 @@ public class DebtModelOperationsTest { public void create_characteristic() { when(dao.selectMaxCharacteristicOrder(session)).thenReturn(2); - DebtCharacteristic result = service.create("Portability", null); + DefaultDebtCharacteristic result = (DefaultDebtCharacteristic) service.create("Portability", null); assertThat(result.id()).isEqualTo(currentId); assertThat(result.key()).isEqualTo("PORTABILITY"); @@ -202,7 +203,7 @@ public class DebtModelOperationsTest { public void create_first_characteristic() { when(dao.selectMaxCharacteristicOrder(session)).thenReturn(0); - DebtCharacteristic result = service.create("Portability", null); + DefaultDebtCharacteristic result = (DefaultDebtCharacteristic) service.create("Portability", null); assertThat(result.id()).isEqualTo(currentId); assertThat(result.key()).isEqualTo("PORTABILITY"); @@ -215,7 +216,7 @@ public class DebtModelOperationsTest { public void rename_characteristic() { when(dao.selectById(10, session)).thenReturn(subCharacteristicDto); - DebtCharacteristic result = service.rename(10, "New Efficiency"); + DefaultDebtCharacteristic result = (DefaultDebtCharacteristic) service.rename(10, "New Efficiency"); assertThat(result.key()).isEqualTo("EFFICIENCY"); assertThat(result.name()).isEqualTo("New Efficiency"); 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 404c1e83c3b..e5309e9f8d8 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 @@ -64,6 +64,12 @@ public class DebtModelServiceTest { verify(debtModelLookup).characteristicById(111); } + @Test + public void find_characteristic_by_key() { + service.characteristicByKey("MEMORY_EFFICIENCY"); + verify(debtModelLookup).characteristicByKey("MEMORY_EFFICIENCY"); + } + @Test public void create_characteristic() { service.create("Compilation name", 1); diff --git a/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java index 1624b8704f0..9f21f8646cc 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/ws/IssueShowWsHandlerTest.java @@ -37,7 +37,6 @@ import org.sonar.api.issue.internal.DefaultIssueComment; import org.sonar.api.issue.internal.FieldDiffs; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; -import org.sonar.api.server.debt.DebtModel; import org.sonar.api.server.debt.internal.DefaultDebtCharacteristic; import org.sonar.api.server.ws.WsTester; import org.sonar.api.user.User; @@ -50,6 +49,7 @@ import org.sonar.core.issue.DefaultActionPlan; import org.sonar.core.issue.DefaultIssueQueryResult; import org.sonar.core.issue.workflow.Transition; import org.sonar.core.user.DefaultUser; +import org.sonar.server.debt.DebtModelService; import org.sonar.server.issue.ActionService; import org.sonar.server.issue.IssueChangelog; import org.sonar.server.issue.IssueChangelogService; @@ -84,7 +84,7 @@ public class IssueShowWsHandlerTest { ActionService actionService; @Mock - DebtModel debtModel; + DebtModelService debtModel; @Mock I18n i18n; diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java index 89b12ef17d3..59c1cdcc04e 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java @@ -129,6 +129,12 @@ public class RubyRuleServiceTest { verify(rules).updateRuleTags(10, ImmutableList.of("tag1", "tag2")); } + @Test + public void find_by_key() { + facade.findByKey("repo:key"); + verify(rules).findByKey(RuleKey.of("repo", "key")); + } + @Test public void find_by_params() { Map params = newHashMap(); -- 2.39.5