Remove the ability to retrospectively register settings classes with …

…config

community/kernel/src/main/java/org/neo4j/kernel/configuration/Config.java

@@ -31,41 +31,37 @@
 
 import org.neo4j.graphdb.config.Configuration;
 import org.neo4j.graphdb.config.Setting;
-import org.neo4j.helpers.collection.Iterables;
 import org.neo4j.kernel.info.DiagnosticsPhase;
 import org.neo4j.kernel.info.DiagnosticsProvider;
 import org.neo4j.logging.BufferingLog;
 import org.neo4j.logging.Log;
 import org.neo4j.logging.Logger;
 
 import static java.util.Arrays.asList;
-import static java.util.Collections.emptyList;
 
 /**
- * This class holds the overall configuration of a Neo4j database instance. Use the accessors
- * to convert the internal key-value settings to other types.
+ * This class holds the overall configuration of a Neo4j database instance. Use the accessors to convert the internal
+ * key-value settings to other types.
  * <p>
- * Users can assume that old settings have been migrated to their new counterparts, and that defaults
- * have been applied.
+ * Users can assume that old settings have been migrated to their new counterparts, and that defaults have been applied.
  * <p>
- * UI's can change configuration by calling applyChanges. Any listener, such as services that use
- * this configuration, can be notified of changes by implementing the {@link ConfigurationChangeListener} interface.
+ * UI's can change configuration by calling augment(). Any listener, such as services that use this configuration, can
+ * be notified of changes by implementing the {@link ConfigurationChangeListener} interface.
  */
 public class Config implements DiagnosticsProvider, Configuration
 {
     private final List<ConfigurationChangeListener> listeners = new CopyOnWriteArrayList<>();
     private final Map<String, String> params = new ConcurrentHashMap<>();
-    private final ConfigValues settingsFunction;
+    private final Iterable<Class<?>> settingsClasses;
+    private final ConfigurationMigrator migrator;
+    private final ConfigurationValidator validator;
 
-    // Messages to this log get replayed into a real logger once logging has been
-    // instantiated.
+    private ConfigValues settingsFunction;
+
+    // Messages to this log get replayed into a real logger once logging has been instantiated.
     private final BufferingLog bufferedLog = new BufferingLog();
     private Log log = bufferedLog;
 
-    private Iterable<Class<?>> settingsClasses = emptyList();
-    private ConfigurationMigrator migrator;
-    private ConfigurationValidator validator;
-
     public static Config empty()
     {
         return new Config();
@@ -91,11 +87,12 @@ public Config( Map<String, String> inputParams, Class<?>... settingsClasses )
         this( inputParams, asList( settingsClasses ) );
     }
 
-    public Config( Map<String, String> inputParams, Iterable<Class<?>> settingsClasses )
+    public Config( Map<String, String> params, Iterable<Class<?>> settingsClasses )
     {
-        this.params.putAll( inputParams );
-        this.settingsFunction = new ConfigValues( params );
-        registerSettingsClasses( settingsClasses );
+        this.settingsClasses = settingsClasses;
+        migrator = new AnnotationBasedConfigurationMigrator( settingsClasses );
+        validator = new ConfigurationValidator( settingsClasses );
+        replaceSettings( params );
     }
 
     /**
@@ -151,21 +148,6 @@ public Config augment( Map<String, String> changes )
         return this;
     }
 
-    /**
-     * Add more settings classes.
-     */
-    public Config registerSettingsClasses( Iterable<Class<?>> settingsClasses )
-    {
-        this.settingsClasses = Iterables.concat( settingsClasses, this.settingsClasses );
-        this.migrator = new AnnotationBasedConfigurationMigrator( settingsClasses );
-        this.validator = new ConfigurationValidator( settingsClasses );
-
-        // Apply the requirements and changes the new settings classes introduce
-        this.replaceSettings( getParams() );
-
-        return this;
-    }
-
     public Iterable<Class<?>> getSettingsClasses()
     {
         return settingsClasses;
@@ -229,7 +211,7 @@ public String toString()
         return output.toString();
     }
 
-    private synchronized Config replaceSettings( Map<String, String> newValues )
+    private synchronized void replaceSettings( Map<String, String> newValues )
     {
         newValues = migrator.apply( newValues, log );
 
@@ -261,7 +243,7 @@ private synchronized Config replaceSettings( Map<String, String> newValues )
             if ( configurationChanges.isEmpty() )
             {
                 // Don't bother... nothing changed.
-                return this;
+                return;
             }
 
             // Make the change
@@ -283,7 +265,6 @@ private synchronized Config replaceSettings( Map<String, String> newValues )
                 listener.notifyConfigurationChanges( configurationChanges );
             }
         }
-
-        return this;
+        settingsFunction = new ConfigValues( this.params );
     }
 }

community/kernel/src/test/java/org/neo4j/kernel/configuration/ConfigTest.java

@@ -32,8 +32,6 @@
 import org.neo4j.graphdb.config.InvalidSettingException;
 import org.neo4j.graphdb.config.Setting;
 
-import static java.util.Arrays.asList;
-
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.nullValue;
 import static org.hamcrest.Matchers.equalTo;
@@ -126,20 +124,6 @@ public void shouldNotAllowSettingInvalidValues()
         fail( "Expected validation to fail." );
     }
 
-    @Test
-    public void shouldBeAbleToRegisterSettingsClassesAfterInstantiation() throws Exception
-    {
-        // Given
-        Config config = new Config( stringMap( "old", "hello!" ) );
-
-        // When
-        config.registerSettingsClasses( asList( MySettingsWithDefaults.class, MyMigratingSettings.class ) );
-
-        // Then
-        assertThat( config.get( MyMigratingSettings.newer ), equalTo( "hello!" ) );
-        assertThat( config.get( MySettingsWithDefaults.hello ), equalTo( "Hello, World!" ) );
-    }
-
     @Test
     public void shouldBeAbleToAugmentConfig() throws Exception
     {

community/server/src/main/java/org/neo4j/server/configuration/BaseServerConfigLoader.java

@@ -46,11 +46,16 @@ public Config loadConfig( File configFile, File legacyConfigFile, Log log, Pair<
         }
 
         HashMap<String, String> settings = calculateSettings( configFile, legacyConfigFile, log, configOverrides );
-        Config config = new Config( settings, asList( ServerSettings.class, GraphDatabaseSettings.class ) );
+        Config config = new Config( settings, settingsClasses(settings) );
         config.setLogger( log );
         return config;
     }
 
+    protected Iterable<Class<?>> settingsClasses( HashMap<String, String> settings )
+    {
+        return asList( ServerSettings.class, GraphDatabaseSettings.class );
+    }
+
     private HashMap<String, String> calculateSettings( File configFile, File legacyConfigFile, Log log,
                                                        Pair<String, String>[] configOverrides )
     {

enterprise/server-enterprise/src/main/java/org/neo4j/server/enterprise/EnterpriseServerConfigLoader.java

@@ -19,27 +19,37 @@
  */
 package org.neo4j.server.enterprise;
 
-import java.io.File;
+import java.util.HashMap;
 
 import org.neo4j.cluster.ClusterSettings;
-import org.neo4j.helpers.collection.Pair;
+import org.neo4j.helpers.collection.Iterables;
 import org.neo4j.kernel.configuration.Config;
 import org.neo4j.kernel.ha.HaSettings;
-import org.neo4j.logging.Log;
 import org.neo4j.server.configuration.BaseServerConfigLoader;
 
 import static java.util.Arrays.asList;
 
+import static org.neo4j.server.enterprise.EnterpriseServerSettings.mode;
+
 public class EnterpriseServerConfigLoader extends BaseServerConfigLoader
 {
     @Override
-    public Config loadConfig( File configFile, File legacyConfigFile, Log log, Pair<String,String>... configOverrides )
+    protected Iterable<Class<?>> settingsClasses( HashMap<String, String> settings )
     {
-        Config config = super.loadConfig( configFile, legacyConfigFile, log, configOverrides );
-        if ( config.get( EnterpriseServerSettings.mode ).equals( "HA" ) )
+        if ( isHAMode( settings ) )
+        {
+            return Iterables.concat(
+                    super.settingsClasses( settings ),
+                    asList( HaSettings.class, ClusterSettings.class ) );
+        }
+        else
         {
-            config.registerSettingsClasses( asList( HaSettings.class, ClusterSettings.class ) );
+            return super.settingsClasses( settings );
         }
-        return config;
+    }
+
+    private boolean isHAMode( HashMap<String, String> settings )
+    {
+        return new Config( settings, EnterpriseServerSettings.class ).get( mode ).equals( "HA" );
     }
 }