Simplifies DatabaseRule settings

Previously DatabaseRule had these ways of specifying settings:
- withSetting
- withConfiguration
- setConfig
- ensureStarted(String... additionalConfig)
- restartDatabase(String... additionalConfig)
- configure (override method)

each doing its own thing with settings and where they were applied
especially withSetting/withConfiguration/setConfig were sensitive
to where they were called. If a specific test would call withSetting
instead of setConfig it would have had no effect and vice versa.

This has been simplified to:
- withSetting/withSettings
- restartDatabase(String... additionalConfig)

withSetting/withSettings can now be called either at rule construction,
where those settings will be considered global to all tests in the class
or be called in test before db is started, where the setting(s) will
be considered local to the test.
restartDatabase is a convenience for shutdown,withSetting...,start so
it's still a nice way to add settings during a restart.

community/community-it/cypher-it/src/test/java/org/neo4j/cypher/CreateIndexStressIT.java

@@ -30,8 +30,8 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
+import org.neo4j.test.rule.DatabaseRule;
 import org.neo4j.test.rule.ImpermanentDatabaseRule;
 
 import static org.junit.Assert.assertFalse;
@@ -42,14 +42,8 @@ public class CreateIndexStressIT
     private final AtomicBoolean hasFailed = new AtomicBoolean( false );
 
     @Rule
-    public ImpermanentDatabaseRule db = new ImpermanentDatabaseRule()
-    {
-        @Override
-        protected void configure( GraphDatabaseBuilder builder )
-        {
-            builder.setConfig( GraphDatabaseSettings.query_cache_size, "0" );
-        }
-    };
+    public DatabaseRule db = new ImpermanentDatabaseRule()
+        .withSetting( GraphDatabaseSettings.query_cache_size, "0" );
 
     private final ExecutorService executorService = Executors.newFixedThreadPool( 10 );
 

community/community-it/index-it/src/test/java/org/neo4j/index/AccessExplicitIndexReadOnlyIT.java

@@ -83,7 +83,7 @@ public void shouldListAndReadExplicitIndexesForReadOnlyDb() throws Exception
     public void shouldNotCreateIndexesForReadOnlyDb()
     {
         // given
-        db.ensureStarted( GraphDatabaseSettings.read_only.name(), TRUE.toString() );
+        db.withSetting( GraphDatabaseSettings.read_only, TRUE.toString() );
 
         // when
         try ( Transaction tx = db.beginTx() )

community/community-it/index-it/src/test/java/org/neo4j/index/IndexFailureOnStartupTest.java

@@ -112,7 +112,7 @@ public void shouldNotBeAbleToViolateConstraintWhenBackingIndexFailsToOpen() thro
     public void shouldArchiveFailedIndex() throws Exception
     {
         // given
-        db.setConfig( GraphDatabaseSettings.archive_failed_index, "true" );
+        db.withSetting( GraphDatabaseSettings.archive_failed_index, "true" );
         try ( Transaction tx = db.beginTx() )
         {
             Node node = db.createNode( PERSON );

community/community-it/it-test-support/src/main/java/org/neo4j/test/rule/DatabaseRule.java

@@ -70,12 +70,12 @@ public abstract class DatabaseRule extends ExternalResource implements GraphData
     private DatabaseLayout databaseLayout;
     private Supplier<Statement> statementSupplier;
     private boolean startEagerly = true;
-    private Map<Setting<?>, String> config;
+    private final Map<Setting<?>, String> globalConfig = new HashMap<>();
     private final Monitors monitors = new Monitors();
 
     /**
      * Means the database will be started on first {@link #getGraphDatabaseAPI()}}
-     * or {@link #ensureStarted(String...)} call.
+     * or {@link #ensureStarted()} call.
      */
     public DatabaseRule startLazily()
     {
@@ -274,7 +274,7 @@ private void create()
             factory.setMonitors( monitors );
             configure( factory );
             databaseBuilder = newBuilder( factory );
-            configure( databaseBuilder );
+            globalConfig.forEach( databaseBuilder::setConfig );
         }
         catch ( RuntimeException e )
         {
@@ -308,57 +308,66 @@ protected void configure( GraphDatabaseFactory databaseFactory )
         // Override to configure the database factory
     }
 
-    protected void configure( GraphDatabaseBuilder builder )
-    {
-        // Override to configure the database
-
-        // Adjusted defaults for testing
-        applyConfigChanges( builder );
-    }
-
-    public GraphDatabaseBuilder setConfig( Setting<?> setting, String value )
-    {
-        return databaseBuilder.setConfig( setting, value );
-    }
-
     /**
      * {@link DatabaseRule} now implements {@link GraphDatabaseAPI} directly, so no need. Also for ensuring
-     * a lazily started database is created, use {@link #ensureStarted( String... )} instead.
+     * a lazily started database is created, use {@link #ensureStarted()} instead.
      */
     public GraphDatabaseAPI getGraphDatabaseAPI()
     {
         ensureStarted();
         return database;
     }
 
-    public synchronized void ensureStarted( String... additionalConfig )
+    public synchronized void ensureStarted()
     {
         if ( database == null )
         {
-            applyConfigChanges( databaseBuilder, additionalConfig );
             database = (GraphDatabaseAPI) databaseBuilder.newGraphDatabase();
             databaseLayout = database.databaseLayout();
             statementSupplier = resolveDependency( ThreadToStatementContextBridge.class );
         }
     }
 
+    /**
+     * Adds or replaces a setting for the database managed by this database rule.
+     * <p>
+     * If this method is called when constructing the rule, the setting is considered a global setting applied to all tests.
+     * <p>
+     * If this method is called inside a specific test, i.e. after {@link #before()}, but before started (a call to {@link #startLazily()} have been made),
+     * then this setting will be considered a test-specific setting, adding to or overriding the global settings for this test only.
+     * Test-specific settings will be remembered throughout a test, even between restarts.
+     * <p>
+     * If this method is called when a database is already started an {@link IllegalStateException} will be thrown since the setting
+     * will have no effect, instead letting the developer notice that and change the test code.
+     */
     public DatabaseRule withSetting( Setting<?> key, String value )
     {
-        if ( this.config == null )
+        if ( database != null )
         {
-            this.config = new HashMap<>();
+            // Database already started
+            throw new IllegalStateException( "Wanted to set " + key + "=" + value + ", but database has already been started" );
+        }
+        if ( databaseBuilder != null )
+        {
+            // Test already started, but db not yet started
+            databaseBuilder.setConfig( key, value );
+        }
+        else
+        {
+            // Test haven't started, we're still in phase of constructing this rule
+            globalConfig.put( key, value );
         }
-        this.config.put( key, value );
         return this;
     }
 
-    public DatabaseRule withConfiguration( Map<Setting<?>,String> configuration )
+    /**
+     * Applies all settings in the settings map.
+     *
+     * @see #withSetting(Setting, String)
+     */
+    public DatabaseRule withSettings( Map<Setting<?>,String> configuration )
     {
-        if ( this.config == null )
-        {
-            this.config = new HashMap<>();
-        }
-        this.config.putAll( configuration );
+        configuration.forEach( this::withSetting );
         return this;
     }
 
@@ -383,19 +392,12 @@ public GraphDatabaseAPI restartDatabase( RestartAction action, String... configC
         database.shutdown();
         action.run( fs, databaseLayout );
         database = null;
-        applyConfigChanges( databaseBuilder, configChanges );
+        // This DatabaseBuilder has already been configured with the global settings as well as any test-specific settings,
+        // so just apply these additional settings.
+        databaseBuilder.setConfig( stringMap( configChanges ) );
         return getGraphDatabaseAPI();
     }
 
-    private void applyConfigChanges( GraphDatabaseBuilder builder, String... configChanges )
-    {
-        if ( config != null )
-        {
-            config.forEach( builder::setConfig );
-        }
-        builder.setConfig( stringMap( configChanges ) );
-    }
-
     @Override
     public void shutdown()
     {

enterprise/backup/src/test/java/org/neo4j/backup/impl/BackupProtocolServiceIT.java

@@ -236,7 +236,7 @@ public void shouldPrintThatIncrementalBackupIsPerformedAndFallingBackToFull() th
     {
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
-        dbRule.setConfig( GraphDatabaseSettings.keep_logical_logs, "false" );
+        dbRule.withSetting( GraphDatabaseSettings.keep_logical_logs, "false" );
         // have logs rotated on every transaction
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
         createSchemaIndex( db );
@@ -676,7 +676,7 @@ public void shouldGiveHelpfulErrorMessageIfLogsPrunedPastThePointOfNoReturn() th
         // Given
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
-        dbRule.setConfig( GraphDatabaseSettings.keep_logical_logs, "false" );
+        dbRule.withSetting( GraphDatabaseSettings.keep_logical_logs, "false" );
         // have logs rotated on every transaction
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
         createSchemaIndex( db );
@@ -719,7 +719,7 @@ public void shouldFallbackToFullBackupIfIncrementalFailsAndExplicitlyAskedToDoTh
         // Given
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
-        dbRule.setConfig( GraphDatabaseSettings.keep_logical_logs, "false" );
+        dbRule.withSetting( GraphDatabaseSettings.keep_logical_logs, "false" );
         // have logs rotated on every transaction
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
         createSchemaIndex( db );
@@ -763,7 +763,7 @@ public void shouldHandleBackupWhenLogFilesHaveBeenDeleted() throws Exception
         // Given
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
-        dbRule.setConfig( GraphDatabaseSettings.keep_logical_logs, "false" );
+        dbRule.withSetting( GraphDatabaseSettings.keep_logical_logs, "false" );
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
         createSchemaIndex( db );
         BackupProtocolService backupProtocolService = backupService();
@@ -841,7 +841,7 @@ public void shouldDoFullBackupOnIncrementalFallbackToFullIfNoBackupFolderExists(
         // Given
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
-        dbRule.setConfig( GraphDatabaseSettings.keep_logical_logs, "false" );
+        dbRule.withSetting( GraphDatabaseSettings.keep_logical_logs, "false" );
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
         createSchemaIndex( db );
         BackupProtocolService backupProtocolService = backupService();
@@ -865,7 +865,7 @@ public void shouldContainTransactionsThatHappenDuringBackupProcess() throws Thro
         defaultBackupPortHostParams();
         Config defaultConfig = Config.defaults();
         defaultConfig.augment( OnlineBackupSettings.online_backup_server, BACKUP_HOST + ":" + backupPort );
-        dbRule.setConfig( OnlineBackupSettings.online_backup_enabled, "false" );
+        dbRule.withSetting( OnlineBackupSettings.online_backup_enabled, "false" );
         Config withOnlineBackupDisabled = Config.defaults();
 
         final Barrier.Control barrier = new Barrier.Control();
@@ -925,7 +925,7 @@ public void backupsShouldBeMentionedInServerConsoleLog() throws Throwable
         defaultBackupPortHostParams();
         Config config = Config.defaults();
         config.augment( OnlineBackupSettings.online_backup_server, BACKUP_HOST + ":" + backupPort );
-        dbRule.setConfig( OnlineBackupSettings.online_backup_enabled, "false" );
+        dbRule.withSetting( OnlineBackupSettings.online_backup_enabled, "false" );
         Config withOnlineBackupDisabled = Config.defaults();
         createAndIndexNode( dbRule, 1 );
 
@@ -1025,7 +1025,7 @@ public void incrementalBackupShouldFailWhenTargetDirContainsDifferentStore() thr
 
     private void defaultBackupPortHostParams()
     {
-        dbRule.setConfig( OnlineBackupSettings.online_backup_server, BACKUP_HOST + ":" + backupPort );
+        dbRule.withSetting( OnlineBackupSettings.online_backup_server, BACKUP_HOST + ":" + backupPort );
     }
 
     private static void createSchemaIndex( GraphDatabaseService db )

enterprise/backup/src/test/java/org/neo4j/backup/impl/OnlineBackupCommandHaIT.java

@@ -452,9 +452,9 @@ private void startDb( Integer backupPort )
 
     private void startDb( EmbeddedDatabaseRule db, Integer backupPort )
     {
-        db.setConfig( GraphDatabaseSettings.record_format, recordFormat );
-        db.setConfig( OnlineBackupSettings.online_backup_enabled, Settings.TRUE );
-        db.setConfig( OnlineBackupSettings.online_backup_server, "127.0.0.1" + ":" + backupPort );
+        db.withSetting( GraphDatabaseSettings.record_format, recordFormat );
+        db.withSetting( OnlineBackupSettings.online_backup_enabled, Settings.TRUE );
+        db.withSetting( OnlineBackupSettings.online_backup_server, "127.0.0.1" + ":" + backupPort );
         db.ensureStarted();
         createSomeData( db );
     }

enterprise/deferred-locks/src/test/java/org/neo4j/kernel/impl/locking/DeferringLocksIT.java

@@ -71,7 +71,7 @@ public class DeferringLocksIT
     @Before
     public void initDb()
     {
-        dbRule.setConfig( DeferringStatementLocksFactory.deferred_locks_enabled, Settings.TRUE );
+        dbRule.withSetting( DeferringStatementLocksFactory.deferred_locks_enabled, Settings.TRUE );
         db = dbRule.getGraphDatabaseAPI();
     }
 

enterprise/kernel/src/test/java/org/neo4j/kernel/enterprise/builtinprocs/ListQueriesProcedureTest.java

@@ -415,7 +415,7 @@ public void shouldDisableCpuTimeTracking() throws Exception
     {
         // given
         String query = "MATCH (n) SET n.v = n.v + 1";
-        db.setConfig( track_query_cpu_time, FALSE );
+        db.withSetting( track_query_cpu_time, FALSE );
         Map<String,Object> data;
 
         // when
@@ -467,7 +467,7 @@ public void shouldDisableHeapAllocationTracking() throws Exception
     {
         // given
         String query = "MATCH (n) SET n.v = n.v + 1";
-        db.setConfig( track_query_allocation, FALSE );
+        db.withSetting( track_query_allocation, FALSE );
         Map<String,Object> data;
 
         // when

enterprise/kernel/src/test/java/org/neo4j/kernel/impl/enterprise/lock/forseti/ForsetiServiceLoadingTest.java

@@ -53,7 +53,7 @@ public void shouldUseForsetiAsDefaultLockManager()
     public void shouldAllowUsingCommunityLockManager()
     {
         // When
-        dbRule.setConfig( GraphDatabaseSettings.lock_manager, "community" );
+        dbRule.withSetting( GraphDatabaseSettings.lock_manager, "community" );
         GraphDatabaseAPI db = dbRule.getGraphDatabaseAPI();
 
         // Then

enterprise/kernel/src/test/java/org/neo4j/locking/MergeLockConcurrencyTest.java

@@ -67,7 +67,7 @@ public static Iterable<Object[]> configurations()
 
     public MergeLockConcurrencyTest( ConfigBuilder config )
     {
-        db.withConfiguration( config.configuration() );
+        db.withSettings( config.configuration() );
     }
 
     @Test

enterprise/neo4j-enterprise/src/test/java/org/neo4j/consistency/ConsistencyCheckServiceRecordFormatIT.java

@@ -77,7 +77,7 @@ public static List<String> recordFormats()
     @Before
     public void configureRecordFormat()
     {
-        db.setConfig( GraphDatabaseSettings.record_format, recordFormat );
+        db.withSetting( GraphDatabaseSettings.record_format, recordFormat );
     }
 
     @Test

enterprise/query-logging/src/test/java/org/neo4j/kernel/impl/query/ShellQueryLoggingIT.java

@@ -38,7 +38,6 @@
 
 import org.neo4j.graphdb.Result;
 import org.neo4j.graphdb.Transaction;
-import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
 import org.neo4j.graphdb.factory.GraphDatabaseFactory;
 import org.neo4j.graphdb.factory.GraphDatabaseSettings;
 import org.neo4j.kernel.configuration.Settings;
@@ -71,14 +70,8 @@ protected void configure( GraphDatabaseFactory factory )
         {
             ((TestGraphDatabaseFactory) factory).setFileSystem( fs.get() );
         }
+    }.withSetting( GraphDatabaseSettings.log_queries, Settings.TRUE ).startLazily();
 
-        @Override
-        protected void configure( GraphDatabaseBuilder builder )
-        {
-            builder.setConfig( GraphDatabaseSettings.log_queries, Settings.TRUE );
-            builder.setConfig( GraphDatabaseSettings.logs_directory, logsDirectory().getPath() );
-        }
-    };
     @Rule
     public final TestRule order = outerRule( dir ).around( fs ).around( db )
             .around( ( base, description ) -> new Statement()
@@ -106,6 +99,8 @@ public void evaluate() throws Throwable
     @Before
     public void setup() throws Exception
     {
+        // Set this config a bit later since it's dependent on directory of the dbRule, which is assigned in its creation phase.
+        db.withSetting( GraphDatabaseSettings.logs_directory, logsDirectory().getPath() );
         server = new GraphDatabaseShellServer( db.getGraphDatabaseAPI() );
         SystemOutput output = new SystemOutput( new PrintWriter( out ) );
         client = ShellLobby.newClient( server, new HashMap<>(), output, action -> () ->