HistoryGuru should perform repository fallback

Author: vladak

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

@@ -65,6 +65,7 @@
 import io.micrometer.core.instrument.MeterRegistry;
 import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.TestOnly;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.opengrok.indexer.Metrics;
 import org.opengrok.indexer.configuration.PathAccepter;
 import org.opengrok.indexer.configuration.RuntimeEnvironment;
@@ -89,21 +90,10 @@ class FileHistoryCache implements HistoryCache {
     private static final String LATEST_REV_FILE_NAME = "OpenGroklatestRev";
 
     private final PathAccepter pathAccepter = env.getPathAccepter();
-    private boolean historyIndexDone = false;
 
     private Counter fileHistoryCacheHits;
     private Counter fileHistoryCacheMisses;
 
-    @Override
-    public void setHistoryIndexDone() {
-        historyIndexDone = true;
-    }
-
-    @Override
-    public boolean isHistoryIndexDone() {
-        return historyIndexDone;
-    }
-
     /**
      * Generate history cache for single renamed file.
      * @param filename file path
@@ -214,8 +204,7 @@ private static File getCachedFile(File file) throws HistoryException, ForbiddenS
             }
             sb.append(add);
         } catch (IOException e) {
-            throw new HistoryException("Failed to get path relative to " +
-                    "source root for " + file, e);
+            throw new HistoryException("Failed to get path relative to source root for " + file, e);
         }
 
         return new File(TandemPath.join(sb.toString(), ".gz"));
@@ -470,6 +459,11 @@ private static String getRevisionString(String revision) {
      * stored under this hierarchy, each file containing history of
      * corresponding source file.
      *
+     * <p>
+     * <b>Note that the history object will be changed in the process of storing the history into cache.
+     * Namely the list of files from the history entries will be stripped.</b>
+     * </p>
+     *
      * @param history history object to process into per-file histories
      * @param repository repository object
      * @param tillRevision end revision (can be null)
@@ -641,13 +635,6 @@ private void createDirectoriesForFiles(Set<String> files, Repository repository,
     public History get(File file, Repository repository, boolean withFiles)
             throws HistoryException, ForbiddenSymlinkException {
 
-        return get(file, repository, withFiles, env.isFetchHistoryWhenNotInCache());
-    }
-
-    @Override
-    public History get(File file, Repository repository, boolean withFiles, boolean fallback)
-            throws HistoryException, ForbiddenSymlinkException {
-
         File cacheFile = getCachedFile(file);
         if (isUpToDate(file, cacheFile)) {
             try {
@@ -664,32 +651,7 @@ public History get(File file, Repository repository, boolean withFiles, boolean
             fileHistoryCacheMisses.increment();
         }
 
-        /*
-         * Some mirrors of repositories which are capable of fetching history
-         * for directories may contain lots of files untracked by given SCM.
-         * For these it would be waste of time to get their history
-         * since the history of all files in this repository should have been
-         * fetched in the first phase of indexing.
-         */
-        if (isHistoryIndexDone() && repository.isHistoryEnabled() && repository.hasHistoryForDirectories() &&
-                !fallback) {
-            return null;
-        }
-
-        if (!pathAccepter.accept(file)) {
-            return null;
-        }
-
-        final History history;
-        try {
-            history = repository.getHistory(file);
-        } catch (UnsupportedOperationException e) {
-            // In this case, we've found a file for which the SCM has no history
-            // An example is a non-SCCS file somewhere in an SCCS-controlled workspace.
-            return null;
-        }
-
-        return history;
+        return null;
     }
 
     /**
@@ -714,7 +676,8 @@ public boolean hasCacheForFile(File file) throws HistoryException {
         }
     }
 
-    public String getRepositoryHistDataDirname(Repository repository) {
+    @VisibleForTesting
+    public static String getRepositoryHistDataDirname(Repository repository) {
         String repoDirBasename;
 
         try {

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

@@ -57,18 +57,10 @@ interface HistoryCache {
      * @param withFiles A flag saying whether the returned history should include a list of files
      *                  touched by each changeset. If false, the implementation is allowed to skip the file list,
      *                  but it doesn't have to.
-     * @param fallback whether to fall back to {@link Repository#getHistory(File)}
-     *                 if the history cannot be retrieved from the cache
      * @throws HistoryException if the history cannot be fetched
      * @throws ForbiddenSymlinkException if symbolic-link checking encounters
      * an ineligible link
      */
-    History get(File file, @Nullable Repository repository, boolean withFiles, boolean fallback)
-            throws HistoryException, ForbiddenSymlinkException;
-
-    /**
-     * Usually a wrapper of {@link HistoryCache#get(File, Repository, boolean, boolean)}.
-     */
     History get(File file, @Nullable Repository repository, boolean withFiles)
             throws HistoryException, ForbiddenSymlinkException;
 
@@ -153,8 +145,4 @@ Map<String, Date> getLastModifiedTimes(File directory, Repository repository)
      * @throws HistoryException if an error occurred while getting the info
      */
     String getInfo() throws HistoryException;
-
-    // Set and query if history index phase is done.
-    void setHistoryIndexDone();
-    boolean isHistoryIndexDone();
 }

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

@@ -95,6 +95,16 @@ public final class HistoryGuru {
      */
     private final RepositoryLookup repositoryLookup;
 
+    private boolean historyIndexDone = false;
+
+    public void setHistoryIndexDone() {
+        historyIndexDone = true;
+    }
+
+    public boolean isHistoryIndexDone() {
+        return historyIndexDone;
+    }
+
     /**
      * Creates a new instance of HistoryGuru, and try to set the default source
      * control system.
@@ -230,6 +240,16 @@ public History getHistoryUI(File file) throws HistoryException {
         return getHistory(file, true, true);
     }
 
+    /**
+     * The idea is that some repositories require reaching out to remote server whenever
+     * a history operation is done. Sometimes this is unwanted and this method decides that.
+     * This should be consulted before the actual repository operation, i.e. not when fetching
+     * history from a cache since that is inherently local operation.
+     * @param repo repository
+     * @param file file to decide the operation for
+     * @param ui whether coming from UI
+     * @return whether to perform the history operation
+     */
     boolean isRepoHistoryEligible(Repository repo, File file, boolean ui) {
         RemoteSCM rscm = env.getRemoteScmSupported();
         boolean doRemote = (ui && (rscm == RemoteSCM.UIONLY))
@@ -241,17 +261,19 @@ boolean isRepoHistoryEligible(Repository repo, File file, boolean ui) {
     }
 
     @Nullable
-    private History getHistoryFromCache(File file, Repository repository, boolean withFiles, boolean fallback)
+    private History getHistoryFromCache(File file, Repository repository, boolean withFiles)
             throws HistoryException, ForbiddenSymlinkException {
 
         if (useCache() && historyCache.supportsRepository(repository)) {
-            return historyCache.get(file, repository, withFiles, fallback);
+            return historyCache.get(file, repository, withFiles);
         }
 
         return null;
     }
 
     /**
+     * Get last {@link HistoryEntry} for a file. First, try to retrieve it from the cache.
+     * If that fails, fallback to the repository method.
      * @param file file to get the history entry for
      * @param ui is the request coming from the UI
      * @return last (newest) history entry for given file or {@code null}
@@ -264,15 +286,9 @@ public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryExc
         final File dir = file.isDirectory() ? file : file.getParentFile();
         final Repository repository = getRepository(dir);
 
-        if (!isRepoHistoryEligible(repository, file, ui)) {
-            LOGGER.log(Level.FINER, "cannot retrieve the last history entry for ''{0}'' in {1} because of settings",
-                    new Object[]{file, repository});
-            return null;
-        }
-
         History history;
         try {
-            history = getHistoryFromCache(file, repository, false, false);
+            history = getHistoryFromCache(file, repository, false);
             if (history != null) {
                 HistoryEntry lastHistoryEntry = history.getLastHistoryEntry();
                 if (lastHistoryEntry != null) {
@@ -286,14 +302,21 @@ public HistoryEntry getLastHistoryEntry(File file, boolean ui) throws HistoryExc
             return null;
         }
 
+        if (!isRepoHistoryEligible(repository, file, ui)) {
+            LOGGER.log(Level.FINER, "cannot retrieve the last history entry for ''{0}'' in {1} because of settings",
+                    new Object[]{file, repository});
+            return null;
+        }
+
+        // Fallback to the repository method.
         HistoryEntry lastHistoryEntry = repository.getLastHistoryEntry(file, ui);
         if (lastHistoryEntry != null) {
             LOGGER.log(Level.FINEST, "got latest history entry {0} for ''{1}'' using repository {2}",
                     new Object[]{lastHistoryEntry, file, repository});
         }
         statistics.report(LOGGER, Level.FINEST,
                 String.format("finished retrieval of last history entry for '%s' (%s)",
-                        file, lastHistoryEntry != null ? "success" : "fail"), "history.lasthistoryentry");
+                        file, lastHistoryEntry != null ? "success" : "fail"), "history.entry.latest");
         return lastHistoryEntry;
     }
 
@@ -318,22 +341,52 @@ public History getHistory(File file, boolean withFiles, boolean ui, boolean fall
         final File dir = file.isDirectory() ? file : file.getParentFile();
         final Repository repository = getRepository(dir);
 
-        if (!isRepoHistoryEligible(repository, file, ui)) {
-            return null;
-        }
-
         History history;
         try {
-            history = getHistoryFromCache(file, repository, withFiles, fallback);
+            history = getHistoryFromCache(file, repository, withFiles);
             if (history != null) {
                 return history;
             }
+
+            return getHistoryFromRepository(file, repository, ui);
         } catch (ForbiddenSymlinkException e) {
             LOGGER.log(Level.FINER, e.getMessage());
             return null;
         }
+    }
 
-        return null;
+    @Nullable
+    private History getHistoryFromRepository(File file, Repository repository, boolean ui) throws HistoryException {
+        History history;
+
+        if (!isRepoHistoryEligible(repository, file, ui)) {
+            return null;
+        }
+
+        /*
+         * Some mirrors of repositories which are capable of fetching history
+         * for directories may contain lots of files untracked by given SCM.
+         * For these it would be waste of time to get their history
+         * since the history of all files in this repository should have been
+         * fetched in the first phase of indexing.
+         */
+        if (isHistoryIndexDone() && repository.isHistoryEnabled() && repository.hasHistoryForDirectories()) {
+            return null;
+        }
+
+        if (!env.getPathAccepter().accept(file)) {
+            return null;
+        }
+
+        try {
+            history = repository.getHistory(file);
+        } catch (UnsupportedOperationException e) {
+            // In this case, we've found a file for which the SCM has no history
+            // An example is a non-SCCS file somewhere in an SCCS-controlled workspace.
+            return null;
+        }
+
+        return history;
     }
 
     /**
@@ -684,7 +737,7 @@ private void createCacheReal(Collection<Repository> repositories) {
                     "Failed optimizing the history cache database", he);
         }
         elapsed.report(LOGGER, "Done history cache for all repositories", "indexer.history.cache");
-        historyCache.setHistoryIndexDone();
+        setHistoryIndexDone();
     }
 
     /**