[MRM-586] transfer failures cause proxying to fail. Added test and some notes on missing tests

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

3 files changed, 118 insertions, 59 deletions

diff --git a/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java b/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
index 70d9c0f20..d2656b7f6 100644
--- a/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
+++ b/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
@@ -69,8 +69,8 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Properties;
 import java.util.Map.Entry;
+import java.util.Properties;
 
 /**
  * DefaultRepositoryProxyConnectors
@@ -137,7 +137,6 @@ public class DefaultRepositoryProxyConnectors
      * @throws ProxyException if there was a problem fetching the artifact.
      */
     public File fetchFromProxies( ManagedRepositoryContent repository, ArtifactReference artifact )
-        throws ProxyException
     {
         File localFile = toLocalFile( repository, artifact );
 
@@ -172,6 +171,13 @@ public class DefaultRepositoryProxyConnectors
                 getLogger().debug( "Artifact " + Keys.toKey( artifact ) + " not updated on repository \""
                                        + targetRepository.getRepository().getId() + "\"." );
             }
+            catch ( ProxyException e )
+            {
+                getLogger().warn( "Transfer error from repository \"" + targetRepository.getRepository().getId() +
+                    "\" for artifact " + Keys.toKey( artifact ) + ", continuing to next repository. Error message: " +
+                    e.getMessage() );
+                getLogger().debug( "Full stack trace", e );
+            }
         }
         getLogger().debug( "Exhausted all target repositories, artifact " + Keys.toKey( artifact ) + " not found." );
 
@@ -184,7 +190,6 @@ public class DefaultRepositoryProxyConnectors
      * @return the (local) metadata file that was fetched/merged/updated, or null if no metadata file exists.
      */
     public File fetchFromProxies( ManagedRepositoryContent repository, VersionedReference metadata )
-        throws ProxyException
     {
         File localFile = toLocalFile( repository, metadata );
 
@@ -201,7 +206,7 @@ public class DefaultRepositoryProxyConnectors
 
             File localRepoFile = toLocalRepoFile( repository, targetRepository, targetPath );
             long originalMetadataTimestamp = getLastModified( localRepoFile );
-            
+
             try
             {
                 transferFile( connector, targetRepository, targetPath, localRepoFile, requestProperties );
@@ -223,6 +228,13 @@ public class DefaultRepositoryProxyConnectors
                                        + " not updated on remote repository \""
                                        + targetRepository.getRepository().getId() + "\"." );
             }
+            catch ( ProxyException e )
+            {
+                getLogger().warn( "Transfer error from repository \"" + targetRepository.getRepository().getId() +
+                    "\" for versioned Metadata " + Keys.toKey( metadata ) +
+                    ", continuing to next repository. Error message: " + e.getMessage() );
+                getLogger().debug( "Full stack trace", e );
+            }
         }
 
         if ( hasBeenUpdated( localFile, originalTimestamp ) )
@@ -278,14 +290,14 @@ public class DefaultRepositoryProxyConnectors
 
         return file.lastModified();
     }
-    
+
     private boolean hasBeenUpdated( File file, long originalLastModified )
     {
         if ( !file.exists() || !file.isFile() )
         {
             return false;
         }
-        
+
         long currentLastModified = getLastModified( file );
         return ( currentLastModified > originalLastModified );
     }
@@ -297,7 +309,6 @@ public class DefaultRepositoryProxyConnectors
      * @throws ProxyException if there was a problem fetching the metadata file.
      */
     public File fetchFromProxies( ManagedRepositoryContent repository, ProjectReference metadata )
-        throws NotFoundException, NotModifiedException, ProxyException
     {
         File localFile = toLocalFile( repository, metadata );
 
@@ -317,7 +328,7 @@ public class DefaultRepositoryProxyConnectors
             try
             {
                 transferFile( connector, targetRepository, targetPath, localRepoFile, requestProperties );
-    
+
                 if ( hasBeenUpdated( localRepoFile, originalMetadataTimestamp ) )
                 {
                     metadataNeedsUpdating = true;
@@ -334,7 +345,14 @@ public class DefaultRepositoryProxyConnectors
                                        + " not updated on remote repository \""
                                        + targetRepository.getRepository().getId() + "\"." );
             }
-            
+            catch ( ProxyException e )
+            {
+                getLogger().warn( "Transfer error from repository \"" + targetRepository.getRepository().getId() +
+                    "\" for project metadata " + Keys.toKey( metadata ) +
+                    ", continuing to next repository. Error message: " + e.getMessage() );
+                getLogger().debug( "Full stack trace", e );
+            }
+
         }
 
         if ( hasBeenUpdated( localFile, originalTimestamp ) )
@@ -400,20 +418,17 @@ public class DefaultRepositoryProxyConnectors
     }
 
     private File toLocalFile( ManagedRepositoryContent repository, ArtifactReference artifact )
-        throws ProxyException
     {
         return repository.toFile( artifact );
     }
 
     private File toLocalFile( ManagedRepositoryContent repository, ProjectReference metadata )
-        throws ProxyException
     {
         String sourcePath = metadataTools.toPath( metadata );
         return new File( repository.getRepoRoot(), sourcePath );
     }
 
     private File toLocalFile( ManagedRepositoryContent repository, VersionedReference metadata )
-        throws ProxyException
     {
         String sourcePath = metadataTools.toPath( metadata );
         return new File( repository.getRepoRoot(), sourcePath );
@@ -461,7 +476,7 @@ public class DefaultRepositoryProxyConnectors
      */
     private File transferFile( ProxyConnector connector, RemoteRepositoryContent remoteRepository, String remotePath,
                                File localFile, Properties requestProperties )
-        throws NotFoundException, NotModifiedException, ProxyException
+        throws ProxyException, NotModifiedException
     {
         String url = remoteRepository.getURL().getUrl() + remotePath;
         requestProperties.setProperty( "url", url );
@@ -530,7 +545,7 @@ public class DefaultRepositoryProxyConnectors
         catch ( NotModifiedException e )
         {
             // Do not cache url here.
-            throw e; 
+            throw e;
         }
         catch ( ProxyException e )
         {
@@ -637,7 +652,7 @@ public class DefaultRepositoryProxyConnectors
      */
     private File transferSimpleFile( Wagon wagon, RemoteRepositoryContent remoteRepository, String remotePath,
                                      File localFile )
-        throws NotFoundException, NotModifiedException, ProxyException
+        throws ProxyException
     {
         assert ( remotePath != null );
 
@@ -674,7 +689,7 @@ public class DefaultRepositoryProxyConnectors
                     throw new NotModifiedException( "Not downloaded, as local file is newer than remote side: "
                                                     + localFile.getAbsolutePath() );
                 }
-                
+
                 if ( temp.exists() )
                 {
                     getLogger().debug( "Downloaded successfully." );
@@ -792,7 +807,7 @@ public class DefaultRepositoryProxyConnectors
             AuthenticationInfo authInfo = null;
             String username = remoteRepository.getRepository().getUsername();
             String password = remoteRepository.getRepository().getPassword();
-            
+
             if ( StringUtils.isNotBlank( username ) && StringUtils.isNotBlank( password ) )
             {
                 getLogger().debug( "Using username " + username + " to connect to remote repository "
diff --git a/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.java b/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.java
index bcb774640..8425bfb56 100644
--- a/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.java
+++ b/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.java
@@ -40,6 +40,9 @@ import java.io.File;
 public class CacheFailuresTransferTest
     extends AbstractProxyTestCase
 {
+    // TODO: test some hard failures (eg TransferFailedException)
+    // TODO: test the various combinations of fetchFrom* (note: need only test when caching is enabled)
+
     public void testGetWithCacheFailuresOn()
         throws Exception
     {
@@ -68,6 +71,8 @@ public class CacheFailuresTransferTest
 
         File downloadedFile = proxyHandler.fetchFromProxies( managedDefaultRepository, artifact );
 
+        // TODO: test failure is cached!
+
         wagonMockControl.verify();
         
         assertNotDownloaded( downloadedFile );
@@ -79,7 +84,6 @@ public class CacheFailuresTransferTest
     {
         String path = "org/apache/maven/test/get-in-second-proxy/1.0/get-in-second-proxy-1.0.jar";
         File expectedFile = new File( managedDefaultDir.getAbsoluteFile(), path );
-        
         setupTestableManagedRepository( path );
 
         assertNotExistsInManagedDefaultRepo( expectedFile );
@@ -103,9 +107,11 @@ public class CacheFailuresTransferTest
 
         File downloadedFile = proxyHandler.fetchFromProxies( managedDefaultRepository, artifact );
 
-        assertNotDownloaded( downloadedFile );
+        // TODO: test failure is not cached!
 
         wagonMockControl.verify();
+
+        assertNotDownloaded( downloadedFile );
         assertNoTempFiles( expectedFile );
     }
 
diff --git a/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/MetadataTransferTest.java b/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/MetadataTransferTest.java
index a4459d6c1..d46d4e9fd 100644
--- a/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/MetadataTransferTest.java
+++ b/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/MetadataTransferTest.java
@@ -33,6 +33,7 @@ import org.apache.maven.archiva.repository.metadata.MetadataTools;
 import org.apache.maven.archiva.repository.metadata.RepositoryMetadataException;
 import org.apache.maven.archiva.repository.metadata.RepositoryMetadataReader;
 import org.apache.maven.archiva.repository.metadata.RepositoryMetadataWriter;
+import org.apache.maven.wagon.TransferFailedException;
 import org.custommonkey.xmlunit.DetailedDiff;
 import org.custommonkey.xmlunit.Diff;
 
@@ -45,14 +46,14 @@ import java.util.Arrays;
  * present in the repository.
  *
  * Test Case Naming is as follows.
- * 
+ *
  * <code>
  * public void testGet[Release|Snapshot|Project]Metadata[Not]Proxied[Not|On]Local[Not|On|Multiple]Remote
  * </code>
- * 
+ *
  * <pre>
  * Which should leave the following matrix of test cases.
- *  
+ *
  *   Metadata  | Proxied  | Local | Remote
  *   ----------+----------+-------+---------
  *   Release   | Not      | Not   | n/a (1)
@@ -79,7 +80,7 @@ import java.util.Arrays;
  *   Project   |          | On    | Not
  *   Project   |          | On    | On
  *   Project   |          | On    | Multiple
- *   
+ *
  * (1) If it isn't proxied, no point in having a remote.
  * </pre>
  *
@@ -94,9 +95,46 @@ public class MetadataTransferTest
      */
     private MetadataTools metadataTools;
 
+    // TODO: same test for other fetch* methods
+    public void testFetchFromTwoProxiesWhenFirstConnectionFails()
+        throws Exception
+    {
+        // Project metadata that does not exist locally, but has multiple versions in remote repos
+        String requestedResource = "org/apache/maven/test/get-default-layout/maven-metadata.xml";
+        setupTestableManagedRepository( requestedResource );
+
+        saveRemoteRepositoryConfig( "badproxied1", "Bad Proxied 1", "test://bad.machine.com/repo/", "default" );
+
+        // Configure Connector (usually done within archiva.xml configuration)
+        saveConnector( ID_DEFAULT_MANAGED, "badproxied1", ChecksumPolicy.FIX, ReleasesPolicy.ALWAYS,
+                       SnapshotsPolicy.ALWAYS, CachedFailuresPolicy.NO );
+        saveConnector( ID_DEFAULT_MANAGED, ID_PROXIED2, ChecksumPolicy.FIX, ReleasesPolicy.ALWAYS,
+                       SnapshotsPolicy.ALWAYS, CachedFailuresPolicy.NO );
+
+        assertResourceNotFound( requestedResource );
+        assertNoRepoMetadata( "badproxied1", requestedResource );
+        assertNoRepoMetadata( ID_PROXIED2, requestedResource );
+
+        // ensure that a hard failure in the first proxy connector is skipped and the second repository checked
+        File expectedFile = new File( managedDefaultDir.getAbsoluteFile(),
+                                      metadataTools.getRepositorySpecificName( "badproxied1", requestedResource ) );
+        wagonMock.get( requestedResource, new File( expectedFile.getParentFile(), expectedFile.getName() + ".tmp" ) );
+        wagonMockControl.setThrowable( new TransferFailedException( "can't connect" ) );
+
+        wagonMockControl.replay();
+
+        assertFetchProject( requestedResource );
+
+        wagonMockControl.verify();
+
+        assertProjectMetadataContents( requestedResource, new String[] { "1.0.1" }, "1.0.1", "1.0.1" );
+        assertNoRepoMetadata( "badproxied1", requestedResource );
+        assertRepoProjectMetadata( ID_PROXIED2, requestedResource, new String[] { "1.0.1" } );
+    }
+
     /**
      * Attempt to get the project metadata for non-existant artifact.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is not created on the managed repository, nor returned
      *                  to the requesting client.
      */
@@ -287,7 +325,7 @@ public class MetadataTransferTest
     /**
      * A request for a release maven-metadata.xml file that does not exist locally, and the managed
      * repository has no proxied repositories set up.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is not created on the managed repository, nor returned
      *                  to the requesting client.
      */
@@ -310,7 +348,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that does exist locally, and the managed
      * repository has no proxied repositories set up.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is updated locally, based off of the managed repository
      *                  information, and then returned to the client.
      */
@@ -330,7 +368,7 @@ public class MetadataTransferTest
     /**
      * A request for a release maven-metadata.xml file that does not exist on the managed repository, but
      * exists on multiple remote repositories.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is downloaded from the remote into the repository specific
      *                  file location on the managed repository, a merge of the contents to the requested
      *                  maven-metadata.xml is performed, and then the merged maven-metadata.xml file is
@@ -362,7 +400,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that does not exist locally, nor does it exist in a remote
      * proxied repository.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is created locally, based off of managed repository
      *                  information, and then return to the client.
      */
@@ -387,7 +425,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that does not exist on the managed repository, but
      * exists on 1 remote repository.
-     * 
+     *
      * Expected result: the maven-metadata.xml file is downloaded from the remote into the repository specific
      *                  file location on the managed repository, a merge of the contents to the requested
      *                  maven-metadata.xml is performed, and then the merged maven-metadata.xml file is
@@ -414,7 +452,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that exists in the managed repository, but
      * not on any remote repository.
-     * 
+     *
      * Expected result: the maven-metadata.xml file does not exist on the remote proxied repository and
      *                  is not downloaded.  There is no repository specific metadata file on the managed
      *                  repository.  The managed repository maven-metadata.xml is returned to the
@@ -441,7 +479,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that exists in the managed repository, and on multiple
      * remote repositories.
-     * 
+     *
      * Expected result: the maven-metadata.xml file on the remote proxied repository is downloaded
      *                  and merged into the contents of the existing managed repository copy of
      *                  the maven-metadata.xml file.
@@ -472,7 +510,7 @@ public class MetadataTransferTest
     /**
      * A request for a maven-metadata.xml file that exists in the managed repository, and on one
      * remote repository.
-     * 
+     *
      * Expected result: the maven-metadata.xml file on the remote proxied repository is downloaded
      *                  and merged into the contents of the existing managed repository copy of
      *                  the maven-metadata.xml file.
@@ -668,9 +706,9 @@ public class MetadataTransferTest
 
     /**
      * Transfer the metadata file.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertFetchProject( String requestedResource )
         throws Exception
@@ -693,9 +731,9 @@ public class MetadataTransferTest
 
     /**
      * Transfer the metadata file, not expected to succeed.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertFetchProjectFailed( String requestedResource )
         throws Exception
@@ -711,9 +749,9 @@ public class MetadataTransferTest
 
     /**
      * Transfer the metadata file.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertFetchVersioned( String requestedResource )
         throws Exception
@@ -736,9 +774,9 @@ public class MetadataTransferTest
 
     /**
      * Transfer the metadata file, not expected to succeed.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertFetchVersionedFailed( String requestedResource )
         throws Exception
@@ -754,9 +792,9 @@ public class MetadataTransferTest
 
     /**
      * Test for the existance of the requestedResource in the default managed repository.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertResourceExists( String requestedResource )
         throws Exception
@@ -787,9 +825,9 @@ public class MetadataTransferTest
 
     /**
      * Ensures that the requested resource is not present in the managed repository.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertNoMetadata( String requestedResource )
         throws Exception
@@ -799,7 +837,7 @@ public class MetadataTransferTest
     }
 
     /**
-     * Ensures that the proxied repository specific maven metadata file does NOT exist in the 
+     * Ensures that the proxied repository specific maven metadata file does NOT exist in the
      * managed repository.
      * @param proxiedRepoId the proxied repository id to validate with.
      * @param requestedResource the resource requested.
@@ -815,9 +853,9 @@ public class MetadataTransferTest
     /**
      * Test for the existance of the requestedResource in the default managed repository, and if it exists,
      * does it contain the specified list of expected versions?
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertProjectMetadataContents( String requestedResource, String expectedVersions[],
                                                 String latestVersion, String releaseVersion )
@@ -851,9 +889,9 @@ public class MetadataTransferTest
     /**
      * Test for the existance of the requestedResource in the default managed repository, and if it exists,
      * does it contain the expected release maven-metadata.xml contents?
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertReleaseMetadataContents( String requestedResource )
         throws Exception
@@ -879,13 +917,13 @@ public class MetadataTransferTest
     /**
      * Test for the existance of the snapshot metadata in the default managed repository, and if it exists,
      * does it contain the expected release maven-metadata.xml contents?
-     * 
+     *
      * @param requestedResource the requested resource
      * @param expectedDate the date in "yyyyMMdd" format
      * @param expectedTime the time in "hhmmss" format
      * @param expectedBuildnumber the build number
-     * 
-     * @throws Exception 
+     *
+     * @throws Exception
      */
     private void assertSnapshotMetadataContents( String requestedResource, String expectedDate, String expectedTime,
                                                  int expectedBuildnumber )
@@ -900,16 +938,16 @@ public class MetadataTransferTest
     }
 
     /**
-     * Test for the existance of the proxied repository specific snapshot metadata in the default managed 
+     * Test for the existance of the proxied repository specific snapshot metadata in the default managed
      * repository, and if it exists, does it contain the expected release maven-metadata.xml contents?
-     * 
+     *
      * @param proxiedRepoId the repository id of the proxied repository.
      * @param requestedResource the requested resource
      * @param expectedDate the date in "yyyyMMdd" format
      * @param expectedTime the time in "hhmmss" format
      * @param expectedBuildnumber the build number
-     * 
-     * @throws Exception 
+     *
+     * @throws Exception
      */
     private void assertRepoSnapshotMetadataContents( String proxiedRepoId, String requestedResource,
                                                      String expectedDate, String expectedTime, int expectedBuildnumber )
@@ -993,7 +1031,7 @@ public class MetadataTransferTest
     /**
      * Ensures that the repository specific maven metadata file exists, and contains the appropriate
      * list of expected versions within.
-     * 
+     *
      * @param proxiedRepoId
      * @param requestedResource
      */
@@ -1022,9 +1060,9 @@ public class MetadataTransferTest
 
     /**
      * Test for the non-existance of the requestedResource in the default managed repository.
-     * 
+     *
      * @param requestedResource the requested resource
-     * @throws Exception 
+     * @throws Exception
      */
     private void assertResourceNotFound( String requestedResource )
         throws Exception