From 1605b4df3ecd7d9880462f1a8973e28d0fa23a5e Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 23 May 2014 10:59:21 +0200 Subject: [PATCH] SONAR-5007 improve registration of Quality profiles --- .../core/qualityprofile/db/ActiveRuleDto.java | 3 + .../qualityprofile/db/ActiveRuleParamDto.java | 18 ++- .../qualityprofile/db/QualityProfileDao.java | 15 +- .../qualityprofile/db/QualityProfileDto.java | 10 +- .../org/sonar/core/rule/RuleParamDto.java | 6 +- .../db/ActiveRuleParamDtoTest.java | 43 ++++++ .../org/sonar/api/profiles/RulesProfile.java | 3 +- .../qualityprofile/ActiveRuleService.java | 57 ++++---- .../RegisterQualityProfiles.java | 138 ++++++++---------- .../persistence/ActiveRuleDao.java | 22 +-- .../ActiveRuleServiceMediumTest.java | 1 - .../RegisterQualityProfilesMediumTest.java | 72 ++++----- 12 files changed, 221 insertions(+), 167 deletions(-) create mode 100644 sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDtoTest.java diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java index 0d95e568856..2967e0b0189 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleDto.java @@ -20,6 +20,7 @@ package org.sonar.core.qualityprofile.db; +import com.google.common.base.Preconditions; import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.apache.commons.lang.builder.ToStringStyle; @@ -142,6 +143,8 @@ public class ActiveRuleDto extends Dto { } public static ActiveRuleDto createFor(QualityProfileDto profileDto, RuleDto ruleDto) { + Preconditions.checkArgument(profileDto.getId()!=null, "Profile is not persisted"); + Preconditions.checkArgument(ruleDto.getId()!=null, "Rule is not persisted"); ActiveRuleDto dto = new ActiveRuleDto(); dto.setProfileId(profileDto.getId()); dto.setRuleId(ruleDto.getId()); diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDto.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDto.java index f9e504984a7..401f1600b50 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDto.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDto.java @@ -20,8 +20,13 @@ package org.sonar.core.qualityprofile.db; +import com.google.common.base.Preconditions; import org.sonar.core.rule.RuleParamDto; +import java.util.Collection; +import java.util.HashMap; +import java.util.Map; + public class ActiveRuleParamDto { private Integer id; @@ -77,14 +82,17 @@ public class ActiveRuleParamDto { } public static ActiveRuleParamDto createFor(RuleParamDto param) { - return new ActiveRuleParamDto() + Preconditions.checkArgument(param.getId() != null, "Parameter is not persisted"); + return new ActiveRuleParamDto() .setKey(param.getName()) .setRulesParameterId(param.getId()); } - public static ActiveRuleParamDto createFrom(ActiveRuleParamDto parentParam) { - return new ActiveRuleParamDto() - .setKey(parentParam.getKey()) - .setRulesParameterId(parentParam.getId()); + public static Map groupByKey(Collection params) { + Map result = new HashMap(); + for (ActiveRuleParamDto param : params) { + result.put(param.getKey(), param); + } + return result; } } 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 5b975d5dcb2..6c598bfc230 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 @@ -22,9 +22,9 @@ package org.sonar.core.qualityprofile.db; import com.google.common.base.Preconditions; import org.apache.ibatis.session.SqlSession; -import org.sonar.core.persistence.DaoComponent; import org.sonar.api.ServerComponent; import org.sonar.core.component.ComponentDto; +import org.sonar.core.persistence.DaoComponent; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; @@ -39,6 +39,15 @@ public class QualityProfileDao implements ServerComponent, DaoComponent { this.mybatis = mybatis; } + @CheckForNull + public QualityProfileDto getByKey(QualityProfileKey key, DbSession session) { + return session.getMapper(QualityProfileMapper.class).selectByNameAndLanguage(key.name(), key.lang()); + } + + public List findAll(DbSession session) { + return session.getMapper(QualityProfileMapper.class).selectAll(); + } + public void insert(QualityProfileDto dto, SqlSession session) { session.getMapper(QualityProfileMapper.class).insert(dto); } @@ -75,8 +84,6 @@ public class QualityProfileDao implements ServerComponent, DaoComponent { /** * @deprecated use {@link #delete(QualityProfileDto, DbSession)} - * @param id - * @param session */ @Deprecated public void delete(int id, SqlSession session) { @@ -84,8 +91,8 @@ public class QualityProfileDao implements ServerComponent, DaoComponent { } /** - * @deprecated use {@link #delete(QualityProfileDto, DbSession)} * @param id + * @deprecated use {@link #delete(QualityProfileDto, DbSession)} */ @Deprecated public void delete(int id) { diff --git a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDto.java b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDto.java index 1b6e7aed729..c8785bbdf01 100644 --- a/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDto.java +++ b/sonar-core/src/main/java/org/sonar/core/qualityprofile/db/QualityProfileDto.java @@ -38,7 +38,7 @@ public class QualityProfileDto extends Dto { * @deprecated use QualityProfileDto.createFor instead */ @Deprecated - public QualityProfileDto(){ + public QualityProfileDto() { } @@ -81,7 +81,7 @@ public class QualityProfileDto extends Dto { @CheckForNull public QualityProfileKey getParentKey() { - if(getParent() != null && !getParent().isEmpty()) { + if (getParent() != null && !getParent().isEmpty()) { return QualityProfileKey.of(this.getParent(), this.getLanguage()); } else { return null; @@ -112,9 +112,13 @@ public class QualityProfileDto extends Dto { return this; } - public static QualityProfileDto createFor(String name, String language){ + public static QualityProfileDto createFor(String name, String language) { return new QualityProfileDto() .setName(name) .setLanguage(language); } + + public static QualityProfileDto createFor(QualityProfileKey key) { + return createFor(key.name(), key.lang()); + } } diff --git a/sonar-core/src/main/java/org/sonar/core/rule/RuleParamDto.java b/sonar-core/src/main/java/org/sonar/core/rule/RuleParamDto.java index 826a260a585..1bd0d912aac 100644 --- a/sonar-core/src/main/java/org/sonar/core/rule/RuleParamDto.java +++ b/sonar-core/src/main/java/org/sonar/core/rule/RuleParamDto.java @@ -25,7 +25,7 @@ import org.apache.commons.lang.builder.ToStringStyle; public class RuleParamDto { - private int id; + private Integer id; private Integer ruleId; private String name; private String type; @@ -36,11 +36,11 @@ public class RuleParamDto { } - public int getId() { + public Integer getId() { return id; } - public RuleParamDto setId(int id) { + public RuleParamDto setId(Integer id) { this.id = id; return this; } diff --git a/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDtoTest.java b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDtoTest.java new file mode 100644 index 00000000000..88745afabfe --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/qualityprofile/db/ActiveRuleParamDtoTest.java @@ -0,0 +1,43 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 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.core.qualityprofile.db; + +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Map; + +import static org.fest.assertions.Assertions.assertThat; + +public class ActiveRuleParamDtoTest { + + @Test + public void groupByKey() throws Exception { + assertThat(ActiveRuleParamDto.groupByKey(Collections.emptyList())).isEmpty(); + + Collection dtos = Arrays.asList( + new ActiveRuleParamDto().setKey("foo"), new ActiveRuleParamDto().setKey("bar") + ); + Map group = ActiveRuleParamDto.groupByKey(dtos); + assertThat(group.keySet()).containsOnly("foo", "bar"); + } +} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java b/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java index 088235051b5..e44438d86c8 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/profiles/RulesProfile.java @@ -33,6 +33,7 @@ import org.sonar.api.rules.RulePriority; import org.sonar.api.utils.MessageException; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import javax.persistence.*; import java.util.ArrayList; @@ -359,7 +360,7 @@ public class RulesProfile implements Cloneable { /** * @param optionalSeverity if null, then the default rule severity is used */ - public ActiveRule activateRule(final Rule rule, RulePriority optionalSeverity) { + public ActiveRule activateRule(final Rule rule, @Nullable RulePriority optionalSeverity) { if (Iterables.any(activeRules, new Predicate() { @Override public boolean apply(ActiveRule input) { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java index d5330cbbdab..098aa191196 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleService.java @@ -31,7 +31,6 @@ import org.sonar.core.preview.PreviewCache; import org.sonar.core.qualityprofile.db.ActiveRuleDto; import org.sonar.core.qualityprofile.db.ActiveRuleKey; import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; -import org.sonar.core.qualityprofile.db.QualityProfileKey; import org.sonar.core.rule.RuleParamDto; import org.sonar.server.db.DbClient; import org.sonar.server.qualityprofile.index.ActiveRuleIndex; @@ -71,7 +70,7 @@ public class ActiveRuleService implements ServerComponent { return index.get(ActiveRuleIndex.class).getByKey(key); } - public List findByRuleKey(RuleKey key){ + public List findByRuleKey(RuleKey key) { return index.get(ActiveRuleIndex.class).findByRule(key); } @@ -81,35 +80,44 @@ public class ActiveRuleService implements ServerComponent { */ public List activate(RuleActivation activation) { verifyPermission(UserSession.get()); - DbSession dbSession = db.openSession(false); - List changes = Lists.newArrayList(); try { - RuleActivationContext context = contextFactory.create(activation.getKey(), dbSession); - ActiveRuleChange change; - if (context.activeRule() == null) { - change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, activation.getKey()); - } else { - change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activation.getKey()); - } - change.setSeverity(StringUtils.defaultIfEmpty(activation.getSeverity(), context.defaultSeverity())); - for (RuleParamDto ruleParamDto : context.ruleParams()) { - String value = activation.getParameters().get(ruleParamDto.getName()); - verifyParam(ruleParamDto, value); - change.setParameter(ruleParamDto.getName(), StringUtils.defaultIfEmpty(value, ruleParamDto.getDefaultValue())); + List changes = activate(activation, dbSession); + if (!changes.isEmpty()) { + dbSession.commit(); + previewCache.reportGlobalModification(); } - changes.add(change); - // TODO filter changes without any differences - - persist(changes, context, dbSession); - return changes; - } finally { dbSession.close(); } } + /** + * Activate the rule WITHOUT committing db session and WITHOUT checking permissions + */ + List activate(RuleActivation activation, DbSession dbSession) { + List changes = Lists.newArrayList(); + RuleActivationContext context = contextFactory.create(activation.getKey(), dbSession); + ActiveRuleChange change; + if (context.activeRule() == null) { + change = new ActiveRuleChange(ActiveRuleChange.Type.ACTIVATED, activation.getKey()); + } else { + change = new ActiveRuleChange(ActiveRuleChange.Type.UPDATED, activation.getKey()); + } + change.setSeverity(StringUtils.defaultIfEmpty(activation.getSeverity(), context.defaultSeverity())); + for (RuleParamDto ruleParamDto : context.ruleParams()) { + String value = activation.getParameters().get(ruleParamDto.getName()); + verifyParam(ruleParamDto, value); + change.setParameter(ruleParamDto.getName(), StringUtils.defaultIfEmpty(value, ruleParamDto.getDefaultValue())); + } + changes.add(change); + // TODO filter changes without any differences + + persist(changes, context, dbSession); + return changes; + } + private void persist(Collection changes, RuleActivationContext context, DbSession dbSession) { ActiveRuleDao dao = db.activeRuleDao(); for (ActiveRuleChange change : changes) { @@ -150,10 +158,6 @@ public class ActiveRuleService implements ServerComponent { } } } - if (!changes.isEmpty()) { - dbSession.commit(); - previewCache.reportGlobalModification(); - } } /** @@ -174,6 +178,7 @@ public class ActiveRuleService implements ServerComponent { change = new ActiveRuleChange(ActiveRuleChange.Type.DEACTIVATED, key); changes.add(change); persist(changes, context, dbSession); + dbSession.commit(); return changes; } finally { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java index 21eb41f951f..0f3e3726d7b 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RegisterQualityProfiles.java @@ -25,29 +25,23 @@ import com.google.common.collect.ListMultimap; import com.google.common.collect.Multimaps; import com.google.common.collect.Sets; import org.apache.commons.lang.StringUtils; -import org.apache.ibatis.session.SqlSession; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.sonar.api.ServerComponent; import org.sonar.api.profiles.ProfileDefinition; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.ActiveRuleParam; import org.sonar.api.utils.TimeProfiler; import org.sonar.api.utils.ValidationMessages; -import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; -import org.sonar.core.qualityprofile.db.ActiveRuleDto; -import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; +import org.sonar.core.qualityprofile.db.ActiveRuleKey; import org.sonar.core.qualityprofile.db.QualityProfileDto; -import org.sonar.core.rule.RuleDto; +import org.sonar.core.qualityprofile.db.QualityProfileKey; import org.sonar.core.template.LoadedTemplateDto; -import org.sonar.jpa.session.DatabaseSessionFactory; import org.sonar.server.db.DbClient; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.platform.PersistentSettings; -import org.sonar.server.user.UserSession; import javax.annotation.Nullable; import java.util.Collection; @@ -56,7 +50,10 @@ import java.util.List; import java.util.Map; import java.util.Set; -public class RegisterQualityProfiles { +/** + * Synchronize Quality profiles during server startup + */ +public class RegisterQualityProfiles implements ServerComponent { private static final Logger LOGGER = LoggerFactory.getLogger(RegisterQualityProfiles.class); private static final String DEFAULT_PROFILE_NAME = "Sonar way"; @@ -64,35 +61,34 @@ public class RegisterQualityProfiles { private final PersistentSettings settings; private final List definitions; private final DefaultProfilesCache defaultProfilesCache; - private final DatabaseSessionFactory sessionFactory; private final DbClient dbClient; + private final ActiveRuleService activeRuleService; - public RegisterQualityProfiles(DatabaseSessionFactory sessionFactory, - PersistentSettings settings, - DefaultProfilesCache defaultProfilesCache, DbClient dbClient) { - this(sessionFactory, settings, defaultProfilesCache, dbClient, - Collections.emptyList()); + /** + * To be kept when no ProfileDefinition are injected + */ + public RegisterQualityProfiles(PersistentSettings settings, + DefaultProfilesCache defaultProfilesCache, + DbClient dbClient, + ActiveRuleService activeRuleService) { + this(settings, defaultProfilesCache, dbClient, activeRuleService, Collections.emptyList()); } - public RegisterQualityProfiles(DatabaseSessionFactory sessionFactory, - PersistentSettings settings, + public RegisterQualityProfiles(PersistentSettings settings, DefaultProfilesCache defaultProfilesCache, DbClient dbClient, + ActiveRuleService activeRuleService, List definitions) { - this.sessionFactory = sessionFactory; this.settings = settings; this.defaultProfilesCache = defaultProfilesCache; - this.definitions = definitions; this.dbClient = dbClient; + this.activeRuleService = activeRuleService; + this.definitions = definitions; } public void start() { TimeProfiler profiler = new TimeProfiler(LOGGER).start("Register Quality Profiles"); - // Hibernate session can contain an invalid cache of rules. - // As long ProfileDefinition API will be used, then we'll have to use this commit as Hibernate is used by plugin to load rules when creating their profiles. - sessionFactory.getSession().commit(); - DbSession session = dbClient.openSession(false); try { ListMultimap profilesByLanguage = profilesByLanguage(); @@ -101,11 +97,12 @@ public class RegisterQualityProfiles { verifyLanguage(language, profiles); for (Map.Entry> entry : profilesByName(profiles).entrySet()) { - String name = entry.getKey(); - if (shouldRegister(language, name, session)) { - register(language, name, entry.getValue(), session); + String profileName = entry.getKey(); + QualityProfileKey profileKey = QualityProfileKey.of(profileName, language); + if (shouldRegister(profileKey, session)) { + register(profileKey, entry.getValue(), session); } - defaultProfilesCache.put(language, name); + defaultProfilesCache.put(language, profileName); } setDefault(language, profiles, session); } @@ -127,69 +124,52 @@ public class RegisterQualityProfiles { } } - private void checkPermission(UserSession userSession) { - userSession.checkLoggedIn(); - userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); - } + private void register(QualityProfileKey key, Collection profiles, DbSession session) { + LOGGER.info("Register profile " + key); - private QualityProfileDto newQualityProfileDto(String name, String language, DbSession session) { - checkPermission(UserSession.get()); - if (dbClient.qualityProfileDao().selectByNameAndLanguage(name, language, session) != null) { - throw BadRequestException.ofL10n("quality_profiles.profile_x_already_exists", name); + QualityProfileDto profileDto = dbClient.qualityProfileDao().getByKey(key, session); + if (profileDto != null) { + // cleanup + cleanUp(key, profileDto, session); } + insertNewProfile(key, session); - QualityProfileDto profile = QualityProfileDto.createFor(name, language) - .setVersion(1).setUsed(false); - dbClient.qualityProfileDao().insert(profile, session); - return profile; - } - - private void register(String language, String name, Collection profiles, DbSession session) { - LOGGER.info("Register " + language + " profile: " + name); - - QualityProfileDto profile = dbClient.qualityProfileDao().selectByNameAndLanguage(name, language, session); - if (profile != null) { - dbClient.activeRuleDao().deleteByProfileKey(profile.getKey(), session); - dbClient.qualityProfileDao().delete(profile, session); - } - profile = newQualityProfileDto(name, language, session); - - for (RulesProfile currentRulesProfile : profiles) { - //TODO trapped on legacy Hibernate object. - for (org.sonar.api.rules.ActiveRule activeRule : currentRulesProfile.getActiveRules()) { + for (RulesProfile profile : profiles) { + for (org.sonar.api.rules.ActiveRule activeRule : profile.getActiveRules()) { RuleKey ruleKey = RuleKey.of(activeRule.getRepositoryKey(), activeRule.getRuleKey()); - RuleDto rule = dbClient.ruleDao().getByKey(ruleKey, session); - if (rule == null) { - throw new NotFoundException(String.format("Rule '%s' does not exists.", ruleKey)); - } - - ActiveRuleDto activeRuleDto = dbClient.activeRuleDao().createActiveRule( - profile.getKey(), ruleKey, activeRule.getSeverity().name(), session); - - //TODO trapped on legacy Hibernate object. + RuleActivation activation = new RuleActivation(ActiveRuleKey.of(key, ruleKey)); + activation.setSeverity(activeRule.getSeverity() != null ? activeRule.getSeverity().name() : null); for (ActiveRuleParam param : activeRule.getActiveRuleParams()) { - String paramKey = param.getKey(); - String value = param.getValue(); - ActiveRuleParamDto paramDto = dbClient.activeRuleDao() - .getParamsByActiveRuleAndKey(activeRuleDto, paramKey, session); - if (value != null && !paramDto.getValue().equals(value)) { - paramDto.setValue(value); - dbClient.activeRuleDao().updateParam(activeRuleDto, paramDto, session); - } + activation.setParameter(param.getKey(), param.getValue()); } + activeRuleService.activate(activation, session); } } - dbClient.loadedTemplateDao() - .insert(new LoadedTemplateDto(templateKey(language, name), LoadedTemplateDto.QUALITY_PROFILE_TYPE), session); + LoadedTemplateDto template = new LoadedTemplateDto(templateKey(key), LoadedTemplateDto.QUALITY_PROFILE_TYPE); + dbClient.loadedTemplateDao().insert(template, session); + } + + private void cleanUp(QualityProfileKey key, QualityProfileDto profileDto, DbSession session) { + dbClient.activeRuleDao().deleteByProfileKey(key, session); + dbClient.qualityProfileDao().delete(profileDto, session); + session.commit(); + } + + private void insertNewProfile(QualityProfileKey key, DbSession session) { + QualityProfileDto profile = QualityProfileDto.createFor(key).setVersion(1).setUsed(false); + dbClient.qualityProfileDao().insert(profile, session); + session.commit(); } - private void setDefault(String language, List profiles, SqlSession session) { + private void setDefault(String language, List profiles, DbSession session) { String propertyKey = "sonar.profile." + language; if (settings.getString(propertyKey) == null) { String defaultProfileName = defaultProfileName(profiles); LOGGER.info("Set default " + language + " profile: " + defaultProfileName); settings.saveProperty(propertyKey, defaultProfileName); + } else { + //TODO check that the declared default profile exists, else fix } } @@ -246,12 +226,12 @@ public class RegisterQualityProfiles { return names; } - private boolean shouldRegister(String language, String profileName, SqlSession session) { + private boolean shouldRegister(QualityProfileKey key, DbSession session) { return dbClient.loadedTemplateDao() - .countByTypeAndKey(LoadedTemplateDto.QUALITY_PROFILE_TYPE, templateKey(language, profileName), session) == 0; + .countByTypeAndKey(LoadedTemplateDto.QUALITY_PROFILE_TYPE, templateKey(key), session) == 0; } - private static String templateKey(String language, String profileName) { - return StringUtils.lowerCase(language) + ":" + profileName; + private static String templateKey(QualityProfileKey key) { + return StringUtils.lowerCase(key.lang()) + ":" + key.name(); } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/persistence/ActiveRuleDao.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/persistence/ActiveRuleDao.java index 898593d0010..be56443888e 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/persistence/ActiveRuleDao.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/persistence/ActiveRuleDao.java @@ -71,17 +71,17 @@ public class ActiveRuleDao extends BaseDao ruleParams = ruleDao.findRuleParamsByRuleKey(ruleKey, session); - List activeRuleParams = newArrayList(); - for (RuleParamDto ruleParam : ruleParams) { - ActiveRuleParamDto activeRuleParam = ActiveRuleParamDto.createFor(ruleParam) - .setKey(ruleParam.getName()) - .setValue(ruleParam.getDefaultValue()); - activeRuleParams.add(activeRuleParam); - this.addParam(activeRule, activeRuleParam, session); - } + insert(activeRule, session); + +// List ruleParams = ruleDao.findRuleParamsByRuleKey(ruleKey, session); +// List activeRuleParams = newArrayList(); +// for (RuleParamDto ruleParam : ruleParams) { +// ActiveRuleParamDto activeRuleParam = ActiveRuleParamDto.createFor(ruleParam) +// .setKey(ruleParam.getName()) +// .setValue(ruleParam.getDefaultValue()); +// activeRuleParams.add(activeRuleParam); +// addParam(activeRule, activeRuleParam, session); +// } return activeRule; } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java index 6a4ffa443bf..da6e03b5be0 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ActiveRuleServiceMediumTest.java @@ -396,7 +396,6 @@ public class ActiveRuleServiceMediumTest { //TODO test params // verify es - ActiveRule activeRule = index.getByKey(key); assertThat(activeRule).isNull(); } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java index 0a92b1d0cfb..22611f3c2aa 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RegisterQualityProfilesMediumTest.java @@ -34,20 +34,20 @@ import org.sonar.api.server.rule.RuleParamType; import org.sonar.api.server.rule.RulesDefinition; import org.sonar.api.utils.ValidationMessages; import org.sonar.core.persistence.DbSession; -import org.sonar.core.persistence.MyBatis; import org.sonar.core.properties.PropertiesDao; import org.sonar.core.properties.PropertyDto; import org.sonar.core.properties.PropertyQuery; import org.sonar.core.qualityprofile.db.ActiveRuleDto; import org.sonar.core.qualityprofile.db.ActiveRuleKey; +import org.sonar.core.qualityprofile.db.ActiveRuleParamDto; import org.sonar.core.qualityprofile.db.QualityProfileDao; import org.sonar.core.qualityprofile.db.QualityProfileKey; -import org.sonar.core.rule.RuleDto; +import org.sonar.server.db.DbClient; import org.sonar.server.qualityprofile.persistence.ActiveRuleDao; -import org.sonar.server.rule2.persistence.RuleDao; import org.sonar.server.tester.ServerTester; import java.util.List; +import java.util.Map; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; @@ -57,11 +57,13 @@ public class RegisterQualityProfilesMediumTest { @org.junit.Rule public ServerTester serverTester = new ServerTester().addComponents(XooRulesDefinition.class, XooProfileDefinition.class); + DbClient dbClient; DbSession session; @Before - public void setup(){ - session = serverTester.get(MyBatis.class).openSession(false); + public void setup() { + dbClient = serverTester.get(DbClient.class); + session = dbClient.openSession(false); } @After @@ -70,35 +72,37 @@ public class RegisterQualityProfilesMediumTest { } @Test - public void register_profile_at_startup() throws Exception { + public void register_profile_definitions() throws Exception { + QualityProfileKey qualityProfileKey = QualityProfileKey.of("Basic", "xoo"); - QualityProfileKey qualityProfileKey = QualityProfileKey.of("Basic","xoo"); + // Check Profile in DB + QualityProfileDao qualityProfileDao = dbClient.qualityProfileDao(); + assertThat(qualityProfileDao.findAll(session)).hasSize(1); + assertThat(qualityProfileDao.getByKey(qualityProfileKey, session)).isNotNull(); - // 1. Check Profile in DB - QualityProfileDao qualityProfileDao = serverTester.get(QualityProfileDao.class); - assertThat(qualityProfileDao.selectAll()).hasSize(1); - assertThat(qualityProfileDao.selectByNameAndLanguage(qualityProfileKey.name(), qualityProfileKey.lang())) - .isNotNull(); - - // 2. Check Default Profile - PropertiesDao propertiesDao = serverTester.get(PropertiesDao.class); + // Check Default Profile + PropertiesDao propertiesDao = dbClient.propertiesDao(); List xooDefault = propertiesDao.selectByQuery(PropertyQuery.builder().setKey("sonar.profile.xoo").build(), session); assertThat(xooDefault).hasSize(1); assertThat(xooDefault.get(0).getValue()).isEqualTo("Basic"); - // 3. Check ActiveRules in DB - ActiveRuleDao activeRuleDao = serverTester.get(ActiveRuleDao.class); - + // Check ActiveRules in DB + ActiveRuleDao activeRuleDao = dbClient.activeRuleDao(); assertThat(activeRuleDao.findByProfileKey(qualityProfileKey, session)).hasSize(2); RuleKey ruleKey = RuleKey.of("xoo", "x1"); - RuleDto rule = serverTester.get(RuleDao.class).getByKey(ruleKey, session); - - - ActiveRuleDto activeRule = activeRuleDao.getByKey(ActiveRuleKey.of(qualityProfileKey,ruleKey), session); + ActiveRuleDto activeRule = activeRuleDao.getByKey(ActiveRuleKey.of(qualityProfileKey, ruleKey), session); assertThat(activeRule.getKey().qProfile()).isEqualTo(qualityProfileKey); - assertThat(activeRule.getKey().ruleKey()).isEqualTo(rule.getKey()); - assertThat(activeRule.getSeverityString()).isEqualToIgnoringCase("MAJOR"); + assertThat(activeRule.getKey().ruleKey()).isEqualTo(ruleKey); + assertThat(activeRule.getSeverityString()).isEqualTo(Severity.CRITICAL); + + // Check ActiveRuleParameters in DB + Map params = ActiveRuleParamDto.groupByKey(activeRuleDao.findParamsByActiveRule(activeRule, session)); + assertThat(params).hasSize(2); + // set by profile + assertThat(params.get("acceptWhitespace").getValue()).isEqualTo("true"); + // default value + assertThat(params.get("max").getValue()).isEqualTo("10"); } public static class XooProfileDefinition extends ProfileDefinition { @@ -107,10 +111,10 @@ public class RegisterQualityProfilesMediumTest { final RulesProfile profile = RulesProfile.create("Basic", "xoo"); ActiveRule activeRule1 = profile.activateRule( org.sonar.api.rules.Rule.create("xoo", "x1").setParams(newArrayList(new RuleParam().setKey("acceptWhitespace"))), - RulePriority.MAJOR); + RulePriority.CRITICAL); activeRule1.setParameter("acceptWhitespace", "true"); - profile.activateRule(org.sonar.api.rules.Rule.create("xoo", "x2"), RulePriority.BLOCKER); + profile.activateRule(org.sonar.api.rules.Rule.create("xoo", "x2"), RulePriority.INFO); return profile; } } @@ -119,24 +123,24 @@ public class RegisterQualityProfilesMediumTest { @Override public void define(Context context) { NewRepository repository = context.createRepository("xoo", "xoo").setName("Xoo Repo"); - repository.createRule("x1") + NewRule x1 = repository.createRule("x1") .setName("x1 name") .setHtmlDescription("x1 desc") - .setSeverity(Severity.MINOR) - .createParam("acceptWhitespace") + .setSeverity(Severity.MINOR); + x1.createParam("acceptWhitespace") .setDefaultValue("false") .setType(RuleParamType.BOOLEAN) .setDescription("Accept whitespaces on the line"); + x1.createParam("max") + .setDefaultValue("10") + .setType(RuleParamType.INTEGER) + .setDescription("Maximum"); repository.createRule("x2") .setName("x2 name") .setHtmlDescription("x2 desc") - .setSeverity(Severity.MAJOR); + .setSeverity(Severity.INFO); repository.done(); } } - - private QualityProfileKey profileKey(QProfile profile){ - return QualityProfileKey.of(profile.name(), profile.language()); - } } -- 2.39.5