Browse Source

SONAR-8867 add and use RuleDao#selectAllDefinitions

and added organizationUuid parameter to RuleDao#selectAll
tags/6.4-RC1
Sébastien Lesaint 7 years ago
parent
commit
46b893b80e

+ 5
- 1
server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleDao.java View File

@@ -86,10 +86,14 @@ public class RuleDao implements Dao {
mapper(session).selectEnabled(resultHandler);
}

public List<RuleDto> selectAll(DbSession session) {
public List<RuleDto> selectAll(DbSession session, String organizationUuid) {
return mapper(session).selectAll();
}

public List<RuleDefinitionDto> selectAllDefinitions(DbSession session) {
return mapper(session).selectAllDefinitions();
}

public List<RuleDto> selectByQuery(DbSession session, RuleQuery ruleQuery) {
return mapper(session).selectByQuery(ruleQuery);
}

+ 2
- 0
server/sonar-db-dao/src/main/java/org/sonar/db/rule/RuleMapper.java View File

@@ -29,6 +29,8 @@ public interface RuleMapper {

List<RuleDto> selectAll();

List<RuleDefinitionDto> selectAllDefinitions();

void selectEnabled(ResultHandler resultHandler);

RuleDto selectById(long id);

+ 7
- 0
server/sonar-db-dao/src/main/resources/org/sonar/db/rule/RuleMapper.xml View File

@@ -45,6 +45,13 @@
rules r
</select>

<select id="selectAllDefinitions" resultType="org.sonar.db.rule.RuleDefinitionDto">
select
<include refid="selectDefinitionColumns"/>
from
rules r
</select>

<select id="selectEnabled" resultType="org.sonar.db.rule.RuleDefinitionDto">
select
<include refid="selectDefinitionColumns"/>

+ 10
- 1
server/sonar-db-dao/src/test/java/org/sonar/db/rule/RuleDaoTest.java View File

@@ -180,7 +180,16 @@ public class RuleDaoTest {
public void selectAll() {
dbTester.prepareDbUnit(getClass(), "shared.xml");

List<RuleDto> ruleDtos = underTest.selectAll(dbTester.getSession());
List<RuleDto> ruleDtos = underTest.selectAll(dbTester.getSession(), "org-1");

assertThat(ruleDtos).extracting("id").containsOnly(1, 2, 10);
}

@Test
public void selectAllDefinitions() {
dbTester.prepareDbUnit(getClass(), "shared.xml");

List<RuleDefinitionDto> ruleDtos = underTest.selectAllDefinitions(dbTester.getSession());

assertThat(ruleDtos).extracting("id").containsOnly(1, 2, 10);
}

+ 6
- 2
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/RuleRepositoryImpl.java View File

@@ -27,6 +27,7 @@ import org.sonar.api.rule.RuleKey;
import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.db.rule.RuleDto;
import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolder;

import static com.google.common.base.Preconditions.checkArgument;
import static java.util.Objects.requireNonNull;
@@ -39,9 +40,11 @@ public class RuleRepositoryImpl implements RuleRepository {
private Map<Integer, Rule> rulesById;

private final DbClient dbClient;
private final AnalysisMetadataHolder analysisMetadataHolder;

public RuleRepositoryImpl(DbClient dbClient) {
public RuleRepositoryImpl(DbClient dbClient, AnalysisMetadataHolder analysisMetadataHolder) {
this.dbClient = dbClient;
this.analysisMetadataHolder = analysisMetadataHolder;
}

@Override
@@ -95,7 +98,8 @@ public class RuleRepositoryImpl implements RuleRepository {
private void loadRulesFromDb(DbSession dbSession) {
ImmutableMap.Builder<RuleKey, Rule> rulesByKeyBuilder = ImmutableMap.builder();
ImmutableMap.Builder<Integer, Rule> rulesByIdBuilder = ImmutableMap.builder();
for (RuleDto ruleDto : dbClient.ruleDao().selectAll(dbSession)) {
String organizationUuid = analysisMetadataHolder.getOrganization().getUuid();
for (RuleDto ruleDto : dbClient.ruleDao().selectAll(dbSession, organizationUuid)) {
Rule rule = new RuleImpl(ruleDto);
rulesByKeyBuilder.put(ruleDto.getKey(), rule);
rulesByIdBuilder.put(ruleDto.getId(), rule);

+ 1
- 1
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/CachingRuleActivatorContextFactory.java View File

@@ -49,7 +49,7 @@ public class CachingRuleActivatorContextFactory extends RuleActivatorContextFact
@Override
public void start() {
try (DbSession dbSession = dbClient.openSession(false)) {
dbClient.ruleDao().selectAll(dbSession).forEach(rule -> rulesByRuleKey.put(rule.getKey(), rule.getDefinition()));
dbClient.ruleDao().selectAllDefinitions(dbSession).forEach(rule -> rulesByRuleKey.put(rule.getKey(), rule));
}
}


+ 24
- 36
server/sonar-server/src/main/java/org/sonar/server/rule/RegisterRules.java View File

@@ -19,7 +19,6 @@
*/
package org.sonar.server.rule;

import com.google.common.base.Function;
import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
@@ -29,7 +28,6 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import org.apache.commons.lang.ObjectUtils;
import org.apache.commons.lang.StringUtils;
@@ -50,9 +48,7 @@ import org.sonar.db.DbSession;
import org.sonar.db.qualityprofile.ActiveRuleDto;
import org.sonar.db.qualityprofile.ActiveRuleParamDto;
import org.sonar.db.rule.RuleDefinitionDto;
import org.sonar.db.rule.RuleDto;
import org.sonar.db.rule.RuleDto.Format;
import org.sonar.db.rule.RuleMetadataDto;
import org.sonar.db.rule.RuleParamDto;
import org.sonar.db.rule.RuleRepositoryDto;
import org.sonar.server.qualityprofile.ActiveRuleChange;
@@ -94,7 +90,7 @@ public class RegisterRules implements Startable {
Profiler profiler = Profiler.create(LOG).startInfo("Register rules");
DbSession session = dbClient.openSession(false);
try {
Map<RuleKey, RuleDto> allRules = loadRules(session);
Map<RuleKey, RuleDefinitionDto> allRules = loadRules(session);

RulesDefinition.Context context = defLoader.load();
for (RulesDefinition.ExtendedRepository repoDef : getRepositories(context)) {
@@ -105,7 +101,7 @@ public class RegisterRules implements Startable {
session.commit();
}
}
List<RuleDto> activeRules = processRemainingDbRules(allRules.values(), session);
List<RuleDefinitionDto> activeRules = processRemainingDbRules(allRules.values(), session);
List<ActiveRuleChange> changes = removeActiveRulesOnStillExistingRepositories(session, activeRules, context);
session.commit();

@@ -133,11 +129,11 @@ public class RegisterRules implements Startable {
// nothing
}

private void registerRule(RulesDefinition.Rule ruleDef, Map<RuleKey, RuleDto> allRules, DbSession session) {
private void registerRule(RulesDefinition.Rule ruleDef, Map<RuleKey, RuleDefinitionDto> allRules, DbSession session) {
RuleKey ruleKey = RuleKey.of(ruleDef.repository().key(), ruleDef.key());

RuleDto existingRule = allRules.remove(ruleKey);
RuleDefinitionDto rule = existingRule == null ? createRuleDto(ruleDef, session) : existingRule.getDefinition();
RuleDefinitionDto existingRule = allRules.remove(ruleKey);
RuleDefinitionDto rule = existingRule == null ? createRuleDto(ruleDef, session) : existingRule;

boolean executeUpdate = false;
if (mergeRule(ruleDef, rule)) {
@@ -159,9 +155,9 @@ public class RegisterRules implements Startable {
mergeParams(ruleDef, rule, session);
}

private Map<RuleKey, RuleDto> loadRules(DbSession session) {
Map<RuleKey, RuleDto> rules = new HashMap<>();
for (RuleDto rule : dbClient.ruleDao().selectAll(session)) {
private Map<RuleKey, RuleDefinitionDto> loadRules(DbSession session) {
Map<RuleKey, RuleDefinitionDto> rules = new HashMap<>();
for (RuleDefinitionDto rule : dbClient.ruleDao().selectAllDefinitions(session)) {
rules.put(rule.getKey(), rule);
}
return rules;
@@ -371,12 +367,12 @@ public class RegisterRules implements Startable {
return changed;
}

private List<RuleDto> processRemainingDbRules(Collection<RuleDto> existingRules, DbSession session) {
private List<RuleDefinitionDto> processRemainingDbRules(Collection<RuleDefinitionDto> existingRules, DbSession session) {
// custom rules check status of template, so they must be processed at the end
List<RuleDto> customRules = newArrayList();
List<RuleDto> removedRules = newArrayList();
List<RuleDefinitionDto> customRules = newArrayList();
List<RuleDefinitionDto> removedRules = newArrayList();

for (RuleDto rule : existingRules) {
for (RuleDefinitionDto rule : existingRules) {
if (rule.getTemplateId() != null) {
customRules.add(rule);
} else if (rule.getStatus() != RuleStatus.REMOVED) {
@@ -384,13 +380,13 @@ public class RegisterRules implements Startable {
}
}

for (RuleDto customRule : customRules) {
for (RuleDefinitionDto customRule : customRules) {
Integer templateId = customRule.getTemplateId();
checkNotNull(templateId, "Template id of the custom rule '%s' is null", customRule);
Optional<RuleDefinitionDto> template = dbClient.ruleDao().selectDefinitionById(templateId, session);
if (template.isPresent() && template.get().getStatus() != RuleStatus.REMOVED) {
if (updateCustomRuleFromTemplateRule(customRule, template.get())) {
update(session, customRule.getDefinition());
update(session, customRule);
}
} else {
removeRule(session, removedRules, customRule);
@@ -401,20 +397,21 @@ public class RegisterRules implements Startable {
return removedRules;
}

private void removeRule(DbSession session, List<RuleDto> removedRules, RuleDto rule) {
private void removeRule(DbSession session, List<RuleDefinitionDto> removedRules, RuleDefinitionDto rule) {
LOG.info(String.format("Disable rule %s", rule.getKey()));
rule.setStatus(RuleStatus.REMOVED);
rule.setSystemTags(Collections.emptySet());
rule.setTags(Collections.emptySet());
update(session, rule.getDefinition());
update(session, rule.getMetadata());
update(session, rule);
// FIXME resetting the tags for all organizations must be handled a different way
// rule.setTags(Collections.emptySet());
// update(session, rule.getMetadata());
removedRules.add(rule);
if (removedRules.size() % 100 == 0) {
session.commit();
}
}

private static boolean updateCustomRuleFromTemplateRule(RuleDto customRule, RuleDefinitionDto templateRule) {
private static boolean updateCustomRuleFromTemplateRule(RuleDefinitionDto customRule, RuleDefinitionDto templateRule) {
boolean changed = false;
if (!StringUtils.equals(customRule.getLanguage(), templateRule.getLanguage())) {
customRule.setLanguage(templateRule.getLanguage());
@@ -462,19 +459,14 @@ public class RegisterRules implements Startable {
* The side effect of this approach is that extended repositories will not be managed the same way.
* If an extended repository do not exists anymore, then related active rules will be removed.
*/
private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession session, Collection<RuleDto> removedRules, RulesDefinition.Context context) {
List<String> repositoryKeys = newArrayList(Iterables.transform(context.repositories(), new Function<RulesDefinition.Repository, String>() {
@Override
public String apply(@Nonnull RulesDefinition.Repository input) {
return input.key();
}
}));
private List<ActiveRuleChange> removeActiveRulesOnStillExistingRepositories(DbSession session, Collection<RuleDefinitionDto> removedRules, RulesDefinition.Context context) {
List<String> repositoryKeys = newArrayList(Iterables.transform(context.repositories(), RulesDefinition.Repository::key));

List<ActiveRuleChange> changes = new ArrayList<>();
for (RuleDto rule : removedRules) {
for (RuleDefinitionDto rule : removedRules) {
// SONAR-4642 Remove active rules only when repository still exists
if (repositoryKeys.contains(rule.getRepositoryKey())) {
changes.addAll(ruleActivator.deactivate(session, rule.getDefinition()));
changes.addAll(ruleActivator.deactivate(session, rule));
}
}
return changes;
@@ -485,8 +477,4 @@ public class RegisterRules implements Startable {
dbClient.ruleDao().update(session, rule);
}

private void update(DbSession session, RuleMetadataDto rule) {
rule.setUpdatedAt(system2.now());
dbClient.ruleDao().update(session, rule);
}
}

+ 12
- 6
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/RuleRepositoryImplTest.java View File

@@ -31,11 +31,13 @@ import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.db.rule.RuleDao;
import org.sonar.db.rule.RuleDto;
import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolderRule;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.guava.api.Assertions.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
@@ -48,21 +50,25 @@ public class RuleRepositoryImplTest {
private static final RuleDto AB_RULE = createABRuleDto();
private static final RuleKey AC_RULE_KEY = RuleKey.of("a", "c");
private static final int AC_RULE_ID = 684;
private static final String ORGANIZATION_UUID = "org-1";

@org.junit.Rule
public ExpectedException expectedException = ExpectedException.none();
@org.junit.Rule
public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule()
.setOrganizationUuid(ORGANIZATION_UUID);

private DbClient dbClient = mock(DbClient.class);
private DbSession dbSession = mock(DbSession.class);
private RuleDao ruleDao = mock(RuleDao.class);

RuleRepositoryImpl underTest = new RuleRepositoryImpl(dbClient);
RuleRepositoryImpl underTest = new RuleRepositoryImpl(dbClient, analysisMetadataHolder);

@Before
public void setUp() throws Exception {
when(dbClient.openSession(anyBoolean())).thenReturn(dbSession);
when(dbClient.ruleDao()).thenReturn(ruleDao);
when(ruleDao.selectAll(any(DbSession.class))).thenReturn(ImmutableList.of(AB_RULE));
when(ruleDao.selectAll(any(DbSession.class), eq(ORGANIZATION_UUID))).thenReturn(ImmutableList.of(AB_RULE));
}

@Test
@@ -74,7 +80,7 @@ public class RuleRepositoryImplTest {
public void first_call_to_getByKey_triggers_call_to_db_and_any_subsequent_get_or_find_call_does_not() {
underTest.getByKey(AB_RULE.getKey());

verify(ruleDao, times(1)).selectAll(any(DbSession.class));
verify(ruleDao, times(1)).selectAll(any(DbSession.class), eq(ORGANIZATION_UUID));

verifyNoMethodCallTriggersCallToDB();
}
@@ -83,7 +89,7 @@ public class RuleRepositoryImplTest {
public void first_call_to_findByKey_triggers_call_to_db_and_any_subsequent_get_or_find_call_does_not() {
underTest.findByKey(AB_RULE.getKey());

verify(ruleDao, times(1)).selectAll(any(DbSession.class));
verify(ruleDao, times(1)).selectAll(any(DbSession.class), eq(ORGANIZATION_UUID));

verifyNoMethodCallTriggersCallToDB();
}
@@ -92,7 +98,7 @@ public class RuleRepositoryImplTest {
public void first_call_to_getById_triggers_call_to_db_and_any_subsequent_get_or_find_call_does_not() {
underTest.getById(AB_RULE.getId());

verify(ruleDao, times(1)).selectAll(any(DbSession.class));
verify(ruleDao, times(1)).selectAll(any(DbSession.class), eq(ORGANIZATION_UUID));

verifyNoMethodCallTriggersCallToDB();
}
@@ -101,7 +107,7 @@ public class RuleRepositoryImplTest {
public void first_call_to_findById_triggers_call_to_db_and_any_subsequent_get_or_find_call_does_not() {
underTest.findById(AB_RULE.getId());

verify(ruleDao, times(1)).selectAll(any(DbSession.class));
verify(ruleDao, times(1)).selectAll(any(DbSession.class), eq(ORGANIZATION_UUID));

verifyNoMethodCallTriggersCallToDB();
}

+ 16
- 22
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/PersistIssuesStepTest.java View File

@@ -42,6 +42,7 @@ import org.sonar.db.organization.OrganizationDto;
import org.sonar.db.rule.RuleDto;
import org.sonar.db.rule.RuleTesting;
import org.sonar.scanner.protocol.output.ScannerReport;
import org.sonar.server.computation.task.projectanalysis.analysis.AnalysisMetadataHolderRule;
import org.sonar.server.computation.task.projectanalysis.batch.BatchReportReaderRule;
import org.sonar.server.computation.task.projectanalysis.issue.IssueCache;
import org.sonar.server.computation.task.projectanalysis.issue.RuleRepositoryImpl;
@@ -58,22 +59,19 @@ public class PersistIssuesStepTest extends BaseStepTest {

@Rule
public TemporaryFolder temp = new TemporaryFolder();

@Rule
public DbTester dbTester = DbTester.create(System2.INSTANCE);

@Rule
public BatchReportReaderRule reportReader = new BatchReportReaderRule();
@Rule
public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule()
.setOrganizationUuid("org-1");

DbSession session = dbTester.getSession();

DbClient dbClient = dbTester.getDbClient();

System2 system2;

IssueCache issueCache;

ComputationStep step;
private DbSession session = dbTester.getSession();
private DbClient dbClient = dbTester.getDbClient();
private System2 system2;
private IssueCache issueCache;
private ComputationStep step;

@Override
protected ComputationStep step() {
@@ -87,7 +85,7 @@ public class PersistIssuesStepTest extends BaseStepTest {
when(system2.now()).thenReturn(NOW);
reportReader.setMetadata(ScannerReport.Metadata.getDefaultInstance());

step = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(dbClient), issueCache);
step = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(dbClient, analysisMetadataHolder), issueCache);
}

@After
@@ -115,8 +113,7 @@ public class PersistIssuesStepTest extends BaseStepTest {
.setSeverity(Severity.BLOCKER)
.setStatus(Issue.STATUS_OPEN)
.setNew(true)
.setType(RuleType.BUG)
).close();
.setType(RuleType.BUG)).close();

step.execute();

@@ -145,8 +142,7 @@ public class PersistIssuesStepTest extends BaseStepTest {
.setResolution(Issue.RESOLUTION_FIXED)
.setSelectedAt(NOW)
.setNew(false)
.setChanged(true)
).close();
.setChanged(true)).close();

step.execute();

@@ -173,9 +169,8 @@ public class PersistIssuesStepTest extends BaseStepTest {
.setIssueKey("ISSUE")
.setUserLogin("john")
.setMarkdownText("Some text")
.setNew(true)
)
).close();
.setNew(true)))
.close();

step.execute();

@@ -200,9 +195,8 @@ public class PersistIssuesStepTest extends BaseStepTest {
.setCurrentChange(new FieldDiffs()
.setIssueKey("ISSUE")
.setUserLogin("john")
.setDiff("technicalDebt", null, 1L)
)
).close();
.setDiff("technicalDebt", null, 1L)))
.close();

step.execute();


+ 8
- 8
server/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesTest.java View File

@@ -94,7 +94,7 @@ public class RegisterRulesTest {
execute(new FakeRepositoryV1());

// verify db
assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2);
assertThat(dbClient.ruleDao().selectAllDefinitions(dbTester.getSession())).hasSize(2);
RuleDto rule1 = dbClient.ruleDao().selectOrFailByKey(dbTester.getSession(), dbTester.getDefaultOrganization().getUuid(), RULE_KEY1);
assertThat(rule1.getName()).isEqualTo("One");
assertThat(rule1.getDescription()).isEqualTo("Description of One");
@@ -138,7 +138,7 @@ public class RegisterRulesTest {
@Test
public void update_and_remove_rules_on_changes() {
execute(new FakeRepositoryV1());
assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2);
assertThat(dbClient.ruleDao().selectAllDefinitions(dbTester.getSession())).hasSize(2);
assertThat(esTester.getIds(RuleIndexDefinition.INDEX_TYPE_RULE)).containsOnly(RULE_KEY1.toString(), RULE_KEY2.toString());

// user adds tags and sets markdown note
@@ -323,7 +323,7 @@ public class RegisterRulesTest {
@Test
public void do_not_update_rules_when_no_changes() {
execute(new FakeRepositoryV1());
assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2);
assertThat(dbClient.ruleDao().selectAllDefinitions(dbTester.getSession())).hasSize(2);

when(system.now()).thenReturn(DATE2.getTime());
execute(new FakeRepositoryV1());
@@ -337,7 +337,7 @@ public class RegisterRulesTest {
@Test
public void do_not_update_already_removed_rules() {
execute(new FakeRepositoryV1());
assertThat(dbClient.ruleDao().selectAll(dbTester.getSession())).hasSize(2);
assertThat(dbClient.ruleDao().selectAllDefinitions(dbTester.getSession())).hasSize(2);
assertThat(esTester.getIds(RuleIndexDefinition.INDEX_TYPE_RULE)).containsOnly(RULE_KEY1.toString(), RULE_KEY2.toString());

String organizationUuid = dbTester.getDefaultOrganization().getUuid();
@@ -380,9 +380,9 @@ public class RegisterRulesTest {
@Test
public void manage_repository_extensions() {
execute(new FindbugsRepository(), new FbContribRepository());
List<RuleDto> rules = dbClient.ruleDao().selectAll(dbTester.getSession());
List<RuleDefinitionDto> rules = dbClient.ruleDao().selectAllDefinitions(dbTester.getSession());
assertThat(rules).hasSize(2);
for (RuleDto rule : rules) {
for (RuleDefinitionDto rule : rules) {
assertThat(rule.getRepositoryKey()).isEqualTo("findbugs");
}
}
@@ -402,9 +402,9 @@ public class RegisterRulesTest {
// Synchronize rule without tag
execute(new FindbugsRepository());

List<RuleDto> rules = dbClient.ruleDao().selectAll(dbTester.getSession());
List<RuleDefinitionDto> rules = dbClient.ruleDao().selectAllDefinitions(dbTester.getSession());
assertThat(rules).hasSize(1);
RuleDto result = rules.get(0);
RuleDefinitionDto result = rules.get(0);
assertThat(result.getKey()).isEqualTo(RuleKey.of("findbugs", "rule1"));
assertThat(result.getSystemTags()).isEmpty();
}

Loading…
Cancel
Save