From 78a8812da2b0c50c3cb00400ffe5ba01bd91015a Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Thu, 9 Nov 2023 17:15:48 +0100 Subject: [PATCH] avoid annotations for binary files (#4476) fixes #4473 --- .../indexer/analysis/AnalyzerGuru.java | 17 +++++++++-------- .../opengrok/indexer/history/HistoryGuru.java | 19 ++++++++++++++++--- .../history/MercurialAnnotationParser.java | 12 ++++++------ .../SCCSRepositoryAnnotationParser.java | 14 ++++++++------ .../java/org/opengrok/web/PageConfig.java | 3 +++ 5 files changed, 42 insertions(+), 23 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java index 39cba9a0dc2..895a2c3742c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/analysis/AnalyzerGuru.java @@ -43,6 +43,7 @@ import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.SortedMap; import java.util.TreeMap; import java.util.TreeSet; @@ -723,8 +724,9 @@ public static void writeDumpedXref(String contextPath, * Get the genre of a file. * * @param file The file to inspect - * @return The genre suitable to decide how to display the file + * @return The genre suitable to decide how to display the file or {@code null} if not found */ + @Nullable public static AbstractAnalyzer.Genre getGenre(String file) { return getGenre(find(file)); } @@ -733,9 +735,10 @@ public static AbstractAnalyzer.Genre getGenre(String file) { * Get the genre of a bulk of data. * * @param in A stream containing the data - * @return The genre suitable to decide how to display the file + * @return The genre suitable to decide how to display the file or {@code null} if not found * @throws java.io.IOException If an error occurs while getting the content */ + @Nullable public static AbstractAnalyzer.Genre getGenre(InputStream in) throws IOException { return getGenre(find(in)); } @@ -743,14 +746,12 @@ public static AbstractAnalyzer.Genre getGenre(InputStream in) throws IOException /** * Get the genre for a named class (this is most likely an analyzer). * - * @param factory the analyzer factory to get the genre for - * @return The genre of this class (null if not found) + * @param factory the analyzer factory to get the genre for or {@code null} if not found + * @return The genre of this class (or {@code null} if not found) */ + @Nullable public static AbstractAnalyzer.Genre getGenre(AnalyzerFactory factory) { - if (factory != null) { - return factory.getGenre(); - } - return null; + return Optional.ofNullable(factory).map(AnalyzerFactory::getGenre).orElse(null); } /** 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 738fdbf9378..1f0c6f5830b 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,8 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.VisibleForTesting; +import org.opengrok.indexer.analysis.AbstractAnalyzer; +import org.opengrok.indexer.analysis.AnalyzerGuru; import org.opengrok.indexer.configuration.CommandTimeoutType; import org.opengrok.indexer.configuration.Configuration; import org.opengrok.indexer.configuration.Configuration.RemoteSCM; @@ -258,11 +260,12 @@ private Annotation getAnnotation(File file, @Nullable String rev, boolean fallba /** * Annotate given file using repository method. Makes sure that the resulting annotation has the revision set. - * @param file file object to generate the annotaiton for + * @param file file object to generate the annotation for * @param rev revision to get the annotation for or {@code null} for latest revision of given file - * @return annotation object + * @return annotation object or {@code null} * @throws IOException on error when getting the annotation */ + @Nullable private Annotation getAnnotationFromRepository(File file, @Nullable String rev) throws IOException { if (!env.getPathAccepter().accept(file)) { LOGGER.log(Level.FINEST, "file ''{0}'' not accepted for annotation", file); @@ -681,7 +684,7 @@ public boolean hasHistoryCacheForFile(File file) { * Check if we can annotate the specified file. * * @param file the file to check - * @return true if the file is under version control and the + * @return whether the file is under version control, can be annotated and the * version control system supports annotation */ public boolean hasAnnotation(File file) { @@ -691,6 +694,16 @@ public boolean hasAnnotation(File file) { return false; } + AbstractAnalyzer.Genre genre = AnalyzerGuru.getGenre(file.toString()); + if (genre == null) { + LOGGER.log(Level.INFO, "will not produce annotation for ''{0}'' with unknown genre", file); + return false; + } + if (genre.equals(AbstractAnalyzer.Genre.DATA) || genre.equals(AbstractAnalyzer.Genre.IMAGE)) { + LOGGER.log(Level.INFO, "no sense to produce annotation for binary file ''{0}''", file); + return false; + } + Repository repo = getRepository(file); if (repo == null) { LOGGER.log(Level.FINEST, "cannot find repository for ''{0}'' to check annotation presence", file); 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 c884e1ab48b..34405a7d2ac 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, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved. */ package org.opengrok.indexer.history; @@ -37,18 +37,18 @@ import org.opengrok.indexer.web.Util; /** - * handles parsing the output of the {@code hg annotate} command - * into an annotation object. + * Handles parsing the output of the {@code hg annotate} command + * into an {@link Annotation} object. */ class MercurialAnnotationParser implements Executor.StreamHandler { private static final Logger LOGGER = LoggerFactory.getLogger(MercurialAnnotationParser.class); private Annotation annotation = null; - HashMap revs; - File file; + private final HashMap revs; + private final File file; /** - * Pattern used to extract author/revision from {@code hg annotate}. + * Pattern used to extract author/revision from the {@code hg annotate} command. */ private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^\\s*(\\d+):"); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepositoryAnnotationParser.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepositoryAnnotationParser.java index 42732c28c9a..e12f431f88c 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepositoryAnnotationParser.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/SCCSRepositoryAnnotationParser.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved. */ package org.opengrok.indexer.history; @@ -42,18 +42,21 @@ public class SCCSRepositoryAnnotationParser implements Executor.StreamHandler { private static final Logger LOGGER = LoggerFactory.getLogger(SCCSRepositoryAnnotationParser.class); /** - * Store annotation created by processStream. + * Store annotation created by {@link #processStream(InputStream)}. */ private final Annotation annotation; private final Map authors; + private final File file; + /** * Pattern used to extract revision from the {@code sccs get} command. */ private static final Pattern ANNOTATION_PATTERN = Pattern.compile("^([\\d.]+)\\s+"); SCCSRepositoryAnnotationParser(File file, Map authors) { + this.file = file; this.annotation = new Annotation(file.getName()); this.authors = authors; } @@ -69,8 +72,7 @@ public Annotation getAnnotation() { @Override public void processStream(InputStream input) throws IOException { - try (BufferedReader in = new BufferedReader( - new InputStreamReader(input))) { + try (BufferedReader in = new BufferedReader(new InputStreamReader(input))) { String line; int lineno = 0; while ((line = in.readLine()) != null) { @@ -86,8 +88,8 @@ public void processStream(InputStream input) throws IOException { annotation.addLine(rev, author, true); } else { LOGGER.log(Level.SEVERE, - "Error: did not find annotations in line {0}: [{1}]", - new Object[]{lineno, line}); + "Error: did not find annotations in line {0} for ''{2}'': [{1}]", + new Object[]{lineno, line, file}); } } } diff --git a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java index 08c41d00e7e..4850ebb31a7 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -360,6 +360,9 @@ private void populateGenreIfEmpty(DiffData data, InputStream[] inArray) { for (int i = 0; i < 2 && data.genre == null; i++) { try { data.genre = AnalyzerGuru.getGenre(inArray[i]); + if (data.genre == null) { + data.errorMsg = "Unable to determine the file type"; + } } catch (IOException e) { data.errorMsg = "Unable to determine the file type: " + Util.htmlize(e.getMessage());