Skip to content

Commit

Permalink
Rationalize handling of the case where no config dir is specified
Browse files Browse the repository at this point in the history
  • Loading branch information
@benbc
benbc committed Mar 22, 2016
1 parent 81740f8 commit f01cc62
Show file tree
Hide file tree
Showing 15 changed files with 78 additions and 71 deletions.
3 changes: 2 additions & 1 deletion community/server/src/main/java/org/neo4j/server/BlockingBootstrapper.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.server;

import java.io.File;
import java.util.Optional;
import java.util.concurrent.CountDownLatch;

import org.neo4j.helpers.collection.Pair;
Expand All @@ -37,7 +38,7 @@ public BlockingBootstrapper( Bootstrapper wrapped )

@SafeVarargs
@Override
public final int start( File configFile, Pair<String, String>... configOverrides )
public final int start( Optional<File> configFile, Pair<String, String>... configOverrides )
{
int status = wrapped.start( configFile, configOverrides );
if ( status != ServerBootstrapper.OK )
Expand Down
3 changes: 2 additions & 1 deletion community/server/src/main/java/org/neo4j/server/Bootstrapper.java
View file
Expand Up @@ -20,12 +20,13 @@
package org.neo4j.server;

import java.io.File;
import java.util.Optional;

import org.neo4j.helpers.collection.Pair;

public interface Bootstrapper
{
int start( File configFile, Pair<String, String>... configOverrides );
int start( Optional<File> configFile, Pair<String, String>... configOverrides );

int stop();
}
5 changes: 3 additions & 2 deletions community/server/src/main/java/org/neo4j/server/ServerBootstrapper.java
View file
Expand Up @@ -22,6 +22,7 @@
import java.io.File;
import java.io.IOException;
import java.util.HashMap;
import java.util.Optional;
import java.util.logging.Level;
import java.util.logging.Logger;

Expand Down Expand Up @@ -61,7 +62,7 @@ public static int start( Bootstrapper boot, String... argv )

@Override
@SafeVarargs
public final int start( File configFile, Pair<String, String>... configOverrides )
public final int start( Optional<File> configFile, Pair<String, String>... configOverrides )
{
LogProvider userLogProvider = setupLogging();
dependencies = dependencies.userLogProvider( userLogProvider );
Expand Down Expand Up @@ -149,7 +150,7 @@ private static LogProvider setupLogging()
return userLogProvider;
}

private Config createConfig( Log log, File file, Pair<String, String>[] configOverrides ) throws IOException
private Config createConfig( Log log, Optional<File> file, Pair<String, String>[] configOverrides ) throws IOException
{
return new ConfigLoader( this::settingsClasses ).loadConfig( file, log, configOverrides );
}
Expand Down
10 changes: 4 additions & 6 deletions community/server/src/main/java/org/neo4j/server/ServerCommandLineArgs.java
View file
Expand Up @@ -21,6 +21,7 @@

import java.io.File;
import java.util.Collection;
import java.util.Optional;

import org.neo4j.helpers.Args;
import org.neo4j.helpers.collection.Pair;
Expand Down Expand Up @@ -62,13 +63,10 @@ public Pair<String, String>[] configOverrides()
return configOverrides;
}

public File configFile()
public Optional<File> configFile()
{
if ( !args.has( CONFIG_DIR_ARG ) )
{
return null;
}
return new File( new File( args.get( CONFIG_DIR_ARG ) ), ConfigLoader.DEFAULT_CONFIG_FILE_NAME );
return Optional.ofNullable( args.get( CONFIG_DIR_ARG ) )
.map( (dirPath) -> new File( new File( dirPath ), ConfigLoader.DEFAULT_CONFIG_FILE_NAME ) );
}

private static Pair<String, String>[] parseConfigOverrides( Args arguments )
Expand Down
34 changes: 13 additions & 21 deletions community/server/src/main/java/org/neo4j/server/configuration/ConfigLoader.java
View file
Expand Up @@ -24,6 +24,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.collection.MapUtil;
Expand All @@ -50,7 +51,7 @@ public ConfigLoader( List<Class<?>> settingsClasses )
this( settings -> settingsClasses );
}

public Config loadConfig( File configFile, Log log, Pair<String, String>... configOverrides )
public Config loadConfig( Optional<File> configFile, Log log, Pair<String, String>... configOverrides )
{
if ( log == null )
{
Expand All @@ -63,13 +64,12 @@ public Config loadConfig( File configFile, Log log, Pair<String, String>... conf
return config;
}

private HashMap<String, String> calculateSettings( File config, Log log, Pair<String, String>[] configOverrides )
private HashMap<String, String> calculateSettings( Optional<File> config, Log log,
Pair<String, String>[] configOverrides )
{
HashMap<String, String> settings = new HashMap<>();
if ( config != null && config.exists() )
{
settings.putAll( loadFromFile( log, config ) );
}

config.ifPresent( ( c ) -> settings.putAll( loadFromFile( log, c ) ) );
settings.putAll( toMap( configOverrides ) );
overrideEmbeddedDefaults( settings );
return settings;
Expand Down Expand Up @@ -98,29 +98,21 @@ private static void overrideEmbeddedDefaults( HashMap<String, String> config )

private static Map<String, String> loadFromFile( Log log, File file )
{
if ( file == null )
if ( !file.exists() )
{
log.warn( "Config file [%s] does not exist.", file );
return new HashMap<>();
}

if ( file.exists() )
try
{
try
{
return MapUtil.load( file );
}
catch ( IOException e )
{
log.error( "Unable to load config file [%s]: %s", file, e.getMessage() );
}
return MapUtil.load( file );
}
else
catch ( IOException e )
{
log.warn( "Config file [%s] does not exist.", file );
log.error( "Unable to load config file [%s]: %s", file, e.getMessage() );
return new HashMap<>();
}

// Default to no user-defined config if no config was found
return new HashMap<>();
}

public interface SettingsClasses
Expand Down
5 changes: 3 additions & 2 deletions community/server/src/test/java/org/neo4j/server/BlockingBootstrapperTest.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.server;

import java.io.File;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -46,7 +47,7 @@ public void shouldBlockUntilStoppedIfTheWrappedStartIsSuccessful()
{
@SafeVarargs
@Override
public final int start( File configFile, Pair<String, String>... configOverrides )
public final int start( Optional<File> configFile, Pair<String, String>... configOverrides )
{
running.set( true );
return 0;
Expand Down Expand Up @@ -85,7 +86,7 @@ public void shouldNotBlockIfTheWrappedStartIsUnsuccessful()
{
@SafeVarargs
@Override
public final int start( File configFile, Pair<String, String>... configOverrides )
public final int start( Optional<File> configFile, Pair<String, String>... configOverrides )
{
return 1;
}
Expand Down
5 changes: 3 additions & 2 deletions community/server/src/test/java/org/neo4j/server/ServerCommandLineArgsTest.java
View file
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.server;

import java.io.File;
import java.util.Optional;

import org.junit.Test;

Expand All @@ -36,15 +37,15 @@ public class ServerCommandLineArgsTest
@Test
public void shouldPickUpSpecifiedConfigFile() throws Exception
{
File expectedFile = new File( "some-dir/" + ConfigLoader.DEFAULT_CONFIG_FILE_NAME );
Optional<File> expectedFile = Optional.of( new File( "some-dir/" + ConfigLoader.DEFAULT_CONFIG_FILE_NAME ) );
assertEquals( expectedFile, parse( "--config-dir", "some-dir" ).configFile() );
assertEquals( expectedFile, parse( "--config-dir=some-dir" ).configFile() );
}

@Test
public void shouldReturnNullIfConfigDirIsNotSpecified()
{
assertEquals( null, parse().configFile() );
assertEquals( Optional.empty(), parse().configFile() );
}

@Test
Expand Down
5 changes: 3 additions & 2 deletions community/server/src/test/java/org/neo4j/server/configuration/ConfigFileBuilder.java
View file
Expand Up @@ -23,6 +23,7 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.Map;
import java.util.Optional;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
import org.neo4j.helpers.collection.MapUtil;
Expand Down Expand Up @@ -55,7 +56,7 @@ private ConfigFileBuilder( File directory )
this.directory = directory;
}

public File build() throws IOException
public Optional<File> build() throws IOException
{
File file = new File( directory, "config" );
Map<String, String> config = MapUtil.stringMap(
Expand All @@ -65,7 +66,7 @@ public File build() throws IOException
for ( Tuple t : nameValuePairs )
config.put( t.name, t.value );
ServerTestUtils.writeConfigToFile( config, file );
return file;
return Optional.of( file );
}

public ConfigFileBuilder withNameValue( String name, String value )
Expand Down
13 changes: 7 additions & 6 deletions community/server/src/test/java/org/neo4j/server/configuration/ConfigLoaderTest.java
View file
Expand Up @@ -24,6 +24,7 @@
import java.io.FileWriter;
import java.io.IOException;
import java.util.List;
import java.util.Optional;

import org.junit.Rule;
import org.junit.Test;
Expand Down Expand Up @@ -58,7 +59,7 @@ public class ConfigLoaderTest
public void shouldProvideAConfiguration() throws IOException
{
// given
File configFile = ConfigFileBuilder.builder( folder.getRoot() )
Optional<File> configFile = ConfigFileBuilder.builder( folder.getRoot() )
.build();

// when
Expand All @@ -72,7 +73,7 @@ public void shouldProvideAConfiguration() throws IOException
public void shouldUseSpecifiedConfigFile() throws Exception
{
// given
File configFile = ConfigFileBuilder.builder( folder.getRoot() )
Optional<File> configFile = ConfigFileBuilder.builder( folder.getRoot() )
.withNameValue( "foo", "bar" )
.build();

Expand All @@ -88,7 +89,7 @@ public void shouldUseSpecifiedConfigFile() throws Exception
public void shouldAcceptDuplicateKeysWithSameValue() throws IOException
{
// given
File configFile = ConfigFileBuilder.builder( folder.getRoot() )
Optional<File> configFile = ConfigFileBuilder.builder( folder.getRoot() )
.withNameValue( "foo", "bar" )
.withNameValue( "foo", "bar" )
.build();
Expand Down Expand Up @@ -119,7 +120,7 @@ public void shouldFindThirdPartyJaxRsPackages() throws IOException
}

// when
Config config = configLoader.loadConfig( file, log );
Config config = configLoader.loadConfig( Optional.of( file ), log );

// then
List<ThirdPartyJaxRsPackage> thirdpartyJaxRsPackages = config.get( ServerSettings.third_party_packages );
Expand All @@ -131,7 +132,7 @@ public void shouldFindThirdPartyJaxRsPackages() throws IOException
public void shouldRetainRegistrationOrderOfThirdPartyJaxRsPackages() throws IOException
{
// given
File configFile = ConfigFileBuilder.builder( folder.getRoot() )
Optional<File> configFile = ConfigFileBuilder.builder( folder.getRoot() )
.withNameValue( ServerSettings.third_party_packages.name(),
"org.neo4j.extension.extension1=/extension1,org.neo4j.extension.extension2=/extension2," +
"org.neo4j.extension.extension3=/extension3" )
Expand All @@ -154,7 +155,7 @@ public void shouldRetainRegistrationOrderOfThirdPartyJaxRsPackages() throws IOEx
public void shouldWorkFineWhenSpecifiedConfigFileDoesNotExist()
{
// Given
File nonExistentConfigFile = new File( "/tmp/" + System.currentTimeMillis() );
Optional<File> nonExistentConfigFile = Optional.of( new File( "/tmp/" + System.currentTimeMillis() ) );

// When
Config config = configLoader.loadConfig( nonExistentConfigFile, log );
Expand Down
22 changes: 12 additions & 10 deletions community/server/src/test/java/org/neo4j/server/helpers/CommunityServerBuilder.java
View file
Expand Up @@ -25,6 +25,7 @@
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.Properties;

import org.neo4j.dbms.DatabaseManagementSystemSettings;
Expand Down Expand Up @@ -100,28 +101,29 @@ public CommunityNeoServer build() throws IOException
{
throw new IllegalStateException( "Must specify path" );
}
final File configFile = buildBefore();
final Optional<File> configFile = buildBefore();

Log log = logProvider.getLog( getClass() );
Config config = new ConfigLoader( CommunityBootstrapper.settingsClasses ).loadConfig( configFile, log );
Config config = new ConfigLoader( CommunityBootstrapper.settingsClasses )
.loadConfig( configFile, log );
return build( configFile, config, GraphDatabaseDependencies.newDependencies().userLogProvider( logProvider )
.monitors( new Monitors() ) );
}

protected CommunityNeoServer build( File configFile, Config config,
protected CommunityNeoServer build( Optional<File> configFile, Config config,
GraphDatabaseFacadeFactory.Dependencies dependencies )
{
return new TestCommunityNeoServer( config, configFile, dependencies, logProvider );
}

public File createConfigFiles() throws IOException
public Optional<File> createConfigFiles() throws IOException
{
File temporaryConfigFile = ServerTestUtils.createTempConfigFile();
File temporaryFolder = ServerTestUtils.createTempDir();

ServerTestUtils.writeConfigToFile( createConfiguration( temporaryFolder ), temporaryConfigFile );

return temporaryConfigFile;
return Optional.of( temporaryConfigFile );
}

public CommunityServerBuilder withClock( Clock clock )
Expand Down Expand Up @@ -320,9 +322,9 @@ protected DatabaseActions createDatabaseActionsObject( Database database, Config
config.get( ServerSettings.script_sandboxing_enabled ), database.getGraph() );
}

protected File buildBefore() throws IOException
protected Optional<File> buildBefore() throws IOException
{
File configFile = createConfigFiles();
Optional<File> configFile = createConfigFiles();

if ( preflightTasks == null )
{
Expand All @@ -340,9 +342,9 @@ public boolean run()

private class TestCommunityNeoServer extends CommunityNeoServer
{
private final File configFile;
private final Optional<File> configFile;

private TestCommunityNeoServer( Config config, File configFile, GraphDatabaseFacadeFactory
private TestCommunityNeoServer( Config config, Optional<File> configFile, GraphDatabaseFacadeFactory
.Dependencies dependencies, LogProvider logProvider )
{
super( config, lifecycleManagingDatabase( persistent ? COMMUNITY_FACTORY : IN_MEMORY_DB ), dependencies,
Expand All @@ -360,7 +362,7 @@ protected DatabaseActions createDatabaseActions()
public void stop()
{
super.stop();
configFile.delete();
configFile.ifPresent( File::delete );
}
}
}
6 changes: 4 additions & 2 deletions community/server/src/test/java/org/neo4j/server/integration/StartupLoggingIT.java
View file
Expand Up @@ -19,10 +19,12 @@
*/
package org.neo4j.server.integration;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.hamcrest.Description;
import org.hamcrest.Matcher;
Expand Down Expand Up @@ -64,12 +66,12 @@ public void shouldLogHelpfulStartupMessages() throws Throwable
CommunityBootstrapper boot = new CommunityBootstrapper();
Pair[] propertyPairs = getPropertyPairs();

boot.start( null, propertyPairs );
boot.start( Optional.of( new File( "nonexistent-file.conf" ) ), propertyPairs );
boot.stop();

List<String> captured = suppressOutput.getOutputVoice().lines();
assertThat( captured, containsAtLeastTheseLines(
warn( "Config file \\[conf.neo4j\\.conf\\] does not exist." ),
warn( "Config file \\[nonexistent-file.conf\\] does not exist." ),
info( "Starting..." ),
info( "Started." ),
info( "Remote interface available at http://.+:7474/" ),
Expand Down

0 comments on commit f01cc62

Please sign in to comment.