This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
commit d0ba8ec152976d6e7268dabfbbe0473cb909febe Author: Arina Ielchiieva <[email protected]> AuthorDate: Tue Nov 20 17:06:07 2018 +0200 DRILL-6858: Add functionality to list directories / files with exceptions suppression 1. Add listDirectoriesSafe, listFilesSafe, listAllSafe in FileSystemUtil and DrillFileSystemUtil classes. 2. Use FileSystemUtil.listAllSafe during listing files in show files command and information_schema.files table. closes #1547 --- .../planner/sql/handlers/ShowFilesHandler.java | 5 +- .../store/ischema/InfoSchemaRecordGenerator.java | 5 +- .../drill/exec/util/DrillFileSystemUtil.java | 57 +++++++- .../org/apache/drill/exec/util/FileSystemUtil.java | 159 ++++++++++++++++----- .../impersonation/TestImpersonationMetadata.java | 13 +- .../drill/exec/util/DrillFileSystemUtilTest.java | 70 +++++---- .../apache/drill/exec/util/FileSystemUtilTest.java | 122 ++++++---------- .../drill/exec/util/FileSystemUtilTestBase.java | 7 +- 8 files changed, 256 insertions(+), 182 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFilesHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFilesHandler.java index 75abfdd..9782bbf 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFilesHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ShowFilesHandler.java @@ -32,7 +32,6 @@ import org.apache.drill.exec.util.FileSystemUtil; import org.apache.drill.exec.work.foreman.ForemanSetupException; import org.apache.hadoop.fs.Path; -import java.io.IOException; import java.sql.Timestamp; import java.util.List; import java.util.stream.Collectors; @@ -46,7 +45,7 @@ public class ShowFilesHandler extends DefaultSqlHandler { } @Override - public PhysicalPlan getPlan(SqlNode sqlNode) throws ForemanSetupException, IOException { + public PhysicalPlan getPlan(SqlNode sqlNode) throws ForemanSetupException { SchemaPlus defaultSchema = config.getConverter().getDefaultSchema(); SchemaPlus drillSchema = defaultSchema; SqlShowFiles showFiles = unwrap(sqlNode, SqlShowFiles.class); @@ -83,7 +82,7 @@ public class ShowFilesHandler extends DefaultSqlHandler { } Path path = new Path(wsSchema.getDefaultLocation(), fromDir); - List<ShowFilesCommandResult> records = FileSystemUtil.listAll(wsSchema.getFS(), path, false).stream() + List<ShowFilesCommandResult> records = FileSystemUtil.listAllSafe(wsSchema.getFS(), path, false).stream() // use ShowFilesCommandResult for backward compatibility .map(fileStatus -> new ShowFilesCommandResult(new Records.File(wsSchema.getFullSchemaName(), wsSchema, fileStatus))) .collect(Collectors.toList()); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaRecordGenerator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaRecordGenerator.java index cfb918b..1e72840 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaRecordGenerator.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaRecordGenerator.java @@ -31,7 +31,6 @@ import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_T import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.SHRD_COL_TABLE_SCHEMA; import static org.apache.drill.exec.store.ischema.InfoSchemaConstants.TBLS_COL_TABLE_TYPE; -import java.io.IOException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -436,14 +435,12 @@ public abstract class InfoSchemaRecordGenerator<S> { String defaultLocation = wsSchema.getDefaultLocation(); FileSystem fs = wsSchema.getFS(); boolean recursive = optionManager.getBoolean(ExecConstants.LIST_FILES_RECURSIVELY); - FileSystemUtil.listAll(fs, new Path(defaultLocation), recursive).forEach( + FileSystemUtil.listAllSafe(fs, new Path(defaultLocation), recursive).forEach( fileStatus -> records.add(new Records.File(schemaName, wsSchema, fileStatus)) ); } } catch (ClassCastException | UnsupportedOperationException e) { // ignore the exception since either this is not a Drill schema or schema does not support files listing - } catch (IOException e) { - logger.warn("Failure while trying to list files", e); } } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/DrillFileSystemUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/DrillFileSystemUtil.java index 2fc135d..fcee3b0 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/DrillFileSystemUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/DrillFileSystemUtil.java @@ -36,12 +36,8 @@ public class DrillFileSystemUtil { /** * Path filter that skips all files and folders that start with dot or underscore. */ - public static final PathFilter DRILL_SYSTEM_FILTER = new PathFilter() { - @Override - public boolean accept(Path path) { - return !path.getName().startsWith(DrillFileSystem.UNDERSCORE_PREFIX) && !path.getName().startsWith(DrillFileSystem.DOT_PREFIX); - } - }; + public static final PathFilter DRILL_SYSTEM_FILTER = path -> + !path.getName().startsWith(DrillFileSystem.UNDERSCORE_PREFIX) && !path.getName().startsWith(DrillFileSystem.DOT_PREFIX); /** * Returns statuses of all directories present in given path applying custom filters if present. @@ -59,6 +55,22 @@ public class DrillFileSystemUtil { } /** + * Returns statuses of all directories present in given path applying custom filters if present. + * Directories that start with dot or underscore are skipped. + * Will include nested directories if recursive flag is set to true. + * Will ignore all exceptions during listing if any. + * + * @param fs current file system + * @param path path to directory + * @param recursive true if nested directories should be included + * @param filters list of custom filters (optional) + * @return list of matching directory statuses + */ + public static List<FileStatus> listDirectoriesSafe(final FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + return FileSystemUtil.listDirectoriesSafe(fs, path, recursive, FileSystemUtil.mergeFilters(DRILL_SYSTEM_FILTER, filters)); + } + + /** * Returns statuses of all files present in given path applying custom filters if present. * Files and nested directories that start with dot or underscore are skipped. * Will also include files from nested directories if recursive flag is set to true. @@ -74,6 +86,23 @@ public class DrillFileSystemUtil { } /** + * Returns statuses of all files present in given path applying custom filters if present. + * Files and nested directories that start with dot or underscore are skipped. + * Will include files from nested directories if recursive flag is set to true. + * Will ignore all exceptions during listing if any. + * + * @param fs current file system + * @param path path to file or directory + * @param recursive true if files in nested directories should be included + * @param filters list of custom filters (optional) + * @return list of matching file statuses + */ + public static List<FileStatus> listFilesSafe(final FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + return FileSystemUtil.listFilesSafe(fs, path, recursive, FileSystemUtil.mergeFilters(DRILL_SYSTEM_FILTER, filters)); + } + + + /** * Returns statuses of all directories and files present in given path applying custom filters if present. * Directories and files that start with dot or underscore are skipped. * Will also include nested directories and their files if recursive flag is set to true. @@ -88,4 +117,20 @@ public class DrillFileSystemUtil { return FileSystemUtil.listAll(fs, path, recursive, FileSystemUtil.mergeFilters(DRILL_SYSTEM_FILTER, filters)); } + /** + * Returns statuses of all directories and files present in given path applying custom filters if present. + * Directories and files that start with dot or underscore are skipped. + * Will include nested directories and their files if recursive flag is set to true. + * Will ignore all exceptions during listing if any. + * + * @param fs current file system + * @param path path to file or directory + * @param recursive true if nested directories and their files should be included + * @param filters list of custom filters (optional) + * @return list of matching directory and file statuses + */ + public static List<FileStatus> listAllSafe(FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + return FileSystemUtil.listAllSafe(fs, path, recursive, FileSystemUtil.mergeFilters(DRILL_SYSTEM_FILTER, filters)); + } + } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/util/FileSystemUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/util/FileSystemUtil.java index f17e215..47ac44c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/util/FileSystemUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/util/FileSystemUtil.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.stream.Stream; /** * Helper class that provides methods to list directories or file or both statuses. @@ -33,15 +34,12 @@ import java.util.List; */ public class FileSystemUtil { + private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(FileSystemUtil.class); + /** * Filter that will accept all files and directories. */ - public static final PathFilter DUMMY_FILTER = new PathFilter() { - @Override - public boolean accept(Path path) { - return true; - } - }; + public static final PathFilter DUMMY_FILTER = path -> true; /** * Returns statuses of all directories present in given path applying custom filters if present. @@ -55,7 +53,28 @@ public class FileSystemUtil { */ public static List<FileStatus> listDirectories(final FileSystem fs, Path path, boolean recursive, PathFilter... filters) throws IOException { List<FileStatus> statuses = new ArrayList<>(); - listDirectories(fs, path, recursive, statuses, mergeFilters(filters)); + listDirectories(fs, path, recursive, false, statuses, mergeFilters(filters)); + return statuses; + } + + /** + * Returns statuses of all directories present in given path applying custom filters if present. + * Will also include nested directories if recursive flag is set to true. + * Will ignore all exceptions during listing if any. + * + * @param fs current file system + * @param path path to directory + * @param recursive true if nested directories should be included + * @param filters list of custom filters (optional) + * @return list of matching directory statuses + */ + public static List<FileStatus> listDirectoriesSafe(final FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + List<FileStatus> statuses = new ArrayList<>(); + try { + listDirectories(fs, path, recursive, true, statuses, mergeFilters(filters)); + } catch (Exception e) { + // all exceptions are ignored + } return statuses; } @@ -71,7 +90,27 @@ public class FileSystemUtil { */ public static List<FileStatus> listFiles(FileSystem fs, Path path, boolean recursive, PathFilter... filters) throws IOException { List<FileStatus> statuses = new ArrayList<>(); - listFiles(fs, path, recursive, statuses, mergeFilters(filters)); + listFiles(fs, path, recursive, false, statuses, mergeFilters(filters)); + return statuses; + } + + /** + * Returns statuses of all files present in given path applying custom filters if present. + * Will also include files from nested directories if recursive flag is set to true. + * + * @param fs current file system + * @param path path to file or directory + * @param recursive true if files in nested directories should be included + * @param filters list of custom filters (optional) + * @return list of matching file statuses + */ + public static List<FileStatus> listFilesSafe(FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + List<FileStatus> statuses = new ArrayList<>(); + try { + listFiles(fs, path, recursive, true, statuses, mergeFilters(filters)); + } catch (Exception e) { + // all exceptions are ignored + } return statuses; } @@ -87,7 +126,28 @@ public class FileSystemUtil { */ public static List<FileStatus> listAll(FileSystem fs, Path path, boolean recursive, PathFilter... filters) throws IOException { List<FileStatus> statuses = new ArrayList<>(); - listAll(fs, path, recursive, statuses, mergeFilters(filters)); + listAll(fs, path, recursive, false, statuses, mergeFilters(filters)); + return statuses; + } + + /** + * Returns statuses of all directories and files present in given path applying custom filters if present. + * Will also include nested directories and their files if recursive flag is set to true. + * Will ignore all exceptions during listing if any. + * + * @param fs current file system + * @param path path to file or directory + * @param recursive true if nested directories and their files should be included + * @param filters list of custom filters (optional) + * @return list of matching directory and file statuses + */ + public static List<FileStatus> listAllSafe(FileSystem fs, Path path, boolean recursive, PathFilter... filters) { + List<FileStatus> statuses = new ArrayList<>(); + try { + listAll(fs, path, recursive, true, statuses, mergeFilters(filters)); + } catch (Exception e) { + // all exceptions are ignored + } return statuses; } @@ -122,39 +182,39 @@ public class FileSystemUtil { return DUMMY_FILTER; } - return new PathFilter() { - @Override - public boolean accept(Path path) { - for (PathFilter filter : filters) { - if (!filter.accept(path)) { - return false; - } - } - return true; - } - }; + return path -> Stream.of(filters).allMatch(filter -> filter.accept(path)); } /** * Helper method that will store in given holder statuses of all directories present in given path applying custom filter. * If recursive flag is set to true, will call itself recursively to add statuses of nested directories. + * If suppress exceptions flag is set to true, will ignore all exceptions during listing. * * @param fs current file system * @param path path to directory * @param recursive true if nested directories should be included + * @param suppressExceptions indicates if exceptions should be ignored during listing * @param statuses holder for directory statuses * @param filter custom filter * @return holder with all matching directory statuses */ - private static List<FileStatus> listDirectories(FileSystem fs, Path path, boolean recursive, List<FileStatus> statuses, PathFilter filter) throws IOException { - FileStatus[] fileStatuses = fs.listStatus(path, filter); - for (FileStatus status: fileStatuses) { - if (status.isDirectory()) { - statuses.add(status); - if (recursive) { - listDirectories(fs, status.getPath(), true, statuses, filter); + private static List<FileStatus> listDirectories(FileSystem fs, Path path, boolean recursive, boolean suppressExceptions, + List<FileStatus> statuses, PathFilter filter) throws IOException { + try { + for (FileStatus status : fs.listStatus(path, filter)) { + if (status.isDirectory()) { + statuses.add(status); + if (recursive) { + listDirectories(fs, status.getPath(), true, suppressExceptions, statuses, filter); + } } } + } catch (Exception e) { + if (suppressExceptions) { + logger.debug("Exception during listing file statuses", e); + } else { + throw e; + } } return statuses; } @@ -162,23 +222,33 @@ public class FileSystemUtil { /** * Helper method that will store in given holder statuses of all files present in given path applying custom filter. * If recursive flag is set to true, will call itself recursively to add file statuses from nested directories. + * If suppress exceptions flag is set to true, will ignore all exceptions during listing. * * @param fs current file system * @param path path to file or directory * @param recursive true if files in nested directories should be included + * @param suppressExceptions indicates if exceptions should be ignored during listing * @param statuses holder for file statuses * @param filter custom filter * @return holder with all matching file statuses */ - private static List<FileStatus> listFiles(FileSystem fs, Path path, boolean recursive, List<FileStatus> statuses, PathFilter filter) throws IOException { - FileStatus[] fileStatuses = fs.listStatus(path, filter); - for (FileStatus status: fileStatuses) { - if (status.isDirectory()) { - if (recursive) { - listFiles(fs, status.getPath(), true, statuses, filter); + private static List<FileStatus> listFiles(FileSystem fs, Path path, boolean recursive, boolean suppressExceptions, + List<FileStatus> statuses, PathFilter filter) throws IOException { + try { + for (FileStatus status : fs.listStatus(path, filter)) { + if (status.isDirectory()) { + if (recursive) { + listFiles(fs, status.getPath(), true, suppressExceptions, statuses, filter); + } + } else { + statuses.add(status); } + } + } catch (Exception e) { + if (suppressExceptions) { + logger.debug("Exception during listing file statuses", e); } else { - statuses.add(status); + throw e; } } return statuses; @@ -187,19 +257,30 @@ public class FileSystemUtil { /** * Helper method that will store in given holder statuses of all directories and files present in given path applying custom filter. * If recursive flag is set to true, will call itself recursively to add nested directories and their file statuses. + * If suppress exceptions flag is set to true, will ignore all exceptions during listing. * * @param fs current file system * @param path path to file or directory * @param recursive true if nested directories and their files should be included + * @param suppressExceptions indicates if exceptions should be ignored during listing * @param statuses holder for directory and file statuses * @param filter custom filter * @return holder with all matching directory and file statuses */ - private static List<FileStatus> listAll(FileSystem fs, Path path, boolean recursive, List<FileStatus> statuses, PathFilter filter) throws IOException { - for (FileStatus status: fs.listStatus(path, filter)) { - statuses.add(status); - if (status.isDirectory() && recursive) { - listAll(fs, status.getPath(), true, statuses, filter); + private static List<FileStatus> listAll(FileSystem fs, Path path, boolean recursive, boolean suppressExceptions, + List<FileStatus> statuses, PathFilter filter) throws IOException { + try { + for (FileStatus status : fs.listStatus(path, filter)) { + statuses.add(status); + if (status.isDirectory() && recursive) { + listAll(fs, status.getPath(), true, suppressExceptions, statuses, filter); + } + } + } catch (Exception e) { + if (suppressExceptions) { + logger.debug("Exception during listing file statuses", e); + } else { + throw e; } } return statuses; diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java index 6dc3615..03754e5 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestImpersonationMetadata.java @@ -30,7 +30,6 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; import org.junit.AfterClass; -import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -40,6 +39,7 @@ import org.junit.rules.ExpectedException; import java.util.Map; import static org.hamcrest.core.StringContains.containsString; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; /** @@ -131,10 +131,10 @@ public class TestImpersonationMetadata extends BaseTestImpersonation { try { test("drop table parquet_table_700"); } catch (UserException e) { - Assert.assertTrue(e.getMessage().contains("PERMISSION ERROR")); + assertTrue(e.getMessage().contains("PERMISSION ERROR")); dropFailed = true; } - Assert.assertTrue("Permission checking failed during drop table", dropFailed); + assertTrue("Permission checking failed during drop table", dropFailed); } @Test // DRILL-3037 @@ -176,12 +176,9 @@ public class TestImpersonationMetadata extends BaseTestImpersonation { @Test public void testShowFilesInWSWithNoPermissionsForQueryUser() throws Exception { updateClient(user2); - - thrown.expect(UserRemoteException.class); - thrown.expectMessage(containsString("Permission denied: user=drillTestUser2, " + - "access=READ_EXECUTE, inode=\"/drill_test_grp_1_700\":drillTestUser1:drill_test_grp_1:drwx------")); // Try show tables in schema "drill_test_grp_1_700" which is owned by "user1" - test("SHOW FILES IN %s.drill_test_grp_1_700", MINI_DFS_STORAGE_PLUGIN_NAME); + int count = testSql(String.format("SHOW FILES IN %s.drill_test_grp_1_700", MINI_DFS_STORAGE_PLUGIN_NAME)); + assertEquals(0, count); } @Test diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/util/DrillFileSystemUtilTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/util/DrillFileSystemUtilTest.java index 175929e..8ec47be 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/util/DrillFileSystemUtilTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/util/DrillFileSystemUtilTest.java @@ -27,6 +27,7 @@ import java.util.Collections; import java.util.List; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @@ -38,12 +39,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListDirectoriesWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listDirectories(fs, base, false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listDirectories(fs, base, false, + (PathFilter) path -> path.getName().endsWith("a")); assertEquals("Directory count should match", 1, statuses.size()); assertEquals("Directory name should match", "a", statuses.get(0).getPath().getName()); } @@ -56,12 +53,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListDirectoriesRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listDirectories(fs, base, true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listDirectories(fs, base, true, + (PathFilter) path -> path.getName().endsWith("a")); assertEquals("Directory count should match", 2, statuses.size()); Collections.sort(statuses); @@ -78,12 +71,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListFilesWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listFiles(fs, new Path(base, "a"), false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listFiles(fs, new Path(base, "a"), false, + (PathFilter) path -> path.getName().endsWith(".txt")); assertEquals("File count should match", 1, statuses.size()); assertEquals("File name should match", "f.txt", statuses.get(0).getPath().getName()); } @@ -96,12 +85,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListFilesRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listFiles(fs, base, true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listFiles(fs, base, true, + (PathFilter) path -> path.getName().endsWith("a") || path.getName().endsWith(".txt")); assertEquals("File count should match", 2, statuses.size()); Collections.sort(statuses); @@ -121,12 +106,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListAllWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listAll(fs, new Path(base, "a"), false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listAll(fs, new Path(base, "a"), false, + (PathFilter) path -> path.getName().endsWith("a") || path.getName().endsWith(".txt")); assertEquals("Directory and file count should match", 2, statuses.size()); Collections.sort(statuses); @@ -142,12 +123,8 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListAllRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = DrillFileSystemUtil.listAll(fs, new Path(base, "a"), true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = DrillFileSystemUtil.listAll(fs, new Path(base, "a"), true, + (PathFilter) path -> path.getName().startsWith("a") || path.getName().endsWith(".txt")); assertEquals("Directory and file count should match", 3, statuses.size()); Collections.sort(statuses); @@ -156,4 +133,25 @@ public class DrillFileSystemUtilTest extends FileSystemUtilTestBase { assertEquals("File name should match", "f.txt", statuses.get(2).getPath().getName()); } + @Test + public void testListDirectoriesSafe() { + Path file = new Path(base, "missing"); + List<FileStatus> fileStatuses = DrillFileSystemUtil.listDirectoriesSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + + @Test + public void testListFilesSafe() { + Path file = new Path(base, "missing.txt"); + List<FileStatus> fileStatuses = DrillFileSystemUtil.listFilesSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + + @Test + public void testListAllSafe() { + Path file = new Path(base, "missing"); + List<FileStatus> fileStatuses = DrillFileSystemUtil.listAllSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTest.java index 764f1d7..299f1cc 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTest.java @@ -40,12 +40,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListDirectoriesWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, false, + (PathFilter) path -> path.getName().endsWith("a")); assertEquals("Directory count should match", 3, statuses.size()); Collections.sort(statuses); @@ -62,12 +58,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListDirectoriesRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, true, + (PathFilter) path -> path.getName().endsWith("a")); assertEquals("Directory count should match", 4, statuses.size()); Collections.sort(statuses); @@ -79,12 +71,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListDirectoriesEmptyResult() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("abc"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listDirectories(fs, base, false, + (PathFilter) path -> path.getName().startsWith("abc")); assertEquals("Directory count should match", 0, statuses.size()); } @@ -96,12 +84,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListFilesWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listFiles(fs, new Path(base, "a"), false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listFiles(fs, new Path(base, "a"), false, + (PathFilter) path -> path.getName().endsWith(".txt")); assertEquals("File count should match", 3, statuses.size()); Collections.sort(statuses); @@ -118,12 +102,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListFilesRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listFiles(fs, base, true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listFiles(fs, base, true, + (PathFilter) path -> path.getName().endsWith("a") || path.getName().endsWith(".txt")); assertEquals("File count should match", 8, statuses.size()); } @@ -142,12 +122,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListAllWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listAll(fs, new Path(base, "a"), false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listAll(fs, new Path(base, "a"), false, + (PathFilter) path -> path.getName().endsWith("a") || path.getName().endsWith(".txt")); assertEquals("Directory and file count should match", 4, statuses.size()); } @@ -159,34 +135,21 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { @Test public void testListAllRecursiveWithFilter() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listAll(fs, new Path(base, "a"), true, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith("a") || path.getName().endsWith(".txt"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listAll(fs, new Path(base, "a"), true, + (PathFilter) path -> path.getName().endsWith("a") || path.getName().endsWith(".txt")); assertEquals("Directory and file count should match", 7, statuses.size()); } @Test public void testListAllEmptyResult() throws IOException { - List<FileStatus> statuses = FileSystemUtil.listAll(fs, base, false, new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("xyz"); - } - }); + List<FileStatus> statuses = FileSystemUtil.listAll(fs, base, false, + (PathFilter) path -> path.getName().startsWith("xyz")); assertEquals("Directory and file count should match", 0, statuses.size()); } @Test public void testMergeFiltersWithMissingParameters() { - PathFilter filter = new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("a"); - } - }; + PathFilter filter = path -> path.getName().startsWith("a"); assertEquals("Should have returned initial filter", filter, FileSystemUtil.mergeFilters(filter, null)); assertEquals("Should have returned initial filter", filter, FileSystemUtil.mergeFilters(filter, new PathFilter[]{})); @@ -197,19 +160,8 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { public void mergeFiltersTrue() { Path file = new Path("abc.txt"); - PathFilter firstFilter = new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("a"); - } - }; - - PathFilter secondFilter = new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith(".txt"); - } - }; + PathFilter firstFilter = path -> path.getName().startsWith("a"); + PathFilter secondFilter = path -> path.getName().endsWith(".txt"); assertTrue("Path should have been included in the path list", FileSystemUtil.mergeFilters(firstFilter, secondFilter).accept(file)); assertTrue("Path should have been included in the path list", FileSystemUtil.mergeFilters(firstFilter, new PathFilter[] {secondFilter}).accept(file)); @@ -219,22 +171,32 @@ public class FileSystemUtilTest extends FileSystemUtilTestBase { public void mergeFiltersFalse() { Path file = new Path("abc.txt"); - PathFilter firstFilter = new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().startsWith("a"); - } - }; - - PathFilter secondFilter = new PathFilter() { - @Override - public boolean accept(Path path) { - return path.getName().endsWith(".csv"); - } - }; + PathFilter firstFilter = path -> path.getName().startsWith("a"); + PathFilter secondFilter = path -> path.getName().endsWith(".csv"); assertFalse("Path should have been excluded from the path list", FileSystemUtil.mergeFilters(firstFilter, secondFilter).accept(file)); assertFalse("Path should have been excluded from the path list", FileSystemUtil.mergeFilters(firstFilter, new PathFilter[] {secondFilter}).accept(file)); } + @Test + public void testListDirectoriesSafe() { + Path file = new Path(base, "missing"); + List<FileStatus> fileStatuses = FileSystemUtil.listDirectoriesSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + + @Test + public void testListFilesSafe() { + Path file = new Path(base, "missing.txt"); + List<FileStatus> fileStatuses = FileSystemUtil.listFilesSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + + @Test + public void testListAllSafe() { + Path file = new Path(base, "missing"); + List<FileStatus> fileStatuses = FileSystemUtil.listAllSafe(fs, file, true); + assertTrue("Should return empty result", fileStatuses.isEmpty()); + } + } diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTestBase.java b/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTestBase.java index 22cf3ea..5081bfd 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTestBase.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/util/FileSystemUtilTestBase.java @@ -68,12 +68,7 @@ public class FileSystemUtilTestBase { // create temporary directory with sub-folders and files final File tempDir = Files.createTempDir(); - Runtime.getRuntime().addShutdownHook(new Thread() { - @Override - public void run() { - FileUtils.deleteQuietly(tempDir); - } - }); + Runtime.getRuntime().addShutdownHook(new Thread(() -> FileUtils.deleteQuietly(tempDir))); base = new Path(tempDir.toURI().getPath()); createDefaultStructure(fs, base, "a", 2);
