Skip to content

Commit c98fd4b

Browse files
committed
hasHitory() should check history cache first
fixes #4064
1 parent d197e5f commit c98fd4b

File tree

5 files changed

+42
-7
lines changed

5 files changed

+42
-7
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public interface AnnotationCache extends Cache {
3737
* @param file file under source root to get the annotation for
3838
* @param rev requested revision
3939
* @return {@link Annotation} object or <code>null</code>
40+
* @throws AnnotationException on error
4041
*/
4142
@Nullable
4243
Annotation get(File file, String rev) throws AnnotationException;

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
package org.opengrok.indexer.history;
2424

2525
import org.opengrok.indexer.logger.LoggerFactory;
26+
import org.opengrok.indexer.util.ForbiddenSymlinkException;
2627

2728
import java.io.File;
2829
import java.util.Collection;
@@ -91,4 +92,13 @@ public interface Cache {
9192
* @return {@code true} if the file is in the cache, {@code false} otherwise
9293
*/
9394
boolean hasCacheForFile(File file) throws HistoryException;
95+
96+
/**
97+
* @param file source file
98+
* @return whether cache entry for given file is fresh
99+
* @throws HistoryException on error
100+
* @throws ForbiddenSymlinkException on error
101+
* TODO: HistoryException -> CacheException
102+
*/
103+
boolean isUpToDate(File file) throws HistoryException, ForbiddenSymlinkException;
94104
}

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,20 @@ Annotation readAnnotation(File file) throws AnnotationException {
9797
}
9898
}
9999

100+
/**
101+
* This is potentially expensive operation as the cache entry has to be retrieved from disk
102+
* in order to tell whether it is stale or not.
103+
* @param file source file
104+
* @return indication whether the cache entry is fresh
105+
*/
106+
public boolean isUpToDate(File file) {
107+
try {
108+
return get(file, null) != null;
109+
} catch (AnnotationException e) {
110+
return false;
111+
}
112+
}
113+
100114
public Annotation get(File file, @Nullable String rev) throws AnnotationException {
101115
Annotation annotation = null;
102116
String latestRevision = LatestRevisionUtil.getLatestRevision(file);

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -595,8 +595,8 @@ private void createDirectoriesForFiles(Set<String> files, Repository repository,
595595
public History get(File file, Repository repository, boolean withFiles)
596596
throws HistoryException, ForbiddenSymlinkException {
597597

598-
File cacheFile = getCachedFile(file);
599-
if (isUpToDate(file, cacheFile)) {
598+
if (isUpToDate(file)) {
599+
File cacheFile = getCachedFile(file);
600600
try {
601601
if (fileHistoryCacheHits != null) {
602602
fileHistoryCacheHits.increment();
@@ -616,10 +616,10 @@ public History get(File file, Repository repository, boolean withFiles)
616616

617617
/**
618618
* @param file the file to check
619-
* @param cachedFile the file which contains the cached history for the file
620619
* @return {@code true} if the cache is up-to-date for the file, {@code false} otherwise
621620
*/
622-
private boolean isUpToDate(File file, File cachedFile) {
621+
public boolean isUpToDate(File file) throws HistoryException, ForbiddenSymlinkException {
622+
File cachedFile = getCachedFile(file);
623623
return cachedFile != null && cachedFile.exists() && file.lastModified() <= cachedFile.lastModified();
624624
}
625625

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,12 +584,22 @@ public InputStream getRevision(String parent, String basename, String rev) {
584584
}
585585

586586
/**
587-
* Does this directory contain files with source control information ?
588-
*
589587
* @param file File object
590-
* @return true if the files in this directory have associated revision history
588+
* @return whether it is possible to retrieve history for the file in any way
591589
*/
592590
public boolean hasHistory(File file) {
591+
// If there is a cache entry that is fresh, there is no need to check the repository,
592+
// as the cache entry will be preferred in getHistory(), barring any time sensitive issues (TOUTOC).
593+
if (hasHistoryCacheForFile(file)) {
594+
try {
595+
if (historyCache.isUpToDate(file)) {
596+
return true;
597+
}
598+
} catch (HistoryException | ForbiddenSymlinkException e) {
599+
LOGGER.log(Level.FINEST, "cannot determine if history cache for ''{0}'' is fresh", file);
600+
}
601+
}
602+
593603
Repository repo = getRepository(file);
594604
if (repo == null) {
595605
LOGGER.log(Level.FINEST, "cannot find repository for ''{0}}'' to check history presence", file);

0 commit comments

Comments
 (0)