From 2402fe1c5bdb12ca392f87873a7e6699af1534c2 Mon Sep 17 00:00:00 2001 From: Chris Fraire Date: Tue, 12 May 2020 21:58:59 -0500 Subject: [PATCH] Resolve #3010 : add --nestingMaximum switch --- .../indexer/configuration/Configuration.java | 24 ++++++++- .../configuration/RuntimeEnvironment.java | 8 +++ .../opengrok/indexer/history/HistoryGuru.java | 27 +++++----- .../org/opengrok/indexer/index/Indexer.java | 4 ++ .../indexer/history/HistoryGuruTest.java | 54 ++++++++++++++++--- .../api/v1/controller/ProjectsController.java | 2 +- 6 files changed, 96 insertions(+), 23 deletions(-) diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java index f1e610650f8..2b99f6d2a1d 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/Configuration.java @@ -19,7 +19,7 @@ /* * Copyright (c) 2007, 2018, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2017-2019, Chris Fraire . + * Portions Copyright (c) 2017-2020, Chris Fraire . * Portions Copyright (c) 2020, Aleksandr Kirillov . */ package org.opengrok.indexer.configuration; @@ -206,6 +206,7 @@ public final class Configuration { private boolean lastEditedDisplayMode; private String CTagsExtraOptionsFile; private int scanningDepth; + private int nestingMaximum; private Set allowedSymlinks; private Set canonicalRoots; private boolean obfuscatingEMailAddresses; @@ -356,6 +357,26 @@ public void setScanningDepth(int scanningDepth) throws IllegalArgumentException this.scanningDepth = scanningDepth; } + /** + * Gets the nesting maximum of repositories. Default is 1. + */ + public int getNestingMaximum() { + return nestingMaximum; + } + + /** + * Sets the nesting maximum of repositories to a specified value. + * @param nestingMaximum the new value + * @throws IllegalArgumentException if {@code nestingMaximum} is negative + */ + public void setNestingMaximum(int nestingMaximum) throws IllegalArgumentException { + if (nestingMaximum < 0) { + throw new IllegalArgumentException( + String.format(NEGATIVE_NUMBER_ERROR, "nestingMaximum", nestingMaximum)); + } + this.nestingMaximum = nestingMaximum; + } + public int getCommandTimeout() { return commandTimeout; } @@ -487,6 +508,7 @@ public Configuration() { setMaxRevisionThreadCount(Runtime.getRuntime().availableProcessors()); setMessageLimit(500); setNavigateWindowEnabled(false); + setNestingMaximum(1); setOptimizeDatabase(true); setPluginDirectory(null); setPluginStack(new AuthorizationStack(AuthControlFlag.REQUIRED, "default stack")); diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java index 60d327b901c..c61f5ce1d5e 100644 --- a/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java +++ b/opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/RuntimeEnvironment.java @@ -214,6 +214,14 @@ public void setScanningDepth(int scanningDepth) { syncWriteConfiguration(scanningDepth, Configuration::setScanningDepth); } + public int getNestingMaximum() { + return syncReadConfiguration(Configuration::getNestingMaximum); + } + + public void setNestingMaximum(int nestingMaximum) { + syncWriteConfiguration(nestingMaximum, Configuration::setNestingMaximum); + } + public int getCommandTimeout() { return syncReadConfiguration(Configuration::getCommandTimeout); } 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 fa9ba06787a..2c51ed2311e 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 @@ -78,13 +78,13 @@ public final class HistoryGuru { /** * Map of repositories, with {@code DirectoryName} as key. */ - private Map repositories = new ConcurrentHashMap<>(); + private final Map repositories = new ConcurrentHashMap<>(); /** * Set of repository roots (using ConcurrentHashMap but a throwaway value) * with parent of {@code DirectoryName} as key. */ - private Map repositoryRoots = new ConcurrentHashMap<>(); + private final Map repositoryRoots = new ConcurrentHashMap<>(); private final int scanningDepth; @@ -388,7 +388,7 @@ public Map getLastModifiedTimes(File directory) * * @param files list of files to check if they contain a repository * @param ignoredNames what files to ignore - * @param recursiveSearch whether to use recursive search + * @param allowedNesting number of levels of nested repos to allow * @param depth current depth - using global scanningDepth - one can limit * this to improve scanning performance * @param isNested a value indicating if a parent {@link Repository} was @@ -396,7 +396,7 @@ public Map getLastModifiedTimes(File directory) * @return collection of added repositories */ private Collection addRepositories(File[] files, - IgnoredNames ignoredNames, boolean recursiveSearch, int depth, + IgnoredNames ignoredNames, int allowedNesting, int depth, boolean isNested) { List repoList = new ArrayList<>(); @@ -436,7 +436,7 @@ private Collection addRepositories(File[] files, file.getAbsolutePath()); } else if (depth <= scanningDepth) { repoList.addAll(addRepositories(subFiles, ignoredNames, - recursiveSearch, depth + 1, isNested)); + allowedNesting, depth + 1, isNested)); } } } else { @@ -446,17 +446,17 @@ private Collection addRepositories(File[] files, repoList.add(new RepositoryInfo(repository)); putRepository(repository); - if (recursiveSearch && repository.supportsSubRepositories()) { + if (allowedNesting > 0 && repository.supportsSubRepositories()) { File[] subFiles = file.listFiles(); if (subFiles == null) { LOGGER.log(Level.WARNING, "Failed to get sub directories for ''{0}'', check access permissions.", file.getAbsolutePath()); } else if (depth <= scanningDepth) { - // Search only one level down - if not: too much + // Search down to a limit -- if not: too much // stat'ing for huge Mercurial repositories repoList.addAll(addRepositories(subFiles, ignoredNames, - false, depth + 1, true)); + allowedNesting - 1, depth + 1, true)); } } } @@ -482,7 +482,7 @@ private Collection addRepositories(File[] files, public Collection addRepositories(File[] files, IgnoredNames ignoredNames) { - return addRepositories(files, ignoredNames, true, 0, false); + return addRepositories(files, ignoredNames, env.getNestingMaximum(), 0, false); } /** @@ -633,11 +633,9 @@ public void createCache(Collection repositories) { * * @param repositories list of repository paths relative to source root * @return list of repository paths that were found and their history data removed - * @throws HistoryException if history cannot be retrieved */ - public List clearCache(Collection repositories) throws HistoryException { + public List clearCache(Collection repositories) { List clearedRepos = new ArrayList<>(); - HistoryCache cache = historyCache; if (!useCache()) { return clearedRepos; @@ -645,7 +643,7 @@ public List clearCache(Collection repositories) throws HistoryEx for (Repository r : getReposFromString(repositories)) { try { - cache.clear(r); + historyCache.clear(r); clearedRepos.add(r.getDirectoryName()); LOGGER.log(Level.INFO, "History cache for {0} cleared.", r.getDirectoryName()); @@ -676,9 +674,8 @@ public void clearCacheFile(String path) { * successfully cleared are removed from the internal list of repositories. * * @param repositories list of repository paths relative to source root - * @throws HistoryException if history cannot be retrieved */ - public void removeCache(Collection repositories) throws HistoryException { + public void removeCache(Collection repositories) { if (!useCache()) { return; } 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 2f0d3a17d90..646fefca8bb 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 @@ -599,6 +599,10 @@ public static String[] parseOptions(String[] argv) throws ParseException { "files), but process all other command line options.").Do(v -> runIndex = false); + parser.on("--nestingMaximum", "=number", + "Maximum of nested repositories. Default is 1.").Do(v -> + cfg.setNestingMaximum((Integer) v)); + parser.on("-O", "--optimize", "=on|off", ON_OFF, Boolean.class, "Turn on/off the optimization of the index database as part of the", "indexing step. Default is on."). 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 59146f0611d..aabe986c31a 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 @@ -19,7 +19,7 @@ /* * Copyright (c) 2008, 2019, Oracle and/or its affiliates. All rights reserved. - * Portions Copyright (c) 2019, Chris Fraire . + * Portions Copyright (c) 2019-2020, Chris Fraire . */ package org.opengrok.indexer.history; @@ -36,6 +36,7 @@ import java.util.Collections; import java.util.List; +import org.junit.After; import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; @@ -58,14 +59,18 @@ public class HistoryGuruTest { private static TestRepository repository = new TestRepository(); private static final List FILES = new ArrayList<>(); + private static RuntimeEnvironment env; + + private static int savedNestingMaximum; @Rule public ConditionalRunRule rule = new ConditionalRunRule(); @BeforeClass public static void setUpClass() throws Exception { - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - + env = RuntimeEnvironment.getInstance(); + savedNestingMaximum = env.getNestingMaximum(); + repository = new TestRepository(); repository.create(HistoryGuru.class.getResourceAsStream( "repositories.zip")); @@ -96,6 +101,11 @@ public static void tearDownClass() { repository.destroy(); } + @After + public void tearDown() { + env.setNestingMaximum(savedNestingMaximum); + } + @Test public void testGetRevision() throws HistoryException, IOException { HistoryGuru instance = HistoryGuru.getInstance(); @@ -147,7 +157,6 @@ public void getCacheInfo() throws HistoryException { @ConditionalRun(RepositoryInstalled.GitInstalled.class) public void testAddRemoveRepositories() { HistoryGuru instance = HistoryGuru.getInstance(); - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); final int numReposOrig = instance.getRepositories().size(); // Try to add non-existent repository. @@ -175,7 +184,6 @@ public void testAddRemoveRepositories() { @ConditionalRun(RepositoryInstalled.MercurialInstalled.class) public void testAddSubRepositoryNotNestable() { HistoryGuru instance = HistoryGuru.getInstance(); - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); // Check out CVS underneath a Git repository. File cvsRoot = new File(repository.getSourceRoot(), "cvs_test"); @@ -197,7 +205,6 @@ public void testAddSubRepositoryNotNestable() { @ConditionalRun(RepositoryInstalled.MercurialInstalled.class) public void testAddSubRepository() { HistoryGuru instance = HistoryGuru.getInstance(); - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); // Clone a Mercurial repository underneath a Mercurial repository. File hgRoot = new File(repository.getSourceRoot(), "mercurial"); @@ -211,4 +218,39 @@ public void testAddSubRepository() { env.getIgnoredNames()); assertEquals(2, addedRepos.size()); } + + @Test + public void testNestingMaximum() throws IOException { + // Just fake a nesting of Repo -> Git -> Git. + File repoRoot = new File(repository.getSourceRoot(), "repoRoot"); + certainlyMkdirs(repoRoot); + File repo0 = new File(repoRoot, ".repo"); + certainlyMkdirs(repo0); + File sub1 = new File(repoRoot, "sub1"); + certainlyMkdirs(sub1); + File repo1 = new File(sub1, ".git"); + certainlyMkdirs(repo1); + File sub2 = new File(sub1, "sub2"); + certainlyMkdirs(sub2); + File repo2 = new File(sub2, ".git"); + certainlyMkdirs(repo2); + + HistoryGuru instance = HistoryGuru.getInstance(); + Collection addedRepos = instance.addRepositories( + Collections.singleton(Paths.get(repository.getSourceRoot(), + "repoRoot").toString()), env.getIgnoredNames()); + assertEquals("should add to default nesting maximum", 2, addedRepos.size()); + + env.setNestingMaximum(2); + addedRepos = instance.addRepositories( + Collections.singleton(Paths.get(repository.getSourceRoot(), + "repoRoot").toString()), env.getIgnoredNames()); + assertEquals("should get one more repo", 3, addedRepos.size()); + } + + private static void certainlyMkdirs(File file) throws IOException { + if (!file.mkdirs()) { + throw new IOException("Couldn't mkdirs " + file); + } + } } diff --git a/opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java b/opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java index c4681cc2090..97a6a00fc08 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java +++ b/opengrok-web/src/main/java/org/opengrok/web/api/v1/controller/ProjectsController.java @@ -208,7 +208,7 @@ public void deleteProjectData(@PathParam("project") String projectName) throws H @DELETE @Path("/{project}/historycache") - public void deleteHistoryCache(@PathParam("project") String projectName) throws HistoryException { + public void deleteHistoryCache(@PathParam("project") String projectName) { // Avoid classification as a taint bug. projectName = Laundromat.launderInput(projectName);