[MNG-1908] recommitting fix for SNAPSHOT handling. As far as I can tell, this doesn't incur the performance penalty it used to.

git-svn-id: https://svn.apache.org/repos/asf/maven/components/trunk@469040 13f79535-47bb-0310-9956-ffa450edef68

maven-artifact-manager/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java

@@ -62,7 +62,7 @@ public class DefaultWagonManager
     extends AbstractLogEnabled
     implements WagonManager, Contextualizable
 {
-    private static final String WILDCARD = "*"; 
+    private static final String WILDCARD = "*";
 
     private PlexusContainer container;
 
@@ -75,8 +75,10 @@ public class DefaultWagonManager
     private Map serverPermissionsMap = new HashMap();
 
     private Map mirrors = new HashMap();
-    
+
-    /** Map( String, XmlPlexusConfiguration ) with the repository id and the wagon configuration */
+    /**
+     * Map( String, XmlPlexusConfiguration ) with the repository id and the wagon configuration
+     */
     private Map serverConfigurationMap = new HashMap();
 
     private TransferListener downloadMonitor;
@@ -148,14 +150,14 @@ public class DefaultWagonManager
         try
         {
             wagon = getWagon( protocol );
-            
+
             configureWagon( wagon, repository );
         }
         catch ( UnsupportedProtocolException e )
         {
             throw new TransferFailedException( "Unsupported Protocol: '" + protocol + "': " + e.getMessage(), e );
         }
-        
+
         if ( downloadMonitor != null )
         {
             wagon.addTransferListener( downloadMonitor );
@@ -280,7 +282,8 @@ public class DefaultWagonManager
             }
         }
 
-        if ( !successful )
+        // if it already exists locally we were just trying to force it - ignore the update
+        if ( !successful && !artifact.getFile().exists() )
         {
             throw new ResourceDoesNotExistException( "Unable to download the artifact from any repository" );
         }
@@ -321,8 +324,8 @@ public class DefaultWagonManager
     }
 
     private void getRemoteFile( ArtifactRepository repository, File destination, String remotePath,
-                                TransferListener downloadMonitor, String checksumPolicy )
+                                   TransferListener downloadMonitor, String checksumPolicy )
-        throws TransferFailedException, ResourceDoesNotExistException, ChecksumFailedException
+        throws TransferFailedException, ResourceDoesNotExistException
     {
         // TODO: better excetpions - transfer failed is not enough?
 
@@ -374,6 +377,8 @@ public class DefaultWagonManager
         File temp = new File( destination + ".tmp" );
         temp.deleteOnExit();
 
+        boolean downloaded = false;
+
         try
         {
             wagon.connect( new Repository( repository.getId(), repository.getUrl() ),
@@ -391,51 +396,38 @@ public class DefaultWagonManager
                 retry = false;
 
                 // This should take care of creating destination directory now on
-                wagon.get( remotePath, temp );
+                if ( destination.exists() )
-
-                // keep the checksum files from showing up on the download monitor...
-                if ( downloadMonitor != null )
                 {
-                    wagon.removeTransferListener( downloadMonitor );
+                    downloaded = wagon.getIfNewer( remotePath, temp, destination.lastModified() );
+                }
+                else
+                {
+                    wagon.get( remotePath, temp );
+                    downloaded = true;
                 }
 
-                // try to verify the SHA-1 checksum for this file.
+                if ( downloaded )
-                try
                 {
-                    verifyChecksum( sha1ChecksumObserver, destination, temp, remotePath, ".sha1", wagon );
+                    // keep the checksum files from showing up on the download monitor...
-                }
+                    if ( downloadMonitor != null )
-                catch ( ChecksumFailedException e )
-                {
-                    // if we catch a ChecksumFailedException, it means the transfer/read succeeded, but the checksum
-                    // doesn't match. This could be a problem with the server (ibiblio HTTP-200 error page), so we'll
-                    // try this up to two times. On the second try, we'll handle it as a bona-fide error, based on the
-                    // repository's checksum checking policy.
-                    if ( firstRun )
                     {
-                        getLogger().warn( "*** CHECKSUM FAILED - " + e.getMessage() + " - RETRYING" );
+                        wagon.removeTransferListener( downloadMonitor );
-                        retry = true;
                     }
-                    else
-                    {
-                        handleChecksumFailure( checksumPolicy, e.getMessage(), e.getCause() );
-                    }
-                }
-                catch ( ResourceDoesNotExistException sha1TryException )
-                {
-                    getLogger().debug( "SHA1 not found, trying MD5", sha1TryException );
 
-                    // if this IS NOT a ChecksumFailedException, it was a problem with transfer/read of the checksum
+                    // try to verify the SHA-1 checksum for this file.
-                    // file...we'll try again with the MD5 checksum.
                     try
                     {
-                        verifyChecksum( md5ChecksumObserver, destination, temp, remotePath, ".md5", wagon );
+                        verifyChecksum( sha1ChecksumObserver, destination, temp, remotePath, ".sha1", wagon );
                     }
                     catch ( ChecksumFailedException e )
                     {
-                        // if we also fail to verify based on the MD5 checksum, and the checksum transfer/read
+                        // if we catch a ChecksumFailedException, it means the transfer/read succeeded, but the checksum
-                        // succeeded, then we need to determine whether to retry or handle it as a failure.
+                        // doesn't match. This could be a problem with the server (ibiblio HTTP-200 error page), so we'll
+                        // try this up to two times. On the second try, we'll handle it as a bona-fide error, based on the
+                        // repository's checksum checking policy.
                         if ( firstRun )
                         {
+                            getLogger().warn( "*** CHECKSUM FAILED - " + e.getMessage() + " - RETRYING" );
                             retry = true;
                         }
                         else
@@ -443,18 +435,42 @@ public class DefaultWagonManager
                             handleChecksumFailure( checksumPolicy, e.getMessage(), e.getCause() );
                         }
                     }
-                    catch ( ResourceDoesNotExistException md5TryException )
+                    catch ( ResourceDoesNotExistException sha1TryException )
                     {
-                        // this was a failed transfer, and we don't want to retry.
+                        getLogger().debug( "SHA1 not found, trying MD5", sha1TryException );
-                        handleChecksumFailure( checksumPolicy, "Error retrieving checksum file for " + remotePath,
-                                               md5TryException );
-                    }
-                }
 
-                // reinstate the download monitor...
+                        // if this IS NOT a ChecksumFailedException, it was a problem with transfer/read of the checksum
-                if ( downloadMonitor != null )
+                        // file...we'll try again with the MD5 checksum.
-                {
+                        try
-                    wagon.addTransferListener( downloadMonitor );
+                        {
+                            verifyChecksum( md5ChecksumObserver, destination, temp, remotePath, ".md5", wagon );
+                        }
+                        catch ( ChecksumFailedException e )
+                        {
+                            // if we also fail to verify based on the MD5 checksum, and the checksum transfer/read
+                            // succeeded, then we need to determine whether to retry or handle it as a failure.
+                            if ( firstRun )
+                            {
+                                retry = true;
+                            }
+                            else
+                            {
+                                handleChecksumFailure( checksumPolicy, e.getMessage(), e.getCause() );
+                            }
+                        }
+                        catch ( ResourceDoesNotExistException md5TryException )
+                        {
+                            // this was a failed transfer, and we don't want to retry.
+                            handleChecksumFailure( checksumPolicy, "Error retrieving checksum file for " + remotePath,
+                                                   md5TryException );
+                        }
+                    }
+
+                    // reinstate the download monitor...
+                    if ( downloadMonitor != null )
+                    {
+                        wagon.addTransferListener( downloadMonitor );
+                    }
                 }
 
                 // unset the firstRun flag, so we don't get caught in an infinite loop...
@@ -480,29 +496,32 @@ public class DefaultWagonManager
             releaseWagon( wagon );
         }
 
-        if ( !temp.exists() )
+        if ( downloaded )
         {
-            throw new ResourceDoesNotExistException( "Downloaded file does not exist: " + temp );
+            if ( !temp.exists() )
-        }
-
-        // The temporary file is named destination + ".tmp" and is done this way to ensure
-        // that the temporary file is in the same file system as the destination because the
-        // File.renameTo operation doesn't really work across file systems.
-        // So we will attempt to do a File.renameTo for efficiency and atomicity, if this fails
-        // then we will use a brute force copy and delete the temporary file.
-
-        if ( !temp.renameTo( destination ) )
-        {
-            try
             {
-                FileUtils.copyFile( temp, destination );
+                throw new ResourceDoesNotExistException( "Downloaded file does not exist: " + temp );
-
-                temp.delete();
             }
-            catch ( IOException e )
+
+            // The temporary file is named destination + ".tmp" and is done this way to ensure
+            // that the temporary file is in the same file system as the destination because the
+            // File.renameTo operation doesn't really work across file systems.
+            // So we will attempt to do a File.renameTo for efficiency and atomicity, if this fails
+            // then we will use a brute force copy and delete the temporary file.
+
+            if ( !temp.renameTo( destination ) )
             {
-                throw new TransferFailedException(
+                try
-                    "Error copying temporary file to the final destination: " + e.getMessage(), e );
+                {
+                    FileUtils.copyFile( temp, destination );
+
+                    temp.delete();
+                }
+                catch ( IOException e )
+                {
+                    throw new TransferFailedException(
+                        "Error copying temporary file to the final destination: " + e.getMessage(), e );
+                }
             }
         }
     }
@@ -531,8 +550,8 @@ public class DefaultWagonManager
         // otherwise it is ignore
     }
 
-    private void verifyChecksum( ChecksumObserver checksumObserver, File destination, File tempDestination, String remotePath,
+    private void verifyChecksum( ChecksumObserver checksumObserver, File destination, File tempDestination,
-                                 String checksumFileExtension, Wagon wagon )
+                                 String remotePath, String checksumFileExtension, Wagon wagon )
         throws ResourceDoesNotExistException, TransferFailedException, AuthorizationException
     {
         try
@@ -568,7 +587,10 @@ public class DefaultWagonManager
             if ( expectedChecksum.equals( actualChecksum ) )
             {
                 File checksumFile = new File( destination + checksumFileExtension );
-                if ( checksumFile.exists() ) checksumFile.delete();
+                if ( checksumFile.exists() )
+                {
+                    checksumFile.delete();
+                }
                 FileUtils.copyFile( tempChecksumFile, checksumFile );
             }
             else
@@ -630,13 +652,13 @@ public class DefaultWagonManager
     /**
      * Set the proxy used for a particular protocol.
      *
-     * @param protocol the protocol (required)
+     * @param protocol      the protocol (required)
-     * @param host the proxy host name (required)
+     * @param host          the proxy host name (required)
-     * @param port the proxy port (required)
+     * @param port          the proxy port (required)
-     * @param username the username for the proxy, or null if there is none
+     * @param username      the username for the proxy, or null if there is none
-     * @param password the password for the proxy, or null if there is none
+     * @param password      the password for the proxy, or null if there is none
      * @param nonProxyHosts the set of hosts not to use the proxy for. Follows Java system
-     * property format: <code>*.foo.com|localhost</code>.
+     *                      property format: <code>*.foo.com|localhost</code>.
      * @todo [BP] would be nice to configure this via plexus in some way
      */
     public void addProxy( String protocol, String host, int port, String username, String password,
@@ -728,19 +750,19 @@ public class DefaultWagonManager
     {
         this.interactive = interactive;
     }
-    
+
 
     /**
      * Applies the server configuration to the wagon
-     * 
+     *
-     * @param wagon the wagon to configure
+     * @param wagon      the wagon to configure
      * @param repository the repository that has the configuration
      * @throws WagonConfigurationException wraps any error given during configuration of the wagon instance
      */
     private void configureWagon( Wagon wagon, ArtifactRepository repository )
         throws WagonConfigurationException
     {
-       configureWagon( wagon, repository.getId() );
+        configureWagon( wagon, repository.getId() );
     }
 
     private void configureWagon( Wagon wagon, String repositoryId )
@@ -757,7 +779,9 @@ public class DefaultWagonManager
             }
             catch ( final ComponentLookupException e )
             {
-                throw new WagonConfigurationException( repositoryId, "Unable to lookup wagon configurator. Wagon configuration cannot be applied.", e );
+                throw new WagonConfigurationException( repositoryId,
+                                                       "Unable to lookup wagon configurator. Wagon configuration cannot be applied.",
+                                                       e );
             }
             catch ( ComponentConfigurationException e )
             {

maven-artifact-manager/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java

@@ -78,10 +78,11 @@ public class DefaultRepositoryMetadataManager
                     File file = new File( localRepository.getBasedir(),
                                           localRepository.pathOfLocalRepositoryMetadata( metadata, repository ) );
 
-                    boolean checkForUpdates = policy.checkOutOfDate( new Date( file.lastModified() ) ) || !file.exists();
+                    boolean checkForUpdates =
+                        policy.checkOutOfDate( new Date( file.lastModified() ) ) || !file.exists();
 
                     boolean metadataIsEmpty = true;
-                    
+
                     if ( checkForUpdates )
                     {
                         getLogger().info( metadata.getKey() + ": checking for updates from " + repository.getId() );
@@ -199,6 +200,8 @@ public class DefaultRepositoryMetadataManager
 
                     if ( !m.getVersioning().getSnapshot().isLocalCopy() )
                     {
+                        // TODO: I think this is incorrect (it results in localCopy set in a remote profile). Probably
+                        //   harmless so not removing at this point until full tests in place.
                         m.getVersioning().getSnapshot().setLocalCopy( true );
                         metadata.setMetadata( m );
                         metadata.storeInLocalRepository( localRepository, repository );
@@ -370,7 +373,7 @@ public class DefaultRepositoryMetadataManager
             getLogger().info( "Repository '" + repository.getId() + "' will be blacklisted" );
             getLogger().debug( "Exception", e );
             repository.setBlacklisted( allowBlacklisting );
-            
+
             throw e;
         }
     }

maven-artifact-manager/src/main/java/org/apache/maven/artifact/resolver/DefaultArtifactResolver.java

@@ -21,6 +21,7 @@ import org.apache.maven.artifact.factory.ArtifactFactory;
 import org.apache.maven.artifact.manager.WagonManager;
 import org.apache.maven.artifact.metadata.ArtifactMetadataSource;
 import org.apache.maven.artifact.repository.ArtifactRepository;
+import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy;
 import org.apache.maven.artifact.resolver.filter.ArtifactFilter;
 import org.apache.maven.artifact.transform.ArtifactTransformationManager;
 import org.apache.maven.wagon.ResourceDoesNotExistException;
@@ -32,6 +33,7 @@ import java.io.File;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -104,6 +106,32 @@ public class DefaultArtifactResolver
                 transformationManager.transformForResolve( artifact, remoteRepositories, localRepository );
 
                 File destination = artifact.getFile();
+                List repositories = remoteRepositories;
+
+                // TODO: would prefer the snapshot transformation took care of this. Maybe we need a "shouldresolve" flag.
+                if ( artifact.isSnapshot() && artifact.getBaseVersion().equals( artifact.getVersion() ) &&
+                    destination.exists() )
+                {
+                    Date comparisonDate = new Date( destination.lastModified() );
+
+                    // cull to list of repositories that would like an update
+                    repositories = new ArrayList( remoteRepositories );
+                    for ( Iterator i = repositories.iterator(); i.hasNext(); )
+                    {
+                        ArtifactRepository repository = (ArtifactRepository) i.next();
+                        ArtifactRepositoryPolicy policy = repository.getSnapshots();
+                        if ( !policy.isEnabled() || !policy.checkOutOfDate( comparisonDate ) )
+                        {
+                            i.remove();
+                        }
+                    }
+
+                    if ( !repositories.isEmpty() )
+                    {
+                        // someone wants to check for updates
+                        force = true;
+                    }
+                }
                 boolean resolved = false;
                 if ( !destination.exists() || force )
                 {
@@ -121,10 +149,10 @@ public class DefaultArtifactResolver
                         }
                         else
                         {
-                            wagonManager.getArtifact( artifact, remoteRepositories );
+                            wagonManager.getArtifact( artifact, repositories );
                         }
 
-                        if ( !artifact.isResolved() )
+                        if ( !artifact.isResolved() && !destination.exists() )
                         {
                             throw new ArtifactResolutionException(
                                 "Failed to resolve artifact, possibly due to a repository list that is not appropriately equipped for this artifact's metadata.",