Skip to content

Commit dcc15bb

Browse files
uhafneruhafner
authored
Merge pull request #33 from jenkinsci/JENKINS-59252-handle-line-0
[JENKINS-59252] Use last committer of the affected file of warning lineNumber == 0.
2 parents 4a922f0 + 942bb19 commit dcc15bb

File tree

3 files changed

+122
-38
lines changed

3 files changed

+122
-38
lines changed

‎src/main/java/io/jenkins/plugins/git/forensics/blame/GitBlamer.java‎

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
import java.io.IOException;
44
import java.nio.file.LinkOption;
55
import java.nio.file.Paths;
6+
import java.util.Optional;
7+
import java.util.stream.StreamSupport;
68

79
import org.eclipse.jgit.api.BlameCommand;
10+
import org.eclipse.jgit.api.Git;
811
import org.eclipse.jgit.api.errors.GitAPIException;
912
import org.eclipse.jgit.api.errors.JGitInternalException;
1013
import org.eclipse.jgit.blame.BlameResult;
@@ -107,7 +110,11 @@ public Blames blame(final FileLocations locations) {
107110

108111
private String getWorkspacePath() {
109112
try {
110-
return Paths.get(workspace.getRemote()).toAbsolutePath().normalize().toRealPath(LinkOption.NOFOLLOW_LINKS).toString();
113+
return Paths.get(workspace.getRemote())
114+
.toAbsolutePath()
115+
.normalize()
116+
.toRealPath(LinkOption.NOFOLLOW_LINKS)
117+
.toString();
111118
}
112119
catch (IOException | InvalidPathException exception) {
113120
return workspace.getRemote();
@@ -119,6 +126,7 @@ private String getWorkspacePath() {
119126
*/
120127
static class BlameCallback implements RepositoryCallback<Blames> {
121128
private static final long serialVersionUID = 8794666938104738260L;
129+
private static final int WHOLE_FILE = 0;
122130

123131
private final ObjectId headCommit;
124132
private final String workspacePath;
@@ -139,7 +147,7 @@ public Blames invoke(final Repository repository, final VirtualChannel channel)
139147
BlameRunner blameRunner = new BlameRunner(repository, headCommit);
140148

141149
for (String file : locations.getRelativePaths()) {
142-
run(file, blameRunner);
150+
run(file, blameRunner, new LastCommitRunner(repository));
143151

144152
if (Thread.interrupted()) { // Cancel request by user
145153
String message = "Blaming has been interrupted while computing blame information";
@@ -164,10 +172,12 @@ public Blames invoke(final Repository repository, final VirtualChannel channel)
164172
* @param fileName
165173
* the file to get the blames for
166174
* @param blameRunner
167-
* the runner to invoke Git
175+
* the runner to invoke Git blame
176+
* @param lastCommitRunner
177+
* the runner to find the last commit
168178
*/
169179
@VisibleForTesting
170-
void run(final String fileName, final BlameRunner blameRunner) {
180+
void run(final String fileName, final BlameRunner blameRunner, final LastCommitRunner lastCommitRunner) {
171181
try {
172182
BlameResult blame = blameRunner.run(fileName);
173183
if (blame == null) {
@@ -176,27 +186,11 @@ void run(final String fileName, final BlameRunner blameRunner) {
176186
else {
177187
for (int line : locations.getLines(fileName)) {
178188
FileBlame fileBlame = new FileBlame(workspacePath + "/" + fileName);
179-
int lineIndex = line - 1; // first line is index 0
180-
if (lineIndex < blame.getResultContents().size()) {
181-
PersonIdent who = blame.getSourceAuthor(lineIndex);
182-
if (who == null) {
183-
who = blame.getSourceCommitter(lineIndex);
184-
}
185-
if (who == null) {
186-
blames.logError("- no author or committer information found for line %d in file %s",
187-
lineIndex, fileName);
188-
}
189-
else {
190-
fileBlame.setName(line, who.getName());
191-
fileBlame.setEmail(line, who.getEmailAddress());
192-
}
193-
RevCommit commit = blame.getSourceCommit(lineIndex);
194-
if (commit == null) {
195-
blames.logError("- no commit ID found for line %d in file %s", lineIndex, fileName);
196-
}
197-
else {
198-
fileBlame.setCommit(line, commit.getName());
199-
}
189+
if (line <= 0) {
190+
fillWithLastCommit(fileName, fileBlame, lastCommitRunner);
191+
}
192+
else if (line <= blame.getResultContents().size()) {
193+
fillWithBlameResult(fileName, fileBlame, blame, line);
200194
}
201195
blames.add(fileBlame);
202196
}
@@ -208,6 +202,47 @@ void run(final String fileName, final BlameRunner blameRunner) {
208202
}
209203
blames.logSummary();
210204
}
205+
206+
private void fillWithBlameResult(final String fileName, final FileBlame fileBlame, final BlameResult blame,
207+
final int line) {
208+
int lineIndex = line - 1; // first line is index 0
209+
PersonIdent who = blame.getSourceAuthor(lineIndex);
210+
if (who == null) {
211+
who = blame.getSourceCommitter(lineIndex);
212+
}
213+
if (who == null) {
214+
blames.logError("- no author or committer information found for line %d in file %s",
215+
lineIndex, fileName);
216+
}
217+
else {
218+
fileBlame.setName(line, who.getName());
219+
fileBlame.setEmail(line, who.getEmailAddress());
220+
}
221+
RevCommit commit = blame.getSourceCommit(lineIndex);
222+
if (commit == null) {
223+
blames.logError("- no commit ID found for line %d in file %s", lineIndex, fileName);
224+
}
225+
else {
226+
fileBlame.setCommit(line, commit.getName());
227+
}
228+
}
229+
230+
private void fillWithLastCommit(final String fileName, final FileBlame fileBlame,
231+
final LastCommitRunner lastCommitRunner) throws GitAPIException {
232+
Optional<RevCommit> commit = lastCommitRunner.run(fileName);
233+
if (commit.isPresent()) {
234+
RevCommit revCommit = commit.get();
235+
fileBlame.setCommit(WHOLE_FILE, revCommit.getName());
236+
PersonIdent who = revCommit.getAuthorIdent();
237+
if (who == null) {
238+
who = revCommit.getCommitterIdent();
239+
}
240+
if (who != null) {
241+
fileBlame.setName(WHOLE_FILE, who.getName());
242+
fileBlame.setEmail(WHOLE_FILE, who.getEmailAddress());
243+
}
244+
}
245+
}
211246
}
212247

213248
/**
@@ -230,4 +265,23 @@ BlameResult run(final String fileName) throws GitAPIException {
230265
return blame.call();
231266
}
232267
}
268+
269+
/**
270+
* Executes the Git log command for a given file.
271+
*/
272+
static class LastCommitRunner {
273+
private final Repository repo;
274+
275+
LastCommitRunner(final Repository repo) {
276+
this.repo = repo;
277+
}
278+
279+
Optional<RevCommit> run(final String fileName) throws GitAPIException {
280+
try (Git git = new Git(repo)) {
281+
Iterable<RevCommit> commits = git.log().addPath(fileName).call();
282+
283+
return StreamSupport.stream(commits.spliterator(), false).findFirst();
284+
}
285+
}
286+
}
233287
}

‎src/test/java/io/jenkins/plugins/git/forensics/blame/GitBlamerITest.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import org.junit.ClassRule;
77
import org.junit.Test;
8+
import org.jvnet.hudson.test.Issue;
89
import org.jvnet.hudson.test.JenkinsRule;
910

1011
import org.jenkinsci.plugins.gitclient.GitClient;
@@ -79,6 +80,33 @@ public void shouldCreateBlamesIfRequestIsExistingFile() {
7980
assertThatBlameIsEmpty(request, 6);
8081
}
8182

83+
/**
84+
* Verifies that the last committer of the whole file is used if no specific line number is given.
85+
*/
86+
@Test @Issue("JENKINS-59252")
87+
public void shouldAssignLastCommitterIfNoLineNumberIsGiven() {
88+
create2RevisionsWithDifferentAuthors();
89+
90+
FileLocations locations = new FileLocations(sampleRepo.getRoot());
91+
String absolutePath = locations.getWorkspace() + FILE_NAME;
92+
locations.addLine(absolutePath, 0);
93+
94+
GitBlamer gitBlamer = createBlamer();
95+
96+
Blames blames = gitBlamer.blame(locations);
97+
98+
assertThat(blames.getFiles()).isNotEmpty().containsExactly(absolutePath);
99+
assertThat(blames.getErrorMessages()).isEmpty();
100+
assertThat(blames.getInfoMessages()).contains("-> blamed authors of issues in 1 files");
101+
102+
FileBlame request = blames.getBlame(absolutePath);
103+
assertThat(request).hasFileName(absolutePath);
104+
105+
assertThat(request.getName(0)).isEqualTo(BAR_NAME);
106+
assertThat(request.getEmail(0)).isEqualTo(BAR_EMAIL);
107+
assertThat(request.getCommit(0)).isEqualTo(getHead());
108+
}
109+
82110
private GitBlamer createBlamer() {
83111
try {
84112
GitSCM scm = new GitSCM(

‎src/test/java/io/jenkins/plugins/git/forensics/blame/GitBlamerTest.java‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import io.jenkins.plugins.forensics.blame.FileLocations.FileSystem;
2626
import io.jenkins.plugins.git.forensics.blame.GitBlamer.BlameCallback;
2727
import io.jenkins.plugins.git.forensics.blame.GitBlamer.BlameRunner;
28+
import io.jenkins.plugins.git.forensics.blame.GitBlamer.LastCommitRunner;
2829

2930
import static io.jenkins.plugins.forensics.assertions.Assertions.*;
3031
import static org.mockito.Mockito.*;
@@ -120,7 +121,7 @@ private void verifyExceptionHandling(final Class<? extends Exception> exception)
120121

121122
BlameRunner runner = mock(BlameRunner.class);
122123
when(runner.run("exception")).thenThrow(exception);
123-
callback.run("exception", runner);
124+
callback.run("exception", runner, createLastCommitRunner());
124125

125126
assertThat(blames.getErrorMessages()).hasSize(3);
126127
assertThat(blames.getErrorMessages().get(1)).startsWith(
@@ -140,12 +141,15 @@ void shouldMapResultToRequestWithOneLine() throws GitAPIException {
140141

141142
stubResultForIndex(result, 0);
142143

143-
BlameRunner blameRunner = createBlameRunner(result);
144-
callback.run(FILE, blameRunner);
144+
callback.run(FILE, createBlameRunner(result), createLastCommitRunner());
145145

146146
verifyResult(blames.getBlame(ABSOLUTE_PATH), 1);
147147
}
148148

149+
private LastCommitRunner createLastCommitRunner() {
150+
return mock(LastCommitRunner.class);
151+
}
152+
149153
@Test
150154
void shouldMapResultToRequestWithTwoLinesOfAbsolutePaths() throws GitAPIException {
151155
FileLocations locations = createLocations();
@@ -160,8 +164,7 @@ void shouldMapResultToRequestWithTwoLinesOfAbsolutePaths() throws GitAPIExceptio
160164
stubResultForIndex(result, 0);
161165
stubResultForIndex(result, 1);
162166

163-
BlameRunner blameRunner = createBlameRunner(result);
164-
callback.run(FILE, blameRunner);
167+
callback.run(FILE, createBlameRunner(result), createLastCommitRunner());
165168

166169
assertThat(blames.contains(ABSOLUTE_PATH)).isTrue();
167170
assertThat(blames.contains(FILE)).isFalse();
@@ -199,7 +202,9 @@ void shouldMapResultToRequestOutOfRange() throws GitAPIException {
199202
stubResultForIndex(result, 0);
200203

201204
BlameRunner blameRunner = createBlameRunner(result);
202-
callback.run(FILE, blameRunner);
205+
LastCommitRunner lastCommitRunner = createLastCommitRunner();
206+
207+
callback.run(FILE, blameRunner, lastCommitRunner);
203208

204209
FileBlame blame = blames.getBlame(ABSOLUTE_PATH);
205210
verifyResult(blame, 1);
@@ -208,7 +213,7 @@ void shouldMapResultToRequestOutOfRange() throws GitAPIException {
208213
assertThat(blame.getName(3)).isEqualTo(EMPTY);
209214
assertThat(blame.getCommit(3)).isEqualTo(EMPTY);
210215

211-
callback.run("otherFile", blameRunner);
216+
callback.run("otherFile", blameRunner, lastCommitRunner);
212217
assertThat(blames.getErrorMessages()).contains("- no blame results for file <otherFile>");
213218
}
214219

@@ -223,8 +228,7 @@ void shouldIgnoreMissingCommit() throws GitAPIException {
223228
BlameResult result = createResult(1);
224229
when(result.getSourceAuthor(0)).thenReturn(new PersonIdent(NAME, EMAIL));
225230

226-
BlameRunner blameRunner = createBlameRunner(result);
227-
callback.run(FILE, blameRunner);
231+
callback.run(FILE, createBlameRunner(result), createLastCommitRunner());
228232

229233
FileBlame blame = blames.getBlame(ABSOLUTE_PATH);
230234
assertThat(blame.getEmail(1)).isEqualTo(EMAIL);
@@ -244,8 +248,7 @@ void shouldIgnoreMissingAuthorAndCommitter() throws GitAPIException {
244248
RevCommit commit = mock(RevCommit.class);
245249
when(result.getSourceCommit(0)).thenReturn(commit);
246250

247-
BlameRunner blameRunner = createBlameRunner(result);
248-
callback.run(FILE, blameRunner);
251+
callback.run(FILE, createBlameRunner(result), createLastCommitRunner());
249252

250253
FileBlame blame = blames.getBlame(ABSOLUTE_PATH);
251254
assertThat(blame.getEmail(1)).isEqualTo(EMPTY);
@@ -266,8 +269,7 @@ void shouldUseCommitterIfAuthorIsMissing() throws GitAPIException {
266269
when(result.getSourceCommit(0)).thenReturn(commit);
267270
when(result.getSourceCommitter(0)).thenReturn(new PersonIdent(NAME + 1, EMAIL + 1));
268271

269-
BlameRunner blameRunner = createBlameRunner(result);
270-
callback.run(FILE, blameRunner);
272+
callback.run(FILE, createBlameRunner(result), createLastCommitRunner());
271273

272274
FileBlame blame = blames.getBlame(ABSOLUTE_PATH);
273275
verifyResult(blame, 1);

0 commit comments

Comments
 (0)