You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by br...@apache.org on 2008/07/07 05:32:09 UTC

svn commit: r674388 - in /maven/artifact/trunk/src: main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java test/java/org/apache/maven/artifact/manager/WagonNoOp.java

Author: brett
Date: Sun Jul  6 20:32:08 2008
New Revision: 674388

URL: http://svn.apache.org/viewvc?rev=674388&view=rev
Log:
[MARTIFACT-28] if an artifact has an md5 but no sha1, a null pointer exception results

Modified:
    maven/artifact/trunk/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java
    maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
    maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java

Modified: maven/artifact/trunk/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java
URL: http://svn.apache.org/viewvc/maven/artifact/trunk/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java?rev=674388&r1=674387&r2=674388&view=diff
==============================================================================
--- maven/artifact/trunk/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java (original)
+++ maven/artifact/trunk/src/main/java/org/apache/maven/artifact/manager/DefaultWagonManager.java Sun Jul  6 20:32:08 2008
@@ -575,29 +575,58 @@
                         wagon.get( remotePath, temp );
                         downloaded = true;
                     }
+                }
+                finally
+                {
+                    wagon.removeTransferListener( md5ChecksumObserver );
+                    wagon.removeTransferListener( sha1ChecksumObserver );
+                }
+
+                if ( downloaded )
+                {
+                    // keep the checksum files from showing up on the download monitor...
+                    if ( downloadMonitor != null )
+                    {
+                        wagon.removeTransferListener( downloadMonitor );
+                    }
 
-                    if ( downloaded )
+                    // try to verify the SHA-1 checksum for this file.
+                    try
                     {
-                        // keep the checksum files from showing up on the download monitor...
-                        if ( downloadMonitor != null )
+                        verifyChecksum( sha1ChecksumObserver, destination, temp, remotePath, ".sha1", wagon );
+                    }
+                    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 )
                         {
-                            wagon.removeTransferListener( downloadMonitor );
+                            getLogger().warn( "*** CHECKSUM FAILED - " + e.getMessage() + " - RETRYING" );
+                            retry = true;
                         }
+                        else
+                        {
+                            handleChecksumFailure( checksumPolicy, e.getMessage(), e.getCause() );
+                        }
+                    }
+                    catch ( ResourceDoesNotExistException sha1TryException )
+                    {
+                        getLogger().debug( "SHA1 not found, trying MD5", sha1TryException );
 
-                        // try to verify the SHA-1 checksum for this file.
+                        // if this IS NOT a ChecksumFailedException, it was a problem with transfer/read of the checksum
+                        // file...we'll try again with the MD5 checksum.
                         try
                         {
-                            verifyChecksum( sha1ChecksumObserver, destination, temp, remotePath, ".sha1", wagon );
+                            verifyChecksum( md5ChecksumObserver, destination, temp, remotePath, ".md5", wagon );
                         }
                         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 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 )
                             {
-                                getLogger().warn( "*** CHECKSUM FAILED - " + e.getMessage() + " - RETRYING" );
                                 retry = true;
                             }
                             else
@@ -605,52 +634,23 @@
                                 handleChecksumFailure( checksumPolicy, e.getMessage(), e.getCause() );
                             }
                         }
-                        catch ( ResourceDoesNotExistException sha1TryException )
+                        catch ( ResourceDoesNotExistException md5TryException )
                         {
-                            getLogger().debug( "SHA1 not found, trying MD5", sha1TryException );
-
-                            // if this IS NOT a ChecksumFailedException, it was a problem with transfer/read of the checksum
-                            // file...we'll try again with the MD5 checksum.
-                            try
-                            {
-                                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 );
+                            // this was a failed transfer, and we don't want to retry.
+                            handleChecksumFailure( checksumPolicy, "Error retrieving checksum file for " + remotePath,
+                                md5TryException );
                         }
                     }
 
-                    // unset the firstRun flag, so we don't get caught in an infinite loop...
-                    firstRun = false;
-                }
-                finally
-                {
-                    wagon.removeTransferListener( md5ChecksumObserver );
-                    wagon.removeTransferListener( sha1ChecksumObserver );
+                    // reinstate the download monitor...
+                    if ( downloadMonitor != null )
+                    {
+                        wagon.addTransferListener( downloadMonitor );
+                    }
                 }
+
+                // unset the firstRun flag, so we don't get caught in an infinite loop...
+                firstRun = false;
             }
         }
         catch ( ConnectionException e )

Modified: maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java
URL: http://svn.apache.org/viewvc/maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java?rev=674388&r1=674387&r2=674388&view=diff
==============================================================================
--- maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java (original)
+++ maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/DefaultWagonManagerTest.java Sun Jul  6 20:32:08 2008
@@ -180,7 +180,7 @@
         ArtifactRepository repo = createNoOpRepo();
         
         WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" );
-        wagon.setExpectedContent( "expected" );
+        wagon.addExpectedContent( repo.getLayout().pathOf( artifact ), "expected" );
         
         MockControl control = MockControl.createControl( UpdateCheckManager.class );
         UpdateCheckManager updateCheckManager = (UpdateCheckManager) control.getMock();
@@ -206,7 +206,7 @@
         ArtifactRepository repo = createNoOpRepo();
 
         WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" );
-        wagon.setExpectedContent( "expected" );
+        wagon.addExpectedContent( repo.getLayout().pathOf( artifact ), "expected" );
         
         MockControl control = MockControl.createControl( UpdateCheckManager.class );
         UpdateCheckManager updateCheckManager = (UpdateCheckManager) control.getMock();
@@ -230,7 +230,7 @@
         ArtifactRepository repo = createNoOpRepo();
 
         WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" );
-        wagon.setExpectedContent( "expected" );
+        wagon.addExpectedContent( repo.getLayout().pathOf( artifact ), "expected" );
         
         MockControl control = MockControl.createControl( UpdateCheckManager.class );
         UpdateCheckManager updateCheckManager = (UpdateCheckManager) control.getMock();
@@ -244,6 +244,22 @@
 
         control.verify();
     }
+    
+    public void testGetArtifactSha1MissingMd5Present()
+        throws IOException, UnsupportedProtocolException, TransferFailedException, ResourceDoesNotExistException
+    {
+        Artifact artifact = createTestPomArtifact( "target/test-data/get-remote-artifact" );
+
+        ArtifactRepository repo = createNoOpRepo();
+
+        WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" );
+        wagon.addExpectedContent( repo.getLayout().pathOf( artifact ), "expected" );
+        wagon.addExpectedContent( repo.getLayout().pathOf( artifact ) + ".md5", "bad_checksum" );
+        
+        wagonManager.getArtifact( artifact, repo, true );
+
+        assertTrue( artifact.getFile().exists() );
+    }
 
     private ArtifactRepository createNoOpRepo()
     {
@@ -506,7 +522,7 @@
             artifact.setFile( tmpFile );
             ArtifactRepository repo = createNoOpRepo();
             WagonNoOp wagon = (WagonNoOp) wagonManager.getWagon( "noop" );
-            wagon.setExpectedContent( "" );
+            wagon.addExpectedContent( repo.getLayout().pathOf( artifact ), "" );
 
             /* getArtifact */
             assertFalse( "Transfer listener is registered before test",
@@ -517,10 +533,13 @@
                          wagon.getTransferEventSupport().hasTransferListener( transferListener ) );
 
             /* putArtifact */
+            File sampleFile = getTestFile( "target/test-file" );
+            FileUtils.fileWrite( sampleFile.getAbsolutePath(), "sample file" );
+            
             assertFalse( "Transfer listener is registered before test",
                          wagon.getTransferEventSupport().hasTransferListener( transferListener ) );
             wagonManager.setDownloadMonitor( transferListener );
-            wagonManager.putArtifact( new File( "sample file" ), artifact, repo );
+            wagonManager.putArtifact( sampleFile, artifact, repo );
             assertFalse( "Transfer listener still registered after putArtifact",
                          wagon.getTransferEventSupport().hasTransferListener( transferListener ) );
         }

Modified: maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java
URL: http://svn.apache.org/viewvc/maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java?rev=674388&r1=674387&r2=674388&view=diff
==============================================================================
--- maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java (original)
+++ maven/artifact/trunk/src/test/java/org/apache/maven/artifact/manager/WagonNoOp.java Sun Jul  6 20:32:08 2008
@@ -21,76 +21,74 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.apache.maven.wagon.AbstractWagon;
+import org.apache.maven.wagon.ConnectionException;
+import org.apache.maven.wagon.InputData;
+import org.apache.maven.wagon.OutputData;
 import org.apache.maven.wagon.ResourceDoesNotExistException;
+import org.apache.maven.wagon.StreamWagon;
 import org.apache.maven.wagon.TransferFailedException;
+import org.apache.maven.wagon.authentication.AuthenticationException;
 import org.apache.maven.wagon.authorization.AuthorizationException;
 import org.apache.maven.wagon.resource.Resource;
 import org.codehaus.plexus.util.FileUtils;
+import org.codehaus.plexus.util.StringInputStream;
+import org.codehaus.plexus.util.StringOutputStream;
 
 public class WagonNoOp
-    extends AbstractWagon
+    extends StreamWagon
 {
-    private String expectedContent;
+    private Map<String, String> expectedContent = new HashMap<String, String>();
     
-    public void setExpectedContent( String expectedContent )
+    public void addExpectedContent( String resourceName, String expectedContent )
     {
-        this.expectedContent = expectedContent ;
+        this.expectedContent.put( resourceName, expectedContent );
     }
 
-    public void closeConnection()
+    public String[] getSupportedProtocols()
     {
-        // NO-OP
+        return new String[] { "noop" };
     }
 
-    public void get( String resourceName, File destination )
-        throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException
+    @Override
+    public void closeConnection()
+        throws ConnectionException
     {
-        getIfNewer( resourceName, destination, 0 );
     }
 
-    public boolean getIfNewer( String resourceName, File destination, long timestamp )
+    @Override
+    public void fillInputData( InputData inputData )
         throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException
     {
-        Resource resource = new Resource( resourceName );
-        fireGetInitiated( resource, destination );
-        fireGetStarted( resource, destination );
-        try
+        Resource resource = inputData.getResource();
+
+        String content = expectedContent.get( resource.getName() );
+        
+        if ( content != null )
         {
-            if ( expectedContent != null )
-            {
-                FileUtils.fileWrite( destination.getAbsolutePath(), expectedContent );
-            }
-            else
-            {
-                throw new ResourceDoesNotExistException( "Mock wagon had no content set" );
-            }
+            resource.setContentLength( content.length() );
+
+            inputData.setInputStream( new StringInputStream( content ) );
         }
-        catch ( IOException e )
+        else
         {
-            return false;
+            throw new ResourceDoesNotExistException( "No content provided for " + resource.getName() );
         }
-        fireGetCompleted( resource, destination );
-        return true;
     }
 
-    public void openConnectionInternal()
+    @Override
+    public void fillOutputData( OutputData outputData )
+        throws TransferFailedException
     {
-        // NO-OP
+        outputData.setOutputStream( new StringOutputStream() );
     }
 
-    public void put( File source, String destination )
-        throws TransferFailedException, ResourceDoesNotExistException, AuthorizationException
-    {
-        Resource resource = new Resource( destination );
-        firePutInitiated( resource, source );
-        firePutStarted( resource, source );
-        firePutCompleted( resource, source );
-    }
-
-    public String[] getSupportedProtocols()
+    @Override
+    protected void openConnectionInternal()
+        throws ConnectionException, AuthenticationException
     {
-        return new String[] { "noop" };
     }
 }