[MRESOLVER-282] Drop PartialFile use (#212)

Drop PartialFile, clean up regarding resource handling and lessen temp file use. Downside: there is no "download resume" happening anymore.

On the other hand, from now on, resolver will download "all or nothing", so no more "partial downloads" can happen either.

HttpClientTransport is STILL ABLE to resume downloads (was the only one anyway), but due removal of PartialFile it will never resume. Later we may replace removed logic with something simpler.

---

https://issues.apache.org/jira/browse/MRESOLVER-282

Author: cstamas

maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/BasicRepositoryConnector.java

@@ -68,6 +68,7 @@
 import org.eclipse.aether.transfer.TransferResource;
 import org.eclipse.aether.transform.FileTransformer;
 import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.FileUtils;
 import org.eclipse.aether.util.concurrency.RunnableErrorForwarder;
 import org.eclipse.aether.util.concurrency.WorkerThreadFactory;
 import org.slf4j.Logger;
@@ -82,10 +83,6 @@ final class BasicRepositoryConnector
 
     private static final String CONFIG_PROP_THREADS = "aether.connector.basic.threads";
 
-    private static final String CONFIG_PROP_RESUME = "aether.connector.resumeDownloads";
-
-    private static final String CONFIG_PROP_RESUME_THRESHOLD = "aether.connector.resumeThreshold";
-
     private static final String CONFIG_PROP_SMART_CHECKSUMS = "aether.connector.smartChecksums";
 
     private static final Logger LOGGER = LoggerFactory.getLogger( BasicRepositoryConnector.class );
@@ -104,8 +101,6 @@ final class BasicRepositoryConnector
 
     private final ChecksumPolicyProvider checksumPolicyProvider;
 
-    private final PartialFile.Factory partialFileFactory;
-
     private final int maxThreads;
 
     private final boolean smartChecksums;
@@ -154,18 +149,6 @@ final class BasicRepositoryConnector
         persistedChecksums =
                 ConfigUtils.getBoolean( session, ConfigurationProperties.DEFAULT_PERSISTED_CHECKSUMS,
                         ConfigurationProperties.PERSISTED_CHECKSUMS );
-
-        boolean resumeDownloads =
-                ConfigUtils.getBoolean( session, true, CONFIG_PROP_RESUME + '.' + repository.getId(),
-                        CONFIG_PROP_RESUME );
-        long resumeThreshold =
-                ConfigUtils.getLong( session, 64 * 1024, CONFIG_PROP_RESUME_THRESHOLD + '.' + repository.getId(),
-                        CONFIG_PROP_RESUME_THRESHOLD );
-        int requestTimeout =
-                ConfigUtils.getInteger( session, ConfigurationProperties.DEFAULT_REQUEST_TIMEOUT,
-                        ConfigurationProperties.REQUEST_TIMEOUT + '.' + repository.getId(),
-                        ConfigurationProperties.REQUEST_TIMEOUT );
-        partialFileFactory = new PartialFile.Factory( resumeDownloads, resumeThreshold, requestTimeout );
     }
 
     private Executor getExecutor( Collection<?> artifacts, Collection<?> metadatas )
@@ -430,7 +413,7 @@ protected void runTask()
 
     class GetTaskRunner
             extends TaskRunner
-            implements PartialFile.RemoteAccessChecker, ChecksumValidator.ChecksumFetcher
+            implements ChecksumValidator.ChecksumFetcher
     {
 
         private final File file;
@@ -449,13 +432,6 @@ class GetTaskRunner
                     checksumPolicy, providedChecksums, safe( checksumLocations ) );
         }
 
-        @Override
-        public void checkRemoteAccess()
-                throws Exception
-        {
-            transporter.peek( new PeekTask( path ) );
-        }
-
         @Override
         public boolean fetchChecksum( URI remote, File local )
                 throws Exception
@@ -481,21 +457,13 @@ protected void runTask()
         {
             fileProcessor.mkdirs( file.getParentFile() );
 
-            PartialFile partFile = partialFileFactory.newInstance( file, this );
-            if ( partFile == null )
-            {
-                LOGGER.debug( "Concurrent download of {} just finished, skipping download", file );
-                return;
-            }
-
-            try
+            try ( FileUtils.TempFile tempFile = FileUtils.newTempFile( file.toPath() ) )
             {
-                File tmp = partFile.getFile();
+                final File tmp = tempFile.getPath().toFile();
                 listener.setChecksumCalculator( checksumValidator.newChecksumCalculator( tmp ) );
                 for ( int firstTrial = 0, lastTrial = 1, trial = firstTrial; ; trial++ )
                 {
-                    boolean resume = partFile.isResume() && trial <= firstTrial;
-                    GetTask task = new GetTask( path ).setDataFile( tmp, resume ).setListener( listener );
+                    GetTask task = new GetTask( path ).setDataFile( tmp, false ).setListener( listener );
                     transporter.get( task );
                     try
                     {
@@ -527,11 +495,6 @@ protected void runTask()
                     checksumValidator.commit();
                 }
             }
-            finally
-            {
-                partFile.close();
-                checksumValidator.close();
-            }
         }
 
     }

maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/ChecksumValidator.java

@@ -24,16 +24,15 @@
 import java.net.URI;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.UUID;
 
 import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
 import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy;
 import org.eclipse.aether.spi.connector.checksum.ChecksumPolicy.ChecksumKind;
 import org.eclipse.aether.spi.connector.layout.RepositoryLayout.ChecksumLocation;
 import org.eclipse.aether.spi.io.FileProcessor;
 import org.eclipse.aether.transfer.ChecksumFailureException;
+import org.eclipse.aether.util.FileUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -61,8 +60,6 @@ boolean fetchChecksum( URI remote, File local )
 
     private final Collection<ChecksumAlgorithmFactory> checksumAlgorithmFactories;
 
-    private final Collection<File> tempFiles;
-
     private final FileProcessor fileProcessor;
 
     private final ChecksumFetcher checksumFetcher;
@@ -73,7 +70,7 @@ boolean fetchChecksum( URI remote, File local )
 
     private final Collection<ChecksumLocation> checksumLocations;
 
-    private final Map<File, Object> checksumFiles;
+    private final Map<File, String> checksumExpectedValues;
 
     ChecksumValidator( File dataFile,
                        Collection<ChecksumAlgorithmFactory> checksumAlgorithmFactories,
@@ -85,13 +82,12 @@ boolean fetchChecksum( URI remote, File local )
     {
         this.dataFile = dataFile;
         this.checksumAlgorithmFactories = checksumAlgorithmFactories;
-        this.tempFiles = new HashSet<>();
         this.fileProcessor = fileProcessor;
         this.checksumFetcher = checksumFetcher;
         this.checksumPolicy = checksumPolicy;
         this.providedChecksums = providedChecksums;
         this.checksumLocations = checksumLocations;
-        this.checksumFiles = new HashMap<>();
+        this.checksumExpectedValues = new HashMap<>();
     }
 
     public ChecksumCalculator newChecksumCalculator( File targetFile )
@@ -152,7 +148,7 @@ private boolean validateChecksums( Map<String, ?> actualChecksums, ChecksumKind
 
             String actual = String.valueOf( calculated );
             String expected = entry.getValue().toString();
-            checksumFiles.put( getChecksumFile( checksumAlgorithmFactory ), expected );
+            checksumExpectedValues.put( getChecksumFile( checksumAlgorithmFactory ), expected );
 
             if ( !isEqualChecksum( expected, actual ) )
             {
@@ -183,10 +179,10 @@ private boolean validateExternalChecksums( Map<String, ?> actualChecksums )
                 );
                 continue;
             }
-            try
+            File checksumFile = getChecksumFile( checksumLocation.getChecksumAlgorithmFactory() );
+            try ( FileUtils.TempFile tempFile = FileUtils.newTempFile( checksumFile.toPath() ) )
             {
-                File checksumFile = getChecksumFile( checksumLocation.getChecksumAlgorithmFactory() );
-                File tmp = createTempFile( checksumFile );
+                File tmp = tempFile.getPath().toFile();
                 try
                 {
                     if ( !checksumFetcher.fetchChecksum(
@@ -206,7 +202,7 @@ private boolean validateExternalChecksums( Map<String, ?> actualChecksums )
 
                 String actual = String.valueOf( calculated );
                 String expected = fileProcessor.readChecksum( tmp );
-                checksumFiles.put( checksumFile, tmp );
+                checksumExpectedValues.put( checksumFile, expected );
 
                 if ( !isEqualChecksum( expected, actual ) )
                 {
@@ -240,33 +236,10 @@ private File getChecksumFile( ChecksumAlgorithmFactory factory )
         return new File( dataFile.getPath() + '.' + factory.getFileExtension() );
     }
 
-    private File createTempFile( File path )
-        throws IOException
-    {
-        File file =
-            File.createTempFile( path.getName() + "-"
-                + UUID.randomUUID().toString().replace( "-", "" ).substring( 0, 8 ), ".tmp", path.getParentFile() );
-        tempFiles.add( file );
-        return file;
-    }
-
-    private void clearTempFiles()
-    {
-        for ( File file : tempFiles )
-        {
-            if ( !file.delete() && file.exists() )
-            {
-                LOGGER.debug( "Could not delete temporary file {}", file );
-            }
-        }
-        tempFiles.clear();
-    }
-
     public void retry()
     {
         checksumPolicy.onTransferRetry();
-        checksumFiles.clear();
-        clearTempFiles();
+        checksumExpectedValues.clear();
     }
 
     public boolean handle( ChecksumFailureException exception )
@@ -276,33 +249,18 @@ public boolean handle( ChecksumFailureException exception )
 
     public void commit()
     {
-        for ( Map.Entry<File, Object> entry : checksumFiles.entrySet() )
+        for ( Map.Entry<File, String> entry : checksumExpectedValues.entrySet() )
         {
             File checksumFile = entry.getKey();
-            Object tmp = entry.getValue();
             try
             {
-                if ( tmp instanceof File )
-                {
-                    fileProcessor.move( (File) tmp, checksumFile );
-                    tempFiles.remove( tmp );
-                }
-                else
-                {
-                    fileProcessor.writeChecksum( checksumFile, String.valueOf( tmp ) );
-                }
+                fileProcessor.writeChecksum( checksumFile, entry.getValue() );
             }
             catch ( IOException e )
             {
                 LOGGER.debug( "Failed to write checksum file {}", checksumFile, e );
             }
         }
-        checksumFiles.clear();
+        checksumExpectedValues.clear();
     }
-
-    public void close()
-    {
-        clearTempFiles();
-    }
-
 }