Skip to content

Commit

Permalink
Fix overriding of settings in Neo4jWithSocket.
Browse files Browse the repository at this point in the history 
Setting has no equals, and when adding duplicate settings
the final result could be random depending on which setting
was used. This caused flaky tests.
  • Loading branch information
@davidegrohmann
Max Sumrall authored and davidegrohmann committed Sep 14, 2016
1 parent 1aca3ed commit b371758
Show file tree
Hide file tree
Showing 28 changed files with 239 additions and 214 deletions.
19 changes: 12 additions & 7 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/runtime/integration/BoltConfigIT.java
View file
Expand Up @@ -21,10 +21,15 @@

import org.junit.Rule;
import org.junit.Test;

import org.neo4j.bolt.v1.messaging.message.InitMessage;
import org.neo4j.bolt.v1.transport.integration.Neo4jWithSocket;
import org.neo4j.bolt.v1.transport.integration.TransportTestUtil;
import org.neo4j.bolt.v1.transport.socket.client.*;
import org.neo4j.bolt.v1.transport.socket.client.SecureSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.SecureWebSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.SocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.WebSocketConnection;
import org.neo4j.helpers.HostnamePort;

import static java.util.Collections.emptyMap;
Expand All @@ -38,13 +43,13 @@ public class BoltConfigIT
{

@Rule
public Neo4jWithSocket server = new Neo4jWithSocket(
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(),
settings -> {
settings.put( boltConnector("0").enabled, "true" );
settings.put( boltConnector("0").address, "localhost:7888" );
settings.put( boltConnector("1").enabled, "true" );
settings.put( boltConnector("1").address, "localhost:7687" );
settings.put( boltConnector("1").encryption_level, REQUIRED.name() );
settings.put( boltConnector("0").enabled.name(), "true" );
settings.put( boltConnector("0").address.name(), "localhost:7888" );
settings.put( boltConnector("1").enabled.name(), "true" );
settings.put( boltConnector("1").address.name(), "localhost:7687" );
settings.put( boltConnector("1").encryption_level.name(), REQUIRED.name() );
} );

@Test
Expand Down
7 changes: 3 additions & 4 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/AuthenticationIT.java
View file
Expand Up @@ -41,7 +41,6 @@
import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.WebSocketConnection;
import org.neo4j.function.Factory;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.HostnamePort;
import org.neo4j.kernel.api.exceptions.Status;
Expand All @@ -60,16 +59,16 @@
public class AuthenticationIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket( getTestGraphDatabaseFactory(), getSettingsFunction() );
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(), getTestGraphDatabaseFactory(), getSettingsFunction() );

protected TestGraphDatabaseFactory getTestGraphDatabaseFactory()
{
return new TestGraphDatabaseFactory();
}

protected Consumer<Map<Setting<?>, String>> getSettingsFunction()
protected Consumer<Map<String, String>> getSettingsFunction()
{
return settings -> settings.put( GraphDatabaseSettings.auth_enabled, "true" );
return settings -> settings.put( GraphDatabaseSettings.auth_enabled.name(), "true" );
}

@Parameterized.Parameter( 0 )
Expand Down
6 changes: 3 additions & 3 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/CertificatesIT.java
View file
Expand Up @@ -49,9 +49,9 @@ public class CertificatesIT
private static Certificates certFactory;

@Rule
public Neo4jWithSocket server = new Neo4jWithSocket( settings -> {
settings.put( tls_certificate_file, certFile.getAbsolutePath() );
settings.put( tls_key_file, keyFile.getAbsolutePath() );
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(), settings -> {
settings.put( tls_certificate_file.name(), certFile.getAbsolutePath() );
settings.put( tls_key_file.name(), keyFile.getAbsolutePath() );
} );

@Test
Expand Down
4 changes: 2 additions & 2 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/ConcurrentAccessIT.java
View file
Expand Up @@ -64,8 +64,8 @@
public class ConcurrentAccessIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket( settings ->
settings.put( GraphDatabaseSettings.auth_enabled, "false" ) );
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(), settings ->
settings.put( GraphDatabaseSettings.auth_enabled.name(), "false" ) );

@Parameterized.Parameter( 0 )
public Factory<TransportConnection> cf;
Expand Down
4 changes: 2 additions & 2 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/ConnectionIT.java
View file
Expand Up @@ -29,10 +29,10 @@
import java.io.IOException;
import java.util.Collection;

import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.SecureSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.SecureWebSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.SocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.WebSocketConnection;
import org.neo4j.helpers.HostnamePort;

Expand All @@ -45,7 +45,7 @@ public class ConnectionIT
public ExpectedException exception = ExpectedException.none();

@Rule
public Neo4jWithSocket server = new Neo4jWithSocket();
public Neo4jWithSocket server = new Neo4jWithSocket( getClass() );

@Parameterized.Parameter(0)
public TransportConnection connection;
Expand Down
96 changes: 57 additions & 39 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/Neo4jWithSocket.java
View file
Expand Up @@ -19,69 +19,77 @@
*/
package org.neo4j.bolt.v1.transport.integration;

import org.junit.rules.TestRule;
import org.junit.rules.ExternalResource;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Supplier;

import org.neo4j.bolt.BoltKernelExtension;
import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.test.TestGraphDatabaseFactory;
import org.neo4j.test.rule.TestDirectory;

import static org.neo4j.graphdb.factory.GraphDatabaseSettings.BoltConnector.EncryptionLevel.OPTIONAL;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.boltConnector;

public class Neo4jWithSocket implements TestRule
public class Neo4jWithSocket extends ExternalResource
{
private Supplier<FileSystemAbstraction> fileSystemProvider;
private final Consumer<Map<Setting<?>,String>> configure;
private final Consumer<Map<String,String>> configure;
private final TestDirectory testDirectory;
private TestGraphDatabaseFactory graphDatabaseFactory;
private GraphDatabaseService gdb;
private File workingDirectory;

public Neo4jWithSocket()
public Neo4jWithSocket( Class<?> testClass )
{
this( new TestGraphDatabaseFactory(), EphemeralFileSystemAbstraction::new, settings -> {} );
this( testClass, settings ->
{
} );
}

public Neo4jWithSocket( Consumer<Map<Setting<?>, String>> configure )
public Neo4jWithSocket( Class<?> testClass, Consumer<Map<String,String>> configure )
{
this( new TestGraphDatabaseFactory(), EphemeralFileSystemAbstraction::new, configure );
this( testClass, new TestGraphDatabaseFactory(), configure );
}

public Neo4jWithSocket( TestGraphDatabaseFactory graphDatabaseFactory, Consumer<Map<Setting<?>, String>> configure )
public Neo4jWithSocket( Class<?> testClass, TestGraphDatabaseFactory graphDatabaseFactory,
Consumer<Map<String,String>> configure )
{
this( graphDatabaseFactory, EphemeralFileSystemAbstraction::new, configure );
this( testClass, graphDatabaseFactory, EphemeralFileSystemAbstraction::new, configure );
}

public Neo4jWithSocket( TestGraphDatabaseFactory graphDatabaseFactory,
Supplier<FileSystemAbstraction> fileSystemProvider,
Consumer<Map<Setting<?>, String>> configure )
public Neo4jWithSocket( Class<?> testClass, TestGraphDatabaseFactory graphDatabaseFactory,
Supplier<FileSystemAbstraction> fileSystemProvider, Consumer<Map<String,String>> configure )
{
this.testDirectory = TestDirectory.testDirectory( testClass, fileSystemProvider.get() );
this.graphDatabaseFactory = graphDatabaseFactory;
this.fileSystemProvider = fileSystemProvider;
this.configure = configure;
}

@Override
public Statement apply( final Statement statement, Description description )
public Statement apply( final Statement statement, final Description description )
{
return new Statement()
Statement testMethod = new Statement()
{
@Override
public void evaluate() throws Throwable
{
restartDatabase( settings -> {} );
// If this is used as class rule then getMethodName() returns null, so use
// getClassName() instead.
String name =
description.getMethodName() != null ? description.getMethodName() : description.getClassName();
workingDirectory = testDirectory.directory( name );
ensureDatabase( settings -> {} );
try
{
statement.evaluate();
Expand All @@ -92,43 +100,53 @@ public void evaluate() throws Throwable
}
}
};

Statement testMethodWithBeforeAndAfter = super.apply( testMethod, description );

return testDirectory.apply( testMethodWithBeforeAndAfter, description );
}

private void shutdownDatabase()
public void shutdownDatabase()
{
gdb.shutdown();
gdb = null;
try
{
gdb.shutdown();
}
finally
{
gdb = null;
}
}

public void restartDatabase( Consumer<Map<Setting<?>, String>> overrideSettingsFunction ) throws IOException
public void ensureDatabase( Consumer<Map<String,String>> overrideSettingsFunction ) throws IOException
{
if ( gdb != null )
{
gdb.shutdown();
return;
}
Map<Setting<?>,String> settings = configure( overrideSettingsFunction );

Map<String,String> settings = configure( overrideSettingsFunction );
File storeDir = new File( workingDirectory, "storeDir" );
graphDatabaseFactory.setFileSystem( fileSystemProvider.get() );
gdb = graphDatabaseFactory.newImpermanentDatabase( settings );
gdb = graphDatabaseFactory.newImpermanentDatabaseBuilder( storeDir ).
setConfig( settings ).newGraphDatabase();
}

public Map<Setting<?>,String> configure( Consumer<Map<Setting<?>, String>> overrideSettingsFunction ) throws IOException
private Map<String,String> configure( Consumer<Map<String,String>> overrideSettingsFunction ) throws IOException
{
Map<Setting<?>, String> settings = new HashMap<>();
settings.put( boltConnector( "0" ).enabled, "true" );
settings.put( boltConnector( "0" ).encryption_level, OPTIONAL.name() );
settings.put( BoltKernelExtension.Settings.tls_key_file, tempPath( "key", ".key" ) );
settings.put( BoltKernelExtension.Settings.tls_certificate_file, tempPath( "cert", ".cert" ) );
configure.andThen( overrideSettingsFunction ).accept( settings );
Map<String,String> settings = new HashMap<>();
settings.put( boltConnector( "0" ).enabled.name(), "true" );
settings.put( boltConnector( "0" ).encryption_level.name(), OPTIONAL.name() );
settings.put( BoltKernelExtension.Settings.tls_key_file.name(), tempPath( "key.key" ) );
settings.put( BoltKernelExtension.Settings.tls_certificate_file.name(), tempPath( "cert.cert" ) );
configure.accept( settings );
overrideSettingsFunction.accept( settings );
return settings;
}

private String tempPath(String prefix, String suffix ) throws IOException
private String tempPath( String filename ) throws IOException
{
Path path = Files.createTempFile( prefix, suffix );
// We don't want an existing file just the path to a temporary file
// a little silly to do it this way
Files.delete( path );
return path.toString();
return new File( new File( workingDirectory, "security" ), filename ).getAbsolutePath();
}

public GraphDatabaseService graphDatabaseService()
Expand Down
7 changes: 3 additions & 4 deletions ...lt/src/test/java/org/neo4j/bolt/v1/transport/integration/RejectTransportEncryptionIT.java
View file
Expand Up @@ -30,9 +30,9 @@
import java.io.IOException;
import java.util.Collection;

import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.SecureSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.SecureWebSocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.function.Factory;
import org.neo4j.helpers.HostnamePort;

Expand All @@ -44,8 +44,8 @@
public class RejectTransportEncryptionIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket(
settings -> settings.put( boltConnector( "0" ).encryption_level, DISABLED.name() ) );
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(),
settings -> settings.put( boltConnector( "0" ).encryption_level.name(), DISABLED.name() ) );
@Rule
public ExpectedException exception = ExpectedException.none();

Expand Down Expand Up @@ -94,7 +94,6 @@ public void shouldRejectConnectionAfterHandshake() throws Throwable
{
exception.expect( expected.getClass() );
exception.expectMessage( expected.getMessage() );

client.connect( address ).send( TransportTestUtil.acceptedVersions( 1, 0, 0, 0 ) );
}
}
9 changes: 5 additions & 4 deletions .../src/test/java/org/neo4j/bolt/v1/transport/integration/RequiredTransportEncryptionIT.java
View file
Expand Up @@ -25,6 +25,9 @@
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Collection;

import org.neo4j.bolt.v1.transport.socket.client.SocketConnection;
import org.neo4j.bolt.v1.transport.socket.client.TransportConnection;
import org.neo4j.bolt.v1.transport.socket.client.WebSocketConnection;
Expand All @@ -33,8 +36,6 @@
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.helpers.HostnamePort;

import java.util.Collection;

import static java.util.Arrays.asList;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.BoltConnector.EncryptionLevel.REQUIRED;
Expand All @@ -44,11 +45,11 @@
public class RequiredTransportEncryptionIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket(
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(),
settings -> {
Setting<GraphDatabaseSettings.BoltConnector.EncryptionLevel> encryption_level =
boltConnector( "0" ).encryption_level;
settings.put( encryption_level, REQUIRED.name() );
settings.put( encryption_level.name(), REQUIRED.name() );
} );

@Parameterized.Parameter( 0 )
Expand Down
2 changes: 1 addition & 1 deletion community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/TransportErrorIT.java
View file
Expand Up @@ -49,7 +49,7 @@
public class TransportErrorIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket();
public Neo4jWithSocket server = new Neo4jWithSocket( getClass() );

@Parameterized.Parameter(0)
public Factory<TransportConnection> cf;
Expand Down
4 changes: 2 additions & 2 deletions community/bolt/src/test/java/org/neo4j/bolt/v1/transport/integration/TransportSessionIT.java
View file
Expand Up @@ -71,8 +71,8 @@
public class TransportSessionIT
{
@Rule
public Neo4jWithSocket server = new Neo4jWithSocket(settings ->
settings.put( GraphDatabaseSettings.auth_enabled, "false" ));
public Neo4jWithSocket server = new Neo4jWithSocket( getClass(), settings ->
settings.put( GraphDatabaseSettings.auth_enabled.name(), "false" ));

@Parameterized.Parameter(0)
public Factory<TransportConnection> cf;
Expand Down

0 comments on commit b371758

Please sign in to comment.