Skip to content

Commit

Permalink
avoid annotations for binary files (#4476)
Browse files Browse the repository at this point in the history
fixes #4473
  • Loading branch information
vladak authored Nov 9, 2023
1 parent 86b9d98 commit 78a8812
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
Expand All @@ -733,24 +735,23 @@ 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));
}

/**
* 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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -681,7 +684,7 @@ public boolean hasHistoryCacheForFile(File file) {
* Check if we can annotate the specified file.
*
* @param file the file to check
* @return <code>true</code> 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) {
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<String, HistoryEntry> revs;
File file;
private final HashMap<String, HistoryEntry> 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+):");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<String, String> 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<String, String> authors) {
this.file = file;
this.annotation = new Annotation(file.getName());
this.authors = authors;
}
Expand All @@ -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) {
Expand All @@ -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});
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions opengrok-web/src/main/java/org/opengrok/web/PageConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit 78a8812

Please sign in to comment.