Skip to content

Commit

Permalink
updateDynamicSetting now logs all changes
Browse files Browse the repository at this point in the history
  • Loading branch information
klaren committed Oct 11, 2017
1 parent f55c599 commit 42c9d40
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 6 deletions.
Expand Up @@ -571,11 +571,11 @@ public Optional<?> getValue( @Nonnull String key )
* @implNote No migration or config validation is done. If you need this you have to refactor this method. * @implNote No migration or config validation is done. If you need this you have to refactor this method.
* *
* @param setting The setting to set to the specified value. * @param setting The setting to set to the specified value.
* @param value The new value to set, passing {@code null} or empty should reset the value back to default value. * @param newValue The new value to set, passing {@code null} or empty should reset the value back to default value.
* @throws IllegalArgumentException if the provided setting is unknown or not dynamic. * @throws IllegalArgumentException if the provided setting is unknown or not dynamic.
* @throws InvalidSettingException if the value is not formatted correctly. * @throws InvalidSettingException if the value is not formatted correctly.
*/ */
public void updateDynamicSetting( String setting, String value ) throws IllegalArgumentException, InvalidSettingException public void updateDynamicSetting( String setting, String newValue ) throws IllegalArgumentException, InvalidSettingException
{ {
// Make sure the setting is valid and is marked as dynamic // Make sure the setting is valid and is marked as dynamic
Optional<ConfigValue> option = findConfigValue( setting ); Optional<ConfigValue> option = findConfigValue( setting );
Expand All @@ -591,19 +591,22 @@ public void updateDynamicSetting( String setting, String value ) throws IllegalA
throw new IllegalArgumentException( "Setting is not dynamic and can not be changed at runtime" ); throw new IllegalArgumentException( "Setting is not dynamic and can not be changed at runtime" );
} }


if ( value == null || value.isEmpty() ) String oldValue;

if ( newValue == null || newValue.isEmpty() )
{ {
// Empty means we want to delete the configured value and fallback to the default value // Empty means we want to delete the configured value and fallback to the default value
params.remove( setting ); oldValue = params.remove( setting );
if ( overriddenDefaults.containsKey( setting ) ) if ( overriddenDefaults.containsKey( setting ) )
{ {
params.put( setting, overriddenDefaults.get( setting ) ); params.put( setting, overriddenDefaults.get( setting ) );
} }
newValue = "<default value>";
} }
else else
{ {
// Change setting, make sure it's valid // Change setting, make sure it's valid
Map<String,String> newEntry = stringMap( setting, value ); Map<String,String> newEntry = stringMap( setting, newValue );
List<SettingValidator> settingValidators = configOptions.stream() List<SettingValidator> settingValidators = configOptions.stream()
.map( ConfigOptions::settingGroup ) .map( ConfigOptions::settingGroup )
.collect( Collectors.toList() ); .collect( Collectors.toList() );
Expand All @@ -612,8 +615,13 @@ public void updateDynamicSetting( String setting, String value ) throws IllegalA
validator.validate( newEntry, ignore -> {} ); // Throws if invalid validator.validate( newEntry, ignore -> {} ); // Throws if invalid
} }


params.put( setting, value ); oldValue = params.put( setting, newValue );
if ( oldValue == null )
{
oldValue = "<default value>";
}
} }
log.info( "Setting changed: '%s' changed from '%s' to '%s'", setting, oldValue, newValue );
} }


private Optional<ConfigValue> findConfigValue( String setting ) private Optional<ConfigValue> findConfigValue( String setting )
Expand Down
Expand Up @@ -33,6 +33,7 @@
import javax.annotation.Nonnull; import javax.annotation.Nonnull;


import org.neo4j.configuration.DocumentedDefaultValue; import org.neo4j.configuration.DocumentedDefaultValue;
import org.neo4j.configuration.Dynamic;
import org.neo4j.configuration.Internal; import org.neo4j.configuration.Internal;
import org.neo4j.configuration.LoadableConfig; import org.neo4j.configuration.LoadableConfig;
import org.neo4j.configuration.ReplacedBy; import org.neo4j.configuration.ReplacedBy;
Expand Down Expand Up @@ -353,4 +354,28 @@ public void augmentDefaults() throws Exception
config.augmentDefaults( MySettingsWithDefaults.hello, "new default" ); config.augmentDefaults( MySettingsWithDefaults.hello, "new default" );
assertEquals( "new default", config.get( MySettingsWithDefaults.hello ) ); assertEquals( "new default", config.get( MySettingsWithDefaults.hello ) );
} }

public static class MyDynamicSettings implements LoadableConfig
{
@Dynamic
public static final Setting<Boolean> boolSetting = setting( "bool_setting", BOOLEAN, Settings.TRUE );
}

@Test
public void updateDynamicShouldLogChanges() throws Exception
{
Config config = Config.builder().withConfigClasses( Collections.singletonList( new MyDynamicSettings() ) ).build();

Log log = mock( Log.class );
config.setLogger( log );

config.updateDynamicSetting( MyDynamicSettings.boolSetting.name(), "false" );
config.updateDynamicSetting( MyDynamicSettings.boolSetting.name(), "true" );
config.updateDynamicSetting( MyDynamicSettings.boolSetting.name(), "" );

verify( log ).info("Setting changed: '%s' changed from '%s' to '%s'", MyDynamicSettings.boolSetting.name(), "<default value>", "false" );
verify( log ).info("Setting changed: '%s' changed from '%s' to '%s'", MyDynamicSettings.boolSetting.name(), "false", "true" );
verify( log ).info("Setting changed: '%s' changed from '%s' to '%s'", MyDynamicSettings.boolSetting.name(), "true", "<default value>" );
verifyNoMoreInteractions( log );
}
} }

0 comments on commit 42c9d40

Please sign in to comment.