Lift most of Oracle's PR #1846 on symlink bug, with assertion fix

Chris Fraire <[email protected]> fixed the assertion.

Author: Vladimir Kotal

src/org/opensolaris/opengrok/history/FileHistoryCache.java

@@ -631,6 +631,11 @@ public boolean hasCacheForDirectory(File directory, Repository repository)
         return dir.exists();
     }
 
+    @Override
+    public boolean hasCacheForFile(File file) throws HistoryException {
+        return getCachedFile(file).exists();
+    }
+
     public String getRepositoryHistDataDirname(Repository repository) {
         RuntimeEnvironment env = RuntimeEnvironment.getInstance();
         String repoDirBasename;
@@ -645,7 +650,7 @@ public String getRepositoryHistDataDirname(Repository repository) {
         }
 
         return env.getDataRootPath() + File.separatorChar
-            + this.historyCacheDirName
+            + FileHistoryCache.historyCacheDirName
             + repoDirBasename;
     }
 

src/org/opensolaris/opengrok/history/HistoryCache.java

@@ -94,6 +94,14 @@ void store(History history, Repository repository)
     boolean hasCacheForDirectory(File directory, Repository repository)
             throws HistoryException;
 
+    /**
+     * Check if the specified file is present in the cache.
+     * @param file the file to check
+     * @return {@code true} if the file is in the cache, {@code false}
+     * otherwise
+     */
+    boolean hasCacheForFile(File file) throws HistoryException;
+
     /**
      * Get the revision identifier for the latest cached revision in a
      * repository.

src/org/opensolaris/opengrok/history/HistoryGuru.java

@@ -295,6 +295,23 @@ public boolean hasHistory(File file) {
                 || !repo.isRemote());
     }
 
+    /**
+     * Does the history cache contain entry for this directory ?
+     * @param file
+     * @return true if there is cache, false otherwise
+     */
+    public boolean hasCacheForFile(File file) {
+        if (!useCache()) {
+            return false;
+        }
+
+        try {
+            return historyCache.hasCacheForFile(file);
+        } catch (HistoryException ex) {
+            return false;
+        }
+    }
+
     /**
      * Check if we can annotate the specified file.
      *
@@ -392,7 +409,6 @@ private Collection<RepositoryInfo> addRepositories(File[] files,
                         }
                     }
                 } else {
-                    repository.setDirectoryName(path);
                     if (RuntimeEnvironment.getInstance().isVerbose()) {
                         LOGGER.log(Level.CONFIG, "Adding <{0}> repository: <{1}>",
                                 new Object[]{repository.getClass().getName(), path});
@@ -812,14 +828,7 @@ public void ensureHistoryCacheExists(File file) throws HistoryException {
     }
 
     protected Repository getRepository(File path) {
-        File file;
-
-        try {
-            file = path.getCanonicalFile();
-        } catch (IOException e) {
-            LOGGER.log(Level.WARNING, "Failed to get canonical path for " + path, e);
-            return null;
-        }
+        File file = path;
 
         while (file != null) {
             Repository r = repositories.get(file.getAbsolutePath());

test/org/opensolaris/opengrok/configuration/RuntimeEnvironmentTest.java

@@ -31,8 +31,12 @@
 import java.io.StringWriter;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
+import java.nio.file.Files;
+import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Date;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
@@ -61,6 +65,8 @@
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import org.opensolaris.opengrok.util.FileUtilities;
+import org.opensolaris.opengrok.util.IOUtils;
 
 /**
  * Test the RuntimeEnvironment class
@@ -1062,4 +1068,55 @@ public void testLoadNullStatistics() throws IOException, ParseException {
     public void testLoadNullStatisticsFile() throws IOException, ParseException {
         RuntimeEnvironment.getInstance().loadStatistics((File) null);
     }
+
+    /**
+     * Verify that getPathRelativeToSourceRoot() returns path relative to
+     * source root for both directories and symbolic links.
+     * @throws java.io.IOException
+     */
+    @Test
+    public void testGetPathRelativeToSourceRoot() throws IOException {
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+
+        // Create and set source root.
+        File sourceRoot = FileUtilities.createTemporaryDirectory("src");
+        assertTrue(sourceRoot.exists());
+        assertTrue(sourceRoot.isDirectory());
+        env.setSourceRoot(sourceRoot.getPath());
+
+        // Create directory underneath source root and check.
+        String filename = "foo";
+        File file = new File(env.getSourceRootFile(), filename);
+        file.createNewFile();
+        assertTrue(file.exists());
+        assertEquals(File.separator + filename,
+                env.getPathRelativeToSourceRoot(file));
+
+        // Create symlink underneath source root.
+        String symlinkName = "symlink";
+        File realDir = FileUtilities.createTemporaryDirectory("realdir");
+        File symlink = new File(sourceRoot, symlinkName);
+        Files.createSymbolicLink(
+                Paths.get(symlink.getPath()),
+                Paths.get(realDir.getPath()));
+        assertTrue(symlink.exists());
+        env.setAllowedSymlinks(new HashSet<>());
+        IOException exexp = null;
+        try {
+            env.getPathRelativeToSourceRoot(symlink);
+        } catch (IOException e) {
+            exexp = e;
+        }
+        assertTrue("getPathRelativeToSourceRoot() should have thrown " +
+            "IOexception for symlink that is not allowed", exexp != null);
+
+        // Allow the symlink and retest.
+        env.setAllowedSymlinks(new HashSet<>(Arrays.asList(symlink.getPath())));
+        assertEquals(File.separator + symlinkName,
+                env.getPathRelativeToSourceRoot(symlink));
+
+        // cleanup
+        IOUtils.removeRecursive(sourceRoot.toPath());
+        IOUtils.removeRecursive(realDir.toPath());
+    }
 }

test/org/opensolaris/opengrok/history/MercurialRepositoryTest.java

@@ -161,17 +161,16 @@ public void testGetHistoryPartial() throws Exception {
     /**
      * Run Mercurial command.
      *
-     * @param command hg command to run
      * @param reposRoot directory of the repository root
-     * @param arg argument to use for the command
+     * @param args {@code hg} command arguments
      */
     static public void runHgCommand(File reposRoot, String ... args) {
         List<String> cmdargs = new ArrayList<>();
         MercurialRepository repo = new MercurialRepository();
+
         cmdargs.add(repo.getRepoCommand());
-        for (String arg: args) {
-            cmdargs.add(arg);
-        }
+        cmdargs.addAll(Arrays.asList(args));
+
         Executor exec = new Executor(cmdargs, reposRoot);
         int exitCode = exec.exec();
         if (exitCode != 0) {

test/org/opensolaris/opengrok/index/IndexerRepoTest.java

@@ -22,19 +22,36 @@
  */
 package org.opensolaris.opengrok.index;
 
+import java.io.File;
 import static org.junit.Assert.assertEquals;
 
 import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
+import org.opensolaris.opengrok.condition.ConditionalRun;
+import org.opensolaris.opengrok.condition.RepositoryInstalled;
+import org.opensolaris.opengrok.configuration.Project;
 import org.opensolaris.opengrok.configuration.RuntimeEnvironment;
 import org.opensolaris.opengrok.history.HistoryGuru;
+import org.opensolaris.opengrok.history.MercurialRepositoryTest;
+import org.opensolaris.opengrok.history.RepositoryInfo;
+import org.opensolaris.opengrok.util.FileUtilities;
 import org.opensolaris.opengrok.util.TestRepository;
+import org.opensolaris.opengrok.util.IOUtils;
 
 /**
- * Test cleanup of renamed thread pool after indexing.
+ * Test indexer w.r.t. repositories.
  *
  * @author Vladimir Kotal
  */
@@ -73,6 +90,76 @@ private void checkNumberOfThreads() {
         }
     }
 
+    /**
+     * Test that symlinked directories from source root get their relative
+     * path set correctly in RepositoryInfo objects.
+     * @throws org.opensolaris.opengrok.index.IndexerException
+     * @throws java.io.IOException
+     */
+    @ConditionalRun(condition = RepositoryInstalled.MercurialInstalled.class)
+    @Test
+    public void testSymlinks() throws IndexerException, IOException {
+
+        final String symlink = "symlink";
+        RuntimeEnvironment env = RuntimeEnvironment.getInstance();
+
+        // Set source root to pristine directory so that there is only one
+        // repository to deal with (which makes this faster and easier to write)
+        // and clone the mercurial repository outside of the source root.
+        File realSource = FileUtilities.createTemporaryDirectory("real");
+        File sourceRoot = FileUtilities.createTemporaryDirectory("src");
+        MercurialRepositoryTest.runHgCommand(sourceRoot,
+                "clone", repository.getSourceRoot() + File.separator + "mercurial",
+                realSource.getPath());
+
+        // Create symlink from source root to the real repository.
+        String symlinkPath = sourceRoot.toString() + File.separator + symlink;
+        Files.createSymbolicLink(Paths.get(symlinkPath), Paths.get(realSource.getPath()));
+
+        env.setSourceRoot(sourceRoot.toString());
+        env.setDataRoot(repository.getDataRoot());
+        // Need to have history cache enabled in order to perform scan of repositories.
+        env.setHistoryEnabled(true);
+        // Normally the Indexer would add the symlink automatically.
+        env.setAllowedSymlinks(new HashSet<String>(Arrays.asList(symlinkPath)));
+
+        // Do a rescan of the projects, and only that (we don't care about
+        // the other aspects of indexing in this test case).
+        Indexer.getInstance().prepareIndexer(
+                env,
+                true, // search for repositories
+                true, // scan and add projects
+                null, // no default project
+                false, // don't list files
+                false, // don't create dictionary
+                null, // subFiles - not needed since we don't list files
+                null, // repositories - not needed when not refreshing history
+                new ArrayList<>(), // don't zap cache
+                false); // don't list repos
+
+        // Check the respository paths.
+        List<RepositoryInfo> repos = env.getRepositories();
+        assertEquals(repos.size(), 1);
+        RepositoryInfo repo = repos.get(0);
+        assertEquals("/" + symlink, repo.getDirectoryNameRelative());
+        assertEquals(sourceRoot.toString() + File.separator + "symlink",
+                repo.getDirectoryName());
+
+        // Check that history exists for a file in the repository.
+        File repoRoot = new File(env.getSourceRootFile(), symlink);
+        File fileInRepo = new File(repoRoot, "main.c");
+        assertTrue(fileInRepo.exists());
+        assertTrue(HistoryGuru.getInstance().hasHistory(fileInRepo));
+        assertTrue(HistoryGuru.getInstance().hasCacheForFile(fileInRepo));
+
+        // cleanup
+        IOUtils.removeRecursive(realSource.toPath());
+        IOUtils.removeRecursive(sourceRoot.toPath());
+    }
+
+    /**
+     * Test cleanup of renamed thread pool after indexing with -H.
+     */
     @Test
     public void testMainWithH() throws IOException {
         System.out.println("Generate index by using command line options with -H");
@@ -88,6 +175,9 @@ public void testMainWithH() throws IOException {
         }
     }
 
+    /**
+     * Test cleanup of renamed thread pool after indexing without -H.
+     */
     @Test
     public void testMainWithoutH() throws IOException {
         System.out.println("Generate index by using command line options without -H");