From c333b3ae6984bf1b402d3398fa71de7f0a75b817 Mon Sep 17 00:00:00 2001 From: Aurelien Poscia Date: Sat, 23 Apr 2022 14:21:23 +0200 Subject: [PATCH] SONAR-16302 persist and fetch rules with new DB structure --- .../java/org/sonar/db/version/SqTables.java | 1 + .../src/main/java/org/sonar/db/MyBatis.java | 2 - .../main/java/org/sonar/db/rule/RuleDao.java | 56 +++--- .../org/sonar/db/rule/RuleDefinitionDto.java | 43 +++-- .../db/rule/RuleDescriptionSectionDto.java | 22 ++- .../main/java/org/sonar/db/rule/RuleDto.java | 8 +- .../java/org/sonar/db/rule/RuleMapper.java | 6 +- .../org/sonar/db/rule/RuleMapper.xml | 165 ++++++++++++++++-- .../java/org/sonar/db/rule/RuleDaoTest.java | 74 ++++++-- .../java/org/sonar/db/rule/RuleTesting.java | 4 +- .../sonar/server/rule/DefaultRuleFinder.java | 4 +- .../rule/HotspotRuleDescriptionTest.java | 18 +- .../rule/RuleDescriptionFormatterTest.java | 6 +- .../server/rule/index/RuleIndexTest.java | 16 +- .../server/rule/index/RuleIndexerTest.java | 30 ++-- .../server/rule/CachingRuleFinderTest.java | 3 +- .../org/sonar/server/rule/RegisterRules.java | 12 +- .../sonar/server/rule/RegisterRulesTest.java | 2 +- ...lityProfileChangeEventServiceImplTest.java | 2 +- .../org/sonar/server/rule/RuleCreator.java | 2 +- .../org/sonar/server/rule/RuleUpdater.java | 9 +- .../org/sonar/server/rule/ws/ListAction.java | 27 +-- .../server/hotspot/ws/ShowActionTest.java | 7 +- .../server/issue/ws/SearchActionTest.java | 6 +- .../QProfileBackuperImplTest.java | 3 +- .../sonar/server/rule/RuleCreatorTest.java | 4 +- .../sonar/server/rule/RuleUpdaterTest.java | 23 +-- .../server/rule/ws/CreateActionTest.java | 2 +- .../server/rule/ws/SearchActionTest.java | 12 +- .../sonar/server/rule/ws/ShowActionTest.java | 7 +- .../server/rule/ws/UpdateActionTest.java | 15 +- 31 files changed, 416 insertions(+), 175 deletions(-) diff --git a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java index 0efe8159a86..b46f6a4a1d9 100644 --- a/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java +++ b/server/sonar-db-core/src/main/java/org/sonar/db/version/SqTables.java @@ -90,6 +90,7 @@ public final class SqTables { "quality_gate_conditions", "saml_message_ids", "rules", + "rule_desc_sections", "rules_metadata", "rules_parameters", "rules_profiles", diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java b/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java index 99875cdcbd7..462ea745804 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/MyBatis.java @@ -131,7 +131,6 @@ import org.sonar.db.qualityprofile.QProfileEditGroupsMapper; import org.sonar.db.qualityprofile.QProfileEditUsersMapper; import org.sonar.db.qualityprofile.QualityProfileExportMapper; import org.sonar.db.qualityprofile.QualityProfileMapper; -import org.sonar.db.rule.RuleDto; import org.sonar.db.rule.RuleMapper; import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleRepositoryMapper; @@ -221,7 +220,6 @@ public class MyBatis { confBuilder.loadAlias("QualityGate", QualityGateDto.class); confBuilder.loadAlias("Resource", ResourceDto.class); confBuilder.loadAlias("RuleParam", RuleParamDto.class); - confBuilder.loadAlias("Rule", RuleDto.class); confBuilder.loadAlias("SchemaMigration", SchemaMigrationDto.class); confBuilder.loadAlias("ScrapProperty", ScrapPropertyDto.class); confBuilder.loadAlias("ScrapAnalysisProperty", ScrapAnalysisPropertyDto.class); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java index 20b7c6edd46..95e81b68c4a 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java @@ -24,7 +24,6 @@ import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Consumer; -import org.apache.ibatis.session.ResultHandler; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleQuery; import org.sonar.core.util.UuidFactory; @@ -34,7 +33,6 @@ import org.sonar.db.RowNotFoundException; import static com.google.common.base.Preconditions.checkNotNull; import static java.util.Collections.emptyList; -import static java.util.Optional.ofNullable; import static org.sonar.db.DatabaseUtils.executeLargeInputs; import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput; import static org.sonar.db.DatabaseUtils.executeLargeUpdates; @@ -48,16 +46,15 @@ public class RuleDao implements Dao { } public Optional selectByKey(DbSession session, RuleKey key) { - RuleDto res = mapper(session).selectByKey(key); - return ofNullable(res); + return Optional.ofNullable(mapper(session).selectByKey(key)); } public Optional selectDefinitionByKey(DbSession session, RuleKey key) { - return ofNullable(mapper(session).selectDefinitionByKey(key)); + return Optional.ofNullable(mapper(session).selectDefinitionByKey(key)); } public Optional selectMetadataByKey(DbSession session, RuleKey key) { - return ofNullable(mapper(session).selectMetadataByKey(key)); + return Optional.ofNullable(mapper(session).selectMetadataByKey(key)); } public List selectMetadataByKeys(DbSession session, Collection keys) { @@ -68,28 +65,21 @@ public class RuleDao implements Dao { } public RuleDto selectOrFailByKey(DbSession session, RuleKey key) { - RuleDto rule = mapper(session).selectByKey(key); - if (rule == null) { - throw new RowNotFoundException(String.format("Rule with key '%s' does not exist", key)); - } - return rule; + return Optional.ofNullable(mapper(session).selectByKey(key)) + .orElseThrow(() -> new RowNotFoundException(String.format("Rule with key '%s' does not exist", key))); } public RuleDefinitionDto selectOrFailDefinitionByKey(DbSession session, RuleKey key) { - RuleDefinitionDto rule = mapper(session).selectDefinitionByKey(key); - if (rule == null) { - throw new RowNotFoundException(String.format("Rule with key '%s' does not exist", key)); - } - return rule; + return Optional.ofNullable(mapper(session).selectDefinitionByKey(key)) + .orElseThrow(() -> new RowNotFoundException(String.format("Rule with key '%s' does not exist", key))); } public Optional selectByUuid(String uuid, DbSession session) { - RuleDto res = mapper(session).selectByUuid(uuid); - return ofNullable(res); + return Optional.ofNullable(mapper(session).selectByUuid(uuid)); } public Optional selectDefinitionByUuid(String uuid, DbSession session) { - return ofNullable(mapper(session).selectDefinitionByUuid(uuid)); + return Optional.ofNullable(mapper(session).selectDefinitionByUuid(uuid)); } public List selectByUuids(DbSession session, List uuids) { @@ -120,8 +110,8 @@ public class RuleDao implements Dao { return executeLargeInputs(keys, mapper(session)::selectDefinitionByKeys); } - public void selectEnabled(DbSession session, ResultHandler resultHandler) { - mapper(session).selectEnabled(resultHandler); + public List selectEnabled(DbSession session) { + return mapper(session).selectEnabled(); } public List selectAll(DbSession session) { @@ -140,9 +130,11 @@ public class RuleDao implements Dao { return mapper(session).selectByQuery(ruleQuery); } - public void insert(DbSession session, RuleDefinitionDto dto) { - checkNotNull(dto.getUuid(), "RuleDefinitionDto has no 'uuid'."); - mapper(session).insertDefinition(dto); + public void insert(DbSession session, RuleDefinitionDto ruleDefinitionDto) { + checkNotNull(ruleDefinitionDto.getUuid(), "RuleDefinitionDto has no 'uuid'."); + RuleMapper mapper = mapper(session); + mapper.insertDefinition(ruleDefinitionDto); + insertRuleDescriptionSectionDtos(ruleDefinitionDto, mapper); } public void insert(DbSession session, RuleMetadataDto dto) { @@ -150,8 +142,20 @@ public class RuleDao implements Dao { mapper(session).insertMetadata(dto); } - public void update(DbSession session, RuleDefinitionDto dto) { - mapper(session).updateDefinition(dto); + public void update(DbSession session, RuleDefinitionDto ruleDefinitionDto) { + RuleMapper mapper = mapper(session); + mapper.updateDefinition(ruleDefinitionDto); + updateRuleDescriptionDtos(ruleDefinitionDto, mapper); + } + + private void updateRuleDescriptionDtos(RuleDefinitionDto ruleDefinitionDto, RuleMapper mapper) { + mapper.deleteRuleDescriptionSection(ruleDefinitionDto.getUuid()); + insertRuleDescriptionSectionDtos(ruleDefinitionDto, mapper); + } + + private void insertRuleDescriptionSectionDtos(RuleDefinitionDto ruleDefinitionDto, RuleMapper mapper) { + ruleDefinitionDto.getRuleDescriptionSectionDtos() + .forEach(section -> mapper.insertRuleDescriptionSection(ruleDefinitionDto.getUuid(), section)); } public void insertOrUpdate(DbSession session, RuleMetadataDto dto) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDefinitionDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDefinitionDto.java index 1cd974855ff..2347e80f8f5 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDefinitionDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDefinitionDto.java @@ -21,10 +21,9 @@ package org.sonar.db.rule; import com.google.common.base.Splitter; import com.google.common.collect.ImmutableSet; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; +import java.util.HashSet; import java.util.Objects; +import java.util.Optional; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -36,7 +35,7 @@ import org.sonar.api.rules.RuleType; import org.sonar.db.rule.RuleDto.Scope; import static com.google.common.base.Preconditions.checkArgument; -import static java.util.Collections.unmodifiableCollection; +import static org.sonar.db.rule.RuleDescriptionSectionDto.DEFAULT_KEY; public class RuleDefinitionDto { @@ -46,7 +45,7 @@ public class RuleDefinitionDto { private String repositoryKey; private String ruleKey; - private Map ruleDescriptionSectionDtos = new HashMap<>(); + private Set ruleDescriptionSectionDtos = new HashSet<>(); /** * Description format can be null on external rule, otherwise it should never be null @@ -167,35 +166,38 @@ public class RuleDefinitionDto { return this; } - public Collection getRuleDescriptionSectionDtos() { - return unmodifiableCollection(ruleDescriptionSectionDtos.values()); - } - @CheckForNull public RuleDescriptionSectionDto getRuleDescriptionSectionDto(String ruleDescriptionSectionKey) { - return ruleDescriptionSectionDtos.get(ruleDescriptionSectionKey); + return findExistingSectionWithSameKey(ruleDescriptionSectionKey).orElse(null); } @CheckForNull public RuleDescriptionSectionDto getDefaultRuleDescriptionSectionDto() { - return ruleDescriptionSectionDtos.get(RuleDescriptionSectionDto.DEFAULT_KEY); + return findExistingSectionWithSameKey(DEFAULT_KEY).orElse(null); } public RuleDefinitionDto addRuleDescriptionSectionDto(RuleDescriptionSectionDto ruleDescriptionSectionDto) { - checkArgument(!isSectionKeyUsed(ruleDescriptionSectionDto.getKey()), "A section with key %s already exists", ruleDescriptionSectionDto.getKey()); - this.ruleDescriptionSectionDtos.put(ruleDescriptionSectionDto.getKey(), ruleDescriptionSectionDto); + checkArgument(sectionWithSameKeyShouldNotExist(ruleDescriptionSectionDto), + "A section with key %s already exists", ruleDescriptionSectionDto.getKey()); + ruleDescriptionSectionDtos.add(ruleDescriptionSectionDto); return this; } - private boolean isSectionKeyUsed(String sectionKey) { - return ruleDescriptionSectionDtos.containsKey(sectionKey); + private boolean sectionWithSameKeyShouldNotExist(RuleDescriptionSectionDto ruleDescriptionSectionDto) { + return findExistingSectionWithSameKey(ruleDescriptionSectionDto.getKey()).isEmpty(); } public RuleDefinitionDto addOrReplaceRuleDescriptionSectionDto(RuleDescriptionSectionDto ruleDescriptionSectionDto) { - this.ruleDescriptionSectionDtos.put(ruleDescriptionSectionDto.getKey(), ruleDescriptionSectionDto); + Optional existingSectionWithSameKey = findExistingSectionWithSameKey(ruleDescriptionSectionDto.getKey()); + existingSectionWithSameKey.ifPresent(ruleDescriptionSectionDtos::remove); + ruleDescriptionSectionDtos.add(ruleDescriptionSectionDto); return this; } + private Optional findExistingSectionWithSameKey(String ruleDescriptionSectionKey) { + return ruleDescriptionSectionDtos.stream().filter(section -> section.getKey().equals(ruleDescriptionSectionKey)).findAny(); + } + @CheckForNull public RuleDto.Format getDescriptionFormat() { return descriptionFormat; @@ -429,6 +431,14 @@ public class RuleDefinitionDto { return this; } + public Set getRuleDescriptionSectionDtos() { + return ruleDescriptionSectionDtos; + } + + void setRuleDescriptionSectionDtos(Set ruleDescriptionSectionDtos) { + this.ruleDescriptionSectionDtos = ruleDescriptionSectionDtos; + } + @Override public boolean equals(Object obj) { if (!(obj instanceof RuleDefinitionDto)) { @@ -475,4 +485,5 @@ public class RuleDefinitionDto { ", scope=" + scope + '}'; } + } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDescriptionSectionDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDescriptionSectionDto.java index bff3a3d30d1..76dc0684d9e 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDescriptionSectionDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDescriptionSectionDto.java @@ -25,14 +25,21 @@ import static org.sonar.api.utils.Preconditions.checkArgument; public class RuleDescriptionSectionDto { static final String DEFAULT_KEY = "default"; + + private final String uuid; private final String key; private final String description; - private RuleDescriptionSectionDto(String key, String description) { + private RuleDescriptionSectionDto(String uuid, String key, String description) { + this.uuid = uuid; this.key = key; this.description = description; } + public String getUuid() { + return uuid; + } + public String getKey() { return key; } @@ -41,9 +48,10 @@ public class RuleDescriptionSectionDto { return description; } - public static RuleDescriptionSectionDto createDefaultRuleDescriptionSection(String description) { + public static RuleDescriptionSectionDto createDefaultRuleDescriptionSection(String uuid, String description) { return RuleDescriptionSectionDto.builder() .setDefault() + .uuid(uuid) .description(description) .build(); } @@ -59,18 +67,25 @@ public class RuleDescriptionSectionDto { @Override public String toString() { return new StringJoiner(", ", RuleDescriptionSectionDto.class.getSimpleName() + "[", "]") + .add("uuid='" + uuid + "'") .add("key='" + key + "'") .add("description='" + description + "'") .toString(); } public static final class RuleDescriptionSectionDtoBuilder { + private String uuid; private String key = null; private String description; private RuleDescriptionSectionDtoBuilder() { } + public RuleDescriptionSectionDtoBuilder uuid(String uuid) { + this.uuid = uuid; + return this; + } + public RuleDescriptionSectionDtoBuilder setDefault() { checkArgument(this.key == null, "Only one of setDefault and key methods can be called"); this.key = DEFAULT_KEY; @@ -83,14 +98,13 @@ public class RuleDescriptionSectionDto { return this; } - public RuleDescriptionSectionDtoBuilder description(String description) { this.description = description; return this; } public RuleDescriptionSectionDto build() { - return new RuleDescriptionSectionDto(key, description); + return new RuleDescriptionSectionDto(uuid, key, description); } } } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java index 3e518801fa7..12f4d4e4625 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDto.java @@ -125,6 +125,10 @@ public class RuleDto { return this; } + void setRuleDescriptionSectionDtos(Set ruleDescriptionSectionDtos) { + definition.setRuleDescriptionSectionDtos(ruleDescriptionSectionDtos); + } + public Format getDescriptionFormat() { return definition.getDescriptionFormat(); } @@ -203,7 +207,7 @@ public class RuleDto { return definition.isAdHoc(); } - public RuleDto setIsAdhoc(boolean isAdHoc) { + public RuleDto setIsAdHoc(boolean isAdHoc) { definition.setIsAdHoc(isAdHoc); return this; } @@ -510,6 +514,8 @@ public class RuleDto { return this; } + + @Override public boolean equals(Object obj) { if (!(obj instanceof RuleDto)) { diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleMapper.java index 34bb0d063b7..74f67110746 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleMapper.java @@ -33,7 +33,7 @@ public interface RuleMapper { List selectAllDefinitions(); - void selectEnabled(ResultHandler resultHandler); + List selectEnabled(); RuleDto selectByUuid(@Param("uuid") String uuid); @@ -67,8 +67,12 @@ public interface RuleMapper { void insertDefinition(RuleDefinitionDto ruleDefinitionDto); + void insertRuleDescriptionSection(@Param("ruleUuid") String ruleUuid, @Param("dto") RuleDescriptionSectionDto ruleDescriptionSectionDto); + void updateDefinition(RuleDefinitionDto ruleDefinitionDto); + void deleteRuleDescriptionSection(String ruleUuid); + int countMetadata(RuleMetadataDto ruleMetadataDto); void insertMetadata(RuleMetadataDto ruleMetadataDto); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml index 06b8316d728..0188bf511d8 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml @@ -3,11 +3,22 @@ + + rds.uuid as "rds_uuid", + rds.kee as "rds_kee", + rds.description as "rds_description", + + + + left outer join rule_desc_sections rds on + rds.rule_uuid = r.uuid + + - r.uuid, + + r.uuid as "r_uuid", r.plugin_rule_key as "ruleKey", r.plugin_name as "repositoryKey", - r.description, r.description_format as "descriptionFormat", r.status, r.name, @@ -60,88 +71,182 @@ rm.rule_uuid = r.uuid - select from rules r + - select from rules r + - select from rules r + where r.status != 'REMOVED' - select from rules r + where r.uuid=#{uuid,jdbcType=VARCHAR} - select from rules r + where r.uuid=#{uuid,jdbcType=VARCHAR} - select from rules r + where r.uuid=#{uuid,jdbcType=VARCHAR} - select from rules r + where r.uuid=#{uuid,jdbcType=VARCHAR} - select from rules r + where r.plugin_name=#{ruleKey.repository,jdbcType=VARCHAR} and r.plugin_rule_key=#{ruleKey.rule,jdbcType=VARCHAR} - select from rules r + where r.plugin_name=#{repository,jdbcType=VARCHAR} and r.plugin_rule_key=#{rule,jdbcType=VARCHAR} @@ -231,23 +336,25 @@ - select from rules r + where (r.plugin_name=#{ruleKey.repository,jdbcType=VARCHAR} and r.plugin_rule_key=#{ruleKey.rule,jdbcType=VARCHAR}) - select from rules r + where (r.plugin_name=#{ruleKey.repository,jdbcType=VARCHAR} and r.plugin_rule_key=#{ruleKey.rule,jdbcType=VARCHAR}) @@ -275,7 +382,6 @@ r.plugin_name as "repository", r.plugin_rule_key as "pluginRuleKey", r.name as "name", - r.description as "description", r.description_format as "descriptionFormat", r.priority as "severity", r.status as "status", @@ -296,12 +402,13 @@ left outer join rules_metadata rm on r.uuid = rm.rule_uuid - select from rules r + where r.status != 'REMOVED' @@ -317,12 +424,13 @@ r.updated_at desc - select from rules r + where r.status != 'REMOVED' and r.is_external=${_false} and r.is_template=${_false} and r.rule_type in @@ -331,13 +439,27 @@ #{language, jdbcType=VARCHAR} + + insert into rule_desc_sections ( + uuid, + rule_uuid, + kee, + description + ) + values ( + #{dto.uuid,jdbcType=VARCHAR}, + #{ruleUuid,jdbcType=VARCHAR}, + #{dto.key,jdbcType=VARCHAR}, + #{dto.description,jdbcType=VARCHAR} + ) + + insert into rules ( uuid, plugin_key, plugin_rule_key, plugin_name, - description, description_format, status, name, @@ -364,7 +486,6 @@ #{pluginKey,jdbcType=VARCHAR}, #{ruleKey,jdbcType=VARCHAR}, #{repositoryKey,jdbcType=VARCHAR}, - #{description,jdbcType=VARCHAR}, #{descriptionFormat,jdbcType=VARCHAR}, #{status,jdbcType=VARCHAR}, #{name,jdbcType=VARCHAR}, @@ -393,7 +514,6 @@ plugin_key=#{pluginKey,jdbcType=VARCHAR}, plugin_rule_key=#{ruleKey,jdbcType=VARCHAR}, plugin_name=#{repositoryKey,jdbcType=VARCHAR}, - description=#{description,jdbcType=VARCHAR}, description_format=#{descriptionFormat,jdbcType=VARCHAR}, status=#{status,jdbcType=VARCHAR}, name=#{name,jdbcType=VARCHAR}, @@ -417,6 +537,13 @@ uuid=#{uuid,jdbcType=VARCHAR} + + delete from + rule_desc_sections + where + rule_uuid=#{uuid,jdbcType=VARCHAR} + +