*/
String getName();
- /**
- * Get component old relative path.
- */
- String getOldName();
-
/**
* The component short name. For files and directories this is the parent relative path (ie filename for files). For projects and view this is the same as {@link #getName()}
*/
return this.name;
}
- @Override
- public String getOldName() {
- return this.getFileAttributes().getOldName();
- }
-
@Override
public String getShortName() {
return this.shortName;
private String uuid;
private String key;
private String name;
- private String oldName;
private String shortName;
private String description;
private FileAttributes fileAttributes;
private final String languageKey;
private final boolean markedAsUnchanged;
private final int lines;
- private String oldName;
+ private String oldRelativePath;
public FileAttributes(boolean unitTest, @Nullable String languageKey, int lines) {
this(unitTest, languageKey, lines, false, null);
}
- public FileAttributes(boolean unitTest, @Nullable String languageKey, int lines, boolean markedAsUnchanged, @Nullable String oldName) {
+ public FileAttributes(boolean unitTest, @Nullable String languageKey, int lines, boolean markedAsUnchanged, @Nullable String oldRelativePath) {
this.unitTest = unitTest;
this.languageKey = languageKey;
this.markedAsUnchanged = markedAsUnchanged;
checkArgument(lines > 0, "Number of lines must be greater than zero");
this.lines = lines;
- this.oldName = formatOldName(oldName);
+ this.oldRelativePath = formatOldRelativePath(oldRelativePath);
}
public boolean isMarkedAsUnchanged() {
return languageKey;
}
+ /**
+ * The old relative path of a file when a move is detected by the SCM in the scope of a Pull Request.
+ */
@CheckForNull
- public String getOldName() {
- return oldName;
+ public String getOldRelativePath() {
+ return oldRelativePath;
}
/**
", unitTest=" + unitTest +
", lines=" + lines +
", markedAsUnchanged=" + markedAsUnchanged +
- ", oldName=" + oldName +
+ ", oldRelativePath='" + oldRelativePath + '\'' +
'}';
}
- private String formatOldName(@Nullable String name) {
- return abbreviate(trimToNull(name), MAX_COMPONENT_NAME_LENGTH);
+ private static String formatOldRelativePath(@Nullable String path) {
+ return abbreviate(trimToNull(path), MAX_COMPONENT_NAME_LENGTH);
}
}
public void register(Component component) {
checkComponent(component);
checkArgument(component.getType() == Component.Type.FILE, "component must be a file");
- checkState(!analysisMetadataHolder.isFirstAnalysis(), "No file can be registered on first analysis");
+ checkState(analysisMetadataHolder.isPullRequest() || !analysisMetadataHolder.isFirstAnalysis(), "No file can be registered on first branch analysis");
addedComponents.add(component);
}
matchesPerFileForScore.put(match.getDbUuid(), match);
matchesPerFileForScore.put(match.getReportUuid(), match);
}
- // validate non ambiguous matches (ie. the match is the only match of either the db file and the report file)
+ // validate non-ambiguous matches (i.e. the match is the only match of either the db file and the report file)
for (Match match : matchesToValidate) {
int dbFileMatchesCount = matchesPerFileForScore.get(match.getDbUuid()).size();
int reportFileMatchesCount = matchesPerFileForScore.get(match.getReportUuid()).size();
}
@Immutable
- private static final class DbComponent {
+ public static final class DbComponent {
private final String key;
private final String uuid;
private final String path;
private final int lineCount;
- private DbComponent(String key, String uuid, String path, int lineCount) {
+ public DbComponent(String key, String uuid, String path, int lineCount) {
this.key = key;
this.uuid = uuid;
this.path = path;
return retrieveOriginalFileFromCache(originalPullRequestFiles, file);
}
- private void storeOriginalFileInCache(Map<String, OriginalFile> originalFiles, Component file, OriginalFile originalFile) {
+ private static void storeOriginalFileInCache(Map<String, OriginalFile> originalFiles, Component file, OriginalFile originalFile) {
requireNonNull(file, "file can't be null");
requireNonNull(originalFile, "originalFile can't be null");
checkArgument(file.getType() == Component.Type.FILE, "file must be of type FILE");
}
}
- private Optional<OriginalFile> retrieveOriginalFileFromCache(Map<String, OriginalFile> originalFiles, Component file) {
+ private static Optional<OriginalFile> retrieveOriginalFileFromCache(Map<String, OriginalFile> originalFiles, Component file) {
requireNonNull(file, "file can't be null");
if (file.getType() != Component.Type.FILE) {
package org.sonar.ce.task.projectanalysis.filemove;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Sets;
import java.util.Collection;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
-import java.util.Set;
-import java.util.function.Consumer;
-import java.util.function.Function;
-import javax.annotation.concurrent.Immutable;
import org.apache.ibatis.session.ResultHandler;
-import org.jetbrains.annotations.NotNull;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder;
import org.sonar.ce.task.projectanalysis.component.DepthTraversalTypeAwareCrawler;
import org.sonar.ce.task.projectanalysis.component.TreeRootHolder;
import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter;
+import org.sonar.ce.task.projectanalysis.filemove.FileMoveDetectionStep.DbComponent;
import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile;
import org.sonar.ce.task.step.ComputationStep;
import org.sonar.db.DbClient;
import org.sonar.db.component.BranchDto;
import org.sonar.db.component.FileMoveRowDto;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap;
import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER;
if (targetBranchDbFilesByUuid.isEmpty()) {
registerNewlyAddedFiles(reportFilesByUuid);
context.getStatistics().add("addedFiles", reportFilesByUuid.size());
- LOG.debug("Previous snapshot has no file. No file move detection.");
+ LOG.debug("Target branch has no files. No file move detection.");
return;
}
- Map<String, Component> movedFilesByUuid = getMovedFilesByUuid(reportFilesByUuid);
- context.getStatistics().add("movedFiles", movedFilesByUuid.size());
+ Collection<Component> movedFiles = getMovedFilesByUuid(reportFilesByUuid);
+ context.getStatistics().add("movedFiles", movedFiles.size());
Map<String, Component> newlyAddedFilesByUuid = getNewlyAddedFilesByUuid(reportFilesByUuid, targetBranchDbFilesByUuid);
context.getStatistics().add("addedFiles", newlyAddedFilesByUuid.size());
- registerMovedFiles(movedFilesByUuid.values(), targetBranchDbFilesByUuid.values());
+ Map<String, DbComponent> dbFilesByPathReference = toDbFilesByPathReferenceMap(targetBranchDbFilesByUuid.values());
+
+ registerMovedFiles(movedFiles, dbFilesByPathReference);
registerNewlyAddedFiles(newlyAddedFilesByUuid);
}
- private void registerMovedFiles(Collection<Component> movedFiles, Collection<DbComponent> dbFiles) {
+ private void registerMovedFiles(Collection<Component> movedFiles, Map<String, DbComponent> dbFilesByPathReference) {
movedFiles
- .forEach(registerMovedFile(dbFiles));
+ .forEach(movedFile -> registerMovedFile(dbFilesByPathReference, movedFile));
}
- private Consumer<Component> registerMovedFile(Collection<DbComponent> dbFiles) {
- return movedFile -> retrieveDbFile(dbFiles, movedFile)
- .ifPresent(dbFile -> movedFilesRepository.setOriginalPullRequestFile(movedFile, toOriginalFile(dbFile)));
+ private void registerMovedFile(Map<String, DbComponent> dbFiles, Component movedFile) {
+ retrieveDbFile(dbFiles, movedFile)
+ .ifPresent(dbFile -> movedFilesRepository.setOriginalPullRequestFile(movedFile, toOriginalFile(dbFile)));
}
private void registerNewlyAddedFiles(Map<String, Component> newAddedFilesByUuid) {
.forEach(addedFileRepository::register);
}
- @NotNull
- private Map<String, Component> getNewlyAddedFilesByUuid(Map<String, Component> reportFilesByUuid, Map<String, DbComponent> dbFilesByUuid) {
+ private static Map<String, Component> getNewlyAddedFilesByUuid(Map<String, Component> reportFilesByUuid, Map<String, DbComponent> dbFilesByUuid) {
return reportFilesByUuid
.values()
.stream()
- .filter(file -> Objects.isNull(file.getOldName()))
+ .filter(file -> Objects.isNull(file.getFileAttributes().getOldRelativePath()))
.filter(file -> !dbFilesByUuid.containsKey(file.getUuid()))
- .collect(toMap(Component::getUuid, Function.identity()));
+ .collect(toMap(Component::getUuid, identity()));
}
- private Map<String, Component> getMovedFilesByUuid(Map<String, Component> reportFilesByUuid) {
+ private static Collection<Component> getMovedFilesByUuid(Map<String, Component> reportFilesByUuid) {
return reportFilesByUuid
.values()
.stream()
- .filter(file -> Objects.nonNull(file.getOldName()))
- .collect(toMap(Component::getUuid, Function.identity()));
+ .filter(file -> Objects.nonNull(file.getFileAttributes().getOldRelativePath()))
+ .collect(toList());
}
- private Optional<DbComponent> retrieveDbFile(Collection<DbComponent> dbFiles, Component file) {
- return dbFiles
- .stream()
- .filter(dbFile -> dbFile.getPath().equals(file.getOldName()))
- .findFirst();
- }
-
- public Set<String> difference(Set<String> set1, Set<String> set2) {
- if (set1.isEmpty() || set2.isEmpty()) {
- return set1;
- }
-
- return Sets.difference(set1, set2).immutableCopy();
+ private static Optional<DbComponent> retrieveDbFile(Map<String, DbComponent> dbFilesByPathReference, Component file) {
+ return Optional.ofNullable(dbFilesByPathReference.get(file.getFileAttributes().getOldRelativePath()));
}
private Map<String, DbComponent> getTargetBranchDbFilesByUuid(AnalysisMetadataHolder analysisMetadataHolder) {
try (DbSession dbSession = dbClient.openSession(false)) {
- String targetBranchUuid = getTargetBranchUuid(dbSession, analysisMetadataHolder.getProject().getUuid(), analysisMetadataHolder.getBranch().getTargetBranchName());
-
- return getTargetBranchDbFiles(dbSession, targetBranchUuid)
- .stream()
- .collect(toMap(DbComponent::getUuid, Function.identity()));
+ return getTargetBranchUuid(dbSession, analysisMetadataHolder.getProject().getUuid(), analysisMetadataHolder.getBranch().getTargetBranchName())
+ .map(targetBranchUUid -> getTargetBranchDbFilesByUuid(dbSession, targetBranchUUid))
+ .orElse(Map.of());
}
}
- private List<DbComponent> getTargetBranchDbFiles(DbSession dbSession, String targetBranchUuid) {
- List<DbComponent> files = new LinkedList<>();
+ private Map<String, DbComponent> getTargetBranchDbFilesByUuid(DbSession dbSession, String targetBranchUuid) {
+ Map<String, DbComponent> files = new HashMap<>();
+ dbClient.componentDao().scrollAllFilesForFileMove(dbSession, targetBranchUuid, accumulateFilesForFileMove(files));
+ return files;
+ }
- ResultHandler<FileMoveRowDto> storeFileMove = resultContext -> {
- FileMoveRowDto row = resultContext.getResultObject();
- files.add(new DbComponent(row.getKey(), row.getUuid(), row.getPath(), row.getLineCount()));
+ private static ResultHandler<FileMoveRowDto> accumulateFilesForFileMove(Map<String, DbComponent> accumulator) {
+ return resultContext -> {
+ DbComponent component = rowToDbComponent(resultContext.getResultObject());
+ accumulator.put(component.getUuid(), component);
};
+ }
- dbClient.componentDao().scrollAllFilesForFileMove(dbSession, targetBranchUuid, storeFileMove);
+ private static DbComponent rowToDbComponent(FileMoveRowDto row) {
+ return new DbComponent(row.getKey(), row.getUuid(), row.getPath(), row.getLineCount());
+ }
- return files;
+ private Optional<String> getTargetBranchUuid(DbSession dbSession, String projectUuid, String targetBranchName) {
+ return dbClient.branchDao().selectByBranchKey(dbSession, projectUuid, targetBranchName)
+ .map(BranchDto::getUuid);
}
- private String getTargetBranchUuid(DbSession dbSession, String projectUuid, String targetBranchName) {
- return dbClient.branchDao().selectByBranchKey(dbSession, projectUuid, targetBranchName)
- .map(BranchDto::getUuid)
- .orElseThrow(() -> new IllegalStateException("Pull Request has no target branch"));
+ private static Map<String, DbComponent> toDbFilesByPathReferenceMap(Collection<DbComponent> dbFiles) {
+ return dbFiles
+ .stream()
+ .collect(toMap(DbComponent::getPath, identity()));
}
private static Map<String, Component> getReportFilesByUuid(Component root) {
private static OriginalFile toOriginalFile(DbComponent dbComponent) {
return new OriginalFile(dbComponent.getUuid(), dbComponent.getKey());
}
-
- @Immutable
- private static final class DbComponent {
- private final String key;
- private final String uuid;
- private final String path;
- private final int lineCount;
-
- private DbComponent(String key, String uuid, String path, int lineCount) {
- this.key = key;
- this.uuid = uuid;
- this.path = path;
- this.lineCount = lineCount;
- }
-
- public String getKey() {
- return key;
- }
-
- public String getUuid() {
- return uuid;
- }
-
- public String getPath() {
- return path;
- }
-
- public int getLineCount() {
- return lineCount;
- }
- }
}
this.targetBranchComponentUuids = targetBranchComponentUuids;
this.dbClient = dbClient;
this.movedFilesRepository = movedFilesRepository;
- // TODO detect file moves?
}
public boolean hasTargetBranchAnalysis() {
}
private String getTargetBranchComponentUuid(Component component) {
- Optional<String> targetBranchOriginalComponentUuid = getOriginalComponentUuid(component);
+ Optional<String> targetBranchOriginalComponentKey = getOriginalComponentKey(component);
- if (targetBranchOriginalComponentUuid.isPresent()) {
- return targetBranchComponentUuids.getTargetBranchComponentUuid(targetBranchOriginalComponentUuid.get());
+ if (targetBranchOriginalComponentKey.isPresent()) {
+ return targetBranchComponentUuids.getTargetBranchComponentUuid(targetBranchOriginalComponentKey.get());
}
return targetBranchComponentUuids.getTargetBranchComponentUuid(component.getKey());
}
- private Optional<String> getOriginalComponentUuid(Component component) {
+ private Optional<String> getOriginalComponentKey(Component component) {
return movedFilesRepository
.getOriginalPullRequestFile(component)
.map(OriginalFile::getKey);
public class FileAttributesTest {
@Test
public void create_production_file() {
- FileAttributes underTest = new FileAttributes(true, "java", 10, true);
+ FileAttributes underTest = new FileAttributes(false, "java", 10, true,"C");
- assertThat(underTest.isUnitTest()).isTrue();
+ assertThat(underTest.isUnitTest()).isFalse();
assertThat(underTest.getLanguageKey()).isEqualTo("java");
assertThat(underTest.getLines()).isEqualTo(10);
assertThat(underTest.isMarkedAsUnchanged()).isTrue();
+ assertThat(underTest.getOldRelativePath()).isEqualTo("C");
}
@Test
public void create_unit_test() {
- FileAttributes underTest = new FileAttributes(true, "java", 10, false);
+ FileAttributes underTest = new FileAttributes(true, "java", 10, false,"oldName");
assertThat(underTest.isUnitTest()).isTrue();
assertThat(underTest.getLanguageKey()).isEqualTo("java");
assertThat(underTest.getLines()).isEqualTo(10);
assertThat(underTest.isMarkedAsUnchanged()).isFalse();
+ assertThat(underTest.getOldRelativePath()).isEqualTo("oldName");
+ }
+
+ @Test
+ public void create_without_oldName() {
+ FileAttributes underTest = new FileAttributes(true, "TypeScript", 10, false, null);
+
+ assertThat(underTest.isUnitTest()).isTrue();
+ assertThat(underTest.getLanguageKey()).isEqualTo("TypeScript");
+ assertThat(underTest.getLines()).isEqualTo(10);
+ assertThat(underTest.getOldRelativePath()).isNull();
}
@Test
public void create_without_language() {
- FileAttributes underTest = new FileAttributes(true, null, 10, false);
+ FileAttributes underTest = new FileAttributes(true, null, 10, false, null);
assertThat(underTest.isUnitTest()).isTrue();
assertThat(underTest.getLanguageKey()).isNull();
@Test
public void fail_with_IAE_when_lines_is_0() {
- assertThatThrownBy(() -> new FileAttributes(true, "java", 0, false))
+ assertThatThrownBy(() -> new FileAttributes(true, "java", 0, false, null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Number of lines must be greater than zero");
}
@Test
public void fail_with_IAE_when_lines_is_less_than_0() {
- assertThatThrownBy(() -> new FileAttributes(true, "java", -10, false))
+ assertThatThrownBy(() -> new FileAttributes(true, "java", -10, false, null))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Number of lines must be greater than zero");
}
@Test
public void test_toString() {
- assertThat(new FileAttributes(true, "java", 10, true))
- .hasToString("FileAttributes{languageKey='java', unitTest=true, lines=10, markedAsUnchanged=true}");
- assertThat(new FileAttributes(false, null, 1, false))
- .hasToString("FileAttributes{languageKey='null', unitTest=false, lines=1, markedAsUnchanged=false}");
+ assertThat(new FileAttributes(true, "java", 10, true, "bobo"))
+ .hasToString("FileAttributes{languageKey='java', unitTest=true, lines=10, markedAsUnchanged=true, oldRelativePath='bobo'}");
+ assertThat(new FileAttributes(false, null, 1, false, null))
+ .hasToString("FileAttributes{languageKey='null', unitTest=false, lines=1, markedAsUnchanged=false, oldRelativePath='null'}");
}
}
@Test
public void isDataUnchanged_returns_false_if_any_SAME_status_is_incorrect() {
Component file1 = ReportComponent.builder(Component.Type.FILE, 2, "FILE1_KEY").setStatus(Component.Status.SAME)
- .setFileAttributes(new FileAttributes(false, null, 10, true)).build();
+ .setFileAttributes(new FileAttributes(false, null, 10, true, null)).build();
Component file2 = ReportComponent.builder(Component.Type.FILE, 3, "FILE2_KEY").setStatus(Component.Status.SAME).build();
addDbFileHash(file1, "hash1");
analysisMetadataHolder.setBaseAnalysis(null);
Component file1 = ReportComponent.builder(Component.Type.FILE, 2, "FILE1_KEY").setStatus(Component.Status.SAME)
- .setFileAttributes(new FileAttributes(false, null, 10, true)).build();
+ .setFileAttributes(new FileAttributes(false, null, 10, true, null)).build();
Component file2 = ReportComponent.builder(Component.Type.FILE, 3, "FILE2_KEY").setStatus(Component.Status.SAME).build();
addReportFileHash(file1, "hash1");
@Test
public void isDataUnchanged_returns_false_if_not_set_by_analyzer() {
Component file1 = ReportComponent.builder(Component.Type.FILE, 2, "FILE1_KEY").setStatus(Component.Status.SAME)
- .setFileAttributes(new FileAttributes(false, null, 10, false)).build();
+ .setFileAttributes(new FileAttributes(false, null, 10, false,null)).build();
Component file2 = ReportComponent.builder(Component.Type.FILE, 3, "FILE2_KEY").setStatus(Component.Status.SAME).build();
addDbFileHash(file1, "hash1");
@Test
public void isDataUnchanged_returns_true_if_set_by_analyzer_and_all_SAME_status_are_correct() {
Component file1 = ReportComponent.builder(Component.Type.FILE, 2, "FILE1_KEY").setStatus(Component.Status.SAME)
- .setFileAttributes(new FileAttributes(false, null, 10, true)).build();
+ .setFileAttributes(new FileAttributes(false, null, 10, true,null)).build();
Component file2 = ReportComponent.builder(Component.Type.FILE, 3, "FILE2_KEY").setStatus(Component.Status.SAME).build();
Component file3 = ReportComponent.builder(Component.Type.FILE, 4, "FILE3_KEY").setStatus(Component.Status.CHANGED).build();
assertThatThrownBy(() -> underTest.register(component))
.isInstanceOf(IllegalStateException.class)
- .hasMessage("No file can be registered on first analysis");
+ .hasMessage("No file can be registered on first branch analysis");
}
private static Component newComponent(Component.Type type) {
import org.sonar.core.util.Uuids;
import org.sonar.db.DbClient;
import org.sonar.db.DbTester;
+import org.sonar.db.component.BranchType;
import org.sonar.db.component.ComponentDto;
import org.sonar.db.component.ComponentTesting;
import org.sonar.db.source.FileSourceDto;
import static org.mockito.Mockito.when;
import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder;
import static org.sonar.ce.task.projectanalysis.filemove.FileMoveDetectionStep.MIN_REQUIRED_SCORE;
+import static org.sonar.db.component.BranchType.*;
public class FileMoveDetectionStepTest {
",}",
};
- @Rule
- public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule();
@Rule
public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();
@Rule
@Rule
public LogTester logTester = new LogTester();
- private DbClient dbClient = dbTester.getDbClient();
+ private final DbClient dbClient = dbTester.getDbClient();
private ComponentDto project;
- private SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class);
- private FileSimilarity fileSimilarity = new FileSimilarityImpl(new SourceSimilarityImpl());
- private CapturingScoreMatrixDumper scoreMatrixDumper = new CapturingScoreMatrixDumper();
- private RecordingMutableAddedFileRepository addedFileRepository = new RecordingMutableAddedFileRepository();
+ private final AnalysisMetadataHolderRule analysisMetadataHolder = mock(AnalysisMetadataHolderRule.class);
+ private final SourceLinesHashRepository sourceLinesHash = mock(SourceLinesHashRepository.class);
+ private final FileSimilarity fileSimilarity = new FileSimilarityImpl(new SourceSimilarityImpl());
+ private final CapturingScoreMatrixDumper scoreMatrixDumper = new CapturingScoreMatrixDumper();
+ private final RecordingMutableAddedFileRepository addedFileRepository = new RecordingMutableAddedFileRepository();
- private FileMoveDetectionStep underTest = new FileMoveDetectionStep(analysisMetadataHolder, treeRootHolder, dbClient,
+ private final FileMoveDetectionStep underTest = new FileMoveDetectionStep(analysisMetadataHolder, treeRootHolder, dbClient,
fileSimilarity, movedFilesRepository, sourceLinesHash, scoreMatrixDumper, addedFileRepository);
@Before
}
@Test
- public void execute_detects_no_move_on_first_analysis() {
- analysisMetadataHolder.setBaseAnalysis(null);
+ public void execute_detects_no_move_if_in_pull_request_scope() {
+ prepareAnalysis(PULL_REQUEST, ANALYSIS);
TestComputationStepContext context = new TestComputationStepContext();
underTest.execute(context);
verifyStatistics(context, null, null, null, null);
}
+ @Test
+ public void execute_detects_no_move_on_first_analysis() {
+ prepareAnalysis(BRANCH, null);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ verifyStatistics(context, 0, null, null, null);
+ }
+
@Test
public void execute_detects_no_move_if_baseSnapshot_has_no_file_and_report_has_no_file() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
TestComputationStepContext context = new TestComputationStepContext();
underTest.execute(context);
@Test
public void execute_detects_no_move_if_baseSnapshot_has_no_file() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
setFilesInReport(file1, file2);
@Test
public void execute_detects_no_move_if_there_is_no_file_in_report() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
insertFiles( /* no components */);
setFilesInReport();
@Test
public void execute_detects_no_move_if_file_key_exists_in_both_DB_and_report() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
insertFiles(file1.getUuid(), file2.getUuid());
@Test
public void execute_detects_move_if_content_of_file_is_same_in_DB_and_report() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, CONTENT1);
ComponentDto[] dtos = insertFiles(file1.getUuid());
@Test
public void execute_detects_no_move_if_content_of_file_is_not_similar_enough() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, LESS_CONTENT1);
insertFiles(file1.getKey());
@Test
public void execute_detects_no_move_if_content_of_file_is_empty_in_DB() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, CONTENT1);
insertFiles(file1.getKey());
@Test
public void execute_detects_no_move_if_content_of_file_has_no_path_in_DB() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, CONTENT1);
insertFiles(key -> newComponentDto(key).setPath(null), file1.getKey());
@Test
public void execute_detects_no_move_if_content_of_file_is_empty_in_report() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, CONTENT_EMPTY);
insertFiles(file1.getKey());
@Test
public void execute_detects_no_move_if_two_added_files_have_same_content_as_the_one_in_db() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, CONTENT1);
Component file3 = fileComponent(FILE_3_REF, CONTENT1);
@Test
public void execute_detects_no_move_if_two_deleted_files_have_same_content_as_the_one_added() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
Component file3 = fileComponent(FILE_3_REF, CONTENT1);
@Test
public void execute_detects_no_move_if_two_files_are_empty_in_DB() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
insertFiles(file1.getUuid(), file2.getUuid());
// - file2 deleted
// - file4 untouched
// - file5 renamed to file6 with a small change
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
Component file3 = fileComponent(FILE_3_REF, CONTENT1);
@Test
public void execute_does_not_compute_any_distance_if_all_files_sizes_are_all_too_different() {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
Component file1 = fileComponent(FILE_1_REF, null);
Component file2 = fileComponent(FILE_2_REF, null);
Component file3 = fileComponent(FILE_3_REF, arrayOf(118));
*/
@Test
public void real_life_use_case() throws Exception {
- analysisMetadataHolder.setBaseAnalysis(ANALYSIS);
+ prepareBranchAnalysis(ANALYSIS);
for (File f : FileUtils.listFiles(new File("src/test/resources/org/sonar/ce/task/projectanalysis/filemove/FileMoveDetectionStepTest/v1"), null, false)) {
insertFiles("uuid_" + f.getName().hashCode());
insertContentOfFileInDb("uuid_" + f.getName().hashCode(), readLines(f));
}
}
- private static void verifyStatistics(TestComputationStepContext context,
+ private void prepareBranchAnalysis(Analysis analysis) {
+ prepareAnalysis(BRANCH, analysis);
+ }
+
+ private void prepareAnalysis(BranchType branch, Analysis analysis) {
+ mockBranchType(branch);
+ analysisMetadataHolder.setBaseAnalysis(analysis);
+ }
+
+ private void mockBranchType(BranchType branchType) {
+ when(analysisMetadataHolder.isPullRequest()).thenReturn(branchType == PULL_REQUEST);
+ }
+
+ public static void verifyStatistics(TestComputationStepContext context,
@Nullable Integer expectedReportFiles, @Nullable Integer expectedDbFiles,
@Nullable Integer expectedAddedFiles, @Nullable Integer expectedMovedFiles) {
context.getStatistics().assertValue("reportFiles", expectedReportFiles);
context.getStatistics().assertValue("movedFiles", expectedMovedFiles);
}
- private static class RecordingMutableAddedFileRepository implements MutableAddedFileRepository {
+ public static class RecordingMutableAddedFileRepository implements MutableAddedFileRepository {
private final List<Component> components = new ArrayList<>();
@Override
--- /dev/null
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 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.filemove;
+
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.sonar.api.utils.System2;
+import org.sonar.api.utils.log.LogTester;
+import org.sonar.ce.task.projectanalysis.analysis.Analysis;
+import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule;
+import org.sonar.ce.task.projectanalysis.analysis.Branch;
+import org.sonar.ce.task.projectanalysis.component.Component;
+import org.sonar.ce.task.projectanalysis.component.FileAttributes;
+import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule;
+import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile;
+import org.sonar.ce.task.step.TestComputationStepContext;
+import org.sonar.core.util.Uuids;
+import org.sonar.db.DbClient;
+import org.sonar.db.DbTester;
+import org.sonar.db.component.BranchType;
+import org.sonar.db.component.ComponentDto;
+import org.sonar.db.component.ComponentTesting;
+import org.sonar.db.source.FileSourceDto;
+import org.sonar.server.project.Project;
+
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toSet;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.sonar.ce.task.projectanalysis.component.Component.Type.FILE;
+import static org.sonar.ce.task.projectanalysis.component.Component.Type.PROJECT;
+import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder;
+import static org.sonar.ce.task.projectanalysis.filemove.FileMoveDetectionStepTest.RecordingMutableAddedFileRepository;
+import static org.sonar.ce.task.projectanalysis.filemove.FileMoveDetectionStepTest.verifyStatistics;
+import static org.sonar.db.component.BranchType.BRANCH;
+import static org.sonar.db.component.BranchType.PULL_REQUEST;
+
+public class PullRequestFileMoveDetectionStepTest {
+ private static final String ROOT_REF = "0";
+ private static final String FILE_1_REF = "1";
+ private static final String FILE_2_REF = "2";
+ private static final String FILE_3_REF = "3";
+ private static final String FILE_4_REF = "4";
+ private static final String FILE_5_REF = "5";
+ private static final String FILE_6_REF = "6";
+ private static final String FILE_7_REF = "7";
+ private static final String TARGET_BRANCH = "target_branch";
+ private static final String BRANCH_UUID = "branch_uuid";
+ private static final String SNAPSHOT_UUID = "uuid_1";
+
+ private static final Analysis ANALYSIS = new Analysis.Builder()
+ .setUuid(SNAPSHOT_UUID)
+ .setCreatedAt(86521)
+ .build();
+
+ @Rule
+ public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();
+ @Rule
+ public MutableMovedFilesRepositoryRule movedFilesRepository = new MutableMovedFilesRepositoryRule();
+ @Rule
+ public DbTester dbTester = DbTester.create(System2.INSTANCE);
+ @Rule
+ public LogTester logTester = new LogTester();
+
+ private ComponentDto branch;
+ private ComponentDto project;
+
+ private final DbClient dbClient = dbTester.getDbClient();
+ private final AnalysisMetadataHolderRule analysisMetadataHolder = mock(AnalysisMetadataHolderRule.class);
+ private final RecordingMutableAddedFileRepository addedFileRepository = new RecordingMutableAddedFileRepository();
+ private final PullRequestFileMoveDetectionStep underTest = new PullRequestFileMoveDetectionStep(analysisMetadataHolder, treeRootHolder, dbClient, movedFilesRepository, addedFileRepository);
+
+ @Before
+ public void setUp() throws Exception {
+ project = dbTester.components().insertPrivateProject();
+ branch = dbTester.components().insertProjectBranch(project, branchDto -> branchDto.setUuid(BRANCH_UUID).setKey(TARGET_BRANCH));
+ treeRootHolder.setRoot(builder(Component.Type.PROJECT, Integer.parseInt(ROOT_REF)).setUuid(project.uuid()).build());
+ }
+
+ @Test
+ public void getDescription_returns_description() {
+ assertThat(underTest.getDescription()).isEqualTo("Detect file moves in Pull Request scope");
+ }
+
+ @Test
+ public void execute_does_not_detect_any_files_if_not_in_pull_request_scope() {
+ prepareAnalysis(BRANCH, ANALYSIS);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ verifyStatistics(context, null, null, null, null);
+ }
+
+ @Test
+ public void execute_detects_no_move_if_report_has_no_file() {
+ preparePullRequestAnalysis(ANALYSIS);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ assertThat(addedFileRepository.getComponents()).isEmpty();
+ verifyStatistics(context, 0, null, null, null);
+ }
+
+ @Test
+ public void execute_detects_no_move_if_target_branch_has_no_files() {
+ preparePullRequestAnalysis(ANALYSIS);
+ Set<FileReference> fileReferences = Set.of(FileReference.of(FILE_1_REF), FileReference.of(FILE_2_REF));
+ Map<String, Component> reportFilesByUuid = initializeAnalysisReportComponents(fileReferences);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ assertThat(addedFileRepository.getComponents()).containsOnlyOnceElementsOf(reportFilesByUuid.values());
+ verifyStatistics(context, 2, 0, 2, null);
+ }
+
+ @Test
+ public void execute_detects_no_move_if_there_are_no_files_in_report() {
+ preparePullRequestAnalysis(ANALYSIS);
+ initializeAnalysisReportComponents(Set.of());
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ assertThat(addedFileRepository.getComponents()).isEmpty();
+ verifyStatistics(context, 0, null, null, null);
+ }
+
+ @Test
+ public void execute_detects_no_move_if_file_key_exists_in_both_database_and_report() {
+ preparePullRequestAnalysis(ANALYSIS);
+
+ Set<FileReference> fileReferences = Set.of(FileReference.of(FILE_1_REF), FileReference.of(FILE_2_REF));
+ initializeAnalysisReportComponents(fileReferences);
+ initializeTargetBranchDatabaseComponents(fileReferences);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).isEmpty();
+ assertThat(addedFileRepository.getComponents()).isEmpty();
+ verifyStatistics(context, 2, 2, 0, 0);
+ }
+
+ @Test
+ public void execute_detects_renamed_file() {
+ // - FILE_1_REF on target branch is renamed to FILE_2_REF on Pull Request
+ preparePullRequestAnalysis(ANALYSIS);
+
+ Set<FileReference> reportFileReferences = Set.of(FileReference.of(FILE_2_REF, FILE_1_REF));
+ Set<FileReference> databaseFileReferences = Set.of(FileReference.of(FILE_1_REF));
+
+ Map<String, Component> reportFilesByUuid = initializeAnalysisReportComponents(reportFileReferences);
+ Map<String, Component> databaseFilesByUuid = initializeTargetBranchDatabaseComponents(databaseFileReferences);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(addedFileRepository.getComponents()).isEmpty();
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).hasSize(1);
+ assertThatFileRenameHasBeenDetected(reportFilesByUuid, databaseFilesByUuid, FILE_2_REF, FILE_1_REF);
+ verifyStatistics(context, 1, 1, 0, 1);
+ }
+
+ @Test
+ public void execute_detects_several_renamed_file() {
+ // - FILE_1_REF has been renamed to FILE_3_REF on Pull Request
+ // - FILE_2_REF has been deleted on Pull Request
+ // - FILE_4_REF has been left untouched
+ // - FILE_5_REF has been renamed to FILE_6_REF on Pull Request
+ // - FILE_7_REF has been added on Pull Request
+ preparePullRequestAnalysis(ANALYSIS);
+
+ Set<FileReference> reportFileReferences = Set.of(
+ FileReference.of(FILE_3_REF, FILE_1_REF),
+ FileReference.of(FILE_4_REF),
+ FileReference.of(FILE_6_REF, FILE_5_REF),
+ FileReference.of(FILE_7_REF));
+
+ Set<FileReference> databaseFileReferences = Set.of(
+ FileReference.of(FILE_1_REF),
+ FileReference.of(FILE_2_REF),
+ FileReference.of(FILE_4_REF),
+ FileReference.of(FILE_5_REF));
+
+ Map<String, Component> reportFilesByUuid = initializeAnalysisReportComponents(reportFileReferences);
+ Map<String, Component> databaseFilesByUuid = initializeTargetBranchDatabaseComponents(databaseFileReferences);
+
+ TestComputationStepContext context = new TestComputationStepContext();
+ underTest.execute(context);
+
+ assertThat(addedFileRepository.getComponents()).hasSize(1);
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).hasSize(2);
+ assertThatFileAdditionHasBeenDetected(reportFilesByUuid, FILE_7_REF);
+ assertThatFileRenameHasBeenDetected(reportFilesByUuid, databaseFilesByUuid, FILE_3_REF, FILE_1_REF);
+ assertThatFileRenameHasBeenDetected(reportFilesByUuid, databaseFilesByUuid, FILE_6_REF, FILE_5_REF);
+ verifyStatistics(context, 4, 4, 1, 2);
+ }
+
+ private void assertThatFileAdditionHasBeenDetected(Map<String, Component> reportFilesByUuid, String fileInReportReference) {
+ Component fileInReport = reportFilesByUuid.get(fileInReportReference);
+
+ assertThat(addedFileRepository.getComponents()).contains(fileInReport);
+ assertThat(movedFilesRepository.getOriginalPullRequestFile(fileInReport)).isEmpty();
+ }
+
+
+ private void assertThatFileRenameHasBeenDetected(Map<String, Component> reportFilesByUuid, Map<String, Component> databaseFilesByUuid, String fileInReportReference, String originalFileInDatabaseReference) {
+ Component fileInReport = reportFilesByUuid.get(fileInReportReference);
+ Component originalFileInDatabase = databaseFilesByUuid.get(originalFileInDatabaseReference);
+
+ assertThat(movedFilesRepository.getComponentsWithOriginal()).contains(fileInReport);
+ assertThat(movedFilesRepository.getOriginalPullRequestFile(fileInReport)).isPresent();
+
+ OriginalFile detectedOriginalFile = movedFilesRepository.getOriginalPullRequestFile(fileInReport).get();
+ assertThat(detectedOriginalFile.getKey()).isEqualTo(originalFileInDatabase.getKey());
+ assertThat(detectedOriginalFile.getUuid()).isEqualTo(originalFileInDatabase.getUuid());
+ }
+
+ private Map<String, Component> initializeTargetBranchDatabaseComponents(Set<FileReference> references) {
+ Set<Component> fileComponents = createFileComponents(references);
+ insertFileComponentsInDatabase(fileComponents);
+ return toFileComponentsByUuidMap(fileComponents);
+ }
+
+ private Map<String, Component> initializeAnalysisReportComponents(Set<FileReference> refs) {
+ Set<Component> fileComponents = createFileComponents(refs);
+ insertFileComponentsInReport(fileComponents);
+ return toFileComponentsByUuidMap(fileComponents);
+ }
+
+ private Map<String, Component> toFileComponentsByUuidMap(Set<Component> fileComponents) {
+ return fileComponents
+ .stream()
+ .collect(toMap(Component::getUuid, identity()));
+ }
+
+ private static Set<Component> createFileComponents(Set<FileReference> references) {
+ return references
+ .stream()
+ .map(PullRequestFileMoveDetectionStepTest::createReportFileComponent)
+ .collect(toSet());
+ }
+
+ private static Component createReportFileComponent(FileReference fileReference) {
+ return builder(FILE, Integer.parseInt(fileReference.getReference()))
+ .setUuid(fileReference.getReference())
+ .setName("report_path" + fileReference.getReference())
+ .setFileAttributes(new FileAttributes(false, null, 1, false, composeComponentPath(fileReference.getPastReference())))
+ .build();
+ }
+
+ private void insertFileComponentsInReport(Set<Component> files) {
+ treeRootHolder
+ .setRoot(builder(PROJECT, Integer.parseInt(ROOT_REF))
+ .setUuid(project.uuid())
+ .addChildren(files.toArray(Component[]::new))
+ .build());
+ }
+
+ private Set<ComponentDto> insertFileComponentsInDatabase(Set<Component> files) {
+ return files
+ .stream()
+ .map(Component::getUuid)
+ .map(this::composeComponentDto)
+ .peek(this::insertComponentDto)
+ .peek(this::insertContentOfFileInDatabase)
+ .collect(toSet());
+ }
+
+ private void insertComponentDto(ComponentDto component) {
+ dbTester.components().insertComponent(component);
+ }
+
+ private ComponentDto composeComponentDto(String uuid) {
+ return ComponentTesting
+ .newFileDto(project)
+ .setBranchUuid(branch.uuid())
+ .setKey("key_" + uuid)
+ .setUuid(uuid)
+ .setPath(composeComponentPath(uuid));
+ }
+
+ @CheckForNull
+ private static String composeComponentPath(@Nullable String reference) {
+ return Optional.ofNullable(reference)
+ .map(r -> String.join("_", "path", r))
+ .orElse(null);
+ }
+
+ private FileSourceDto insertContentOfFileInDatabase(ComponentDto file) {
+ FileSourceDto fileSourceDto = composeFileSourceDto(file);
+ persistFileSourceDto(fileSourceDto);
+ return fileSourceDto;
+ }
+
+ private static FileSourceDto composeFileSourceDto(ComponentDto file) {
+ return new FileSourceDto()
+ .setUuid(Uuids.createFast())
+ .setFileUuid(file.uuid())
+ .setProjectUuid(file.branchUuid());
+ }
+
+ private void persistFileSourceDto(FileSourceDto fileSourceDto) {
+ dbTester.getDbClient().fileSourceDao().insert(dbTester.getSession(), fileSourceDto);
+ dbTester.commit();
+ }
+
+ private void preparePullRequestAnalysis(Analysis analysis) {
+ prepareAnalysis(PULL_REQUEST, analysis);
+ }
+
+ private void prepareAnalysis(BranchType branch, Analysis analysis) {
+ mockBranchType(branch);
+ analysisMetadataHolder.setBaseAnalysis(analysis);
+ }
+
+ private void mockBranchType(BranchType branchType) {
+ Branch branch = mock(Branch.class);
+ when(analysisMetadataHolder.getBranch()).thenReturn(branch);
+ when(analysisMetadataHolder.getBranch().getTargetBranchName()).thenReturn(TARGET_BRANCH);
+ when(analysisMetadataHolder.isPullRequest()).thenReturn(branchType == PULL_REQUEST);
+ when(analysisMetadataHolder.getProject()).thenReturn(Project.from(project));
+ }
+
+ @Immutable
+ private static class FileReference {
+ private final String reference;
+ private final String pastReference;
+
+ private FileReference(String reference, @Nullable String pastReference) {
+ this.reference = reference;
+ this.pastReference = pastReference;
+ }
+
+ public String getReference() {
+ return reference;
+ }
+
+ @CheckForNull
+ public String getPastReference() {
+ return pastReference;
+ }
+
+ public static FileReference of(String reference, String pastReference) {
+ return new FileReference(reference, pastReference);
+ }
+
+ public static FileReference of(String reference) {
+ return new FileReference(reference, null);
+ }
+ }
+}
package org.sonar.ce.task.projectanalysis.issue;
import java.util.Collections;
+import java.util.Optional;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
public class TrackerTargetBranchInputFactoryTest {
private final static String COMPONENT_KEY = "file1";
private final static String COMPONENT_UUID = "uuid1";
+ private final static String ORIGINAL_COMPONENT_KEY = "file2";
+ private final static String ORIGINAL_COMPONENT_UUID = "uuid2";
@Rule
public DbTester db = DbTester.create();
ComponentDto fileDto = ComponentTesting.newFileDto(ComponentTesting.newPublicProjectDto()).setUuid(COMPONENT_UUID);
db.fileSources().insertFileSource(fileDto, 3);
- Component component = mock(Component.class);
- when(component.getKey()).thenReturn(COMPONENT_KEY);
- when(component.getType()).thenReturn(Component.Type.FILE);
+ Component component = getComponent();
Input<DefaultIssue> input = underTest.createForTargetBranch(component);
assertThat(input.getIssues()).containsOnly(issue1);
ComponentDto fileDto = ComponentTesting.newFileDto(ComponentTesting.newPublicProjectDto()).setUuid(COMPONENT_UUID);
db.fileSources().insertFileSource(fileDto, 0);
- Component component = mock(Component.class);
- when(component.getKey()).thenReturn(COMPONENT_KEY);
- when(component.getType()).thenReturn(Component.Type.FILE);
+ Component component = getComponent();
Input<DefaultIssue> input = underTest.createForTargetBranch(component);
assertThat(input.getIssues()).containsOnly(issue1);
@Test
public void gets_nothing_when_there_is_no_matching_component() {
- Component component = mock(Component.class);
- when(component.getKey()).thenReturn(COMPONENT_KEY);
- when(component.getType()).thenReturn(Component.Type.FILE);
+ Component component = getComponent();
Input<DefaultIssue> input = underTest.createForTargetBranch(component);
assertThat(input.getIssues()).isEmpty();
assertThat(input.getLineHashSequence().length()).isZero();
}
+ @Test
+ public void uses_original_component_uuid_when_component_is_moved_file() {
+ Component component = getComponent();
+ MovedFilesRepository.OriginalFile originalFile = new MovedFilesRepository.OriginalFile(ORIGINAL_COMPONENT_UUID, ORIGINAL_COMPONENT_KEY);
+ when(movedFilesRepository.getOriginalPullRequestFile(component)).thenReturn(Optional.of(originalFile));
+ when(targetBranchComponentUuids.getTargetBranchComponentUuid(ORIGINAL_COMPONENT_KEY))
+ .thenReturn(ORIGINAL_COMPONENT_UUID);
+ DefaultIssue issue1 = new DefaultIssue();
+ when(componentIssuesLoader.loadOpenIssuesWithChanges(ORIGINAL_COMPONENT_UUID)).thenReturn(Collections.singletonList(issue1));
+
+
+ Input<DefaultIssue> targetBranchIssue = underTest.createForTargetBranch(component);
+
+ verify(targetBranchComponentUuids).getTargetBranchComponentUuid(ORIGINAL_COMPONENT_KEY);
+ assertThat(targetBranchIssue.getIssues()).containsOnly(issue1);
+ }
+
+ private Component getComponent() {
+ Component component = mock(Component.class);
+ when(component.getKey()).thenReturn(COMPONENT_KEY);
+ when(component.getType()).thenReturn(Component.Type.FILE);
+ return component;
+ }
+
@Test
public void hasTargetBranchAnalysis_returns_true_if_source_branch_of_pr_was_analysed() {
when(targetBranchComponentUuids.hasTargetBranchAnalysis()).thenReturn(true);
*/
public class ReportComponent implements Component {
- private static final FileAttributes DEFAULT_FILE_ATTRIBUTES = new FileAttributes(false, null, 1, false);
+ private static final FileAttributes DEFAULT_FILE_ATTRIBUTES = new FileAttributes(false, null, 1, false, null);
public static final Component DUMB_PROJECT = builder(Type.PROJECT, 1)
.setKey("PROJECT_KEY")
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
private final Type type;
private final Path absolutePath;
private final SensorStrategy sensorStrategy;
- private final String oldFilePath;
+ private final String oldRelativeFilePath;
/**
* Testing purposes only!
}
public DefaultIndexedFile(Path absolutePath, String projectKey, String projectRelativePath, String moduleRelativePath, Type type, @Nullable String language, int batchId,
- SensorStrategy sensorStrategy, @Nullable String oldFilePath) {
+ SensorStrategy sensorStrategy, @Nullable String oldRelativeFilePath) {
super(batchId);
this.projectKey = projectKey;
this.projectRelativePath = PathUtils.sanitize(projectRelativePath);
this.language = language;
this.sensorStrategy = sensorStrategy;
this.absolutePath = absolutePath;
- this.oldFilePath = oldFilePath;
+ this.oldRelativeFilePath = oldRelativeFilePath;
}
@Override
}
@CheckForNull
- public String oldPath() {
- return oldFilePath;
- }
-
- public boolean isMovedFile() {
- return Objects.nonNull(this.oldPath());
+ public String oldRelativePath() {
+ return oldRelativeFilePath;
}
@Override
}
@CheckForNull
- public String oldPath() {
- return indexedFile.oldPath();
- }
-
- public boolean isMovedFile() {
- return indexedFile.isMovedFile();
+ public String oldRelativePath() {
+ return indexedFile.oldRelativePath();
}
@Override
private final int id;
private final String relativePath;
+ private String oldRelativePath;
private final String projectKey;
@CheckForNull
private Path projectBaseDir;
this.id = id;
}
+ public TestInputFileBuilder(String projectKey, String relativePath, String oldRelativePath, int id) {
+ this.projectKey = projectKey;
+ setModuleBaseDir(Paths.get(projectKey));
+ this.relativePath = PathUtils.sanitize(relativePath);
+ this.oldRelativePath = oldRelativePath;
+ this.id = id;
+ }
+
public static TestInputFileBuilder create(String moduleKey, File moduleBaseDir, File filePath) {
return new TestInputFileBuilder(moduleKey, moduleBaseDir, filePath);
}
projectBaseDir = moduleBaseDir;
}
String projectRelativePath = projectBaseDir.relativize(absolutePath).toString();
- DefaultIndexedFile indexedFile = new DefaultIndexedFile(absolutePath, projectKey, projectRelativePath, relativePath, type, language, id, new SensorStrategy());
+ DefaultIndexedFile indexedFile = new DefaultIndexedFile(absolutePath, projectKey, projectRelativePath, relativePath, type, language, id, new SensorStrategy(), oldRelativePath);
DefaultInputFile inputFile = new DefaultInputFile(indexedFile,
f -> f.setMetadata(new Metadata(lines, nonBlankLines, hash, originalLineStartOffsets, originalLineEndOffsets, lastValidOffset)),
contents);
private static final String PROJECT_RELATIVE_PATH = "module1/src/Foo.php";
private static final String MODULE_RELATIVE_PATH = "src/Foo.php";
+ private static final String OLD_RELATIVE_PATH = "src/previous/Foo.php";
@Rule
public TemporaryFolder temp = new TemporaryFolder();
assertThat(new File(inputFile.relativePath())).isRelative();
}
+ @Test
+ public void test_moved_file() {
+ DefaultIndexedFile indexedFileForMovedFile = new DefaultIndexedFile(baseDir.resolve(PROJECT_RELATIVE_PATH), "ABCDE", PROJECT_RELATIVE_PATH, MODULE_RELATIVE_PATH, InputFile.Type.TEST, "php", 0,
+ sensorStrategy, OLD_RELATIVE_PATH);
+ Metadata metadata = new Metadata(42, 42, "", new int[0], new int[0], 10);
+ DefaultInputFile inputFile = new DefaultInputFile(indexedFileForMovedFile, (f) -> f.setMetadata(metadata))
+ .setStatus(InputFile.Status.ADDED)
+ .setCharset(StandardCharsets.ISO_8859_1);
+
+ assertThat(inputFile.oldRelativePath()).isEqualTo(OLD_RELATIVE_PATH);
+ }
+
@Test
public void test_content() throws IOException {
Path testFile = baseDir.resolve(PROJECT_RELATIVE_PATH);
Files.createDirectories(testFile.getParent());
String content = "test é string";
- Files.write(testFile, content.getBytes(StandardCharsets.ISO_8859_1));
+ Files.writeString(testFile, content, StandardCharsets.ISO_8859_1);
assertThat(Files.readAllLines(testFile, StandardCharsets.ISO_8859_1).get(0)).hasSize(content.length());
@Test
public void test_toString() {
DefaultInputFile file = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), MODULE_RELATIVE_PATH, null), (f) -> mock(Metadata.class));
- assertThat(file.toString()).isEqualTo(MODULE_RELATIVE_PATH);
+ assertThat(file).hasToString(MODULE_RELATIVE_PATH);
}
@Test
public void checkValidPointer() {
- Metadata metadata = new Metadata(2, 2, "", new int[] {0, 10}, new int[] {9, 15}, 16);
+ Metadata metadata = new Metadata(2, 2, "", new int[]{0, 10}, new int[]{9, 15}, 16);
DefaultInputFile file = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), MODULE_RELATIVE_PATH, null), f -> f.setMetadata(metadata));
assertThat(file.newPointer(1, 0).line()).isOne();
assertThat(file.newPointer(1, 0).lineOffset()).isZero();
@Test
public void checkValidPointerUsingGlobalOffset() {
- Metadata metadata = new Metadata(2, 2, "", new int[] {0, 10}, new int[] {8, 15}, 16);
+ Metadata metadata = new Metadata(2, 2, "", new int[]{0, 10}, new int[]{8, 15}, 16);
DefaultInputFile file = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), MODULE_RELATIVE_PATH, null), f -> f.setMetadata(metadata));
assertThat(file.newPointer(0).line()).isOne();
assertThat(file.newPointer(0).lineOffset()).isZero();
@Test
public void checkValidRangeUsingGlobalOffset() {
- Metadata metadata = new Metadata(2, 2, "", new int[] {0, 10}, new int[] {9, 15}, 16);
+ Metadata metadata = new Metadata(2, 2, "", new int[]{0, 10}, new int[]{9, 15}, 16);
DefaultInputFile file = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), MODULE_RELATIVE_PATH, null), f -> f.setMetadata(metadata));
TextRange newRange = file.newRange(10, 13);
assertThat(newRange.start().line()).isEqualTo(2);
@Test
public void testRangeOverlap() {
- Metadata metadata = new Metadata(2, 2, "", new int[] {0, 10}, new int[] {9, 15}, 16);
+ Metadata metadata = new Metadata(2, 2, "", new int[]{0, 10}, new int[]{9, 15}, 16);
DefaultInputFile file = new DefaultInputFile(new DefaultIndexedFile("ABCDE", Paths.get("module"), MODULE_RELATIVE_PATH, null), f -> f.setMetadata(metadata));
// Don't fail
assertThat(file.newRange(file.newPointer(1, 0), file.newPointer(1, 1)).overlap(file.newRange(file.newPointer(1, 0), file.newPointer(1, 1)))).isTrue();
package org.sonar.scanner.report;
import java.nio.file.Path;
+import java.util.Collection;
import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.stream.Collectors;
import java.util.stream.StreamSupport;
+import javax.annotation.CheckForNull;
import org.sonar.api.batch.fs.internal.DefaultInputFile;
import org.sonar.api.batch.fs.internal.DefaultInputProject;
import org.sonar.api.batch.scm.ScmProvider;
import org.sonar.scanner.scan.branch.BranchConfiguration;
import org.sonar.scanner.scan.filesystem.InputComponentStore;
import org.sonar.scanner.scm.ScmConfiguration;
+import org.sonar.scm.git.ChangedFile;
import org.sonar.scm.git.GitScmProvider;
import static java.util.Optional.empty;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
public class ChangedLinesPublisher implements ReportPublisherStep {
private static final Logger LOG = Loggers.get(ChangedLinesPublisher.class);
private int writeChangedLines(ScmProvider provider, ScannerReportWriter writer, String targetScmBranch) {
Path rootBaseDir = project.getBaseDir();
Map<Path, DefaultInputFile> changedFiles = StreamSupport.stream(inputComponentStore.allChangedFilesToPublish().spliterator(), false)
- .collect(Collectors.toMap(DefaultInputFile::path, f -> f));
+ .collect(toMap(DefaultInputFile::path, identity()));
- Map<Path, Set<Integer>> pathSetMap = ((GitScmProvider) provider).branchChangedLines(targetScmBranch, rootBaseDir, changedFiles); // TODO: Extend ScmProvider abstract
+ Map<Path, Set<Integer>> pathSetMap = getBranchChangedLinesByScm(provider, targetScmBranch, rootBaseDir, toChangedFilesByPathMap(changedFiles.values()));
int count = 0;
if (pathSetMap == null) {
builder.addAllLine(changedLines);
writer.writeComponentChangedLines(fileRef, builder.build());
}
+
+ @CheckForNull
+ private static Map<Path, Set<Integer>> getBranchChangedLinesByScm(ScmProvider scmProvider, String targetScmBranch, Path rootBaseDir, Map<Path, ChangedFile> changedFiles) {
+ if (scmProvider instanceof GitScmProvider) {
+ return ((GitScmProvider) scmProvider).branchChangedLinesWithFileMovementDetection(targetScmBranch, rootBaseDir, changedFiles);
+ }
+
+ return scmProvider.branchChangedLines(targetScmBranch, rootBaseDir, changedFiles.keySet());
+ }
+
+ private static Map<Path, ChangedFile> toChangedFilesByPathMap(Collection<DefaultInputFile> files) {
+ return files
+ .stream()
+ .map(ChangedFile::of)
+ .collect(toMap(ChangedFile::getAbsolutFilePath, identity()));
+ }
}
fileBuilder.setStatus(convert(file.status()));
fileBuilder.setMarkedAsUnchanged(file.isMarkedAsUnchanged());
- if (file.isMovedFile()) {
- fileBuilder.setOldRelativeFilePath(file.oldPath());
+ String oldRelativePath = file.oldRelativePath();
+ if (oldRelativePath != null) {
+ fileBuilder.setOldRelativeFilePath(oldRelativePath);
}
String lang = getLanguageKey(file);
language != null ? language.key() : null,
scannerComponentIdGenerator.getAsInt(),
sensorStrategy,
- scmChangedFiles.getFileOldPath(realAbsoluteFile)
+ scmChangedFiles.getOldRelativeFilePath(realAbsoluteFile)
);
DefaultInputFile inputFile = new DefaultInputFile(indexedFile, f -> metadataGenerator.setMetadata(module.key(), f, module.getEncoding()));
import java.nio.file.Path;
import java.util.Collection;
-import java.util.List;
+import java.util.Map;
import java.util.Optional;
-import java.util.function.Predicate;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import org.sonar.scm.git.ChangedFile;
+import static java.util.Collections.emptyMap;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+
@Immutable
public class ScmChangedFiles {
@Nullable
private final Collection<ChangedFile> changedFiles;
+ private final Map<Path, ChangedFile> changedFilesByPath;
public ScmChangedFiles(@Nullable Collection<ChangedFile> changedFiles) {
this.changedFiles = changedFiles;
+ this.changedFilesByPath = toChangedFilesByPathMap(changedFiles);
}
public boolean isChanged(Path file) {
throw new IllegalStateException("Scm didn't provide valid data");
}
- return this.findFile(file).isPresent();
+ return this.getChangedFile(file).isPresent();
}
public boolean isValid() {
}
@CheckForNull
- public String getFileOldPath(Path absoluteFilePath) {
- return this.findFile(absoluteFilePath)
- .filter(ChangedFile::isMoved)
- .map(ChangedFile::getOldFilePath)
+ public String getOldRelativeFilePath(Path absoluteFilePath) {
+ return this.getChangedFile(absoluteFilePath)
+ .filter(ChangedFile::isMovedFile)
+ .map(ChangedFile::getOldRelativeFilePathReference)
.orElse(null);
}
- private Optional<ChangedFile> findFile(Path absoluteFilePath) {
- Predicate<ChangedFile> isTargetFile = file -> file.getAbsolutFilePath().equals(absoluteFilePath);
+ private Optional<ChangedFile> getChangedFile(Path absoluteFilePath) {
+ return Optional.ofNullable(changedFilesByPath.get(absoluteFilePath));
+ }
- return Optional.ofNullable(this.get())
- .orElseGet(List::of)
- .stream()
- .filter(isTargetFile)
- .findFirst();
+ private static Map<Path, ChangedFile> toChangedFilesByPathMap(@Nullable Collection<ChangedFile> changedFiles) {
+ return Optional.ofNullable(changedFiles)
+ .map(files -> files.stream().collect(toMap(ChangedFile::getAbsolutFilePath, identity())))
+ .orElse(emptyMap());
}
}
import java.nio.file.Path;
import java.util.Collection;
import java.util.Set;
-import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
import org.sonar.api.batch.fs.internal.DefaultInputProject;
+import org.sonar.api.batch.scm.ScmProvider;
import org.sonar.api.impl.utils.ScannerUtils;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
import org.sonar.scm.git.GitScmProvider;
import org.springframework.context.annotation.Bean;
+import static java.util.stream.Collectors.toSet;
+
public class ScmChangedFilesProvider {
private static final Logger LOG = Loggers.get(ScmChangedFilesProvider.class);
private static final String LOG_MSG = "SCM collecting changed files in the branch";
Collection<ChangedFile> changedFiles = loadChangedFilesIfNeeded(scmConfiguration, branchConfiguration, rootBaseDir);
if (changedFiles != null) {
- validatePaths(getFilePaths(changedFiles));
+ validatePaths(getAbsoluteFilePaths(changedFiles));
}
return new ScmChangedFiles(changedFiles);
}
private static void validatePaths(Set<Path> changedFilePaths) {
- if (changedFilePaths != null && changedFilePaths.stream().anyMatch(p -> !p.isAbsolute())) {
+ if (changedFilePaths.stream().anyMatch(p -> !p.isAbsolute())) {
throw new IllegalStateException("SCM provider returned a changed file with a relative path but paths must be absolute. Please fix the provider.");
}
}
- private static Set<Path> getFilePaths(Collection<ChangedFile> changedFiles) {
+ private static Set<Path> getAbsoluteFilePaths(Collection<ChangedFile> changedFiles) {
return changedFiles
.stream()
.map(ChangedFile::getAbsolutFilePath)
- .collect(Collectors.toSet());
+ .collect(toSet());
}
@CheckForNull
private static Collection<ChangedFile> loadChangedFilesIfNeeded(ScmConfiguration scmConfiguration, BranchConfiguration branchConfiguration, Path rootBaseDir) {
final String targetBranchName = branchConfiguration.targetBranchName();
if (branchConfiguration.isPullRequest() && targetBranchName != null) {
- GitScmProvider scmProvider = (GitScmProvider) scmConfiguration.provider();
+ ScmProvider scmProvider = scmConfiguration.provider();
if (scmProvider != null) {
Profiler profiler = Profiler.create(LOG).startInfo(LOG_MSG);
- Collection<ChangedFile> changedFiles = scmProvider.branchModifiedFiles(targetBranchName, rootBaseDir);
+ Collection<ChangedFile> changedFiles = getChangedFilesByScm(scmProvider, targetBranchName, rootBaseDir);
profiler.stopInfo();
if (changedFiles != null) {
LOG.debug("SCM reported {} {} changed in the branch", changedFiles.size(), ScannerUtils.pluralize("file", changedFiles.size()));
return null;
}
+ private static Collection<ChangedFile> getChangedFilesByScm(ScmProvider scmProvider, String targetBranchName, Path rootBaseDir) {
+ if (scmProvider instanceof GitScmProvider) {
+ return ((GitScmProvider) scmProvider).branchChangedFilesWithFileMovementDetection(targetBranchName, rootBaseDir);
+ }
+
+ return toChangedFiles(scmProvider.branchChangedFiles(targetBranchName, rootBaseDir));
+ }
+
+ @CheckForNull
+ private static Collection<ChangedFile> toChangedFiles(@Nullable Set<Path> changedPaths) {
+ if (changedPaths == null) {
+ return null;
+ }
+
+ return changedPaths
+ .stream()
+ .map(ChangedFile::of)
+ .collect(toSet());
+ }
}
import java.util.Objects;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
+import org.sonar.api.batch.fs.internal.DefaultInputFile;
+@Immutable
public class ChangedFile {
- @Nullable
- private final String oldFilePath;
- private final String filePath;
private final Path absoluteFilePath;
+ private final String oldRelativeFilePathReference;
- public ChangedFile(String filePath, Path absoluteFilePath) {
- this(filePath, absoluteFilePath, null);
+ private ChangedFile(Path absoluteFilePath, @Nullable String oldRelativeFilePathReference) {
+ this.absoluteFilePath = absoluteFilePath;
+ this.oldRelativeFilePathReference = oldRelativeFilePathReference;
}
- public ChangedFile(String filePath, Path absoluteFilePath, @Nullable String oldFilePath) {
- this.filePath = filePath;
- this.oldFilePath = oldFilePath;
- this.absoluteFilePath = absoluteFilePath;
+ public Path getAbsolutFilePath() {
+ return absoluteFilePath;
}
@CheckForNull
- public String getOldFilePath() {
- return oldFilePath;
+ public String getOldRelativeFilePathReference() {
+ return oldRelativeFilePathReference;
}
- public boolean isMoved() {
- return Objects.nonNull(this.getOldFilePath());
+ public boolean isMovedFile() {
+ return this.getOldRelativeFilePathReference() != null;
}
- public String getFilePath() {
- return filePath;
+ public static ChangedFile of(Path path) {
+ return new ChangedFile(path, null);
}
- public Path getAbsolutFilePath() {
- return absoluteFilePath;
+ public static ChangedFile of(Path path, @Nullable String oldRelativeFilePathReference) {
+ return new ChangedFile(path, oldRelativeFilePathReference);
+ }
+
+ public static ChangedFile of(DefaultInputFile file) {
+ return new ChangedFile(file.path(), file.oldRelativePath());
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+
+ ChangedFile that = (ChangedFile) o;
+
+ return Objects.equals(oldRelativeFilePathReference, that.oldRelativeFilePathReference)
+ && Objects.equals(absoluteFilePath, that.absoluteFilePath);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(oldRelativeFilePathReference, absoluteFilePath);
}
}
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
-import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.regex.Pattern;
-import java.util.stream.Collectors;
import javax.annotation.CheckForNull;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.treewalk.filter.PathFilter;
import org.eclipse.jgit.treewalk.filter.PathFilterGroup;
import org.eclipse.jgit.treewalk.filter.TreeFilter;
-import org.sonar.api.batch.fs.internal.DefaultInputFile;
import org.sonar.api.batch.scm.BlameCommand;
import org.sonar.api.batch.scm.ScmProvider;
import org.sonar.api.notifications.AnalysisWarnings;
import org.sonar.api.utils.log.Logger;
import org.sonar.api.utils.log.Loggers;
+import static java.lang.String.format;
+import static java.util.function.Function.identity;
+import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toSet;
+import static java.util.stream.Collectors.toUnmodifiableMap;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.ADD;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.MODIFY;
import static org.eclipse.jgit.diff.DiffEntry.ChangeType.RENAME;
private static final Logger LOG = Loggers.get(GitScmProvider.class);
private static final String COULD_NOT_FIND_REF = "Could not find ref '%s' in refs/heads, refs/remotes, refs/remotes/upstream or refs/remotes/origin";
+ private static final String NO_MERGE_BASE_FOUND_MESSAGE = "No merge base found between HEAD and %s";
private final BlameCommand blameCommand;
private final AnalysisWarnings analysisWarnings;
private final GitIgnoreCommand gitIgnoreCommand;
this.analysisWarnings = analysisWarnings;
this.gitIgnoreCommand = gitIgnoreCommand;
this.system2 = system2;
- this.documentationLink = "/documentation/analysis/scm-integration/";
+ this.documentationLink = "/documentation/analysis/scm-integration/";
}
@Override
@CheckForNull
@Override
public Set<Path> branchChangedFiles(String targetBranchName, Path rootBaseDir) {
- try (Repository repo = buildRepo(rootBaseDir)) {
- Ref targetRef = resolveTargetRef(targetBranchName, repo);
- if (targetRef == null) {
- addWarningTargetNotFound(targetBranchName);
- return null;
- }
-
- if (isDiffAlgoInvalid(repo.getConfig())) {
- LOG.warn("The diff algorithm configured in git is not supported. "
- + "No information regarding changes in the branch will be collected, which can lead to unexpected results.");
- return null;
- }
-
- Optional<RevCommit> mergeBaseCommit = findMergeBase(repo, targetRef);
- if (!mergeBaseCommit.isPresent()) {
- LOG.warn("No merge base found between HEAD and " + targetRef.getName());
- return null;
- }
- AbstractTreeIterator mergeBaseTree = prepareTreeParser(repo, mergeBaseCommit.get());
-
- // we compare a commit with HEAD, so no point ignoring line endings (it will be whatever is committed)
- try (Git git = newGit(repo)) {
- List<DiffEntry> diffEntries = git.diff()
- .setShowNameAndStatusOnly(true)
- .setOldTree(mergeBaseTree)
- .setNewTree(prepareNewTree(repo))
- .call();
-
- return diffEntries.stream()
- .filter(diffEntry -> diffEntry.getChangeType() == DiffEntry.ChangeType.ADD || diffEntry.getChangeType() == DiffEntry.ChangeType.MODIFY)
- .map(diffEntry -> repo.getWorkTree().toPath().resolve(diffEntry.getNewPath()))
- .collect(Collectors.toSet());
- }
- } catch (IOException | GitAPIException e) {
- LOG.warn(e.getMessage(), e);
- }
- return null;
+ return Optional.ofNullable((branchChangedFilesWithFileMovementDetection(targetBranchName, rootBaseDir)))
+ .map(GitScmProvider::extractAbsoluteFilePaths)
+ .orElse(null);
}
- // TODO: Adjust ScmProvider abstract
@CheckForNull
- public Collection<ChangedFile> branchModifiedFiles(String targetBranchName, Path rootBaseDir) {
+ public Collection<ChangedFile> branchChangedFilesWithFileMovementDetection(String targetBranchName, Path rootBaseDir) {
try (Repository repo = buildRepo(rootBaseDir)) {
Ref targetRef = resolveTargetRef(targetBranchName, repo);
if (targetRef == null) {
Optional<RevCommit> mergeBaseCommit = findMergeBase(repo, targetRef);
if (mergeBaseCommit.isEmpty()) {
- LOG.warn("No merge base found between HEAD and " + targetRef.getName());
+ LOG.warn(composeNoMergeBaseFoundWarning(targetRef.getName()));
return null;
}
AbstractTreeIterator mergeBaseTree = prepareTreeParser(repo, mergeBaseCommit.get());
Map<String, String> renamedFilePaths = computeRenamedFilePaths(repository, diffEntries);
Set<String> changedFilePaths = computeChangedFilePaths(diffEntries);
- List<ChangedFile> changedFiles = new LinkedList<>();
-
- Consumer<String> collectChangedFiles = filePath -> changedFiles.add(new ChangedFile(filePath, workingDirectory.resolve(filePath), renamedFilePaths.getOrDefault(filePath, null)));
- changedFilePaths.forEach(collectChangedFiles);
+ return collectChangedFiles(workingDirectory, renamedFilePaths, changedFilePaths);
+ }
+ private static List<ChangedFile> collectChangedFiles(Path workingDirectory, Map<String, String> renamedFilePaths, Set<String> changedFilePaths) {
+ List<ChangedFile> changedFiles = new LinkedList<>();
+ changedFilePaths.forEach(filePath -> changedFiles.add(ChangedFile.of(workingDirectory.resolve(filePath), renamedFilePaths.get(filePath))));
return changedFiles;
}
.compute()
.stream()
.filter(entry -> RENAME.equals(entry.getChangeType()))
- .collect(Collectors.toUnmodifiableMap(DiffEntry::getNewPath, DiffEntry::getOldPath));
+ .collect(toUnmodifiableMap(DiffEntry::getNewPath, DiffEntry::getOldPath));
}
private static Set<String> computeChangedFilePaths(List<DiffEntry> diffEntries) {
.stream()
.filter(isAllowedChangeType(ADD, MODIFY))
.map(DiffEntry::getNewPath)
- .collect(Collectors.toSet());
+ .collect(toSet());
}
- private static Predicate<DiffEntry> isAllowedChangeType(ChangeType ...changeTypes) {
+ private static Predicate<DiffEntry> isAllowedChangeType(ChangeType... changeTypes) {
Function<ChangeType, Predicate<DiffEntry>> isChangeType = type -> entry -> type.equals(entry.getChangeType());
return Arrays
@CheckForNull
@Override
public Map<Path, Set<Integer>> branchChangedLines(String targetBranchName, Path projectBaseDir, Set<Path> changedFiles) {
- try (Repository repo = buildRepo(projectBaseDir)) {
- Ref targetRef = resolveTargetRef(targetBranchName, repo);
- if (targetRef == null) {
- addWarningTargetNotFound(targetBranchName);
- return null;
- }
-
- if (isDiffAlgoInvalid(repo.getConfig())) {
- // we already print a warning when branchChangedFiles is called
- return null;
- }
-
- // force ignore different line endings when comparing a commit with the workspace
- repo.getConfig().setBoolean("core", null, "autocrlf", true);
-
- Optional<RevCommit> mergeBaseCommit = findMergeBase(repo, targetRef);
- if (!mergeBaseCommit.isPresent()) {
- LOG.warn("No merge base found between HEAD and " + targetRef.getName());
- return null;
- }
-
- Map<Path, Set<Integer>> changedLines = new HashMap<>();
-
- for (Path path : changedFiles) {
- collectChangedLines(repo, mergeBaseCommit.get(), changedLines, path);
- }
- return changedLines;
- } catch (Exception e) {
- LOG.warn("Failed to get changed lines from git", e);
- }
- return null;
+ return branchChangedLinesWithFileMovementDetection(targetBranchName, projectBaseDir, toChangedFileByPathsMap(changedFiles));
}
- // TODO: Adjust ScmProvider abstract
- public Map<Path, Set<Integer>> branchChangedLines(String targetBranchName, Path projectBaseDir, Map<Path, DefaultInputFile> changedFiles) {
+ @CheckForNull
+ public Map<Path, Set<Integer>> branchChangedLinesWithFileMovementDetection(String targetBranchName, Path projectBaseDir, Map<Path, ChangedFile> changedFiles) {
try (Repository repo = buildRepo(projectBaseDir)) {
Ref targetRef = resolveTargetRef(targetBranchName, repo);
if (targetRef == null) {
Optional<RevCommit> mergeBaseCommit = findMergeBase(repo, targetRef);
if (mergeBaseCommit.isEmpty()) {
- LOG.warn("No merge base found between HEAD and " + targetRef.getName());
+ LOG.warn(composeNoMergeBaseFoundWarning(targetRef.getName()));
return null;
}
Map<Path, Set<Integer>> changedLines = new HashMap<>();
- for (Map.Entry<Path, DefaultInputFile> entry : changedFiles.entrySet()) {
+ for (Map.Entry<Path, ChangedFile> entry : changedFiles.entrySet()) {
collectChangedLines(repo, mergeBaseCommit.get(), changedLines, entry.getKey(), entry.getValue());
}
} catch (Exception e) {
LOG.warn("Failed to get changed lines from git", e);
}
+
return null;
}
+ private static String composeNoMergeBaseFoundWarning(String targetRef) {
+ return format(NO_MERGE_BASE_FOUND_MESSAGE, targetRef);
+ }
+
private void addWarningTargetNotFound(String targetBranchName) {
- analysisWarnings.addUnique(String.format(COULD_NOT_FIND_REF
+ analysisWarnings.addUnique(format(COULD_NOT_FIND_REF
+ ". You may see unexpected issues and changes. "
+ "Please make sure to fetch this ref before pull request analysis and refer to"
+ " <a href=\"%s\" target=\"_blank\">the documentation</a>.", targetBranchName, documentationLink));
}
- private void collectChangedLines(Repository repo, RevCommit mergeBaseCommit, Map<Path, Set<Integer>> changedLines, Path changedFile) {
+ private void collectChangedLines(Repository repo, RevCommit mergeBaseCommit, Map<Path, Set<Integer>> changedLines, Path changedFilePath, ChangedFile changedFile) {
ChangedLinesComputer computer = new ChangedLinesComputer();
try (DiffFormatter diffFmt = new DiffFormatter(new BufferedOutputStream(computer.receiver()))) {
- // copied from DiffCommand so that we can use a custom DiffFormatter which ignores white spaces.
diffFmt.setRepository(repo);
diffFmt.setProgressMonitor(NullProgressMonitor.INSTANCE);
diffFmt.setDiffComparator(RawTextComparator.WS_IGNORE_ALL);
- Path workTree = repo.getWorkTree().toPath();
- String relativePath = workTree.relativize(changedFile).toString();
- PathFilter pathFilter = PathFilter.create(toGitPath(relativePath));
- diffFmt.setPathFilter(pathFilter);
-
- AbstractTreeIterator mergeBaseTree = prepareTreeParser(repo, mergeBaseCommit);
- List<DiffEntry> diffEntries = diffFmt.scan(mergeBaseTree, new FileTreeIterator(repo));
- diffFmt.format(diffEntries);
- diffFmt.flush();
- diffEntries.stream()
- .filter(diffEntry -> diffEntry.getChangeType() == DiffEntry.ChangeType.ADD || diffEntry.getChangeType() == DiffEntry.ChangeType.MODIFY)
- .findAny()
- .ifPresent(diffEntry -> changedLines.put(changedFile, computer.changedLines()));
- } catch (Exception e) {
- LOG.warn("Failed to get changed lines from git for file " + changedFile, e);
- }
- }
-
- // TODO: Adjust ScmProvider abstract
- private void collectChangedLines(Repository repo, RevCommit mergeBaseCommit, Map<Path, Set<Integer>> changedLines, Path changedFilePath, DefaultInputFile changedFileData) {
- ChangedLinesComputer computer = new ChangedLinesComputer();
-
- try (DiffFormatter diffFmt = new DiffFormatter(new BufferedOutputStream(computer.receiver()))) {
- diffFmt.setRepository(repo);
- diffFmt.setProgressMonitor(NullProgressMonitor.INSTANCE);
- diffFmt.setDiffComparator(RawTextComparator.WS_IGNORE_ALL);
-
- diffFmt.setDetectRenames(changedFileData.isMovedFile());
+ diffFmt.setDetectRenames(changedFile.isMovedFile());
Path workTree = repo.getWorkTree().toPath();
- TreeFilter treeFilter = getTreeFilter(changedFileData, workTree);
+ TreeFilter treeFilter = getTreeFilter(changedFile, workTree);
diffFmt.setPathFilter(treeFilter);
AbstractTreeIterator mergeBaseTree = prepareTreeParser(repo, mergeBaseCommit);
return path.replaceAll(Pattern.quote(File.separator), "/");
}
- private TreeFilter getTreeFilter(DefaultInputFile changedFile, Path baseDir) {
- String oldPath = toGitPath(changedFile.oldPath());
- String path = toGitPath(relativizeFilePath(baseDir, changedFile.path()));
+ private static TreeFilter getTreeFilter(ChangedFile changedFile, Path baseDir) {
+ String path = toGitPath(relativizeFilePath(baseDir, changedFile.getAbsolutFilePath()));
+ String oldRelativePath = changedFile.getOldRelativeFilePathReference();
- if (changedFile.isMovedFile()) {
- return PathFilterGroup.createFromStrings(path, oldPath);
+ if (oldRelativePath != null) {
+ return PathFilterGroup.createFromStrings(path, toGitPath(oldRelativePath));
}
return PathFilter.create(path);
}
+ private static Set<Path> extractAbsoluteFilePaths(Collection<ChangedFile> changedFiles) {
+ return changedFiles
+ .stream()
+ .map(ChangedFile::getAbsolutFilePath)
+ .collect(toSet());
+ }
+
@CheckForNull
private Ref resolveTargetRef(String targetBranchName, Repository repo) throws IOException {
String localRef = "refs/heads/" + targetBranchName;
}
}
+ private static Map<Path, ChangedFile> toChangedFileByPathsMap(Set<Path> changedFiles) {
+ return changedFiles
+ .stream()
+ .collect(toMap(identity(), ChangedFile::of, (x, y) -> y, LinkedHashMap::new));
+ }
+
private static String relativizeFilePath(Path baseDirectory, Path filePath) {
return baseDirectory.relativize(filePath).toString();
}
import org.sonar.scanner.scan.branch.BranchConfiguration;
import org.sonar.scanner.scan.filesystem.InputComponentStore;
import org.sonar.scanner.scm.ScmConfiguration;
+import org.sonar.scm.git.GitScmProvider;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assumptions.assumeThat;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.ArgumentMatchers.anySet;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.when;
private static final String TARGET_BRANCH = "target";
private static final Path BASE_DIR = Paths.get("/root");
- private ScmConfiguration scmConfiguration = mock(ScmConfiguration.class);
- private InputModuleHierarchy inputModuleHierarchy = mock(InputModuleHierarchy.class);
- private InputComponentStore inputComponentStore = mock(InputComponentStore.class);
- private BranchConfiguration branchConfiguration = mock(BranchConfiguration.class);
- private ReferenceBranchSupplier referenceBranchSupplier = mock(ReferenceBranchSupplier.class);
+ private final ScmConfiguration scmConfiguration = mock(ScmConfiguration.class);
+ private final InputModuleHierarchy inputModuleHierarchy = mock(InputModuleHierarchy.class);
+ private final InputComponentStore inputComponentStore = mock(InputComponentStore.class);
+ private final BranchConfiguration branchConfiguration = mock(BranchConfiguration.class);
+ private final ReferenceBranchSupplier referenceBranchSupplier = mock(ReferenceBranchSupplier.class);
private ScannerReportWriter writer;
- private ScmProvider provider = mock(ScmProvider.class);
- private DefaultInputProject project = mock(DefaultInputProject.class);
+ private final ScmProvider provider = mock(ScmProvider.class);
+ private final DefaultInputProject project = mock(DefaultInputProject.class);
@Rule
public TemporaryFolder temp = new TemporaryFolder();
@Rule
public LogTester logTester = new LogTester();
- private ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration, referenceBranchSupplier);
+ private final ChangedLinesPublisher publisher = new ChangedLinesPublisher(scmConfiguration, project, inputComponentStore, branchConfiguration, referenceBranchSupplier);
@Before
public void setUp() {
assumeThat(logTester.logs()).contains("File '/root/path2' was detected as changed but without having changed lines");
}
+ @Test
+ public void write_changed_file_with_GitScmProvider() {
+ GitScmProvider provider = mock(GitScmProvider.class);
+ when(scmConfiguration.provider()).thenReturn(provider);
+ Set<Integer> lines = new HashSet<>(Arrays.asList(1, 10));
+ when(provider.branchChangedLines(eq(TARGET_BRANCH), eq(BASE_DIR), anySet()))
+ .thenReturn(ImmutableMap.of(BASE_DIR.resolve("path1"), lines, BASE_DIR.resolve("path3"), Collections.emptySet()));
+
+ publisher.publish(writer);
+
+ verify(provider).branchChangedLinesWithFileMovementDetection(eq(TARGET_BRANCH), eq(BASE_DIR), anyMap());
+ }
+
@Test
public void write_last_line_as_changed_if_all_other_lines_are_changed_and_last_line_is_empty() {
DefaultInputFile fileWithChangedLines = createInputFile("path1", "l1\nl2\nl3\n");
}
@Test
- public void dont_write_last_line_as_changed_if_its_not_empty() {
+ public void do_not_write_last_line_as_changed_if_its_not_empty() {
DefaultInputFile fileWithChangedLines = createInputFile("path1", "l1\nl2\nl3\nl4");
DefaultInputFile fileWithoutChangedLines = createInputFile("path2", "l1\nl2\nl3\nl4");
Set<Path> paths = new HashSet<>(Arrays.asList(BASE_DIR.resolve("path1"), BASE_DIR.resolve("path2")));
import org.sonar.api.batch.bootstrap.ProjectDefinition;
import org.sonar.api.batch.fs.InputFile;
import org.sonar.api.batch.fs.InputFile.Type;
-import org.sonar.api.utils.DateUtils;
-import org.sonar.scanner.ProjectInfo;
import org.sonar.api.batch.fs.internal.DefaultInputFile;
import org.sonar.api.batch.fs.internal.DefaultInputProject;
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
+import org.sonar.api.utils.DateUtils;
+import org.sonar.scanner.ProjectInfo;
import org.sonar.scanner.protocol.output.FileStructure;
import org.sonar.scanner.protocol.output.ScannerReport.Component;
import org.sonar.scanner.protocol.output.ScannerReport.Component.FileStatus;
DefaultInputFile testFile = new TestInputFileBuilder("foo", "module1/test/FooTest.java", 7).setType(Type.TEST).setStatus(InputFile.Status.ADDED).setLines(4).build();
store.put("module1", testFile);
+ DefaultInputFile movedFile = new TestInputFileBuilder("foo", "module1/src/MovedFile.java", "module0/src/MovedFile.java", 9).setStatus(InputFile.Status.CHANGED).setLines(4).build();
+ store.put("module1", movedFile);
+
ComponentsPublisher publisher = new ComponentsPublisher(project, store);
publisher.publish(writer);
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 4)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 6)).isTrue();
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 7)).isTrue();
+ assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 9)).isTrue();
// not marked for publishing
assertThat(writer.hasComponentData(FileStructure.Domain.COMPONENT, 5)).isFalse();
assertThat(rootProtobuf.getDescription()).isEqualTo("Root description");
assertThat(rootProtobuf.getLinkCount()).isZero();
+ Component movedFileProtobuf = reader.readComponent(9);
+ assertThat(movedFileProtobuf.getOldRelativeFilePath()).isEqualTo("module0/src/MovedFile.java");
+
assertThat(reader.readComponent(4).getStatus()).isEqualTo(FileStatus.SAME);
assertThat(reader.readComponent(6).getStatus()).isEqualTo(FileStatus.CHANGED);
assertThat(reader.readComponent(7).getStatus()).isEqualTo(FileStatus.ADDED);
@Test
public void detect_status_branches_confirm() {
Path filePath = Paths.get("module", "src", "Foo.java");
- ScmChangedFiles changedFiles = new ScmChangedFiles(List.of(new ChangedFile(filePath.toString(), filePath)));
+ ScmChangedFiles changedFiles = new ScmChangedFiles(List.of(ChangedFile.of(filePath)));
StatusDetection statusDetection = new StatusDetection(projectRepositories, changedFiles);
assertThat(statusDetection.status("foo", createFile("src/Foo.java"), "XXXXX")).isEqualTo(InputFile.Status.CHANGED);
import org.mockito.MockitoAnnotations;
import org.sonar.api.batch.fs.internal.DefaultInputProject;
import org.sonar.api.batch.scm.ScmProvider;
-import org.sonar.scanner.fs.InputModuleHierarchy;
import org.sonar.scanner.scan.branch.BranchConfiguration;
import org.sonar.scm.git.ChangedFile;
+import org.sonar.scm.git.GitScmProvider;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@Mock
private BranchConfiguration branchConfiguration;
@Mock
- private InputModuleHierarchy inputModuleHierarchy;
- @Mock
private ScmProvider scmProvider;
- private Path rootBaseDir = Paths.get("root");
+ private final Path rootBaseDir = Paths.get("root");
+ private final DefaultInputProject project = mock(DefaultInputProject.class);
+
private ScmChangedFilesProvider provider;
- private DefaultInputProject project = mock(DefaultInputProject.class);
@Before
public void setUp() {
verify(scmConfiguration).provider();
}
+ @Test
+ public void testGitScmProvider(){
+ GitScmProvider gitScmProvider = mock(GitScmProvider.class);
+
+ when(scmConfiguration.provider()).thenReturn(gitScmProvider);
+ when(branchConfiguration.isPullRequest()).thenReturn(true);
+ when(branchConfiguration.targetBranchName()).thenReturn("target");
+
+ ScmChangedFiles scmChangedFiles = provider.provide(scmConfiguration, branchConfiguration, project);
+
+ assertThat(scmChangedFiles.get()).isEmpty();
+ verify(scmConfiguration).provider();
+
+ }
+
@Test
public void testReturnChangedFiles() {
when(branchConfiguration.targetBranchName()).thenReturn("target");
ScmChangedFiles scmChangedFiles = provider.provide(scmConfiguration, branchConfiguration, project);
Path filePath = Paths.get("changedFile").toAbsolutePath();
- ChangedFile changedFile = new ChangedFile(filePath.toString(), filePath);
+ ChangedFile changedFile = ChangedFile.of(filePath);
assertThat(scmChangedFiles.get()).containsOnly(changedFile);
verify(scmProvider).branchChangedFiles("target", rootBaseDir);
}
@Test
public void testGetter() {
Path filePath = Paths.get("files");
- ChangedFile file = new ChangedFile(filePath.toString(), filePath);
+ ChangedFile file = ChangedFile.of(filePath);
Collection<ChangedFile> files = Collections.singletonList(file);
scmChangedFiles = new ScmChangedFiles(files);
assertThat(scmChangedFiles.get()).containsOnly(file);
@Test
public void testConfirm() {
Path filePath = Paths.get("files");
- Collection<ChangedFile> files = Collections.singletonList(new ChangedFile(filePath.toString(), filePath));
+ Collection<ChangedFile> files = Collections.singletonList(ChangedFile.of(filePath));
scmChangedFiles = new ScmChangedFiles(files);
assertThat(scmChangedFiles.isValid()).isTrue();
assertThat(scmChangedFiles.isChanged(Paths.get("files"))).isTrue();
--- /dev/null
+/*
+ * SonarQube
+ * Copyright (C) 2009-2022 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.scm.git;
+
+import java.nio.file.Path;
+import org.junit.Test;
+import org.sonar.api.batch.fs.InputFile;
+import org.sonar.api.batch.fs.internal.DefaultIndexedFile;
+import org.sonar.api.batch.fs.internal.DefaultInputFile;
+import org.sonar.api.batch.fs.internal.SensorStrategy;
+
+import static org.apache.commons.lang.RandomStringUtils.random;
+import static org.apache.commons.lang.RandomStringUtils.randomNumeric;
+import static org.assertj.core.api.Assertions.assertThat;
+
+
+public class ChangedFileTest {
+
+ @Test
+ public void test_unMovedFile(){
+ Path absolutePath = Path.of("/absolutePath");
+ ChangedFile changedFile = ChangedFile.of(absolutePath);
+
+ assertThat(changedFile.getAbsolutFilePath()).isSameAs(absolutePath);
+ assertThat(changedFile.getOldRelativeFilePathReference()).isNull();
+ assertThat(changedFile.isMovedFile()).isFalse();
+ }
+
+ @Test
+ public void test_movedFile(){
+ Path absolutePath = Path.of("/absolutePath");
+ ChangedFile changedFile = ChangedFile.of(absolutePath, "/oldRelativePath");
+
+ assertThat(changedFile.getAbsolutFilePath()).isSameAs(absolutePath);
+ assertThat(changedFile.getOldRelativeFilePathReference()).isEqualTo("/oldRelativePath");
+ assertThat(changedFile.isMovedFile()).isTrue();
+ }
+
+ @Test
+ public void test_equalsAndHashCode(){
+ Path absolutePath = Path.of("/absolutePath");
+ ChangedFile changedFile1 = ChangedFile.of(absolutePath, "/oldRelativePath");
+ ChangedFile changedFile2 = ChangedFile.of(absolutePath, "/oldRelativePath");
+
+ assertThat(changedFile1).isEqualTo(changedFile2).hasSameHashCodeAs(changedFile2);
+ }
+
+ @Test
+ public void test_changed_file_is_created_properly_from_default_input_file() {
+ String oldRelativeReference = "old/relative/path";
+ Path path = Path.of("absolute/path");
+ DefaultInputFile file = composeDefaultInputFile(path, oldRelativeReference);
+ ChangedFile changedFile = ChangedFile.of(file);
+
+ assertThat(changedFile.getAbsolutFilePath()).isEqualTo(path);
+ assertThat(changedFile.isMovedFile()).isTrue();
+ assertThat(changedFile.getOldRelativeFilePathReference()).isEqualTo(oldRelativeReference);
+ }
+
+ private DefaultInputFile composeDefaultInputFile(Path path, String oldRelativeReference) {
+ DefaultIndexedFile indexedFile = composeDefaultIndexFile(path, oldRelativeReference);
+ return new DefaultInputFile(indexedFile, f -> f.setPublished(true));
+ }
+
+ private DefaultIndexedFile composeDefaultIndexFile(Path path, String oldRelativePath) {
+ return new DefaultIndexedFile(
+ path,
+ random(5),
+ random(5),
+ random(5),
+ InputFile.Type.MAIN,
+ random(5),
+ Integer.parseInt(randomNumeric(5)),
+ new SensorStrategy(),
+ oldRelativePath);
+ }
+
+}
import org.sonar.api.utils.log.LogAndArguments;
import org.sonar.api.utils.log.LogTester;
+import static java.lang.String.format;
import static java.util.Collections.emptySet;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.AssertionsForClassTypes.tuple;
import static org.assertj.core.data.MapEntry.entry;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
worktree.resolve("file-m1.xoo"));
}
+ @Test
+ public void branchChangedFilesWithFileMovementDetection_correctly_detects_several_file_moves_in_pull_request_base_branch() throws IOException, GitAPIException {
+ String fileM1 = "file-m1.xoo";
+ String newFileM1 = "new-file-m1.xoo";
+ String fileM2 = "file-m2.xoo";
+ String newFileM2 = "new-file-m2.xoo";
+
+ Path newFileM1AbsolutPath = worktree.resolve(newFileM1);
+ Path newFileM2AbsolutPath = worktree.resolve(newFileM2);
+
+ createAndCommitFile(fileM1);
+ createAndCommitFile(fileM2);
+
+ createBranch();
+
+ editLineOfFile(fileM1, 1);
+ commit(fileM1);
+
+ moveAndCommitFile(fileM1, newFileM1);
+ moveAndCommitFile(fileM2, newFileM2);
+
+ assertThat(newScmProvider().branchChangedFilesWithFileMovementDetection("master", worktree))
+ .extracting(ChangedFile::getAbsolutFilePath, ChangedFile::getOldRelativeFilePathReference)
+ .containsExactlyInAnyOrder(
+ tuple(newFileM1AbsolutPath, fileM1),
+ tuple(newFileM2AbsolutPath, fileM2)
+ );
+ }
+
@Test
public void branchChangedFiles_should_not_fail_with_patience_diff_algo() throws IOException {
Path gitConfig = worktree.resolve(".git").resolve("config");
assertThat(newScmProvider().branchChangedLines("master", worktree, changedFiles))
.containsOnly(
entry(worktree.resolve("lao.txt"), new HashSet<>(Arrays.asList(2, 3, 11, 12, 13))),
- entry(worktree.resolve("file-m1.xoo"), new HashSet<>(Arrays.asList(4))),
+ entry(worktree.resolve("file-m1.xoo"), new HashSet<>(List.of(4))),
entry(worktree.resolve("file-b2.xoo"), new HashSet<>(Arrays.asList(1, 2, 3))));
assertThat(newScmProvider().branchChangedLines("master", worktree, Collections.singleton(worktree.resolve("nonexistent"))))
assertThat(changedLines).containsExactly(entry(filePath, new HashSet<>(Arrays.asList(1, 4))));
}
+ @Test
+ public void branchChangedLinesWithFileMovementDetection_detects_modified_lines_on_renamed_pr_files_compared_to_original_files_on_target_branch() throws GitAPIException, IOException {
+ String fileM1 = "file-m1.xoo";
+ String newFileM1 = "new-file-m1.xoo";
+ String fileM2 = "file-m2.xoo";
+ String newFileM2 = "new-file-m2.xoo";
+
+ Path newFileM1AbsolutPath = worktree.resolve(newFileM1);
+ Path newFileM2AbsolutPath = worktree.resolve(newFileM2);
+
+ createAndCommitFile(fileM1);
+ createAndCommitFile(fileM2);
+
+ createBranch();
+
+ editLineOfFile(fileM1, 2);
+ commit(fileM1);
+
+ editLineOfFile(fileM2, 1);
+ commit(fileM2);
+
+ moveAndCommitFile(fileM1, newFileM1);
+ moveAndCommitFile(fileM2, newFileM2);
+
+ Map<Path, ChangedFile> changedFiles = Map.of(
+ newFileM1AbsolutPath, ChangedFile.of(newFileM1AbsolutPath, fileM1),
+ newFileM2AbsolutPath, ChangedFile.of(newFileM2AbsolutPath, fileM2)
+ );
+
+ Map<Path, Set<Integer>> changedLines = newScmProvider().branchChangedLinesWithFileMovementDetection("master",
+ worktree.resolve("project1"), changedFiles);
+
+ assertThat(changedLines)
+ .containsOnly(
+ entry(newFileM1AbsolutPath, Set.of(2)),
+ entry(newFileM2AbsolutPath, Set.of(1))
+ );
+ }
+
@Test
public void branchChangedFiles_when_git_work_tree_is_above_project_basedir() throws IOException, GitAPIException {
git.branchCreate().setName("b1").call();
git.branchCreate().setName("b1").setStartPoint(forkPoint.getName()).call();
git.checkout().setName("b1").call();
- String newFileContent = new String(Files.readAllBytes(filePath), StandardCharsets.UTF_8).replaceAll("\n", "\r\n");
+ String newFileContent = Files.readString(filePath).replaceAll("\n", "\r\n");
Files.write(filePath, newFileContent.getBytes(StandardCharsets.UTF_8), StandardOpenOption.TRUNCATE_EXISTING);
commit("file-m1.xoo");
}
@Test
- public void branchChangedFiles_should_throw_when_repo_nonexistent() throws IOException {
+ public void branchChangedFiles_should_throw_when_repo_nonexistent() {
assertThatThrownBy(() -> newScmProvider().branchChangedFiles("master", temp.newFolder().toPath()))
.isInstanceOf(MessageException.class)
.hasMessageContaining("Not inside a Git work tree: ");
public void branchChangedLines_omits_files_with_git_api_errors() throws IOException, GitAPIException {
String f1 = "file-in-first-commit.xoo";
String f2 = "file2-in-first-commit.xoo";
+ String f3 = "file3-in-first-commit.xoo";
createAndCommitFile(f2);
+ createAndCommitFile(f3);
git.branchCreate().setName("b1").call();
git.checkout().setName("b1").call();
// both files modified
addLineToFile(f1, 1);
addLineToFile(f2, 2);
+ addLineToFile(f3, 3);
commit(f1);
commit(f2);
+ commit(f3);
AtomicInteger callCount = new AtomicInteger(0);
GitScmProvider provider = new GitScmProvider(mockCommand(), analysisWarnings, gitIgnoreCommand, system2) {
@Override
AbstractTreeIterator prepareTreeParser(Repository repo, RevCommit commit) throws IOException {
- if (callCount.getAndIncrement() == 1) {
+ if (callCount.getAndIncrement() == 2) {
throw new RuntimeException("error");
}
return super.prepareTreeParser(repo, commit);
}
};
- Set<Path> changedFiles = new LinkedHashSet<>();
- changedFiles.add(worktree.resolve(f1));
- changedFiles.add(worktree.resolve(f2));
+
+ Set<Path> changedFiles = new LinkedHashSet<>(List.of(worktree.resolve(f1), worktree.resolve(f2), worktree.resolve(f3)));
assertThat(provider.branchChangedLines("master", worktree, changedFiles))
- .isEqualTo(Collections.singletonMap(worktree.resolve(f1), Collections.singleton(1)));
+ .containsOnly(
+ entry(worktree.resolve(f1), Collections.singleton(1)),
+ entry(worktree.resolve(f2), Collections.singleton(2))
+ );
}
@Test
git.commit().setAuthor(person).setCommitter(person).setMessage(relativePath).call();
}
+ private void createBranch() throws IOException, GitAPIException {
+ ObjectId forkPoint = git.getRepository().exactRef("HEAD").getObjectId();
+ git.branchCreate().setName(BRANCH_NAME).setStartPoint(forkPoint.getName()).call();
+ git.checkout().setName(BRANCH_NAME).call();
+ }
+
+ private void editLineOfFile(String relativePath, int lineNumber) throws IOException {
+ Path filePath = worktree.resolve(relativePath);
+ List<String> lines = Files.readAllLines(filePath);
+ lines.set(lineNumber - 1, randomizedLine(relativePath));
+ Files.write(filePath, lines, StandardOpenOption.TRUNCATE_EXISTING);
+ }
+
+ private void moveAndCommitFile(String filename, String newFilename) throws GitAPIException, IOException {
+ Path file = worktree.resolve(filename);
+ Files.move(file, file.resolveSibling(newFilename));
+ git.add().addFilepattern(filename).setUpdate(true).call();
+ git.add().addFilepattern(newFilename).call();
+ git.commit().setAuthor("joe", "joe@example.com").setMessage(format("Move file from [%s] to [%s]", filename, newFilename)).call();
+ }
+
private GitScmProvider newScmProvider() {
return new GitScmProvider(mockCommand(), analysisWarnings, gitIgnoreCommand, system2);
}
projects / sonarqube.git / commitdiff