From: Julien Lancelot Date: Mon, 13 Jan 2014 16:55:11 +0000 (+0100) Subject: SONAR-4923 finders from lookup should return null and not throw exceptions X-Git-Tag: 4.2~725 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=0c67addd5cbe173c2300fd022297fa4c5cb5f30b;p=sonarqube.git SONAR-4923 finders from lookup should return null and not throw exceptions --- diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java index d9e2c885c67..1aef41d8f35 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperations.java @@ -46,6 +46,8 @@ import org.sonar.server.rule.ProfileRules; import org.sonar.server.rule.RuleRegistry; import org.sonar.server.user.UserSession; +import javax.annotation.Nullable; + import java.util.Date; import java.util.List; @@ -81,6 +83,18 @@ public class QProfileActiveRuleOperations implements ServerComponent { this.system = system; } +// public ProfileRuleChanged activateRule(int profileId, int ruleId, String severity) { +// QProfile profile = profileLookup.profile(profileId); +// RuleDto rule = findRuleNotNull(ruleId); +// ActiveRuleDto activeRule = findActiveRule(qualityProfile, rule); +// if (activeRule == null) { +// activeRule = activeRuleOperations.createActiveRule(qualityProfile, rule, severity, UserSession.get()); +// } else { +// activeRuleOperations.updateSeverity(activeRule, severity, UserSession.get()); +// } +// return activeRuleChanged(qualityProfile, activeRule); +// } + public ActiveRuleDto createActiveRule(QualityProfileDto qualityProfile, RuleDto rule, String severity, UserSession userSession) { validatePermission(userSession); validateSeverity(severity); @@ -403,4 +417,29 @@ public class QProfileActiveRuleOperations implements ServerComponent { return Severity.ALL.indexOf(severity); } + public static class ProfileRuleChanged { + + private QProfile profile; + private QProfile parentProfile; + private QProfileRule rule; + + public ProfileRuleChanged(QProfile profile, @Nullable QProfile parentProfile, QProfileRule rule) { + this.profile = profile; + this.parentProfile = parentProfile; + this.rule = rule; + } + + public QProfile profile() { + return profile; + } + + public QProfile parentProfile() { + return parentProfile; + } + + public QProfileRule rule() { + return rule; + } + } + } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLookup.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLookup.java index a192ce10780..b6dc599d6b0 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLookup.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileLookup.java @@ -53,14 +53,22 @@ public class QProfileLookup implements ServerComponent { return toQProfiles(dao.selectByLanguage(language)); } + @CheckForNull public QProfile profile(int id) { - return QProfile.from(findNotNull(id)); + QualityProfileDto dto = findQualityProfile(id); + if (dto != null) { + return QProfile.from(dto); + } + return null; } + @CheckForNull public QProfile profile(String name, String language) { - QualityProfileDto profile = findQualityProfile(name, language); - checkNotNull(profile); - return QProfile.from(profile); + QualityProfileDto dto = findQualityProfile(name, language); + if (dto != null) { + return QProfile.from(dto); + } + return null; } @CheckForNull @@ -72,10 +80,15 @@ public class QProfileLookup implements ServerComponent { return null; } + @CheckForNull public QProfile parent(QProfile profile) { - QualityProfileDto parent = findQualityProfile(profile.parent(), profile.language()); - checkNotNull(parent); - return QProfile.from(parent); + if (profile.parent() != null) { + QualityProfileDto parent = findQualityProfile(profile.parent(), profile.language()); + if (parent != null) { + return QProfile.from(parent); + } + } + return null; } public List children(QProfile profile) { @@ -118,11 +131,6 @@ public class QProfileLookup implements ServerComponent { })); } - private QualityProfileDto findNotNull(int id) { - QualityProfileDto qualityProfile = findQualityProfile(id); - return checkNotNull(qualityProfile); - } - @CheckForNull private QualityProfileDto findQualityProfile(int id) { return dao.selectById(id); diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java index 4cab55a6ee6..a8440691db3 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java @@ -41,6 +41,7 @@ import org.sonar.core.properties.PropertyDto; import org.sonar.core.qualityprofile.db.*; import org.sonar.server.configuration.ProfilesManager; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.RuleRegistry; import org.sonar.server.user.UserSession; @@ -113,7 +114,8 @@ public class QProfileOperations implements ServerComponent { public void renameProfile(int profileId, String newName, UserSession userSession) { checkPermission(userSession); - QProfile profile = profileLookup.profile(profileId); + QProfile profile = findNotNull(profileId); + if (!profile.name().equals(newName)) { checkNotAlreadyExists(newName, profile.language()); } @@ -129,10 +131,11 @@ public class QProfileOperations implements ServerComponent { public void updateParentProfile(int profileId, @Nullable Integer parentId, UserSession userSession) { checkPermission(userSession); - QualityProfileDto profile = profileLookup.profile(profileId).toDto(); + QualityProfileDto profile = findNotNull(profileId).toDto(); + QualityProfileDto parentProfile = null; if (parentId != null) { - parentProfile = profileLookup.profile(parentId).toDto(); + parentProfile = findNotNull(parentId).toDto(); } SqlSession session = myBatis.openSession(); @@ -250,8 +253,16 @@ public class QProfileOperations implements ServerComponent { userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); } + private QProfile findNotNull(int profileId) { + QProfile profile = profileLookup.profile(profileId); + if (profile == null) { + throw new NotFoundException("This quality profile does not exists."); + } + return profile; + } + private void checkNotAlreadyExists(String name, String language) { - if (dao.selectByNameAndLanguage(name, language) != null) { + if (profileLookup.profile(name, language) != null) { throw BadRequestException.ofL10n("quality_profiles.already_exists"); } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java index 13a76151ed8..7023de4c422 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfiles.java @@ -88,10 +88,12 @@ public class QProfiles implements ServerComponent { return profileLookup.profiles(language); } + @CheckForNull public QProfile profile(int id) { return profileLookup.profile(id); } + @CheckForNull public QProfile profile(String name, String language) { validateProfileName(name); Validation.checkMandatoryParameter(language, LANGUAGE_PARAM); @@ -122,7 +124,8 @@ public class QProfiles implements ServerComponent { QualityProfileDto qualityProfile = findNotNull(profileId); operations.setDefaultProfile(qualityProfile, UserSession.get()); } - + + @CheckForNull public QProfile parent(QProfile profile) { return profileLookup.parent(profile); } @@ -189,7 +192,7 @@ public class QProfiles implements ServerComponent { // PROFILE RULES public ProfileRules.QProfileRuleResult searchProfileRules(ProfileRuleQuery query, Paging paging) { - return rules.searchProfileRules(query, paging); + return rules.search(query, paging); } public long countProfileRules(ProfileRuleQuery query) { @@ -197,7 +200,7 @@ public class QProfiles implements ServerComponent { } public ProfileRules.QProfileRuleResult searchInactiveProfileRules(ProfileRuleQuery query, Paging paging) { - return rules.searchInactiveProfileRules(query, paging); + return rules.searchInactives(query, paging); } public long countInactiveProfileRules(ProfileRuleQuery query) { @@ -265,20 +268,20 @@ public class QProfiles implements ServerComponent { } // Empty note -> do nothing - return rules.getFromActiveRuleId(activeRule.getId()); + return rules.findByActiveRuleId(activeRule.getId()); } public QProfileRule deleteActiveRuleNote(int activeRuleId) { ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); activeRuleOperations.deleteActiveRuleNote(activeRule, UserSession.get()); - return rules.getFromActiveRuleId(activeRule.getId()); + return rules.findByActiveRuleId(activeRule.getId()); } @CheckForNull public QProfileRule parentProfileRule(QProfileRule rule) { Integer parentId = rule.parentId(); if (parentId != null) { - return rules.getFromActiveRuleId(parentId); + return rules.findByActiveRuleId(parentId); } return null; } @@ -295,19 +298,19 @@ public class QProfiles implements ServerComponent { ruleOperations.deleteRuleNote(rule, UserSession.get()); } ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); - return rules.getFromActiveRuleId(activeRule.getId()); + return rules.findByActiveRuleId(activeRule.getId()); } @CheckForNull public QProfileRule rule(int ruleId) { - return rules.getFromRuleId(ruleId); + return rules.findByRuleId(ruleId); } public QProfileRule createRule(int ruleId, @Nullable String name, @Nullable String severity, @Nullable String description, Map paramsByKey) { RuleDto rule = findRuleNotNull(ruleId); validateRule(null, name, severity, description); RuleDto newRule = ruleOperations.createRule(rule, name, severity, description, paramsByKey, UserSession.get()); - return rules.getFromRuleId(newRule.getId()); + return rules.findByRuleId(newRule.getId()); } public QProfileRule updateRule(int ruleId, @Nullable String name, @Nullable String severity, @Nullable String description, Map paramsByKey) { @@ -315,7 +318,7 @@ public class QProfiles implements ServerComponent { validateRuleParent(rule); validateRule(ruleId, name, severity, description); ruleOperations.updateRule(rule, name, severity, description, paramsByKey, UserSession.get()); - return rules.getFromRuleId(ruleId); + return rules.findByRuleId(ruleId); } public void deleteRule(int ruleId) { @@ -465,12 +468,12 @@ public class QProfiles implements ServerComponent { private ProfileRuleChanged activeRuleChanged(QualityProfileDto qualityProfile, ActiveRuleDto activeRule) { QProfile profile = QProfile.from(qualityProfile); - return new ProfileRuleChanged(profile, findParent(profile), rules.getFromActiveRuleId(activeRule.getId())); + return new ProfileRuleChanged(profile, findParent(profile), rules.findByActiveRuleId(activeRule.getId())); } private ProfileRuleChanged activeRuleChanged(QualityProfileDto qualityProfile, RuleDto rule) { QProfile profile = QProfile.from(qualityProfile); - return new ProfileRuleChanged(profile, findParent(profile), rules.getFromRuleId(rule.getId())); + return new ProfileRuleChanged(profile, findParent(profile), rules.findByRuleId(rule.getId())); } public static class ProfileRuleChanged { diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java index ee7ecf31ff2..37b6ac4c7e7 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java @@ -39,6 +39,8 @@ import org.sonar.server.qualityprofile.PagingResult; import org.sonar.server.qualityprofile.QProfileRule; import org.sonar.server.search.SearchIndex; +import javax.annotation.CheckForNull; + import java.util.Collection; import java.util.List; import java.util.Map; @@ -61,23 +63,33 @@ public class ProfileRules implements ServerExtension { this.index = index; } - public QProfileRule getFromActiveRuleId(int activeRuleId) { + @CheckForNull + public QProfileRule findByActiveRuleId(int activeRuleId) { GetResponse activeRuleResponse = index.client().prepareGet(INDEX_RULES, TYPE_ACTIVE_RULE, Integer.toString(activeRuleId)) .setFields(FIELD_SOURCE, FIELD_PARENT) .execute().actionGet(); Map activeRuleSource = activeRuleResponse.getSourceAsMap(); - Map ruleSource = index.client().prepareGet(INDEX_RULES, TYPE_RULE, (String) activeRuleResponse.getField(FIELD_PARENT).getValue()) - .execute().actionGet().getSourceAsMap(); - return new QProfileRule(ruleSource, activeRuleSource); + if (activeRuleSource != null) { + Map ruleSource = index.client().prepareGet(INDEX_RULES, TYPE_RULE, (String) activeRuleResponse.getField(FIELD_PARENT).getValue()) + .execute().actionGet().getSourceAsMap(); + if (ruleSource != null) { + return new QProfileRule(ruleSource, activeRuleSource); + } + } + return null; } - public QProfileRule getFromRuleId(Integer ruleId) { + @CheckForNull + public QProfileRule findByRuleId(Integer ruleId) { Map ruleSource = index.client().prepareGet(INDEX_RULES, TYPE_RULE, Integer.toString(ruleId)) .execute().actionGet().getSourceAsMap(); - return new QProfileRule(ruleSource); + if (ruleSource != null) { + return new QProfileRule(ruleSource); + } + return null; } - public QProfileRuleResult searchProfileRules(ProfileRuleQuery query, Paging paging) { + public QProfileRuleResult search(ProfileRuleQuery query, Paging paging) { SearchHits ruleHits = searchRules(query, paging, ruleFilterForActiveRuleSearch(query).must(hasChildFilter(TYPE_ACTIVE_RULE, activeRuleFilter(query)))); List ruleIds = Lists.newArrayList(); for (SearchHit ruleHit : ruleHits) { @@ -139,7 +151,7 @@ public class ProfileRules implements ServerExtension { ); } - public QProfileRuleResult searchInactiveProfileRules(ProfileRuleQuery query, Paging paging) { + public QProfileRuleResult searchInactives(ProfileRuleQuery query, Paging paging) { SearchHits hits = searchRules(query, paging, ruleFilterForInactiveRuleSearch(query), FIELD_SOURCE, FIELD_PARENT); List result = Lists.newArrayList(); for (SearchHit hit : hits.getHits()) { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb index 241cf15636b..92bdbde14f7 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/profiles_controller.rb @@ -95,6 +95,7 @@ class Api::ProfilesController < Api::ApiController def set_as_default verify_post_request profile = Internal.quality_profiles.profile(params[:name], params[:language]) + not_found('Profile not found') unless profile Internal.quality_profiles.setDefaultProfile(profile.id) render_success end diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileLookupTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileLookupTest.java index f474a70c7cc..d1f04813e47 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileLookupTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileLookupTest.java @@ -29,7 +29,6 @@ import org.mockito.runners.MockitoJUnitRunner; import org.sonar.core.persistence.MyBatis; import org.sonar.core.qualityprofile.db.QualityProfileDao; import org.sonar.core.qualityprofile.db.QualityProfileDto; -import org.sonar.server.exceptions.NotFoundException; import java.util.List; @@ -61,7 +60,7 @@ public class QProfileLookupTest { } @Test - public void search_profile_from_id() throws Exception { + public void find_by_id() throws Exception { when(dao.selectById(1)).thenReturn( new QualityProfileDto().setId(1).setName("Sonar Way with Findbugs").setLanguage("java").setParent("Sonar Way").setVersion(1).setUsed(false) ); @@ -76,22 +75,20 @@ public class QProfileLookupTest { } @Test - public void search_profile_from_name_and_language() throws Exception { + public void find_by_id_return_null_if_not_exists() throws Exception { + assertThat(search.profile(1)).isNull(); + } + + @Test + public void find_by_name_and_language() throws Exception { when(dao.selectByNameAndLanguage("Sonar Way", "java")).thenReturn(new QualityProfileDto().setId(1).setName("Sonar Way").setLanguage("java")); assertThat(search.profile("Sonar Way", "java")).isNotNull(); } @Test - public void fail_to_search_profile_from_id_if_not_found() throws Exception { - when(dao.selectById(1)).thenReturn(null); - - try { - search.profile(1); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(NotFoundException.class); - } + public void find_by_name_and_language_return_null_if_not_exists() throws Exception { + assertThat(search.profile("Sonar Way", "java")).isNull(); } @Test @@ -120,21 +117,21 @@ public class QProfileLookupTest { @Test - public void search_parent() throws Exception { + public void find_parent() throws Exception { when(dao.selectByNameAndLanguage("Sonar Way", "java")).thenReturn(new QualityProfileDto().setId(1).setName("Sonar Way").setLanguage("java")); search.parent(new QProfile().setName("Sonar Way with Findbugs").setLanguage("java").setParent("Sonar Way")); verify(dao).selectByNameAndLanguage("Sonar Way", "java"); } @Test - public void fail_to_search_parent_if_parent_cannot_be_found() throws Exception { + public void find_parent_return_null_if_no_parent() throws Exception { + assertThat(search.parent(new QProfile().setName("Sonar Way with Findbugs").setLanguage("java").setParent(null))).isNull(); + } + + @Test + public void find_parent_return_null_if_parent_not_exists() throws Exception { when(dao.selectByNameAndLanguage("Sonar Way", "java")).thenReturn(null); - try { - search.parent(new QProfile().setName("Sonar Way").setLanguage("java").setParent("Parent")); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(NotFoundException.class); - } + assertThat(search.parent(new QProfile().setName("Sonar Way with Findbugs").setLanguage("java").setParent("Sonar Way"))).isNull(); } @Test diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java index 11cc725acb1..4cfe652f7de 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java @@ -46,6 +46,7 @@ import org.sonar.core.qualityprofile.db.*; import org.sonar.server.configuration.ProfilesManager; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.RuleRegistry; import org.sonar.server.user.MockUserSession; import org.sonar.server.user.UserSession; @@ -151,7 +152,7 @@ public class QProfileOperationsTest { @Test public void fail_to_create_profile_if_already_exists() throws Exception { - when(qualityProfileDao.selectByNameAndLanguage(anyString(), anyString())).thenReturn(new QualityProfileDto()); + when(profileLookup.profile(anyString(), anyString())).thenReturn(new QProfile()); try { operations.newProfile("Default", "java", Maps.newHashMap(), authorizedUserSession); fail(); @@ -236,10 +237,21 @@ public class QProfileOperationsTest { assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java"); } + @Test + public void fail_to_rename_profile_if_not_exists() throws Exception { + when(profileLookup.profile(1)).thenReturn(null); + try { + operations.renameProfile(1, "New Default", authorizedUserSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class); + } + } + @Test public void fail_to_rename_profile_if_already_exists() throws Exception { when(profileLookup.profile(1)).thenReturn(new QProfile().setId(1).setName("Default").setLanguage("java")); - when(qualityProfileDao.selectByNameAndLanguage(anyString(), anyString())).thenReturn(new QualityProfileDto()); + when(profileLookup.profile(anyString(), anyString())).thenReturn(new QProfile()); try { operations.renameProfile(1, "New Default", authorizedUserSession); fail(); @@ -319,6 +331,19 @@ public class QProfileOperationsTest { verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } + @Test + public void fail_to_update_parent_if_not_exists() throws Exception { + when(profileLookup.profile(1)).thenReturn(null); + when(profileLookup.profile(3)).thenReturn(new QProfile().setId(3).setName("Parent").setLanguage("java")); + + try { + operations.updateParentProfile(1, 3, authorizedUserSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class); + } + } + @Test public void fail_to_update_parent_on_cycle() { when(profileLookup.profile(1)).thenReturn(new QProfile().setId(1).setName("Child").setLanguage("java").setParent("parent")); diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java index f2e8da548c4..9693c6d548e 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfilesTest.java @@ -284,7 +284,7 @@ public class QProfilesTest { when(rule.parentId()).thenReturn(6); qProfiles.parentProfileRule(rule); - verify(rules).getFromActiveRuleId(6); + verify(rules).findByActiveRuleId(6); } @Test @@ -294,7 +294,7 @@ public class QProfilesTest { when(rule.parentId()).thenReturn(null); assertThat(qProfiles.parentProfileRule(rule)).isNull(); - verify(rules, never()).getFromActiveRuleId(anyInt()); + verify(rules, never()).findByActiveRuleId(anyInt()); } @Test @@ -303,7 +303,7 @@ public class QProfilesTest { ProfileRuleQuery query = ProfileRuleQuery.create(profileId); Paging paging = Paging.create(20, 1); ProfileRules.QProfileRuleResult result = mock(ProfileRules.QProfileRuleResult.class); - when(rules.searchProfileRules(query, paging)).thenReturn(result); + when(rules.search(query, paging)).thenReturn(result); assertThat(qProfiles.searchProfileRules(query, paging)).isEqualTo(result); } @@ -313,7 +313,7 @@ public class QProfilesTest { ProfileRuleQuery query = ProfileRuleQuery.create(profileId); Paging paging = Paging.create(20, 1); ProfileRules.QProfileRuleResult result = mock(ProfileRules.QProfileRuleResult.class); - when(rules.searchInactiveProfileRules(query, paging)).thenReturn(result); + when(rules.searchInactives(query, paging)).thenReturn(result); assertThat(qProfiles.searchInactiveProfileRules(query, paging)).isEqualTo(result); } @@ -553,7 +553,7 @@ public class QProfilesTest { public void get_rule_from_id() throws Exception { qProfiles.rule(10); - verify(rules).getFromRuleId(10); + verify(rules).findByRuleId(10); } @Test @@ -568,7 +568,7 @@ public class QProfilesTest { qProfiles.createRule(10, "Rule name", Severity.MAJOR, "My note", paramsByKey); verify(ruleOperations).createRule(eq(rule), eq("Rule name"), eq(Severity.MAJOR), eq("My note"), eq(paramsByKey), any(UserSession.class)); - verify(rules).getFromRuleId(11); + verify(rules).findByRuleId(11); } @Test @@ -624,7 +624,7 @@ public class QProfilesTest { qProfiles.updateRule(11, "Updated name", Severity.MAJOR, "Updated description", paramsByKey); verify(ruleOperations).updateRule(eq(rule), eq("Updated name"), eq(Severity.MAJOR), eq("Updated description"), eq(paramsByKey), any(UserSession.class)); - verify(rules).getFromRuleId(11); + verify(rules).findByRuleId(11); } @Test @@ -638,7 +638,7 @@ public class QProfilesTest { qProfiles.updateRule(11, "Rule name", Severity.MAJOR, "Updated description", paramsByKey); verify(ruleOperations).updateRule(eq(rule), eq("Rule name"), eq(Severity.MAJOR), eq("Updated description"), eq(paramsByKey), any(UserSession.class)); - verify(rules).getFromRuleId(11); + verify(rules).findByRuleId(11); } @Test diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java index 6121c111660..d65a7275095 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java @@ -85,66 +85,78 @@ public class ProfileRulesTest { esSetup.terminate(); } + @Test + public void find_by_rule_id() { + assertThat(profileRules.findByRuleId(25)).isNotNull(); + assertThat(profileRules.findByRuleId(9999)).isNull(); + } + + @Test + public void find_by_active_rule_id() { + assertThat(profileRules.findByActiveRuleId(391)).isNotNull(); + assertThat(profileRules.findByActiveRuleId(9999)).isNull(); + } + @Test public void find_profile_rules() { Paging paging = Paging.create(10, 1); // All rules for profile 1 - List rules1 = profileRules.searchProfileRules(ProfileRuleQuery.create(1), paging).rules(); + List rules1 = profileRules.search(ProfileRuleQuery.create(1), paging).rules(); assertThat(rules1).hasSize(3); assertThat(rules1.get(0).key()).isEqualTo("ArchitecturalConstraint"); assertThat(rules1.get(0).severity()).isEqualTo(Severity.CRITICAL); // All rules for profile 2 - List rules2 = profileRules.searchProfileRules(ProfileRuleQuery.create(2), paging).rules(); + List rules2 = profileRules.search(ProfileRuleQuery.create(2), paging).rules(); assertThat(rules2).hasSize(1); assertThat(rules2.get(0).id()).isEqualTo(759); assertThat(rules2.get(0).activeRuleId()).isEqualTo(523); // Match on key - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).setNameOrKey("DM_CONVERT_CASE"), paging).rules()).hasSize(1); + assertThat(profileRules.search(ProfileRuleQuery.create(1).setNameOrKey("DM_CONVERT_CASE"), paging).rules()).hasSize(1); // Match on name - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).setNameOrKey("Unused Check"), paging).rules()).hasSize(1); + assertThat(profileRules.search(ProfileRuleQuery.create(1).setNameOrKey("Unused Check"), paging).rules()).hasSize(1); // Match on repositoryKey - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).addRepositoryKeys("findbugs"), paging).rules()).hasSize(1); + assertThat(profileRules.search(ProfileRuleQuery.create(1).addRepositoryKeys("findbugs"), paging).rules()).hasSize(1); - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).addSeverities(Severity.CRITICAL), paging).rules()).hasSize(1); + assertThat(profileRules.search(ProfileRuleQuery.create(1).addSeverities(Severity.CRITICAL), paging).rules()).hasSize(1); // Active rule 25 is in MINOR (rule 25 is in INFO) - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).addSeverities(Severity.INFO), paging).rules()).isEmpty(); + assertThat(profileRules.search(ProfileRuleQuery.create(1).addSeverities(Severity.INFO), paging).rules()).isEmpty(); // Match on key, rule has parameters - List rulesWParam = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setNameOrKey("ArchitecturalConstraint"), paging).rules(); + List rulesWParam = profileRules.search(ProfileRuleQuery.create(1).setNameOrKey("ArchitecturalConstraint"), paging).rules(); assertThat(rulesWParam).hasSize(1); assertThat(rulesWParam.get(0).params()).hasSize(2); // Inexistent profile - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(3), paging).rules()).hasSize(0); + assertThat(profileRules.search(ProfileRuleQuery.create(3), paging).rules()).hasSize(0); // Inexistent name/key - assertThat(profileRules.searchProfileRules(ProfileRuleQuery.create(1).setNameOrKey("polop"), paging).rules()).hasSize(0); + assertThat(profileRules.search(ProfileRuleQuery.create(1).setNameOrKey("polop"), paging).rules()).hasSize(0); } @Test public void find_profile_rules_with_inheritance() { Paging paging = Paging.create(10, 1); - List rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1), paging).rules(); + List rules = profileRules.search(ProfileRuleQuery.create(1), paging).rules(); assertThat(rules).hasSize(3); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setAnyInheritance(true), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setAnyInheritance(true), paging).rules(); assertThat(rules).hasSize(3); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setInheritance(QProfileRule.INHERITED), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setInheritance(QProfileRule.INHERITED), paging).rules(); assertThat(rules).hasSize(1); assertThat(rules.get(0).activeRuleId()).isEqualTo(391); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setInheritance(QProfileRule.OVERRIDES), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setInheritance(QProfileRule.OVERRIDES), paging).rules(); assertThat(rules).hasSize(1); assertThat(rules.get(0).activeRuleId()).isEqualTo(25); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setNoInheritance(true), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setNoInheritance(true), paging).rules(); assertThat(rules).hasSize(1); assertThat(rules.get(0).activeRuleId()).isEqualTo(2702); } @@ -153,13 +165,13 @@ public class ProfileRulesTest { public void find_profile_rules_sorted_by_name() { Paging paging = Paging.create(10, 1); - List rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(true), paging).rules(); + List rules = profileRules.search(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(true), paging).rules(); assertThat(rules).hasSize(3); assertThat(rules.get(0).name()).isEqualTo("Architectural constraint"); assertThat(rules.get(1).name()).isEqualTo("Internationalization - Consider using Locale parameterized version of invoked method"); assertThat(rules.get(2).name()).isEqualTo("Unused Null Check In Equals"); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(false), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(false), paging).rules(); assertThat(rules).hasSize(3); assertThat(rules.get(0).name()).isEqualTo("Unused Null Check In Equals"); assertThat(rules.get(1).name()).isEqualTo("Internationalization - Consider using Locale parameterized version of invoked method"); @@ -170,13 +182,13 @@ public class ProfileRulesTest { public void find_profile_rules_sorted_by_creation_date() { Paging paging = Paging.create(10, 1); - List rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(true), paging).rules(); + List rules = profileRules.search(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(true), paging).rules(); assertThat(rules).hasSize(3); assertThat(rules.get(0).key()).isEqualTo("DM_CONVERT_CASE"); assertThat(rules.get(1).key()).isEqualTo("UnusedNullCheckInEquals"); assertThat(rules.get(2).key()).isEqualTo("ArchitecturalConstraint"); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(false), paging).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(false), paging).rules(); assertThat(rules).hasSize(3); assertThat(rules.get(0).key()).isEqualTo("ArchitecturalConstraint"); assertThat(rules.get(1).key()).isEqualTo("UnusedNullCheckInEquals"); @@ -185,12 +197,12 @@ public class ProfileRulesTest { @Test public void find_profile_rules_with_paging() { - List rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1), Paging.create(2, 1)).rules(); + List rules = profileRules.search(ProfileRuleQuery.create(1), Paging.create(2, 1)).rules(); assertThat(rules).hasSize(2); assertThat(rules.get(0).key()).isEqualTo("ArchitecturalConstraint"); assertThat(rules.get(1).key()).isEqualTo("DM_CONVERT_CASE"); - rules = profileRules.searchProfileRules(ProfileRuleQuery.create(1), Paging.create(2, 2)).rules(); + rules = profileRules.search(ProfileRuleQuery.create(1), Paging.create(2, 2)).rules(); assertThat(rules).hasSize(1); assertThat(rules.get(0).key()).isEqualTo("UnusedNullCheckInEquals"); } @@ -229,28 +241,28 @@ public class ProfileRulesTest { Paging paging = Paging.create(10, 1); // Search of inactive rule on profile 2 - assertThat(profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(2), paging).rules()).hasSize(4); + assertThat(profileRules.searchInactives(ProfileRuleQuery.create(2), paging).rules()).hasSize(4); // Search of inactive rule on profile 1 - assertThat(profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(1), paging).rules()).hasSize(2); + assertThat(profileRules.searchInactives(ProfileRuleQuery.create(1), paging).rules()).hasSize(2); // Match on key - assertThat(profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(2).setNameOrKey("Boolean expressions"), paging).rules()).hasSize(1); + assertThat(profileRules.searchInactives(ProfileRuleQuery.create(2).setNameOrKey("Boolean expressions"), paging).rules()).hasSize(1); // Mach on severity - assertThat(profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(2).addSeverities(Severity.INFO), paging).rules()).hasSize(1); + assertThat(profileRules.searchInactives(ProfileRuleQuery.create(2).addSeverities(Severity.INFO), paging).rules()).hasSize(1); } @Test public void find_inactive_profile_rules_sorted_by_name() { Paging paging = Paging.create(10, 1); - List rules = profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(true), paging).rules(); + List rules = profileRules.searchInactives(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(true), paging).rules(); assertThat(rules).hasSize(2); assertThat(rules.get(0).name()).isEqualTo("Boolean expressions should not be compared to true or false"); assertThat(rules.get(1).name()).isEqualTo("Double Checked Locking"); - rules = profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(false), paging).rules(); + rules = profileRules.searchInactives(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_RULE_NAME).setAsc(false), paging).rules(); assertThat(rules).hasSize(2); assertThat(rules.get(0).name()).isEqualTo("Double Checked Locking"); assertThat(rules.get(1).name()).isEqualTo("Boolean expressions should not be compared to true or false"); @@ -260,12 +272,12 @@ public class ProfileRulesTest { public void find_inactive_profile_rules_sorted_by_creation_date() { Paging paging = Paging.create(10, 1); - List rules = profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(true), paging).rules(); + List rules = profileRules.searchInactives(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(true), paging).rules(); assertThat(rules).hasSize(2); assertThat(rules.get(0).key()).isEqualTo("com.puppycrawl.tools.checkstyle.checks.coding.DoubleCheckedLockingCheck"); assertThat(rules.get(1).key()).isEqualTo("S1125"); - rules = profileRules.searchInactiveProfileRules(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(false), paging).rules(); + rules = profileRules.searchInactives(ProfileRuleQuery.create(1).setSort(ProfileRuleQuery.SORT_BY_CREATION_DATE).setAsc(false), paging).rules(); assertThat(rules).hasSize(2); assertThat(rules.get(0).key()).isEqualTo("S1125"); assertThat(rules.get(1).key()).isEqualTo("com.puppycrawl.tools.checkstyle.checks.coding.DoubleCheckedLockingCheck"); @@ -303,12 +315,12 @@ public class ProfileRulesTest { @Test public void get_from_active_rule() { - assertThat(profileRules.getFromActiveRuleId(391)).isNotNull(); + assertThat(profileRules.findByActiveRuleId(391)).isNotNull(); } @Test public void get_from_rule() { - assertThat(profileRules.getFromActiveRuleId(25)).isNotNull(); + assertThat(profileRules.findByActiveRuleId(25)).isNotNull(); } private String testFileAsString(String testFile) throws Exception {