[MRM-758] cron expressions with a comma are not being scheduled after configuration save

Merged from: r642886


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

3 files changed, 40 insertions, 27 deletions

diff --git a/archiva-modules/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java b/archiva-modules/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
index 28ceeaa7b..ea0f57ac0 100644
--- a/archiva-modules/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
+++ b/archiva-modules/archiva-base/archiva-configuration/src/main/java/org/apache/maven/archiva/configuration/DefaultArchivaConfiguration.java
@@ -145,7 +145,7 @@ public class DefaultArchivaConfiguration
         if ( configuration == null )
         {
             configuration = load();
-            configuration = processExpressions( configuration );
+            configuration = unescapeExpressions( configuration );
         }
 
         return configuration;
@@ -432,19 +432,7 @@ public class DefaultArchivaConfiguration
         }
 
         // escape all cron expressions to handle ','
-        for ( Iterator<ManagedRepositoryConfiguration> i = configuration.getManagedRepositories().iterator(); i
-            .hasNext(); )
-        {
-            ManagedRepositoryConfiguration c = i.next();
-            c.setRefreshCronExpression( escapeCronExpression( c.getRefreshCronExpression() ) );
-        }
-
-        if ( configuration.getDatabaseScanning() != null )
-        {
-            configuration.getDatabaseScanning().setCronExpression(
-                                                                   escapeCronExpression( configuration
-                                                                       .getDatabaseScanning().getCronExpression() ) );
-        }
+        escapeCronExpressions( configuration );
 
         // [MRM-661] Due to a bug in the modello registry writer, we need to take these out by hand. They'll be put back by the writer.
         if ( configuration.getManagedRepositories().isEmpty() )
@@ -493,9 +481,23 @@ public class DefaultArchivaConfiguration
         new ConfigurationRegistryWriter().write( configuration, section );
         section.save();
 
+        this.configuration = unescapeExpressions( configuration );
+
         triggerEvent( ConfigurationEvent.SAVED );
+    }
 
-        this.configuration = processExpressions( configuration );
+    private void escapeCronExpressions( Configuration configuration )
+    {
+        for ( ManagedRepositoryConfiguration c : (List<ManagedRepositoryConfiguration>) configuration.getManagedRepositories() )
+        {
+            c.setRefreshCronExpression( escapeCronExpression( c.getRefreshCronExpression() ) );
+        }
+
+        DatabaseScanningConfiguration scanning = configuration.getDatabaseScanning();
+        if ( scanning != null )
+        {
+            scanning.setCronExpression( escapeCronExpression( scanning.getCronExpression() ) );
+        }
     }
 
     private Registry createDefaultConfigurationFile()
@@ -576,14 +578,7 @@ public class DefaultArchivaConfiguration
         ConfigurationEvent evt = new ConfigurationEvent( type );
         for ( ConfigurationListener listener : listeners )
         {
-            try
-            {
-                listener.configurationEvent( evt );
-            }
-            catch ( Throwable t )
-            {
-                log.warn( "Unable to notify of saved configuration event.", t );
-            }
+            listener.configurationEvent( evt );
         }
     }
 
@@ -678,7 +673,7 @@ public class DefaultArchivaConfiguration
         return StringUtils.replace( cronExpression, ",", "\\," );
     }
 
-    private Configuration processExpressions( Configuration config )
+    private Configuration unescapeExpressions( Configuration config )
     {
         // TODO: for commons-configuration 1.3 only
         for ( Iterator<ManagedRepositoryConfiguration> i = config.getManagedRepositories().iterator(); i.hasNext(); )
diff --git a/archiva-modules/archiva-base/archiva-configuration/src/test/conf/escape-cron-expressions.xml b/archiva-modules/archiva-base/archiva-configuration/src/test/conf/escape-cron-expressions.xml
index 48de2878d..547e8c0b5 100644
--- a/archiva-modules/archiva-base/archiva-configuration/src/test/conf/escape-cron-expressions.xml
+++ b/archiva-modules/archiva-base/archiva-configuration/src/test/conf/escape-cron-expressions.xml
@@ -26,7 +26,7 @@
       <location>file://${appserver.base}/repositories/internal</location>
       <releases>false</releases>
       <snapshots>true</snapshots>
-      <refreshCronExpression>0 0\,30 * * ?</refreshCronExpression>
+      <refreshCronExpression>0 0\,30 * * * ?</refreshCronExpression>
     </managedRepository>
   </managedRepositories>
   <databaseScanning>
diff --git a/archiva-modules/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/ArchivaConfigurationTest.java b/archiva-modules/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/ArchivaConfigurationTest.java
index d96965c98..82b79fc93 100644
--- a/archiva-modules/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/ArchivaConfigurationTest.java
+++ b/archiva-modules/archiva-base/archiva-configuration/src/test/java/org/apache/maven/archiva/configuration/ArchivaConfigurationTest.java
@@ -618,7 +618,7 @@ public class ArchivaConfigurationTest
         userFile.getParentFile().mkdirs();
         FileUtils.writeStringToFile( userFile, "<configuration/>", null );
 
-        ArchivaConfiguration archivaConfiguration =
+        final ArchivaConfiguration archivaConfiguration =
             (ArchivaConfiguration) lookup( ArchivaConfiguration.class.getName(), "test-cron-expressions" );
 
         Configuration configuration = archivaConfiguration.getConfiguration();
@@ -626,10 +626,28 @@ public class ArchivaConfigurationTest
         ManagedRepositoryConfiguration repository =
             (ManagedRepositoryConfiguration) configuration.getManagedRepositories().get( 0 );
 
-        assertEquals( "check cron expression", "0 0,30 * * ?", repository.getRefreshCronExpression().trim() );
+        assertEquals( "check cron expression", "0 0,30 * * * ?", repository.getRefreshCronExpression().trim() );
 
         configuration.getDatabaseScanning().setCronExpression( "0 0,15 0 * * ?" );
 
+        // add a test listener to confirm it doesn't see the escaped format. We don't need to test the number of calls,
+        // etc. as it's done in other tests
+        archivaConfiguration.addListener( new ConfigurationListener()
+        {
+            public void configurationEvent( ConfigurationEvent event )
+            {
+                assertEquals( ConfigurationEvent.SAVED, event.getType() );
+
+                Configuration configuration = archivaConfiguration.getConfiguration();
+
+                ManagedRepositoryConfiguration repository =
+                    (ManagedRepositoryConfiguration) configuration.getManagedRepositories().get( 0 );
+
+                assertEquals( "check cron expression", "0 0,15 0 * * ?",
+                              configuration.getDatabaseScanning().getCronExpression() );
+            }
+        } );
+
         archivaConfiguration.save( configuration );
 
         configuration = archivaConfiguration.getConfiguration();