From 3ed005864cdac8d7d04d61307fe1493b38e1ea97 Mon Sep 17 00:00:00 2001 From: Arina Ielchiieva Date: Wed, 26 Apr 2017 13:27:19 +0000 Subject: [PATCH] DRILL-5391: CTAS: make folder and file permission configurable --- .../org/apache/drill/exec/ExecConstants.java | 3 + .../sql/handlers/CreateTableHandler.java | 3 +- .../server/options/SystemOptionManager.java | 3 +- .../drill/exec/store/AbstractSchema.java | 2 +- .../drill/exec/store/StorageStrategy.java | 69 ++++++++++---- .../store/easy/json/JsonRecordWriter.java | 2 +- .../store/parquet/ParquetRecordWriter.java | 2 +- .../store/text/DrillTextRecordWriter.java | 2 +- .../org/apache/drill/exec/sql/TestCTAS.java | 28 +++++- .../drill/exec/store/StorageStrategyTest.java | 90 ++++++++++--------- 10 files changed, 138 insertions(+), 66 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java index 91498fced7e..e291524c6a0 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java @@ -446,4 +446,7 @@ public interface ExecConstants { String QUERY_TRANSIENT_STATE_UPDATE_KEY = "exec.query.progress.update"; BooleanValidator QUERY_TRANSIENT_STATE_UPDATE = new BooleanValidator(QUERY_TRANSIENT_STATE_UPDATE_KEY, true); + String PERSISTENT_TABLE_UMASK = "exec.persistent_table.umask"; + StringValidator PERSISTENT_TABLE_UMASK_VALIDATOR = new StringValidator(PERSISTENT_TABLE_UMASK, "002"); + } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java index d50391c8816..72444ca7157 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java @@ -91,7 +91,8 @@ public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConv log("Calcite", newTblRelNodeWithPCol, logger, null); // Convert the query to Drill Logical plan and insert a writer operator on top. StorageStrategy storageStrategy = sqlCreateTable.isTemporary() ? - StorageStrategy.TEMPORARY : StorageStrategy.PERSISTENT; + StorageStrategy.TEMPORARY : + new StorageStrategy(context.getOption(ExecConstants.PERSISTENT_TABLE_UMASK).string_val, false); // If we are creating temporary table, initial table name will be replaced with generated table name. // Generated table name is unique, UUID.randomUUID() is used for its generation. diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java index a7d6526aca8..8d0e96cfc37 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java @@ -169,7 +169,8 @@ public class SystemOptionManager extends BaseOptionManager implements OptionMana ExecConstants.ENABLE_QUERY_PROFILE_VALIDATOR, ExecConstants.QUERY_PROFILE_DEBUG_VALIDATOR, ExecConstants.USE_DYNAMIC_UDFS, - ExecConstants.QUERY_TRANSIENT_STATE_UPDATE + ExecConstants.QUERY_TRANSIENT_STATE_UPDATE, + ExecConstants.PERSISTENT_TABLE_UMASK_VALIDATOR }; final Map tmp = new HashMap<>(); for (final OptionValidator validator : validators) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java index 618841be4b8..7d6bfe35ef3 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/AbstractSchema.java @@ -140,7 +140,7 @@ public CreateTableEntry createNewTable(String tableName, List partitionC * @return create table entry */ public CreateTableEntry createNewTable(String tableName, List partitionColumns) { - return createNewTable(tableName, partitionColumns, StorageStrategy.PERSISTENT); + return createNewTable(tableName, partitionColumns, StorageStrategy.DEFAULT); } /** diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StorageStrategy.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StorageStrategy.java index a125baefbfb..fdb8da85bcd 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/StorageStrategy.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/StorageStrategy.java @@ -18,6 +18,7 @@ package org.apache.drill.exec.store; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; import com.google.common.collect.Lists; import org.apache.hadoop.fs.FileSystem; @@ -31,12 +32,11 @@ public class StorageStrategy { /** - * Primary is used for persistent tables. * For directories: drwxrwxr-x (owner and group have full access, others can read and execute). - * For files: -rw-r--r-- (owner can read and write, group and others can read). + * For files: -rw-rw--r-- (owner and group can read and write, others can read). * Folders and files are not deleted on file system close. */ - public static final StorageStrategy PERSISTENT = new StorageStrategy("775", "644", false); + public static final StorageStrategy DEFAULT = new StorageStrategy("002", false); /** * Primary is used for temporary tables. @@ -44,31 +44,44 @@ public class StorageStrategy { * For files: -rw------- (owner can read and write, group and others have no access). * Folders and files are deleted on file system close. */ - public static final StorageStrategy TEMPORARY = new StorageStrategy("700", "600", true); + public static final StorageStrategy TEMPORARY = new StorageStrategy("077", true); - private final String folderPermission; - private final String filePermission; + private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(StorageStrategy.class); + + private final String umask; private final boolean deleteOnExit; @JsonCreator - public StorageStrategy(@JsonProperty("folderPermission") String folderPermission, - @JsonProperty("filePermission") String filePermission, + public StorageStrategy(@JsonProperty("umask") String umask, @JsonProperty("deleteOnExit") boolean deleteOnExit) { - this.folderPermission = folderPermission; - this.filePermission = filePermission; + this.umask = validateUmask(umask); this.deleteOnExit = deleteOnExit; } - public String getFolderPermission() { - return folderPermission; + public String getUmask() { + return umask; } - public String getFilePermission() { return filePermission; } - public boolean isDeleteOnExit() { return deleteOnExit; } + /** + * @return folder permission after applying umask + */ + @JsonIgnore + public FsPermission getFolderPermission() { + return FsPermission.getDirDefault().applyUMask(new FsPermission(umask)); + } + + /** + * @return file permission after applying umask + */ + @JsonIgnore + public FsPermission getFilePermission() { + return FsPermission.getFileDefault().applyUMask(new FsPermission(umask)); + } + /** * Creates passed path on appropriate file system. * Before creation checks which parent directories do not exists. @@ -94,7 +107,7 @@ public Path createPathAndApply(FileSystem fs, Path path) throws IOException { } fs.mkdirs(path); for (Path location : locations) { - applyStrategy(fs, location, folderPermission, deleteOnExit); + applyStrategy(fs, location, getFolderPermission(), deleteOnExit); } return locations.get(locations.size() - 1); } @@ -130,7 +143,7 @@ public Path createFileAndApply(FileSystem fs, Path file) throws IOException { } for (Path location : locations) { - applyStrategy(fs, location, folderPermission, deleteOnExit); + applyStrategy(fs, location, getFolderPermission(), deleteOnExit); } return locations.get(locations.size() - 1); } @@ -145,7 +158,25 @@ public Path createFileAndApply(FileSystem fs, Path file) throws IOException { * or adding file to delete on exit list */ public void applyToFile(FileSystem fs, Path file) throws IOException { - applyStrategy(fs, file, filePermission, deleteOnExit); + applyStrategy(fs, file, getFilePermission(), deleteOnExit); + } + + /** + * Validates if passed umask is valid. + * If umask is valid, returns given umask. + * If umask is invalid, returns default umask and logs error. + * + * @param umask umask string representation + * @return valid umask value + */ + private String validateUmask(String umask) { + try { + new FsPermission(umask); + return umask; + } catch (IllegalArgumentException | NullPointerException e) { + logger.error("Invalid umask value [{}]. Using default [{}].", umask, DEFAULT.getUmask(), e); + return DEFAULT.getUmask(); + } } /** @@ -185,8 +216,8 @@ private List getNonExistentLocations(FileSystem fs, Path path) throws IOEx * @throws IOException is thrown in case of problems while setting permission * or adding path to delete on exit list */ - private void applyStrategy(FileSystem fs, Path path, String permission, boolean deleteOnExit) throws IOException { - fs.setPermission(path, new FsPermission(permission)); + private void applyStrategy(FileSystem fs, Path path, FsPermission permission, boolean deleteOnExit) throws IOException { + fs.setPermission(path, permission); if (deleteOnExit) { fs.deleteOnExit(path); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java index c37da8a0741..345c05656ce 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonRecordWriter.java @@ -65,7 +65,7 @@ public class JsonRecordWriter extends JSONOutputRecordWriter implements RecordWr private boolean fRecordStarted = false; // true once the startRecord() is called until endRecord() is called public JsonRecordWriter(StorageStrategy storageStrategy){ - this.storageStrategy = storageStrategy == null ? StorageStrategy.PERSISTENT : storageStrategy; + this.storageStrategy = storageStrategy == null ? StorageStrategy.DEFAULT : storageStrategy; } @Override diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java index 6850d36619e..7536d781849 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java @@ -121,7 +121,7 @@ public ParquetRecordWriter(FragmentContext context, ParquetWriter writer) throws this.hasPartitions = partitionColumns != null && partitionColumns.size() > 0; this.extraMetaData.put(DRILL_VERSION_PROPERTY, DrillVersionInfo.getVersion()); this.extraMetaData.put(WRITER_VERSION_PROPERTY, String.valueOf(ParquetWriter.WRITER_VERSION)); - this.storageStrategy = writer.getStorageStrategy() == null ? StorageStrategy.PERSISTENT : writer.getStorageStrategy(); + this.storageStrategy = writer.getStorageStrategy() == null ? StorageStrategy.DEFAULT : writer.getStorageStrategy(); this.cleanUpLocations = Lists.newArrayList(); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java index d65a3eba559..f991abb7b19 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordWriter.java @@ -58,7 +58,7 @@ public class DrillTextRecordWriter extends StringOutputRecordWriter { public DrillTextRecordWriter(BufferAllocator allocator, StorageStrategy storageStrategy) { super(allocator); - this.storageStrategy = storageStrategy == null ? StorageStrategy.PERSISTENT : storageStrategy; + this.storageStrategy = storageStrategy == null ? StorageStrategy.DEFAULT : storageStrategy; } @Override diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java index 9d9b403ede3..88d23d3f73b 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/sql/TestCTAS.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -20,13 +20,20 @@ import com.google.common.collect.Maps; import org.apache.commons.io.FileUtils; import org.apache.drill.BaseTestQuery; +import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.proto.UserBitShared; import org.apache.drill.exec.rpc.user.QueryDataBatch; +import org.apache.drill.exec.store.StorageStrategy; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; import org.junit.Test; import java.io.File; import java.util.Map; +import static org.junit.Assert.assertEquals; + public class TestCTAS extends BaseTestQuery { @Test // DRILL-2589 public void withDuplicateColumnsInDef1() throws Exception { @@ -276,6 +283,25 @@ public void testPartitionByForAllTypes() throws Exception { } } + @Test + public void createTableWithCustomUmask() throws Exception { + test("use %s", TEMP_SCHEMA); + String tableName = "with_custom_permission"; + StorageStrategy storageStrategy = new StorageStrategy("000", false); + try (FileSystem fs = FileSystem.get(new Configuration())) { + test("alter session set `%s` = '%s'", ExecConstants.PERSISTENT_TABLE_UMASK, storageStrategy.getUmask()); + test("create table %s as select 'A' from (values(1))", tableName); + Path tableLocation = new Path(getDfsTestTmpSchemaLocation(), tableName); + assertEquals("Directory permission should match", + storageStrategy.getFolderPermission(), fs.getFileStatus(tableLocation).getPermission()); + assertEquals("File permission should match", + storageStrategy.getFilePermission(), fs.listLocatedStatus(tableLocation).next().getPermission()); + } finally { + test("alter session reset `%s`", ExecConstants.PERSISTENT_TABLE_UMASK); + test("drop table if exists %s", tableName); + } + } + private static void ctasErrorTestHelper(final String ctasSql, final String expErrorMsg) throws Exception { final String createTableSql = String.format(ctasSql, TEMP_SCHEMA, "testTableName"); errorMsgTestHelper(createTableSql, expErrorMsg); diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/store/StorageStrategyTest.java b/exec/java-exec/src/test/java/org/apache/drill/exec/store/StorageStrategyTest.java index 6a377ecfef9..32eb2345f04 100644 --- a/exec/java-exec/src/test/java/org/apache/drill/exec/store/StorageStrategyTest.java +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/store/StorageStrategyTest.java @@ -16,6 +16,7 @@ */ package org.apache.drill.exec.store; +import com.google.common.collect.Lists; import com.google.common.io.Files; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -33,11 +34,11 @@ public class StorageStrategyTest { - private static final Configuration configuration = new Configuration(); - private static final FsPermission full_permission = new FsPermission("777"); - private static final StorageStrategy persistent_strategy = new StorageStrategy("775", "644", false); - private static final StorageStrategy temporary_strategy = new StorageStrategy("700", "600", true); - private FileSystem fs; + private static final Configuration CONFIGURATION = new Configuration(); + private static final FsPermission FULL_PERMISSION = FsPermission.getDirDefault(); + private static final StorageStrategy PERSISTENT_STRATEGY = new StorageStrategy("002", false); + private static final StorageStrategy TEMPORARY_STRATEGY = new StorageStrategy("077", true); + private FileSystem FS; @Before public void setup() throws Exception { @@ -50,10 +51,10 @@ public void testPermissionAndDeleteOnExitFalseForFileWithParent() throws Excepti Path file = addNLevelsAndFile(initialPath, 2, true); Path firstCreatedParentPath = addNLevelsAndFile(initialPath, 1, false); - Path createdParentPath = persistent_strategy.createFileAndApply(fs, file); + Path createdParentPath = PERSISTENT_STRATEGY.createFileAndApply(FS, file); assertEquals("Path should match", firstCreatedParentPath, createdParentPath); - checkPathAndPermission(initialPath, file, true, 2, persistent_strategy); + checkPathAndPermission(initialPath, file, true, 2, PERSISTENT_STRATEGY); checkDeleteOnExit(firstCreatedParentPath, true); } @@ -63,10 +64,10 @@ public void testPermissionAndDeleteOnExitTrueForFileWithParent() throws Exceptio Path file = addNLevelsAndFile(initialPath, 2, true); Path firstCreatedParentPath = addNLevelsAndFile(initialPath, 1, false); - Path createdParentPath = temporary_strategy.createFileAndApply(fs, file); + Path createdParentPath = TEMPORARY_STRATEGY.createFileAndApply(FS, file); assertEquals("Path should match", firstCreatedParentPath, createdParentPath); - checkPathAndPermission(initialPath, file, true, 2, temporary_strategy); + checkPathAndPermission(initialPath, file, true, 2, TEMPORARY_STRATEGY); checkDeleteOnExit(firstCreatedParentPath, false); } @@ -75,10 +76,10 @@ public void testPermissionAndDeleteOnExitFalseForFileOnly() throws Exception { Path initialPath = prepareStorageDirectory(); Path file = addNLevelsAndFile(initialPath, 0, true); - Path createdFile = persistent_strategy.createFileAndApply(fs, file); + Path createdFile = PERSISTENT_STRATEGY.createFileAndApply(FS, file); assertEquals("Path should match", file, createdFile); - checkPathAndPermission(initialPath, file, true, 0, persistent_strategy); + checkPathAndPermission(initialPath, file, true, 0, PERSISTENT_STRATEGY); checkDeleteOnExit(file, true); } @@ -87,10 +88,10 @@ public void testPermissionAndDeleteOnExitTrueForFileOnly() throws Exception { Path initialPath = prepareStorageDirectory(); Path file = addNLevelsAndFile(initialPath, 0, true); - Path createdFile = temporary_strategy.createFileAndApply(fs, file); + Path createdFile = TEMPORARY_STRATEGY.createFileAndApply(FS, file); assertEquals("Path should match", file, createdFile); - checkPathAndPermission(initialPath, file, true, 0, temporary_strategy); + checkPathAndPermission(initialPath, file, true, 0, TEMPORARY_STRATEGY); checkDeleteOnExit(file, false); } @@ -98,13 +99,13 @@ public void testPermissionAndDeleteOnExitTrueForFileOnly() throws Exception { public void testFailureOnExistentFile() throws Exception { Path initialPath = prepareStorageDirectory(); Path file = addNLevelsAndFile(initialPath, 0, true); - fs.createNewFile(file); - assertTrue("File should exist", fs.exists(file)); + FS.createNewFile(file); + assertTrue("File should exist", FS.exists(file)); try { - persistent_strategy.createFileAndApply(fs, file); + PERSISTENT_STRATEGY.createFileAndApply(FS, file); } catch (IOException e) { assertEquals("Error message should match", String.format("File [%s] already exists on file system [%s].", - file.toUri().getPath(), fs.getUri()), e.getMessage()); + file.toUri().getPath(), FS.getUri()), e.getMessage()); throw e; } } @@ -115,10 +116,10 @@ public void testCreatePathAndDeleteOnExitFalse() throws Exception { Path resultPath = addNLevelsAndFile(initialPath, 2, false); Path firstCreatedParentPath = addNLevelsAndFile(initialPath, 1, false); - Path createdParentPath = persistent_strategy.createPathAndApply(fs, resultPath); + Path createdParentPath = PERSISTENT_STRATEGY.createPathAndApply(FS, resultPath); assertEquals("Path should match", firstCreatedParentPath, createdParentPath); - checkPathAndPermission(initialPath, resultPath, false, 2, persistent_strategy); + checkPathAndPermission(initialPath, resultPath, false, 2, PERSISTENT_STRATEGY); checkDeleteOnExit(firstCreatedParentPath, true); } @@ -128,10 +129,10 @@ public void testCreatePathAndDeleteOnExitTrue() throws Exception { Path resultPath = addNLevelsAndFile(initialPath, 2, false); Path firstCreatedParentPath = addNLevelsAndFile(initialPath, 1, false); - Path createdParentPath = temporary_strategy.createPathAndApply(fs, resultPath); + Path createdParentPath = TEMPORARY_STRATEGY.createPathAndApply(FS, resultPath); assertEquals("Path should match", firstCreatedParentPath, createdParentPath); - checkPathAndPermission(initialPath, resultPath, false, 2, temporary_strategy); + checkPathAndPermission(initialPath, resultPath, false, 2, TEMPORARY_STRATEGY); checkDeleteOnExit(firstCreatedParentPath, false); } @@ -139,46 +140,55 @@ public void testCreatePathAndDeleteOnExitTrue() throws Exception { public void testCreateNoPath() throws Exception { Path path = prepareStorageDirectory(); - Path createdParentPath = temporary_strategy.createPathAndApply(fs, path); + Path createdParentPath = TEMPORARY_STRATEGY.createPathAndApply(FS, path); assertNull("Path should be null", createdParentPath); - assertEquals("Permission should match", full_permission, fs.getFileStatus(path).getPermission()); + assertEquals("Permission should match", FULL_PERMISSION, FS.getFileStatus(path).getPermission()); } @Test public void testStrategyForExistingFile() throws Exception { Path initialPath = prepareStorageDirectory(); Path file = addNLevelsAndFile(initialPath, 0, true); - fs.createNewFile(file); - fs.setPermission(file, full_permission); + FS.createNewFile(file); + FS.setPermission(file, FULL_PERMISSION); - assertTrue("File should exist", fs.exists(file)); - assertEquals("Permission should match", full_permission, fs.getFileStatus(file).getPermission()); + assertTrue("File should exist", FS.exists(file)); + assertEquals("Permission should match", FULL_PERMISSION, FS.getFileStatus(file).getPermission()); - temporary_strategy.applyToFile(fs, file); + TEMPORARY_STRATEGY.applyToFile(FS, file); - assertEquals("Permission should match", new FsPermission(temporary_strategy.getFilePermission()), - fs.getFileStatus(file).getPermission()); + assertEquals("Permission should match", new FsPermission(TEMPORARY_STRATEGY.getFilePermission()), + FS.getFileStatus(file).getPermission()); checkDeleteOnExit(file, false); } + @Test + public void testInvalidUmask() throws Exception { + for (String invalid : Lists.newArrayList("ABC", "999", null)) { + StorageStrategy storageStrategy = new StorageStrategy(invalid, true); + assertEquals("Umask value should match default", StorageStrategy.DEFAULT.getUmask(), storageStrategy.getUmask()); + assertTrue("deleteOnExit flag should be set to true", storageStrategy.isDeleteOnExit()); + } + } + private Path prepareStorageDirectory() throws IOException { File storageDirectory = Files.createTempDir(); storageDirectory.deleteOnExit(); Path path = new Path(storageDirectory.toURI().getPath()); - fs.setPermission(path, full_permission); + FS.setPermission(path, FULL_PERMISSION); return path; } private void initFileSystem() throws IOException { - if (fs != null) { + if (FS != null) { try { - fs.close(); + FS.close(); } catch (Exception e) { // do nothing } } - fs = FileSystem.get(configuration); + FS = FileSystem.get(CONFIGURATION); } private Path addNLevelsAndFile(Path initialPath, int levels, boolean addFile) { @@ -198,25 +208,25 @@ private void checkPathAndPermission(Path initialPath, int levels, StorageStrategy storageStrategy) throws IOException { - assertEquals("Path type should match", isFile, fs.isFile(resultPath)); - assertEquals("Permission should match", full_permission, fs.getFileStatus(initialPath).getPermission()); + assertEquals("Path type should match", isFile, FS.isFile(resultPath)); + assertEquals("Permission should match", FULL_PERMISSION, FS.getFileStatus(initialPath).getPermission()); if (isFile) { assertEquals("Permission should match", new FsPermission(storageStrategy.getFilePermission()), - fs.getFileStatus(resultPath).getPermission()); + FS.getFileStatus(resultPath).getPermission()); } Path startingPath = initialPath; FsPermission folderPermission = new FsPermission(storageStrategy.getFolderPermission()); for (int i = 1; i <= levels; i++) { startingPath = new Path(startingPath, "level" + i); - assertEquals("Permission should match", folderPermission, fs.getFileStatus(startingPath).getPermission()); + assertEquals("Permission should match", folderPermission, FS.getFileStatus(startingPath).getPermission()); } } private void checkDeleteOnExit(Path path, boolean isPresent) throws IOException { - assertTrue("Path should be present", fs.exists(path)); + assertTrue("Path should be present", FS.exists(path)); // close and open file system to check for path presence initFileSystem(); - assertEquals("Path existence flag should match", isPresent, fs.exists(path)); + assertEquals("Path existence flag should match", isPresent, FS.exists(path)); } }