Skip to content

Commit 3ece9b2

Browse files
committed
refactor: Make GitDiffer stateless
The `GitDiffer` class doesn't own (in the sense that the actual truth of these members lies in the `Repository` class) any of its members. Furthermore, it just bundles the same members as `Repository`, just leaving those out, that are unneeded. The constructor of `GitDiffer` even extracts these members from `Repository` and doesn't even allow them to be past any other way. Furthermore, most of the interesting functions are already static anyways. A different approach would be to move all of methods into `Repository`. However, there seems to be no reason to override any of the actual diffing methods and the drawback would be a huge `Repository` class which would serve a second purpose.
1 parent b07cf50 commit 3ece9b2

8 files changed

Lines changed: 122 additions & 167 deletions

File tree

src/main/java/org/variantsync/diffdetective/analysis/Analysis.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import org.apache.commons.lang3.function.FailableBiConsumer;
1313
import org.apache.commons.lang3.function.FailableBiFunction;
14+
import org.eclipse.jgit.api.Git;
1415
import org.eclipse.jgit.revwalk.RevCommit;
1516
import org.tinylog.Logger;
1617
import org.variantsync.diffdetective.analysis.AnalysisResult.ResultKey;
@@ -60,7 +61,7 @@ public class Analysis {
6061
protected final List<Hooks> hooks;
6162
protected final Repository repository;
6263

63-
protected GitDiffer differ;
64+
protected Git git;
6465
protected RevCommit currentCommit;
6566
protected CommitDiff currentCommitDiff;
6667
protected PatchDiff currentPatch;
@@ -307,16 +308,14 @@ public static void forEachRepository(
307308
* @param analysis the analysis to run
308309
*/
309310
public static AnalysisResult forSingleCommit(final String commitHash, final Analysis analysis) {
310-
analysis.differ = new GitDiffer(analysis.getRepository());
311-
312311
final Clock clock = new Clock();
313312
// prepare tasks
314313
Logger.info(">>> Running Analysis on single commit {} in {}", commitHash, analysis.getRepository().getRepositoryName());
315314
clock.start();
316315

317316
AnalysisResult result = null;
318317
try {
319-
final RevCommit commit = analysis.differ.getCommit(commitHash);
318+
final RevCommit commit = analysis.getRepository().getCommit(commitHash);
320319
analysis.processCommitBatch(List.of(commit));
321320
result = analysis.getResult();
322321
} catch (Exception e) {
@@ -397,7 +396,6 @@ public static AnalysisResult forEachCommit(
397396
final int nThreads
398397
) {
399398
var analysis = analysisFactory.get();
400-
analysis.differ = new GitDiffer(analysis.getRepository());
401399
analysis.result.append(RuntimeWithMultithreadingResult.KEY, new RuntimeWithMultithreadingResult());
402400

403401
final Clock clock = new Clock();
@@ -409,14 +407,18 @@ public static AnalysisResult forEachCommit(
409407
final Iterator<Callable<AnalysisResult>> tasks = new MappedIterator<>(
410408
/// 1.) Retrieve COMMITS_TO_PROCESS_PER_THREAD commits from the differ and cluster them into one list.
411409
new ClusteredIterator<>(
412-
analysis.differ.yieldRevCommitsAfter(numberOfTotalCommits),
410+
analysis.getRepository().getDiffFilter().filter(
411+
new MappedIterator<>(
412+
analysis.getRepository().getCommits(),
413+
numberOfTotalCommits
414+
)
415+
),
413416
commitsToProcessPerThread
414417
),
415418
/// 2.) Create a MiningTask for the list of commits. This task will then be processed by one
416419
/// particular thread.
417420
commitList -> () -> {
418421
Analysis thisThreadsAnalysis = analysisFactory.get();
419-
thisThreadsAnalysis.differ = analysis.differ;
420422
thisThreadsAnalysis.processCommitBatch(commitList);
421423
return thisThreadsAnalysis.getResult();
422424
}
@@ -519,7 +521,7 @@ protected void processCommitBatch(List<RevCommit> commits) throws Exception {
519521

520522
protected void processCommit() throws Exception {
521523
// parse the commit
522-
final CommitDiffResult commitDiffResult = differ.createCommitDiff(currentCommit);
524+
final CommitDiffResult commitDiffResult = GitDiffer.createCommitDiffFromFirstParent(repository, currentCommit);
523525

524526
// report any errors that occurred and exit in case no VariationDiff could be parsed.
525527
getResult().reportDiffErrors(commitDiffResult.errors());

src/main/java/org/variantsync/diffdetective/datasets/Repository.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
package org.variantsync.diffdetective.datasets;
22

33
import org.eclipse.jgit.api.Git;
4+
import org.eclipse.jgit.api.errors.GitAPIException;
5+
import org.eclipse.jgit.lib.ObjectId;
6+
import org.eclipse.jgit.revwalk.RevCommit;
47
import org.tinylog.Logger;
58
import org.variantsync.diffdetective.diff.git.DiffFilter;
69
import org.variantsync.diffdetective.load.GitLoader;
710
import org.variantsync.diffdetective.util.IO;
811
import org.variantsync.functjonal.Lazy;
912

13+
import java.io.IOException;
1014
import java.net.URI;
1115
import java.nio.file.Path;
16+
import java.util.Iterator;
1217
import java.util.Optional;
1318

1419
/**
@@ -234,6 +239,26 @@ public void preload() {
234239
getGitRepo();
235240
}
236241

242+
/**
243+
* Returns a single commit from the repository.
244+
* Note that this commit may not be part of {@link #getCommits}.
245+
*/
246+
public RevCommit getCommit(String commitHash) throws IOException {
247+
return getGitRepo().getRepository().parseCommit(ObjectId.fromString(commitHash));
248+
}
249+
250+
/**
251+
* Returns all commits in the repository's history.
252+
*/
253+
public Iterator<RevCommit> getCommits() {
254+
try {
255+
return getGitRepo().log().call().iterator();
256+
} catch (GitAPIException e) {
257+
Logger.warn("Could not get log for git repository {}", getRepositoryName());
258+
throw new RuntimeException(e);
259+
}
260+
}
261+
237262
/**
238263
* Loads this repository and returns a jgit representation to access it.
239264
*/

src/main/java/org/variantsync/diffdetective/diff/git/DiffFilter.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
import org.eclipse.jgit.diff.DiffEntry;
55
import org.eclipse.jgit.revwalk.RevCommit;
66
import org.variantsync.diffdetective.variation.diff.Time;
7+
import org.variantsync.functjonal.iteration.Yield;
78

89
import java.util.ArrayList;
910
import java.util.Arrays;
11+
import java.util.Iterator;
1012
import java.util.List;
1113

1214
/**
@@ -330,6 +332,31 @@ public boolean filter(RevCommit commit) {
330332
;
331333
}
332334

335+
/**
336+
* Filters all undesired commits from the given list of commits.
337+
* @param commitsIterator Commits to filter.
338+
* @return All commits from the given set that should not be filtered out.
339+
*/
340+
public Iterator<RevCommit> filter(final Iterator<RevCommit> commitsIterator) {
341+
return new Yield<>(
342+
() -> {
343+
while (commitsIterator.hasNext()) {
344+
final RevCommit c = commitsIterator.next();
345+
// If this commit is filtered, go to the next one.
346+
// filter returns true if we want to include the commit
347+
// so if we do not want to filter it, we do not want to have it. Thus skip.
348+
if (!filter(c)) {
349+
continue;
350+
}
351+
352+
return c;
353+
}
354+
355+
return null;
356+
}
357+
);
358+
}
359+
333360
private boolean isAllowedPath(String filename) {
334361
return allowedPaths.stream().anyMatch(filename::matches);
335362
}

0 commit comments

Comments
 (0)