diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Annotation.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Annotation.java index 32fe05daf3b..226c0d9f85a 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Annotation.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/Annotation.java @@ -129,6 +129,10 @@ public final Set getRevisions() { return annotationData.getRevisions(); } + final Set getDisplayRevisions() { + return annotationData.getDisplayRevisions(); + } + @TestOnly Set getAuthors() { return annotationData.getAuthors(); @@ -138,8 +142,7 @@ Set getAuthors() { * Gets the author who last modified the specified line. * * @param line line number (counting from 1) - * @return author, or an empty string if there is no information about the - * specified line + * @return author, or an empty string if there is no information about the specified line */ public String getAuthor(int line) { return annotationData.getAuthor(line); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AnnotationData.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AnnotationData.java index 8c5dbf71c68..5a206936296 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AnnotationData.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/AnnotationData.java @@ -52,6 +52,10 @@ public AnnotationData(String filename) { this.filename = filename; } + /** + * The transient does not matter here since the object is serialized explicitly + * in {@link FileAnnotationCache#store(File, Annotation)}. + */ private transient List annotationLines = new ArrayList<>(); private int widestRevision; private int widestAuthor; @@ -224,6 +228,14 @@ public Set getRevisions() { return ret; } + Set getDisplayRevisions() { + Set ret = new HashSet<>(); + for (AnnotationLine ln : annotationLines) { + ret.add(ln.getDisplayRevision()); + } + return ret; + } + @TestOnly Set getAuthors() { return annotationLines.stream().map(AnnotationLine::getAuthor).collect(Collectors.toSet()); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java index 4bd650becd3..038d099c62f 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryGuru.java @@ -49,6 +49,7 @@ import org.apache.lucene.document.Document; import org.apache.lucene.queryparser.classic.ParseException; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.VisibleForTesting; import org.opengrok.indexer.analysis.AnalyzerFactory; @@ -352,32 +353,35 @@ public Annotation annotate(File file, @Nullable String rev, boolean fallback) th } private void completeAnnotationWithHistory(File file, Annotation annotation, Repository repo) { - History history = null; try { - history = getHistory(file); + History history = getHistory(file); + if (history != null) { + completeAnnotationWithHistory(annotation, history, repo); + } } catch (HistoryException ex) { LOGGER.log(Level.WARNING, "Cannot get messages for tooltip: ", ex); } + } - if (history != null) { - Set revs = annotation.getRevisions(); - int revsMatched = 0; - // !!! cannot do this because of not matching rev ids (keys) - // first is the most recent one, so we need the position of "rev" - // until the end of the list - //if (hent.indexOf(rev)>0) { - // hent = hent.subList(hent.indexOf(rev), hent.size()); - //} - for (HistoryEntry he : history.getHistoryEntries()) { - String hist_rev = he.getRevision(); - String short_rev = repo.getRevisionForAnnotate(hist_rev); - if (revs.contains(short_rev)) { - annotation.addDesc(short_rev, he.getDescription()); - // History entries are coming from recent to older, - // file version should be from oldest to newer. - annotation.addFileVersion(short_rev, revs.size() - revsMatched); - revsMatched++; - } + @VisibleForTesting + static void completeAnnotationWithHistory(Annotation annotation, @NotNull History history, Repository repo) { + Set revs = annotation.getRevisions(); + int revsMatched = 0; + // !!! cannot do this because of not matching rev ids (keys) + // first is the most recent one, so we need the position of "rev" + // until the end of the list + //if (hent.indexOf(rev)>0) { + // hent = hent.subList(hent.indexOf(rev), hent.size()); + //} + for (HistoryEntry historyEntry : history.getHistoryEntries()) { + String historyEntryRevision = historyEntry.getRevision(); + String revisionForAnnotate = repo.getRevisionForAnnotate(historyEntryRevision); + if (revs.contains(revisionForAnnotate)) { + annotation.addDesc(revisionForAnnotate, historyEntry.getDescription()); + // History entries are coming from recent to older, + // file version should be from oldest to newer. + annotation.addFileVersion(revisionForAnnotate, revs.size() - revsMatched); + revsMatched++; } } } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialAnnotationParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialAnnotationParser.java index 34405a7d2ac..012f7ef19c9 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialAnnotationParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialAnnotationParser.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved. */ package org.opengrok.indexer.history; @@ -28,10 +28,13 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.util.HashMap; +import java.util.Optional; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; import java.util.regex.Pattern; + +import org.jetbrains.annotations.NotNull; import org.opengrok.indexer.logger.LoggerFactory; import org.opengrok.indexer.util.Executor; import org.opengrok.indexer.web.Util; @@ -49,10 +52,12 @@ class MercurialAnnotationParser implements Executor.StreamHandler { /** * Pattern used to extract author/revision from the {@code hg annotate} command. + * Obviously, this has to be in concordance with the output template used by + * {@link MercurialRepository#annotate(File, String)}. */ - private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^\\s*(\\d+):"); + private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^(\\d+)\\t([0-9a-f]+):"); - MercurialAnnotationParser(File file, HashMap revs) { + MercurialAnnotationParser(File file, @NotNull HashMap revs) { this.file = file; this.revs = revs; } @@ -69,13 +74,12 @@ public void processStream(InputStream input) throws IOException { ++lineno; matcher.reset(line); if (matcher.find()) { - String rev = matcher.group(1); - String author = "N/A"; + String displayRev = matcher.group(1); + String fullRev = displayRev + ":" + matcher.group(2); // Use the history index hash map to get the author. - if (revs.get(rev) != null) { - author = revs.get(rev).getAuthor(); - } - annotation.addLine(rev, Util.getEmail(author.trim()), true); + String author = Optional.ofNullable(revs.get(fullRev)).map(HistoryEntry::getAuthor). + orElse("N/A"); + annotation.addLine(fullRev, Util.getEmail(author.trim()), true, displayRev); } else { LOGGER.log(Level.WARNING, "Error: did not find annotation in line {0} for ''{1}'': [{2}]", diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java index cbedf781c7f..d6baee4d954 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/MercurialRepository.java @@ -322,8 +322,7 @@ private HistoryRevResult getHistoryRev(BufferSink sink, String fullpath, String * of a file in historical revision. * * @param fullpath file path - * @param fullRevToFind revision number (in the form of - * {rev}:{node|short}) + * @param fullRevToFind revision number (in the form of {rev}:{node|short}) * @return original filename */ private String findOriginalName(String fullpath, String fullRevToFind) throws IOException { @@ -464,7 +463,11 @@ public Annotation annotate(File file, String revision) throws IOException { ensureCommand(CMD_PROPERTY_KEY, CMD_FALLBACK); argv.add(RepoCommand); argv.add("annotate"); - argv.add("-n"); + argv.add("--template"); + /* + * This has to be in concordance with TEMPLATE_REVS, in particular the format of the 'node'. + */ + argv.add("{lines % '{rev}\t{node|short}:{line}'}"); if (!this.isHandleRenamedFiles()) { argv.add("--no-follow"); } @@ -486,20 +489,12 @@ public Annotation annotate(File file, String revision) throws IOException { try { History hist = HistoryGuru.getInstance().getHistory(file, false); if (Objects.isNull(hist)) { - LOGGER.log(Level.SEVERE, - "Error: cannot get history for file ''{0}''", file); + LOGGER.log(Level.SEVERE, "Error: cannot get history for file ''{0}''", file); return null; } - for (HistoryEntry e : hist.getHistoryEntries()) { - // Chop out the colon and all hexadecimal what follows. - // This is because the whole changeset identification is - // stored in history index while annotate only needs the - // revision identifier. - revs.put(e.getRevision().replaceFirst(":[a-f0-9]+", ""), e); - } + hist.getHistoryEntries().forEach(rev -> revs.put(rev.getRevision(), rev)); } catch (HistoryException he) { - LOGGER.log(Level.SEVERE, - "Error: cannot get history for file ''{0}''", file); + LOGGER.log(Level.SEVERE, "Error: cannot get history for file ''{0}''", file); return null; } @@ -509,13 +504,6 @@ public Annotation annotate(File file, String revision) throws IOException { return annotator.getAnnotation(); } - @Override - protected String getRevisionForAnnotate(String historyRevision) { - String[] brev = historyRevision.split(":"); - - return brev[0]; - } - @Override public boolean fileHasAnnotation(File file) { return true; diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java index 26318d75193..4cb1661c709 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java @@ -717,41 +717,41 @@ public static void readableLine(int num, Writer out, Annotation annotation, Stri private static void writeAnnotation(int num, Writer out, Annotation annotation, String userPageLink, String userPageSuffix, String project) throws IOException { - String r = annotation.getRevision(num); + String revision = annotation.getRevision(num); boolean enabled = annotation.isEnabled(num); out.write(""); if (enabled) { out.write(ANCHOR_CLASS_START); out.write("r title-tooltip"); out.write("\" style=\"background-color: "); - out.write(annotation.getColors().getOrDefault(r, "inherit")); + out.write(annotation.getColors().getOrDefault(revision, "inherit")); out.write("\" href=\""); out.write(uriEncode(annotation.getFilename())); out.write("?"); out.write(QueryParameters.ANNOTATION_PARAM_EQ_TRUE); out.write(AMP); out.write(QueryParameters.REVISION_PARAM_EQ); - out.write(uriEncode(r)); - String msg = annotation.getDesc(r); + out.write(uriEncode(revision)); + String msg = annotation.getDesc(revision); out.write("\" title=\""); if (msg != null) { out.write(Util.encode(msg)); } - if (annotation.getFileVersion(r) != 0) { - out.write("<br/>version: " + annotation.getFileVersion(r) + "/" + if (annotation.getFileVersion(revision) != 0) { + out.write("<br/>version: " + annotation.getFileVersion(revision) + "/" + annotation.getRevisions().size()); } out.write(CLOSE_QUOTED_TAG); } StringBuilder buf = new StringBuilder(); - final boolean most_recent_revision = annotation.getFileVersion(r) == annotation.getRevisions().size(); + final boolean isMostRecentRevision = annotation.getFileVersion(revision) == annotation.getRevisions().size(); // print an asterisk for the most recent revision - if (most_recent_revision) { + if (isMostRecentRevision) { buf.append(""); buf.append('*'); } htmlize(annotation.getRevisionForDisplay(num), buf); - if (most_recent_revision) { + if (isMostRecentRevision) { buf.append(SPAN_END); // recent revision span } out.write(buf.toString()); @@ -773,7 +773,7 @@ private static void writeAnnotation(int num, Writer out, Annotation annotation, out.write(AMP); out.write(QueryParameters.HIST_SEARCH_PARAM_EQ); out.write(QUOTE); - out.write(uriEncode(r)); + out.write(uriEncode(revision)); out.write(""&"); out.write(QueryParameters.TYPE_SEARCH_PARAM_EQ); out.write("\" title=\"Search history for this revision"); diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java index 40c4ad02fc5..1c8c1e47b8f 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/MercurialRepositoryTest.java @@ -23,7 +23,7 @@ */ package org.opengrok.indexer.history; -import org.apache.commons.lang3.tuple.Pair; +import org.apache.commons.lang3.tuple.Triple; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -37,6 +37,7 @@ import org.opengrok.indexer.util.Executor; import org.opengrok.indexer.util.IOUtils; import org.opengrok.indexer.util.TestRepository; +import org.opengrok.indexer.web.Util; import java.io.File; import java.io.IOException; @@ -435,14 +436,14 @@ void testAnnotationNegative() throws Exception { assertNull(annotation); } - private static Stream>> provideParametersForPositiveAnnotationTest() { - return Stream.of(Pair.of("8:6a8c423f5624", List.of("7", "8")), - Pair.of("7:db1394c05268", List.of("7"))); + private static Stream, List>> provideParametersForPositiveAnnotationTest() { + return Stream.of(Triple.of("8:6a8c423f5624", List.of("7", "8"), List.of("8:6a8c423f5624", "7:db1394c05268")), + Triple.of("7:db1394c05268", List.of("7"), List.of("7:db1394c05268"))); } @ParameterizedTest @MethodSource("provideParametersForPositiveAnnotationTest") - void testAnnotationPositive(Pair> pair) throws Exception { + void testAnnotationPositive(Triple, List> triple) throws Exception { MercurialRepository hgRepo = (MercurialRepository) RepositoryFactory.getRepository(repositoryRoot); assertNotNull(hgRepo); File file = new File(repositoryRoot, "novel.txt"); @@ -450,10 +451,25 @@ void testAnnotationPositive(Pair> pair) throws Exception { // The annotate() method calls uses HistoryGuru's getHistory() method which requires the RepositoryLookup // to be initialized. Do so via setRepositories(). RuntimeEnvironment.getInstance().setRepositories(repository.getSourceRoot()); - Annotation annotation = hgRepo.annotate(file, pair.getKey()); + Annotation annotation = hgRepo.annotate(file, triple.getLeft()); assertNotNull(annotation); + List displayRevisions = new ArrayList<>(annotation.getDisplayRevisions()); + assertEquals(triple.getMiddle(), displayRevisions); List revisions = new ArrayList<>(annotation.getRevisions()); - assertEquals(pair.getValue(), revisions); + assertEquals(triple.getRight(), revisions); + History history = HistoryGuru.getInstance().getHistory(file); + assertNotNull(history); + assertFalse(history.getHistoryEntries().isEmpty()); + HistoryGuru.completeAnnotationWithHistory(annotation, history, hgRepo); + List relevantEntries = history.getHistoryEntries().stream(). + filter(e -> annotation.getRevisions().contains(e.getRevision())). + collect(Collectors.toList()); + assertFalse(relevantEntries.isEmpty()); + for (HistoryEntry entry : relevantEntries) { + assertTrue(annotation.getRevisions().contains(entry.getRevision())); + assertEquals(entry.getDescription(), annotation.getDesc(entry.getRevision())); + assertTrue(annotation.getAuthors().contains(Util.getEmail(entry.getAuthor()))); + } } /**