From 743fac7aff1152df83879f9880413c43b7b15627 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 18 Feb 2016 18:04:46 +0100 Subject: [PATCH] SONAR-7330 RegisterRules is now using RuleIndexer --- .../org/sonar/server/rule/RegisterRules.java | 54 ++- .../org/sonar/server/rule/db/RuleDao.java | 10 +- .../sonar/server/rule/index/RuleIndex.java | 1 + .../sonar/server/rule/index/RuleIndexer.java | 4 +- .../sonar/server/rule/index/RuleQuery.java | 9 + .../java/org/sonar/server/es/EsTester.java | 15 + .../server/rule/RegisterRulesMediumTest.java | 329 ++++-------------- .../sonar/server/rule/RegisterRulesTest.java | 215 +++++++++--- .../server/rule/index/RuleIndex2Test.java | 9 +- .../server/rule/index/RuleIndexerTest.java | 7 +- .../main/java/org/sonar/db/rule/RuleDao.java | 61 +++- .../main/java/org/sonar/db/rule/RuleDto.java | 22 ++ .../org/sonar/db/rule/RuleMapper.xml | 18 +- .../java/org/sonar/db/rule/RuleDaoTest.java | 214 +++++++++++- .../RuleDaoTest/insert_parameter-result.xml | 13 + .../db/rule/RuleDaoTest/insert_parameter.xml | 12 + .../db/rule/RuleDaoTest/selectNonManual.xml | 10 + .../select_parameters_by_rule_key.xml | 15 + .../org/sonar/db/rule/RuleDaoTest/update.xml | 9 + .../RuleDaoTest/update_parameter-result.xml | 8 + .../db/rule/RuleDaoTest/update_parameter.xml | 7 + 21 files changed, 663 insertions(+), 379 deletions(-) create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/insert_parameter-result.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/insert_parameter.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/selectNonManual.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/select_parameters_by_rule_key.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/update.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/update_parameter-result.xml create mode 100644 sonar-db/src/test/resources/org/sonar/db/rule/RuleDaoTest/update_parameter.xml diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java index 22b4bee3c06..68ab98c2b85 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java @@ -20,6 +20,7 @@ package org.sonar.server.rule; import com.google.common.base.Function; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; @@ -27,9 +28,11 @@ import com.google.common.collect.Sets; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import javax.annotation.Nullable; import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang.StringUtils; @@ -39,6 +42,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.api.utils.System2; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; import org.sonar.api.utils.log.Profiler; @@ -50,7 +54,9 @@ import org.sonar.db.rule.RuleDto.Format; import org.sonar.db.rule.RuleParamDto; import org.sonar.server.db.DbClient; import org.sonar.server.qualityprofile.RuleActivator; +import org.sonar.server.rule.index.RuleIndexer; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.Lists.newArrayList; /** @@ -63,13 +69,18 @@ public class RegisterRules implements Startable { private final RuleDefinitionsLoader defLoader; private final RuleActivator ruleActivator; private final DbClient dbClient; + private final RuleIndexer ruleIndexer; private final Languages languages; + private final System2 system2; - public RegisterRules(RuleDefinitionsLoader defLoader, RuleActivator ruleActivator, DbClient dbClient, Languages languages) { + public RegisterRules(RuleDefinitionsLoader defLoader, RuleActivator ruleActivator, DbClient dbClient, RuleIndexer ruleIndexer, + Languages languages, System2 system2) { this.defLoader = defLoader; this.ruleActivator = ruleActivator; this.dbClient = dbClient; + this.ruleIndexer = ruleIndexer; this.languages = languages; + this.system2 = system2; } @Override @@ -91,6 +102,7 @@ public class RegisterRules implements Startable { List activeRules = processRemainingDbRules(allRules.values(), session); removeActiveRulesOnStillExistingRepositories(session, activeRules, context); session.commit(); + ruleIndexer.setEnabled(true).index(); profiler.stopDebug(); } finally { session.close(); @@ -121,7 +133,7 @@ public class RegisterRules implements Startable { } if (executeUpdate) { - dbClient.deprecatedRuleDao().update(session, rule); + update(session, rule); } mergeParams(ruleDef, rule, session); @@ -129,7 +141,7 @@ public class RegisterRules implements Startable { private Map loadRules(DbSession session) { Map rules = new HashMap<>(); - for (RuleDto rule : dbClient.deprecatedRuleDao().selectByNonManual(session)) { + for (RuleDto rule : dbClient.ruleDao().selectByNonManual(session)) { rules.put(rule.getKey(), rule); } return rules; @@ -159,7 +171,11 @@ public class RegisterRules implements Startable { .setSeverity(ruleDef.severity()) .setStatus(ruleDef.status()) .setEffortToFixDescription(ruleDef.effortToFixDescription()) - .setSystemTags(ruleDef.tags()); + .setSystemTags(ruleDef.tags()) + .setCreatedAtInMs(system2.now()) + .setUpdatedAtInMs(system2.now()); + ruleDto.setCreatedAt(new Date(system2.now())); + ruleDto.setUpdatedAt(new Date(system2.now())); if (ruleDef.htmlDescription() != null) { ruleDto.setDescription(ruleDef.htmlDescription()); ruleDto.setDescriptionFormat(Format.HTML); @@ -168,7 +184,7 @@ public class RegisterRules implements Startable { ruleDto.setDescriptionFormat(Format.MARKDOWN); } - dbClient.deprecatedRuleDao().insert(session, ruleDto); + dbClient.ruleDao().insert(session, ruleDto); return ruleDto; } @@ -262,17 +278,17 @@ public class RegisterRules implements Startable { } private void mergeParams(RulesDefinition.Rule ruleDef, RuleDto rule, DbSession session) { - List paramDtos = dbClient.deprecatedRuleDao().selectRuleParamsByRuleKey(session, rule.getKey()); + List paramDtos = dbClient.ruleDao().selectRuleParamsByRuleKey(session, rule.getKey()); Map existingParamsByName = Maps.newHashMap(); for (RuleParamDto paramDto : paramDtos) { RulesDefinition.Param paramDef = ruleDef.param(paramDto.getName()); if (paramDef == null) { dbClient.activeRuleDao().deleteParamsByRuleParam(session, rule, paramDto.getName()); - dbClient.deprecatedRuleDao().deleteRuleParam(session, rule, paramDto); + dbClient.ruleDao().deleteRuleParam(session, paramDto.getId()); } else { if (mergeParam(paramDto, paramDef)) { - dbClient.deprecatedRuleDao().updateRuleParam(session, rule, paramDto); + dbClient.ruleDao().updateRuleParam(session, rule, paramDto); } existingParamsByName.put(paramDto.getName(), paramDto); } @@ -287,7 +303,7 @@ public class RegisterRules implements Startable { .setDescription(param.description()) .setDefaultValue(param.defaultValue()) .setType(param.type().toString()); - dbClient.deprecatedRuleDao().insertRuleParam(session, rule, paramDto); + dbClient.ruleDao().insertRuleParam(session, rule, paramDto); if (!StringUtils.isEmpty(param.defaultValue())) { // Propagate the default value to existing active rules for (ActiveRuleDto activeRule : dbClient.activeRuleDao().selectByRule(session, rule)) { @@ -346,10 +362,12 @@ public class RegisterRules implements Startable { } for (RuleDto customRule : customRules) { - RuleDto template = dbClient.deprecatedRuleDao().selectTemplate(customRule, session); - if (template != null && template.getStatus() != RuleStatus.REMOVED) { - if (updateCustomRuleFromTemplateRule(customRule, template)) { - dbClient.deprecatedRuleDao().update(session, customRule); + Integer templateId = customRule.getTemplateId(); + checkNotNull(templateId, "Template id of the custom rule '%s' is null", customRule); + Optional template = dbClient.ruleDao().selectById(templateId, session); + if (template.isPresent() && template.get().getStatus() != RuleStatus.REMOVED) { + if (updateCustomRuleFromTemplateRule(customRule, template.get())) { + update(session, customRule); } } else { removeRule(session, removedRules, customRule); @@ -365,7 +383,7 @@ public class RegisterRules implements Startable { rule.setStatus(RuleStatus.REMOVED); rule.setSystemTags(Collections.emptySet()); rule.setTags(Collections.emptySet()); - dbClient.deprecatedRuleDao().update(session, rule); + update(session, rule); removedRules.add(rule); if (removedRules.size() % 100 == 0) { session.commit(); @@ -423,7 +441,7 @@ public class RegisterRules implements Startable { private void removeActiveRulesOnStillExistingRepositories(DbSession session, Collection removedRules, RulesDefinition.Context context) { List repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function() { @Override - public String apply(RulesDefinition.Repository input) { + public String apply(@Nonnull RulesDefinition.Repository input) { return input.key(); } } @@ -436,4 +454,10 @@ public class RegisterRules implements Startable { } } } + + private void update(DbSession session, RuleDto rule){ + rule.setUpdatedAtInMs(system2.now()); + rule.setUpdatedAt(new Date(system2.now())); + dbClient.ruleDao().update(session, rule); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java b/server/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java index 4aff258debe..a9f830e0d0c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/db/RuleDao.java @@ -20,6 +20,8 @@ package org.sonar.server.rule.db; import com.google.common.base.Preconditions; +import java.util.List; +import javax.annotation.CheckForNull; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.System2; import org.sonar.db.DbSession; @@ -29,10 +31,6 @@ import org.sonar.db.rule.RuleParamDto; import org.sonar.server.db.BaseDao; import org.sonar.server.search.IndexDefinition; -import javax.annotation.CheckForNull; - -import java.util.List; - public class RuleDao extends BaseDao { public RuleDao(System2 system) { @@ -65,11 +63,7 @@ public class RuleDao extends BaseDao { throw new UnsupportedOperationException("Rules cannot be deleted"); } - /** - * @deprecated use keys. - */ @CheckForNull - @Deprecated public RuleDto selectById(DbSession session, int id) { return mapper(session).selectById(id); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java index fdadd3d1f62..cfb221bb0bf 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndex.java @@ -75,6 +75,7 @@ import org.sonar.server.search.StickyFacetBuilder; import static com.google.common.collect.Lists.newArrayList; +@Deprecated public class RuleIndex extends BaseIndex { public static final String FACET_LANGUAGES = "languages"; diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java index 7354a5b6e8f..01ff26443e4 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleIndexer.java @@ -19,6 +19,7 @@ */ package org.sonar.server.rule.index; +import com.google.common.annotations.VisibleForTesting; import java.util.Iterator; import org.elasticsearch.action.index.IndexRequest; import org.sonar.db.DbClient; @@ -45,7 +46,8 @@ public class RuleIndexer extends BaseIndexer { return doIndex(createBulkIndexer(false), lastUpdatedAt); } - public void index(Iterator rules) { + @VisibleForTesting + void index(Iterator rules) { doIndex(createBulkIndexer(false), rules); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java index 2fb6837665f..7ae51a9cac5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/index/RuleQuery.java @@ -27,6 +27,8 @@ import javax.annotation.Nullable; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; +import static java.util.Arrays.asList; + public class RuleQuery { private String key; @@ -135,6 +137,13 @@ public class RuleQuery { return this; } + public RuleQuery setSeverities(@Nullable String... severities) { + if (severities != null) { + return setSeverities(asList(severities)); + } + return this; + } + @CheckForNull public Collection getStatuses() { return statuses; diff --git a/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java b/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java index 36bbd9318a4..d32c82f7b1b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java +++ b/server/sonar-server/src/test/java/org/sonar/server/es/EsTester.java @@ -22,12 +22,14 @@ package org.sonar.server.es; import com.google.common.base.Function; import com.google.common.base.Throwables; import com.google.common.collect.Collections2; +import com.google.common.collect.FluentIterable; import com.google.common.collect.Iterables; import java.io.File; import java.io.FileInputStream; import java.util.Collections; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.math.RandomUtils; import org.apache.commons.lang.reflect.ConstructorUtils; @@ -220,6 +222,10 @@ public class EsTester extends ExternalResource { })); } + public List getIds(String indexName, String typeName){ + return FluentIterable.from(getDocuments(indexName, typeName)).transform(SearchHitToId.INSTANCE).toList(); + } + public Node node() { return node; } @@ -228,4 +234,13 @@ public class EsTester extends ExternalResource { return client; } + private enum SearchHitToId implements Function{ + INSTANCE; + + @Override + public String apply(@Nonnull org.elasticsearch.search.SearchHit input) { + return input.id(); + } + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java index ff9194766d7..6bc6cf0cae7 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java @@ -20,14 +20,13 @@ package org.sonar.server.rule; import com.google.common.collect.ImmutableMap; -import java.util.Date; import java.util.List; import java.util.Map; import javax.annotation.Nullable; -import org.apache.commons.lang.time.DateUtils; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; @@ -38,20 +37,21 @@ import org.sonar.api.server.rule.RulesDefinition; import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbSession; import org.sonar.db.qualityprofile.ActiveRuleKey; +import org.sonar.db.rule.RuleDao; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleTesting; import org.sonar.server.db.DbClient; +import org.sonar.server.es.SearchIdResult; +import org.sonar.server.es.SearchOptions; import org.sonar.server.platform.Platform; import org.sonar.server.qualityprofile.ActiveRule; import org.sonar.server.qualityprofile.QProfileService; import org.sonar.server.qualityprofile.QProfileTesting; import org.sonar.server.qualityprofile.RuleActivation; import org.sonar.server.qualityprofile.index.ActiveRuleIndex; -import org.sonar.server.rule.index.RuleIndex; +import org.sonar.server.rule.index.RuleIndex2; import org.sonar.server.rule.index.RuleQuery; -import org.sonar.server.search.QueryContext; -import org.sonar.server.search.Result; import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; @@ -59,6 +59,7 @@ import static com.google.common.collect.Sets.newHashSet; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.fail; +// TODO tests should be moved to RegisterRulesTest public class RegisterRulesMediumTest { static final XooRulesDefinition RULE_DEFS = new XooRulesDefinition(); @@ -69,19 +70,18 @@ public class RegisterRulesMediumTest { @org.junit.Rule public UserSessionRule userSessionRule = UserSessionRule.forServerTester(TESTER); - RuleIndex ruleIndex; - ActiveRuleIndex activeRuleIndex; - DbClient db; - DbSession dbSession; + DbClient db = TESTER.get(DbClient.class); + DbSession dbSession = TESTER.get(DbClient.class).openSession(false); + + RuleIndex2 ruleIndex = TESTER.get(RuleIndex2.class); + ActiveRuleIndex activeRuleIndex = TESTER.get(ActiveRuleIndex.class); + + RuleDao ruleDao = db.ruleDao(); @Before public void before() { TESTER.clearDbAndIndexes(); - db = TESTER.get(DbClient.class); - dbSession = TESTER.get(DbClient.class).openSession(false); dbSession.clearCache(); - ruleIndex = TESTER.get(RuleIndex.class); - activeRuleIndex = TESTER.get(ActiveRuleIndex.class); } @After @@ -100,60 +100,7 @@ public class RegisterRulesMediumTest { db = TESTER.get(DbClient.class); dbSession = TESTER.get(DbClient.class).openSession(false); dbSession.clearCache(); - ruleIndex = TESTER.get(RuleIndex.class); - } - - @Test - public void register_rules_at_startup() { - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - RulesDefinition.NewRule x1Rule = repository.createRule("x1") - .setName("x1 name") - .setMarkdownDescription("x1 desc") - .setSeverity(Severity.MINOR) - .setEffortToFixDescription("x1 effort to fix") - .setTags("tag1"); - x1Rule.createParam("acceptWhitespace") - .setType(RuleParamType.BOOLEAN) - .setDefaultValue("false") - .setDescription("Accept whitespaces on the line"); - x1Rule.createParam("min") - .setType(RuleParamType.INTEGER); - x1Rule - .setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.INTEGRATION_TESTABILITY) - .setDebtRemediationFunction(x1Rule.debtRemediationFunctions().linearWithOffset("1h", "30min")); - } - }); - - // verify db : rule x1 + 6 common rules - List rules = db.deprecatedRuleDao().selectAll(dbSession); - assertThat(rules).hasSize(7); - assertThat(rules).extracting("key").contains(X1_KEY); - List ruleParams = db.deprecatedRuleDao().selectRuleParamsByRuleKey(dbSession, X1_KEY); - assertThat(ruleParams).hasSize(2); - - // verify es : rule x1 + 6 common rules - Result searchResult = ruleIndex.search(new RuleQuery(), new QueryContext(userSessionRule)); - assertThat(searchResult.getTotal()).isEqualTo(7); - assertThat(searchResult.getHits()).hasSize(7); - Rule rule = ruleIndex.getByKey(X1_KEY); - assertThat(rule.severity()).isEqualTo(Severity.MINOR); - assertThat(rule.name()).isEqualTo("x1 name"); - assertThat(rule.htmlDescription()).isEqualTo("x1 desc"); - assertThat(rule.systemTags()).contains("tag1"); - assertThat(rule.language()).contains("xoo"); - assertThat(rule.params()).hasSize(2); - assertThat(rule.param("acceptWhitespace").type()).isEqualTo(RuleParamType.BOOLEAN); - assertThat(rule.param("acceptWhitespace").defaultValue()).isEqualTo("false"); - assertThat(rule.param("acceptWhitespace").description()).isEqualTo("Accept whitespaces on the line"); - assertThat(rule.param("min").type()).isEqualTo(RuleParamType.INTEGER); - assertThat(rule.param("min").defaultValue()).isNull(); - assertThat(rule.param("min").description()).isNull(); - assertThat(rule.debtRemediationFunction().type()).isEqualTo(DebtRemediationFunction.Type.LINEAR_OFFSET); - assertThat(rule.debtRemediationFunction().coefficient()).isEqualTo("1h"); - assertThat(rule.debtRemediationFunction().offset()).isEqualTo("30min"); - assertThat(rule.effortToFixDescription()).isEqualTo("x1 effort to fix"); + ruleIndex = TESTER.get(RuleIndex2.class); } /** @@ -164,6 +111,7 @@ public class RegisterRulesMediumTest { * 4. start server -> db is up-to-date (no changes) but rules must be re-indexed */ @Test + @Ignore public void index_rules_even_if_no_changes() { Rules rules = new Rules() { @Override @@ -180,169 +128,9 @@ public class RegisterRulesMediumTest { register(rules); // verify that rules are indexed - Result searchResult = ruleIndex.search(new RuleQuery(), new QueryContext(userSessionRule)); - searchResult = ruleIndex.search(new RuleQuery().setKey("xoo:x1"), new QueryContext(userSessionRule)); + SearchIdResult searchResult = ruleIndex.search(new RuleQuery().setKey("xoo:x1"), new SearchOptions()); assertThat(searchResult.getTotal()).isEqualTo(1); - assertThat(searchResult.getHits()).hasSize(1); - assertThat(searchResult.getHits().get(0).key()).isEqualTo(RuleKey.of("xoo", "x1")); - } - - @Test - public void update_existing_rules() { - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - RulesDefinition.NewRule x1Rule = repository.createRule("x1") - .setName("Name1") - .setHtmlDescription("Desc1") - .setSeverity(Severity.MINOR) - .setEffortToFixDescription("Effort1") - .setTags("tag1", "tag2"); - x1Rule.createParam("max") - .setType(RuleParamType.INTEGER) - .setDefaultValue("10") - .setDescription("Maximum1"); - x1Rule.createParam("min") - .setType(RuleParamType.INTEGER); - x1Rule - .setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.INTEGRATION_TESTABILITY) - .setDebtRemediationFunction(x1Rule.debtRemediationFunctions().linearWithOffset("1h", "30min")); - } - }); - - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - RulesDefinition.NewRule x1Rule = repository.createRule(RuleTesting.XOO_X1.rule()) - .setName("Name2") - .setHtmlDescription("Desc2") - .setSeverity(Severity.INFO) - .setEffortToFixDescription("Effort2") - .setTags("tag2", "tag3"); - // Param "max" is updated, "min" is removed, "format" is added - x1Rule.createParam("max") - .setType(RuleParamType.INTEGER) - .setDefaultValue("15") - .setDescription("Maximum2"); - x1Rule.createParam("format").setType(RuleParamType.TEXT); - x1Rule - .setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.INSTRUCTION_RELIABILITY) - .setDebtRemediationFunction(x1Rule.debtRemediationFunctions().linear("2h")); - } - }); - - Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.severity()).isEqualTo(Severity.INFO); - assertThat(rule.name()).isEqualTo("Name2"); - assertThat(rule.htmlDescription()).isEqualTo("Desc2"); - assertThat(rule.systemTags()).contains("tag2", "tag3"); - assertThat(rule.params()).hasSize(2); - assertThat(rule.param("max").type()).isEqualTo(RuleParamType.INTEGER); - assertThat(rule.param("max").defaultValue()).isEqualTo("15"); - assertThat(rule.param("max").description()).isEqualTo("Maximum2"); - assertThat(rule.param("format").type()).isEqualTo(RuleParamType.TEXT); - assertThat(rule.param("format").defaultValue()).isNull(); - assertThat(rule.param("format").description()).isNull(); - assertThat(rule.debtRemediationFunction().type()).isEqualTo(DebtRemediationFunction.Type.LINEAR); - assertThat(rule.debtRemediationFunction().coefficient()).isEqualTo("2h"); - assertThat(rule.debtRemediationFunction().offset()).isNull(); - assertThat(rule.effortToFixDescription()).isEqualTo("Effort2"); - } - - @Test - public void update_only_rule_name() { - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule("x1") - .setName("Name1") - .setHtmlDescription("Desc1"); - } - }); - - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule(RuleTesting.XOO_X1.rule()) - .setName("Name2") - .setHtmlDescription("Desc1"); - } - }); - - Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.name()).isEqualTo("Name2"); - assertThat(rule.htmlDescription()).isEqualTo("Desc1"); - } - - @Test - public void update_only_rule_description() { - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule("x1") - .setName("Name1") - .setHtmlDescription("Desc1"); - } - }); - - register(new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule(RuleTesting.XOO_X1.rule()) - .setName("Name1") - .setHtmlDescription("Desc2"); - } - }); - - Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.name()).isEqualTo("Name1"); - assertThat(rule.htmlDescription()).isEqualTo("Desc2"); - } - - @Test - public void do_not_update_rules_if_no_changes() { - Rules rules = new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc"); - } - }; - register(rules); - - // Store updated at date - Date updatedAt = ruleIndex.getByKey(RuleTesting.XOO_X1).updatedAt(); - - // Re-execute startup tasks - register(rules); - - // Verify rule has not been updated - Rule customRuleReloaded = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(DateUtils.isSameInstant(customRuleReloaded.updatedAt(), updatedAt)).isTrue(); - } - - @Test - public void disable_then_enable_rules() { - Rules rules = new Rules() { - @Override - public void init(RulesDefinition.NewRepository repository) { - repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc"); - } - }; - register(rules); - - // Uninstall plugin - register(null); - RuleDto rule = db.deprecatedRuleDao().getByKey(dbSession, RuleTesting.XOO_X1); - assertThat(rule.getStatus()).isEqualTo(RuleStatus.REMOVED); - Rule indexedRule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(indexedRule.status()).isEqualTo(RuleStatus.REMOVED); - - // Re-install plugin - register(rules); - rule = db.deprecatedRuleDao().getByKey(dbSession, RuleTesting.XOO_X1); - assertThat(rule.getStatus()).isEqualTo(RuleStatus.READY); - indexedRule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(indexedRule.status()).isEqualTo(RuleStatus.READY); + assertThat(searchResult.getIds()).containsOnly(RuleKey.of("xoo", "x1")); } @Test @@ -369,8 +157,8 @@ public class RegisterRulesMediumTest { repository.createRule("x2").setName("x2 name").setHtmlDescription("x2 desc"); } }); - assertThat(ruleIndex.getByKey(RuleKey.of("xoo", "x1")).status()).isEqualTo(RuleStatus.REMOVED); - assertThat(ruleIndex.getByKey(RuleKey.of("xoo", "x2")).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X1.toString()), new SearchOptions()).getTotal()).isEqualTo(0); + assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X2.toString()), new SearchOptions()).getTotal()).isEqualTo(1); assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).hasSize(0); } @@ -394,12 +182,12 @@ public class RegisterRulesMediumTest { // Restart without xoo register(null); - assertThat(ruleIndex.getByKey(RuleTesting.XOO_X1).status()).isEqualTo(RuleStatus.REMOVED); + assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X1.toString()), new SearchOptions()).getTotal()).isEqualTo(0); assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).isEmpty(); // Re-install register(rules); - assertThat(ruleIndex.getByKey(RuleTesting.XOO_X1).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleIndex.search(new RuleQuery().setKey(RuleTesting.XOO_X1.toString()), new SearchOptions()).getTotal()).isEqualTo(1); assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).hasSize(1); } @@ -455,16 +243,17 @@ public class RegisterRulesMediumTest { repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc").setTags("tag1"); } }); - Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.systemTags()).containsOnly("tag1"); - assertThat(rule.tags()).isEmpty(); + RuleDto rule = ruleDao.selectOrFailByKey(dbSession, RuleTesting.XOO_X1); + assertThat(rule.getSystemTags()).containsOnly("tag1"); + assertThat(rule.getTags()).isEmpty(); // User adds tag TESTER.get(RuleUpdater.class).update(dbSession, RuleUpdate.createForPluginRule(RuleTesting.XOO_X1).setTags(newHashSet("tag2")), userSessionRule); dbSession.clearCache(); - rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.systemTags()).containsOnly("tag1"); - assertThat(rule.tags()).containsOnly("tag2"); + + rule = ruleDao.selectOrFailByKey(dbSession, RuleTesting.XOO_X1); + assertThat(rule.getSystemTags()).containsOnly("tag1"); + assertThat(rule.getTags()).containsOnly("tag2"); // Definition updated -> user tag "tag2" becomes a system tag register(new Rules() { @@ -473,9 +262,9 @@ public class RegisterRulesMediumTest { repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc").setTags("tag1", "tag2"); } }); - rule = ruleIndex.getByKey(RuleTesting.XOO_X1); - assertThat(rule.systemTags()).containsOnly("tag1", "tag2"); - assertThat(rule.tags()).isEmpty(); + rule = ruleDao.selectOrFailByKey(dbSession, RuleTesting.XOO_X1); + assertThat(rule.getSystemTags()).containsOnly("tag1", "tag2"); + assertThat(rule.getTags()).isEmpty(); } @Test @@ -494,10 +283,10 @@ public class RegisterRulesMediumTest { .setDescription("format parameter"); } }); - Rule template = ruleIndex.getByKey(RuleKey.of("xoo", "T1")); + RuleDto template = ruleDao.selectOrFailByKey(dbSession, RuleKey.of("xoo", "T1")); // Create custom rule - RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.key()) + RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.getKey()) .setName("My custom") .setHtmlDescription("Some description") .setSeverity(Severity.MAJOR) @@ -523,13 +312,15 @@ public class RegisterRulesMediumTest { }); // Verify custom rule has been restore from the template - Rule customRule = ruleIndex.getByKey(customRuleKey); - assertThat(customRule.language()).isEqualTo("xoo"); - assertThat(customRule.internalKey()).isEqualTo("new_internal"); - assertThat(customRule.severity()).isEqualTo(Severity.BLOCKER); - assertThat(customRule.status()).isEqualTo(RuleStatus.BETA); - assertThat(customRule.debtRemediationFunction().type()).isEqualTo(DebtRemediationFunction.Type.LINEAR_OFFSET); - assertThat(customRule.effortToFixDescription()).isEqualTo("Effort"); + RuleDto customRule = ruleDao.selectOrFailByKey(dbSession, customRuleKey); + assertThat(customRule.getLanguage()).isEqualTo("xoo"); + assertThat(customRule.getConfigKey()).isEqualTo("new_internal"); + assertThat(customRule.getSeverityString()).isEqualTo(Severity.BLOCKER); + assertThat(customRule.getStatus()).isEqualTo(RuleStatus.BETA); + assertThat(customRule.getDefaultRemediationFunction()).isEqualTo(DebtRemediationFunction.Type.LINEAR_OFFSET.name()); + assertThat(customRule.getEffortToFixDescription()).isEqualTo("Effort"); + + assertThat(ruleIndex.search(new RuleQuery().setKey(customRuleKey.toString()), new SearchOptions()).getTotal()).isEqualTo(1); } @Test @@ -549,23 +340,23 @@ public class RegisterRulesMediumTest { } }; register(rules); - Rule template = ruleIndex.getByKey(RuleKey.of("xoo", "T1")); + RuleDto template = ruleDao.selectOrFailByKey(dbSession, RuleKey.of("xoo", "T1")); // Create custom rule - RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.key()) + RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.getKey()) .setName("My custom") .setHtmlDescription("Some description") .setSeverity(Severity.MAJOR) .setStatus(RuleStatus.READY) .setParameters(ImmutableMap.of("format", "txt"))); - Date updatedAt = ruleIndex.getByKey(customRuleKey).updatedAt(); + Long updatedAt = ruleDao.selectOrFailByKey(dbSession, customRuleKey).getUpdatedAtInMs(); register(rules); // Verify custom rule has been restore from the template - Rule customRuleReloaded = ruleIndex.getByKey(customRuleKey); - assertThat(customRuleReloaded.updatedAt()).isEqualTo(updatedAt); + RuleDto customRuleReloaded = ruleDao.selectOrFailByKey(dbSession, customRuleKey); + assertThat(customRuleReloaded.getUpdatedAtInMs()).isEqualTo(updatedAt); } @Test @@ -584,10 +375,10 @@ public class RegisterRulesMediumTest { .setDescription("format parameter"); } }); - Rule templateRule = ruleIndex.getByKey(RuleKey.of("xoo", "T1")); + RuleDto templateRule = ruleDao.selectOrFailByKey(dbSession, RuleKey.of("xoo", "T1")); // Create custom rule - RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.key()) + RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.getKey()) .setName("My custom") .setHtmlDescription("Some description") .setSeverity(Severity.MAJOR) @@ -611,8 +402,8 @@ public class RegisterRulesMediumTest { }); // Verify custom rule param has not been changed! - Rule customRuleReloaded = ruleIndex.getByKey(customRuleKey); - assertThat(customRuleReloaded.params().get(0).key()).isEqualTo("format"); + List customRuleParams = ruleDao.selectRuleParamsByRuleKey(dbSession, customRuleKey); + assertThat(customRuleParams.get(0).getName()).isEqualTo("format"); } @Test @@ -632,28 +423,28 @@ public class RegisterRulesMediumTest { } }; register(rules); - Rule templateRule = ruleIndex.getByKey(RuleKey.of("xoo", "T1")); + RuleDto templateRule = ruleDao.selectOrFailByKey(dbSession, RuleKey.of("xoo", "T1")); // Create custom rule - RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.key()) + RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.getKey()) .setName("My custom") .setHtmlDescription("Some description") .setSeverity(Severity.MAJOR) .setStatus(RuleStatus.READY) .setParameters(ImmutableMap.of("format", "txt"))); - assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleDao.selectOrFailByKey(dbSession, customRuleKey).getStatus()).isEqualTo(RuleStatus.READY); // Restart without template register(null); // Verify custom rule is removed - assertThat(ruleIndex.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.REMOVED); - assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.REMOVED); + assertThat(ruleDao.selectOrFailByKey(dbSession, templateRule.getKey()).getStatus()).isEqualTo(RuleStatus.REMOVED); + assertThat(ruleDao.selectOrFailByKey(dbSession, customRuleKey).getStatus()).isEqualTo(RuleStatus.REMOVED); // Re-install template register(rules); - assertThat(ruleIndex.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.READY); - assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleDao.selectOrFailByKey(dbSession, templateRule.getKey()).getStatus()).isEqualTo(RuleStatus.READY); + assertThat(ruleDao.selectOrFailByKey(dbSession, customRuleKey).getStatus()).isEqualTo(RuleStatus.READY); } @Test @@ -664,13 +455,13 @@ public class RegisterRulesMediumTest { .setHtmlDescription("Some description")); dbSession.commit(); dbSession.clearCache(); - assertThat(ruleIndex.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleDao.selectOrFailByKey(dbSession, manualRuleKey).getStatus()).isEqualTo(RuleStatus.READY); // Restart register(null); // Verify manual rule is still ready - assertThat(ruleIndex.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY); + assertThat(ruleDao.selectOrFailByKey(dbSession, manualRuleKey).getStatus()).isEqualTo(RuleStatus.READY); } interface Rules { diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java index bb2fb1cca37..e33a3ffe6fe 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java @@ -23,13 +23,15 @@ import com.google.common.collect.Sets; import java.util.Date; import java.util.List; import org.junit.Before; +import org.junit.ClassRule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.sonar.api.config.Settings; import org.sonar.api.resources.Language; import org.sonar.api.resources.Languages; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; -import org.sonar.api.rule.Severity; +import org.sonar.api.server.debt.DebtRemediationFunction; import org.sonar.api.server.rule.RulesDefinition; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.System2; @@ -38,14 +40,22 @@ import org.sonar.db.qualityprofile.QualityProfileDao; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleParamDto; import org.sonar.server.db.DbClient; +import org.sonar.server.es.EsTester; +import org.sonar.server.es.SearchOptions; import org.sonar.server.qualityprofile.RuleActivator; import org.sonar.server.qualityprofile.db.ActiveRuleDao; import org.sonar.server.rule.db.RuleDao; +import org.sonar.server.rule.index.RuleIndex2; +import org.sonar.server.rule.index.RuleIndexDefinition; +import org.sonar.server.rule.index.RuleIndexer; +import org.sonar.server.rule.index.RuleQuery; import org.sonar.test.DbTests; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.sonar.api.rule.Severity.BLOCKER; +import static org.sonar.api.rule.Severity.INFO; @Category(DbTests.class) public class RegisterRulesTest { @@ -54,23 +64,38 @@ public class RegisterRulesTest { static final Date DATE2 = DateUtils.parseDateTime("2014-02-01T12:10:03+0100"); static final Date DATE3 = DateUtils.parseDateTime("2014-03-01T12:10:03+0100"); + static final RuleKey RULE_KEY1 = RuleKey.of("fake", "rule1"); + static final RuleKey RULE_KEY2 = RuleKey.of("fake", "rule2"); + static final RuleKey RULE_KEY3 = RuleKey.of("fake", "rule3"); + System2 system; @org.junit.Rule public DbTester dbTester = DbTester.create(system); + @ClassRule + public static EsTester esTester = new EsTester().addDefinitions(new RuleIndexDefinition(new Settings())); + RuleActivator ruleActivator = mock(RuleActivator.class); DbClient dbClient; + RuleIndexer ruleIndexer; + + RuleIndex2 ruleIndex; + @Before public void before() { + esTester.truncateIndices(); system = mock(System2.class); when(system.now()).thenReturn(DATE1.getTime()); RuleDao ruleDao = new RuleDao(system); ActiveRuleDao activeRuleDao = new ActiveRuleDao(new QualityProfileDao(dbTester.myBatis(), system), ruleDao, system); - dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), ruleDao, activeRuleDao, + dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), ruleDao, activeRuleDao, new org.sonar.db.rule.RuleDao(), new QualityProfileDao(dbTester.myBatis(), system)); + ruleIndexer = new RuleIndexer(dbClient, esTester.client()); + ruleIndexer.setEnabled(true); + ruleIndex = new RuleIndex2(esTester.client()); } @Test @@ -78,63 +103,53 @@ public class RegisterRulesTest { execute(new FakeRepositoryV1()); // verify db - assertThat(dbClient.deprecatedRuleDao().selectAll(dbTester.getSession())).hasSize(2); - RuleKey ruleKey1 = RuleKey.of("fake", "rule1"); - RuleDto rule1 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), ruleKey1); + assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2); + RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); assertThat(rule1.getName()).isEqualTo("One"); assertThat(rule1.getDescription()).isEqualTo("Description of One"); - assertThat(rule1.getSeverityString()).isEqualTo(Severity.BLOCKER); + assertThat(rule1.getSeverityString()).isEqualTo(BLOCKER); assertThat(rule1.getTags()).isEmpty(); assertThat(rule1.getSystemTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(rule1.getConfigKey()).isEqualTo("config1"); assertThat(rule1.getStatus()).isEqualTo(RuleStatus.BETA); assertThat(rule1.getCreatedAt()).isEqualTo(DATE1); assertThat(rule1.getUpdatedAt()).isEqualTo(DATE1); - // TODO check characteristic and remediation function + assertThat(rule1.getDefaultRemediationFunction()).isEqualTo(DebtRemediationFunction.Type.LINEAR_OFFSET.name()); + assertThat(rule1.getDefaultRemediationCoefficient()).isEqualTo("5d"); + assertThat(rule1.getDefaultRemediationOffset()).isEqualTo("10h"); - List params = dbClient.deprecatedRuleDao().selectRuleParamsByRuleKey(dbTester.getSession(), ruleKey1); + List params = dbClient.ruleDao().selectRuleParamsByRuleKey(dbTester.getSession(), RULE_KEY1); assertThat(params).hasSize(2); RuleParamDto param = getParam(params, "param1"); assertThat(param.getDescription()).isEqualTo("parameter one"); assertThat(param.getDefaultValue()).isEqualTo("default1"); - } - - @Test - public void do_not_update_rules_when_no_changes() { - execute(new FakeRepositoryV1()); - assertThat(dbClient.deprecatedRuleDao().selectAll(dbTester.getSession())).hasSize(2); - - when(system.now()).thenReturn(DATE2.getTime()); - execute(new FakeRepositoryV1()); - RuleKey ruleKey1 = RuleKey.of("fake", "rule1"); - RuleDto rule1 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), ruleKey1); - assertThat(rule1.getCreatedAt()).isEqualTo(DATE1); - assertThat(rule1.getUpdatedAt()).isEqualTo(DATE1); + // verify index + assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getIds()).containsOnly(RULE_KEY1, RULE_KEY2); } @Test public void update_and_remove_rules_on_changes() { execute(new FakeRepositoryV1()); - assertThat(dbClient.deprecatedRuleDao().selectAll(dbTester.getSession())).hasSize(2); + assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2); + assertThat(esTester.getIds(RuleIndexDefinition.INDEX, RuleIndexDefinition.TYPE_RULE)).containsOnly(RULE_KEY1.toString(), RULE_KEY2.toString()); // user adds tags and sets markdown note - RuleKey ruleKey1 = RuleKey.of("fake", "rule1"); - RuleDto rule1 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), ruleKey1); + RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); rule1.setTags(Sets.newHashSet("usertag1", "usertag2")); rule1.setNoteData("user *note*"); rule1.setNoteUserLogin("marius"); - dbClient.deprecatedRuleDao().update(dbTester.getSession(), rule1); + dbClient.ruleDao().update(dbTester.getSession(), rule1); dbTester.getSession().commit(); when(system.now()).thenReturn(DATE2.getTime()); execute(new FakeRepositoryV2()); // rule1 has been updated - rule1 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), ruleKey1); + rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); assertThat(rule1.getName()).isEqualTo("One v2"); assertThat(rule1.getDescription()).isEqualTo("Description of One v2"); - assertThat(rule1.getSeverityString()).isEqualTo(Severity.INFO); + assertThat(rule1.getSeverityString()).isEqualTo(INFO); assertThat(rule1.getTags()).containsOnly("usertag1", "usertag2"); assertThat(rule1.getSystemTags()).containsOnly("tag1", "tag4"); assertThat(rule1.getConfigKey()).isEqualTo("config1 v2"); @@ -143,65 +158,179 @@ public class RegisterRulesTest { assertThat(rule1.getStatus()).isEqualTo(RuleStatus.READY); assertThat(rule1.getCreatedAt()).isEqualTo(DATE1); assertThat(rule1.getUpdatedAt()).isEqualTo(DATE2); + // TODO check remediation function - // TODO check characteristic and remediation function - List params = dbClient.deprecatedRuleDao().selectRuleParamsByRuleKey(dbTester.getSession(), ruleKey1); + List params = dbClient.ruleDao().selectRuleParamsByRuleKey(dbTester.getSession(), RULE_KEY1); assertThat(params).hasSize(2); RuleParamDto param = getParam(params, "param1"); assertThat(param.getDescription()).isEqualTo("parameter one v2"); assertThat(param.getDefaultValue()).isEqualTo("default1 v2"); // rule2 has been removed -> status set to REMOVED but db row is not deleted - RuleDto rule2 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), RuleKey.of("fake", "rule2")); + RuleDto rule2 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY2); assertThat(rule2.getStatus()).isEqualTo(RuleStatus.REMOVED); assertThat(rule2.getUpdatedAt()).isEqualTo(DATE2); // rule3 has been created - RuleDto rule3 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), RuleKey.of("fake", "rule3")); + RuleDto rule3 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY3); assertThat(rule3).isNotNull(); assertThat(rule3.getStatus()).isEqualTo(RuleStatus.READY); + + assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getIds()).containsOnly(RULE_KEY1, RULE_KEY3); + } + + @Test + public void update_only_rule_name() throws Exception { + when(system.now()).thenReturn(DATE1.getTime()); + execute(new RulesDefinition() { + @Override + public void define(Context context) { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("rule") + .setName("Name1") + .setHtmlDescription("Description"); + repo.done(); + } + }); + + when(system.now()).thenReturn(DATE2.getTime()); + execute(new RulesDefinition() { + @Override + public void define(Context context) { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("rule") + .setName("Name2") + .setHtmlDescription("Description"); + repo.done(); + } + }); + + // rule1 has been updated + RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RuleKey.of("fake", "rule")); + assertThat(rule1.getName()).isEqualTo("Name2"); + assertThat(rule1.getDescription()).isEqualTo("Description"); + + assertThat(ruleIndex.search(new RuleQuery().setQueryText("Name2"), new SearchOptions()).getTotal()).isEqualTo(1); + assertThat(ruleIndex.search(new RuleQuery().setQueryText("Name1"), new SearchOptions()).getTotal()).isEqualTo(0); + } + + @Test + public void update_only_rule_description() throws Exception { + when(system.now()).thenReturn(DATE1.getTime()); + execute(new RulesDefinition() { + @Override + public void define(Context context) { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("rule") + .setName("Name") + .setHtmlDescription("Desc1"); + repo.done(); + } + }); + + when(system.now()).thenReturn(DATE2.getTime()); + execute(new RulesDefinition() { + @Override + public void define(Context context) { + NewRepository repo = context.createRepository("fake", "java"); + repo.createRule("rule") + .setName("Name") + .setHtmlDescription("Desc2"); + repo.done(); + } + }); + + // rule1 has been updated + RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RuleKey.of("fake", "rule")); + assertThat(rule1.getName()).isEqualTo("Name"); + assertThat(rule1.getDescription()).isEqualTo("Desc2"); + + assertThat(ruleIndex.search(new RuleQuery().setQueryText("Desc2"), new SearchOptions()).getTotal()).isEqualTo(1); + assertThat(ruleIndex.search(new RuleQuery().setQueryText("Desc1"), new SearchOptions()).getTotal()).isEqualTo(0); + } + + @Test + public void disable_then_enable_rule() throws Exception { + // Install rule + when(system.now()).thenReturn(DATE1.getTime()); + execute(new FakeRepositoryV1()); + + // Uninstall rule + when(system.now()).thenReturn(DATE2.getTime()); + execute(); + + RuleDto rule = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); + assertThat(rule.getStatus()).isEqualTo(RuleStatus.REMOVED); + assertThat(ruleIndex.search(new RuleQuery().setKey(RULE_KEY1.toString()), new SearchOptions()).getTotal()).isEqualTo(0); + + // Re-install rule + when(system.now()).thenReturn(DATE3.getTime()); + execute(new FakeRepositoryV1()); + + rule = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); + assertThat(rule.getStatus()).isEqualTo(RuleStatus.BETA); + assertThat(ruleIndex.search(new RuleQuery().setKey(RULE_KEY1.toString()), new SearchOptions()).getTotal()).isEqualTo(1); + } + + @Test + public void do_not_update_rules_when_no_changes() { + execute(new FakeRepositoryV1()); + assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2); + + when(system.now()).thenReturn(DATE2.getTime()); + execute(new FakeRepositoryV1()); + + RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1); + assertThat(rule1.getCreatedAt()).isEqualTo(DATE1); + assertThat(rule1.getUpdatedAt()).isEqualTo(DATE1); } @Test public void do_not_update_already_removed_rules() { execute(new FakeRepositoryV1()); - assertThat(dbClient.deprecatedRuleDao().selectAll(dbTester.getSession())).hasSize(2); + assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2); + assertThat(esTester.getIds(RuleIndexDefinition.INDEX, RuleIndexDefinition.TYPE_RULE)).containsOnly(RULE_KEY1.toString(), RULE_KEY2.toString()); - RuleDto rule2 = dbClient.deprecatedRuleDao().getByKey(dbTester.getSession(), RuleKey.of("fake", "rule2")); + RuleDto rule2 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY2); assertThat(rule2.getStatus()).isEqualTo(RuleStatus.READY); when(system.now()).thenReturn(DATE2.getTime()); execute(new FakeRepositoryV2()); // On MySQL, need to update a rule otherwise rule2 will be seen as READY, but why ??? - dbClient.deprecatedRuleDao().update(dbTester.getSession(), dbClient.deprecatedRuleDao().getByKey(dbTester.getSession(), RuleKey.of("fake", "rule1"))); + dbClient.ruleDao().update(dbTester.getSession(), dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY1)); dbTester.getSession().commit(); // rule2 is removed - rule2 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), RuleKey.of("fake", "rule2")); + rule2 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY2); assertThat(rule2.getStatus()).isEqualTo(RuleStatus.REMOVED); + assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getIds()).containsOnly(RULE_KEY1, RULE_KEY3); + when(system.now()).thenReturn(DATE3.getTime()); execute(new FakeRepositoryV2()); dbTester.getSession().commit(); // -> rule2 is still removed, but not update at DATE3 - rule2 = dbClient.deprecatedRuleDao().getNullableByKey(dbTester.getSession(), RuleKey.of("fake", "rule2")); + rule2 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), RULE_KEY2); assertThat(rule2.getStatus()).isEqualTo(RuleStatus.REMOVED); assertThat(rule2.getUpdatedAt()).isEqualTo(DATE2); + + assertThat(ruleIndex.search(new RuleQuery(), new SearchOptions()).getIds()).containsOnly(RULE_KEY1, RULE_KEY3); } @Test public void mass_insert() { execute(new BigRepository()); - assertThat(dbClient.deprecatedRuleDao().selectAll(dbTester.getSession())).hasSize(BigRepository.SIZE); - assertThat(dbClient.deprecatedRuleDao().selectAllRuleParams(dbTester.getSession())).hasSize(BigRepository.SIZE * 20); + assertThat(dbTester.countRowsOfTable("rules")).isEqualTo(BigRepository.SIZE); + assertThat(dbTester.countRowsOfTable("rules_parameters")).isEqualTo(BigRepository.SIZE * 20); + assertThat(esTester.getIds(RuleIndexDefinition.INDEX, RuleIndexDefinition.TYPE_RULE)).hasSize(BigRepository.SIZE); } @Test public void manage_repository_extensions() { execute(new FindbugsRepository(), new FbContribRepository()); - List rules = dbClient.deprecatedRuleDao().selectAll(dbTester.getSession()); + List rules = dbClient.ruleDao().selectAll(dbTester.getSession()); assertThat(rules).hasSize(2); for (RuleDto rule : rules) { assertThat(rule.getRepositoryKey()).isEqualTo("findbugs"); @@ -213,7 +342,7 @@ public class RegisterRulesTest { Languages languages = mock(Languages.class); when(languages.get("java")).thenReturn(mock(Language.class)); - RegisterRules task = new RegisterRules(loader, ruleActivator, dbClient, languages); + RegisterRules task = new RegisterRules(loader, ruleActivator, dbClient, ruleIndexer, languages, system); task.start(); // Execute a commit to refresh session state as the task is using its own session dbTester.getSession().commit(); @@ -235,7 +364,7 @@ public class RegisterRulesTest { NewRule rule1 = repo.createRule("rule1") .setName("One") .setHtmlDescription("Description of One") - .setSeverity(Severity.BLOCKER) + .setSeverity(BLOCKER) .setInternalKey("config1") .setTags("tag1", "tag2", "tag3") .setStatus(RuleStatus.BETA) @@ -265,9 +394,9 @@ public class RegisterRulesTest { NewRule rule1 = repo.createRule("rule1") .setName("One v2") .setHtmlDescription("Description of One v2") - .setSeverity(Severity.INFO) + .setSeverity(INFO) .setInternalKey("config1 v2") - // tag2 and tag3 removed, tag4 added + // tag2 and tag3 removed, tag4 added .setTags("tag1", "tag4") .setStatus(RuleStatus.READY) .setDebtSubCharacteristic("MEMORY_EFFICIENCY") diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndex2Test.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndex2Test.java index 27c1deef067..66072c24953 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndex2Test.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndex2Test.java @@ -27,7 +27,6 @@ import java.util.Map; import org.junit.Before; import org.junit.ClassRule; import org.junit.Ignore; -import org.junit.Rule; import org.junit.Test; import org.sonar.api.config.Settings; import org.sonar.api.rule.RuleKey; @@ -35,7 +34,6 @@ import org.sonar.api.rule.RuleStatus; import org.sonar.server.es.EsTester; import org.sonar.server.es.SearchIdResult; import org.sonar.server.es.SearchOptions; -import org.sonar.server.tester.UserSessionRule; import static java.util.Arrays.asList; import static java.util.Collections.singleton; @@ -56,9 +54,6 @@ public class RuleIndex2Test { @ClassRule public static EsTester tester = new EsTester().addDefinitions(new RuleIndexDefinition(new Settings())); - @Rule - public UserSessionRule userSessionRule = UserSessionRule.standalone(); - RuleIndex2 index; RuleIndexer ruleIndexer; @@ -300,7 +295,7 @@ public class RuleIndex2Test { assertThat(index.search(query, new SearchOptions()).getIds()).hasSize(2); // null list => no filter - query = new RuleQuery().setSeverities(null); + query = new RuleQuery().setSeverities(); assertThat(index.search(query, new SearchOptions()).getIds()).hasSize(2); } @@ -651,7 +646,7 @@ public class RuleIndex2Test { // descending query = new RuleQuery().setSortField(RuleIndexDefinition.FIELD_RULE_NAME).setAscendingSort(false); results = index.search(query, new SearchOptions()); - assertThat(results.getIds()).containsExactly(RuleKey.of("java", "S003"),RuleKey.of("java", "S001"), RuleKey.of("java", "S002")); + assertThat(results.getIds()).containsExactly(RuleKey.of("java", "S003"), RuleKey.of("java", "S001"), RuleKey.of("java", "S002")); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java index e1914d17251..d77445a6f4f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/index/RuleIndexerTest.java @@ -29,7 +29,6 @@ import org.sonar.api.config.Settings; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.utils.System2; -import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleTesting; @@ -79,7 +78,7 @@ public class RuleIndexerTest { } @Test - public void removed_rule_is_removed_from_index() { + public void removed_rule_is_not_removed_from_index() { RuleIndexer indexer = createIndexer(); // Create and Index rule @@ -97,11 +96,11 @@ public class RuleIndexerTest { dbTester.getSession().commit(); indexer.index(); - assertThat(esTester.countDocuments(RuleIndexDefinition.INDEX, RuleIndexDefinition.TYPE_RULE)).isZero(); + assertThat(esTester.countDocuments(RuleIndexDefinition.INDEX, RuleIndexDefinition.TYPE_RULE)).isEqualTo(1); } private RuleIndexer createIndexer() { - RuleIndexer indexer = new RuleIndexer(new DbClient(dbTester.database(), dbTester.myBatis()), esTester.client()); + RuleIndexer indexer = new RuleIndexer(dbTester.getDbClient(), esTester.client()); indexer.setEnabled(true); return indexer; } diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java index 92cabb76426..3c79e9dfc99 100644 --- a/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleDao.java @@ -32,6 +32,7 @@ import org.sonar.db.DbSession; import org.sonar.db.RowNotFoundException; import static java.util.Collections.emptyList; +import static com.google.common.base.Preconditions.checkNotNull; import static org.sonar.db.DatabaseUtils.executeLargeInputs; public class RuleDao implements Dao { @@ -85,27 +86,23 @@ public class RuleDao implements Dao { mapper(session).selectEnabledAndNonManual(resultHandler); } - public List selectAll(DbSession session) { - return mapper(session).selectAll(); + public List selectByNonManual(DbSession session) { + return mapper(session).selectNonManual(); } - public List selectRuleParamsByRuleIds(final DbSession dbSession, List ruleIds) { - if (ruleIds.isEmpty()) { - return emptyList(); - } - return DatabaseUtils.executeLargeInputs(ruleIds, new Function, List>() { - @Override - public List apply(@Nonnull List input) { - return mapper(dbSession).selectParamsByRuleIds(input); - } - }); + public List selectAll(DbSession session) { + return mapper(session).selectAll(); } public void insert(DbSession session, RuleDto dto) { mapper(session).insert(dto); } - private static RuleMapper mapper(DbSession session) { + public void update(DbSession session, RuleDto dto) { + mapper(session).update(dto); + } + + private RuleMapper mapper(DbSession session) { return session.getMapper(RuleMapper.class); } @@ -122,4 +119,42 @@ public class RuleDao implements Dao { } } + /** + * RuleParams + */ + + public List selectRuleParamsByRuleKey(DbSession session, RuleKey key) { + return mapper(session).selectParamsByRuleKey(key); + } + + public List selectRuleParamsByRuleIds(final DbSession dbSession, List ruleIds) { + if (ruleIds.isEmpty()) { + return emptyList(); + } + return DatabaseUtils.executeLargeInputs(ruleIds, new Function, List>() { + @Override + public List apply(@Nonnull List input) { + return mapper(dbSession).selectParamsByRuleIds(input); + } + }); + } + + public void insertRuleParam(DbSession session, RuleDto rule, RuleParamDto param) { + checkNotNull(rule.getId(), "Rule id must be set"); + param.setRuleId(rule.getId()); + mapper(session).insertParameter(param); + } + + public RuleParamDto updateRuleParam(DbSession session, RuleDto rule, RuleParamDto param) { + checkNotNull(rule.getId(), "Rule id must be set"); + checkNotNull(param.getId(), "Rule parameter is not yet persisted must be set"); + param.setRuleId(rule.getId()); + mapper(session).updateParameter(param); + return param; + } + + public void deleteRuleParam(DbSession session, int ruleParameterId) { + mapper(session).deleteParameter(ruleParameterId); + } + } diff --git a/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java b/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java index 62e85dec94f..11e38b067c9 100644 --- a/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java +++ b/sonar-db/src/main/java/org/sonar/db/rule/RuleDto.java @@ -37,6 +37,7 @@ import org.sonar.db.Dto; import static com.google.common.base.Preconditions.checkArgument; +// TODO remove extends Dto public class RuleDto extends Dto { public static final int DISABLED_CHARACTERISTIC_ID = -1; @@ -75,6 +76,9 @@ public class RuleDto extends Dto { private RuleKey key; + private long createdAtInMs; + private long updatedAtInMs; + @Override public RuleKey getKey() { if (key == null) { @@ -377,6 +381,24 @@ public class RuleDto extends Dto { return this; } + public long getCreatedAtInMs() { + return createdAtInMs; + } + + public RuleDto setCreatedAtInMs(long createdAt) { + this.createdAtInMs = createdAt; + return this; + } + + public long getUpdatedAtInMs() { + return updatedAtInMs; + } + + public RuleDto setUpdatedAtInMs(long updatedAt) { + this.updatedAtInMs = updatedAt; + return this; + } + @Override public boolean equals(Object obj) { if (!(obj instanceof RuleDto)) { diff --git a/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml b/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml index 74c3901a988..d34b00c4fe7 100644 --- a/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml +++ b/sonar-db/src/main/resources/org/sonar/db/rule/RuleMapper.xml @@ -32,7 +32,9 @@ r.tags as "tagsField", r.system_tags as "systemTagsField", r.created_at as "createdAt", - r.updated_at as "updatedAt" + r.updated_at as "updatedAt", + r.created_at_ms as "createdAtInMs", + r.updated_at_ms as "updatedAtInMs"