diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Project.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Project.java index aefd23e661a..cb34b8cedd4 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Project.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Project.java @@ -84,10 +84,15 @@ public class Project implements Comparable, Nameable, Serializable { private Boolean handleRenamedFiles = null; /** - * This flag enables/disables per-project history cache. + * This flag enables/disables per-project history queries. */ private Boolean historyEnabled = null; + /** + * This flag enables/disables per-project history cache. + */ + private Boolean historyCacheEnabled = null; + /** * This flag enables/disables per-project annotation cache. */ @@ -301,6 +306,20 @@ public void setHistoryEnabled(boolean flag) { this.historyEnabled = flag; } + /** + * @return true if this project should have history cache. + */ + public boolean isHistoryCacheEnabled() { + return historyCacheEnabled != null && historyCacheEnabled; + } + + /** + * @param flag true if project should have history cache, false otherwise. + */ + public void setHistoryCacheEnabled(boolean flag) { + this.historyCacheEnabled = flag; + } + /** * @return true if this project should have annotation cache. */ @@ -494,11 +513,16 @@ public final void completeWithDefaults() { setHandleRenamedFiles(env.isHandleHistoryOfRenamedFiles()); } - // Allow project to override global setting of history cache generation. + // Allow project to override global setting of history queries. if (historyEnabled == null) { setHistoryEnabled(env.isHistoryEnabled()); } + // Allow project to override global setting of history cache generation. + if (historyCacheEnabled == null) { + setHistoryCacheEnabled(env.useHistoryCache()); + } + // Allow project to override global setting of annotation cache generation. if (annotationCacheEnabled == null) { setAnnotationCacheEnabled(env.isAnnotationCacheEnabled()); 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 7066469d060..4bd650becd3 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 @@ -135,6 +135,11 @@ private HistoryGuru() { repositoryLookup = RepositoryLookup.cached(); } + @VisibleForTesting + HistoryCache getHistoryCache() { + return historyCache; + } + /** * Set annotation cache to its default implementation. * @return {@link AnnotationCache} instance or {@code null} on error @@ -164,18 +169,16 @@ static AnnotationCache initializeAnnotationCache() { * @return {@link HistoryCache} instance */ private HistoryCache initializeHistoryCache() { - HistoryCache historyCacheResult = null; - if (env.useHistoryCache()) { - historyCacheResult = new FileHistoryCache(); + HistoryCache historyCacheResult = new FileHistoryCache(); - try { - historyCacheResult.initialize(); - } catch (CacheException he) { - LOGGER.log(Level.WARNING, "Failed to initialize the history cache", he); - // Failed to initialize, run without a history cache. - historyCacheResult = null; - } + try { + historyCacheResult.initialize(); + } catch (CacheException he) { + LOGGER.log(Level.WARNING, "Failed to initialize the history cache", he); + // Failed to initialize, run without a history cache. + historyCacheResult = null; } + return historyCacheResult; } @@ -188,13 +191,24 @@ public static HistoryGuru getInstance() { return INSTANCE; } - /** - * Return whether cache should be used for the history log. - * - * @return {@code true} if the history cache has been enabled and initialized, {@code false} otherwise - */ - private boolean useHistoryCache() { - return historyCache != null; + private boolean useHistoryCache(File file) { + if (historyCache == null) { + return false; + } + + return useHistoryCache(getRepository(file)); + } + + private boolean useHistoryCache(@Nullable Repository repository) { + if (historyCache == null || repository == null) { + return false; + } + + if (!historyCache.supportsRepository(repository)) { + return false; + } + + return repository.isHistoryCacheEnabled(); } /** @@ -418,7 +432,7 @@ boolean isRepoHistoryEligible(Repository repo, File file, boolean ui) { private History getHistoryFromCache(File file, Repository repository, boolean withFiles) throws CacheException { - if (useHistoryCache() && historyCache.supportsRepository(repository)) { + if (useHistoryCache(repository)) { return historyCache.get(file, repository, withFiles); } @@ -428,7 +442,7 @@ private History getHistoryFromCache(File file, Repository repository, boolean wi @Nullable private HistoryEntry getLastHistoryEntryFromCache(File file, Repository repository) throws CacheException { - if (useHistoryCache() && historyCache.supportsRepository(repository)) { + if (useHistoryCache(repository)) { return historyCache.getLastHistoryEntry(file); } @@ -451,6 +465,9 @@ public HistoryEntry getLastHistoryEntry(File file, boolean ui, boolean fallback) launderLog(file.toString()))); final File dir = file.isDirectory() ? file : file.getParentFile(); final Repository repository = getRepository(dir); + if (repository == null) { + return null; + } final String meterName = "history.entry.latest"; try { @@ -479,7 +496,7 @@ public HistoryEntry getLastHistoryEntry(File file, boolean ui, boolean fallback) if (!isRepoHistoryEligible(repository, file, ui)) { statistics.report(LOGGER, Level.FINEST, String.format("cannot retrieve the last history entry for ''%s'' in %s because of settings", - launderLog(file.toString()), repository), meterName); + launderLog(file.toString()), repository), meterName); return null; } @@ -691,7 +708,7 @@ private static boolean repositoryHasHistory(File file, Repository repo) { * @return if there is history cache entry for the file */ public boolean hasHistoryCacheForFile(File file) { - if (!useHistoryCache()) { + if (!useHistoryCache(file)) { LOGGER.finest(() -> String.format("history cache is off for '%s' to check history cache presence", launderLog(file.toString()))); return false; @@ -823,15 +840,15 @@ public boolean fillLastHistoryEntries(File directory, List entri return true; } - if (!useHistoryCache()) { - LOGGER.finest(() -> String.format("history cache is disabled for '%s' to retrieve last modified times", + Repository repository = getRepository(directory); + if (repository == null) { + LOGGER.finest(() -> String.format("cannot find repository for '%s' to retrieve last modified times", launderLog(directory.toString()))); return true; } - Repository repository = getRepository(directory); - if (repository == null) { - LOGGER.finest(() -> String.format("cannot find repository for '%s' to retrieve last modified times", + if (!useHistoryCache(repository)) { + LOGGER.finest(() -> String.format("history cache is disabled for '%s' to retrieve last modified times", launderLog(directory.toString()))); return true; } @@ -1036,9 +1053,17 @@ public void storeHistory(File file, History history) { } private void createHistoryCache(Repository repository, String sinceRevision) throws CacheException, HistoryException { + if (!repository.isHistoryCacheEnabled()) { + LOGGER.log(Level.INFO, + "Skipping history cache creation for {0} and its subdirectories: history cache disabled", + repository); + return; + } + if (!repository.isHistoryEnabled()) { LOGGER.log(Level.INFO, - "Skipping history cache creation for {0} and its subdirectories", repository); + "Skipping history cache creation for {0} and its subdirectories: history disabled", + repository); return; } @@ -1139,9 +1164,14 @@ private Map> createHistoryCacheReal(Collection> createHistoryCache(Collection repositories) { - if (!useHistoryCache()) { + if (repositories.stream(). + map(e -> new File(env.getSourceRootPath(), e)). + map(this::getRepository). + filter(Objects::nonNull). + noneMatch(RepositoryInfo::isHistoryCacheEnabled)) { return Collections.emptyMap(); } + return createHistoryCacheReal(getReposFromString(repositories)); } @@ -1151,12 +1181,12 @@ public Map> createHistoryCache(Collection removeHistoryCache(Collection repositories) { - if (!useHistoryCache()) { + if (repositories.stream().noneMatch(RepositoryInfo::isHistoryCacheEnabled)) { return List.of(); } @@ -1258,7 +1288,7 @@ public List removeAnnotationCache(Collection repositorie * @return map of repository to optional exception */ public Map> createHistoryCache() { - if (!useHistoryCache()) { + if (repositories.values().stream().noneMatch(RepositoryInfo::isHistoryCacheEnabled)) { return Collections.emptyMap(); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java index ffe1dd1a74b..f9acbbf38ad 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/history/RepositoryInfo.java @@ -78,6 +78,8 @@ public class RepositoryInfo implements Serializable { @DTOElement private boolean historyEnabled; @DTOElement + private boolean historyCacheEnabled; + @DTOElement private boolean annotationCacheEnabled; @DTOElement private boolean mergeCommitsEnabled; @@ -105,6 +107,7 @@ public RepositoryInfo(RepositoryInfo orig) { this.branch = orig.branch; this.currentVersion = orig.currentVersion; this.historyEnabled = orig.historyEnabled; + this.historyCacheEnabled = orig.historyCacheEnabled; this.annotationCacheEnabled = orig.annotationCacheEnabled; this.handleRenamedFiles = orig.handleRenamedFiles; this.mergeCommitsEnabled = orig.mergeCommitsEnabled; @@ -164,7 +167,7 @@ public void setHistoryBasedReindex(boolean flag) { } /** - * @return true if the repository should have history cache. + * @return true if the repository should support history queries. */ public boolean isHistoryEnabled() { return this.historyEnabled; @@ -174,6 +177,17 @@ public void setHistoryEnabled(boolean flag) { this.historyEnabled = flag; } + /** + * @return true if the history for this repository should be stored in history cache. + */ + public boolean isHistoryCacheEnabled() { + return this.historyCacheEnabled; + } + + public void setHistoryCacheEnabled(boolean flag) { + this.historyCacheEnabled = flag; + } + public boolean isAnnotationCacheEnabled() { return annotationCacheEnabled; } @@ -381,6 +395,7 @@ public void fillFromProject() { Project proj = Project.getProject(getDirectoryNameRelative()); if (proj != null) { setHistoryEnabled(proj.isHistoryEnabled()); + setHistoryCacheEnabled(proj.isHistoryCacheEnabled()); setAnnotationCacheEnabled(proj.isAnnotationCacheEnabled()); setHandleRenamedFiles(proj.isHandleRenamedFiles()); setMergeCommitsEnabled(proj.isMergeCommitsEnabled()); @@ -391,6 +406,7 @@ public void fillFromProject() { RuntimeEnvironment env = RuntimeEnvironment.getInstance(); setHistoryEnabled(env.isHistoryEnabled()); + setHistoryCacheEnabled(env.useHistoryCache()); setAnnotationCacheEnabled(env.isAnnotationCacheEnabled()); setHandleRenamedFiles(env.isHandleHistoryOfRenamedFiles()); setMergeCommitsEnabled(env.isMergeCommitsEnabled()); @@ -433,9 +449,20 @@ public String toString() { stringBuilder.append(","); if (!isHistoryEnabled()) { + stringBuilder.append("history=off"); + } else { + stringBuilder.append("history=on"); + } + + stringBuilder.append(","); + if (!isHistoryCacheEnabled()) { stringBuilder.append("historyCache=off"); } else { stringBuilder.append("historyCache=on,"); + } + + if (isHandleRenamedFiles()) { + stringBuilder.append(","); stringBuilder.append("renamed="); stringBuilder.append(this.isHandleRenamedFiles()); } diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java index 945dd90b113..8be66c46548 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/index/Indexer.java @@ -1123,13 +1123,12 @@ public Map> prepareIndexer(RuntimeEnvironment en * @param repositories list of repository paths relative to source root * @return map of repository to exception * @throws IndexerException indexer exception - * @throws IOException I/O exception */ public Map> prepareIndexer(RuntimeEnvironment env, Set searchPaths, boolean addProjects, boolean createHistoryCache, - List repositories) throws IndexerException, IOException { + List repositories) throws IndexerException { if (!env.validateUniversalCtags()) { throw new IndexerException("Could not find working Universal ctags. " + diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruTest.java index cd78582194e..9b6a8ed0dd9 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruTest.java @@ -38,6 +38,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -95,7 +96,9 @@ static void setUpClass() throws Exception { savedNestingMaximum = env.getNestingMaximum(); repository = new TestRepository(); - repository.create(HistoryGuru.class.getResource("/repositories")); + URL resourceURL = HistoryGuru.class.getResource("/repositories"); + assertNotNull(resourceURL); + repository.create(resourceURL); RepositoryFactory.initializeIgnoredNames(env); FileUtilities.getAllFiles(new File(repository.getSourceRoot()), FILES, true); assertNotEquals(0, FILES.size()); @@ -483,9 +486,10 @@ void testGetLastHistoryEntryRepoHistoryDisabled() throws Exception { assertTrue(file.exists()); HistoryGuru instance = HistoryGuru.getInstance(); Repository repository = instance.getRepository(file); + assertNotNull(repository); // HistoryGuru is final class so cannot be reasonably mocked with Mockito. - // In order to avoid getting the history from the cache, move the cache away for a bit. + // In order to avoid getting the history from the cache, move the history cache directory aside for a bit. String dirName = CacheUtil.getRepositoryCacheDataDirname(repository, new FileHistoryCache()); assertNotNull(dirName); Path histPath = Path.of(dirName); @@ -493,7 +497,6 @@ void testGetLastHistoryEntryRepoHistoryDisabled() throws Exception { Files.move(histPath, tmpHistPath); assertFalse(histPath.toFile().exists()); - assertNotNull(repository); repository.setHistoryEnabled(false); assertNull(instance.getLastHistoryEntry(file, false, true)); @@ -502,6 +505,23 @@ void testGetLastHistoryEntryRepoHistoryDisabled() throws Exception { Files.move(tmpHistPath, histPath); } + @Test + void testGetHistoryVsHistoryCacheEnabled() throws Exception { + File file = Path.of(repository.getSourceRoot(), "git", "main.c").toFile(); + assertTrue(file.exists()); + HistoryGuru instance = HistoryGuru.getInstance(); + Repository repository = instance.getRepository(file); + assertNotNull(repository); + + assertTrue(repository.isHistoryCacheEnabled()); + assertNotNull(instance.getHistory(file, false, false, false)); + repository.setHistoryCacheEnabled(false); + assertNull(instance.getHistory(file, false, false, false)); + + // cleanup + repository.setHistoryCacheEnabled(true); + } + /** * Test that it is not possible to get last history entries for repository * that does not have the merge changesets enabled. diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruVsRepositoryCacheTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruVsRepositoryCacheTest.java new file mode 100644 index 00000000000..0a82363106f --- /dev/null +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/history/HistoryGuruVsRepositoryCacheTest.java @@ -0,0 +1,101 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + */ +package org.opengrok.indexer.history; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.opengrok.indexer.configuration.CommandTimeoutType; +import org.opengrok.indexer.configuration.RuntimeEnvironment; +import org.opengrok.indexer.util.IOUtils; +import org.opengrok.indexer.util.TestRepository; + +import java.io.File; +import java.net.URL; +import java.nio.file.Path; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class HistoryGuruVsRepositoryCacheTest { + private RuntimeEnvironment env; + + private static TestRepository testRepository; + + @BeforeEach + void setUpClass() throws Exception { + env = RuntimeEnvironment.getInstance(); + + testRepository = new TestRepository(); + URL resourceURL = HistoryGuru.class.getResource("/repositories"); + assertNotNull(resourceURL); + testRepository.create(resourceURL); + + env.setSourceRoot(testRepository.getSourceRoot()); + env.setDataRoot(testRepository.getDataRoot()); + env.setHistoryEnabled(true); + env.setProjectsEnabled(true); + RepositoryFactory.initializeIgnoredNames(env); + + // Restore the project and repository information. + env.setProjects(new HashMap<>()); + env.setRepositories(testRepository.getSourceRoot()); + HistoryGuru.getInstance().invalidateRepositories(env.getRepositories(), CommandTimeoutType.INDEXER); + env.generateProjectRepositoriesMap(); + } + + @AfterEach + void tearDownClass() throws Exception { + testRepository.destroy(); + } + + /** + * Test that {@link HistoryGuru#createHistoryCache(Collection)} honors + * the {@link Repository#isHistoryCacheEnabled()} setting. + */ + @ParameterizedTest + @ValueSource(booleans = {false, true}) + void testCreateCacheVsRepository(boolean historyCacheEnabled) throws Exception { + Path filePath = Path.of(env.getSourceRootPath(), "git", "main.c"); + File file = filePath.toFile(); + assertTrue(file.exists()); + HistoryGuru histGuru = HistoryGuru.getInstance(); + Repository repository = histGuru.getRepository(file); + assertNotNull(repository); + + repository.setHistoryCacheEnabled(historyCacheEnabled); + histGuru.createHistoryCache(List.of(repository.getDirectoryNameRelative())); + + HistoryCache historyCache = histGuru.getHistoryCache(); + assertNotNull(historyCache); + assertEquals(historyCacheEnabled, historyCache.hasCacheForFile(file)); + + // cleanup + IOUtils.removeRecursive(Path.of(env.getDataRootPath())); + } +} diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java index da056fc9452..269efea9dfb 100644 --- a/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java +++ b/opengrok-indexer/src/test/java/org/opengrok/indexer/index/IndexDatabaseTest.java @@ -215,7 +215,7 @@ void testGetDefinitions() throws Exception { *
  • the directory names used
    • *
  • the way file paths are constructed ({@link TandemPath})
    • * - * @param fileName file name to check + * @param fileName path relative to source root * @param shouldExist whether the data file should exist */ private void checkDataExistence(String fileName, boolean shouldExist) { @@ -224,20 +224,12 @@ private void checkDataExistence(String fileName, boolean shouldExist) { for (String dirName : new String[] {"historycache", "annotationcache", IndexDatabase.XREF_DIR}) { File dataDir = new File(env.getDataRootFile(), dirName); - File file = new File(env.getServerName(), fileName); + File file = new File(env.getSourceRootFile(), fileName); String cacheFile; if (dirName.equals("annotationcache")) { - if (shouldExist) { - assertTrue(historyGuru.hasAnnotationCacheForFile(file)); - } else { - assertFalse(historyGuru.hasAnnotationCacheForFile(file)); - } + assertEquals(shouldExist, historyGuru.hasAnnotationCacheForFile(file)); } else if (dirName.equals("historycache")) { - if (shouldExist) { - assertTrue(historyGuru.hasHistoryCacheForFile(file)); - } else { - assertFalse(historyGuru.hasHistoryCacheForFile(file)); - } + assertEquals(shouldExist, historyGuru.hasHistoryCacheForFile(file)); } else { cacheFile = TandemPath.join(fileName, ".gz"); File dataFile = new File(dataDir, cacheFile);