From 619c442f11667d89be065000ba1267391652414c Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 9 Jan 2014 20:40:55 +0100 Subject: [PATCH] SONAR-4923 Update profile parent use Java facade --- .../qualityprofile/db/QualityProfileDao.java | 6 +- .../org/sonar/api/rules/ActiveRuleParam.java | 3 +- .../server/configuration/ProfilesManager.java | 45 ++++---- .../QProfileActiveRuleOperations.java | 12 +-- .../qualityprofile/QProfileOperations.java | 55 +++++++++- .../server/qualityprofile/QProfiles.java | 43 +++++--- .../java/org/sonar/server/ui/JRubyFacade.java | 4 - .../app/controllers/profiles_controller.rb | 10 +- .../configuration/InheritedProfilesTest.java | 12 +-- .../server/configuration/RuleChangeTest.java | 2 +- .../QProfileActiveRuleOperationsTest.java | 36 ++++--- .../QProfileOperationsTest.java | 101 +++++++++++++++++- .../qualityprofile/QProfileSearchTest.java | 4 +- .../server/qualityprofile/QProfilesTest.java | 48 +++++++++ 14 files changed, 287 insertions(+), 94 deletions(-) diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java index 090283b56dc..d0f3469ef65 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDao.java @@ -52,10 +52,14 @@ public class QualityProfileDao implements ServerComponent { } } + public void update(QualityProfileDto dto, SqlSession session) { + session.getMapper(QualityProfileMapper.class).update(dto); + } + public void update(QualityProfileDto dto) { SqlSession session = mybatis.openSession(); try { - session.getMapper(QualityProfileMapper.class).update(dto); + update(dto, session); session.commit(); } finally { MyBatis.closeQuietly(session); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/rules/ActiveRuleParam.java b/sonar-plugin-api/src/main/java/org/sonar/api/rules/ActiveRuleParam.java index f634ce38110..41451e6c305 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/rules/ActiveRuleParam.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/rules/ActiveRuleParam.java @@ -138,7 +138,8 @@ public class ActiveRuleParam implements Cloneable { @Override public Object clone() { - return new ActiveRuleParam(getActiveRule(), getRuleParam(), getParamKey(), getValue()); + String paramKey = getParamKey() != null ? getParamKey() : getRuleParam().getKey(); + return new ActiveRuleParam(getActiveRule(), getRuleParam(), paramKey, getValue()); } } diff --git a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java index 50578dd53c9..ae5854e39d5 100644 --- a/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java +++ b/sonar-server/src/main/java/org/sonar/server/configuration/ProfilesManager.java @@ -25,12 +25,13 @@ import org.apache.commons.lang.StringUtils; import org.sonar.api.database.DatabaseSession; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rules.*; -import org.sonar.api.utils.ValidationMessages; import org.sonar.core.preview.PreviewCache; import org.sonar.jpa.dao.BaseDao; import org.sonar.jpa.dao.RulesDao; import org.sonar.server.qualityprofile.RuleInheritanceActions; +import javax.annotation.Nullable; + import java.util.List; public class ProfilesManager extends BaseDao { @@ -67,36 +68,30 @@ public class ProfilesManager extends BaseDao { dryRunCache.reportGlobalModification(); } + // Managing inheritance of profiles - public ValidationMessages changeParentProfile(Integer profileId, String parentName, String userName) { - ValidationMessages messages = ValidationMessages.create(); + public RuleInheritanceActions profileParentChanged(Integer profileId, @Nullable String parentName, String userName) { RulesProfile profile = getSession().getEntity(RulesProfile.class, profileId); - if (profile != null) { - RulesProfile oldParent = getParentProfile(profile); - RulesProfile newParent = getProfile(profile.getLanguage(), parentName); - if (isCycle(profile, newParent)) { - messages.addWarningText("Please do not select a child profile as parent."); - return messages; - } - // Deactivate all inherited rules - if (oldParent != null) { - for (ActiveRule activeRule : oldParent.getActiveRules()) { - deactivate(profile, activeRule.getRule(), userName); - } + RulesProfile oldParent = getParentProfile(profile); + RulesProfile newParent = getProfile(profile.getLanguage(), parentName); + + RuleInheritanceActions actions = new RuleInheritanceActions(); + // Deactivate all inherited rules from old parent + if (oldParent != null) { + for (ActiveRule activeRule : oldParent.getActiveRules()) { + actions.add(deactivate(profile, activeRule.getRule(), userName)); } - // Activate all inherited rules - if (newParent != null) { - for (ActiveRule activeRule : newParent.getActiveRules()) { - activateOrChange(profile, activeRule, userName); - } + } + // Activate all inherited rules of new parent + if (newParent != null) { + for (ActiveRule activeRule : newParent.getActiveRules()) { + actions.add(activateOrChange(profile, activeRule, userName)); } - profile.setParentName(newParent == null ? null : newParent.getName()); - getSession().saveWithoutFlush(profile); - getSession().commit(); - dryRunCache.reportGlobalModification(); } - return messages; + getSession().commit(); + dryRunCache.reportGlobalModification(); + return actions; } /** 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 eabf11c8aac..6db8bb0bd3c 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 @@ -22,8 +22,6 @@ package org.sonar.server.qualityprofile; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.Multimap; import org.apache.commons.lang.math.NumberUtils; import org.apache.ibatis.session.SqlSession; import org.elasticsearch.common.base.Predicate; @@ -234,7 +232,8 @@ public class QProfileActiveRuleOperations implements ServerComponent { throw new IllegalStateException("Can't find parent of active rule : " + activeRule.getId()); } - private List restoreActiveParametersFromActiveRuleParent(ActiveRuleDto activeRule, ActiveRuleDto parent, RuleInheritanceActions actions, UserSession userSession, SqlSession session) { + private List restoreActiveParametersFromActiveRuleParent(ActiveRuleDto activeRule, ActiveRuleDto parent, RuleInheritanceActions actions, + UserSession userSession, SqlSession session) { // Restore all parameters from parent List parentParams = activeRuleDao.selectParamsByActiveRuleId(parent.getId(), session); List activeRuleParams = activeRuleDao.selectParamsByActiveRuleId(activeRule.getId(), session); @@ -345,12 +344,7 @@ public class QProfileActiveRuleOperations implements ServerComponent { private void reindexInheritanceResult(RuleInheritanceActions actions, SqlSession session) { ruleRegistry.deleteActiveRules(actions.idsToDelete()); - List activeRules = activeRuleDao.selectByIds(actions.idsToIndex(), session); - Multimap paramsByActiveRule = ArrayListMultimap.create(); - for (ActiveRuleParamDto param : activeRuleDao.selectParamsByActiveRuleIds(actions.idsToIndex(), session)) { - paramsByActiveRule.put(param.getActiveRuleId(), param); - } - ruleRegistry.bulkIndexActiveRules(activeRules, paramsByActiveRule); + ruleRegistry.bulkIndexActiveRules(actions.idsToIndex(), session); } private void reindexActiveRule(ActiveRuleDto activeRuleDto, SqlSession session) { 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 9f3680d017e..6943949b5aa 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 @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Multimap; @@ -38,10 +39,14 @@ import org.sonar.core.preview.PreviewCache; import org.sonar.core.properties.PropertiesDao; 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.rule.RuleRegistry; import org.sonar.server.user.UserSession; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + import java.io.StringReader; import java.util.List; import java.util.Map; @@ -59,17 +64,18 @@ public class QProfileOperations implements ServerComponent { private final List importers; private final PreviewCache dryRunCache; private final RuleRegistry ruleRegistry; + private final ProfilesManager profilesManager; /** * Used by pico when no plugin provide profile exporter / importer */ public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, PropertiesDao propertiesDao, - PreviewCache dryRunCache, RuleRegistry ruleRegistry) { - this(myBatis, dao, activeRuleDao, propertiesDao, Lists.newArrayList(), dryRunCache, ruleRegistry); + PreviewCache dryRunCache, RuleRegistry ruleRegistry, ProfilesManager profilesManager) { + this(myBatis, dao, activeRuleDao, propertiesDao, Lists.newArrayList(), dryRunCache, ruleRegistry, profilesManager); } public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, PropertiesDao propertiesDao, - List importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry) { + List importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry, ProfilesManager profilesManager) { this.myBatis = myBatis; this.dao = dao; this.activeRuleDao = activeRuleDao; @@ -77,6 +83,7 @@ public class QProfileOperations implements ServerComponent { this.importers = importers; this.dryRunCache = dryRunCache; this.ruleRegistry = ruleRegistry; + this.profilesManager = profilesManager; } public NewProfileResult newProfile(String name, String language, Map xmlProfilesByPlugin, UserSession userSession) { @@ -112,6 +119,48 @@ public class QProfileOperations implements ServerComponent { propertiesDao.setProperty(new PropertyDto().setKey(PROPERTY_PREFIX + qualityProfile.getLanguage()).setValue(qualityProfile.getName())); } + public void updateParentProfile(QualityProfileDto profile, @Nullable QualityProfileDto parentProfile, UserSession userSession) { + checkPermission(userSession); + + SqlSession session = myBatis.openSession(); + try { + if (isCycle(profile, parentProfile, session)) { + throw new BadRequestException("Please do not select a child profile as parent."); + } + String parentName = parentProfile != null ? parentProfile.getName() : null; + + RuleInheritanceActions actions = profilesManager.profileParentChanged(profile.getId(), parentName, userSession.name()); + ruleRegistry.deleteActiveRules(actions.idsToDelete()); + ruleRegistry.bulkIndexActiveRules(actions.idsToIndex(), session); + + profile.setParent(parentName); + dao.update(profile, session); + session.commit(); + + } finally { + MyBatis.closeQuietly(session); + } + } + + @VisibleForTesting + boolean isCycle(QualityProfileDto childProfile, QualityProfileDto parentProfile, SqlSession session) { + while (parentProfile != null) { + if (childProfile.getName().equals(parentProfile.getName())) { + return true; + } + parentProfile = getParent(parentProfile, session); + } + return false; + } + + @CheckForNull + private QualityProfileDto getParent(QualityProfileDto profile, SqlSession session) { + if (profile.getParent() != null) { + return dao.selectParent(profile.getId(), session); + } + return null; + } + private List readProfilesFromXml(NewProfileResult result, Map xmlProfilesByPlugin) { List profiles = newArrayList(); ValidationMessages messages = ValidationMessages.create(); 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 5b5453e7de3..49bac9b44f3 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 @@ -113,23 +113,6 @@ public class QProfiles implements ServerComponent { return search.defaultProfile(language); } - @CheckForNull - public QProfile parent(QProfile profile) { - QualityProfileDto parent = findQualityProfile(profile.parent(), profile.language()); - if (parent != null) { - return QProfile.from(parent); - } - return null; - } - - public List children(QProfile profile) { - return search.children(profile); - } - - public List ancestors(QProfile profile) { - return search.ancestors(profile); - } - public int countChildren(QProfile profile) { return search.countChildren(profile); } @@ -149,6 +132,32 @@ public class QProfiles implements ServerComponent { operations.setDefaultProfile(qualityProfile, UserSession.get()); } + @CheckForNull + public QProfile parent(QProfile profile) { + QualityProfileDto parent = findQualityProfile(profile.parent(), profile.language()); + if (parent != null) { + return QProfile.from(parent); + } + return null; + } + + public List children(QProfile profile) { + return search.children(profile); + } + + public List ancestors(QProfile profile) { + return search.ancestors(profile); + } + + public void updateParentProfile(int profileId, @Nullable String parentName) { + QualityProfileDto profile = findNotNull(profileId); + QualityProfileDto parent = null; + if (!Strings.isNullOrEmpty(parentName)) { + parent = findNotNull(parentName, profile.getLanguage()); + } + operations.updateParentProfile(profile, parent, UserSession.get()); + } + /** * Used by WS */ diff --git a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java index 7d5c67751c5..0ba0f84edee 100644 --- a/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java +++ b/sonar-server/src/main/java/org/sonar/server/ui/JRubyFacade.java @@ -288,10 +288,6 @@ public final class JRubyFacade { getProfilesManager().copyProfile((int) profileId, newProfileName); } - public ValidationMessages changeParentProfile(int profileId, String parentName, String userName) { - return getProfilesManager().changeParentProfile(profileId, parentName, userName); - } - public void ruleActivated(int parentProfileId, int activeRuleId, String userName) { getProfilesManager().activated(parentProfileId, activeRuleId, userName); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb index cab5e63475a..da9003e0bc0 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/profiles_controller.rb @@ -237,7 +237,6 @@ class ProfilesController < ApplicationController set_profile_breadcrumbs end - # POST /profiles/change_parent?id=&parent_name= def change_parent verify_post_request @@ -245,17 +244,12 @@ class ProfilesController < ApplicationController require_parameters 'id' id = params[:id].to_i - parent_name = params[:parent_name] - if parent_name.blank? - messages = java_facade.changeParentProfile(id, nil, current_user.name) - else - messages = java_facade.changeParentProfile(id, parent_name, current_user.name) + call_backend do + Internal.quality_profiles.updateParentProfile(id, params[:parent_name]) end - flash_messages(messages) redirect_to :action => 'inheritance', :id => id end - # # # GET /profiles/permalinks?id= diff --git a/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java b/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java index 633f51ca5d5..a235e50ce0a 100644 --- a/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/configuration/InheritedProfilesTest.java @@ -60,22 +60,22 @@ public class InheritedProfilesTest extends AbstractDbUnitTestCase { @Test public void shouldSetParent() { setupData("shouldSetParent"); - profilesManager.changeParentProfile(2, "parent", "admin"); - checkTables("shouldSetParent", "active_rules", "rules_profiles"); + profilesManager.profileParentChanged(2, "parent", "admin"); + checkTables("shouldSetParent", "active_rules"); } @Test public void shouldChangeParent() { setupData("shouldChangeParent"); - profilesManager.changeParentProfile(3, "new_parent", "admin"); - checkTables("shouldChangeParent", "active_rules", "rules_profiles"); + profilesManager.profileParentChanged(3, "new_parent", "admin"); + checkTables("shouldChangeParent", "active_rules"); } @Test public void shouldRemoveParent() { setupData("shouldRemoveParent"); - profilesManager.changeParentProfile(2, null, "admin"); - checkTables("shouldRemoveParent", "active_rules", "rules_profiles"); + profilesManager.profileParentChanged(2, null, "admin"); + checkTables("shouldRemoveParent", "active_rules"); } @Test diff --git a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java index ef568bd94d5..23da77803a7 100644 --- a/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java +++ b/sonar-server/src/test/java/org/sonar/server/configuration/RuleChangeTest.java @@ -119,7 +119,7 @@ public class RuleChangeTest extends AbstractDbUnitTestCase { @Test public void should_track_change_parent_profile() { setupData("changeParentProfile"); - profilesManager.changeParentProfile(2, "parent", "admin"); + profilesManager.profileParentChanged(2, "parent", "admin"); checkTables("changeParentProfile", new String[] {"change_date"}, "active_rule_changes"); } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java index 0d3d0b4e4c5..d53224f6b22 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileActiveRuleOperationsTest.java @@ -20,8 +20,6 @@ package org.sonar.server.qualityprofile; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Multimap; import org.apache.ibatis.session.SqlSession; import org.junit.Before; import org.junit.Test; @@ -61,9 +59,7 @@ import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyList; import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.isA; import static org.mockito.Mockito.anyListOf; import static org.mockito.Mockito.*; @@ -141,8 +137,6 @@ public class QProfileActiveRuleOperationsTest { .addToIndex(idActiveRuleToUpdate) .addToDelete(idActiveRuleToDelete); when(profilesManager.activated(eq(1), anyInt(), eq("Nicolas"))).thenReturn(inheritanceActions); - when(activeRuleDao.selectByIds(anyList(), isA(SqlSession.class))).thenReturn(ImmutableList.of(mock(ActiveRuleDto.class))); - when(activeRuleDao.selectParamsByActiveRuleIds(anyList(), isA(SqlSession.class))).thenReturn(ImmutableList.of(mock(ActiveRuleParamDto.class))); ActiveRuleDto result = operations.createActiveRule(qualityProfile, rule, Severity.CRITICAL, authorizedUserSession); assertThat(result).isNotNull(); @@ -159,7 +153,8 @@ public class QProfileActiveRuleOperationsTest { verify(session).commit(); verify(profilesManager).activated(eq(1), anyInt(), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(eq(newArrayList(idActiveRuleToDelete))); + verify(ruleRegistry).bulkIndexActiveRules(eq(newArrayList(idActiveRuleToUpdate)), eq(session)); } @Test @@ -174,7 +169,8 @@ public class QProfileActiveRuleOperationsTest { verify(activeRuleDao).update(eq(activeRule), eq(session)); verify(session).commit(); verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } @Test @@ -205,7 +201,8 @@ public class QProfileActiveRuleOperationsTest { verify(activeRuleDao).deleteParameters(eq(5), eq(session)); verify(session).commit(); verify(profilesManager).deactivated(eq(1), anyInt(), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } @Test @@ -225,7 +222,8 @@ public class QProfileActiveRuleOperationsTest { verify(session).commit(); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } @Test @@ -277,7 +275,8 @@ public class QProfileActiveRuleOperationsTest { verify(session).commit(); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("30"), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } @Test @@ -291,7 +290,8 @@ public class QProfileActiveRuleOperationsTest { verify(session).commit(); verify(activeRuleDao).deleteParameter(100, session); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq((String) null), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); } @Test @@ -312,7 +312,8 @@ public class QProfileActiveRuleOperationsTest { verify(session, times(2)).commit(); verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class)); } @@ -358,7 +359,8 @@ public class QProfileActiveRuleOperationsTest { verify(session, times(2)).commit(); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("15"), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class)); } @@ -384,7 +386,8 @@ public class QProfileActiveRuleOperationsTest { verify(session, times(2)).commit(); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("format"), eq("abc"), eq((String) null), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class)); } @@ -413,7 +416,8 @@ public class QProfileActiveRuleOperationsTest { verify(session, times(2)).commit(); verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("minimum"), eq((String) null), eq("2"), eq("Nicolas")); - verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); verify(ruleRegistry).save(eq(activeRule), anyListOf(ActiveRuleParamDto.class)); } 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 c9800d2e232..625f87d360a 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 @@ -43,6 +43,7 @@ import org.sonar.core.preview.PreviewCache; import org.sonar.core.properties.PropertiesDao; 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.ForbiddenException; import org.sonar.server.rule.RuleRegistry; @@ -60,6 +61,8 @@ import static org.fest.assertions.Fail.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyListOf; import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.*; @RunWith(MockitoJUnitRunner.class) @@ -86,6 +89,9 @@ public class QProfileOperationsTest { @Mock RuleRegistry ruleRegistry; + @Mock + ProfilesManager profilesManager; + List importers = newArrayList(); Integer currentId = 1; @@ -109,7 +115,7 @@ public class QProfileOperationsTest { } }).when(activeRuleDao).insert(any(ActiveRuleDto.class), any(SqlSession.class)); - operations = new QProfileOperations(myBatis, qualityProfileDao, activeRuleDao, propertiesDao, importers, dryRunCache, ruleRegistry); + operations = new QProfileOperations(myBatis, qualityProfileDao, activeRuleDao, propertiesDao, importers, dryRunCache, ruleRegistry, profilesManager); } @Test @@ -227,4 +233,97 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getValue()).isEqualTo("My profile"); } + @Test + public void update_parent_profile() { + QualityProfileDto child = new QualityProfileDto().setId(1).setName("Child").setLanguage("java").setParent("Old Parent"); + QualityProfileDto oldParent = new QualityProfileDto().setId(2).setName("Old Parent").setLanguage("java"); + QualityProfileDto newParent = new QualityProfileDto().setId(3).setName("Parent").setLanguage("java"); + + when(qualityProfileDao.selectParent(2, session)).thenReturn(oldParent); + when(profilesManager.profileParentChanged(anyInt(), anyString(), anyString())).thenReturn(new RuleInheritanceActions()); + + operations.updateParentProfile(child, newParent, authorizedUserSession); + ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); + verify(qualityProfileDao).update(profileArgument.capture(), eq(session)); + assertThat(profileArgument.getValue().getParent()).isEqualTo("Parent"); + assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java"); + + verify(profilesManager).profileParentChanged(1, "Parent", "Nicolas"); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); + } + + @Test + public void set_parent_profile() { + QualityProfileDto child = new QualityProfileDto().setId(1).setName("Child").setLanguage("java").setParent(null); + QualityProfileDto parent = new QualityProfileDto().setId(2).setName("Parent").setLanguage("java"); + + when(profilesManager.profileParentChanged(anyInt(), anyString(), anyString())).thenReturn(new RuleInheritanceActions()); + + operations.updateParentProfile(child, parent, authorizedUserSession); + + ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); + verify(qualityProfileDao).update(profileArgument.capture(), eq(session)); + assertThat(profileArgument.getValue().getParent()).isEqualTo("Parent"); + assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java"); + + verify(profilesManager).profileParentChanged(1, "Parent", "Nicolas"); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); + } + + @Test + public void remove_parent_profile() { + QualityProfileDto child = new QualityProfileDto().setId(1).setName("Child").setLanguage("java").setParent("Old Parent"); + QualityProfileDto parent = new QualityProfileDto().setId(2).setName("Old Parent").setLanguage("java"); + + when(qualityProfileDao.selectParent(2, session)).thenReturn(parent); + when(profilesManager.profileParentChanged(anyInt(), anyString(), anyString())).thenReturn(new RuleInheritanceActions()); + + operations.updateParentProfile(child, null, authorizedUserSession); + + ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); + verify(qualityProfileDao).update(profileArgument.capture(), eq(session)); + assertThat(profileArgument.getValue().getParent()).isNull(); + assertThat(profileArgument.getValue().getLanguage()).isEqualTo("java"); + + verify(profilesManager).profileParentChanged(1, null, "Nicolas"); + verify(ruleRegistry).deleteActiveRules(anyListOf(Integer.class)); + verify(ruleRegistry).bulkIndexActiveRules(anyListOf(Integer.class), eq(session)); + } + + @Test + public void fail_to_update_parent_on_cycle() { + QualityProfileDto child = new QualityProfileDto().setId(1).setName("Child").setLanguage("java").setParent("parent"); + QualityProfileDto parent = new QualityProfileDto().setId(2).setName("Parent").setLanguage("java"); + + when(qualityProfileDao.selectParent(1, session)).thenReturn(parent); + when(qualityProfileDao.selectParent(2, session)).thenReturn(null); + + try { + operations.updateParentProfile(parent, child, authorizedUserSession); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(BadRequestException.class).hasMessage("Please do not select a child profile as parent."); + } + } + + @Test + public void detect_cycle() { + QualityProfileDto level1 = new QualityProfileDto().setId(1).setName("level1").setLanguage("java"); + QualityProfileDto level2 = new QualityProfileDto().setId(2).setName("level2").setLanguage("java").setParent("level1"); + QualityProfileDto level3 = new QualityProfileDto().setId(3).setName("level3").setLanguage("java").setParent("level2"); + + when(qualityProfileDao.selectParent(1, session)).thenReturn(null); + when(qualityProfileDao.selectParent(2, session)).thenReturn(level1); + when(qualityProfileDao.selectParent(3, session)).thenReturn(level2); + + assertThat(operations.isCycle(level3, level1, session)).isFalse(); + assertThat(operations.isCycle(level1, level1, session)).isTrue(); + assertThat(operations.isCycle(level1, level1, session)).isTrue(); + assertThat(operations.isCycle(level1, level3, session)).isTrue(); + assertThat(operations.isCycle(level1, level2, session)).isTrue(); + assertThat(operations.isCycle(level2, level3, session)).isTrue(); + } + } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileSearchTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileSearchTest.java index 4b9093a5472..0fd40510e62 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileSearchTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileSearchTest.java @@ -112,9 +112,9 @@ public class QProfileSearchTest { @Test public void search_ancestors() throws Exception { - when(dao.selectParent(eq(3), eq(session))).thenReturn(new QualityProfileDto().setId(2).setName("Child").setLanguage("java").setParent("Parent")); - when(dao.selectParent(eq(2), eq(session))).thenReturn(new QualityProfileDto().setId(1).setName("Parent").setLanguage("java")); when(dao.selectParent(eq(1), eq(session))).thenReturn(null); + when(dao.selectParent(eq(2), eq(session))).thenReturn(new QualityProfileDto().setId(1).setName("Parent").setLanguage("java")); + when(dao.selectParent(eq(3), eq(session))).thenReturn(new QualityProfileDto().setId(2).setName("Child").setLanguage("java").setParent("Parent")); List result = search.ancestors(new QProfile().setId(3).setName("Grandchild").setLanguage("java").setParent("Child")); assertThat(result).hasSize(2); 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 7e5f79896b0..c13f0b1bdda 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 @@ -241,6 +241,54 @@ public class QProfilesTest { verify(service).setDefaultProfile(eq(qualityProfile), any(UserSession.class)); } + @Test + public void update_parent_profile() throws Exception { + QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("Default").setLanguage("java"); + when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); + QualityProfileDto parent = new QualityProfileDto().setId(2).setName("Parent").setLanguage("java"); + when(qualityProfileDao.selectByNameAndLanguage("Parent", "java")).thenReturn(parent); + + qProfiles.updateParentProfile(1, "Parent"); + verify(service).updateParentProfile(eq(qualityProfile), eq(parent), any(UserSession.class)); + } + + @Test + public void remove_parent_profile() throws Exception { + QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("Default").setLanguage("java"); + when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); + + qProfiles.updateParentProfile(1, null); + verify(service).updateParentProfile(eq(qualityProfile), eq((QualityProfileDto) null), any(UserSession.class)); + } + + @Test + public void fail_to_update_parent_profile_on_unknown_profile() throws Exception { + try { + when(qualityProfileDao.selectById(1)).thenReturn(null); + QualityProfileDto parent = new QualityProfileDto().setId(2).setName("Parent").setLanguage("java"); + when(qualityProfileDao.selectByNameAndLanguage("Parent", "java")).thenReturn(parent); + + qProfiles.updateParentProfile(1, "Parent"); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class); + } + } + + @Test + public void fail_to_update_parent_profile_on_unknown_parent_profile() throws Exception { + try { + QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("Default").setLanguage("java"); + when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); + when(qualityProfileDao.selectByNameAndLanguage("Parent", "java")).thenReturn(null); + + qProfiles.updateParentProfile(1, "Parent"); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(NotFoundException.class); + } + } + @Test public void projects() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); -- 2.39.5