From: Julien Lancelot Date: Mon, 23 Dec 2013 11:21:14 +0000 (+0100) Subject: SONAR-4535 Reindex active rule note update X-Git-Tag: 4.2~861 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=676932e588e925e22da9f5b83313607258f392df;p=sonarqube.git SONAR-4535 Reindex active rule note update --- diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDao.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDao.java index 2b8b69feed3..1ab38d9a113 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDao.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDao.java @@ -204,23 +204,36 @@ public class ActiveRuleDao implements ServerComponent { return dtosList; } - public List selectParamsByRuleIds(List ids) { + public List selectParamsByActiveRuleId(Integer activeRuleId) { SqlSession session = mybatis.openSession(); try { - return selectParamsByRuleIds(ids, session); + return selectParamsByActiveRuleId(activeRuleId, session); } finally { MyBatis.closeQuietly(session); } } - public List selectParamsByRuleIds(Collection ids, SqlSession session) { - if (ids.isEmpty()) { + public List selectParamsByActiveRuleId(Integer activeRuleId, SqlSession session) { + return session.getMapper(ActiveRuleMapper.class).selectParamsByActiveRuleId(activeRuleId); + } + + public List selectParamsByActiveRuleIds(List activeRuleIds) { + SqlSession session = mybatis.openSession(); + try { + return selectParamsByActiveRuleIds(activeRuleIds, session); + } finally { + MyBatis.closeQuietly(session); + } + } + + public List selectParamsByActiveRuleIds(Collection activeRuleIds, SqlSession session) { + if (activeRuleIds.isEmpty()) { return Collections.emptyList(); } List dtosList = newArrayList(); - List> idsPartitionList = Lists.partition(newArrayList(ids), 1000); + List> idsPartitionList = Lists.partition(newArrayList(activeRuleIds), 1000); for (List idsPartition : idsPartitionList) { - List dtos = session.selectList("org.sonar.core.qualityprofile.db.ActiveRuleMapper.selectParamsByRuleIds", newArrayList(idsPartition)); + List dtos = session.selectList("org.sonar.core.qualityprofile.db.ActiveRuleMapper.selectParamsByActiveRuleIds", newArrayList(idsPartition)); dtosList.addAll(dtos); } return dtosList; diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleMapper.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleMapper.java index 34050e6da9e..dd142cd99bc 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleMapper.java @@ -24,6 +24,8 @@ import org.apache.ibatis.annotations.Param; import javax.annotation.CheckForNull; +import java.util.List; + public interface ActiveRuleMapper { @CheckForNull @@ -32,6 +34,8 @@ public interface ActiveRuleMapper { @CheckForNull ActiveRuleDto selectByProfileAndRule(@Param("profileId") Integer profileId, @Param("ruleId") Integer ruleId); + List selectParamsByActiveRuleId(Integer activeRuleId); + @CheckForNull ActiveRuleParamDto selectParamById(Integer activeRuleParamId); diff --git a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml index ea158ba518f..66346a0f28e 100644 --- a/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/qualityprofile/db/ActiveRuleMapper.xml @@ -106,22 +106,29 @@ from active_rules a - and a.id in - - #{id} - + ( + a.id=#{id} + ) - select from active_rule_parameters p - and p.active_rule_id in - - #{id} - + p.active_rule_id=#{id} + + + + diff --git a/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest.java b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest.java index abb6e9c8450..b1b9ca7b7d8 100644 --- a/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleDaoTest.java @@ -90,6 +90,14 @@ public class ActiveRuleDaoTest extends AbstractDaoTestCase { assertThat(result.getValue()).isEqualTo("20"); } + @Test + public void select_params_by_active_rule_id() { + setupData("shared"); + + assertThat(dao.selectParamsByActiveRuleId(1)).hasSize(2); + assertThat(dao.selectParamsByActiveRuleId(2)).hasSize(1); + } + @Test public void select_param_by_active_rule_and_key() { setupData("shared"); @@ -106,9 +114,9 @@ public class ActiveRuleDaoTest extends AbstractDaoTestCase { public void select_params_by_active_rule_ids() { setupData("shared"); - assertThat(dao.selectParamsByRuleIds(ImmutableList.of(1))).hasSize(2); - assertThat(dao.selectParamsByRuleIds(ImmutableList.of(2))).hasSize(1); - assertThat(dao.selectParamsByRuleIds(ImmutableList.of(1, 2))).hasSize(3); + assertThat(dao.selectParamsByActiveRuleIds(ImmutableList.of(1))).hasSize(2); + assertThat(dao.selectParamsByActiveRuleIds(ImmutableList.of(2))).hasSize(1); + assertThat(dao.selectParamsByActiveRuleIds(ImmutableList.of(1, 2))).hasSize(3); } @Test diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChanged.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChanged.java new file mode 100644 index 00000000000..7dbd234b165 --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChanged.java @@ -0,0 +1,41 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.qualityprofile; + + +public class ActiveRuleChanged { + + private QProfile profile; + private QProfileRule rule; + + public ActiveRuleChanged(QProfile profile, QProfileRule rule) { + this.profile = profile; + this.rule = rule; + } + + public QProfile profile() { + return profile; + } + + public QProfileRule rule() { + return rule; + } +} 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 a501046efa7..91c795870b2 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 @@ -33,7 +33,6 @@ import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rule.Severity; import org.sonar.api.rules.ActiveRule; import org.sonar.api.rules.ActiveRuleParam; -import org.sonar.api.rules.Rule; import org.sonar.api.rules.RulePriority; import org.sonar.api.utils.System2; import org.sonar.api.utils.ValidationMessages; @@ -96,8 +95,8 @@ public class QProfileOperations implements ServerComponent { @VisibleForTesting QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao, - List importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry, - ProfilesManager profilesManager, ProfileRules profileRules, System2 system) { + List importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry, + ProfilesManager profilesManager, ProfileRules profileRules, System2 system) { this.myBatis = myBatis; this.dao = dao; this.activeRuleDao = activeRuleDao; @@ -144,7 +143,7 @@ public class QProfileOperations implements ServerComponent { propertiesDao.setProperty(new PropertyDto().setKey(PROPERTY_PREFIX + qualityProfile.getLanguage()).setValue(qualityProfile.getName())); } - public RuleActivationResult createActiveRule(QualityProfileDto qualityProfile, Rule rule, String severity, UserSession userSession) { + public ActiveRuleDto createActiveRule(QualityProfileDto qualityProfile, RuleDto rule, String severity, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); try { @@ -170,13 +169,13 @@ public class QProfileOperations implements ServerComponent { RuleInheritanceActions actions = profilesManager.activated(qualityProfile.getId(), activeRule.getId(), userSession.name()); reindexInheritanceResult(actions, session); - return new RuleActivationResult(QProfile.from(qualityProfile), profileRules.getFromActiveRuleId(activeRule.getId())); + return activeRule; } finally { MyBatis.closeQuietly(session); } } - public RuleActivationResult updateSeverity(QualityProfileDto qualityProfile, ActiveRuleDto activeRule, String newSeverity, UserSession userSession) { + public void updateSeverity(QualityProfileDto qualityProfile, ActiveRuleDto activeRule, String newSeverity, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); try { @@ -185,21 +184,20 @@ public class QProfileOperations implements ServerComponent { activeRuleDao.update(activeRule, session); session.commit(); - RuleInheritanceActions actions = profilesManager.ruleSeverityChanged(activeRule.getProfileId(), activeRule.getId(), RulePriority.valueOfInt(oldSeverity), RulePriority.valueOf(newSeverity), + RuleInheritanceActions actions = profilesManager.ruleSeverityChanged(activeRule.getProfileId(), activeRule.getId(), + RulePriority.valueOfInt(oldSeverity), RulePriority.valueOf(newSeverity), userSession.name()); reindexInheritanceResult(actions, session); - return new RuleActivationResult(QProfile.from(qualityProfile), profileRules.getFromActiveRuleId(activeRule.getId())); } finally { MyBatis.closeQuietly(session); } } - public RuleActivationResult deactivateRule(QualityProfileDto qualityProfile, Rule rule, UserSession userSession) { + public void deactivateRule(ActiveRuleDto activeRule, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); try { - ActiveRuleDto activeRule = validate(qualityProfile, rule); RuleInheritanceActions actions = profilesManager.deactivated(activeRule.getProfileId(), activeRule.getId(), userSession.name()); activeRuleDao.delete(activeRule.getId(), session); @@ -208,14 +206,12 @@ public class QProfileOperations implements ServerComponent { session.commit(); reindexInheritanceResult(actions, session); - - return new RuleActivationResult(QProfile.from(qualityProfile), profileRules.getFromRuleId(rule.getId())); } finally { MyBatis.closeQuietly(session); } } - public QProfileRule createActiveRuleParam(ActiveRuleDto activeRule, String key, String value, UserSession userSession) { + public void createActiveRuleParam(ActiveRuleDto activeRule, String key, String value, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); @@ -227,32 +223,31 @@ public class QProfileOperations implements ServerComponent { ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setActiveRuleId(activeRule.getId()).setKey(key).setValue(value).setRulesParameterId(ruleParam.getId()); activeRuleDao.insert(activeRuleParam, session); session.commit(); + RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), key, null, value, userSession.name()); reindexInheritanceResult(actions, session); - - return profileRules.getFromActiveRuleId(activeRule.getId()); } finally { MyBatis.closeQuietly(session); } } - public QProfileRule deleteActiveRuleParam(ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam, UserSession userSession) { + public void deleteActiveRuleParam(ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); try { activeRuleDao.deleteParameter(activeRuleParam.getId(), session); session.commit(); - RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), activeRuleParam.getValue(), null, userSession.name()); - reindexInheritanceResult(actions, session); - return profileRules.getFromActiveRuleId(activeRule.getId()); + RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), activeRuleParam.getValue(), + null, userSession.name()); + reindexInheritanceResult(actions, session); } finally { MyBatis.closeQuietly(session); } } - public QProfileRule updateActiveRuleParam(ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam, String value, UserSession userSession) { + public void updateActiveRuleParam(ActiveRuleDto activeRule, ActiveRuleParamDto activeRuleParam, String value, UserSession userSession) { checkPermission(userSession); SqlSession session = myBatis.openSession(); @@ -262,10 +257,10 @@ public class QProfileOperations implements ServerComponent { activeRuleParam.setValue(sanitizedValue); activeRuleDao.update(activeRuleParam, session); session.commit(); - RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), oldValue, sanitizedValue, userSession.name()); - reindexInheritanceResult(actions, session); - return profileRules.getFromActiveRuleId(activeRule.getId()); + RuleInheritanceActions actions = profilesManager.ruleParamChanged(activeRule.getProfileId(), activeRule.getId(), activeRuleParam.getKey(), oldValue, + sanitizedValue, getLoggedName(userSession)); + reindexInheritanceResult(actions, session); } finally { MyBatis.closeQuietly(session); } @@ -274,63 +269,99 @@ public class QProfileOperations implements ServerComponent { public void updateActiveRuleNote(ActiveRuleDto activeRule, String note, UserSession userSession) { checkPermission(userSession); Date now = new Date(system.now()); + SqlSession session = myBatis.openSession(); + try { + if (activeRule.getNoteData() == null) { + activeRule.setNoteCreatedAt(now); + activeRule.setNoteUserLogin(userSession.login()); + } + activeRule.setNoteUpdatedAt(now); + activeRule.setNoteData(note); + activeRuleDao.update(activeRule, session); + session.commit(); - if (activeRule.getNoteData() == null) { - activeRule.setNoteCreatedAt(now); - activeRule.setNoteUserLogin(userSession.login()); - } - activeRule.setNoteUpdatedAt(now); - activeRule.setNoteData(note); - activeRuleDao.update(activeRule); - // TODO notify E/S of active rule change - } - - 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.selectParamsByRuleIds(actions.idsToIndex(), session)) { - paramsByActiveRule.put(param.getActiveRuleId(), param); + reindexActiveRule(activeRule, session); + } finally { + MyBatis.closeQuietly(session); } - ruleRegistry.bulkIndexActiveRules(activeRules, paramsByActiveRule); } public void deleteActiveRuleNote(ActiveRuleDto activeRule, UserSession userSession) { checkPermission(userSession); - activeRule.setNoteUpdatedAt(new Date(system.now())); - activeRule.setNoteData(null); - activeRule.setNoteUserLogin(null); - activeRule.setNoteCreatedAt(null); - activeRule.setNoteUpdatedAt(null); - activeRuleDao.update(activeRule); - // TODO notify E/S of active rule change + SqlSession session = myBatis.openSession(); + try { + activeRule.setNoteUpdatedAt(new Date(system.now())); + activeRule.setNoteData(null); + activeRule.setNoteUserLogin(null); + activeRule.setNoteCreatedAt(null); + activeRule.setNoteUpdatedAt(null); + activeRuleDao.update(activeRule); + session.commit(); + + reindexActiveRule(activeRule, session); + } finally { + MyBatis.closeQuietly(session); + } } - public void updateRuleNote(RuleDto rule, String note, UserSession userSession) { + public void updateRuleNote(ActiveRuleDto activeRule, RuleDto rule, String note, UserSession userSession) { checkPermission(userSession); Date now = new Date(system.now()); - if (rule.getNoteData() == null) { - rule.setNoteCreatedAt(now); - rule.setNoteUserLogin(userSession.login()); + SqlSession session = myBatis.openSession(); + try { + if (rule.getNoteData() == null) { + rule.setNoteCreatedAt(now); + rule.setNoteUserLogin(userSession.login()); + } + rule.setNoteUpdatedAt(now); + rule.setNoteData(note); + ruleDao.update(rule); + session.commit(); + + // TODO notify E/S of rule change + } finally { + MyBatis.closeQuietly(session); } - rule.setNoteUpdatedAt(now); - rule.setNoteData(note); - ruleDao.update(rule); - // TODO notify E/S of active rule change } - public void deleteRuleNote(RuleDto rule, UserSession userSession) { + public void deleteRuleNote(ActiveRuleDto activeRule, RuleDto rule, UserSession userSession) { checkPermission(userSession); - rule.setNoteUpdatedAt(new Date(system.now())); - rule.setNoteData(null); - rule.setNoteUserLogin(null); - rule.setNoteCreatedAt(null); - rule.setNoteUpdatedAt(null); - ruleDao.update(rule); - // TODO notify E/S of active rule change + SqlSession session = myBatis.openSession(); + try { + rule.setNoteUpdatedAt(new Date(system.now())); + rule.setNoteData(null); + rule.setNoteUserLogin(null); + rule.setNoteCreatedAt(null); + rule.setNoteUpdatedAt(null); + ruleDao.update(rule); + session.commit(); + + // TODO notify E/S of rule change + } finally { + MyBatis.closeQuietly(session); + } + } + + 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); + } + + private void reindexActiveRule(ActiveRuleDto activeRuleDto, SqlSession session) { + ruleRegistry.deleteActiveRules(newArrayList(activeRuleDto.getId())); + Multimap paramsByActiveRule = ArrayListMultimap.create(); + for (ActiveRuleParamDto param : activeRuleDao.selectParamsByActiveRuleId(activeRuleDto.getId(), session)) { + paramsByActiveRule.put(param.getActiveRuleId(), param); + } + ruleRegistry.bulkIndexActiveRules(newArrayList(activeRuleDto), paramsByActiveRule); } private List readProfilesFromXml(NewProfileResult result, Map xmlProfilesByPlugin) { @@ -403,21 +434,22 @@ public class QProfileOperations implements ServerComponent { .setValue(activeRuleParam.getValue()); } + @CheckForNull + private ActiveRuleDto findActiveRule(QualityProfileDto qualityProfile, RuleDto rule) { + return activeRuleDao.selectByProfileAndRule(qualityProfile.getId(), rule.getId()); + } + private void checkPermission(UserSession userSession) { + userSession.checkLoggedIn(); userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); } - private ActiveRuleDto validate(QualityProfileDto qualityProfile, Rule rule) { - ActiveRuleDto activeRuleDto = findActiveRule(qualityProfile, rule); - if (activeRuleDto == null) { - throw new BadRequestException("No rule has been activated on this profile."); + private String getLoggedName(UserSession userSession) { + String name = userSession.name(); + if (Strings.isNullOrEmpty(name)) { + throw new BadRequestException("User name can't be null"); } - return activeRuleDto; - } - - @CheckForNull - private ActiveRuleDto findActiveRule(QualityProfileDto qualityProfile, Rule rule) { - return activeRuleDao.selectByProfileAndRule(qualityProfile.getId(), rule.getId()); + return name; } } 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 e2ca5db806e..03fa7b127d2 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 @@ -23,11 +23,11 @@ package org.sonar.server.qualityprofile; import com.google.common.base.Strings; import org.sonar.api.ServerComponent; import org.sonar.api.component.Component; -import org.sonar.api.rules.Rule; -import org.sonar.api.rules.RuleFinder; import org.sonar.core.component.ComponentDto; import org.sonar.core.qualityprofile.db.*; import org.sonar.core.resource.ResourceDao; +import org.sonar.core.rule.RuleDao; +import org.sonar.core.rule.RuleDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.ProfileRuleQuery; @@ -48,8 +48,8 @@ public class QProfiles implements ServerComponent { private final QualityProfileDao qualityProfileDao; private final ActiveRuleDao activeRuleDao; + private final RuleDao ruleDao; private final ResourceDao resourceDao; - private final RuleFinder ruleFinder; private final QProfileProjectService projectService; @@ -57,12 +57,12 @@ public class QProfiles implements ServerComponent { private final QProfileOperations operations; private final ProfileRules rules; - public QProfiles(QualityProfileDao qualityProfileDao, ActiveRuleDao activeRuleDao, ResourceDao resourceDao, RuleFinder ruleFinder, + public QProfiles(QualityProfileDao qualityProfileDao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, ResourceDao resourceDao, QProfileProjectService projectService, QProfileSearch search, QProfileOperations operations, ProfileRules rules) { this.qualityProfileDao = qualityProfileDao; this.activeRuleDao = activeRuleDao; + this.ruleDao = ruleDao; this.resourceDao = resourceDao; - this.ruleFinder = ruleFinder; this.projectService = projectService; this.search = search; this.operations = operations; @@ -191,52 +191,74 @@ public class QProfiles implements ServerComponent { return rules.countInactiveRules(query); } - public RuleActivationResult activateRule(int profileId, int ruleId, String severity) { + public ActiveRuleChanged activateRule(int profileId, int ruleId, String severity) { QualityProfileDto qualityProfile = findNotNull(profileId); - Rule rule = findRuleNotNull(ruleId); + RuleDto rule = findRuleNotNull(ruleId); ActiveRuleDto activeRule = findActiveRule(qualityProfile, rule); if (activeRule == null) { - return operations.createActiveRule(qualityProfile, rule, severity, UserSession.get()); + activeRule = operations.createActiveRule(qualityProfile, rule, severity, UserSession.get()); } else { - return operations.updateSeverity(qualityProfile, activeRule, severity, UserSession.get()); + operations.updateSeverity(qualityProfile, activeRule, severity, UserSession.get()); } + return activeRuleChanged(qualityProfile, activeRule); } - public RuleActivationResult deactivateRule(int profileId, int ruleId) { + public ActiveRuleChanged deactivateRule(int profileId, int ruleId) { QualityProfileDto qualityProfile = findNotNull(profileId); - Rule rule = findRuleNotNull(ruleId); - return operations.deactivateRule(qualityProfile, rule, UserSession.get()); + RuleDto rule = findRuleNotNull(ruleId); + ActiveRuleDto activeRule = findActiveRuleNotNull(qualityProfile, rule); + operations.deactivateRule(activeRule, UserSession.get()); + return activeRuleChanged(qualityProfile, activeRule); } - public QProfileRule updateActiveRuleParam(int activeRuleId, String key, @Nullable String value) { + public ActiveRuleChanged updateActiveRuleParam(int profileId, int activeRuleId, String key, @Nullable String value) { String sanitizedValue = Strings.emptyToNull(value); + QualityProfileDto qualityProfile = findNotNull(profileId); ActiveRuleParamDto activeRuleParam = findActiveRuleParam(activeRuleId, key); ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); UserSession userSession = UserSession.get(); - QProfileRule result = null; if (activeRuleParam == null && sanitizedValue != null) { - result = operations.createActiveRuleParam(activeRule, key, value, userSession); + operations.createActiveRuleParam(activeRule, key, value, userSession); } else if (activeRuleParam != null && sanitizedValue == null) { - result = operations.deleteActiveRuleParam(activeRule, activeRuleParam, userSession); + operations.deleteActiveRuleParam(activeRule, activeRuleParam, userSession); } else if (activeRuleParam != null) { operations.updateActiveRuleParam(activeRule, activeRuleParam, value, userSession); + } else { + // No active rule param and no value -> do nothing } - return result; + return activeRuleChanged(qualityProfile, activeRule); } - public void updateActiveRuleNote(int activeRuleId, String note) { - String sanitizedNote = Strings.emptyToNull(note); + public QProfileRule updateActiveRuleNote(int activeRuleId, String note) { ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); + String sanitizedNote = Strings.emptyToNull(note); if (sanitizedNote != null) { - operations.updateActiveRuleNote(activeRule, sanitizedNote, UserSession.get()); + operations.updateActiveRuleNote(activeRule, note, UserSession.get()); + } else { + // Empty note -> do nothing } + return rules.getFromActiveRuleId(activeRule.getId()); } - public void deleteActiveRuleNote(int activeRuleId) { + public QProfileRule deleteActiveRuleNote(int activeRuleId) { ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); operations.deleteActiveRuleNote(activeRule, UserSession.get()); + return rules.getFromActiveRuleId(activeRule.getId()); } + public QProfileRule updateRuleNote(int activeRuleId, int ruleId, String note) { + ActiveRuleDto activeRule = findActiveRuleNotNull(activeRuleId); + RuleDto rule = findRuleNotNull(ruleId); + String sanitizedNote = Strings.emptyToNull(note); + if (sanitizedNote != null) { + operations.updateRuleNote(activeRule, rule, note, UserSession.get()); + } else { + operations.deleteRuleNote(activeRule, rule, UserSession.get()); + } + return rules.getFromActiveRuleId(activeRule.getId()); + } + + // // Quality profile validation // @@ -312,8 +334,8 @@ public class QProfiles implements ServerComponent { // Rule validation // - private Rule findRuleNotNull(int ruleId) { - Rule rule = ruleFinder.findById(ruleId); + private RuleDto findRuleNotNull(int ruleId) { + RuleDto rule = ruleDao.selectById(ruleId); if (rule == null) { throw new NotFoundException("This rule does not exists."); } @@ -332,8 +354,16 @@ public class QProfiles implements ServerComponent { return activeRule; } + private ActiveRuleDto findActiveRuleNotNull(QualityProfileDto qualityProfile, RuleDto rule) { + ActiveRuleDto activeRuleDto = findActiveRule(qualityProfile, rule); + if (activeRuleDto == null) { + throw new BadRequestException("No rule has been activated on this profile."); + } + return activeRuleDto; + } + @CheckForNull - private ActiveRuleDto findActiveRule(QualityProfileDto qualityProfile, Rule rule) { + private ActiveRuleDto findActiveRule(QualityProfileDto qualityProfile, RuleDto rule) { return activeRuleDao.selectByProfileAndRule(qualityProfile.getId(), rule.getId()); } @@ -342,5 +372,8 @@ public class QProfiles implements ServerComponent { return activeRuleDao.selectParamByActiveRuleAndKey(activeRuleId, key); } + private ActiveRuleChanged activeRuleChanged(QualityProfileDto qualityProfile, ActiveRuleDto activeRule){ + return new ActiveRuleChanged(QProfile.from(qualityProfile), rules.getFromActiveRuleId(activeRule.getId())); + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationResult.java deleted file mode 100644 index a787804ec57..00000000000 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationResult.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * SonarQube, open source software quality management tool. - * Copyright (C) 2008-2013 SonarSource - * mailto:contact AT sonarsource DOT com - * - * SonarQube is free software; you can redistribute it and/or - * modify it under the terms of the GNU Lesser General Public - * License as published by the Free Software Foundation; either - * version 3 of the License, or (at your option) any later version. - * - * SonarQube is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ - -package org.sonar.server.qualityprofile; - - -public class RuleActivationResult { - - private QProfile profile; - private QProfileRule rule; - - public RuleActivationResult(QProfile profile, QProfileRule rule) { - this.profile = profile; - this.rule = rule; - } - - public QProfile profile() { - return profile; - } - - public QProfileRule rule() { - return rule; - } -} diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java index a364bf02d4d..74c07b9cbbd 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java @@ -339,6 +339,14 @@ public class RuleRegistry { .field(ActiveRuleDocument.FIELD_SEVERITY, Severity.get(activeRule.getSeverity())) .field(ActiveRuleDocument.FIELD_PROFILE_ID, activeRule.getProfileId()) .field(ActiveRuleDocument.FIELD_INHERITANCE, activeRule.getInheritance()); + if (activeRule.getNoteData() != null || activeRule.getNoteUserLogin() != null) { + document.startObject(RuleDocument.FIELD_NOTE) + .field(ActiveRuleDocument.FIELD_NOTE_DATA, activeRule.getNoteData()) + .field(ActiveRuleDocument.FIELD_NOTE_USER_LOGIN, activeRule.getNoteUserLogin()) + .field(ActiveRuleDocument.FIELD_NOTE_CREATED_AT, activeRule.getNoteCreatedAt()) + .field(ActiveRuleDocument.FIELD_NOTE_UPDATED_AT, activeRule.getNoteUpdatedAt()) + .endObject(); + } if (!params.isEmpty()) { document.startArray(ActiveRuleDocument.FIELD_PARAMS); for (ActiveRuleParamDto param : params) { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/new_rules_configuration_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/new_rules_configuration_controller.rb index 5a7a60b6417..2000c99a2ad 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/new_rules_configuration_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/new_rules_configuration_controller.rb @@ -309,14 +309,13 @@ class NewRulesConfigurationController < ApplicationController access_denied unless has_role?(:profileadmin) require_parameters :param_id, :active_rule_id, :profile_id + result = nil call_backend do - Internal.quality_profiles.updateActiveRuleParam(params[:active_rule_id].to_i, params[:param_id], params[:value]) + result = Internal.quality_profiles.updateActiveRuleParam(params[:profile_id].to_i, params[:active_rule_id].to_i, params[:param_id], params[:value]) end - - # TODO use a QProfileRule instead of rails objects - profile = Profile.find(params[:profile_id].to_i) - active_rule = ActiveRule.find(params[:active_rule_id].to_i) - render :partial => 'rule', :locals => {:profile => profile, :rule => active_rule.rule, :active_rule => active_rule} + profile = result.profile + rule = result.rule + render :partial => 'rule', :locals => {:profile => profile, :rule => rule} end @@ -342,13 +341,11 @@ class NewRulesConfigurationController < ApplicationController verify_post_request require_parameters :active_rule_id, :note + rule = nil call_backend do - Internal.quality_profiles.updateActiveRuleNote(params[:active_rule_id].to_i, params[:note]) + rule = Internal.quality_profiles.updateActiveRuleNote(params[:active_rule_id].to_i, params[:note]) end - - # TODO use a QProfileRule instead of rails objects - active_rule = ActiveRule.find(params[:active_rule_id]) - render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :profile => active_rule.rules_profile} + render :partial => 'active_rule_note', :locals => {:rule => rule} end @@ -356,13 +353,11 @@ class NewRulesConfigurationController < ApplicationController verify_post_request require_parameters :active_rule_id + rule = nil call_backend do - Internal.quality_profiles.deleteActiveRuleNote(params[:active_rule_id].to_i) + rule = Internal.quality_profiles.deleteActiveRuleNote(params[:active_rule_id].to_i) end - - # TODO use a QProfileRule instead of rails objects - active_rule = ActiveRule.find(params[:active_rule_id]) - render :partial => 'active_rule_note', :locals => {:active_rule => active_rule, :profile => active_rule.rules_profile} + render :partial => 'active_rule_note', :locals => {:rule => rule} end diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule.html.erb index 31694628def..01d8a2d03de 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/new_rules_configuration/_rule.html.erb @@ -1,3 +1,5 @@ +<% #locals = rule, profile -%> +
<% enable_modification = profiles_administrator? @@ -87,7 +89,7 @@ <% end %>
- <%= render :partial => 'active_rule_note', :locals => {:rule => rule, :profile => profile} %> + <%= render :partial => 'active_rule_note', :locals => {:rule => rule} %>
<% if profiles_administrator? %> 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 95ceb3af366..6a95948e7bf 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 @@ -56,6 +56,7 @@ import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.rule.ProfileRules; import org.sonar.server.rule.RuleRegistry; import org.sonar.server.user.MockUserSession; +import org.sonar.server.user.UserSession; import java.io.Reader; import java.util.Date; @@ -112,10 +113,13 @@ public class QProfileOperationsTest { List importers = newArrayList(); - QProfileOperations operations; - Integer currentId = 1; + UserSession authorizedUserSession = MockUserSession.create().setLogin("nicolas").setName("Nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); + UserSession unauthorizedUserSession = MockUserSession.create().setLogin("nicolas").setName("Nicolas"); + + QProfileOperations operations; + @Before public void setUp() throws Exception { when(myBatis.openSession()).thenReturn(session); @@ -136,7 +140,7 @@ public class QProfileOperationsTest { @Test public void create_profile() throws Exception { - NewProfileResult result = operations.newProfile("Default", "java", Maps.newHashMap(), MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + NewProfileResult result = operations.newProfile("Default", "java", Maps.newHashMap(), authorizedUserSession); assertThat(result.profile().name()).isEqualTo("Default"); assertThat(result.profile().language()).isEqualTo("java"); @@ -153,7 +157,7 @@ public class QProfileOperationsTest { @Test public void fail_to_create_profile_without_profile_admin_permission() throws Exception { try { - operations.newProfile("Default", "java", Maps.newHashMap(), MockUserSession.create()); + operations.newProfile("Default", "java", Maps.newHashMap(), unauthorizedUserSession); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(ForbiddenException.class); @@ -178,7 +182,7 @@ public class QProfileOperationsTest { when(importer.importProfile(any(Reader.class), any(ValidationMessages.class))).thenReturn(profile); importers.add(importer); - operations.newProfile("Default", "java", xmlProfilesByPlugin, MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.newProfile("Default", "java", xmlProfilesByPlugin, authorizedUserSession); verify(session).commit(); ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); @@ -217,7 +221,7 @@ public class QProfileOperationsTest { } }).when(importer).importProfile(any(Reader.class), any(ValidationMessages.class)); - operations.newProfile("Default", "java", xmlProfilesByPlugin, MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.newProfile("Default", "java", xmlProfilesByPlugin, authorizedUserSession); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(BadRequestException.class); @@ -228,7 +232,7 @@ public class QProfileOperationsTest { @Test public void rename_profile() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("Default").setLanguage("java"); - operations.renameProfile(qualityProfile, "Default profile", MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.renameProfile(qualityProfile, "Default profile", authorizedUserSession); ArgumentCaptor profileArgument = ArgumentCaptor.forClass(QualityProfileDto.class); verify(qualityProfileDao).update(profileArgument.capture()); @@ -241,7 +245,7 @@ public class QProfileOperationsTest { public void update_default_profile() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); - operations.setDefaultProfile(qualityProfile, MockUserSession.create().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.setDefaultProfile(qualityProfile, authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(PropertyDto.class); verify(propertiesDao).setProperty(argumentCaptor.capture()); @@ -252,8 +256,7 @@ public class QProfileOperationsTest { @Test public void activate_rule() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); when(ruleDao.selectParameters(eq(10), eq(session))).thenReturn(newArrayList(new RuleParamDto().setId(20).setName("max").setDefaultValue("10"))); when(profileRules.getFromActiveRuleId(anyInt())).thenReturn(mock(QProfileRule.class)); final int idActiveRuleToUpdate = 42; @@ -261,14 +264,12 @@ public class QProfileOperationsTest { RuleInheritanceActions inheritanceActions = new RuleInheritanceActions() .addToIndex(idActiveRuleToUpdate) .addToDelete(idActiveRuleToDelete); - when(profilesManager.activated(eq(1), anyInt(), eq("nicolas"))).thenReturn(inheritanceActions); + 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.selectParamsByRuleIds(anyList(), isA(SqlSession.class))).thenReturn(ImmutableList. of(mock(ActiveRuleParamDto.class))); + when(activeRuleDao.selectParamsByActiveRuleIds(anyList(), isA(SqlSession.class))).thenReturn(ImmutableList. of(mock(ActiveRuleParamDto.class))); - RuleActivationResult result = operations.createActiveRule(qualityProfile, rule, Severity.CRITICAL, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); - assertThat(result.profile()).isNotNull(); - assertThat(result.rule()).isNotNull(); - assertThat(result.rule().activeRuleId()).isNotNull(); + ActiveRuleDto result = operations.createActiveRule(qualityProfile, rule, Severity.CRITICAL, authorizedUserSession); + assertThat(result).isNotNull(); ArgumentCaptor activeRuleArgument = ArgumentCaptor.forClass(ActiveRuleDto.class); verify(activeRuleDao).insert(activeRuleArgument.capture(), eq(session)); @@ -281,8 +282,7 @@ public class QProfileOperationsTest { assertThat(activeRuleParamArgument.getValue().getValue()).isEqualTo("10"); verify(session).commit(); - verify(profileRules).getFromActiveRuleId(anyInt()); - verify(profilesManager).activated(eq(1), anyInt(), eq("nicolas")); + verify(profilesManager).activated(eq(1), anyInt(), eq("Nicolas")); verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @@ -293,84 +293,62 @@ public class QProfileOperationsTest { rule.setId(10); ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); when(profileRules.getFromActiveRuleId(anyInt())).thenReturn(mock(QProfileRule.class)); - when(profilesManager.ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("nicolas"))).thenReturn(new RuleInheritanceActions()); + when(profilesManager.ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas"))).thenReturn(new RuleInheritanceActions()); - RuleActivationResult result = operations.updateSeverity(qualityProfile, activeRule, Severity.MAJOR, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); - assertThat(result.profile()).isNotNull(); - assertThat(result.rule()).isNotNull(); - assertThat(result.rule().activeRuleId()).isNotNull(); + operations.updateSeverity(qualityProfile, activeRule, Severity.MAJOR, authorizedUserSession); verify(activeRuleDao).update(eq(activeRule), eq(session)); verify(session).commit(); - verify(profileRules).getFromActiveRuleId(anyInt()); - verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("nicolas")); + verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("Nicolas")); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test public void deactivate_rule() throws Exception { - QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule); when(profileRules.getFromRuleId(anyInt())).thenReturn(mock(QProfileRule.class)); - when(profilesManager.deactivated(eq(1), anyInt(), eq("nicolas"))).thenReturn(new RuleInheritanceActions()); + when(profilesManager.deactivated(eq(1), anyInt(), eq("Nicolas"))).thenReturn(new RuleInheritanceActions()); - RuleActivationResult result = operations.deactivateRule(qualityProfile, rule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); - assertThat(result.profile()).isNotNull(); - assertThat(result.rule()).isNotNull(); + operations.deactivateRule(activeRule, authorizedUserSession); verify(activeRuleDao).delete(eq(5), eq(session)); verify(activeRuleDao).deleteParameters(eq(5), eq(session)); verify(session).commit(); - verify(profileRules).getFromRuleId(anyInt()); - verify(profilesManager).deactivated(eq(1), anyInt(), eq("nicolas")); - } - - @Test - public void fail_to_deactivate_rule_if_no_active_rule_on_profile() throws Exception { - QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); - when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(null); - - try { - operations.deactivateRule(qualityProfile, rule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); - fail(); - } catch (Exception e) { - assertThat(e).isInstanceOf(BadRequestException.class); - } - verify(activeRuleDao, never()).update(any(ActiveRuleDto.class), eq(session)); - verify(session, never()).commit(); - verifyZeroInteractions(profilesManager); + verify(profilesManager).deactivated(eq(1), anyInt(), eq("Nicolas")); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test public void update_active_rule_param() throws Exception { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20"); - when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("30"), eq("nicolas"))).thenReturn(new RuleInheritanceActions()); + when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("30"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions()); - operations.updateActiveRuleParam(activeRule, activeRuleParam, "30", MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.updateActiveRuleParam(activeRule, activeRuleParam, "30", authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class); verify(activeRuleDao).update(argumentCaptor.capture(), eq(session)); assertThat(argumentCaptor.getValue().getId()).isEqualTo(100); assertThat(argumentCaptor.getValue().getValue()).isEqualTo("30"); - verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq("30"), eq("nicolas")); + 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)); } @Test public void remove_active_rule_param() throws Exception { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20"); - when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq((String) null), eq("nicolas"))).thenReturn(new RuleInheritanceActions()); + when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq((String) null), eq("Nicolas"))).thenReturn(new RuleInheritanceActions()); - operations.deleteActiveRuleParam(activeRule, activeRuleParam, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.deleteActiveRuleParam(activeRule, activeRuleParam, authorizedUserSession); + 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(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq("20"), eq((String) null), eq("Nicolas")); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test @@ -378,9 +356,9 @@ public class QProfileOperationsTest { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); RuleParamDto ruleParam = new RuleParamDto().setRuleId(10).setName("max").setDefaultValue("20"); when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(ruleParam); - when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("nicolas"))).thenReturn(new RuleInheritanceActions()); + when(profilesManager.ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("Nicolas"))).thenReturn(new RuleInheritanceActions()); - operations.createActiveRuleParam(activeRule, "max", "30", MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ActiveRuleParamDto.class); verify(activeRuleDao).insert(argumentCaptor.capture(), eq(session)); @@ -388,7 +366,9 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getValue()).isEqualTo("30"); assertThat(argumentCaptor.getValue().getActiveRuleId()).isEqualTo(5); - verify(profilesManager).ruleParamChanged(eq(1), eq(5), eq("max"), eq((String) null), eq("30"), eq("nicolas")); + 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)); } @Test @@ -396,7 +376,7 @@ public class QProfileOperationsTest { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); when(ruleDao.selectParamByRuleAndKey(10, "max", session)).thenReturn(null); try { - operations.createActiveRuleParam(activeRule, "max", "30", MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.createActiveRuleParam(activeRule, "max", "30", authorizedUserSession); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(IllegalArgumentException.class); @@ -406,20 +386,23 @@ public class QProfileOperationsTest { } @Test - public void update_active_rule_note_when_no_note() throws Exception { + public void update_active_rule_note_when_no_existing_note() throws Exception { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1).setNoteCreatedAt(null).setNoteData(null); long now = System.currentTimeMillis(); doReturn(now).when(system).now(); - operations.updateActiveRuleNote(activeRule, "My note", MockUserSession.create().setLogin("nicolas").setName("Nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.updateActiveRuleNote(activeRule, "My note", authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class); - verify(activeRuleDao).update(argumentCaptor.capture()); + verify(activeRuleDao).update(argumentCaptor.capture(), eq(session)); assertThat(argumentCaptor.getValue().getNoteData()).isEqualTo("My note"); assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas"); assertThat(argumentCaptor.getValue().getNoteCreatedAt().getTime()).isEqualTo(now); assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now); + + verify(session).commit(); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test @@ -434,11 +417,14 @@ public class QProfileOperationsTest { operations.updateActiveRuleNote(activeRule, "My new note", MockUserSession.create().setLogin("guy").setName("Guy").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class); - verify(activeRuleDao).update(argumentCaptor.capture()); + verify(activeRuleDao).update(argumentCaptor.capture(), eq(session)); assertThat(argumentCaptor.getValue().getNoteData()).isEqualTo("My new note"); assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas"); assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isEqualTo(createdAt); assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now); + + verify(session).commit(); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test @@ -450,7 +436,7 @@ public class QProfileOperationsTest { long now = System.currentTimeMillis(); doReturn(now).when(system).now(); - operations.deleteActiveRuleNote(activeRule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.deleteActiveRuleNote(activeRule, authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(ActiveRuleDto.class); verify(activeRuleDao).update(argumentCaptor.capture()); @@ -458,16 +444,20 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getNoteUserLogin()).isNull(); assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isNull(); assertThat(argumentCaptor.getValue().getNoteUpdatedAt()).isNull(); + + verify(session).commit(); + verify(ruleRegistry).bulkIndexActiveRules(anyList(), isA(Multimap.class)); } @Test - public void update_rule_note_when_no_note() throws Exception { + public void update_rule_note_when_no_existing_note() throws Exception { RuleDto rule = new RuleDto().setId(10).setNoteCreatedAt(null).setNoteData(null); + ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); long now = System.currentTimeMillis(); doReturn(now).when(system).now(); - operations.updateRuleNote(rule, "My note", MockUserSession.create().setLogin("nicolas").setName("Nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.updateRuleNote(activeRule, rule, "My note", authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(RuleDto.class); verify(ruleDao).update(argumentCaptor.capture()); @@ -475,17 +465,20 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas"); assertThat(argumentCaptor.getValue().getNoteCreatedAt().getTime()).isEqualTo(now); assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now); + + verify(session).commit(); } @Test public void update_rule_note_when_already_note() throws Exception { Date createdAt = DateUtils.parseDate("2013-12-20"); RuleDto rule = new RuleDto().setId(10).setNoteCreatedAt(createdAt).setNoteData("My previous note").setNoteUserLogin("nicolas"); + ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); long now = System.currentTimeMillis(); doReturn(now).when(system).now(); - operations.updateRuleNote(rule, "My new note", MockUserSession.create().setLogin("guy").setName("Guy").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.updateRuleNote(activeRule, rule, "My new note", MockUserSession.create().setLogin("guy").setName("Guy").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(RuleDto.class); verify(ruleDao).update(argumentCaptor.capture()); @@ -493,17 +486,20 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getNoteUserLogin()).isEqualTo("nicolas"); assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isEqualTo(createdAt); assertThat(argumentCaptor.getValue().getNoteUpdatedAt().getTime()).isEqualTo(now); + + verify(session).commit(); } @Test public void delete_rule_note() throws Exception { Date createdAt = DateUtils.parseDate("2013-12-20"); RuleDto rule = new RuleDto().setId(10).setNoteData("My note").setNoteUserLogin("nicolas").setNoteCreatedAt(createdAt).setNoteUpdatedAt(createdAt); + ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); long now = System.currentTimeMillis(); doReturn(now).when(system).now(); - operations.deleteRuleNote(rule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN)); + operations.deleteRuleNote(activeRule, rule, authorizedUserSession); ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(RuleDto.class); verify(ruleDao).update(argumentCaptor.capture()); @@ -511,6 +507,8 @@ public class QProfileOperationsTest { assertThat(argumentCaptor.getValue().getNoteUserLogin()).isNull(); assertThat(argumentCaptor.getValue().getNoteCreatedAt()).isNull(); assertThat(argumentCaptor.getValue().getNoteUpdatedAt()).isNull(); + + verify(session).commit(); } } 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 098d6af11bd..b391b82fafe 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 @@ -27,11 +27,11 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.sonar.api.rule.Severity; -import org.sonar.api.rules.Rule; -import org.sonar.api.rules.RuleFinder; import org.sonar.core.component.ComponentDto; import org.sonar.core.qualityprofile.db.*; import org.sonar.core.resource.ResourceDao; +import org.sonar.core.rule.RuleDao; +import org.sonar.core.rule.RuleDto; import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.rule.ProfileRuleQuery; @@ -58,10 +58,10 @@ public class QProfilesTest { ActiveRuleDao activeRuleDao; @Mock - ResourceDao resourceDao; + RuleDao ruleDao; @Mock - RuleFinder ruleFinder; + ResourceDao resourceDao; @Mock QProfileProjectService projectService; @@ -79,7 +79,7 @@ public class QProfilesTest { @Before public void setUp() throws Exception { - qProfiles = new QProfiles(qualityProfileDao, activeRuleDao, resourceDao, ruleFinder, projectService, search, service, rules); + qProfiles = new QProfiles(qualityProfileDao, activeRuleDao, ruleDao, resourceDao, projectService, search, service, rules); } @Test @@ -285,9 +285,11 @@ public class QProfilesTest { public void activate_rule() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); - when(ruleFinder.findById(10)).thenReturn(rule); + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); + + when(service.createActiveRule(eq(qualityProfile), eq(rule), eq(Severity.BLOCKER), any(UserSession.class))) + .thenReturn(new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1)); qProfiles.activateRule(1, 10, Severity.BLOCKER); @@ -319,9 +321,8 @@ public class QProfilesTest { ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); - when(ruleFinder.findById(10)).thenReturn(rule); + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); qProfiles.activateRule(1, 10, Severity.BLOCKER); @@ -332,37 +333,57 @@ public class QProfilesTest { public void deactivate_rule() throws Exception { QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); - Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle"); - rule.setId(10); - when(ruleFinder.findById(10)).thenReturn(rule); + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); + ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1); + when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule); qProfiles.deactivateRule(1, 10); - verify(service).deactivateRule(eq(qualityProfile), eq(rule), any(UserSession.class)); + verify(service).deactivateRule(eq(activeRule), any(UserSession.class)); + } + + @Test + public void fail_to_deactivate_rule_if_no_active_rule_on_profile() throws Exception { + QualityProfileDto qualityProfile = new QualityProfileDto().setId(1).setName("My profile").setLanguage("java"); + when(qualityProfileDao.selectById(1)).thenReturn(qualityProfile); + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); + when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(null); + + try { + qProfiles.deactivateRule(1, 10); + fail(); + } catch (Exception e) { + assertThat(e).isInstanceOf(BadRequestException.class); + } } @Test public void update_active_rule_param() throws Exception { + when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); + ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); when(activeRuleDao.selectById(50)).thenReturn(activeRule); ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20"); when(activeRuleDao.selectParamByActiveRuleAndKey(50, "max")).thenReturn(activeRuleParam); - qProfiles.updateActiveRuleParam(50, "max", "20"); + qProfiles.updateActiveRuleParam(1, 50, "max", "20"); verify(service).updateActiveRuleParam(eq(activeRule), eq(activeRuleParam), eq("20"), any(UserSession.class)); } @Test public void fail_to_update_active_rule_param_if_active_rule_not_found() throws Exception { + when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20"); when(activeRuleDao.selectParamByActiveRuleAndKey(50, "max")).thenReturn(activeRuleParam); when(activeRuleDao.selectById(50)).thenReturn(null); try { - qProfiles.updateActiveRuleParam(50, "max", "20"); + qProfiles.updateActiveRuleParam(1, 50, "max", "20"); fail(); } catch (Exception e) { assertThat(e).isInstanceOf(NotFoundException.class); @@ -372,35 +393,38 @@ public class QProfilesTest { @Test public void create_active_rule_param() throws Exception { + when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); when(activeRuleDao.selectById(50)).thenReturn(activeRule); when(activeRuleDao.selectParamByActiveRuleAndKey(50, "max")).thenReturn(null); - qProfiles.updateActiveRuleParam(50, "max", "20"); + qProfiles.updateActiveRuleParam(1, 50, "max", "20"); verify(service).createActiveRuleParam(eq(activeRule), eq("max"), eq("20"), any(UserSession.class)); } @Test public void delete_active_rule_param() throws Exception { + when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); when(activeRuleDao.selectById(50)).thenReturn(activeRule); ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto().setId(100).setActiveRuleId(5).setKey("max").setValue("20"); when(activeRuleDao.selectParamByActiveRuleAndKey(50, "max")).thenReturn(activeRuleParam); - qProfiles.updateActiveRuleParam(50, "max", ""); + qProfiles.updateActiveRuleParam(1, 50, "max", ""); verify(service).deleteActiveRuleParam(eq(activeRule), eq(activeRuleParam), any(UserSession.class)); } @Test public void do_nothing_when_updating_active_rule_param_with_no_param_and_empty_value() throws Exception { + when(qualityProfileDao.selectById(1)).thenReturn(new QualityProfileDto().setId(1).setName("My profile").setLanguage("java")); ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); when(activeRuleDao.selectById(50)).thenReturn(activeRule); when(activeRuleDao.selectParamByActiveRuleAndKey(50, "max")).thenReturn(null); - qProfiles.updateActiveRuleParam(50, "max", ""); + qProfiles.updateActiveRuleParam(1, 50, "max", ""); verifyZeroInteractions(service); } @@ -416,13 +440,13 @@ public class QProfilesTest { } @Test - public void do_nothing_when_create_active_rule_note_with_empty_note() throws Exception { + public void not_update_rule_note_when_empty_note() throws Exception { ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); when(activeRuleDao.selectById(50)).thenReturn(activeRule); qProfiles.updateActiveRuleNote(50, ""); - verifyZeroInteractions(service); + verify(service, never()).updateActiveRuleNote(eq(activeRule), anyString(), any(UserSession.class)); } @Test @@ -435,4 +459,30 @@ public class QProfilesTest { verify(service).deleteActiveRuleNote(eq(activeRule), any(UserSession.class)); } + @Test + public void create_rule_note() throws Exception { + ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); + when(activeRuleDao.selectById(50)).thenReturn(activeRule); + + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); + + qProfiles.updateRuleNote(50, 10, "My note"); + + verify(service).updateRuleNote(eq(activeRule), eq(rule), eq("My note"), any(UserSession.class)); + } + + @Test + public void delete_rule_note() throws Exception { + ActiveRuleDto activeRule = new ActiveRuleDto().setId(50); + when(activeRuleDao.selectById(50)).thenReturn(activeRule); + + RuleDto rule = new RuleDto().setId(10).setRepositoryKey("squid").setRuleKey("AvoidCycle"); + when(ruleDao.selectById(10)).thenReturn(rule); + + qProfiles.updateRuleNote(50, 10, ""); + + verify(service).deleteRuleNote(eq(activeRule), eq(rule), any(UserSession.class)); + } + }