Skip to content

Commit

Permalink
Simplify Config settings update logic
Browse files Browse the repository at this point in the history
As well as refactoring, this removes inconsistently implemented
filtering of null config values. That was originally there because
internally we use a Map which doesn't allow null values. But there is no
reason for null values to be supplied, so it's better to just fail
loudly if someone does this rather than silently ignoring them.
  • Loading branch information
benbc committed Feb 29, 2016
1 parent feb5858 commit a70ad41
Showing 1 changed file with 24 additions and 45 deletions.
Expand Up @@ -25,6 +25,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.function.Function;
Expand All @@ -42,9 +43,9 @@
/**
* This class holds the overall configuration of a Neo4j database instance. Use the accessors to convert the internal
* key-value settings to other types.
* <p>
* <p/>
* Users can assume that old settings have been migrated to their new counterparts, and that defaults have been applied.
* <p>
* <p/>
* UI's can change configuration by calling augment(). Any listener, such as services that use this configuration, can
* be notified of changes by implementing the {@link ConfigurationChangeListener} interface.
*/
Expand Down Expand Up @@ -211,60 +212,38 @@ public String toString()
return output.toString();
}

private synchronized void replaceSettings( Map<String, String> newValues )
private synchronized void replaceSettings( Map<String, String> newSettings )
{
newValues = migrator.apply( newValues, log );
HashMap<String, String> oldSettings = new HashMap<>( params );

// Make sure all changes are valid
validator.validate( newValues );
newSettings = migrator.apply( newSettings, log );
validator.validate( newSettings );
params.clear();
params.putAll( newSettings );
settingsFunction = new ConfigValues( params );

// Figure out what changed
if ( listeners.isEmpty() )
{
// Make the change
params.clear();
params.putAll( newValues );
}
else
{
List<ConfigurationChange> configurationChanges = new ArrayList<>();
for ( Map.Entry<String, String> stringStringEntry : newValues.entrySet() )
{
String oldValue = params.get( stringStringEntry.getKey() );
String newValue = stringStringEntry.getValue();
if ( !(oldValue == null && newValue == null) &&
(oldValue == null || newValue == null || !oldValue.equals( newValue )) )
{
configurationChanges.add( new ConfigurationChange( stringStringEntry.getKey(), oldValue,
newValue ) );
}
}

if ( configurationChanges.isEmpty() )
{
// Don't bother... nothing changed.
return;
}
notifyListeners( newSettings, oldSettings );
}

// Make the change
params.clear();
for ( Map.Entry<String, String> entry : newValues.entrySet() )
private void notifyListeners( Map<String, String> newSettings, HashMap<String, String> oldSettings )
{
List<ConfigurationChange> configurationChanges = new ArrayList<>();
for ( Map.Entry<String, String> setting : newSettings.entrySet() )
{
String oldValue = oldSettings.get( setting.getKey() );
String newValue = setting.getValue();
if ( !Objects.equals( oldValue, newValue ) )
{
// Filter out nulls because we are using a ConcurrentHashMap under the covers, which doesn't support
// null keys or values.
String value = entry.getValue();
if ( value != null )
{
params.put( entry.getKey(), value );
}
configurationChanges.add( new ConfigurationChange( setting.getKey(), oldValue, newValue ) );
}
}

// Notify listeners
if ( !configurationChanges.isEmpty() )
{
for ( ConfigurationChangeListener listener : listeners )
{
listener.notifyConfigurationChanges( configurationChanges );
}
}
settingsFunction = new ConfigValues( this.params );
}
}

0 comments on commit a70ad41

Please sign in to comment.