[GitHub] [incubator-druid] suneet-amp commented on a change in pull request #8932: Add FileUtils.createTempDir() and enforce its usage.

[GitBox <gi...@apache.org>]

suneet-amp commented on a change in pull request #8932: Add FileUtils.createTempDir() and enforce its usage.
URL: https://github.com/apache/incubator-druid/pull/8932#discussion_r349745087
 
 

 ##########
 File path: core/src/main/java/org/apache/druid/java/util/common/FileUtils.java
 ##########
 @@ -333,6 +339,65 @@ public static long copyLarge(
     }
   }
 
+  /**
+   * Creates a temporary directory inside the configured temporary space (java.io.tmpdir). Similar to the method
+   * {@link com.google.common.io.Files#createTempDir()} from Guava, but has nicer error messages.
+   *
+   * @throws IllegalStateException if the directory could not be created
+   */
+  public static File createTempDir()
+  {
+    return createTempDir(null);
+  }
+
+  /**
+   * Creates a temporary directory inside the configured temporary space (java.io.tmpdir). Similar to the method
+   * {@link com.google.common.io.Files#createTempDir()} from Guava, but has nicer error messages.
+   *
+   * @param prefix base directory name; if null/empty then this method will use "druid"
+   *
+   * @throws IllegalStateException if the directory could not be created
+   */
+  @SuppressForbidden(reason = "Files#createTempDirectory")
+  public static File createTempDir(@Nullable final String prefix)
+  {
+    final String parentDirectory = System.getProperty("java.io.tmpdir");
+
+    if (parentDirectory == null) {
+      // Not expected.
+      throw new ISE("System property java.io.tmpdir is not set, cannot create temporary directories");
+    }
+
+    try {
+      final Path tmpPath = Files.createTempDirectory(
+          new File(parentDirectory).toPath(),
+          prefix == null || prefix.isEmpty() ? "druid" : prefix
+      );
+      return tmpPath.toFile();
+    }
+    catch (IOException e) {
+      // Some inspection to improve error messages.
+      if (e instanceof NoSuchFileException && !new File(parentDirectory).exists()) {
+        throw new ISE("java.io.tmpdir (%s) does not exist", parentDirectory);
+      } else if ((e instanceof FileSystemException && e.getMessage().contains("Read-only file system"))
+                 || (e instanceof AccessDeniedException)) {
+        throw new ISE("java.io.tmpdir (%s) is not writable, check permissions", parentDirectory);
+      } else {
+        // Well, maybe it was something else.
+        throw new ISE(e, "Failed to create temporary directory in java.io.tmpdir (%s)", parentDirectory);
+      }
+    }
+  }
+
+  /**
+   * Equivalent to {@link org.apache.commons.io.FileUtils#deleteDirectory(File)}. Exists here mostly so callers
+   * can avoid dealing with our FileUtils and the Commons FileUtils having the same name.
+   */
+  public static void deleteDirectory(final File directory) throws IOException
 
 Review comment:
   missing `@SuppressForbidden` here?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org