From 8ed071e551478184d56eed924cadf8d8160801d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Tulinger?= Date: Tue, 31 Jan 2017 00:34:37 +0100 Subject: [PATCH 1/5] using include and ignore names in history generation fixes #1351 fixes #1315 --- .../opengrok/history/FileHistoryCache.java | 38 ++- .../opengrok/index/IndexDatabase.java | 151 +---------- .../opengrok/util/AcceptHelper.java | 237 ++++++++++++++++++ .../opensolaris/opengrok/web/PageConfig.java | 11 +- web/history.jsp | 8 +- 5 files changed, 292 insertions(+), 153 deletions(-) create mode 100644 src/org/opensolaris/opengrok/util/AcceptHelper.java diff --git a/src/org/opensolaris/opengrok/history/FileHistoryCache.java b/src/org/opensolaris/opengrok/history/FileHistoryCache.java index fb0ffa1541e..e8cdf25ed91 100644 --- a/src/org/opensolaris/opengrok/history/FileHistoryCache.java +++ b/src/org/opensolaris/opengrok/history/FileHistoryCache.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2008, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved. */ package org.opensolaris.opengrok.history; @@ -52,8 +52,10 @@ import java.util.logging.Logger; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; +import org.opensolaris.opengrok.configuration.Project; import org.opensolaris.opengrok.configuration.RuntimeEnvironment; import org.opensolaris.opengrok.logger.LoggerFactory; +import org.opensolaris.opengrok.util.AcceptHelper; import org.opensolaris.opengrok.util.IOUtils; /* @@ -364,6 +366,29 @@ public void store(History history, Repository repository) continue; } + if (env.hasProjects()) { + /* + * Problem here is that if the original file was a symlink + * it's been already dereferenced by + * getPathRelativeToSourceRoot(). We have to solve the only + * case which is that the symlink led to the project's root. + * + * @see + * RuntimeEnvironment#getPathRelativeToSourceRoot(java.io.File, int) + */ + Project project = Project.getProject(test); + File parent = test.equals(new File(env.getSourceRootPath(), project.getPath())) + /* this is a project's root */ + ? test + /* this isn't a project's root */ + : test.getParentFile(); + if (!AcceptHelper.accept(project, parent, test)) { + continue; + } + } else if (!AcceptHelper.accept(null, test.getParentFile(), test)) { + continue; + } + List list = map.get(s); if (list == null) { list = new ArrayList<>(); @@ -380,7 +405,7 @@ public void store(History history, Repository repository) } } } - + /* * Now traverse the list of files from the hash map built above * and for each file store its history (saved in the value of the @@ -492,6 +517,10 @@ public History get(File file, Repository repository, boolean withFiles) return null; } + if (!AcceptHelper.accept(Project.getProject(file), file)) { + return null; + } + final History history; long time; try { @@ -584,6 +613,7 @@ private String getRepositoryCachedRevPath(Repository repository) { /** * Store latest indexed revision for the repository under data directory. + * * @param repository repository * @param rev latest revision which has been just indexed */ @@ -592,11 +622,11 @@ private void storeLatestCachedRevision(Repository repository, String rev) { try { writer = new BufferedWriter(new OutputStreamWriter( - new FileOutputStream(getRepositoryCachedRevPath(repository)))); + new FileOutputStream(getRepositoryCachedRevPath(repository)))); writer.write(rev); } catch (IOException ex) { LOGGER.log(Level.WARNING, "Cannot write latest cached revision to file for "+repository.getDirectoryName(), - ex); + ex); } finally { try { if (writer != null) { diff --git a/src/org/opensolaris/opengrok/index/IndexDatabase.java b/src/org/opensolaris/opengrok/index/IndexDatabase.java index 728ceabe3cb..ae29f452dab 100644 --- a/src/org/opensolaris/opengrok/index/IndexDatabase.java +++ b/src/org/opensolaris/opengrok/index/IndexDatabase.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2008, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved. */ package org.opensolaris.opengrok.index; @@ -76,6 +76,7 @@ import org.opensolaris.opengrok.history.HistoryGuru; import org.opensolaris.opengrok.logger.LoggerFactory; import org.opensolaris.opengrok.search.QueryBuilder; +import org.opensolaris.opengrok.util.AcceptHelper; import org.opensolaris.opengrok.util.IOUtils; import org.opensolaris.opengrok.web.Util; @@ -665,150 +666,6 @@ private void cleanupResources(Document doc) { } } - /** - * Check if I should accept this file into the index database - * - * @param file the file to check - * @return true if the file should be included, false otherwise - */ - private boolean accept(File file) { - - if (!includedNames.isEmpty() - && // the filter should not affect directory names - (!(file.isDirectory() || includedNames.match(file)))) { - return false; - } - - String absolutePath = file.getAbsolutePath(); - - if (ignoredNames.ignore(file)) { - LOGGER.log(Level.FINER, "ignoring {0}", absolutePath); - return false; - } - - if (!file.canRead()) { - LOGGER.log(Level.WARNING, "Could not read {0}", absolutePath); - return false; - } - - try { - String canonicalPath = file.getCanonicalPath(); - if (!absolutePath.equals(canonicalPath) - && !acceptSymlink(absolutePath, canonicalPath)) { - - LOGGER.log(Level.FINE, "Skipped symlink ''{0}'' -> ''{1}''", - new Object[]{absolutePath, canonicalPath}); - return false; - } - //below will only let go files and directories, anything else is considered special and is not added - if (!file.isFile() && !file.isDirectory()) { - LOGGER.log(Level.WARNING, "Ignored special file {0}", - absolutePath); - return false; - } - } catch (IOException exp) { - LOGGER.log(Level.WARNING, "Failed to resolve name: {0}", - absolutePath); - LOGGER.log(Level.FINE, "Stack Trace: ", exp); - } - - if (file.isDirectory()) { - // always accept directories so that their files can be examined - return true; - } - - if (HistoryGuru.getInstance().hasHistory(file)) { - // versioned files should always be accepted - return true; - } - - // this is an unversioned file, check if it should be indexed - return !RuntimeEnvironment.getInstance().isIndexVersionedFilesOnly(); - } - - boolean accept(File parent, File file) { - try { - File f1 = parent.getCanonicalFile(); - File f2 = file.getCanonicalFile(); - if (f1.equals(f2)) { - LOGGER.log(Level.INFO, "Skipping links to itself...: {0} {1}", - new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()}); - return false; - } - - // Now, let's verify that it's not a link back up the chain... - File t1 = f1; - while ((t1 = t1.getParentFile()) != null) { - if (f2.equals(t1)) { - LOGGER.log(Level.INFO, "Skipping links to parent...: {0} {1}", - new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()}); - return false; - } - } - - return accept(file); - } catch (IOException ex) { - LOGGER.log(Level.WARNING, "Failed to resolve name: {0} {1}", - new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()}); - } - return false; - } - - /** - * Check if I should accept the path containing a symlink - * - * @param absolutePath the path with a symlink to check - * @param canonicalPath the canonical path to the file - * @return true if the file should be accepted, false otherwise - */ - private boolean acceptSymlink(String absolutePath, String canonicalPath) throws IOException { - // Always accept local symlinks - if (isLocal(canonicalPath)) { - return true; - } - - for (String allowedSymlink : RuntimeEnvironment.getInstance().getAllowedSymlinks()) { - if (absolutePath.startsWith(allowedSymlink)) { - String allowedTarget = new File(allowedSymlink).getCanonicalPath(); - if (canonicalPath.startsWith(allowedTarget) - && absolutePath.substring(allowedSymlink.length()).equals(canonicalPath.substring(allowedTarget.length()))) { - return true; - } - } - } - return false; - } - - /** - * Check if a file is local to the current project. If we don't have - * projects, check if the file is in the source root. - * - * @param path the path to a file - * @return true if the file is local to the current repository - */ - private boolean isLocal(String path) { - RuntimeEnvironment env = RuntimeEnvironment.getInstance(); - String srcRoot = env.getSourceRootPath(); - - boolean local = false; - - if (path.startsWith(srcRoot)) { - if (env.hasProjects()) { - String relPath = path.substring(srcRoot.length()); - if (project.equals(Project.getProject(relPath))) { - // File is under the current project, so it's local. - local = true; - } - } else { - // File is under source root, and we don't have projects, so - // consider it local. - local = true; - } - } - - return local; - } - /** * Generate indexes recursively * @@ -828,7 +685,7 @@ private int indexDown(File dir, String parent, boolean count_only, return lcur_count; } - if (!accept(dir)) { + if (!AcceptHelper.accept(project, dir)) { return lcur_count; } @@ -841,7 +698,7 @@ private int indexDown(File dir, String parent, boolean count_only, Arrays.sort(files, fileComparator); for (File file : files) { - if (accept(dir, file)) { + if (AcceptHelper.accept(project, dir, file)) { String path = parent + '/' + file.getName(); if (file.isDirectory()) { diff --git a/src/org/opensolaris/opengrok/util/AcceptHelper.java b/src/org/opensolaris/opengrok/util/AcceptHelper.java new file mode 100644 index 00000000000..5ac655ae295 --- /dev/null +++ b/src/org/opensolaris/opengrok/util/AcceptHelper.java @@ -0,0 +1,237 @@ +/* + * 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) 2017, Oracle and/or its affiliates. All rights reserved. + */ +package org.opensolaris.opengrok.util; + +import java.io.File; +import java.io.IOException; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.opensolaris.opengrok.configuration.Project; +import org.opensolaris.opengrok.configuration.RuntimeEnvironment; +import org.opensolaris.opengrok.history.HistoryGuru; +import org.opensolaris.opengrok.logger.LoggerFactory; + +/** + * + * @author Krystof Tulinger + */ +public class AcceptHelper { + + private static final Logger LOGGER = LoggerFactory.getLogger(AcceptHelper.class); + + /** + * Check if I should accept this file into the database. This method has the + * same effect as the call accept(null, file). + * + * @see AcceptHelper#accept(Project, File) + * + * @param file the file to check + * @return true if the file should be included, false otherwise + */ + public static boolean accept(File file) { + return accept((Project) null, file); + } + + /** + * Check if I should accept this file into the database + * + * @param project if the file is relevant to some project (may be null) + * @param file the file to check + * @return true if the file should be included, false otherwise + */ + public static boolean accept(Project project, File file) { + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + if (!env.getIncludedNames().isEmpty() // we have collected some included names + && !file.isDirectory() // the filter should not affect directory names + && !env.getIncludedNames().match(file)) { + return false; + } + + String absolutePath = file.getAbsolutePath(); + + if (env.getIgnoredNames().ignore(file)) { + LOGGER.log(Level.FINER, "ignoring {0}", absolutePath); + return false; + } + + if (!file.canRead()) { + LOGGER.log(Level.WARNING, "Could not read {0}", absolutePath); + return false; + } + + try { + String canonicalPath = file.getCanonicalPath(); + if (!absolutePath.equals(canonicalPath) + && !acceptSymlink(project, file)) { + + LOGGER.log(Level.FINE, "Skipped symlink ''{0}'' -> ''{1}''", + new Object[]{absolutePath, canonicalPath}); + return false; + } + //below will only let go files and directories, anything else is considered special and is not added + if (!file.isFile() && !file.isDirectory()) { + LOGGER.log(Level.WARNING, "Ignored special file {0}", + absolutePath); + return false; + } + } catch (IOException exp) { + LOGGER.log(Level.WARNING, "Failed to resolve name: {0}", + absolutePath); + LOGGER.log(Level.FINE, "Stack Trace: ", exp); + } + + if (file.isDirectory()) { + // always accept directories so that their files can be examined + return true; + } + + if (HistoryGuru.getInstance().hasHistory(file)) { + // versioned files should always be accepted + return true; + } + + // this is an unversioned file, check if it should be indexed + return !env.isIndexVersionedFilesOnly(); + } + + /** + * Check if this file should be accepted. This method has the same effect as + * the call accept(null, parent, file). + * + * @see AcceptHelper#accept(Project, File, File) + * @param parent parent directory of the file + * @param file the file to check + * @return true if the file should be included, false otherwise + */ + public static boolean accept(File parent, File file) { + return accept(null, parent, file); + } + + /** + * Check if this file should be accepted. + * + * @param project if the file is relevant to some project (may be null) + * @param parent parent directory of the file + * @param file the file to check + * @return true if the file should be included, false otherwise + */ + public static boolean accept(Project project, File parent, File file) { + try { + File f1 = parent.getCanonicalFile(); + File f2 = file.getCanonicalFile(); + if (f2.equals(f1)) { + LOGGER.log(Level.INFO, "Skipping links to itself...: link '{1}' -> '{0}'", + new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()}); + return false; + } + + // Now, let's verify that it's not a link back up the chain... + File t1 = f1; + while ((t1 = t1.getParentFile()) != null) { + if (f2.equals(t1)) { + LOGGER.log(Level.INFO, "Skipping links to parent...: link '{1}' -> '{0}'", + new Object[]{t1.getAbsolutePath(), file.getAbsolutePath()}); + return false; + } + } + + return accept(project, file); + } catch (IOException ex) { + LOGGER.log(Level.WARNING, "Failed to resolve name: parent '{0}' file '{1}'", + new Object[]{parent.getAbsolutePath(), file.getAbsolutePath()}); + } + return false; + } + + /** + * Check if I should accept the path containing a symlink. This method has + * the same effect as the call acceptSymlink(null, file). + * + * @see AcceptHelper#acceptSymlink(Project, File) + * @param file the file symlink + * @return true if the file should be accepted, false otherwise + * @throws java.io.IOException + */ + public static boolean acceptSymlink(File file) throws IOException { + return acceptSymlink(null, file); + } + + /** + * Check if I should accept the path containing a symlink. + * + * @param project if the file is relevant to some project (may be null) + * @param file the file symlink + * @return true if the file should be accepted, false otherwise + * @throws java.io.IOException + */ + public static boolean acceptSymlink(Project project, File file) throws IOException { + String absolutePath = file.getAbsolutePath(), canonicalPath = file.getCanonicalPath(); + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + + // Always accept local symlinks + if (isLocal(project, canonicalPath)) { + return true; + } + + for (String allowedSymlink : env.getAllowedSymlinks()) { + if (absolutePath.startsWith(allowedSymlink)) { + String allowedTarget = new File(allowedSymlink).getCanonicalPath(); + if (canonicalPath.startsWith(allowedTarget) + && absolutePath.substring(allowedSymlink.length()).equals(canonicalPath.substring(allowedTarget.length()))) { + // accept a file which is under the allowed (and dereferenced) symlink + return true; + } + } + } + return false; + } + + /** + * Check if a file is local to the current project. If we don't have + * projects, check if the file is in the source root. + * + * @param project a project (may be null) + * @param path the path to a file + * @return true if the file is local to the current repository + */ + public static boolean isLocal(Project project, String path) { + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + String srcRoot = env.getSourceRootPath(); + + if (path.startsWith(srcRoot)) { + if (env.hasProjects()) { + if (project != null + && project.equals(Project.getProject(path.substring(srcRoot.length())))) { + // File is under the current project, so it's local. + return true; + } + } else { + // File is under source root, and we don't have projects, so + // consider it local. + return true; + } + } + + return false; + } +} diff --git a/src/org/opensolaris/opengrok/web/PageConfig.java b/src/org/opensolaris/opengrok/web/PageConfig.java index c9ccd2cd2db..b7acf5d82e7 100644 --- a/src/org/opensolaris/opengrok/web/PageConfig.java +++ b/src/org/opensolaris/opengrok/web/PageConfig.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2011, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2017, Oracle and/or its affiliates. All rights reserved. * Portions copyright (c) 2011 Jens Elkner. */ package org.opensolaris.opengrok.web; @@ -604,6 +604,15 @@ public boolean hasHistory() { return hasHistory; } + /** + * Set if the request related resource has history information. + * + * @param history boolean to set + */ + public void hasHistory(boolean history) { + hasHistory = history; + } + /** * Check, whether annotations are available for the related resource. * diff --git a/web/history.jsp b/web/history.jsp index 4e4856f0ac0..d2720c4393f 100644 --- a/web/history.jsp +++ b/web/history.jsp @@ -85,7 +85,13 @@ include file="httpheader.jspf" // should not happen %>

Problem

<%= e.getMessage() %>

<% } - if (hist != null) { + + if (hist == null) { + /* No history for this file */ + cfg.hasHistory(false); + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } else { %> <% // We have a lots of results to show: create a slider for From 8ad37b1eb93440204ad0e1b45cedbb2daf86337f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Tulinger?= Date: Tue, 31 Jan 2017 00:34:56 +0100 Subject: [PATCH 2/5] setting page name for error pages --- web/enoent.jsp | 4 ++-- web/error.jsp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/web/enoent.jsp b/web/enoent.jsp index d340c0b89ea..862d604f131 100644 --- a/web/enoent.jsp +++ b/web/enoent.jsp @@ -16,7 +16,7 @@ information: Portions Copyright [yyyy] [name of copyright owner] CDDL HEADER END -Copyright (c) 2005, 2016, Oracle and/or its affiliates. All rights reserved. +Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved. Portions Copyright 2011 Jens Elkner. --%><%@page session="false" isErrorPage="true" import=" @@ -53,7 +53,7 @@ include file="httpheader.jspf" include file="pageheader.jspf" %> -
+
OpenGrok error
<%@ include file="menu.jspf" diff --git a/web/error.jsp b/web/error.jsp index 1ae40beebde..6c6ad186cea 100644 --- a/web/error.jsp +++ b/web/error.jsp @@ -60,7 +60,7 @@ include file="pageheader.jspf" %>
-
+
OpenGrok error
<%@ include file="menu.jspf" From f0dd85b110056913daa210e9c5b34d513139ae68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Tulinger?= Date: Tue, 31 Jan 2017 00:35:25 +0100 Subject: [PATCH 3/5] making sure the history cache is created before the last revision file hopefully fixes #1297 --- .../opengrok/history/FileHistoryCache.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/org/opensolaris/opengrok/history/FileHistoryCache.java b/src/org/opensolaris/opengrok/history/FileHistoryCache.java index e8cdf25ed91..fd88a4741c0 100644 --- a/src/org/opensolaris/opengrok/history/FileHistoryCache.java +++ b/src/org/opensolaris/opengrok/history/FileHistoryCache.java @@ -619,13 +619,21 @@ private String getRepositoryCachedRevPath(Repository repository) { */ private void storeLatestCachedRevision(Repository repository, String rev) { Writer writer = null; - + File file = new File(getRepositoryHistDataDirname(repository)); + if (!file.exists() || !file.isDirectory()) { + if (!file.mkdirs()) { + LOGGER.log(Level.WARNING, + "Cannot create the history cache directory to write the latest cached revision for {}", + repository.getDirectoryName()); + } + } try { writer = new BufferedWriter(new OutputStreamWriter( new FileOutputStream(getRepositoryCachedRevPath(repository)))); writer.write(rev); } catch (IOException ex) { - LOGGER.log(Level.WARNING, "Cannot write latest cached revision to file for "+repository.getDirectoryName(), + LOGGER.log(Level.WARNING, + "Cannot write latest cached revision to file for " + repository.getDirectoryName(), ex); } finally { try { From c2d1b726fb31f4353b300d7f29a26131f5fe0510 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Tulinger?= Date: Mon, 6 Feb 2017 17:38:33 +0100 Subject: [PATCH 4/5] adding a test case for accept files --- .../opengrok/util/AcceptHelperTest.java | 381 ++++++++++++++++++ 1 file changed, 381 insertions(+) create mode 100644 test/org/opensolaris/opengrok/util/AcceptHelperTest.java diff --git a/test/org/opensolaris/opengrok/util/AcceptHelperTest.java b/test/org/opensolaris/opengrok/util/AcceptHelperTest.java new file mode 100644 index 00000000000..66aa6b36f85 --- /dev/null +++ b/test/org/opensolaris/opengrok/util/AcceptHelperTest.java @@ -0,0 +1,381 @@ +/* + * 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) 2017, Oracle and/or its affiliates. All rights reserved. + */ +package org.opensolaris.opengrok.util; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.opensolaris.opengrok.configuration.Project; +import org.opensolaris.opengrok.configuration.RuntimeEnvironment; +import org.opensolaris.opengrok.index.IgnoredNames; + +/** + * + * @author Krystof Tulinger + */ +public class AcceptHelperTest { + + private File tempDir; + + @Before + public void setUp() throws IOException { + tempDir = File.createTempFile("temp", Long.toString(System.currentTimeMillis())); + if (!tempDir.delete()) { + throw new IOException("Could not delete temporary file to create a directory: " + tempDir.getAbsolutePath()); + } + if (!tempDir.mkdir()) { + throw new IOException("Could not create a temporary directory: " + tempDir.getAbsolutePath()); + } + } + + @After + public void tearDown() throws IOException { + IOUtils.removeRecursive(tempDir.toPath()); + } + + protected void runAcceptTests(File sourceRoot, File[] tests, boolean accept) throws IOException { + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + IgnoredNames ignoredNames = env.getIgnoredNames(); + + createSourceRoot(sourceRoot); + String oldSourceRoot = env.getSourceRootPath(); + env.setSourceRoot(sourceRoot.getAbsolutePath()); + + IgnoredNames newIgnored = new IgnoredNames(); + newIgnored.add("f:inside.c"); + newIgnored.add("d:foo"); + newIgnored.add("d:subdir"); + newIgnored.add("d:ignoredDir"); + newIgnored.add("*/ignored/*"); + newIgnored.add("foolink"); + env.setIgnoredNames(newIgnored); + + for (File test : tests) { + Assert.assertEquals(accept, AcceptHelper.accept(test)); + } + + env.setIgnoredNames(ignoredNames); + if (oldSourceRoot != null) { + env.setSourceRoot(oldSourceRoot); + } + } + + /** + * Test of accept method, of class AcceptHelper. + */ + @Test + public void testAcceptIgnored() throws IOException { + File sourceRoot = new File(tempDir.getAbsolutePath(), "source"); + + File ignored[] = new File[]{ + createFile(sourceRoot + File.separator + "inside.c"), + createFile(sourceRoot + File.separator + "subdir" + File.separator + "inside.c"), + createFile(sourceRoot + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), + createFile(sourceRoot + File.separator + "ignored" + File.separator + "main.c"), + createFile(sourceRoot + File.separator + "ignored" + File.separator + "random.c"), + createDirectory(sourceRoot + File.separator + "ignoredDir"), + createDirectory(sourceRoot + File.separator + "subdir" + File.separator + "ignoredDir") + }; + + createFile(sourceRoot + File.separator + "main.c"); + createFile(sourceRoot + File.separator + "random.c"); + createFile(sourceRoot + File.separator + "subdir" + File.separator + "main.c"); + createFile(sourceRoot + File.separator + "subdir" + File.separator + "random.c"); + createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "link"); + createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "subdir" + File.separator + "link"); + createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "foolink"); + createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "subdir" + File.separator + "foolink"); + + runAcceptTests(sourceRoot, ignored, false); + IOUtils.removeRecursive(sourceRoot.toPath()); + } + + /** + * Test of accept method, of class AcceptHelper. + */ + @Test + public void testAcceptAccepted() throws IOException { + + File sourceRoot = new File(tempDir.getAbsolutePath(), "source"); + + createFile(sourceRoot + File.separator + "inside.c"); + createFile(sourceRoot + File.separator + "subdir" + File.separator + "inside.c"); + createFile(sourceRoot + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"); + createFile(sourceRoot + File.separator + "ignored" + File.separator + "main.c"); + createFile(sourceRoot + File.separator + "ignored" + File.separator + "random.c"); + createDirectory(sourceRoot + File.separator + "ignoredDir"); + createDirectory(sourceRoot + File.separator + "subdir" + File.separator + "ignoredDir"); + + File accepted[] = new File[]{ + createFile(sourceRoot + File.separator + "main.c"), + createFile(sourceRoot + File.separator + "random.c"), + createFile(sourceRoot + File.separator + "subdir" + File.separator + "main.c"), + createFile(sourceRoot + File.separator + "subdir" + File.separator + "random.c"), + createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "link"), + createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "subdir" + File.separator + "link"), + createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "foolink"), + createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "subdir" + File.separator + "foolink") + }; + + runAcceptTests(sourceRoot, accepted, true); + IOUtils.removeRecursive(sourceRoot.toPath()); + } + + /** + * Test of acceptSymlink method, of class AcceptHelper. + */ + @Test + public void testAcceptSymlink() throws IOException { + File sourceRoot = createSourceRoot(new File(tempDir.getAbsolutePath(), "source/root")); + File symlinkRoot = sourceRoot.getParentFile(); + + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + Set allowedSymlinks = env.getAllowedSymlinks(); + + File t1 = createFile(symlinkRoot + File.separator + "file1.c"); + File t2 = createFile(symlinkRoot + File.separator + "file2.c"); + File t3 = createFile(symlinkRoot + File.separator + "file3.c"); + + env.setAllowedSymlinks(new TreeSet<>(Arrays.asList(new String[]{ + sourceRoot + File.separator + "link1", + sourceRoot + File.separator + "link2", + sourceRoot + File.separator + "link3" + }))); + + File[] tests = new File[]{ + createSymlink(t1.getAbsolutePath(), sourceRoot + File.separator + "link1"), + createSymlink(t2.getAbsolutePath(), sourceRoot + File.separator + "link2"), + createSymlink(t3.getAbsolutePath(), sourceRoot + File.separator + "link3") + }; + runSymlinkTests(sourceRoot, tests, true); + + env.setAllowedSymlinks(new TreeSet<>()); + runSymlinkTests(sourceRoot, tests, false); + + env.setAllowedSymlinks(allowedSymlinks); + IOUtils.removeRecursive(sourceRoot.toPath()); + } + + private void runSymlinkTests(File sourceRoot, File[] tests, boolean expected) throws IOException { + createSourceRoot(sourceRoot); + + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + String oldSourceRoot = env.getSourceRootPath(); + env.setSourceRoot(sourceRoot.getAbsolutePath()); + + for (File file : tests) { + Assert.assertEquals(expected, AcceptHelper.acceptSymlink(null, file)); + } + + if (oldSourceRoot != null) { + env.setSourceRoot(oldSourceRoot); + } + } + + /** + * + * @param sourceRoot + * @param tests + * @throws IOException + */ + protected void runLocalTests(File sourceRoot, Object[][] tests) throws IOException { + createSourceRoot(sourceRoot); + + RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + List projects = env.getProjects(); + String oldSourceRoot = env.getSourceRootPath(); + env.setProjects(new ArrayList<>()); + env.setSourceRoot(sourceRoot.getAbsolutePath()); + + for (Object[] test : tests) { + Project project = (Project) test[0]; + if (project != null && !env.getProjects().contains(project)) { + env.getProjects().add(project); + } + File file = (File) test[1]; + Boolean expected = (Boolean) test[2]; + Assert.assertEquals(expected.booleanValue(), AcceptHelper.isLocal(project, file.getCanonicalPath())); + } + + env.setProjects(projects); + if (oldSourceRoot != null) { + env.setSourceRoot(oldSourceRoot); + } + } + + protected File createSourceRoot(File sourceRoot) throws IOException { + if (!sourceRoot.exists() && !sourceRoot.mkdirs()) { + throw new IOException("Could not create a testing source root: " + sourceRoot.getAbsolutePath()); + } + return sourceRoot; + } + + /** + * Test of isLocal method, of class AcceptHelper. + * + * @throws java.io.IOException + */ + @Test + public void testIsLocalWithoutProjects() throws IOException { + File sourceRoot = new File(tempDir.getAbsolutePath(), "source"); + Object[][] tests = new Object[][]{ + {null, createFile(sourceRoot + File.separator + "inside.c"), true}, + {null, createFile(sourceRoot + File.separator + "subdir" + File.separator + "inside.c"), true}, + {null, createFile(sourceRoot + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), true}, + {null, createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "link"), true}, + {null, createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "subdir" + File.separator + "link"), true}, + {null, createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "foolink"), true}, + {null, createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "subdir" + File.separator + "foolink"), true} + }; + runLocalTests(sourceRoot, tests); + IOUtils.removeRecursive(sourceRoot.toPath()); + } + + /** + * Test of isLocal method, of class AcceptHelper. + * + * @throws java.io.IOException + */ + @Test + public void testIsLocalWithProjects() throws IOException { + File sourceRoot = new File(tempDir.getAbsolutePath(), "source/subroot"); + Object[][] tests = new Object[][]{ + {createProject("/project1"), createFile(sourceRoot + File.separator + "project1" + File.separator + "inside.c"), true}, + {createProject("/project1"), createFile(sourceRoot + File.separator + "project1" + File.separator + "subdir" + File.separator + "inside.c"), true}, + {createProject("/project1"), createFile(sourceRoot + File.separator + "project1" + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), true}, + {createProject("/project1"), createSymlink(sourceRoot + File.separator + "project1" + File.separator + "subdir", sourceRoot + File.separator + "project1" + File.separator + "link"), true}, + {createProject("/project1"), createSymlink(sourceRoot + File.separator + "project1" + File.separator + "subdir", sourceRoot + File.separator + "project1" + File.separator + "subdir" + File.separator + "link"), true}, + {createProject("/project1"), createSymlink(sourceRoot.getAbsolutePath() + File.separator + "project1", sourceRoot + File.separator + "project1" + File.separator + "foolink"), true}, + {createProject("/project1"), createSymlink(sourceRoot.getAbsolutePath() + File.separator + "project1", sourceRoot + File.separator + "project1" + File.separator + "subdir" + File.separator + "foolink"), true}, + {createProject("/project2"), createFile(sourceRoot + File.separator + "project2" + File.separator + "inside.c"), true}, + {createProject("/project2"), createFile(sourceRoot + File.separator + "project2" + File.separator + "subdir" + File.separator + "inside.c"), true}, + {createProject("/project2"), createFile(sourceRoot + File.separator + "project2" + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), true}, + {createProject("/project2"), createSymlink(sourceRoot + File.separator + "project2" + File.separator + "subdir", sourceRoot + File.separator + "project2" + File.separator + "link"), true}, + {createProject("/project2"), createSymlink(sourceRoot + File.separator + "project2" + File.separator + "subdir", sourceRoot + File.separator + "project2" + File.separator + "subdir" + File.separator + "link"), true}, + {createProject("/project2"), createSymlink(sourceRoot.getAbsolutePath() + File.separator + "project2", sourceRoot + File.separator + "project2" + File.separator + "foolink"), true}, + {createProject("/project2"), createSymlink(sourceRoot.getAbsolutePath() + File.separator + "project2", sourceRoot + File.separator + "project2" + File.separator + "subdir" + File.separator + "foolink"), true} + }; + runLocalTests(sourceRoot, tests); + } + + /** + * Test of isLocal method, of class AcceptHelper. + * + * @throws java.io.IOException + */ + @Test + public void testIsNotLocalWithProjects() throws IOException { + File sourceRoot = new File(tempDir.getAbsolutePath(), "source/subroot"); + Object[][] tests = new Object[][]{ + {createProject("/project1"), createFile(sourceRoot + File.separator + "inside.c"), false}, + {createProject("/project1"), createFile(sourceRoot + File.separator + "subdir" + File.separator + "inside.c"), false}, + {createProject("/project1"), createFile(sourceRoot + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), false}, + {createProject("/project1"), createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "link"), false}, + {createProject("/project1"), createSymlink(sourceRoot + File.separator + "subdir", sourceRoot + File.separator + "subdir" + File.separator + "link"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "foolink"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getAbsolutePath(), sourceRoot + File.separator + "subdir" + File.separator + "foolink"), false}, + {createProject("/project1"), createFile(sourceRoot.getParentFile() + File.separator + "inside.c"), false}, + {createProject("/project1"), createFile(sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "inside.c"), false}, + {createProject("/project1"), createFile(sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getParentFile() + File.separator + "subdir", sourceRoot.getParentFile() + File.separator + "link"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getParentFile() + File.separator + "subdir", sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "link"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getParentFile().getAbsolutePath(), sourceRoot.getParentFile() + File.separator + "foolink"), false}, + {createProject("/project1"), createSymlink(sourceRoot.getParentFile().getAbsolutePath(), sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "foolink"), false}, + {createProject("/project1"), createFile(sourceRoot + File.separator + "project2" + File.separator + "subdir" + File.separator + "main.c"), false}, + {createProject("/project2"), createFile(sourceRoot + File.separator + "project1" + File.separator + "main.c"), false}, + {createProject("/project1"), createSymlink(sourceRoot + File.separator + "project2", sourceRoot + File.separator + "project1" + File.separator + "external"), false} + }; + runLocalTests(sourceRoot, tests); + } + + /** + * Test of isLocal method, of class AcceptHelper. + * + * @throws java.io.IOException + */ + @Test + public void testIsNotLocalWithoutProjects() throws IOException { + File sourceRoot = new File(tempDir.getAbsolutePath(), "source/subroot"); + Object[][] tests = new Object[][]{ + {null, createFile(sourceRoot.getParentFile() + File.separator + "inside.c"), false}, + {null, createFile(sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "inside.c"), false}, + {null, createFile(sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "subdir" + File.separator + "inside.c"), false}, + {null, createSymlink(sourceRoot.getParentFile() + File.separator + "subdir", sourceRoot.getParentFile() + File.separator + "link"), false}, + {null, createSymlink(sourceRoot.getParentFile() + File.separator + "subdir", sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "link"), false}, + {null, createSymlink(sourceRoot.getParentFile().getAbsolutePath(), sourceRoot.getParentFile() + File.separator + "foolink"), false}, + {null, createSymlink(sourceRoot.getParentFile().getAbsolutePath(), sourceRoot.getParentFile() + File.separator + "subdir" + File.separator + "foolink"), false} + }; + runLocalTests(sourceRoot, tests); + } + + protected Project createProject(String path) { + Project p = new Project(); + p.setDescription(path); + p.setPath(path); + return p; + } + + protected File createSymlink(String target, String name) throws IOException { + Path link = Files.createSymbolicLink( + new File(name).toPath(), + new File(target).toPath()); + return link.toFile(); + } + + protected File createFile(String path) throws IOException { + return createFile(path, false); + } + + protected File createDirectory(String path) throws IOException { + return createFile(path, true); + } + + protected File createFile(String path, boolean directory) throws IOException { + File test = new File(path); + if (!test.exists()) { + if (!test.getParentFile().exists() && !test.getParentFile().mkdirs()) { + throw new IOException("Could not create the path: " + test.getParentFile().getAbsolutePath()); + } + if (directory) { + if (!test.mkdir()) { + throw new IOException("Could not create the directory: " + test.getAbsolutePath()); + } + } else { + if (!test.createNewFile()) { + throw new IOException("Could not create the file: " + test.getAbsolutePath()); + } + } + } + return test; + } +} From e90286a4f60a5d79be3a33a33e3b18571676381f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Tulinger?= Date: Tue, 7 Feb 2017 14:37:00 +0100 Subject: [PATCH 5/5] improving performance when ignoring files --- .../opengrok/history/FileHistoryCache.java | 48 +++++++++++-------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/org/opensolaris/opengrok/history/FileHistoryCache.java b/src/org/opensolaris/opengrok/history/FileHistoryCache.java index fd88a4741c0..6cf4ae9d258 100644 --- a/src/org/opensolaris/opengrok/history/FileHistoryCache.java +++ b/src/org/opensolaris/opengrok/history/FileHistoryCache.java @@ -366,34 +366,42 @@ public void store(History history, Repository repository) continue; } - if (env.hasProjects()) { + List list = map.get(s); + if (list == null) { /* - * Problem here is that if the original file was a symlink - * it's been already dereferenced by - * getPathRelativeToSourceRoot(). We have to solve the only - * case which is that the symlink led to the project's root. - * - * @see - * RuntimeEnvironment#getPathRelativeToSourceRoot(java.io.File, int) + * This means that the file has not been added yet. So we + * try to see if we should accept it. */ - Project project = Project.getProject(test); - File parent = test.equals(new File(env.getSourceRootPath(), project.getPath())) - /* this is a project's root */ - ? test - /* this isn't a project's root */ - : test.getParentFile(); - if (!AcceptHelper.accept(project, parent, test)) { + if (env.hasProjects()) { + /* + * Problem here is that if the original file was a + * symlink it's been already dereferenced by + * getPathRelativeToSourceRoot(). We have to solve the + * only case which is that the symlink led to the + * project's root. + * + * @see + * RuntimeEnvironment#getPathRelativeToSourceRoot(java.io.File, + * int) + */ + Project project = Project.getProject(test); + File parent = test.equals(new File(env.getSourceRootPath(), project.getPath())) + /* this is a project's root */ + ? test + /* this isn't a project's root */ + : test.getParentFile(); + if (!AcceptHelper.accept(project, parent, test)) { + continue; + } + } else if (list == null && !AcceptHelper.accept(null, test.getParentFile(), test)) { continue; } - } else if (!AcceptHelper.accept(null, test.getParentFile(), test)) { - continue; - } - List list = map.get(s); - if (list == null) { + // create a new empty record in the map list = new ArrayList<>(); map.put(s, list); } + /* * We need to do deep copy in order to have different tags * per each commit.