[MRM-482] Saving a proxy connector after adding a new property results to HTTP 500 error

[MRM-477] Missing ability to manage proxy order.
[MRM-437] admin editing of proxy connectors fails in multiple instances
* Split giant ball of mud ConfigureProxyConnectorAction into seperate Add/Edit/Delete/Sort/List actions
* Added ability to maintain sort order.
* Added unit testing.
* Cleaned up Proxy Connector admin UI to allow for extra long Location / URL paths.
* Slideout Hide of the Advanced configuration options on the Proxy Connector list screen.



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

4 files changed, 295 insertions, 24 deletions

diff --git a/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java b/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
index f4c99d5d1..72d5b6734 100644
--- a/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
+++ b/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
@@ -20,6 +20,7 @@ package org.apache.maven.archiva.configuration;
  */
 
 import org.apache.commons.io.FileUtils;
+import org.apache.maven.archiva.configuration.functors.ProxyConnectorConfigurationOrderComparator;
 import org.apache.maven.archiva.configuration.io.registry.ConfigurationRegistryReader;
 import org.apache.maven.archiva.configuration.io.registry.ConfigurationRegistryWriter;
 import org.codehaus.plexus.logging.AbstractLogEnabled;
@@ -33,9 +34,11 @@ import org.codehaus.plexus.util.StringUtils;
 import java.io.File;
 import java.io.IOException;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
 
 /**
  * Implementation of configuration holder that retrieves it from the registry.
@@ -145,7 +148,28 @@ public class DefaultArchivaConfiguration
                 }
             }
         }
-        
+
+        // Normalize the order fields in the proxy connectors.
+        if ( !config.getProxyConnectors().isEmpty() )
+        {
+            Map<String, java.util.List<ProxyConnectorConfiguration>> proxyConnectorMap = config
+                .getProxyConnectorAsMap();
+
+            for ( String key : proxyConnectorMap.keySet() )
+            {
+                List<ProxyConnectorConfiguration> connectors = proxyConnectorMap.get( key );
+                // Sort connectors by order field.
+                Collections.sort( connectors, ProxyConnectorConfigurationOrderComparator.getInstance() );
+
+                // Normalize the order field values.
+                int order = 1;
+                for ( ProxyConnectorConfiguration connector : connectors )
+                {
+                    connector.setOrder( order++ );
+                }
+            }
+        }
+
         return config;
     }
 
@@ -161,7 +185,8 @@ public class DefaultArchivaConfiguration
         catch ( RegistryException e )
         {
             throw new ConfigurationRuntimeException(
-                "Fatal error: Unable to find the built-in default configuration and load it into the registry", e );
+                                                     "Fatal error: Unable to find the built-in default configuration and load it into the registry",
+                                                     e );
         }
         return registry.getSubset( KEY );
     }
@@ -189,10 +214,10 @@ public class DefaultArchivaConfiguration
 
                 // a little aggressive with the repositoryScanning and databaseScanning - should be no need to split
                 // that configuration
-                if ( key.startsWith( "repositories" ) || key.startsWith( "proxyConnectors" ) ||
-                    key.startsWith( "networkProxies" ) || key.startsWith( "repositoryScanning" ) ||
-                    key.startsWith( "databaseScanning" ) || key.startsWith( "remoteRepositories" ) ||
-                    key.startsWith( "managedRepositories" ) )
+                if ( key.startsWith( "repositories" ) || key.startsWith( "proxyConnectors" )
+                    || key.startsWith( "networkProxies" ) || key.startsWith( "repositoryScanning" )
+                    || key.startsWith( "databaseScanning" ) || key.startsWith( "remoteRepositories" )
+                    || key.startsWith( "managedRepositories" ) )
                 {
                     foundList = true;
                 }
@@ -203,12 +228,13 @@ public class DefaultArchivaConfiguration
                 this.configuration = null;
 
                 throw new IndeterminateConfigurationException(
-                    "Configuration can not be saved when it is loaded from two sources" );
+                                                               "Configuration can not be saved when it is loaded from two sources" );
             }
         }
 
         // escape all cron expressions to handle ','
-        for ( Iterator<ManagedRepositoryConfiguration> i = configuration.getManagedRepositories().iterator(); i.hasNext(); )
+        for ( Iterator<ManagedRepositoryConfiguration> i = configuration.getManagedRepositories().iterator(); i
+            .hasNext(); )
         {
             ManagedRepositoryConfiguration c = i.next();
             c.setRefreshCronExpression( escapeCronExpression( c.getRefreshCronExpression() ) );
@@ -217,7 +243,8 @@ public class DefaultArchivaConfiguration
         if ( configuration.getDatabaseScanning() != null )
         {
             configuration.getDatabaseScanning().setCronExpression(
-                escapeCronExpression( configuration.getDatabaseScanning().getCronExpression() ) );
+                                                                   escapeCronExpression( configuration
+                                                                       .getDatabaseScanning().getCronExpression() ) );
         }
 
         new ConfigurationRegistryWriter().write( configuration, section );
@@ -299,10 +326,10 @@ public class DefaultArchivaConfiguration
 
     private String removeExpressions( String directory )
     {
-        String value = StringUtils.replace( directory, "${appserver.base}",
-                                            registry.getString( "appserver.base", "${appserver.base}" ) );
-        value = StringUtils.replace( value, "${appserver.home}",
-                                     registry.getString( "appserver.home", "${appserver.home}" ) );
+        String value = StringUtils.replace( directory, "${appserver.base}", registry.getString( "appserver.base",
+                                                                                                "${appserver.base}" ) );
+        value = StringUtils.replace( value, "${appserver.home}", registry.getString( "appserver.home",
+                                                                                     "${appserver.home}" ) );
         return value;
     }
 
diff --git a/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparator.java b/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparator.java
new file mode 100644
index 000000000..89dd30b01
--- /dev/null
+++ b/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparator.java
@@ -0,0 +1,73 @@
+package org.apache.maven.archiva.configuration.functors;
+
+/*
+ * 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.archiva.configuration.ProxyConnectorConfiguration;
+
+import java.util.Comparator;
+
+/**
+ * ProxyConnectorConfigurationOrderComparator 
+ *
+ * @author <a href="mailto:joakim@erdfelt.com">Joakim Erdfelt</a>
+ * @version $Id$
+ */
+public class ProxyConnectorConfigurationOrderComparator
+    implements Comparator<ProxyConnectorConfiguration>
+{
+    private static ProxyConnectorConfigurationOrderComparator INSTANCE = new ProxyConnectorConfigurationOrderComparator();
+    
+    public int compare( ProxyConnectorConfiguration o1, ProxyConnectorConfiguration o2 )
+    {
+        if ( o1 == null && o2 == null )
+        {
+            return 0;
+        }
+
+        // Ensure null goes to end of list.
+        if ( o1 == null && o2 != null )
+        {
+            return 1;
+        }
+
+        if ( o1 != null && o2 == null )
+        {
+            return -1;
+        }
+
+        // Ensure 0 (unordered) goes to end of list.
+        if ( o1.getOrder() == 0 && o2.getOrder() != 0 )
+        {
+            return 1;
+        }
+        
+        if ( o1.getOrder() != 0 && o2.getOrder() == 0 )
+        {
+            return -1;
+        }
+
+        return o1.getOrder() - o2.getOrder();
+    }
+
+    public static ProxyConnectorConfigurationOrderComparator getInstance()
+    {
+        return INSTANCE;
+    }
+}
diff --git a/archiva-base/archiva-configuration/src/main/mdo/configuration.mdo b/archiva-base/archiva-configuration/src/main/mdo/configuration.mdo
index 08e2b8c2a..bd2c91a1f 100644
--- a/archiva-base/archiva-configuration/src/main/mdo/configuration.mdo
+++ b/archiva-base/archiva-configuration/src/main/mdo/configuration.mdo
@@ -141,23 +141,49 @@
         java.util.Map<String, NetworkProxyConfiguration> map = new java.util.HashMap<String, NetworkProxyConfiguration>();

         if ( networkProxies != null )

         {

-            for ( java.util.Iterator i = networkProxies.iterator(); i.hasNext(); )

+            for ( java.util.Iterator<NetworkProxyConfiguration> i = networkProxies.iterator(); i.hasNext(); )

             {

-                NetworkProxyConfiguration proxy = (NetworkProxyConfiguration) i.next();

+                NetworkProxyConfiguration proxy = i.next();

                 map.put( proxy.getId(), proxy );

             }

         }

         return map;

-    }

+    }
+    
+    public java.util.Map<String, java.util.List<ProxyConnectorConfiguration>> getProxyConnectorAsMap()
+    {
+        java.util.Map<String, java.util.List<ProxyConnectorConfiguration>> proxyConnectorMap = 
+            new java.util.HashMap<String, java.util.List<ProxyConnectorConfiguration>>();
+
+        java.util.Iterator<ProxyConnectorConfiguration> it = proxyConnectors.iterator();
+        while ( it.hasNext() )
+        {
+            ProxyConnectorConfiguration proxyConfig = it.next();
+            String key = proxyConfig.getSourceRepoId();
+
+            java.util.List<ProxyConnectorConfiguration> connectors = proxyConnectorMap.get( key );
+            if ( connectors == null )
+            {
+                connectors = new java.util.ArrayList<ProxyConnectorConfiguration>();
+                proxyConnectorMap.put( key, connectors );
+            }
+
+            connectors.add( proxyConfig );
+            java.util.Collections.sort( connectors,
+                org.apache.maven.archiva.configuration.functors.ProxyConnectorConfigurationOrderComparator.getInstance() );
+        }
+
+        return proxyConnectorMap;
+    }    
 

     public java.util.Map<String, RemoteRepositoryConfiguration> getRemoteRepositoriesAsMap()

     {

         java.util.Map<String, RemoteRepositoryConfiguration> map = new java.util.HashMap<String, RemoteRepositoryConfiguration>();

         if ( remoteRepositories != null )

         {

-            for ( java.util.Iterator i = remoteRepositories.iterator(); i.hasNext(); )

+            for ( java.util.Iterator<RemoteRepositoryConfiguration> i = remoteRepositories.iterator(); i.hasNext(); )

             {

-                RemoteRepositoryConfiguration repo = (RemoteRepositoryConfiguration) i.next();

+                RemoteRepositoryConfiguration repo = i.next();

                 map.put( repo.getId(), repo );

             }

         }

@@ -168,9 +194,9 @@
     {

         if ( remoteRepositories != null )

         {

-            for ( java.util.Iterator i = remoteRepositories.iterator(); i.hasNext(); )

+            for ( java.util.Iterator<RemoteRepositoryConfiguration> i = remoteRepositories.iterator(); i.hasNext(); )

             {

-                RemoteRepositoryConfiguration repo = (RemoteRepositoryConfiguration) i.next();

+                RemoteRepositoryConfiguration repo = i.next();

                 if ( repo.getId().equals( id ) )

                 {

                     return repo;

@@ -185,9 +211,9 @@
         java.util.Map<String, ManagedRepositoryConfiguration> map = new java.util.HashMap<String, ManagedRepositoryConfiguration>();

         if ( managedRepositories != null )

         {

-            for ( java.util.Iterator i = managedRepositories.iterator(); i.hasNext(); )

+            for ( java.util.Iterator<ManagedRepositoryConfiguration> i = managedRepositories.iterator(); i.hasNext(); )

             {

-                ManagedRepositoryConfiguration repo = (ManagedRepositoryConfiguration) i.next();

+                ManagedRepositoryConfiguration repo = i.next();

                 map.put( repo.getId(), repo );

             }

         }

@@ -198,9 +224,9 @@
     {

         if ( managedRepositories != null )

         {

-            for ( java.util.Iterator i = managedRepositories.iterator(); i.hasNext(); )

+            for ( java.util.Iterator<ManagedRepositoryConfiguration> i = managedRepositories.iterator(); i.hasNext(); )

             {

-                ManagedRepositoryConfiguration repo = (ManagedRepositoryConfiguration) i.next();

+                ManagedRepositoryConfiguration repo = i.next();

                 if ( repo.getId().equals( id ) )

                 {

                     return repo;

@@ -517,11 +543,25 @@
       <name>ProxyConnectorConfiguration</name>

       <version>1.0.0+</version>

       <fields>

+        <field>
+          <name>order</name>
+          <version>1.0.0+</version>
+          <description>
+            The order of the proxy connectors. (0 means no order specified)
+          </description>
+          <type>int</type>
+          <defaultValue>0</defaultValue>
+        </field>
       </fields>

       <codeSegments>

         <codeSegment>

           <version>1.0.0+</version>

           <code><![CDATA[

+    /**
+     * The order id for UNORDERED
+     */
+    public static final int UNORDERED = 0;
+    
     /**

      * The policy key {@link #getPolicies()} for snapshot handling.

      * See {@link org.apache.maven.archiva.policies.SnapshotsPolicy} 

diff --git a/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparatorTest.java b/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparatorTest.java
new file mode 100644
index 000000000..ea44cce67
--- /dev/null
+++ b/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/functors/ProxyConnectorConfigurationOrderComparatorTest.java
@@ -0,0 +1,131 @@
+package org.apache.maven.archiva.configuration.functors;
+
+/*
+ * 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.commons.lang.StringUtils;
+import org.apache.maven.archiva.configuration.ProxyConnectorConfiguration;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import junit.framework.TestCase;
+
+/**
+ * ProxyConnectorConfigurationOrderComparatorTest 
+ *
+ * @author <a href="mailto:joakim@erdfelt.com">Joakim Erdfelt</a>
+ * @version $Id$
+ */
+public class ProxyConnectorConfigurationOrderComparatorTest
+    extends TestCase
+{
+    public void testSortOfAllZeros()
+    {
+        List<ProxyConnectorConfiguration> proxies = new ArrayList<ProxyConnectorConfiguration>();
+
+        proxies.add( createConnector( "corporate", 0 ) );
+        proxies.add( createConnector( "snapshots", 0 ) );
+        proxies.add( createConnector( "3rdparty", 0 ) );
+        proxies.add( createConnector( "sandbox", 0 ) );
+
+        Collections.sort( proxies, ProxyConnectorConfigurationOrderComparator.getInstance() );
+
+        assertProxyOrder( new String[] { "corporate", "snapshots", "3rdparty", "sandbox" }, proxies );
+    }
+
+    public void testSortNormal()
+    {
+        List<ProxyConnectorConfiguration> proxies = new ArrayList<ProxyConnectorConfiguration>();
+
+        proxies.add( createConnector( "corporate", 3 ) );
+        proxies.add( createConnector( "snapshots", 1 ) );
+        proxies.add( createConnector( "3rdparty", 2 ) );
+        proxies.add( createConnector( "sandbox", 4 ) );
+
+        Collections.sort( proxies, new ProxyConnectorConfigurationOrderComparator() );
+
+        assertProxyOrder( new String[] { "snapshots", "3rdparty", "corporate", "sandbox" }, proxies );
+    }
+
+    public void testSortPartial()
+    {
+        List<ProxyConnectorConfiguration> proxies = new ArrayList<ProxyConnectorConfiguration>();
+
+        proxies.add( createConnector( "corporate", 3 ) );
+        proxies.add( createConnector( "snapshots", 0 ) );
+        proxies.add( createConnector( "3rdparty", 2 ) );
+        proxies.add( createConnector( "sandbox", 0 ) );
+
+        Collections.sort( proxies, new ProxyConnectorConfigurationOrderComparator() );
+
+        assertProxyOrder( new String[] { "3rdparty", "corporate", "snapshots", "sandbox" }, proxies );
+    }
+
+    private void assertProxyOrder( String[] ids, List<ProxyConnectorConfiguration> proxies )
+    {
+        assertEquals( "Proxies.size() == ids.length", ids.length, proxies.size() );
+
+        int orderFailedAt = -1;
+
+        for ( int i = 0; i < ids.length; i++ )
+        {
+            if ( !StringUtils.equals( ids[i], proxies.get( i ).getProxyId() ) )
+            {
+                orderFailedAt = i;
+                break;
+            }
+        }
+
+        if ( orderFailedAt >= 0 )
+        {
+            StringBuffer msg = new StringBuffer();
+
+            msg.append( "Failed expected order of the proxies <" );
+            msg.append( StringUtils.join( ids, ", " ) );
+            msg.append( ">, actual <" );
+
+            boolean needsComma = false;
+            for ( ProxyConnectorConfiguration proxy : proxies )
+            {
+                if ( needsComma )
+                {
+                    msg.append( ", " );
+                }
+                msg.append( proxy.getProxyId() );
+                needsComma = true;
+            }
+            msg.append( "> failure at index <" ).append( orderFailedAt ).append( ">." );
+
+            fail( msg.toString() );
+        }
+    }
+
+    private ProxyConnectorConfiguration createConnector( String id, int order )
+    {
+        ProxyConnectorConfiguration proxy = new ProxyConnectorConfiguration();
+        proxy.setProxyId( id );
+        proxy.setOrder( order );
+        proxy.setSourceRepoId( id + "_m" );
+        proxy.setTargetRepoId( id + "_r" );
+
+        return proxy;
+    }
+}