[1.9.x] TrackingFileManager changes

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManager.java

@@ -21,17 +21,19 @@
 import javax.inject.Named;
 import javax.inject.Singleton;
 
-import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.IOException;
-import java.io.RandomAccessFile;
 import java.io.UncheckedIOException;
+import java.nio.ByteBuffer;
+import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.channels.OverlappingFileLockException;
 import java.nio.file.Files;
+import java.nio.file.NoSuchFileException;
+import java.nio.file.Path;
+import java.nio.file.StandardOpenOption;
 import java.util.Map;
 import java.util.Properties;
 
@@ -45,6 +47,9 @@
  * to back off two parallel implementations that coexist in Maven (this class and {@code maven-compat} one), as in
  * certain cases the two implementations may collide on properties files. This locking must remain in place for as long
  * as {@code maven-compat} code exists.
+ *
+ * <em>IMPORTANT:</em> This class is kept fully in sync with the master branch one (w/ simple change to convert File
+ * to Path instances).
  */
 @Singleton
 @Named
@@ -53,15 +58,18 @@ public final class DefaultTrackingFileManager implements TrackingFileManager {
 
     @Override
     public Properties read(File file) {
-        if (Files.isReadable(file.toPath())) {
-            synchronized (getMutex(file)) {
-                try (FileInputStream stream = new FileInputStream(file);
-                        FileLock unused = fileLock(stream.getChannel(), Math.max(1, file.length()), true)) {
+        Path path = file.toPath();
+        if (Files.isReadable(path)) {
+            synchronized (mutex(path)) {
+                try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.READ);
+                        FileLock unused = fileLock(fileChannel, true)) {
                     Properties props = new Properties();
-                    props.load(stream);
+                    props.load(Channels.newInputStream(fileChannel));
                     return props;
+                } catch (NoSuchFileException e) {
+                    LOGGER.debug("No such file to read {}: {}", path, e.getMessage());
                 } catch (IOException e) {
-                    LOGGER.warn("Failed to read tracking file '{}'", file, e);
+                    LOGGER.warn("Failed to read tracking file '{}'", path, e);
                     throw new UncheckedIOException(e);
                 }
             }
@@ -71,22 +79,20 @@ public Properties read(File file) {
 
     @Override
     public Properties update(File file, Map<String, String> updates) {
-        Properties props = new Properties();
-
+        Path path = file.toPath();
         try {
-            Files.createDirectories(file.getParentFile().toPath());
+            Files.createDirectories(path.getParent());
         } catch (IOException e) {
-            LOGGER.warn("Failed to create tracking file parent '{}'", file, e);
+            LOGGER.warn("Failed to create tracking file parent '{}'", path, e);
             throw new UncheckedIOException(e);
         }
-
-        synchronized (getMutex(file)) {
-            try (RandomAccessFile raf = new RandomAccessFile(file, "rw");
-                    FileLock unused = fileLock(raf.getChannel(), Math.max(1, raf.length()), false)) {
-                if (raf.length() > 0) {
-                    byte[] buffer = new byte[(int) raf.length()];
-                    raf.readFully(buffer);
-                    props.load(new ByteArrayInputStream(buffer));
+        synchronized (mutex(path)) {
+            try (FileChannel fileChannel = FileChannel.open(
+                            path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE);
+                    FileLock unused = fileLock(fileChannel, false)) {
+                Properties props = new Properties();
+                if (fileChannel.size() > 0) {
+                    props.load(Channels.newInputStream(fileChannel));
                 }
 
                 for (Map.Entry<String, String> update : updates.entrySet()) {
@@ -97,46 +103,54 @@ public Properties update(File file, Map<String, String> updates) {
                     }
                 }
 
-                LOGGER.debug("Writing tracking file '{}'", file);
+                LOGGER.debug("Writing tracking file '{}'", path);
                 ByteArrayOutputStream stream = new ByteArrayOutputStream(1024 * 2);
                 props.store(
                         stream,
                         "NOTE: This is a Maven Resolver internal implementation file"
                                 + ", its format can be changed without prior notice.");
-                raf.seek(0L);
-                raf.write(stream.toByteArray());
-                raf.setLength(raf.getFilePointer());
+                fileChannel.position(0);
+                int written = fileChannel.write(ByteBuffer.wrap(stream.toByteArray()));
+                fileChannel.truncate(written);
+                return props;
             } catch (IOException e) {
-                LOGGER.warn("Failed to write tracking file '{}'", file, e);
+                LOGGER.warn("Failed to write tracking file '{}'", path, e);
                 throw new UncheckedIOException(e);
             }
         }
+    }
 
-        return props;
+    @Override
+    public boolean delete(File file) {
+        Path path = file.toPath();
+        if (Files.isReadable(path)) {
+            synchronized (mutex(path)) {
+                try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.WRITE);
+                        FileLock unused = fileLock(fileChannel, false)) {
+                    Files.delete(path);
+                    return true;
+                } catch (NoSuchFileException e) {
+                    LOGGER.debug("No such file to delete {}: {}", path, e.getMessage());
+                } catch (IOException e) {
+                    LOGGER.warn("Failed to delete tracking file '{}'", path, e);
+                    throw new UncheckedIOException(e);
+                }
+            }
+        }
+        return false;
     }
 
-    private Object getMutex(File file) {
+    private Object mutex(Path path) {
         // The interned string of path is (mis)used as mutex, to exclude different threads going for same file,
         // as JVM file locking happens on JVM not on Thread level. This is how original code did it  ¯\_(ツ)_/¯
-        /*
-         * NOTE: Locks held by one JVM must not overlap and using the canonical path is our best bet, still another
-         * piece of code might have locked the same file (unlikely though) or the canonical path fails to capture file
-         * identity sufficiently as is the case with Java 1.6 and symlinks on Windows.
-         */
-        try {
-            return file.getCanonicalPath().intern();
-        } catch (IOException e) {
-            LOGGER.warn("Failed to canonicalize path {}", file, e);
-            // TODO This is code smell and deprecated
-            return file.getAbsolutePath().intern();
-        }
+        return path.toAbsolutePath().normalize().toString().intern();
     }
 
-    private FileLock fileLock(FileChannel channel, long size, boolean shared) throws IOException {
+    private FileLock fileLock(FileChannel channel, boolean shared) throws IOException {
         FileLock lock = null;
         for (int attempts = 8; attempts >= 0; attempts--) {
             try {
-                lock = channel.lock(0, size, shared);
+                lock = channel.lock(0, Long.MAX_VALUE, shared);
                 break;
             } catch (OverlappingFileLockException e) {
                 if (attempts <= 0) {

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultUpdateCheckManager.java

@@ -476,7 +476,7 @@ public void touchArtifact(RepositorySystemSession session, UpdateCheck<Artifact,
         Properties props = write(touchFile, dataKey, transferKey, check.getException());
 
         if (artifactFile.exists() && !hasErrors(props)) {
-            touchFile.delete();
+            trackingFileManager.delete(touchFile);
         }
     }
 

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/TrackingFileManager.java

@@ -36,4 +36,11 @@ public interface TrackingFileManager {
      * as in updated file, never {@code null}.
      */
     Properties update(File file, Map<String, String> updates);
+
+    /**
+     * Deletes the specified properties file, if exists. If file existed and was deleted, returns {@code true}.
+     *
+     * @since 1.9.25
+     */
+    boolean delete(File file);
 }

maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultTrackingFileManagerTest.java

@@ -20,6 +20,7 @@
 
 import java.io.File;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,6 +32,7 @@
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -104,6 +106,17 @@ public void testUpdateNoFileLeak() throws Exception {
         }
     }
 
+    @Test
+    public void testDeleteFileIsGone() throws Exception {
+        TrackingFileManager tfm = new DefaultTrackingFileManager();
+
+        for (int i = 0; i < 1000; i++) {
+            File propFile = TestFileUtils.createTempFile("#COMMENT\nkey1=value1\nkey2 : value2");
+            assertTrue(tfm.delete(propFile));
+            assertFalse("File is not gone", Files.isRegularFile(propFile.toPath()));
+        }
+    }
+
     @Test
     public void testLockingOnCanonicalPath() throws Exception {
         final TrackingFileManager tfm = new DefaultTrackingFileManager();