From: Julien HENRY Date: Thu, 6 Sep 2018 16:02:56 +0000 (+0200) Subject: SONAR-11209 Store ad hoc rules coming from scanner in rules_metadata X-Git-Tag: 7.5~439 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1cacbb1d2ed72d720058580746d678ba8da1b453;p=sonarqube.git SONAR-11209 Store ad hoc rules coming from scanner in rules_metadata --- diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReader.java index f8bc0774d7a..a8e82f3d65f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReader.java @@ -31,6 +31,8 @@ public interface BatchReportReader { CloseableIterator readActiveRules(); + CloseableIterator readAdHocRules(); + CloseableIterator readComponentMeasures(int componentRef); @CheckForNull diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderImpl.java index 680baaeca43..3c324d7a8ed 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderImpl.java @@ -86,6 +86,13 @@ public class BatchReportReaderImpl implements BatchReportReader { return delegate.readActiveRules(); } + @Override + public CloseableIterator readAdHocRules() { + ensureInitialized(); + return delegate.readAdHocRules(); + } + + @Override public CloseableIterator readComponentMeasures(int componentRef) { ensureInitialized(); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java new file mode 100644 index 00000000000..432961896e1 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreator.java @@ -0,0 +1,114 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.issue; + +import com.google.common.base.Preconditions; +import java.util.Objects; +import java.util.Optional; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.System2; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.rule.RuleDao; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleMetadataDto; +import org.sonar.server.rule.index.RuleIndexer; + +import static java.util.Objects.requireNonNull; +import static org.sonar.api.rule.RuleStatus.READY; +import static org.sonar.db.rule.RuleDto.Scope.ALL; + +public class AdHocRuleCreator { + + private final DbClient dbClient; + private final System2 system2; + private final RuleIndexer ruleIndexer; + + public AdHocRuleCreator(DbClient dbClient, System2 system2, RuleIndexer ruleIndexer) { + this.dbClient = dbClient; + this.system2 = system2; + this.ruleIndexer = ruleIndexer; + } + + /** + * Persists a new add hoc rule in the DB and indexes it. + * @return the rule that was inserted in the DB, which includes the generated ID. + */ + public RuleDto persistAndIndex(DbSession dbSession, NewAdHocRule adHoc, OrganizationDto organizationDto) { + RuleDao dao = dbClient.ruleDao(); + Optional existingRuleDtoOpt = dao.selectByKey(dbSession, organizationDto, adHoc.getKey()); + RuleMetadataDto metadata; + long now = system2.now(); + if (!existingRuleDtoOpt.isPresent()) { + RuleDefinitionDto dto = new RuleDefinitionDto() + .setRuleKey(adHoc.getKey()) + .setIsExternal(true) + .setIsAdHoc(true) + .setName(adHoc.getEngineId() + ": " + adHoc.getRuleId()) + .setScope(ALL) + .setStatus(READY) + .setCreatedAt(now) + .setUpdatedAt(now); + dao.insert(dbSession, dto); + metadata = new RuleMetadataDto() + .setRuleId(dto.getId()) + .setOrganizationUuid(organizationDto.getUuid()); + } else { + // No need to update the rule, only org specific metadata + RuleDto ruleDto = existingRuleDtoOpt.get(); + Preconditions.checkState(ruleDto.isExternal() && ruleDto.isAdHoc()); + metadata = ruleDto.getMetadata(); + } + + if (adHoc.hasDetails()) { + boolean changed = false; + if (!Objects.equals(metadata.getAdHocName(), adHoc.getName())) { + metadata.setAdHocName(adHoc.getName()); + changed = true; + } + if (!Objects.equals(metadata.getAdHocDescription(), adHoc.getDescription())) { + metadata.setAdHocDescription(adHoc.getDescription()); + changed = true; + } + if (!Objects.equals(metadata.getAdHocSeverity(), adHoc.getSeverity())) { + metadata.setAdHocSeverity(adHoc.getSeverity()); + changed = true; + } + RuleType ruleType = requireNonNull(adHoc.getRuleType(), "Rule type should not be null"); + if (!Objects.equals(metadata.getAdHocType(), ruleType.getDbConstant())) { + metadata.setAdHocType(ruleType); + changed = true; + } + if (changed) { + metadata.setUpdatedAt(now); + metadata.setCreatedAt(now); + dao.insertOrUpdate(dbSession, metadata); + } + + } + + RuleDto ruleDto = dao.selectOrFailByKey(dbSession, organizationDto, adHoc.getKey()); + ruleIndexer.commitAndIndex(dbSession, ruleDto.getId()); + return ruleDto; + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java new file mode 100644 index 00000000000..029430cf6c8 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRule.java @@ -0,0 +1,128 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.issue; + +import com.google.common.base.Preconditions; +import java.util.Objects; +import javax.annotation.CheckForNull; +import javax.annotation.concurrent.Immutable; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rules.RuleType; +import org.sonar.scanner.protocol.Constants; +import org.sonar.scanner.protocol.output.ScannerReport; + +import static org.apache.commons.lang.StringUtils.isNotBlank; +import static org.apache.commons.lang.StringUtils.trimToNull; + +@Immutable +public class NewAdHocRule { + private final RuleKey key; + private final String engineId; + private final String ruleId; + private final String name; + private final String description; + private final String severity; + private final RuleType ruleType; + private final boolean hasDetails; + + public NewAdHocRule(ScannerReport.AdHocRule ruleFromScannerReport) { + Preconditions.checkArgument(isNotBlank(ruleFromScannerReport.getEngineId()), "'engine id' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(isNotBlank(ruleFromScannerReport.getRuleId()), "'rule id' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(isNotBlank(ruleFromScannerReport.getName()), "'name' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(ruleFromScannerReport.getSeverity() != Constants.Severity.UNSET_SEVERITY , "'severity' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(ruleFromScannerReport.getType() != ScannerReport.IssueType.UNSET, "'issue type' not expected to be null for an ad hoc rule"); + this.key = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + ruleFromScannerReport.getEngineId(), ruleFromScannerReport.getRuleId()); + this.engineId = ruleFromScannerReport.getEngineId(); + this.ruleId = ruleFromScannerReport.getRuleId(); + this.name = ruleFromScannerReport.getName(); + this.description = trimToNull(ruleFromScannerReport.getDescription()); + this.severity = ruleFromScannerReport.getSeverity().name(); + this.ruleType = RuleType.valueOf(ruleFromScannerReport.getType().name()); + this.hasDetails = true; + } + + public NewAdHocRule(ScannerReport.ExternalIssue fromIssue) { + Preconditions.checkArgument(isNotBlank(fromIssue.getEngineId()), "'engine id' not expected to be null for an ad hoc rule"); + Preconditions.checkArgument(isNotBlank(fromIssue.getRuleId()), "'rule id' not expected to be null for an ad hoc rule"); + this.key = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + fromIssue.getEngineId(), fromIssue.getRuleId()); + this.engineId = fromIssue.getEngineId(); + this.ruleId = fromIssue.getRuleId(); + this.name = null; + this.description = null; + this.severity = null; + this.ruleType = null; + this.hasDetails = false; + } + + public RuleKey getKey() { + return key; + } + + public String getEngineId() { + return engineId; + } + + public String getRuleId() { + return ruleId; + } + + @CheckForNull + public String getName() { + return name; + } + + @CheckForNull + public String getDescription() { + return description; + } + + @CheckForNull + public String getSeverity() { + return severity; + } + + @CheckForNull + public RuleType getRuleType() { + return ruleType; + } + + public boolean hasDetails() { + return hasDetails; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + NewAdHocRule that = (NewAdHocRule) o; + return Objects.equals(key, that.key); + } + + @Override + public int hashCode() { + return key.hashCode(); + } + + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java index 6cba8ab28e4..eb52047cdfc 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java @@ -44,6 +44,8 @@ public interface Rule { boolean isExternal(); + boolean isAdHoc(); + /** * Get all tags, whatever system or user tags. */ diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java index 43b0d294995..d81ab7930ff 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java @@ -44,7 +44,8 @@ public class RuleImpl implements Rule { private final DebtRemediationFunction remediationFunction; private final RuleType type; private final String pluginKey; - private final boolean external; + private final boolean isExternal; + private final boolean isAdHoc; public RuleImpl(RuleDto dto) { this.id = dto.getId(); @@ -55,7 +56,8 @@ public class RuleImpl implements Rule { this.remediationFunction = effectiveRemediationFunction(dto); this.type = RuleType.valueOfNullable(dto.getType()); this.pluginKey = dto.getPluginKey(); - this.external = dto.isExternal(); + this.isExternal = dto.isExternal(); + this.isAdHoc = dto.isAdHoc(); } @Override @@ -141,8 +143,13 @@ public class RuleImpl implements Rule { return null; } + @Override + public boolean isAdHoc() { + return isAdHoc; + } + @Override public boolean isExternal() { - return external; + return isExternal; } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepository.java index a780920f733..7104dcbce99 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepository.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepository.java @@ -23,7 +23,6 @@ import java.util.Optional; import java.util.function.Supplier; import org.sonar.api.rule.RuleKey; import org.sonar.db.DbSession; -import org.sonar.server.rule.NewAddHocRule; /** * Repository of every rule in DB (including manual rules) whichever their status. @@ -48,8 +47,8 @@ public interface RuleRepository { Optional findById(int id); - void addNewAddHocRuleIfAbsent(RuleKey ruleKey, Supplier ruleSupplier); + void addOrUpdateAddHocRuleIfNeeded(RuleKey ruleKey, Supplier ruleSupplier); - void persistNewAddHocRules(DbSession dbSession); + void saveOrUpdateAddHocRules(DbSession dbSession); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java index 7a99e846c1c..e91923c267f 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java @@ -31,14 +31,12 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rules.RuleType; import org.sonar.api.server.debt.DebtRemediationFunction; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.core.util.stream.MoreCollectors; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.rule.DeprecatedRuleKeyDto; import org.sonar.db.rule.RuleDto; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; -import org.sonar.server.rule.AddHocRuleCreator; -import org.sonar.server.rule.NewAddHocRule; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; @@ -50,37 +48,40 @@ public class RuleRepositoryImpl implements RuleRepository { @CheckForNull private Map rulesById; - private final AddHocRuleCreator creator; + private final AdHocRuleCreator creator; private final DbClient dbClient; private final AnalysisMetadataHolder analysisMetadataHolder; - public RuleRepositoryImpl(AddHocRuleCreator creator, DbClient dbClient, AnalysisMetadataHolder analysisMetadataHolder) { + private Map adHocRulesPersist = new HashMap<>(); + + public RuleRepositoryImpl(AdHocRuleCreator creator, DbClient dbClient, AnalysisMetadataHolder analysisMetadataHolder) { this.creator = creator; this.dbClient = dbClient; this.analysisMetadataHolder = analysisMetadataHolder; } - public void addNewAddHocRuleIfAbsent(RuleKey ruleKey, Supplier ruleSupplier) { + public void addOrUpdateAddHocRuleIfNeeded(RuleKey ruleKey, Supplier ruleSupplier) { ensureInitialized(); - if (!rulesByKey.containsKey(ruleKey)) { - rulesByKey.computeIfAbsent(ruleKey, s -> new AdHocRuleWrapper(ruleSupplier.get())); + Rule existingRule = rulesByKey.get(ruleKey); + if (existingRule == null || (existingRule.isAdHoc() && !adHocRulesPersist.containsKey(ruleKey))) { + NewAdHocRule newAdHocRule = ruleSupplier.get(); + adHocRulesPersist.put(ruleKey, newAdHocRule); + rulesByKey.put(ruleKey, new AdHocRuleWrapper(newAdHocRule)); } } @Override - public void persistNewAddHocRules(DbSession dbSession) { + public void saveOrUpdateAddHocRules(DbSession dbSession) { ensureInitialized(); - rulesByKey.values().stream() - .filter(AdHocRuleWrapper.class::isInstance) - .forEach(r -> persistAndIndex(dbSession, (AdHocRuleWrapper) r)); + adHocRulesPersist.values().forEach(r -> persistAndIndex(dbSession, r)); } - private void persistAndIndex(DbSession dbSession, AdHocRuleWrapper external) { - Rule rule = new RuleImpl(creator.persistAndIndex(dbSession, external.getDelegate())); + private void persistAndIndex(DbSession dbSession, NewAdHocRule adHocRule) { + Rule rule = new RuleImpl(creator.persistAndIndex(dbSession, adHocRule, analysisMetadataHolder.getOrganization().toDto())); rulesById.put(rule.getId(), rule); - rulesByKey.put(external.getKey(), rule); + rulesByKey.put(adHocRule.getKey(), rule); } @Override @@ -146,19 +147,19 @@ public class RuleRepositoryImpl implements RuleRepository { } private static class AdHocRuleWrapper implements Rule { - private final NewAddHocRule addHocRule; + private final NewAdHocRule addHocRule; - private AdHocRuleWrapper(NewAddHocRule addHocRule) { + private AdHocRuleWrapper(NewAdHocRule addHocRule) { this.addHocRule = addHocRule; } - public NewAddHocRule getDelegate() { + public NewAdHocRule getDelegate() { return addHocRule; } @Override public int getId() { - return 0; + throw new UnsupportedOperationException("Rule is not persisted, can't know the id"); } @Override @@ -187,6 +188,11 @@ public class RuleRepositoryImpl implements RuleRepository { return true; } + @Override + public boolean isAdHoc() { + return true; + } + @Override public Set getTags() { return Collections.emptySet(); @@ -201,7 +207,7 @@ public class RuleRepositoryImpl implements RuleRepository { @CheckForNull @Override public String getPluginKey() { - return addHocRule.getPluginKey(); + return null; } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java index 767627d6a79..0ee7f7e1e49 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java @@ -21,14 +21,23 @@ package org.sonar.ce.task.projectanalysis.issue; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; +import javax.annotation.Nullable; import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.api.utils.log.Loggers; +import org.sonar.ce.task.projectanalysis.batch.BatchReportReader; import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; +import org.sonar.ce.task.projectanalysis.issue.commonrule.CommonRuleEngine; +import org.sonar.ce.task.projectanalysis.issue.filter.IssueFilter; +import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; +import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.Input; import org.sonar.core.issue.tracking.LazyInput; @@ -39,14 +48,7 @@ import org.sonar.db.protobuf.DbIssues; import org.sonar.scanner.protocol.Constants.Severity; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReport.IssueType; -import org.sonar.ce.task.projectanalysis.batch.BatchReportReader; -import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; -import org.sonar.ce.task.projectanalysis.issue.commonrule.CommonRuleEngine; -import org.sonar.ce.task.projectanalysis.issue.filter.IssueFilter; -import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; -import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.server.rule.CommonRuleKeys; -import org.sonar.server.rule.NewAddHocRule; import static org.apache.commons.lang.StringUtils.isNotEmpty; @@ -120,12 +122,20 @@ public class TrackerRawInputFactory { } } - try (CloseableIterator reportIssues = reportReader.readComponentExternalIssues(component.getReportAttributes().getRef())) { + Map adHocRuleMap = new HashMap<>(); + try (CloseableIterator reportAdHocRule = reportReader.readAdHocRules()) { + while (reportAdHocRule.hasNext()) { + ScannerReport.AdHocRule adHocRule = reportAdHocRule.next(); + adHocRuleMap.put(RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + adHocRule.getEngineId(), adHocRule.getRuleId()), adHocRule); + } + } + + try (CloseableIterator reportExternalIssues = reportReader.readComponentExternalIssues(component.getReportAttributes().getRef())) { // optimization - do not load line hashes if there are no issues -> getLineHashSequence() is executed // as late as possible - while (reportIssues.hasNext()) { - ScannerReport.ExternalIssue reportExternalIssue = reportIssues.next(); - result.add(toExternalIssue(getLineHashSequence(), reportExternalIssue)); + while (reportExternalIssues.hasNext()) { + ScannerReport.ExternalIssue reportExternalIssue = reportExternalIssues.next(); + result.add(toExternalIssue(getLineHashSequence(), reportExternalIssue, adHocRuleMap)); } } @@ -181,11 +191,12 @@ public class TrackerRawInputFactory { return issue; } - private DefaultIssue toExternalIssue(LineHashSequence lineHashSeq, ScannerReport.ExternalIssue reportExternalIssue) { + private DefaultIssue toExternalIssue(LineHashSequence lineHashSeq, ScannerReport.ExternalIssue reportExternalIssue, Map adHocRuleMap) { DefaultIssue issue = new DefaultIssue(); init(issue); - issue.setRuleKey(RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + reportExternalIssue.getEngineId(), reportExternalIssue.getRuleId())); + RuleKey ruleKey = RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + reportExternalIssue.getEngineId(), reportExternalIssue.getRuleId()); + issue.setRuleKey(ruleKey); if (reportExternalIssue.hasTextRange()) { int startLine = reportExternalIssue.getTextRange().getStartLine(); issue.setLine(startLine); @@ -217,16 +228,15 @@ public class TrackerRawInputFactory { issue.setLocations(dbLocationsBuilder.build()); issue.setType(toRuleType(reportExternalIssue.getType())); - ruleRepository.addNewAddHocRuleIfAbsent(issue.getRuleKey(), () -> toAdHocRule(reportExternalIssue)); + ruleRepository.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> toAdHocRule(reportExternalIssue, adHocRuleMap.get(issue.ruleKey()))); return issue; } - private NewAddHocRule toAdHocRule(ScannerReport.ExternalIssue reportIssue) { - NewAddHocRule.Builder builder = new NewAddHocRule.Builder() - .setName(reportIssue.getEngineId() + " " + reportIssue.getRuleId()) - .setKey(RuleKey.of(RuleKey.EXTERNAL_RULE_REPO_PREFIX + reportIssue.getEngineId(), reportIssue.getRuleId())); - - return builder.build(); + private NewAdHocRule toAdHocRule(ScannerReport.ExternalIssue reportIssue, @Nullable ScannerReport.AdHocRule adHocRule) { + if (adHocRule != null) { + return new NewAdHocRule(adHocRule); + } + return new NewAdHocRule(reportIssue); } private RuleType toRuleType(IssueType type) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStep.java new file mode 100644 index 00000000000..4c6e4ef565f --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStep.java @@ -0,0 +1,50 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.step; + +import org.sonar.ce.task.projectanalysis.issue.RuleRepository; +import org.sonar.ce.task.step.ComputationStep; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; + +public class PersistAdHocRulesStep implements ComputationStep { + + private final DbClient dbClient; + private final RuleRepository ruleRepository; + + public PersistAdHocRulesStep(DbClient dbClient, RuleRepository ruleRepository) { + this.dbClient = dbClient; + this.ruleRepository = ruleRepository; + } + + @Override + public void execute(ComputationStep.Context context) { + try (DbSession dbSession = dbClient.openSession(false)) { + ruleRepository.saveOrUpdateAddHocRules(dbSession); + } + + } + + @Override + public String getDescription() { + return "Persist new ad hoc Rules"; + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStep.java deleted file mode 100644 index 28c871d75fa..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStep.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program 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.ce.task.projectanalysis.step; - -import org.sonar.ce.task.projectanalysis.issue.RuleRepository; -import org.sonar.ce.task.step.ComputationStep; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; - -public class PersistExternalRulesStep implements ComputationStep { - - private final DbClient dbClient; - private final RuleRepository ruleRepository; - - public PersistExternalRulesStep(DbClient dbClient, RuleRepository ruleRepository) { - this.dbClient = dbClient; - this.ruleRepository = ruleRepository; - } - - @Override - public void execute(ComputationStep.Context context) { - try (DbSession dbSession = dbClient.openSession(false)) { - ruleRepository.persistNewAddHocRules(dbSession); - } - - } - - @Override - public String getDescription() { - return "Persist new externally defined Rules"; - } - -} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java index 631acb001b2..47b209e3589 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java @@ -94,7 +94,7 @@ public class ReportComputationSteps extends AbstractComputationSteps { PersistAnalysisPropertiesStep.class, PersistMeasuresStep.class, PersistLiveMeasuresStep.class, - PersistExternalRulesStep.class, + PersistAdHocRulesStep.class, PersistIssuesStep.class, PersistProjectLinksStep.class, PersistEventsStep.class, diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderRule.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderRule.java index 0572a9b7a3c..06a0b1e1594 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderRule.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/batch/BatchReportReaderRule.java @@ -46,6 +46,7 @@ public class BatchReportReaderRule implements TestRule, BatchReportReader { private Map components = new HashMap<>(); private Map> issues = new HashMap<>(); private Map> externalIssues = new HashMap<>(); + private List adHocRules = new ArrayList<>(); private Map> duplications = new HashMap<>(); private Map> duplicationBlocks = new HashMap<>(); private Map> symbols = new HashMap<>(); @@ -179,6 +180,16 @@ public class BatchReportReaderRule implements TestRule, BatchReportReader { return closeableIterator(externalIssues.get(componentRef)); } + @Override + public CloseableIterator readAdHocRules() { + return closeableIterator(adHocRules); + } + + public BatchReportReaderRule putAdHocRules(List adHocRules) { + this.adHocRules = adHocRules; + return this; + } + public BatchReportReaderRule putIssues(int componentRef, List issues) { this.issues.put(componentRef, issues); return this; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreatorTest.java new file mode 100644 index 00000000000..295cf953716 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/AdHocRuleCreatorTest.java @@ -0,0 +1,180 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.issue; + +import org.junit.Test; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.rule.Severity; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.System2; +import org.sonar.db.DbSession; +import org.sonar.db.DbTester; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.rule.RuleDto; +import org.sonar.db.rule.RuleMetadataDto; +import org.sonar.scanner.protocol.Constants; +import org.sonar.scanner.protocol.output.ScannerReport; +import org.sonar.server.es.EsTester; +import org.sonar.server.rule.index.RuleIndexer; + +import static org.assertj.core.api.Assertions.assertThat; + +public class AdHocRuleCreatorTest { + + @org.junit.Rule + public DbTester db = DbTester.create(System2.INSTANCE); + @org.junit.Rule + public EsTester es = EsTester.create(); + + private RuleIndexer indexer = new RuleIndexer(es.client(), db.getDbClient()); + private AdHocRuleCreator underTest = new AdHocRuleCreator(db.getDbClient(), System2.INSTANCE, indexer); + private DbSession dbSession = db.getSession(); + + @Test + public void create_ad_hoc_rule_from_issue() { + OrganizationDto organization = db.organizations().insert(); + NewAdHocRule addHocRule = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build()); + + RuleDto rule = underTest.persistAndIndex(dbSession, addHocRule, organization); + + assertThat(rule).isNotNull(); + assertThat(rule.isExternal()).isTrue(); + assertThat(rule.isAdHoc()).isTrue(); + assertThat(rule.getId()).isGreaterThan(0); + assertThat(rule.getKey()).isEqualTo(RuleKey.of("external_eslint", "no-cond-assign")); + assertThat(rule.getName()).isEqualTo("eslint: no-cond-assign"); + assertThat(rule.getDescription()).isNull(); + assertThat(rule.getSeverity()).isNull(); + assertThat(rule.getType()).isEqualTo(0); + assertThat(rule.getMetadata().getAdHocName()).isNull(); + assertThat(rule.getMetadata().getAdHocDescription()).isNull(); + assertThat(rule.getMetadata().getAdHocSeverity()).isNull(); + assertThat(rule.getMetadata().getAdHocType()).isNull(); + } + + @Test + public void create_ad_hoc_rule_from_scanner_report() { + OrganizationDto organization = db.organizations().insert(); + NewAdHocRule addHocRule = new NewAdHocRule(ScannerReport.AdHocRule.newBuilder() + .setEngineId("eslint") + .setRuleId("no-cond-assign") + .setName("No condition assigned") + .setDescription("A description") + .setSeverity(Constants.Severity.BLOCKER) + .setType(ScannerReport.IssueType.BUG) + .build()); + + RuleDto rule = underTest.persistAndIndex(dbSession, addHocRule, organization); + + assertThat(rule).isNotNull(); + assertThat(rule.isExternal()).isTrue(); + assertThat(rule.isAdHoc()).isTrue(); + assertThat(rule.getId()).isGreaterThan(0); + assertThat(rule.getKey()).isEqualTo(RuleKey.of("external_eslint", "no-cond-assign")); + assertThat(rule.getName()).isEqualTo("eslint: no-cond-assign"); + assertThat(rule.getDescription()).isNull(); + assertThat(rule.getSeverity()).isNull(); + assertThat(rule.getType()).isEqualTo(0); + assertThat(rule.getMetadata().getAdHocName()).isEqualTo("No condition assigned"); + assertThat(rule.getMetadata().getAdHocDescription()).isEqualTo("A description"); + assertThat(rule.getMetadata().getAdHocSeverity()).isEqualTo(Severity.BLOCKER); + assertThat(rule.getMetadata().getAdHocType()).isEqualTo(RuleType.BUG.getDbConstant()); + } + + @Test + public void update_metadata_only() { + OrganizationDto organization = db.organizations().insert(); + NewAdHocRule addHocRule = new NewAdHocRule(ScannerReport.AdHocRule.newBuilder() + .setEngineId("eslint") + .setRuleId("no-cond-assign") + .setName("No condition assigned") + .setDescription("A description") + .setSeverity(Constants.Severity.BLOCKER) + .setType(ScannerReport.IssueType.BUG) + .build()); + RuleDto rule = underTest.persistAndIndex(dbSession, addHocRule, organization); + long creationDate = rule.getCreatedAt(); + NewAdHocRule addHocRuleUpdated = new NewAdHocRule(ScannerReport.AdHocRule.newBuilder() + .setEngineId("eslint") + .setRuleId("no-cond-assign") + .setName("No condition assigned updated") + .setDescription("A description updated") + .setSeverity(Constants.Severity.CRITICAL) + .setType(ScannerReport.IssueType.CODE_SMELL) + .build()); + + RuleDto ruleUpdated = underTest.persistAndIndex(dbSession, addHocRuleUpdated, organization); + + assertThat(ruleUpdated).isNotNull(); + assertThat(ruleUpdated.isExternal()).isTrue(); + assertThat(ruleUpdated.isAdHoc()).isTrue(); + assertThat(ruleUpdated.getId()).isGreaterThan(0); + assertThat(ruleUpdated.getKey()).isEqualTo(RuleKey.of("external_eslint", "no-cond-assign")); + assertThat(ruleUpdated.getName()).isEqualTo("eslint: no-cond-assign"); + assertThat(ruleUpdated.getDescription()).isNull(); + assertThat(ruleUpdated.getSeverity()).isNull(); + assertThat(ruleUpdated.getType()).isEqualTo(0); + assertThat(ruleUpdated.getMetadata().getAdHocName()).isEqualTo("No condition assigned updated"); + assertThat(ruleUpdated.getMetadata().getAdHocDescription()).isEqualTo("A description updated"); + assertThat(ruleUpdated.getMetadata().getAdHocSeverity()).isEqualTo(Severity.CRITICAL); + assertThat(ruleUpdated.getMetadata().getAdHocType()).isEqualTo(RuleType.CODE_SMELL.getDbConstant()); + + assertThat(ruleUpdated.getDefinition().getCreatedAt()).isEqualTo(creationDate); + assertThat(ruleUpdated.getMetadata().getCreatedAt()).isEqualTo(creationDate); + assertThat(ruleUpdated.getMetadata().getUpdatedAt()).isGreaterThan(creationDate); + } + + @Test + public void does_not_update_rule_when_no_change() { + OrganizationDto organization = db.organizations().insert(); + RuleDefinitionDto rule = db.rules().insert(r -> r.setRepositoryKey("external_eslint").setIsExternal(true).setIsAdHoc(true)); + RuleMetadataDto ruleMetadata = db.rules().insertOrUpdateMetadata(rule, organization); + + RuleDto ruleUpdated = underTest.persistAndIndex(dbSession, new NewAdHocRule(ScannerReport.AdHocRule.newBuilder() + .setEngineId("eslint") + .setRuleId(rule.getKey().rule()) + .setName(ruleMetadata.getAdHocName()) + .setDescription(ruleMetadata.getAdHocDescription()) + .setSeverity(Constants.Severity.valueOf(ruleMetadata.getAdHocSeverity())) + .setType(ScannerReport.IssueType.forNumber(ruleMetadata.getAdHocType())) + .build()), + organization); + + assertThat(ruleUpdated).isNotNull(); + assertThat(ruleUpdated.isExternal()).isTrue(); + assertThat(ruleUpdated.isAdHoc()).isTrue(); + assertThat(ruleUpdated.getKey()).isEqualTo(rule.getKey()); + assertThat(ruleUpdated.getName()).isEqualTo(rule.getName()); + assertThat(ruleUpdated.getDescription()).isEqualTo(rule.getDescription()); + assertThat(ruleUpdated.getSeverity()).isEqualTo(rule.getSeverity()); + assertThat(ruleUpdated.getType()).isEqualTo(rule.getType()); + assertThat(ruleUpdated.getDefinition().getCreatedAt()).isEqualTo(rule.getCreatedAt()); + assertThat(ruleUpdated.getDefinition().getUpdatedAt()).isEqualTo(rule.getUpdatedAt()); + + assertThat(ruleUpdated.getMetadata().getAdHocName()).isEqualTo(ruleMetadata.getAdHocName()); + assertThat(ruleUpdated.getMetadata().getAdHocDescription()).isEqualTo(ruleMetadata.getAdHocDescription()); + assertThat(ruleUpdated.getMetadata().getAdHocSeverity()).isEqualTo(ruleMetadata.getAdHocSeverity()); + assertThat(ruleUpdated.getMetadata().getAdHocType()).isEqualTo(ruleMetadata.getAdHocType()); + assertThat(ruleUpdated.getMetadata().getCreatedAt()).isEqualTo(rule.getCreatedAt()); + assertThat(ruleUpdated.getMetadata().getUpdatedAt()).isEqualTo(rule.getUpdatedAt()); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java index 058cbf3ae66..9acde59120f 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java @@ -39,6 +39,7 @@ public class DumbRule implements Rule { private DebtRemediationFunction function; private String pluginKey; private boolean isExternal; + private boolean isAdHoc; public DumbRule(RuleKey key) { this.key = key; @@ -90,6 +91,11 @@ public class DumbRule implements Rule { return isExternal; } + @Override + public boolean isAdHoc() { + return isAdHoc; + } + public DumbRule setId(Integer id) { this.id = id; return this; @@ -130,4 +136,9 @@ public class DumbRule implements Rule { return this; } + public DumbRule setIsAdHoc(boolean isAdHoc) { + this.isAdHoc = isAdHoc; + return this; + } + } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java new file mode 100644 index 00000000000..982d4d2a894 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/NewAdHocRuleTest.java @@ -0,0 +1,55 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.issue; + +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.scanner.protocol.output.ScannerReport; + +import static org.assertj.core.api.Assertions.assertThat; + +public class NewAdHocRuleTest { + @org.junit.Rule + public ExpectedException exception = ExpectedException.none(); + + @Test + public void fail_if_engine_id_is_not_set() { + exception.expect(IllegalArgumentException.class); + exception.expectMessage("'engine id' not expected to be null for an ad hoc rule"); + + new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().build()); + } + + @Test + public void test_equals_and_hashcode() { + NewAdHocRule adHocRule1 = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build()); + NewAdHocRule adHocRule2 = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build()); + NewAdHocRule anotherAdHocRule = new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("another").build()); + + assertThat(adHocRule1).isEqualTo(adHocRule1); + assertThat(adHocRule1).isEqualTo(adHocRule2); + assertThat(adHocRule1).isNotEqualTo(null); + assertThat(adHocRule1).isNotEqualTo(anotherAdHocRule); + + assertThat(adHocRule1.hashCode()).isEqualTo(adHocRule1.hashCode()); + assertThat(adHocRule1.hashCode()).isEqualTo(adHocRule2.hashCode()); + assertThat(adHocRule1.hashCode()).isNotEqualTo(anotherAdHocRule.hashCode()); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImplTest.java index c359b6919d1..44111ca1b0f 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImplTest.java @@ -37,8 +37,7 @@ import org.sonar.db.rule.DeprecatedRuleKeyDto; import org.sonar.db.rule.RuleDao; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleDto; -import org.sonar.server.rule.AddHocRuleCreator; -import org.sonar.server.rule.NewAddHocRule; +import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.server.rule.index.RuleIndexer; import static org.assertj.core.api.Assertions.assertThat; @@ -78,8 +77,8 @@ public class RuleRepositoryImplTest { private RuleDao ruleDao = mock(RuleDao.class); private RuleIndexer ruleIndexer = mock(RuleIndexer.class); - private AddHocRuleCreator addHocRuleCreator = new AddHocRuleCreator(db.getDbClient(), System2.INSTANCE, ruleIndexer); - private RuleRepositoryImpl underTest = new RuleRepositoryImpl(addHocRuleCreator, dbClient, analysisMetadataHolder); + private AdHocRuleCreator adHocRuleCreator = new AdHocRuleCreator(db.getDbClient(), System2.INSTANCE, ruleIndexer); + private RuleRepositoryImpl underTest = new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder); @Before public void setUp() throws Exception { @@ -268,15 +267,11 @@ public class RuleRepositoryImplTest { @Test public void accept_new_externally_defined_Rules() { - RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); + RuleKey ruleKey = RuleKey.of("external_eslint", "no-cond-assign"); - underTest.addNewAddHocRuleIfAbsent(ruleKey, () -> new NewAddHocRule.Builder() - .setKey(ruleKey) - .setPluginKey("eslint") - .build()); + underTest.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build())); assertThat(underTest.getByKey(ruleKey)).isNotNull(); - assertThat(underTest.getByKey(ruleKey).getPluginKey()).isEqualTo("eslint"); assertThat(underTest.getByKey(ruleKey).getType()).isNull(); RuleDao ruleDao = dbClient.ruleDao(); @@ -286,15 +281,12 @@ public class RuleRepositoryImplTest { @Test public void persist_new_externally_defined_Rules() { - underTest = new RuleRepositoryImpl(addHocRuleCreator, db.getDbClient(), analysisMetadataHolder); + underTest = new RuleRepositoryImpl(adHocRuleCreator, db.getDbClient(), analysisMetadataHolder); - RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); - underTest.addNewAddHocRuleIfAbsent(ruleKey, () -> new NewAddHocRule.Builder() - .setKey(ruleKey) - .setPluginKey("eslint") - .build()); + RuleKey ruleKey = RuleKey.of("external_eslint", "no-cond-assign"); + underTest.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build())); - underTest.persistNewAddHocRules(db.getSession()); + underTest.saveOrUpdateAddHocRules(db.getSession()); db.commit(); Optional ruleDefinitionDto = db.getDbClient().ruleDao().selectDefinitionByKey(db.getSession(), ruleKey); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryRule.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryRule.java index ee875f21287..a676e006511 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryRule.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryRule.java @@ -26,7 +26,6 @@ import java.util.function.Supplier; import org.junit.rules.ExternalResource; import org.sonar.api.rule.RuleKey; import org.sonar.db.DbSession; -import org.sonar.server.rule.NewAddHocRule; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; @@ -35,7 +34,7 @@ public class RuleRepositoryRule extends ExternalResource implements RuleReposito private final Map rulesByKey = new HashMap<>(); private final Map rulesById = new HashMap<>(); - private final Map newExternalRulesById = new HashMap<>(); + private final Map newExternalRulesById = new HashMap<>(); @Override protected void after() { @@ -68,7 +67,7 @@ public class RuleRepositoryRule extends ExternalResource implements RuleReposito } @Override - public void persistNewAddHocRules(DbSession dbSession) { + public void saveOrUpdateAddHocRules(DbSession dbSession) { throw new UnsupportedOperationException(); } @@ -87,7 +86,7 @@ public class RuleRepositoryRule extends ExternalResource implements RuleReposito } @Override - public void addNewAddHocRuleIfAbsent(RuleKey ruleKey, Supplier ruleSupplier) { + public void addOrUpdateAddHocRuleIfNeeded(RuleKey ruleKey, Supplier ruleSupplier) { newExternalRulesById.computeIfAbsent(ruleKey, k -> ruleSupplier.get()); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStepTest.java new file mode 100644 index 00000000000..98ccf035b7a --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistAdHocRulesStepTest.java @@ -0,0 +1,114 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program 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. + * + * This program 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.ce.task.projectanalysis.step; + +import java.util.Optional; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.System2; +import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; +import org.sonar.ce.task.projectanalysis.issue.AdHocRuleCreator; +import org.sonar.ce.task.projectanalysis.issue.NewAdHocRule; +import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryImpl; +import org.sonar.ce.task.step.ComputationStep; +import org.sonar.ce.task.step.TestComputationStepContext; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.rule.RuleDao; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.scanner.protocol.output.ScannerReport; +import org.sonar.server.es.EsTester; +import org.sonar.server.rule.index.RuleIndexDefinition; +import org.sonar.server.rule.index.RuleIndexer; + +import static org.assertj.core.api.Assertions.assertThat; + +public class PersistAdHocRulesStepTest extends BaseStepTest { + + @Rule + public DbTester db = DbTester.create(System2.INSTANCE); + + @Rule + public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule() + .setOrganizationUuid("org-1", "qg-uuid-1"); + + private DbClient dbClient = db.getDbClient(); + + private ComputationStep underTest; + private RuleRepositoryImpl ruleRepository; + + @org.junit.Rule + public EsTester es = EsTester.create(); + + private RuleIndexer indexer = new RuleIndexer(es.client(), dbClient); + private AdHocRuleCreator adHocRuleCreator = new AdHocRuleCreator(dbClient, System2.INSTANCE, indexer); + + @Override + protected ComputationStep step() { + return underTest; + } + + @Before + public void setup() { + ruleRepository = new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder); + underTest = new PersistAdHocRulesStep(dbClient, ruleRepository); + } + + @Test + public void persist_and_index_new_ad_hoc_rules() { + + RuleKey ruleKey = RuleKey.of("external_eslint", "no-cond-assign"); + ruleRepository.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build())); + + underTest.execute(new TestComputationStepContext()); + + RuleDao ruleDao = dbClient.ruleDao(); + Optional ruleDefinitionDtoOptional = ruleDao.selectDefinitionByKey(dbClient.openSession(false), ruleKey); + assertThat(ruleDefinitionDtoOptional).isPresent(); + + RuleDefinitionDto reloaded = ruleDefinitionDtoOptional.get(); + assertThat(reloaded.getRuleKey()).isEqualTo("no-cond-assign"); + assertThat(reloaded.getRepositoryKey()).isEqualTo("external_eslint"); + assertThat(reloaded.isExternal()).isTrue(); + assertThat(reloaded.isAdHoc()).isTrue(); + assertThat(reloaded.getType()).isEqualTo(0); + assertThat(reloaded.getSeverity()).isNull(); + assertThat(reloaded.getName()).isEqualTo("eslint: no-cond-assign"); + + assertThat(es.countDocuments(RuleIndexDefinition.INDEX_TYPE_RULE)).isEqualTo(1l); + assertThat(es.getDocuments(RuleIndexDefinition.INDEX_TYPE_RULE).iterator().next().getId()).isEqualTo(Integer.toString(reloaded.getId())); + } + + @Test + public void do_not_persist_existing_external_rules() { + RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); + db.rules().insert(ruleKey, r -> r.setIsExternal(true)); + ruleRepository.addOrUpdateAddHocRuleIfNeeded(ruleKey, () -> new NewAdHocRule(ScannerReport.ExternalIssue.newBuilder().setEngineId("eslint").setRuleId("no-cond-assign").build())); + + underTest.execute(new TestComputationStepContext()); + + RuleDao ruleDao = dbClient.ruleDao(); + assertThat(ruleDao.selectAllDefinitions(dbClient.openSession(false))).hasSize(1); + assertThat(es.countDocuments(RuleIndexDefinition.INDEX_TYPE_RULE)).isZero(); + } + +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStepTest.java deleted file mode 100644 index 2832628b193..00000000000 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistExternalRulesStepTest.java +++ /dev/null @@ -1,120 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program 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.ce.task.projectanalysis.step; - -import java.util.Optional; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.utils.System2; -import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; -import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryImpl; -import org.sonar.ce.task.step.ComputationStep; -import org.sonar.ce.task.step.TestComputationStepContext; -import org.sonar.db.DbClient; -import org.sonar.db.DbTester; -import org.sonar.db.rule.RuleDao; -import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.server.es.EsTester; -import org.sonar.server.rule.AddHocRuleCreator; -import org.sonar.server.rule.NewAddHocRule; -import org.sonar.server.rule.index.RuleIndexDefinition; -import org.sonar.server.rule.index.RuleIndexer; - -import static org.assertj.core.api.Assertions.assertThat; - -public class PersistExternalRulesStepTest extends BaseStepTest { - - @Rule - public DbTester db = DbTester.create(System2.INSTANCE); - - @Rule - public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule() - .setOrganizationUuid("org-1", "qg-uuid-1"); - - private DbClient dbClient = db.getDbClient(); - - private ComputationStep underTest; - private RuleRepositoryImpl ruleRepository; - - @org.junit.Rule - public EsTester es = EsTester.create(); - - private RuleIndexer indexer = new RuleIndexer(es.client(), dbClient); - private AddHocRuleCreator addHocRuleCreator = new AddHocRuleCreator(dbClient, System2.INSTANCE, indexer); - - @Override - protected ComputationStep step() { - return underTest; - } - - @Before - public void setup() { - ruleRepository = new RuleRepositoryImpl(addHocRuleCreator, dbClient, analysisMetadataHolder); - underTest = new PersistExternalRulesStep(dbClient, ruleRepository); - } - - @Test - public void persist_and_index_new_external_rules() { - - RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); - ruleRepository.addNewAddHocRuleIfAbsent(ruleKey, () -> new NewAddHocRule.Builder() - .setKey(ruleKey) - .setPluginKey("eslint") - .setName("eslint:no-cond-assign") - .build()); - - underTest.execute(new TestComputationStepContext()); - - RuleDao ruleDao = dbClient.ruleDao(); - Optional ruleDefinitionDtoOptional = ruleDao.selectDefinitionByKey(dbClient.openSession(false), ruleKey); - assertThat(ruleDefinitionDtoOptional).isPresent(); - - RuleDefinitionDto reloaded = ruleDefinitionDtoOptional.get(); - assertThat(reloaded.getRuleKey()).isEqualTo("no-cond-assign"); - assertThat(reloaded.getRepositoryKey()).isEqualTo("eslint"); - assertThat(reloaded.isExternal()).isTrue(); - assertThat(reloaded.getType()).isEqualTo(0); - assertThat(reloaded.getSeverity()).isNull(); - assertThat(reloaded.getName()).isEqualTo("eslint:no-cond-assign"); - assertThat(reloaded.getPluginKey()).isEqualTo("eslint"); - - assertThat(es.countDocuments(RuleIndexDefinition.INDEX_TYPE_RULE)).isEqualTo(1l); - assertThat(es.getDocuments(RuleIndexDefinition.INDEX_TYPE_RULE).iterator().next().getId()).isEqualTo(Integer.toString(reloaded.getId())); - } - - @Test - public void do_not_persist_existing_external_rules() { - RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); - db.rules().insert(ruleKey, r -> r.setIsExternal(true)); - ruleRepository.addNewAddHocRuleIfAbsent(ruleKey, () -> new NewAddHocRule.Builder() - .setKey(ruleKey) - .setPluginKey("eslint") - .build()); - - underTest.execute(new TestComputationStepContext()); - - RuleDao ruleDao = dbClient.ruleDao(); - assertThat(ruleDao.selectAllDefinitions(dbClient.openSession(false))).hasSize(1); - assertThat(es.countDocuments(RuleIndexDefinition.INDEX_TYPE_RULE)).isZero(); - } - -} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java index e5cdb6f6a12..b4f9e66e558 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java @@ -32,6 +32,7 @@ import org.sonar.api.rules.RuleType; import org.sonar.api.utils.System2; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.batch.BatchReportReaderRule; +import org.sonar.ce.task.projectanalysis.issue.AdHocRuleCreator; import org.sonar.ce.task.projectanalysis.issue.IssueCache; import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryImpl; import org.sonar.ce.task.projectanalysis.issue.UpdateConflictResolver; @@ -53,7 +54,6 @@ import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleTesting; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.server.issue.IssueStorage; -import org.sonar.server.rule.AddHocRuleCreator; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; @@ -85,7 +85,7 @@ public class PersistIssuesStepTest extends BaseStepTest { private IssueCache issueCache; private ComputationStep underTest; - private AddHocRuleCreator addHocRuleCreator = mock(AddHocRuleCreator.class); + private AdHocRuleCreator adHocRuleCreator = mock(AdHocRuleCreator.class); @Override protected ComputationStep step() { @@ -99,7 +99,7 @@ public class PersistIssuesStepTest extends BaseStepTest { when(system2.now()).thenReturn(NOW); reportReader.setMetadata(ScannerReport.Metadata.getDefaultInstance()); - underTest = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(addHocRuleCreator, dbClient, analysisMetadataHolder), issueCache, + underTest = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder), issueCache, new IssueStorage()); } diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java index dacec29f0b0..58669757d7d 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java @@ -59,6 +59,7 @@ import org.sonar.ce.queue.CeQueueCleaner; import org.sonar.ce.queue.PurgeCeActivities; import org.sonar.ce.task.projectanalysis.ProjectAnalysisTaskModule; import org.sonar.ce.task.projectanalysis.analysis.ProjectConfigurationFactory; +import org.sonar.ce.task.projectanalysis.issue.AdHocRuleCreator; import org.sonar.ce.task.projectanalysis.notification.ReportAnalysisFailureNotificationModule; import org.sonar.ce.taskprocessor.CeProcessingScheduler; import org.sonar.ce.taskprocessor.CeTaskProcessorModule; @@ -141,7 +142,6 @@ import org.sonar.server.qualitygate.QualityGateEvaluatorImpl; import org.sonar.server.qualitygate.QualityGateFinder; import org.sonar.server.qualityprofile.index.ActiveRuleIndexer; import org.sonar.server.rule.DefaultRuleFinder; -import org.sonar.server.rule.AddHocRuleCreator; import org.sonar.server.rule.index.RuleIndex; import org.sonar.server.rule.index.RuleIndexer; import org.sonar.server.setting.DatabaseSettingLoader; @@ -370,7 +370,7 @@ public class ComputeEngineContainerImpl implements ComputeEngineContainer { XMLRuleParser.class, DefaultRuleFinder.class, RulesDefinitionXmlLoader.class, - AddHocRuleCreator.class, + AdHocRuleCreator.class, RuleIndexer.class, // languages diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/rule/AddHocRuleCreator.java b/server/sonar-server-common/src/main/java/org/sonar/server/rule/AddHocRuleCreator.java deleted file mode 100644 index a2f938f9e7a..00000000000 --- a/server/sonar-server-common/src/main/java/org/sonar/server/rule/AddHocRuleCreator.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.rule; - -import org.sonar.api.utils.System2; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.rule.RuleDao; -import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.rule.RuleDto; -import org.sonar.server.rule.index.RuleIndexer; - -import static org.sonar.api.rule.RuleStatus.READY; -import static org.sonar.db.rule.RuleDto.Scope.ALL; - -public class AddHocRuleCreator { - - private final DbClient dbClient; - private final System2 system2; - private final RuleIndexer ruleIndexer; - - public AddHocRuleCreator(DbClient dbClient, System2 system2, RuleIndexer ruleIndexer) { - this.dbClient = dbClient; - this.system2 = system2; - this.ruleIndexer = ruleIndexer; - } - - /** - * Persists a new add hoc rule in the DB and indexes it. - * @return the rule that was inserted in the DB, which includes the generated ID. - */ - public RuleDto persistAndIndex(DbSession dbSession, NewAddHocRule adHoc) { - RuleDao dao = dbClient.ruleDao(); - dao.insert(dbSession, new RuleDefinitionDto() - .setRuleKey(adHoc.getKey()) - .setPluginKey(adHoc.getPluginKey()) - .setIsExternal(true) - .setName(adHoc.getName()) - .setIsAdHoc(true) - .setScope(ALL) - .setStatus(READY) - .setCreatedAt(system2.now()) - .setUpdatedAt(system2.now())); - - RuleDto ruleDto = dao.selectOrFailByKey(dbSession, adHoc.getKey()); - ruleIndexer.commitAndIndex(dbSession, ruleDto.getId()); - return ruleDto; - } - -} diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/rule/NewAddHocRule.java b/server/sonar-server-common/src/main/java/org/sonar/server/rule/NewAddHocRule.java deleted file mode 100644 index c84896b7791..00000000000 --- a/server/sonar-server-common/src/main/java/org/sonar/server/rule/NewAddHocRule.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.rule; - -import java.util.Objects; -import javax.annotation.concurrent.Immutable; -import org.sonar.api.rule.RuleKey; - -@Immutable -public class NewAddHocRule { - private final RuleKey key; - private final String name; - private final String pluginKey; - - private NewAddHocRule(Builder builder) { - Objects.requireNonNull(builder.key, "'key' not expected to be null for an external rule"); - this.key = builder.key; - this.pluginKey = builder.pluginKey; - this.name = builder.name; - } - - public RuleKey getKey() { - return key; - } - - public String getName() { - return name; - } - - public String getPluginKey() { - return pluginKey; - } - - public static class Builder { - private RuleKey key; - private String pluginKey; - private String name; - - public Builder setKey(RuleKey key) { - this.key = key; - return this; - } - - public Builder setName(String name) { - this.name = name; - return this; - } - - public String name() { - return name; - } - - public NewAddHocRule build() { - return new NewAddHocRule(this); - } - - public Builder setPluginKey(String pluginKey) { - this.pluginKey = pluginKey; - return this; - } - } -} diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/rule/AddHocRuleCreatorTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/rule/AddHocRuleCreatorTest.java deleted file mode 100644 index db5269e1b4b..00000000000 --- a/server/sonar-server-common/src/test/java/org/sonar/server/rule/AddHocRuleCreatorTest.java +++ /dev/null @@ -1,65 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.rule; - -import org.junit.Test; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.utils.System2; -import org.sonar.db.DbSession; -import org.sonar.db.DbTester; -import org.sonar.db.rule.RuleDto; -import org.sonar.server.es.EsTester; -import org.sonar.server.rule.index.RuleIndexer; - -import static org.assertj.core.api.Assertions.assertThat; - -public class AddHocRuleCreatorTest { - - @org.junit.Rule - public DbTester dbTester = DbTester.create(System2.INSTANCE); - @org.junit.Rule - public EsTester es = EsTester.create(); - - private RuleIndexer indexer = new RuleIndexer(es.client(), dbTester.getDbClient()); - private AddHocRuleCreator underTest = new AddHocRuleCreator(dbTester.getDbClient(), System2.INSTANCE, indexer); - private DbSession dbSession = dbTester.getSession(); - - @Test - public void create_external_rule() { - RuleKey ruleKey = RuleKey.of("eslint", "no-cond-assign"); - NewAddHocRule externalRule = new NewAddHocRule.Builder() - .setKey(ruleKey) - .setPluginKey("eslint") - .setName("name") - .build(); - - RuleDto rule1 = underTest.persistAndIndex(dbSession, externalRule); - - assertThat(rule1).isNotNull(); - assertThat(rule1.isExternal()).isTrue(); - assertThat(rule1.isAdHoc()).isTrue(); - assertThat(rule1.getId()).isGreaterThan(0); - assertThat(rule1.getKey()).isEqualTo(ruleKey); - assertThat(rule1.getPluginKey()).isEqualTo("eslint"); - assertThat(rule1.getName()).isEqualTo("name"); - assertThat(rule1.getType()).isEqualTo(0); - } - -} diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/NewAddHocRuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/NewAddHocRuleTest.java deleted file mode 100644 index 11917668f11..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/NewAddHocRuleTest.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2018 SonarSource SA - * mailto:info AT sonarsource DOT com - * - * This program 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. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with this program; if not, write to the Free Software Foundation, - * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - */ -package org.sonar.server.rule; - -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.sonar.api.rule.RuleKey; - -import static org.assertj.core.api.Assertions.assertThat; - -public class NewAddHocRuleTest { - @org.junit.Rule - public ExpectedException exception = ExpectedException.none(); - - @Test - public void should_build_new_external_rule() { - NewAddHocRule.Builder builder = new NewAddHocRule.Builder() - .setKey(RuleKey.of("repo", "rule")) - .setPluginKey("repo") - .setName("name"); - - assertThat(builder.name()).isEqualTo("name"); - - NewAddHocRule rule = builder.build(); - - assertThat(rule.getName()).isEqualTo("name"); - assertThat(rule.getPluginKey()).isEqualTo("repo"); - } - - @Test - public void fail_if_rule_key_is_not_set() { - exception.expect(NullPointerException.class); - exception.expectMessage("'key' not expected to be null for an external rule"); - - new NewAddHocRule.Builder() - .build(); - } -} diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/AdHocRule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/AdHocRule.java index 6708bcd8533..aa7f5483dd9 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/AdHocRule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/AdHocRule.java @@ -19,6 +19,7 @@ */ package org.sonar.api.batch.sensor.rule; +import javax.annotation.CheckForNull; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.Sensor; import org.sonar.api.rules.RuleType; @@ -47,6 +48,7 @@ public interface AdHocRule { /** * Description of the rule. */ + @CheckForNull String description(); /** diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/NewAdHocRule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/NewAdHocRule.java index 993b19eeb02..03159df8589 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/NewAdHocRule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/NewAdHocRule.java @@ -19,6 +19,7 @@ */ package org.sonar.api.batch.sensor.rule; +import javax.annotation.Nullable; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.Sensor; import org.sonar.api.rules.RuleType; @@ -50,7 +51,7 @@ public interface NewAdHocRule { /** * The description of the rule. */ - NewAdHocRule description(String description); + NewAdHocRule description(@Nullable String description); /** * Type of the rule. diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java index 0251f5ce839..eb0b8eb1d6e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRule.java @@ -19,6 +19,7 @@ */ package org.sonar.api.batch.sensor.rule.internal; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.batch.rule.Severity; import org.sonar.api.batch.sensor.internal.DefaultStorable; @@ -67,6 +68,7 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA return name; } + @CheckForNull @Override public String description() { return description; @@ -82,7 +84,6 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA checkState(isNotBlank(engineId), "Engine id is mandatory on ad hoc rule"); checkState(isNotBlank(ruleId), "Rule id is mandatory on ad hoc rule"); checkState(isNotBlank(name), "Name is mandatory on every ad hoc rule"); - checkState(isNotBlank(description), "Description is mandatory on every ad hoc rule"); checkState(severity != null, "Severity is mandatory on every ad hoc rule"); checkState(type != null, "Type is mandatory on every ad hoc rule"); storage.store(this); @@ -112,7 +113,7 @@ public class DefaultAdHocRule extends DefaultStorable implements AdHocRule, NewA } @Override - public DefaultAdHocRule description(String description) { + public DefaultAdHocRule description(@Nullable String description) { this.description = description; return this; } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java index 7a2b38084bf..241ae701379 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/rule/internal/DefaultAdHocRuleTest.java @@ -51,6 +51,21 @@ public class DefaultAdHocRuleTest { verify(storage).store(any(DefaultAdHocRule.class)); } + + @Test + public void description_is_optional() { + SensorStorage storage = mock(SensorStorage.class); + new DefaultAdHocRule(storage) + .engineId("engine") + .ruleId("ruleId") + .name("name") + .severity(Severity.BLOCKER) + .type(RuleType.CODE_SMELL) + .save(); + + verify(storage).store(any(DefaultAdHocRule.class)); + } + @Test public void fail_to_store_if_no_engine_id() { SensorStorage storage = mock(SensorStorage.class); @@ -99,21 +114,6 @@ public class DefaultAdHocRuleTest { rule.save(); } - @Test - public void fail_to_store_if_no_description() { - SensorStorage storage = mock(SensorStorage.class); - NewAdHocRule rule = new DefaultAdHocRule(storage) - .engineId("engine") - .ruleId("ruleId") - .name("name") - .description(" ") - .severity(Severity.BLOCKER) - .type(RuleType.CODE_SMELL); - - exception.expect(IllegalStateException.class); - exception.expectMessage("Description is mandatory"); - rule.save(); - } @Test public void fail_to_store_if_no_severity() { diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java index f7f3b9898d6..88f47c292ab 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java @@ -407,7 +407,10 @@ public class DefaultSensorStorage implements SensorStorage { builder.setEngineId(adHocRule.engineId()); builder.setRuleId(adHocRule.ruleId()); builder.setName(adHocRule.name()); - builder.setDescription(adHocRule.description()); + String description = adHocRule.description(); + if (description != null) { + builder.setDescription(description); + } builder.setSeverity(Constants.Severity.valueOf(adHocRule.severity().name())); builder.setType(ScannerReport.IssueType.valueOf(adHocRule.type().name())); writer.appendAdHocRule(builder.build());