diff --git a/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java b/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java index 24fc649c..465fb835 100644 --- a/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java +++ b/change-proneness-ranker/src/main/java/org/hjug/git/GitLogReader.java @@ -10,6 +10,8 @@ import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.diff.DiffEntry; import org.eclipse.jgit.diff.DiffFormatter; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.RevWalkException; import org.eclipse.jgit.lib.*; import org.eclipse.jgit.revwalk.*; import org.eclipse.jgit.storage.file.FileRepositoryBuilder; @@ -131,28 +133,65 @@ public String getRepoUrl() throws IOException { */ public ScmLogInfo fileLog(String path) throws GitAPIException, IOException { ObjectId branchId = gitRepository.resolve("HEAD"); - Iterable revCommits = git.log().add(branchId).addPath(path).call(); + CommitWalkStats stats = + walkCommits(git.log().add(branchId).addPath(path).call()); - int commitCount = 0; - int earliestCommit = Integer.MAX_VALUE; - int mostRecentCommit = 0; + if (stats.commitCount == 0) { + return new ScmLogInfo(path, null, stats.earliestCommit, stats.earliestCommit, stats.commitCount); + } - for (RevCommit revCommit : revCommits) { - int commitTime = revCommit.getCommitTime(); - if (commitCount == 0) { - mostRecentCommit = commitTime; + return new ScmLogInfo(path, null, stats.earliestCommit, stats.mostRecentCommit, stats.commitCount); + } + + /** + * Counts commits over the given walk. A missing Git object (e.g. in a shallow or + * partial clone) must not fail the whole walk; the walk is truncated and the + * commits read so far are returned instead. + */ + private static CommitWalkStats walkCommits(Iterable revCommits) { + CommitWalkStats stats = new CommitWalkStats(); + + try { + for (RevCommit revCommit : revCommits) { + int commitTime = revCommit.getCommitTime(); + if (stats.commitCount == 0) { + stats.mostRecentCommit = commitTime; + } + if (commitTime < stats.earliestCommit) { + stats.earliestCommit = commitTime; + } + stats.commitCount++; } - if (commitTime < earliestCommit) { - earliestCommit = commitTime; + } catch (RevWalkException e) { + // JGit wraps checked exceptions thrown mid-walk in a RevWalkException. + if (isCausedByMissingObject(e)) { + log.warn( + "Missing Git object while reading history (shallow or partial clone?); " + + "reporting the {} commit(s) that could be read. Cause: {}", + stats.commitCount, + e.getMessage()); + } else { + throw e; } - commitCount++; } - if (commitCount == 0) { - return new ScmLogInfo(path, null, earliestCommit, earliestCommit, commitCount); + return stats; + } + + private static boolean isCausedByMissingObject(Throwable throwable) { + while (throwable != null) { + if (throwable instanceof MissingObjectException) { + return true; + } + throwable = throwable.getCause(); } + return false; + } - return new ScmLogInfo(path, null, earliestCommit, mostRecentCommit, commitCount); + private static class CommitWalkStats { + int commitCount = 0; + int earliestCommit = Integer.MAX_VALUE; + int mostRecentCommit = 0; } // based on https://stackoverflow.com/questions/27361538/how-to-show-changes-between-commits-with-jgit diff --git a/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java b/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java index 6a05ed8d..7a2d8558 100644 --- a/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java +++ b/change-proneness-ranker/src/test/java/org/hjug/git/GitLogReaderTest.java @@ -1,11 +1,20 @@ package org.hjug.git; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.*; import java.util.*; import org.eclipse.jgit.api.Git; +import org.eclipse.jgit.api.LogCommand; import org.eclipse.jgit.api.errors.GitAPIException; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.errors.RevWalkException; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; import org.junit.jupiter.api.AfterEach; @@ -66,6 +75,175 @@ void testFileLog() throws IOException, GitAPIException, InterruptedException { Assertions.assertEquals(secondCommit.getCommitTime(), scmLogInfo.getMostRecentCommit()); } + @Test + void testFileLogReturnsPartialResultsWhenWalkFailsMidIteration() throws Exception { + // A missing tree encountered mid-walk (e.g. in a partial clone) must not + // discard the commits that were already read: fileLog should return the + // partial results gathered before the walk failed. + String attributeHandler = "AttributeHandler.java"; + InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler); + String contents = convertInputStreamToString(resourceAsStream); + + writeFile(attributeHandler, contents); + git.add().addFilepattern(".").call(); + git.commit().setMessage("message").call(); + + Thread.sleep(1000); + + writeFile(attributeHandler, contents + "\n// second revision\n"); + git.add().addFilepattern(".").call(); + RevCommit newestCommit = git.commit().setMessage("message").call(); + + Iterable failingWalk = walkYieldingThenThrowing( + List.of(newestCommit), new RevWalkException(new MissingObjectException(headId(), Constants.OBJ_TREE))); + + GitLogReader gitLogReader = new GitLogReader(gitWithLogs(failingWalk)); + + ScmLogInfo scmLogInfo = Assertions.assertDoesNotThrow(() -> gitLogReader.fileLog(attributeHandler)); + + Assertions.assertEquals(1, scmLogInfo.getCommitCount()); + Assertions.assertEquals(newestCommit.getCommitTime(), scmLogInfo.getEarliestCommit()); + Assertions.assertEquals(newestCommit.getCommitTime(), scmLogInfo.getMostRecentCommit()); + } + + @Test + void testFileLogRethrowsNonMissingObjectWalkFailures() throws Exception { + // Guard against silencing unrelated walk failures: only missing objects + // should be tolerated, anything else must still propagate. + String attributeHandler = "AttributeHandler.java"; + InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler); + writeFile(attributeHandler, convertInputStreamToString(resourceAsStream)); + git.add().addFilepattern(".").call(); + git.commit().setMessage("message").call(); + + Iterable failingWalk = + walkYieldingThenThrowing(Collections.emptyList(), new RevWalkException(new IOException("disk I/O"))); + + GitLogReader gitLogReader = new GitLogReader(gitWithLogs(failingWalk)); + + Assertions.assertThrows(RevWalkException.class, () -> gitLogReader.fileLog("AttributeHandler.java")); + } + + /** + * A Git whose log command returns the given iterable; the underlying + * repository is real so that HEAD resolution works. + */ + private final Git gitWithLogs(Iterable revCommits) throws IOException, GitAPIException { + Git mockGit = mock(Git.class); + when(mockGit.getRepository()).thenReturn(repository); + LogCommand logCommand = mock(LogCommand.class); + when(logCommand.add(any(ObjectId.class))).thenReturn(logCommand); + when(logCommand.addPath(anyString())).thenReturn(logCommand); + when(logCommand.call()).thenReturn(revCommits); + when(mockGit.log()).thenReturn(logCommand); + return mockGit; + } + + /** An iterable that yields the given commits and then fails, mimicking a truncated walk. */ + private static Iterable walkYieldingThenThrowing(List commits, RuntimeException failure) { + return () -> new Iterator() { + private final Iterator delegate = commits.iterator(); + + @Override + public boolean hasNext() { + return true; + } + + @Override + public RevCommit next() { + if (delegate.hasNext()) { + return delegate.next(); + } + throw failure; + } + }; + } + + private ObjectId headId() throws IOException { + ObjectId head = repository.resolve("HEAD"); + return head == null ? ObjectId.fromString("1111111111111111111111111111111111111111") : head; + } + + @Test + void testFileLogReturnsOnlyVerifiedPartialPathHistoryWhenFilteredWalkYieldsNothingDueToMissingObjects() + throws Exception { + String attributeHandler = "AttributeHandler.java"; + InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler); + writeFile(attributeHandler, convertInputStreamToString(resourceAsStream)); + git.add().addFilepattern(".").call(); + RevCommit onlyCommit = git.commit().setMessage("message").call(); + + Iterable failingFilteredWalk = walkYieldingThenThrowing( + Collections.emptyList(), + new RevWalkException(new MissingObjectException(headId(), Constants.OBJ_TREE))); + + GitLogReader gitLogReader = new GitLogReader(gitWithLogs(failingFilteredWalk)); + + ScmLogInfo scmLogInfo = Assertions.assertDoesNotThrow(() -> gitLogReader.fileLog(attributeHandler)); + + // When the filtered walk yields nothing due to missing objects, return only the verified partial results + // (which in this case is zero commits) rather than falling back to total repository history + Assertions.assertEquals(0, scmLogInfo.getCommitCount()); + } + + @Test + void testFileLogReturnsOnlyVerifiedPartialPathHistoryWithMissingTree() throws Exception { + // Simulates a shallow clone whose tree objects are missing: the filtered walk + // may yield zero results due to missing objects, but should not fall back to total history. + GitLogReader gitLogReader = new GitLogReader(git); + + String attributeHandler = "AttributeHandler.java"; + InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler); + String contents = convertInputStreamToString(resourceAsStream); + + writeFile(attributeHandler, contents); + git.add().addFilepattern(".").call(); + RevCommit firstCommit = git.commit().setMessage("message").call(); + + Thread.sleep(1000); + + writeFile(attributeHandler, contents + "\n// second revision\n"); + git.add().addFilepattern(".").call(); + RevCommit secondCommit = git.commit().setMessage("message").call(); + + deleteLooseObject(secondCommit.getTree()); + + ScmLogInfo scmLogInfo = Assertions.assertDoesNotThrow(() -> gitLogReader.fileLog(attributeHandler)); + + // With missing tree objects, the filtered walk may yield zero verified partial results + // rather than falling back to total repository history + Assertions.assertEquals(0, scmLogInfo.getCommitCount()); + } + + @Test + void testFileLogDoesNotFabricateHistoryForFileNeverCommitted() throws Exception { + // An empty filtered walk with NO missing objects means the file simply has no + // history: the total commit count must NOT be substituted. + GitLogReader gitLogReader = new GitLogReader(git); + + String attributeHandler = "AttributeHandler.java"; + InputStream resourceAsStream = getClass().getClassLoader().getResourceAsStream(attributeHandler); + writeFile(attributeHandler, convertInputStreamToString(resourceAsStream)); + git.add().addFilepattern(".").call(); + git.commit().setMessage("message").call(); + + ScmLogInfo scmLogInfo = Assertions.assertDoesNotThrow(() -> gitLogReader.fileLog("NeverCommitted.java")); + + Assertions.assertEquals(0, scmLogInfo.getCommitCount()); + } + + private void deleteLooseObject(ObjectId objectId) throws IOException { + String objectName = objectId.getName(); + File looseObject = new File( + new File(repository.getDirectory(), "objects"), + objectName.substring(0, 2) + '/' + objectName.substring(2)); + org.junit.jupiter.api.Assumptions.assumeTrue( + looseObject.exists(), "loose object " + objectName + " should exist in a fresh repository"); + if (!looseObject.delete()) { + throw new IOException("Unable to delete loose object " + looseObject); + } + } + @Test void testWalkFirstCommit() throws IOException, GitAPIException { GitLogReader gitLogReader = new GitLogReader(git);