import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static org.sonar.api.issue.Issue.STATUS_CLOSED;
+import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
public class ComponentIssuesLoader {
private static final int DEFAULT_CLOSED_ISSUES_MAX_AGE = 30;
}
}
- public void loadChanges(Collection<DefaultIssue> issues) {
- if (issues.isEmpty()) {
- return;
- }
-
- try (DbSession dbSession = dbClient.openSession(false)) {
- loadChanges(dbSession, issues);
- }
- }
-
public List<DefaultIssue> loadChanges(DbSession dbSession, Collection<DefaultIssue> issues) {
Map<String, List<IssueChangeDto>> changeDtoByIssueKey = dbClient.issueChangeDao()
.selectByIssueKeys(dbSession, issues.stream().map(DefaultIssue::key).collect(toList()))
.collect(toList());
}
+ /**
+ * Loads the most recent diff changes of the specified issues which contain the latest status and resolution of the
+ * issue.
+ */
+ public void loadLatestDiffChangesForReopeningOfClosedIssues(Collection<DefaultIssue> issues) {
+ if (issues.isEmpty()) {
+ return;
+ }
+
+ try (DbSession dbSession = dbClient.openSession(false)) {
+ loadLatestDiffChangesForReopeningOfClosedIssues(dbSession, issues);
+ }
+ }
+
+ /**
+ * To be efficient both in term of memory and speed:
+ * <ul>
+ * <li>only diff changes are loaded from DB, sorted by issue and then change creation date</li>
+ * <li>data from DB is streamed</li>
+ * <li>only the latest change(s) with status and resolution are added to the {@link DefaultIssue} objects</li>
+ * </ul>
+ */
+ private void loadLatestDiffChangesForReopeningOfClosedIssues(DbSession dbSession, Collection<DefaultIssue> issues) {
+ Map<String, DefaultIssue> issuesByKey = issues.stream().collect(uniqueIndex(DefaultIssue::key));
+
+ dbClient.issueChangeDao()
+ .scrollDiffChangesOfIssues(dbSession, issuesByKey.keySet(), new ResultHandler<IssueChangeDto>() {
+ private DefaultIssue currentIssue = null;
+ private boolean previousStatusFound = false;
+ private boolean previousResolutionFound = false;
+
+ @Override
+ public void handleResult(ResultContext<? extends IssueChangeDto> resultContext) {
+ IssueChangeDto issueChangeDto = resultContext.getResultObject();
+ if (currentIssue == null || !currentIssue.key().equals(issueChangeDto.getIssueKey())) {
+ currentIssue = issuesByKey.get(issueChangeDto.getIssueKey());
+ previousStatusFound = false;
+ previousResolutionFound = false;
+ }
+
+ if (currentIssue != null) {
+ FieldDiffs fieldDiffs = issueChangeDto.toFieldDiffs();
+ boolean hasPreviousStatus = fieldDiffs.get("status") != null;
+ boolean hasPreviousResolution = fieldDiffs.get("resolution") != null;
+ if ((!previousStatusFound && hasPreviousStatus) || (!previousResolutionFound && hasPreviousResolution)) {
+ currentIssue.addChange(fieldDiffs);
+ }
+ previousStatusFound |= hasPreviousStatus;
+ previousResolutionFound |= hasPreviousResolution;
+ }
+ }
+ });
+ }
+
private List<DefaultIssue> loadOpenIssues(String componentUuid, DbSession dbSession) {
List<DefaultIssue> result = new ArrayList<>();
dbSession.getMapper(IssueMapper.class).scrollNonClosedByComponentUuid(componentUuid, resultContext -> {
}
private static void setChanges(Map<String, List<IssueChangeDto>> changeDtoByIssueKey, DefaultIssue i) {
- changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList()).forEach(c -> {
- switch (c.getChangeType()) {
- case IssueChangeDto.TYPE_FIELD_CHANGE:
- i.addChange(c.toFieldDiffs());
- break;
- case IssueChangeDto.TYPE_COMMENT:
- i.addComment(c.toComment());
- break;
- default:
- throw new IllegalStateException("Unknow change type: " + c.getChangeType());
- }
- });
+ changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList())
+ .forEach(c -> addChangeOrComment(i, c));
+ }
+
+ private static void addChangeOrComment(DefaultIssue i, IssueChangeDto c) {
+ switch (c.getChangeType()) {
+ case IssueChangeDto.TYPE_FIELD_CHANGE:
+ i.addChange(c.toFieldDiffs());
+ break;
+ case IssueChangeDto.TYPE_COMMENT:
+ i.addComment(c.toComment());
+ break;
+ default:
+ throw new IllegalStateException("Unknow change type: " + c.getChangeType());
+ }
}
private boolean isActive(RuleKey ruleKey) {
Set<DefaultIssue> matchesClosedIssues = closedIssuesTracking.getMatchedRaws().values().stream()
.filter(t -> Issue.STATUS_CLOSED.equals(t.getStatus()))
.collect(MoreCollectors.toSet());
- componentIssuesLoader.loadChanges(matchesClosedIssues);
+ componentIssuesLoader.loadLatestDiffChangesForReopeningOfClosedIssues(matchesClosedIssues);
return closedIssuesTracking;
}
*/
package org.sonar.ce.task.projectanalysis.issue;
+import com.google.common.collect.ImmutableList;
import com.tngtech.java.junit.dataprovider.DataProvider;
import com.tngtech.java.junit.dataprovider.DataProviderRunner;
import com.tngtech.java.junit.dataprovider.UseDataProvider;
+import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collections;
import java.util.Date;
import java.util.List;
import java.util.Random;
+import java.util.stream.IntStream;
import javax.annotation.Nullable;
import org.junit.Rule;
import org.junit.Test;
import org.sonar.db.organization.OrganizationDto;
import org.sonar.db.rule.RuleDefinitionDto;
+import static java.util.Collections.emptyList;
import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
verifyZeroInteractions(dbClient, system2);
}
+ @Test
+ public void loadLatestDiffChangesForReopeningOfClosedIssues_does_not_query_DB_if_issue_list_is_empty() {
+ DbClient dbClient = mock(DbClient.class);
+ ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
+ null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */);
+
+ underTest.loadLatestDiffChangesForReopeningOfClosedIssues(emptyList());
+
+ verifyZeroInteractions(dbClient, system2);
+ }
+
+ @Test
+ @UseDataProvider("statusOrResolutionFieldName")
+ public void loadLatestDiffChangesForReopeningOfClosedIssues_add_diff_change_with_most_recent_status_or_resolution(String statusOrResolutionFieldName) {
+ ComponentDto project = dbTester.components().insertPublicProject();
+ ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project));
+ RuleDefinitionDto rule = dbTester.rules().insert();
+ IssueDto issue = dbTester.issues().insert(rule, project, file);
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val1")).setIssueChangeCreationDate(5));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val2")).setIssueChangeCreationDate(20));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val3")).setIssueChangeCreationDate(13));
+ ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
+ null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */);
+ DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey());
+
+ underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue));
+
+ assertThat(defaultIssue.changes())
+ .hasSize(1);
+ assertThat(defaultIssue.changes())
+ .extracting(t -> t.get(statusOrResolutionFieldName))
+ .filteredOn(t -> hasValue(t, "val2"))
+ .hasSize(1);
+ }
+
+ @Test
+ public void loadLatestDiffChangesForReopeningOfClosedIssues_add_single_diff_change_when_most_recent_status_and_resolution_is_the_same_diff() {
+ ComponentDto project = dbTester.components().insertPublicProject();
+ ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project));
+ RuleDefinitionDto rule = dbTester.rules().insert();
+ IssueDto issue = dbTester.issues().insert(rule, project, file);
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus1")).setIssueChangeCreationDate(5));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus2")).setIssueChangeCreationDate(19));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus3", "resolution", "valRes3")).setIssueChangeCreationDate(20));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("resolution", "valRes4")).setIssueChangeCreationDate(13));
+ ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
+ null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */);
+ DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey());
+
+ underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue));
+
+ assertThat(defaultIssue.changes())
+ .hasSize(1);
+ assertThat(defaultIssue.changes())
+ .extracting(t -> t.get("status"))
+ .filteredOn(t -> hasValue(t, "valStatus3"))
+ .hasSize(1);
+ assertThat(defaultIssue.changes())
+ .extracting(t -> t.get("resolution"))
+ .filteredOn(t -> hasValue(t, "valRes3"))
+ .hasSize(1);
+ }
+
+ @Test
+ public void loadLatestDiffChangesForReopeningOfClosedIssues_adds_2_diff_changes_if_most_recent_status_and_resolution_are_not_the_same_diff() {
+ ComponentDto project = dbTester.components().insertPublicProject();
+ ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project));
+ RuleDefinitionDto rule = dbTester.rules().insert();
+ IssueDto issue = dbTester.issues().insert(rule, project, file);
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus1")).setIssueChangeCreationDate(5));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus2", "resolution", "valRes2")).setIssueChangeCreationDate(19));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus3")).setIssueChangeCreationDate(20));
+ dbTester.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("resolution", "valRes4")).setIssueChangeCreationDate(13));
+ ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient,
+ null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */);
+ DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey());
+
+ underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue));
+
+ assertThat(defaultIssue.changes())
+ .hasSize(2);
+ assertThat(defaultIssue.changes())
+ .extracting(t -> t.get("status"))
+ .filteredOn(t -> hasValue(t, "valStatus3"))
+ .hasSize(1);
+ assertThat(defaultIssue.changes())
+ .extracting(t -> t.get("resolution"))
+ .filteredOn(t -> hasValue(t, "valRes2"))
+ .hasSize(1);
+ }
+
+ private static boolean hasValue(@Nullable FieldDiffs.Diff t, String value) {
+ if (t == null) {
+ return false;
+ }
+ return (t.oldValue() == null || value.equals(t.oldValue())) && (t.newValue() == null || value.equals(t.newValue()));
+ }
+
+ @DataProvider
+ public static Object[][] statusOrResolutionFieldName() {
+ return new Object[][] {
+ {"status"},
+ {"resolution"},
+ };
+ }
+
+ private static String randomDiffWith(String... fieldsAndValues) {
+ Random random = new Random();
+ List<Diff> diffs = new ArrayList<>();
+ for (int i = 0; i < fieldsAndValues.length; i++) {
+ int oldOrNew = random.nextInt(3);
+ String value = fieldsAndValues[i + 1];
+ diffs.add(new Diff(fieldsAndValues[i], oldOrNew <= 2 ? value : null, oldOrNew >= 2 ? value : null));
+ i++;
+ }
+ IntStream.range(0, random.nextInt(5))
+ .forEach(i -> diffs.add(new Diff(randomAlphabetic(10), random.nextBoolean() ? null : randomAlphabetic(11), random.nextBoolean() ? null : randomAlphabetic(12))));
+ Collections.shuffle(diffs);
+
+ FieldDiffs res = new FieldDiffs();
+ diffs.forEach(diff -> res.setDiff(diff.field, diff.oldValue, diff.newValue));
+ return res.toEncodedString();
+ }
+
+ private static final class Diff {
+ private final String field;
+ private final String oldValue;
+ private final String newValue;
+
+ private Diff(String field, @Nullable String oldValue, @Nullable String newValue) {
+ this.field = field;
+ this.oldValue = oldValue;
+ this.newValue = newValue;
+ }
+ }
+
private static FieldDiffs newToClosedDiffsWithLine(Date creationDate, @Nullable Integer oldLineValue) {
FieldDiffs fieldDiffs = new FieldDiffs().setCreationDate(addDays(creationDate, -5))
.setDiff("status", randomNonCloseStatus(), STATUS_CLOSED);
assertThat(tracking).isSameAs(closedTracking);
verify(tracker).trackNonClosed(rawInput, openIssuesInput);
verify(tracker).trackClosed(nonClosedTracking, closedIssuesInput);
- verify(componentIssuesLoader).loadChanges(mappedClosedIssues);
+ verify(componentIssuesLoader).loadLatestDiffChangesForReopeningOfClosedIssues(mappedClosedIssues);
verifyNoMoreInteractions(tracker);
}
}
import java.util.Collection;
import java.util.List;
import java.util.Optional;
+import org.apache.ibatis.session.ResultHandler;
import org.sonar.core.issue.FieldDiffs;
import org.sonar.core.util.stream.MoreCollectors;
import org.sonar.db.Dao;
import static java.util.Collections.singletonList;
import static org.sonar.db.DatabaseUtils.executeLargeInputs;
+import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput;
public class IssueChangeDao implements Dao {
return Optional.ofNullable(mapper(session).selectByKeyAndType(commentKey, IssueChangeDto.TYPE_COMMENT));
}
+ public void scrollDiffChangesOfIssues(DbSession dbSession, Collection<String> issueKeys, ResultHandler<IssueChangeDto> handler) {
+ if (issueKeys.isEmpty()) {
+ return;
+ }
+
+ executeLargeInputsWithoutOutput(issueKeys, issueKeySubList -> mapper(dbSession).scrollDiffChangesOfIssues(issueKeySubList, handler));
+ }
+
public void insert(DbSession session, IssueChangeDto change) {
mapper(session).insert(change);
}
import java.util.List;
import javax.annotation.CheckForNull;
import org.apache.ibatis.annotations.Param;
+import org.apache.ibatis.session.ResultHandler;
public interface IssueChangeMapper {
/**
* Issue changes by chronological date of creation
*/
- List<IssueChangeDto> selectByIssuesAndType(@Param("issueKeys") List<String> issueKeys,
- @Param("changeType") String changeType);
+ List<IssueChangeDto> selectByIssuesAndType(@Param("issueKeys") List<String> issueKeys, @Param("changeType") String changeType);
- List<IssueChangeDto> selectByIssues(@Param("issueKeys") List<String> issueKeys);
+ /**
+ * Scrolls through all changes with type {@link IssueChangeDto#TYPE_FIELD_CHANGE diff}, sorted by issue key and
+ * then change creation date.
+ */
+ void scrollDiffChangesOfIssues(@Param("issueKeys") List<String> issueKeys, ResultHandler<IssueChangeDto> handler);
+ List<IssueChangeDto> selectByIssues(@Param("issueKeys") List<String> issueKeys);
}
import org.sonar.api.rule.Severity;
import org.sonar.api.rules.RuleType;
import org.sonar.api.utils.DateUtils;
+import org.sonar.core.util.UuidFactoryFast;
import org.sonar.core.util.Uuids;
import org.sonar.db.component.ComponentDto;
import org.sonar.db.rule.RuleDefinitionDto;
public static IssueChangeDto newIssuechangeDto(IssueDto issue) {
return new IssueChangeDto()
- .setKey("uuid_" + randomAlphabetic(10))
+ .setKey(UuidFactoryFast.getInstance().create())
.setIssueKey(issue.getKey())
.setChangeData("data_" + randomAlphanumeric(40))
.setChangeType(IssueChangeDto.TYPE_FIELD_CHANGE)
</foreach>
order by c.created_at
</select>
+
+ <select id="scrollDiffChangesOfIssues" parameterType="map" resultType="IssueChange" fetchSize="${_scrollFetchSize}" resultSetType="FORWARD_ONLY">
+ select
+ <include refid="issueChangeColumns"/>
+ from issue_changes c
+ where
+ c.change_type='diff'
+ and c.issue_key in
+ <foreach collection="issueKeys" open="(" close=")" item="key" separator=",">
+ #{key,jdbcType=VARCHAR}
+ </foreach>
+ order by
+ c.issue_key,c.issue_change_creation_date desc
+ </select>
<select id="selectByIssues" parameterType="map" resultType="IssueChange">
select
*/
package org.sonar.db.issue;
+import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
+import java.util.stream.Stream;
+import org.apache.ibatis.session.ResultContext;
+import org.apache.ibatis.session.ResultHandler;
import org.junit.Rule;
import org.junit.Test;
import org.sonar.api.utils.DateUtils;
import org.sonar.core.issue.FieldDiffs;
import org.sonar.db.DbTester;
+import static com.google.common.collect.ImmutableList.of;
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
issueChange.getUpdatedAt()));
}
+ @Test
+ public void scrollDiffChangesOfIssues_scrolls_only_diff_changes_of_selected_issues() {
+ IssueDto issue1 = db.issues().insertIssue();
+ IssueChangeDto diffChange1 = db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE));
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_COMMENT));
+ IssueDto issue2 = db.issues().insertIssue();
+ IssueChangeDto diffChange2 = db.issues().insertChange(issue2, t -> t.setChangeType(TYPE_FIELD_CHANGE));
+ db.issues().insertChange(issue2, t -> t.setChangeType(TYPE_COMMENT));
+ IssueDto issue3 = db.issues().insertIssue();
+ IssueChangeDto diffChange31 = db.issues().insertChange(issue3, t -> t.setChangeType(TYPE_FIELD_CHANGE));
+ IssueChangeDto diffChange32 = db.issues().insertChange(issue3, t -> t.setChangeType(TYPE_FIELD_CHANGE));
+ db.issues().insertChange(issue3, t -> t.setChangeType(TYPE_COMMENT));
+ RecordingIssueChangeDtoResultHandler recordingHandler = new RecordingIssueChangeDtoResultHandler();
+
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of(), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).isEmpty();
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of("fooBarCacahuete"), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).isEmpty();
+
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of(issue1.getKee()), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).containsOnly(diffChange1.getKey());
+
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of(issue2.getKee()), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).containsOnly(diffChange2.getKey());
+
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of(issue1.getKee(), issue3.getKee()), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).containsOnly(diffChange1.getKey(), diffChange31.getKey(), diffChange32.getKey());
+ }
+
+ @Test
+ public void scrollDiffChangesOfIssues_orders_changes_by_issue_and_then_creationDate() {
+ IssueDto issue1 = db.issues().insertIssue();
+ IssueChangeDto[] diffChanges = {
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE).setCreatedAt(1L).setIssueChangeCreationDate(50L)),
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE).setCreatedAt(2L).setIssueChangeCreationDate(20L)),
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE).setCreatedAt(3L).setIssueChangeCreationDate(30L)),
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE).setCreatedAt(4L).setIssueChangeCreationDate(80L)),
+ db.issues().insertChange(issue1, t -> t.setChangeType(TYPE_FIELD_CHANGE).setCreatedAt(5L).setIssueChangeCreationDate(10L)),
+ };
+ RecordingIssueChangeDtoResultHandler recordingHandler = new RecordingIssueChangeDtoResultHandler();
+ underTest.scrollDiffChangesOfIssues(db.getSession(), of(issue1.getKee()), recordingHandler.clear());
+ assertThat(recordingHandler.getDtoKeys()).containsExactly(
+ diffChanges[3].getKey(),
+ diffChanges[0].getKey(),
+ diffChanges[2].getKey(),
+ diffChanges[1].getKey(),
+ diffChanges[4].getKey());
+ }
+
+ private static class RecordingIssueChangeDtoResultHandler implements ResultHandler<IssueChangeDto> {
+ private final List<IssueChangeDto> dtos = new ArrayList<>();
+
+ @Override
+ public void handleResult(ResultContext<? extends IssueChangeDto> resultContext) {
+ dtos.add(resultContext.getResultObject());
+ }
+
+ public RecordingIssueChangeDtoResultHandler clear() {
+ dtos.clear();
+ return this;
+ }
+
+ public Stream<String> getDtoKeys() {
+ return dtos.stream().map(IssueChangeDto::getKey);
+ }
+ }
}
projects / sonarqube.git / commitdiff