Skip to content

Commit 2a5d034

Browse files
vladakvladak
authored
use longer revision for Mercurial repository annotation search links (#4767)
fixes #4761
1 parent ac9afaf commit 2a5d034

File tree

7 files changed

+97
-70
lines changed

7 files changed

+97
-70
lines changed

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/Annotation.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ public final Set<String> getRevisions() {
129129
return annotationData.getRevisions();
130130
}
131131

132+
final Set<String> getDisplayRevisions() {
133+
return annotationData.getDisplayRevisions();
134+
}
135+
132136
@TestOnly
133137
Set<String> getAuthors() {
134138
return annotationData.getAuthors();
@@ -138,8 +142,7 @@ Set<String> getAuthors() {
138142
* Gets the author who last modified the specified line.
139143
*
140144
* @param line line number (counting from 1)
141-
* @return author, or an empty string if there is no information about the
142-
* specified line
145+
* @return author, or an empty string if there is no information about the specified line
143146
*/
144147
public String getAuthor(int line) {
145148
return annotationData.getAuthor(line);

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/AnnotationData.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ public AnnotationData(String filename) {
5252
this.filename = filename;
5353
}
5454

55+
/**
56+
* The <code>transient</code> does not matter here since the object is serialized explicitly
57+
* in {@link FileAnnotationCache#store(File, Annotation)}.
58+
*/
5559
private transient List<AnnotationLine> annotationLines = new ArrayList<>();
5660
private int widestRevision;
5761
private int widestAuthor;
@@ -224,6 +228,14 @@ public Set<String> getRevisions() {
224228
return ret;
225229
}
226230

231+
Set<String> getDisplayRevisions() {
232+
Set<String> ret = new HashSet<>();
233+
for (AnnotationLine ln : annotationLines) {
234+
ret.add(ln.getDisplayRevision());
235+
}
236+
return ret;
237+
}
238+
227239
@TestOnly
228240
Set<String> getAuthors() {
229241
return annotationLines.stream().map(AnnotationLine::getAuthor).collect(Collectors.toSet());

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949

5050
import org.apache.lucene.document.Document;
5151
import org.apache.lucene.queryparser.classic.ParseException;
52+
import org.jetbrains.annotations.NotNull;
5253
import org.jetbrains.annotations.Nullable;
5354
import org.jetbrains.annotations.VisibleForTesting;
5455
import org.opengrok.indexer.analysis.AnalyzerFactory;
@@ -352,32 +353,35 @@ public Annotation annotate(File file, @Nullable String rev, boolean fallback) th
352353
}
353354

354355
private void completeAnnotationWithHistory(File file, Annotation annotation, Repository repo) {
355-
History history = null;
356356
try {
357-
history = getHistory(file);
357+
History history = getHistory(file);
358+
if (history != null) {
359+
completeAnnotationWithHistory(annotation, history, repo);
360+
}
358361
} catch (HistoryException ex) {
359362
LOGGER.log(Level.WARNING, "Cannot get messages for tooltip: ", ex);
360363
}
364+
}
361365

362-
if (history != null) {
363-
Set<String> revs = annotation.getRevisions();
364-
int revsMatched = 0;
365-
// !!! cannot do this because of not matching rev ids (keys)
366-
// first is the most recent one, so we need the position of "rev"
367-
// until the end of the list
368-
//if (hent.indexOf(rev)>0) {
369-
// hent = hent.subList(hent.indexOf(rev), hent.size());
370-
//}
371-
for (HistoryEntry he : history.getHistoryEntries()) {
372-
String hist_rev = he.getRevision();
373-
String short_rev = repo.getRevisionForAnnotate(hist_rev);
374-
if (revs.contains(short_rev)) {
375-
annotation.addDesc(short_rev, he.getDescription());
376-
// History entries are coming from recent to older,
377-
// file version should be from oldest to newer.
378-
annotation.addFileVersion(short_rev, revs.size() - revsMatched);
379-
revsMatched++;
380-
}
366+
@VisibleForTesting
367+
static void completeAnnotationWithHistory(Annotation annotation, @NotNull History history, Repository repo) {
368+
Set<String> revs = annotation.getRevisions();
369+
int revsMatched = 0;
370+
// !!! cannot do this because of not matching rev ids (keys)
371+
// first is the most recent one, so we need the position of "rev"
372+
// until the end of the list
373+
//if (hent.indexOf(rev)>0) {
374+
// hent = hent.subList(hent.indexOf(rev), hent.size());
375+
//}
376+
for (HistoryEntry historyEntry : history.getHistoryEntries()) {
377+
String historyEntryRevision = historyEntry.getRevision();
378+
String revisionForAnnotate = repo.getRevisionForAnnotate(historyEntryRevision);
379+
if (revs.contains(revisionForAnnotate)) {
380+
annotation.addDesc(revisionForAnnotate, historyEntry.getDescription());
381+
// History entries are coming from recent to older,
382+
// file version should be from oldest to newer.
383+
annotation.addFileVersion(revisionForAnnotate, revs.size() - revsMatched);
384+
revsMatched++;
381385
}
382386
}
383387
}

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialAnnotationParser.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
2222
*/
2323
package org.opengrok.indexer.history;
2424

@@ -28,10 +28,13 @@
2828
import java.io.InputStream;
2929
import java.io.InputStreamReader;
3030
import java.util.HashMap;
31+
import java.util.Optional;
3132
import java.util.logging.Level;
3233
import java.util.logging.Logger;
3334
import java.util.regex.Matcher;
3435
import java.util.regex.Pattern;
36+
37+
import org.jetbrains.annotations.NotNull;
3538
import org.opengrok.indexer.logger.LoggerFactory;
3639
import org.opengrok.indexer.util.Executor;
3740
import org.opengrok.indexer.web.Util;
@@ -49,10 +52,12 @@ class MercurialAnnotationParser implements Executor.StreamHandler {
4952

5053
/**
5154
* Pattern used to extract author/revision from the {@code hg annotate} command.
55+
* Obviously, this has to be in concordance with the output template used by
56+
* {@link MercurialRepository#annotate(File, String)}.
5257
*/
53-
private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^\\s*(\\d+):");
58+
private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^(\\d+)\\t([0-9a-f]+):");
5459

55-
MercurialAnnotationParser(File file, HashMap<String, HistoryEntry> revs) {
60+
MercurialAnnotationParser(File file, @NotNull HashMap<String, HistoryEntry> revs) {
5661
this.file = file;
5762
this.revs = revs;
5863
}
@@ -69,13 +74,12 @@ public void processStream(InputStream input) throws IOException {
6974
++lineno;
7075
matcher.reset(line);
7176
if (matcher.find()) {
72-
String rev = matcher.group(1);
73-
String author = "N/A";
77+
String displayRev = matcher.group(1);
78+
String fullRev = displayRev + ":" + matcher.group(2);
7479
// Use the history index hash map to get the author.
75-
if (revs.get(rev) != null) {
76-
author = revs.get(rev).getAuthor();
77-
}
78-
annotation.addLine(rev, Util.getEmail(author.trim()), true);
80+
String author = Optional.ofNullable(revs.get(fullRev)).map(HistoryEntry::getAuthor).
81+
orElse("N/A");
82+
annotation.addLine(fullRev, Util.getEmail(author.trim()), true, displayRev);
7983
} else {
8084
LOGGER.log(Level.WARNING,
8185
"Error: did not find annotation in line {0} for ''{1}'': [{2}]",

‎opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java

Lines changed: 9 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,7 @@ private HistoryRevResult getHistoryRev(BufferSink sink, String fullpath, String
322322
* of a file in historical revision.
323323
*
324324
* @param fullpath file path
325-
* @param fullRevToFind revision number (in the form of
326-
* {rev}:{node|short})
325+
* @param fullRevToFind revision number (in the form of <code>{rev}:{node|short}</code>)
327326
* @return original filename
328327
*/
329328
private String findOriginalName(String fullpath, String fullRevToFind) throws IOException {
@@ -464,7 +463,11 @@ public Annotation annotate(File file, String revision) throws IOException {
464463
ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK);
465464
argv.add(RepoCommand);
466465
argv.add("annotate");
467-
argv.add("-n");
466+
argv.add("--template");
467+
/*
468+
* This has to be in concordance with TEMPLATE_REVS, in particular the format of the 'node'.
469+
*/
470+
argv.add("{lines % '{rev}\t{node|short}:{line}'}");
468471
if (!this.isHandleRenamedFiles()) {
469472
argv.add("--no-follow");
470473
}
@@ -486,20 +489,12 @@ public Annotation annotate(File file, String revision) throws IOException {
486489
try {
487490
History hist = HistoryGuru.getInstance().getHistory(file, false);
488491
if (Objects.isNull(hist)) {
489-
LOGGER.log(Level.SEVERE,
490-
"Error: cannot get history for file ''{0}''", file);
492+
LOGGER.log(Level.SEVERE, "Error: cannot get history for file ''{0}''", file);
491493
return null;
492494
}
493-
for (HistoryEntry e : hist.getHistoryEntries()) {
494-
// Chop out the colon and all hexadecimal what follows.
495-
// This is because the whole changeset identification is
496-
// stored in history index while annotate only needs the
497-
// revision identifier.
498-
revs.put(e.getRevision().replaceFirst(":[a-f0-9]+", ""), e);
499-
}
495+
hist.getHistoryEntries().forEach(rev -> revs.put(rev.getRevision(), rev));
500496
} catch (HistoryException he) {
501-
LOGGER.log(Level.SEVERE,
502-
"Error: cannot get history for file ''{0}''", file);
497+
LOGGER.log(Level.SEVERE, "Error: cannot get history for file ''{0}''", file);
503498
return null;
504499
}
505500

@@ -509,13 +504,6 @@ public Annotation annotate(File file, String revision) throws IOException {
509504
return annotator.getAnnotation();
510505
}
511506

512-
@Override
513-
protected String getRevisionForAnnotate(String historyRevision) {
514-
String[] brev = historyRevision.split(":");
515-
516-
return brev[0];
517-
}
518-
519507
@Override
520508
public boolean fileHasAnnotation(File file) {
521509
return true;

‎opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -717,41 +717,41 @@ public static void readableLine(int num, Writer out, Annotation annotation, Stri
717717

718718
private static void writeAnnotation(int num, Writer out, Annotation annotation, String userPageLink,
719719
String userPageSuffix, String project) throws IOException {
720-
String r = annotation.getRevision(num);
720+
String revision = annotation.getRevision(num);
721721
boolean enabled = annotation.isEnabled(num);
722722
out.write("<span class=\"blame\">");
723723
if (enabled) {
724724
out.write(ANCHOR_CLASS_START);
725725
out.write("r title-tooltip");
726726
out.write("\" style=\"background-color: ");
727-
out.write(annotation.getColors().getOrDefault(r, "inherit"));
727+
out.write(annotation.getColors().getOrDefault(revision, "inherit"));
728728
out.write("\" href=\"");
729729
out.write(uriEncode(annotation.getFilename()));
730730
out.write("?");
731731
out.write(QueryParameters.ANNOTATION_PARAM_EQ_TRUE);
732732
out.write(AMP);
733733
out.write(QueryParameters.REVISION_PARAM_EQ);
734-
out.write(uriEncode(r));
735-
String msg = annotation.getDesc(r);
734+
out.write(uriEncode(revision));
735+
String msg = annotation.getDesc(revision);
736736
out.write("\" title=\"");
737737
if (msg != null) {
738738
out.write(Util.encode(msg));
739739
}
740-
if (annotation.getFileVersion(r) != 0) {
741-
out.write("&lt;br/&gt;version: " + annotation.getFileVersion(r) + "/"
740+
if (annotation.getFileVersion(revision) != 0) {
741+
out.write("&lt;br/&gt;version: " + annotation.getFileVersion(revision) + "/"
742742
+ annotation.getRevisions().size());
743743
}
744744
out.write(CLOSE_QUOTED_TAG);
745745
}
746746
StringBuilder buf = new StringBuilder();
747-
final boolean most_recent_revision = annotation.getFileVersion(r) == annotation.getRevisions().size();
747+
final boolean isMostRecentRevision = annotation.getFileVersion(revision) == annotation.getRevisions().size();
748748
// print an asterisk for the most recent revision
749-
if (most_recent_revision) {
749+
if (isMostRecentRevision) {
750750
buf.append("<span class=\"most_recent_revision\">");
751751
buf.append('*');
752752
}
753753
htmlize(annotation.getRevisionForDisplay(num), buf);
754-
if (most_recent_revision) {
754+
if (isMostRecentRevision) {
755755
buf.append(SPAN_END); // recent revision span
756756
}
757757
out.write(buf.toString());
@@ -773,7 +773,7 @@ private static void writeAnnotation(int num, Writer out, Annotation annotation,
773773
out.write(AMP);
774774
out.write(QueryParameters.HIST_SEARCH_PARAM_EQ);
775775
out.write(QUOTE);
776-
out.write(uriEncode(r));
776+
out.write(uriEncode(revision));
777777
out.write("&quot;&amp;");
778778
out.write(QueryParameters.TYPE_SEARCH_PARAM_EQ);
779779
out.write("\" title=\"Search history for this revision");

‎opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
*/
2424
package org.opengrok.indexer.history;
2525

26-
import org.apache.commons.lang3.tuple.Pair;
26+
import org.apache.commons.lang3.tuple.Triple;
2727
import org.junit.jupiter.api.AfterEach;
2828
import org.junit.jupiter.api.BeforeAll;
2929
import org.junit.jupiter.api.BeforeEach;
@@ -37,6 +37,7 @@
3737
import org.opengrok.indexer.util.Executor;
3838
import org.opengrok.indexer.util.IOUtils;
3939
import org.opengrok.indexer.util.TestRepository;
40+
import org.opengrok.indexer.web.Util;
4041

4142
import java.io.File;
4243
import java.io.IOException;
@@ -435,25 +436,40 @@ void testAnnotationNegative() throws Exception {
435436
assertNull(annotation);
436437
}
437438

438-
private static Stream<Pair<String, List<String>>> provideParametersForPositiveAnnotationTest() {
439-
return Stream.of(Pair.of("8:6a8c423f5624", List.of("7", "8")),
440-
Pair.of("7:db1394c05268", List.of("7")));
439+
private static Stream<Triple<String, List<String>, List<String>>> provideParametersForPositiveAnnotationTest() {
440+
return Stream.of(Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")),
441+
Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268")));
441442
}
442443

443444
@ParameterizedTest
444445
@MethodSource("provideParametersForPositiveAnnotationTest")
445-
void testAnnotationPositive(Pair<String, List<String>> pair) throws Exception {
446+
void testAnnotationPositive(Triple<String, List<String>, List<String>> triple) throws Exception {
446447
MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot);
447448
assertNotNull(hgRepo);
448449
File file = new File(repositoryRoot, "novel.txt");
449450
assertTrue(file.exists());
450451
// The annotate() method calls uses HistoryGuru's getHistory() method which requires the RepositoryLookup
451452
// to be initialized. Do so via setRepositories().
452453
RuntimeEnvironment.getInstance().setRepositories(repository.getSourceRoot());
453-
Annotation annotation = hgRepo.annotate(file, pair.getKey());
454+
Annotation annotation = hgRepo.annotate(file, triple.getLeft());
454455
assertNotNull(annotation);
456+
List<String> displayRevisions = new ArrayList<>(annotation.getDisplayRevisions());
457+
assertEquals(triple.getMiddle(), displayRevisions);
455458
List<String> revisions = new ArrayList<>(annotation.getRevisions());
456-
assertEquals(pair.getValue(), revisions);
459+
assertEquals(triple.getRight(), revisions);
460+
History history = HistoryGuru.getInstance().getHistory(file);
461+
assertNotNull(history);
462+
assertFalse(history.getHistoryEntries().isEmpty());
463+
HistoryGuru.completeAnnotationWithHistory(annotation, history, hgRepo);
464+
List<HistoryEntry> relevantEntries = history.getHistoryEntries().stream().
465+
filter(e -> annotation.getRevisions().contains(e.getRevision())).
466+
collect(Collectors.toList());
467+
assertFalse(relevantEntries.isEmpty());
468+
for (HistoryEntry entry : relevantEntries) {
469+
assertTrue(annotation.getRevisions().contains(entry.getRevision()));
470+
assertEquals(entry.getDescription(), annotation.getDesc(entry.getRevision()));
471+
assertTrue(annotation.getAuthors().contains(Util.getEmail(entry.getAuthor())));
472+
}
457473
}
458474

459475
/**

0 commit comments

Comments
 (0)