ctubbsii commented on code in PR #58:
URL: 
https://github.com/apache/accumulo-classloaders/pull/58#discussion_r2764627743


##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LocalStore.java:
##########
@@ -120,62 +121,65 @@ Path resourcesDir() {
   public static String localResourceName(Resource r) {
     requireNonNull(r);
     String remoteFileName = r.getFileName();
-    String checksum = checksumForFileName(r.getAlgorithm(), r.getChecksum());
+    String checksum = checksumForFileName(r);
     var matcher = fileNamesWithExtensionPattern.matcher(remoteFileName);
     if (matcher.matches()) {
       return String.format("%s-%s.%s", matcher.group(1), checksum, 
matcher.group(2));
     }
     return String.format("%s-%s", remoteFileName, checksum);
   }
 
-  private static String tempName(String baseName) {
-    return "." + requireNonNull(baseName) + "_PID" + PID + "_" + 
UUID.randomUUID() + ".tmp";
+  // creates a new empty file with a unique name, for use as a temporary file
+  @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN",
+      justification = "the working directory is intentionally controlled by 
the user config")
+  private Path createTempFile(String baseName) {
+    try {
+      return Files.createTempFile(workingDir, "PID_" + PID + "_" + baseName + 
"_", ".tmp");
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    }
   }
 
-  static String checksumForFileName(String algorithm, String checksum) {
-    return algorithm.replace('/', '_') + "-" + checksum;
+  // creates a new empty directory with a unique name, for use as a temporary 
directory
+  @SuppressFBWarnings(value = "PATH_TRAVERSAL_IN",
+      justification = "the working directory is intentionally controlled by 
the user config")
+  private Path createTempDirectory(String baseName) {
+    try {
+      return Files.createTempDirectory(workingDir, "PID_" + PID + "_" + 
baseName + "_");
+    } catch (IOException e) {
+      throw new UncheckedIOException(e);
+    }
   }
 
   /**
    * Save the {@link ContextDefinition} to the contexts directory, and all of 
its resources to the
    * resources directory.
    */
-  public URL[] storeContextResources(final ContextDefinition 
contextDefinition) throws IOException {
+  public void storeContextResources(final ContextDefinition contextDefinition) 
{
     requireNonNull(contextDefinition, "definition must be supplied");
-    // use a LinkedHashSet to preserve the order of the context resources
-    final Set<Path> localFiles = new LinkedHashSet<>();
-    final String destinationName = 
checksumForFileName(contextDefinition.getChecksumAlgorithm(),
-        contextDefinition.getChecksum()) + ".json";
+    final String destinationName = checksumForFileName(contextDefinition) + 
".json";
     try {
       storeContextDefinition(contextDefinition, destinationName);
       boolean successful = false;
       while (!successful) {

Review Comment:
   That's what I had originally, but I've been advised in the past to avoid 
that form, because it's more rare, and therefore less intuitive. I generally 
don't care, but if the code is similarly in readability, I tend to avoid 
do-while for that reason. I can make the change, since you suggested it.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to