diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java index 8684ecc631d..0c7ace80c66 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/History.java @@ -24,6 +24,7 @@ package org.opengrok.indexer.history; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import java.io.Serializable; import java.util.ArrayList; @@ -65,7 +66,8 @@ public History() { this(new ArrayList<>()); } - History(List entries) { + @VisibleForTesting + public History(List entries) { this(entries, Collections.emptyList()); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/HistoryContext.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/HistoryContext.java index 3dfe178b200..b119152d80e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/HistoryContext.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/search/context/HistoryContext.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2017, 2020, Chris Fraire . */ package org.opengrok.indexer.search.context; @@ -34,6 +34,8 @@ import java.util.logging.Logger; import org.apache.lucene.search.Query; +import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.VisibleForTesting; import org.opengrok.indexer.history.History; import org.opengrok.indexer.history.HistoryEntry; import org.opengrok.indexer.history.HistoryException; @@ -119,24 +121,67 @@ public boolean getContext(File src, String path, Writer out, String context) thr return getHistoryContext(hist, path, out, null, context); } + private int matchLine(String line, String urlPrefix, String path, @Nullable Writer out, @Nullable List hits, + String rev, String nrev) throws IOException { + + int matchedLines = 0; + tokens.reInit(line); + String token; + int matchState; + long start = -1; + while ((token = tokens.next()) != null) { + for (LineMatcher lineMatcher : m) { + matchState = lineMatcher.match(token); + if (matchState == LineMatcher.MATCHED) { + if (start < 0) { + start = tokens.getMatchStart(); + } + long end = tokens.getMatchEnd(); + if (start > Integer.MAX_VALUE || end > Integer.MAX_VALUE) { + LOGGER.log(Level.WARNING, "Unexpected out of bounds for {0}", path); + } else if (hits != null) { + StringBuilder sb = new StringBuilder(); + writeMatch(sb, line, (int) start, (int) end, + true, path, urlPrefix, nrev, rev); + hits.add(new Hit(path, sb.toString(), "", false, false)); + } else { + writeMatch(out, line, (int) start, (int) end, + false, path, urlPrefix, nrev, rev); + } + matchedLines++; + break; + } else if (matchState == LineMatcher.WAIT) { + if (start < 0) { + start = tokens.getMatchStart(); + } + } else { + start = -1; + } + } + } + + return matchedLines; + } + /** - * Writes matching History log entries from 'in' to 'out' or to 'hits'. - * @param in the history to fetch entries from + * Writes matching history log entries to either 'out' or to 'hits'. + * @param history the history to fetch entries from * @param out to write matched context * @param path path to the file - * @param hits list of hits - * @param wcontext web context - beginning of url + * @param hits list of {@link Hit} instances + * @param urlPrefix URL prefix + * @return whether there was at least one line that matched */ - private boolean getHistoryContext( - History in, String path, Writer out, List hits, String wcontext) { - if (in == null) { + @VisibleForTesting + boolean getHistoryContext(History history, String path, @Nullable Writer out, @Nullable List hits, + String urlPrefix) { + if (history == null) { throw new IllegalArgumentException("`in' is null"); } if ((out == null) == (hits == null)) { // There should be exactly one destination for the output. If // none or both are specified, it's a bug. - throw new IllegalArgumentException( - "Exactly one of out and hits should be non-null"); + throw new IllegalArgumentException("Exactly one of out and hits should be non-null"); } if (m == null) { @@ -144,7 +189,7 @@ private boolean getHistoryContext( } int matchedLines = 0; - Iterator it = in.getHistoryEntries().iterator(); + Iterator it = history.getHistoryEntries().stream().filter(HistoryEntry::isActive).iterator(); try { HistoryEntry he; HistoryEntry nhe = null; @@ -153,10 +198,11 @@ private boolean getHistoryContext( if (nhe == null) { he = it.next(); } else { - he = nhe; //nhe is the lookahead revision + he = nhe; // nhe is the lookahead revision } String line = he.getLine(); String rev = he.getRevision(); + if (it.hasNext()) { nhe = it.next(); } else { @@ -169,40 +215,7 @@ private boolean getHistoryContext( } else { nrev = nhe.getRevision(); } - tokens.reInit(line); - String token; - int matchState; - long start = -1; - while ((token = tokens.next()) != null) { - for (LineMatcher lineMatcher : m) { - matchState = lineMatcher.match(token); - if (matchState == LineMatcher.MATCHED) { - if (start < 0) { - start = tokens.getMatchStart(); - } - long end = tokens.getMatchEnd(); - if (start > Integer.MAX_VALUE || end > Integer.MAX_VALUE) { - LOGGER.log(Level.INFO, "Unexpected out of bounds for {0}", path); - } else if (out == null) { - StringBuilder sb = new StringBuilder(); - writeMatch(sb, line, (int) start, (int) end, - true, path, wcontext, nrev, rev); - hits.add(new Hit(path, sb.toString(), "", false, false)); - } else { - writeMatch(out, line, (int) start, (int) end, - false, path, wcontext, nrev, rev); - } - matchedLines++; - break; - } else if (matchState == LineMatcher.WAIT) { - if (start < 0) { - start = tokens.getMatchStart(); - } - } else { - start = -1; - } - } - } + matchedLines += matchLine(line, urlPrefix, path, out, hits, rev, nrev); } } catch (Exception e) { LOGGER.log(Level.WARNING, "Could not get history context for " + path, e); @@ -219,24 +232,23 @@ private boolean getHistoryContext( * @param end position of the first char after the match * @param flatten should multi-line log entries be flattened to a single * @param path path to the file - * @param wcontext web context (begin of URL) + * @param urlPrefix URL prefix * @param nrev old revision * @param rev current revision - * line? If {@code true}, replace newline with space. * @throws IOException IO exception */ protected static void writeMatch(Appendable out, String line, int start, int end, boolean flatten, String path, - String wcontext, String nrev, String rev) + String urlPrefix, String nrev, String rev) throws IOException { String prefix = line.substring(0, start); String match = line.substring(start, end); String suffix = line.substring(end); - if (wcontext != null && nrev != null && !wcontext.isEmpty()) { + if (urlPrefix != null && nrev != null && !urlPrefix.isEmpty()) { out.append(". */ package org.opengrok.indexer.search.context; +import java.io.File; import java.io.IOException; import java.io.StringWriter; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import java.util.Set; + import org.apache.lucene.index.Term; import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; @@ -37,11 +42,14 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.opengrok.indexer.condition.EnabledForRepository; +import org.opengrok.indexer.history.History; +import org.opengrok.indexer.history.HistoryEntry; import org.opengrok.indexer.search.Hit; import org.opengrok.indexer.util.TestRepository; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.opengrok.indexer.condition.RepositoryInstalled.Type.MERCURIAL; @@ -190,4 +198,37 @@ void testWriteMatch() throws IOException { "r1=/foo%20bar/haf%2Bhaf@1\" title=\"diff to previous version\">diff foo", sb.toString()); } + + /** + * Test that inactive history entries are skipped when generating history context. + */ + @Test + void testGetHistoryContextVsInactiveHistoryEntries() { + Set filePaths = Set.of(File.separator + Paths.get("teamware", "foo.c")); + History history = new History(List.of( + new HistoryEntry("1.2", "1.2", new Date(1485438707000L), + "Totoro", + "Uaaah\n", true, filePaths), + new HistoryEntry("1.2", "1.2", new Date(1485438706000L), + "Catbus", + "Miau\n", false, filePaths), + new HistoryEntry("1.1", "1.1", new Date(1485438705000L), + "Totoro", + "Hmmm\n", true, filePaths) + )); + + ArrayList hits = new ArrayList<>(); + BooleanQuery.Builder query = new BooleanQuery.Builder(); + HistoryEntry lastHistoryEntry = history.getLastHistoryEntry(); + assertNotNull(lastHistoryEntry); + query.add(new TermQuery(new Term("hist", lastHistoryEntry.getMessage().trim())), Occur.MUST); + HistoryContext historyContext = new HistoryContext(query.build()); + final String path = "/foo/bar"; + final String prefix = "prefix"; + assertTrue(historyContext.getHistoryContext(history, path, null, hits, prefix)); + assertEquals(1, hits.size()); + assertTrue(hits.get(0).getLine(). + startsWith(String.format("