Add HA and CC configuration validation

community/dbms/src/main/java/org/neo4j/server/configuration/ConfigLoader.java

@@ -21,6 +21,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -29,8 +30,8 @@
 
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.helpers.collection.Pair;
-import org.neo4j.kernel.configuration.BoltConnector;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.configuration.ConfigurationValidator;
 import org.neo4j.kernel.configuration.Connector;
 import org.neo4j.kernel.configuration.Settings;
 
@@ -58,14 +59,14 @@ public static Config loadConfig( Optional<File> homeDir, Optional<File> configFi
     public static Config loadServerConfig( Optional<File> configFile, Pair<String,String>... configOverrides )
             throws IOException
     {
-        return loadServerConfig( Optional.empty(), configFile, configOverrides );
+        return loadServerConfig( Optional.empty(), configFile, configOverrides, Collections.emptyList() );
     }
 
     public static Config loadServerConfig( Optional<File> homeDir, Optional<File> configFile,
-            Pair<String,String>[] configOverrides )
+            Pair<String,String>[] configOverrides, Collection<ConfigurationValidator> additionalValidators )
     {
         Map<String,String> overriddenSettings = calculateSettings( homeDir, configOverrides );
-        return Config.serverDefaults( configFile, overriddenSettings, Collections.emptyList() );
+        return Config.serverDefaults( configFile, overriddenSettings, additionalValidators );
     }
 
     public static Config loadConfigWithConnectorsDisabled( Optional<File> homeDir, Optional<File> configFile,

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

@@ -171,7 +171,7 @@ public static Config serverDefaults( Map<String,String> additionalConfig )
     public static Config serverDefaults( Optional<File> configFile, Map<String,String> additionalConfig,
             Collection<ConfigurationValidator> additionalValidators )
     {
-        ArrayList<ConfigurationValidator> validators = new ArrayList<>(  );
+        ArrayList<ConfigurationValidator> validators = new ArrayList<>();
         validators.addAll( additionalValidators );
         validators.add( new ServerConfigurationValidator() );
 
@@ -288,7 +288,7 @@ public void setLogger( Log log )
     {
         if ( this.log instanceof BufferingLog )
         {
-            ( (BufferingLog) this.log ).replayInto( log );
+            ((BufferingLog) this.log).replayInto( log );
         }
         this.log = log;
     }
@@ -407,6 +407,7 @@ private synchronized void replaceSettings( Map<String,String> newSettings )
         params.putAll( validSettings );
     }
 
+    @Nonnull
     private static Map<String,String> initSettings( @Nonnull Optional<File> configFile,
             @Nonnull Consumer<Map<String,String>> settingsPostProcessor,
             @Nonnull Map<String,String> overriddenSettings,
@@ -419,6 +420,7 @@ private static Map<String,String> initSettings( @Nonnull Optional<File> configFi
         return settings;
     }
 
+    @Nonnull
     private static Map<String,String> loadFromFile( @Nonnull File file, @Nonnull Log log )
     {
         if ( !file.exists() )
@@ -440,7 +442,17 @@ private static Map<String,String> loadFromFile( @Nonnull File file, @Nonnull Log
     /**
      * @return a list of all connector names like 'http' in 'dbms.connector.http.enabled = true'
      */
+    @Nonnull
     public List<String> allConnectorIdentifiers()
+    {
+        return allConnectorIdentifiers( params );
+    }
+
+    /**
+     * @return a list of all connector names like 'http' in 'dbms.connector.http.enabled = true'
+     */
+    @Nonnull
+    public static List<String> allConnectorIdentifiers( @Nonnull Map<String,String> params )
     {
         Pattern pattern = Pattern.compile(
                 Pattern.quote( "dbms.connector." ) + "([^\\.]+)\\.(.+)" );
@@ -456,9 +468,19 @@ public List<String> allConnectorIdentifiers()
     /**
      * @return list of all configured bolt connectors
      */
+    @Nonnull
     public List<BoltConnector> boltConnectors()
     {
-        return allConnectorIdentifiers().stream()
+        return boltConnectors( params );
+    }
+
+    /**
+     * @return list of all configured bolt connectors
+     */
+    @Nonnull
+    public static List<BoltConnector> boltConnectors( @Nonnull Map<String,String> params )
+    {
+        return allConnectorIdentifiers( params ).stream()
                 .map( BoltConnector::new )
                 .filter( c ->
                         c.group.groupKey.equalsIgnoreCase( "bolt" ) || BOLT.equals( c.type.apply( params::get ) ) )
@@ -468,19 +490,39 @@ public List<BoltConnector> boltConnectors()
     /**
      * @return list of all configured bolt connectors which are enabled
      */
+    @Nonnull
     public List<BoltConnector> enabledBoltConnectors()
     {
-        return boltConnectors().stream()
+        return enabledBoltConnectors( params );
+    }
+
+    /**
+     * @return list of all configured bolt connectors which are enabled
+     */
+    @Nonnull
+    public static List<BoltConnector> enabledBoltConnectors( @Nonnull Map<String,String> params )
+    {
+        return boltConnectors( params ).stream()
                 .filter( c -> c.enabled.apply( params::get ) )
                 .collect( Collectors.toList() );
     }
 
     /**
      * @return list of all configured http connectors
      */
+    @Nonnull
     public List<HttpConnector> httpConnectors()
     {
-        return allConnectorIdentifiers().stream()
+        return httpConnectors( params );
+    }
+
+    /**
+     * @return list of all configured http connectors
+     */
+    @Nonnull
+    public static List<HttpConnector> httpConnectors( @Nonnull Map<String,String> params )
+    {
+        return allConnectorIdentifiers( params ).stream()
                 .map( name -> new Connector( name, "ignored" ) )
                 .filter( c -> c.group.groupKey.equalsIgnoreCase( "http" ) ||
                         c.group.groupKey.equalsIgnoreCase( "https" ) ||
@@ -509,9 +551,19 @@ public List<HttpConnector> httpConnectors()
     /**
      * @return list of all configured http connectors which are enabled
      */
+    @Nonnull
     public List<HttpConnector> enabledHttpConnectors()
     {
-        return httpConnectors().stream()
+        return enabledHttpConnectors( params );
+    }
+
+    /**
+     * @return list of all configured http connectors which are enabled
+     */
+    @Nonnull
+    public static List<HttpConnector> enabledHttpConnectors( @Nonnull Map<String,String> params )
+    {
+        return httpConnectors( params ).stream()
                 .filter( c -> c.enabled.apply( params::get ) )
                 .collect( Collectors.toList() );
     }

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

@@ -43,6 +43,7 @@ public interface ConfigurationValidator
      * @return a Map of valid keys and values.
      * @throws InvalidSettingException in case of invalid values
      */
+    @Nonnull
     Map<String,String> validate( @Nonnull Collection<SettingValidator> settingValidators,
             @Nonnull Map<String,String> rawConfig,
             @Nonnull Log log ) throws InvalidSettingException;

community/kernel/src/main/java/org/neo4j/kernel/impl/proc/ProcedureAllowedConfig.java

@@ -21,13 +21,10 @@
 
 import java.util.Collections;
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import org.neo4j.configuration.ConfigValue;
 import org.neo4j.kernel.configuration.Config;
 
 import static java.util.Arrays.stream;
@@ -54,19 +51,24 @@ public ProcedureAllowedConfig( Config config )
     {
         this.defaultValue = config.getValue( PROC_ALLOWED_SETTING_DEFAULT_NAME )
                 .map( Object::toString )
-                .orElse( null );
-        Optional<?> allowedRoles = config.getValue( PROC_ALLOWED_SETTING_ROLES );
-        this.matchers = allowedRoles
-                .map( o -> Stream.of( o.toString().split( SETTING_DELIMITER ) )
-                        .map( procToRoleSpec ->
-                        {
-                            String[] spec = procToRoleSpec.split( MAPPING_DELIMITER );
-                            String[] roles = stream( spec[1].split( ROLES_DELIMITER ) )
-                                    .map( String::trim ).toArray( String[]::new );
-                            return new ProcMatcher( spec[0].trim(), roles );
-                        } )
-                        .collect( Collectors.toList() ) )
-                .orElseGet( Collections::emptyList );
+                .orElse( "" );
+        String allowedRoles = config.getValue( PROC_ALLOWED_SETTING_ROLES ).map( Object::toString ).orElse( "" );
+        if ( allowedRoles.isEmpty() )
+        {
+            this.matchers = Collections.emptyList();
+        }
+        else
+        {
+            this.matchers = Stream.of( allowedRoles.split( SETTING_DELIMITER ) )
+                    .map( procToRoleSpec ->
+                    {
+                        String[] spec = procToRoleSpec.split( MAPPING_DELIMITER );
+                        String[] roles = stream( spec[1].split( ROLES_DELIMITER ) )
+                                .map( String::trim ).toArray( String[]::new );
+                        return new ProcMatcher( spec[0].trim(), roles );
+                    } )
+                    .collect( Collectors.toList() );
+        }
     }
 
     String[] rolesFor( String procedureName )

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

@@ -22,20 +22,27 @@
 import org.junit.Test;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Map;
 import java.util.Optional;
 
+import javax.annotation.Nonnull;
+
 import org.neo4j.configuration.LoadableConfig;
 import org.neo4j.graphdb.config.InvalidSettingException;
 import org.neo4j.graphdb.config.Setting;
+import org.neo4j.graphdb.config.SettingValidator;
 import org.neo4j.logging.Log;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.neo4j.helpers.collection.MapUtil.stringMap;
@@ -202,4 +209,30 @@ public void shouldPassOnBufferedLogInWithDefaults() throws Exception
         verify( log ).warn( "Unknown config option: %s", "third.jibberish" );
         verifyNoMoreInteractions( log );
     }
+
+    @Test
+    public void shouldPassOnValidatorsOnWithMethods() throws Exception
+    {
+        // Given
+        ConfigurationValidator validator = spy( new ConfigurationValidator()
+        {
+            @Nonnull
+            @Override
+            public Map<String,String> validate( @Nonnull Collection<SettingValidator> settingValidators,
+                    @Nonnull Map<String,String> rawConfig, @Nonnull Log log ) throws InvalidSettingException
+            {
+                return rawConfig;
+            }
+        } );
+
+        Config first = Config.embeddedDefaults( stringMap( "first.jibberish", "bah" ),
+                Collections.singleton( validator ) );
+
+        // When
+        Config second = first.withDefaults( stringMap( "second.jibberish", "baah" ) );
+        second.with( stringMap( "third.jibberish", "baah" ) );
+
+        // Then
+        verify( validator, times( 3 ) ).validate( any(), any(), any() );
+    }
 }

community/server/pom.xml

@@ -181,6 +181,11 @@
       <version>${project.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>annotations</artifactId>
+    </dependency>
+
     <dependency>
       <groupId>org.eclipse.jetty</groupId>
       <artifactId>jetty-server</artifactId>

community/server/src/main/java/org/neo4j/server/CommunityBootstrapper.java

@@ -19,22 +19,18 @@
  */
 package org.neo4j.server;
 
-import java.util.List;
-import java.util.Map;
+import java.util.Collection;
+import java.util.Collections;
+import javax.annotation.Nonnull;
 
-import org.neo4j.dbms.DatabaseManagementSystemSettings;
-import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.kernel.GraphDatabaseDependencies;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.configuration.ConfigurationValidator;
+import org.neo4j.kernel.configuration.ServerConfigurationValidator;
 import org.neo4j.logging.LogProvider;
-import org.neo4j.server.configuration.ServerSettings;
-
-import static java.util.Arrays.asList;
 
 public class CommunityBootstrapper extends ServerBootstrapper
 {
-    public static final List<Class<?>> settingsClasses =
-            asList( ServerSettings.class, GraphDatabaseSettings.class, DatabaseManagementSystemSettings.class );
 
     @Override
     protected NeoServer createNeoServer( Config config, GraphDatabaseDependencies dependencies,
@@ -44,9 +40,10 @@ protected NeoServer createNeoServer( Config config, GraphDatabaseDependencies de
     }
 
     @Override
-    protected Iterable<Class<?>> settingsClasses( Map<String, String> settings )
+    @Nonnull
+    protected Collection<ConfigurationValidator> configurationValidators()
     {
-        return settingsClasses;
+        return Collections.singletonList( new ServerConfigurationValidator() );
     }
 
 }

community/server/src/main/java/org/neo4j/server/ServerBootstrapper.java

@@ -20,16 +20,18 @@
 package org.neo4j.server;
 
 import java.io.File;
-import java.util.Map;
+import java.util.Collection;
 import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.annotation.Nonnull;
 
 import org.neo4j.graphdb.TransactionFailureException;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.helpers.collection.Pair;
 import org.neo4j.kernel.GraphDatabaseDependencies;
 import org.neo4j.kernel.configuration.Config;
+import org.neo4j.kernel.configuration.ConfigurationValidator;
 import org.neo4j.kernel.configuration.HttpConnector.Encryption;
 import org.neo4j.kernel.info.JvmChecker;
 import org.neo4j.kernel.info.JvmMetadataRepository;
@@ -149,7 +151,8 @@ public NeoServer getServer()
     protected abstract NeoServer createNeoServer( Config config, GraphDatabaseDependencies dependencies,
                                                   LogProvider userLogProvider );
 
-    protected abstract Iterable<Class<?>> settingsClasses( Map<String, String> settings );
+    @Nonnull
+    protected abstract Collection<ConfigurationValidator> configurationValidators();
 
     private static LogProvider setupLogging( Config config )
     {
@@ -165,7 +168,8 @@ private static LogProvider setupLogging( Config config )
 
     private Config createConfig( File homeDir, Optional<File> file, Pair<String, String>[] configOverrides )
     {
-        return ConfigLoader.loadConfig( Optional.of( homeDir ), file, configOverrides );
+        return ConfigLoader.loadServerConfig( Optional.of( homeDir ), file, configOverrides,
+                configurationValidators() );
     }
 
     private void addShutdownHook()