*/
package org.sonar.ce.task.projectanalysis.step;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
import org.sonar.api.utils.System2;
import org.sonar.ce.task.projectanalysis.issue.IssueCache;
import org.sonar.ce.task.projectanalysis.issue.RuleRepository;
import org.sonar.ce.task.step.ComputationStep;
import org.sonar.core.issue.DefaultIssue;
import org.sonar.core.util.CloseableIterator;
+import org.sonar.db.BatchSession;
import org.sonar.db.DbClient;
import org.sonar.db.DbSession;
import org.sonar.db.issue.IssueChangeMapper;
import org.sonar.db.issue.IssueMapper;
import org.sonar.server.issue.IssueStorage;
+import static org.sonar.core.util.stream.MoreCollectors.toList;
+import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
+
public class PersistIssuesStep implements ComputationStep {
+ // holding up to 1000 DefaultIssue (max size of addedIssues and updatedIssues at any given time) in memory should not
+ // be a problem while making sure we leverage extensively the batch feature to speed up persistence
+ private static final int ISSUE_BATCHING_SIZE = BatchSession.MAX_BATCH_SIZE * 2;
private final DbClient dbClient;
private final System2 system2;
IssueStatistics statistics = new IssueStatistics();
try (DbSession dbSession = dbClient.openSession(true);
CloseableIterator<DefaultIssue> issues = issueCache.traverse()) {
+ List<DefaultIssue> addedIssues = new ArrayList<>(ISSUE_BATCHING_SIZE);
+ List<DefaultIssue> updatedIssues = new ArrayList<>(ISSUE_BATCHING_SIZE);
IssueMapper mapper = dbSession.getMapper(IssueMapper.class);
IssueChangeMapper changeMapper = dbSession.getMapper(IssueChangeMapper.class);
while (issues.hasNext()) {
DefaultIssue issue = issues.next();
- boolean saved = persistIssueIfRequired(mapper, issue, statistics);
- if (saved) {
- issueStorage.insertChanges(changeMapper, issue);
+ if (issue.isNew() || issue.isCopied()) {
+ addedIssues.add(issue);
+ if (addedIssues.size() >= ISSUE_BATCHING_SIZE) {
+ persistNewIssues(statistics, addedIssues, mapper, changeMapper);
+ addedIssues.clear();
+ }
+ } else if (issue.isChanged()) {
+ updatedIssues.add(issue);
+ if (updatedIssues.size() >= ISSUE_BATCHING_SIZE) {
+ persistUpdatedIssues(statistics, updatedIssues, mapper, changeMapper);
+ updatedIssues.clear();
+ }
+ } else {
+ statistics.untouched++;
}
}
- dbSession.flushStatements();
- dbSession.commit();
+ persistNewIssues(statistics, addedIssues, mapper, changeMapper);
+ persistUpdatedIssues(statistics, updatedIssues, mapper, changeMapper);
+ flushSession(dbSession);
} finally {
statistics.dumpTo(context);
}
}
- private boolean persistIssueIfRequired(IssueMapper mapper, DefaultIssue issue, IssueStatistics issueStatistics) {
- if (issue.isNew() || issue.isCopied()) {
- persistNewIssue(mapper, issue);
- issueStatistics.inserts++;
- return true;
+ private void persistNewIssues(IssueStatistics statistics, List<DefaultIssue> addedIssues, IssueMapper mapper, IssueChangeMapper changeMapper) {
+ if (addedIssues.isEmpty()) {
+ return;
}
- if (issue.isChanged()) {
- persistChangedIssue(mapper, issue);
- issueStatistics.updates++;
- return true;
- }
+ long now = system2.now();
+ addedIssues.forEach(i -> {
+ int ruleId = ruleRepository.getByKey(i.ruleKey()).getId();
+ IssueDto dto = IssueDto.toDtoForComputationInsert(i, ruleId, now);
+ mapper.insert(dto);
+ statistics.inserts++;
+ });
- issueStatistics.untouched++;
- return false;
+ addedIssues.forEach(i -> issueStorage.insertChanges(changeMapper, i));
}
- private void persistNewIssue(IssueMapper mapper, DefaultIssue issue) {
- int ruleId = ruleRepository.getByKey(issue.ruleKey()).getId();
- IssueDto dto = IssueDto.toDtoForComputationInsert(issue, ruleId, system2.now());
- mapper.insert(dto);
- }
+ private void persistUpdatedIssues(IssueStatistics statistics, List<DefaultIssue> updatedIssues, IssueMapper mapper, IssueChangeMapper changeMapper) {
+ if (updatedIssues.isEmpty()) {
+ return;
+ }
- private void persistChangedIssue(IssueMapper mapper, DefaultIssue issue) {
- IssueDto dto = IssueDto.toDtoForUpdate(issue, system2.now());
- int updateCount = mapper.updateIfBeforeSelectedDate(dto);
- if (updateCount == 0) {
- // End-user and scan changed the issue at the same time.
- // See https://jira.sonarsource.com/browse/SONAR-4309
- conflictResolver.resolve(issue, mapper);
+ long now = system2.now();
+ updatedIssues.forEach(i -> {
+ IssueDto dto = IssueDto.toDtoForUpdate(i, now);
+ mapper.updateIfBeforeSelectedDate(dto);
+ statistics.updates++;
+ });
+
+ // retrieve those of the updatedIssues which have not been updated and apply conflictResolver on them
+ List<String> updatedIssueKeys = updatedIssues.stream().map(DefaultIssue::key).collect(toList(updatedIssues.size()));
+ List<IssueDto> conflictIssueKeys = mapper.selectByKeysIfNotUpdatedAt(updatedIssueKeys, now);
+ if (!conflictIssueKeys.isEmpty()) {
+ Map<String, DefaultIssue> issuesByKeys = updatedIssues.stream().collect(uniqueIndex(DefaultIssue::key, updatedIssues.size()));
+ conflictIssueKeys
+ .forEach(dbIssue -> {
+ DefaultIssue updatedIssue = issuesByKeys.get(dbIssue.getKey());
+ conflictResolver.resolve(updatedIssue, dbIssue, mapper);
+ statistics.merged++;
+ });
}
+
+ updatedIssues.forEach(i -> issueStorage.insertChanges(changeMapper, i));
+ }
+
+ private static void flushSession(DbSession dbSession) {
+ dbSession.flushStatements();
+ dbSession.commit();
}
@Override
private static class IssueStatistics {
private int inserts = 0;
private int updates = 0;
+ private int merged = 0;
private int untouched = 0;
private void dumpTo(ComputationStep.Context context) {
context.getStatistics()
.add("inserts", String.valueOf(inserts))
.add("updates", String.valueOf(updates))
+ .add("merged", String.valueOf(merged))
.add("untouched", String.valueOf(untouched));
}
}
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
+import org.mockito.ArgumentCaptor;
import org.sonar.api.rule.RuleKey;
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.component.ComponentDto;
-import org.sonar.db.component.ComponentTesting;
import org.sonar.db.issue.IssueChangeDto;
import org.sonar.db.issue.IssueDto;
+import org.sonar.db.issue.IssueMapper;
import org.sonar.db.organization.OrganizationDto;
import org.sonar.db.rule.RuleDefinitionDto;
import org.sonar.db.rule.RuleTesting;
import static java.util.Collections.singletonList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.data.MapEntry.entry;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
import static org.sonar.api.issue.Issue.STATUS_CLOSED;
public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule()
.setOrganizationUuid("org-1", "qg-uuid-1");
+ private System2 system2 = mock(System2.class);
private DbSession session = db.getSession();
private DbClient dbClient = db.getDbClient();
+ private UpdateConflictResolver conflictResolver = mock(UpdateConflictResolver.class);
private IssueCache issueCache;
private ComputationStep underTest;
@Before
public void setup() throws Exception {
issueCache = new IssueCache(temp.newFile(), System2.INSTANCE);
- System2 system2 = mock(System2.class);
- when(system2.now()).thenReturn(NOW);
reportReader.setMetadata(ScannerReport.Metadata.getDefaultInstance());
- underTest = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder), issueCache,
+ underTest = new PersistIssuesStep(dbClient, system2, conflictResolver, new RuleRepositoryImpl(adHocRuleCreator, dbClient, analysisMetadataHolder), issueCache,
new IssueStorage());
}
RuleDefinitionDto rule = RuleTesting.newRule(RuleKey.of("xoo", "S01"));
db.rules().insert(rule);
OrganizationDto organizationDto = db.organizations().insert();
- ComponentDto project = ComponentTesting.newPrivateProjectDto(organizationDto);
- dbClient.componentDao().insert(session, project);
- ComponentDto file = newFileDto(project, null);
- dbClient.componentDao().insert(session, file);
- session.commit();
+ ComponentDto project = db.components().insertPrivateProject(organizationDto);
+ ComponentDto file = db.components().insertComponent(newFileDto(project, null));
+ when(system2.now()).thenReturn(NOW);
issueCache.newAppender().append(new DefaultIssue()
.setKey("ISSUE")
.setNew(false)
.setCopied(true)
.setType(RuleType.BUG)
+ .setSelectedAt(NOW)
.addComment(new DefaultIssueComment()
.setKey("COMMENT")
.setIssueKey("ISSUE")
List<IssueChangeDto> changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE"));
assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE);
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0"));
+ entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0"));
}
@Test
RuleDefinitionDto rule = RuleTesting.newRule(RuleKey.of("xoo", "S01"));
db.rules().insert(rule);
OrganizationDto organizationDto = db.organizations().insert();
- ComponentDto project = ComponentTesting.newPrivateProjectDto(organizationDto);
- dbClient.componentDao().insert(session, project);
- ComponentDto file = newFileDto(project, null);
- dbClient.componentDao().insert(session, file);
- session.commit();
+ ComponentDto project = db.components().insertPrivateProject(organizationDto);
+ ComponentDto file = db.components().insertComponent(newFileDto(project, null));
+ when(system2.now()).thenReturn(NOW);
issueCache.newAppender().append(new DefaultIssue()
.setKey("ISSUE")
.setNew(true)
.setCopied(true)
.setType(RuleType.BUG)
+ .setSelectedAt(NOW)
.addComment(new DefaultIssueComment()
.setKey("COMMENT")
.setIssueKey("ISSUE")
List<IssueChangeDto> changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE"));
assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE);
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0"));
+ entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0"));
+ }
+
+ @Test
+ public void update_conflicting_issue() {
+ RuleDefinitionDto rule = RuleTesting.newRule(RuleKey.of("xoo", "S01"));
+ db.rules().insert(rule);
+ OrganizationDto organizationDto = db.organizations().insert();
+ ComponentDto project = db.components().insertPrivateProject(organizationDto);
+ ComponentDto file = db.components().insertComponent(newFileDto(project, null));
+ IssueDto issue = db.issues().insert(rule, project, file,
+ i -> i.setStatus(STATUS_OPEN)
+ .setResolution(null)
+ .setCreatedAt(NOW - 1_000_000_000L)
+ // simulate the issue has been updated after the analysis ran
+ .setUpdatedAt(NOW + 1_000_000_000L));
+ issue = dbClient.issueDao().selectByKey(db.getSession(), issue.getKey()).get();
+ DiskCache<DefaultIssue>.DiskAppender issueCacheAppender = issueCache.newAppender();
+ when(system2.now()).thenReturn(NOW);
+
+ DefaultIssue defaultIssue = issue.toDefaultIssue()
+ .setStatus(STATUS_CLOSED)
+ .setResolution(RESOLUTION_FIXED)
+ .setSelectedAt(NOW)
+ .setNew(false)
+ .setChanged(true);
+ issueCacheAppender.append(defaultIssue).close();
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ ArgumentCaptor<IssueDto> issueDtoCaptor = ArgumentCaptor.forClass(IssueDto.class);
+ verify(conflictResolver).resolve(eq(defaultIssue), issueDtoCaptor.capture(), any(IssueMapper.class));
+ assertThat(issueDtoCaptor.getValue().getId()).isEqualTo(issue.getId());
+ assertThat(context.getStatistics().getAll()).containsOnly(
+ entry("inserts", "0"), entry("updates", "1"), entry("merged", "1"), entry("untouched", "0"));
+
}
@Test
RuleDefinitionDto rule = RuleTesting.newRule(RuleKey.of("xoo", "S01"));
db.rules().insert(rule);
OrganizationDto organizationDto = db.organizations().insert();
- ComponentDto project = ComponentTesting.newPrivateProjectDto(organizationDto);
- dbClient.componentDao().insert(session, project);
- ComponentDto file = newFileDto(project, null);
- dbClient.componentDao().insert(session, file);
+ ComponentDto project = db.components().insertPrivateProject(organizationDto);
+ ComponentDto file = db.components().insertComponent(newFileDto(project, null));
session.commit();
issueCache.newAppender().append(new DefaultIssue()
assertThat(result.getStatus()).isEqualTo(STATUS_OPEN);
assertThat(result.getType()).isEqualTo(RuleType.BUG.getDbConstant());
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0"));
+ entry("inserts", "1"), entry("updates", "0"), entry("merged", "0"), entry("untouched", "0"));
}
@Test
assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CLOSED);
assertThat(issueReloaded.getResolution()).isEqualTo(RESOLUTION_FIXED);
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0"));
+ entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0"));
}
@Test
IssueChangeDto::getIssueChangeCreationDate)
.containsOnly(IssueChangeDto.TYPE_COMMENT, "john_uuid", "Some text", issue.getKey(), NOW);
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0"));
+ entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0"));
}
@Test
IssueChangeDto::getIssueChangeCreationDate)
.containsOnly(IssueChangeDto.TYPE_FIELD_CHANGE, "john_uuid", "technicalDebt=1", issue.getKey(), NOW);
assertThat(context.getStatistics().getAll()).containsOnly(
- entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0"));
+ entry("inserts", "0"), entry("updates", "1"), entry("merged", "0"), entry("untouched", "0"));
}
}