[MRM-631] ensure wagon lookups get a new instance on each request, reset HTTP properties after request

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

15 files changed, 192 insertions, 39 deletions

diff --git a/archiva-modules/archiva-base/archiva-policies/src/main/resources/META-INF/spring-context.xml b/archiva-modules/archiva-base/archiva-policies/src/main/resources/META-INF/spring-context.xml
index 840223b1a..75a5d41c0 100644
--- a/archiva-modules/archiva-base/archiva-policies/src/main/resources/META-INF/spring-context.xml
+++ b/archiva-modules/archiva-base/archiva-policies/src/main/resources/META-INF/spring-context.xml
@@ -1,12 +1,32 @@
 <?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~   http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+-->
 <beans xmlns="http://www.springframework.org/schema/beans"
-       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-       xsi:schemaLocation="http://www.springframework.org/schema/beans
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://www.springframework.org/schema/beans
            http://www.springframework.org/schema/beans/spring-beans-2.5.xsd">
 
-  <bean id="urlFailureCache" class="org.apache.maven.archiva.policies.urlcache.DefaultUrlFailureCache">
+  <bean id="urlFailureCache"
+    class="org.apache.maven.archiva.policies.urlcache.DefaultUrlFailureCache">
     <!-- collaborators and configuration for this bean go here -->
-    <constructor-arg ref="cache#url-failures-cache" type="org.codehaus.plexus.cache.Cache"/>
+    <constructor-arg ref="cache#url-failures-cache"
+      type="org.codehaus.plexus.cache.Cache" />
   </bean>
 
 </beans>
\ No newline at end of file
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
index f8cfca84f..f27b94aa7 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
+++ b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/DefaultRepositoryProxyConnectors.java
@@ -95,11 +95,6 @@ public class DefaultRepositoryProxyConnectors
     private ArchivaConfiguration archivaConfiguration;
 
     /**
-     * @plexus.requirement role="org.apache.maven.wagon.Wagon"
-     */
-    private Map<String, Wagon> wagons;
-
-    /**
      * @plexus.requirement
      */
     private RepositoryContentFactory repositoryFactory;
@@ -138,6 +133,11 @@ public class DefaultRepositoryProxyConnectors
      */
     private RepositoryContentConsumers consumers;
 
+    /**
+     * @plexus.requirement
+     */
+    private WagonFactory wagonFactory;
+
     public File fetchFromProxies( ManagedRepositoryContent repository, ArtifactReference artifact )
         throws ProxyDownloadException
     {
@@ -589,12 +589,17 @@ public class DefaultRepositoryProxyConnectors
             return null;
         }
 
+        // MRM-631 - the lightweight wagon does not reset these - remove if we switch to httpclient based wagon
+        String previousHttpProxyHost = System.getProperty( "http.proxyHost" );
+        String previousHttpProxyPort = System.getProperty( "http.proxyPort" );
+        String previousProxyExclusions = System.getProperty( "http.nonProxyHosts" );
+
         Wagon wagon = null;
         try
         {
             RepositoryURL repoUrl = remoteRepository.getURL();
             String protocol = repoUrl.getProtocol();
-            wagon = (Wagon) wagons.get( protocol );
+            wagon = (Wagon) wagonFactory.getWagon( "wagon#" + protocol );
             if ( wagon == null )
             {
                 throw new ProxyException( "Unsupported target repository protocol: " + protocol );
@@ -633,6 +638,32 @@ public class DefaultRepositoryProxyConnectors
                 try
                 {
                     wagon.disconnect();
+
+                    // MRM-631 - the lightweight wagon does not reset these - remove if we switch to httpclient based wagon
+                    if ( previousHttpProxyHost != null )
+                    {
+                        System.setProperty( "http.proxyHost", previousHttpProxyHost );
+                    }
+                    else
+                    {
+                        System.getProperties().remove( "http.proxyHost" );
+                    }
+                    if ( previousHttpProxyPort != null )
+                    {
+                        System.setProperty( "http.proxyPort", previousHttpProxyPort );
+                    }
+                    else
+                    {
+                        System.getProperties().remove( "http.proxyPort" );
+                    }
+                    if ( previousProxyExclusions != null )
+                    {
+                        System.setProperty( "http.nonProxyHosts", previousProxyExclusions );
+                    }
+                    else
+                    {
+                        System.getProperties().remove( "http.nonProxyHosts" );
+                    }
                 }
                 catch ( ConnectionException e )
                 {
@@ -970,14 +1001,7 @@ public class DefaultRepositoryProxyConnectors
             wagon.setTimeout(timeoutInMilliseconds);
 
             Repository wagonRepository = new Repository( remoteRepository.getId(), remoteRepository.getURL().toString() );
-            if ( networkProxy != null )
-            {
-                wagon.connect( wagonRepository, authInfo, networkProxy );
-            }
-            else
-            {
-                wagon.connect( wagonRepository, authInfo );
-            }
+            wagon.connect( wagonRepository, authInfo, networkProxy );
             connected = true;
         }
         catch ( ConnectionException e )
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/WagonFactory.java b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/WagonFactory.java
new file mode 100644
index 000000000..a32d1550d
--- /dev/null
+++ b/archiva-modules/archiva-base/archiva-proxy/src/main/java/org/apache/maven/archiva/proxy/WagonFactory.java
@@ -0,0 +1,39 @@
+package org.apache.maven.archiva.proxy;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.wagon.Wagon;
+
+/**
+ * Create a Wagon instance for the given protocol. Implementation will be provided by a Spring service locator.
+ * 
+ * @author Brett Porter
+ */
+public interface WagonFactory
+{
+    /**
+     * Create a new Wagon instance for the given protocol.
+     * 
+     * @param   protocol the protocol to find the Wagon for, which must be prefixed with <code>wagon#</code>, for example 
+     *          <code>wagon#http</code>.
+     * @return  the Wagon instance
+     */
+    Wagon getWagon( String protocol );
+}
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/main/resources/META-INF/spring-context.xml b/archiva-modules/archiva-base/archiva-proxy/src/main/resources/META-INF/spring-context.xml
new file mode 100644
index 000000000..baae58725
--- /dev/null
+++ b/archiva-modules/archiva-base/archiva-proxy/src/main/resources/META-INF/spring-context.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~   http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+-->
+<beans xmlns="http://www.springframework.org/schema/beans"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://www.springframework.org/schema/beans
+           http://www.springframework.org/schema/beans/spring-beans-2.5.xsd">
+
+  <bean id="wagonFactory"
+    class="org.springframework.beans.factory.config.ServiceLocatorFactoryBean">
+    <property name="serviceLocatorInterface"
+      value="org.apache.maven.archiva.proxy.WagonFactory" />
+  </bean>
+</beans>
\ No newline at end of file
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/HttpProxyTransferTest.java b/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/HttpProxyTransferTest.java
index 63285c658..1c2aa9a20 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/HttpProxyTransferTest.java
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/HttpProxyTransferTest.java
@@ -21,7 +21,6 @@ package org.apache.maven.archiva.proxy;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Enumeration;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -160,6 +159,9 @@ public class HttpProxyTransferTest
     public void testGetOverHttpProxy()
         throws Exception
     {
+        assertNull( System.getProperty( "http.proxyHost" ) );
+        assertNull( System.getProperty( "http.proxyPort" ) );
+        
         String path = "org/apache/maven/test/get-default-layout/1.0/get-default-layout-1.0.jar";
 
         // Configure Connector (usually done within archiva.xml configuration)
@@ -183,6 +185,9 @@ public class HttpProxyTransferTest
         String expectedContents = FileUtils.readFileToString( sourceFile, null );
         String actualContents = FileUtils.readFileToString( downloadedFile, null );
         assertEquals( "Check file contents.", expectedContents, actualContents );
+        
+        assertNull( System.getProperty( "http.proxyHost" ) );
+        assertNull( System.getProperty( "http.proxyPort" ) );
     }
 
     private void addConnector()
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/WagonFactoryTest.java b/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/WagonFactoryTest.java
new file mode 100644
index 000000000..3e31f9ae7
--- /dev/null
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/java/org/apache/maven/archiva/proxy/WagonFactoryTest.java
@@ -0,0 +1,43 @@
+package org.apache.maven.archiva.proxy;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.wagon.Wagon;
+import org.codehaus.plexus.spring.PlexusInSpringTestCase;
+
+/**
+ * Test the WagonFactory works through Spring to be bound into the RepositoryProxyConnectors implementation.
+ * 
+ * @author Brett Porter
+ */
+public class WagonFactoryTest
+    extends PlexusInSpringTestCase
+{
+    public void testLookupSuccessiveWagons()
+    {
+        WagonFactory factory = (WagonFactory) lookup( WagonFactory.class );
+        
+        Wagon first = factory.getWagon( "wagon#file" );
+        
+        Wagon second = factory.getWagon( "wagon#file" );
+        
+        assertNotSame( first, second );
+    }
+}
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.xml
index 70fec59ff..7ad475f57 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/CacheFailuresTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ChecksumTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ChecksumTransferTest.xml
index 5e48717e4..e9dfa4cc6 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ChecksumTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ChecksumTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ErrorHandlingTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ErrorHandlingTest.xml
index b1ada8f02..fad392c50 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ErrorHandlingTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ErrorHandlingTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/HttpProxyTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/HttpProxyTransferTest.xml
index 9c3909d62..bc10178b5 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/HttpProxyTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/HttpProxyTransferTest.xml
@@ -49,10 +49,6 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
-        </requirement>
-        <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
           <role-hint>mocked</role-hint>
         </requirement>
@@ -75,6 +71,10 @@
           <role>org.apache.maven.archiva.policies.urlcache.UrlFailureCache</role>
           <field-name>urlFailureCache</field-name>
         </requirement>
+        <requirement>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
+          <role-hint>default</role-hint>
+        </requirement>
       </requirements>
     </component>
 
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedDefaultTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedDefaultTransferTest.xml
index 5e48717e4..e9dfa4cc6 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedDefaultTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedDefaultTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedLegacyTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedLegacyTransferTest.xml
index 5e48717e4..e9dfa4cc6 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedLegacyTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/ManagedLegacyTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/MetadataTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/MetadataTransferTest.xml
index 1ca34af8a..fe348d3f7 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/MetadataTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/MetadataTransferTest.xml
@@ -73,8 +73,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/RelocateTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/RelocateTransferTest.xml
index 5e48717e4..e9dfa4cc6 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/RelocateTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/RelocateTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>
diff --git a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/SnapshotTransferTest.xml b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/SnapshotTransferTest.xml
index 5e48717e4..e9dfa4cc6 100644
--- a/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/SnapshotTransferTest.xml
+++ b/archiva-modules/archiva-base/archiva-proxy/src/test/resources/org/apache/maven/archiva/proxy/SnapshotTransferTest.xml
@@ -54,8 +54,7 @@
           <field-name>archivaConfiguration</field-name>
         </requirement>
         <requirement>
-          <role>org.apache.maven.wagon.Wagon</role>
-          <field-name>wagons</field-name>
+          <role>org.apache.maven.archiva.proxy.WagonFactory</role>
         </requirement>
         <requirement>
           <role>org.apache.maven.archiva.repository.RepositoryContentFactory</role>