use history cache dates in directory listing for regular files (#4275)

fixes #4087

Author: vladak

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java

@@ -306,6 +306,8 @@ public final class Configuration {
 
     private boolean historyBasedReindex;
 
+    private boolean useHistoryCacheForDirectoryListing;
+
     /**
      * History handling types for remote SCM repositories.
      * <ul>
@@ -544,12 +546,14 @@ public Configuration() {
         setAllowLeadingWildcard(true);
         setAllowedSymlinks(new HashSet<>());
         setAnnotationCacheEnabled(false);
+        setApiTimeout(300); // 5 minutes
         setAuthenticationTokens(new HashSet<>());
         setAuthorizationWatchdogEnabled(false);
         //setBugPage("http://bugs.myserver.org/bugdatabase/view_bug.do?bug_id=");
         setBugPattern("\\b([12456789][0-9]{6})\\b");
         setCachePages(5);
         setCanonicalRoots(new HashSet<>());
+        setConnectTimeout(10);
         setIndexerCommandTimeout(600); // 10 minutes
         setRestfulCommandTimeout(60);
         setInteractiveCommandTimeout(30);
@@ -568,6 +572,7 @@ public Configuration() {
         setGroups(new TreeSet<>());
         setGroupsCollapseThreshold(4);
         setHandleHistoryOfRenamedFiles(false);
+        setHistoryBasedReindex(true);
         setHistoryCache(true);
         setHistoryEnabled(true);
         setHitsPerPage(25);
@@ -602,15 +607,13 @@ public Configuration() {
         setSourceRoot(null);
         //setTabSize(4);
         setTagsEnabled(false);
+        setUseHistoryCacheForDirectoryListing(true);
         //setUserPage("http://www.myserver.org/viewProfile.jspa?username=");
-        // Set to empty string so we can append it to the URL unconditionally later.
-        setHistoryBasedReindex(true);
+        // Set to empty string, so we can append it to the URL unconditionally later.
         setUserPageSuffix("");
         setWebappLAF("default");
         // webappCtags is default(boolean)
         setXrefTimeout(30);
-        setApiTimeout(300); // 5 minutes
-        setConnectTimeout(10);
     }
 
     public String getRepoCmd(String clazzName) {
@@ -1462,6 +1465,14 @@ public void setHistoryBasedReindex(boolean flag) {
         historyBasedReindex = flag;
     }
 
+    public boolean isUseHistoryCacheForDirectoryListing() {
+        return useHistoryCacheForDirectoryListing;
+    }
+
+    public void setUseHistoryCacheForDirectoryListing(boolean flag) {
+        useHistoryCacheForDirectoryListing = flag;
+    }
+
     /**
      * Write the current configuration to a file.
      *

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java

@@ -1499,6 +1499,14 @@ public void setHistoryBasedReindex(boolean flag) {
         syncWriteConfiguration(flag, Configuration::setHistoryBasedReindex);
     }
 
+    public boolean isUseHistoryCacheForDirectoryListing() {
+        return syncReadConfiguration(Configuration::isUseHistoryCacheForDirectoryListing);
+    }
+
+    public void setUseHistoryCacheForDirectoryListing(boolean flag) {
+        syncWriteConfiguration(flag, Configuration::setUseHistoryCacheForDirectoryListing);
+    }
+
     public FileCollector getFileCollector(String projectName) {
         return fileCollectorMap.get(projectName);
     }

opengrok-indexer/src/main/java/org/opengrok/indexer/history/FileHistoryCache.java

@@ -40,7 +40,6 @@
 import java.nio.file.Path;
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -66,6 +65,7 @@
 import org.opengrok.indexer.configuration.PathAccepter;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.logger.LoggerFactory;
+import org.opengrok.indexer.search.DirectoryEntry;
 import org.opengrok.indexer.util.Progress;
 import org.opengrok.indexer.util.Statistics;
 
@@ -666,7 +666,7 @@ public History get(File file, Repository repository, boolean withFiles) throws C
                     fileHistoryCacheHits.increment();
                 }
                 return readHistory(cacheFile, repository);
-            } catch (Exception e) {
+            } catch (IOException e) {
                 LOGGER.log(Level.WARNING, String.format("Error when reading cache file '%s'", cacheFile), e);
             }
         }
@@ -679,6 +679,7 @@ public History get(File file, Repository repository, boolean withFiles) throws C
     }
 
     @Override
+    @Nullable
     public HistoryEntry getLastHistoryEntry(File file) throws CacheException {
         if (isUpToDate(file)) {
             File cacheFile = getCachedFile(file);
@@ -687,7 +688,7 @@ public HistoryEntry getLastHistoryEntry(File file) throws CacheException {
                     fileHistoryCacheHits.increment();
                 }
                 return readLastHistoryEntry(cacheFile);
-            } catch (Exception e) {
+            } catch (IOException e) {
                 LOGGER.log(Level.WARNING, String.format("Error when reading cache file '%s'", cacheFile), e);
             }
         }
@@ -771,11 +772,32 @@ private String getCachedRevision(Repository repository, String revPath) {
     }
 
     @Override
-    public Map<String, Date> getLastModifiedTimes(File directory, Repository repository) {
-        // We don't have a good way to get this information from the file
-        // cache, so leave it to the caller to find a reasonable time to
-        // display (typically the last modified time on the file system).
-        return Collections.emptyMap();
+    public Map<String, HistoryEntry> getLastHistoryEntries(List<DirectoryEntry> entries) {
+        if (entries == null) {
+            return Collections.emptyMap();
+        }
+
+        Map<String, HistoryEntry> map = new HashMap<>();
+        for (DirectoryEntry directoryEntry : entries) {
+            try {
+                File file = directoryEntry.getFile();
+                if (file.isDirectory()) {
+                    continue;
+                }
+
+                HistoryEntry historyEntry = getLastHistoryEntry(file);
+                if (historyEntry != null && historyEntry.getDate() != null) {
+                    map.put(directoryEntry.getFile().getName(), historyEntry);
+                } else {
+                    LOGGER.log(Level.FINE, "cannot get last history entry for ''{0}''",
+                            directoryEntry.getFile());
+                }
+            } catch (CacheException e) {
+                LOGGER.log(Level.FINER, "cannot get last history entry for ''{0}''", directoryEntry.getFile());
+            }
+        }
+
+        return map;
     }
 
     @Override

opengrok-indexer/src/main/java/org/opengrok/indexer/history/GitRepository.java

@@ -110,6 +110,11 @@ public GitRepository() {
         ignoredFiles.add(".git");
     }
 
+    @Override
+    public boolean isMergeCommitsSupported() {
+        return true;
+    }
+
     /**
      * Be careful, git uses only forward slashes in its command and output (not in file path).
      * Using backslashes together with git show will get empty output and 0 status code.

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryCache.java

@@ -23,10 +23,11 @@
 package org.opengrok.indexer.history;
 
 import java.io.File;
-import java.util.Date;
+import java.util.List;
 import java.util.Map;
 
 import org.jetbrains.annotations.Nullable;
+import org.opengrok.indexer.search.DirectoryEntry;
 
 interface HistoryCache extends Cache {
 
@@ -49,6 +50,7 @@ interface HistoryCache extends Cache {
      * @param file The file to retrieve history for
      * @throws CacheException if the history cache cannot be read
      */
+    @Nullable
     HistoryEntry getLastHistoryEntry(File file) throws CacheException;
 
     /**
@@ -93,12 +95,11 @@ interface HistoryCache extends Cache {
      * Get the last modified times for all files and subdirectories in the
      * specified directory.
      *
-     * @param directory which directory to fetch modification times for
-     * @param repository the repository in which the directory lives
-     * @return a map from file names to modification times
+     * @param entries list of {@link DirectoryEntry} instances
+     * @return a map from file names to {@link HistoryEntry} instance
      * @throws CacheException on error
      */
-    Map<String, Date> getLastModifiedTimes(File directory, Repository repository) throws CacheException;
+    Map<String, HistoryEntry> getLastHistoryEntries(List<DirectoryEntry> entries) throws CacheException;
 
     /**
      * Clear entry for single file from history cache.

opengrok-indexer/src/main/java/org/opengrok/indexer/history/HistoryEntry.java

@@ -123,7 +123,17 @@ public void dump() {
                     new Object[]{separator, file});
             separator = ">";
         }
-   }
+    }
+
+    /**
+     * @return description of selected fields; used in the web app.
+     */
+    @JsonIgnore
+    public String getDescription() {
+        return "changeset: " + getRevision()
+                + "\nsummary: " + getMessage() + "\nuser: "
+                + getAuthor() + "\ndate: " + getDate();
+    }
 
     public String getAuthor() {
         return author;

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

@@ -31,7 +31,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -55,6 +54,7 @@
 import org.opengrok.indexer.configuration.PathAccepter;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
 import org.opengrok.indexer.logger.LoggerFactory;
+import org.opengrok.indexer.search.DirectoryEntry;
 import org.opengrok.indexer.util.ForbiddenSymlinkException;
 import org.opengrok.indexer.util.PathUtils;
 import org.opengrok.indexer.util.Statistics;
@@ -337,9 +337,7 @@ private void completeAnnotationWithHistory(File file, Annotation annotation, Rep
                 String hist_rev = he.getRevision();
                 String short_rev = repo.getRevisionForAnnotate(hist_rev);
                 if (revs.contains(short_rev)) {
-                    annotation.addDesc(short_rev, "changeset: " + he.getRevision()
-                            + "\nsummary: " + he.getMessage() + "\nuser: "
-                            + he.getAuthor() + "\ndate: " + he.getDate());
+                    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);
@@ -714,13 +712,21 @@ public boolean hasAnnotationCacheForFile(File file) {
 
     /**
      * Get the last modified times for all files and subdirectories in the specified directory.
-     *
+     * If the related {@link Repository} instance does not exist or if it is capable or merge commits,
+     * however merge commits are disabled in its properties, empty map will be returned.
      * @param directory the directory whose files to check
-     * @return a map from file names to modification times for the files that
+     * @param entries list of {@link DirectoryEntry} instances
+     * @return a map from file names to {@link HistoryEntry} instance for the files that
      * the history cache has information about
      * @throws org.opengrok.indexer.history.CacheException if history cannot be retrieved
      */
-    public Map<String, Date> getLastModifiedTimes(File directory) throws CacheException {
+    public Map<String, HistoryEntry> getLastHistoryEntries(File directory, List<DirectoryEntry> entries) throws CacheException {
+
+        if (!env.isUseHistoryCacheForDirectoryListing()) {
+            LOGGER.log(Level.FINEST, "using history cache to retrieve last modified times for ''{0}}'' is disabled",
+                    directory);
+            return Collections.emptyMap();
+        }
 
         Repository repository = getRepository(directory);
         if (repository == null) {
@@ -729,13 +735,23 @@ public Map<String, Date> getLastModifiedTimes(File directory) throws CacheExcept
             return Collections.emptyMap();
         }
 
+        // Do not use history cache for repositories with merge commits disabled as some files in the repository
+        // could be introduced and changed solely via merge changesets. The call would presumably fall back
+        // to file system based time stamps, however that might be confusing, so avoid that.
+        if (repository.isMergeCommitsSupported() && !repository.isMergeCommitsEnabled()) {
+            LOGGER.log(Level.FINEST,
+                    "will not retrieve last modified times due to merge changesets disabled for ''{0}}''",
+                    directory);
+            return Collections.emptyMap();
+        }
+
         if (!useHistoryCache()) {
             LOGGER.log(Level.FINEST, "history cache is disabled for ''{0}'' to retrieve last modified times",
                     directory);
             return Collections.emptyMap();
         }
 
-        return historyCache.getLastModifiedTimes(directory, repository);
+        return historyCache.getLastHistoryEntries(entries);
     }
 
     /**

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

@@ -129,6 +129,11 @@ public MercurialRepository() {
         ignoredDirs.add(".hg");
     }
 
+    @Override
+    public boolean isMergeCommitsSupported() {
+        return true;
+    }
+
     /**
      * Return name of the branch or "default".
      */

opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java

@@ -127,6 +127,14 @@ public void setHandleRenamedFiles(boolean flag) {
         this.handleRenamedFiles = flag;
     }
 
+    /**
+     * Default implementation, should be overridden by individual repository implementations.
+     * @return whether merge commits are supported
+     */
+    public boolean isMergeCommitsSupported() {
+        return false;
+    }
+
     /**
      * @return true if the repository handles merge commits.
      */
@@ -430,6 +438,9 @@ public String toString() {
             stringBuilder.append("historyCache=on,");
             stringBuilder.append("renamed=");
             stringBuilder.append(this.isHandleRenamedFiles());
+        }
+
+        if (isMergeCommitsSupported()) {
             stringBuilder.append(",");
             stringBuilder.append("merge=");
             stringBuilder.append(this.isMergeCommitsEnabled());

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

@@ -735,7 +735,7 @@ private static void writeAnnotation(int num, Writer out, Annotation annotation,
         out.write("<span class=\"blame\">");
         if (enabled) {
             out.write(ANCHOR_CLASS_START);
-            out.write("r");
+            out.write("r title-tooltip");
             out.write("\" style=\"background-color: ");
             out.write(annotation.getColors().getOrDefault(r, "inherit"));
             out.write("\" href=\"");