Skip to content

Commit

Permalink
Refactor config api slightly
Browse files Browse the repository at this point in the history 
* Moves information from SettingGroup-level to Setting-level
  This improves the ability to identify deprecations, internal
  settings, and similarly. Especially for connectors, which
  previously were not able to set the values for the containing
  settings.
* Adds several methods for use in config parsing located in
  BaseSetting. These values are set via annotations at startup
  when settings are loaded via ServiceLoading in LoadableConfig.

As a consequence:

* Connector-settings (like dbms.connector.http.enabled) are now
  listed in the output of dbms.listConfig procedure, with correct
  descriptions and everything.
* Connector-settings are included in the normal deprecation
  warnings.
* All settings can now be documented via the config api.
  • Loading branch information
@spacecowboy
spacecowboy committed Mar 9, 2017
1 parent fde3dfd commit f40d0cf
Show file tree
Hide file tree
Showing 22 changed files with 721 additions and 213 deletions.
50 changes: 13 additions & 37 deletions community/configuration/src/main/java/org/neo4j/configuration/ConfigOptions.java
View file
Expand Up @@ -25,6 +25,8 @@
import java.util.stream.Collectors;
import javax.annotation.Nonnull;

import org.neo4j.graphdb.config.BaseSetting;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.config.SettingGroup;

/**
Expand All @@ -33,22 +35,10 @@
public class ConfigOptions
{
private final SettingGroup<?> settingGroup;
private final Optional<String> description;
private final Optional<String> documentedDefaultValue;
private final boolean internal;
private final boolean deprecated;
private final Optional<String> replacement;

public ConfigOptions( @Nonnull SettingGroup<?> settingGroup, @Nonnull Optional<String> description,
@Nonnull Optional<String> documentedDefaultValue, boolean internal, boolean deprecated,
@Nonnull Optional<String> replacement )
public ConfigOptions( @Nonnull SettingGroup<?> settingGroup )
{
this.settingGroup = settingGroup;
this.description = description;
this.documentedDefaultValue = documentedDefaultValue;
this.internal = internal;
this.deprecated = deprecated;
this.replacement = replacement;
}

@Nonnull
Expand All @@ -57,34 +47,20 @@ public SettingGroup<?> settingGroup()
return settingGroup;
}

@Nonnull
public Optional<String> description()
{
return description;
}

@Nonnull
public Optional<String> documentedDefaultValue() {
return documentedDefaultValue;
}

@Nonnull
public List<ConfigValue> asConfigValues( @Nonnull Map<String,String> validConfig )
{
Map<String,Setting<?>> settings = settingGroup.settings( validConfig ).stream()
.collect( Collectors.toMap( Setting::name, s -> s ) );

return settingGroup.values( validConfig ).entrySet().stream()
.map( val -> new ConfigValue( val.getKey(), description(), documentedDefaultValue(),
Optional.ofNullable( val.getValue() ), internal, deprecated, replacement ) )
.map( val -> {
BaseSetting<?> setting = (BaseSetting) settings.get( val.getKey() );
return new ConfigValue( setting.name(), setting.description(),
setting.documentedDefaultValue(),
Optional.ofNullable( val.getValue() ),
setting.valueDescription(), setting.internal(),
setting.deprecated(), setting.replacement() ); } )
.collect( Collectors.toList() );
}

public boolean deprecated()
{
return deprecated;
}

@Nonnull
public Optional<String> replacement()
{
return replacement;
}
}
13 changes: 11 additions & 2 deletions community/configuration/src/main/java/org/neo4j/configuration/ConfigValue.java
View file
Expand Up @@ -31,18 +31,21 @@ public class ConfigValue
private final Optional<String> description;
private final Optional<String> documentedDefaultValue;
private final Optional<?> value;
private final String valueDescription;
private final boolean internal;
private final boolean deprecated;
private final Optional<String> replacement;

public ConfigValue( @Nonnull String name, @Nonnull Optional<String> description,
@Nonnull Optional<String> documentedDefaultValue, @Nonnull Optional<?> value,
boolean internal, boolean deprecated, @Nonnull Optional<String> replacement )
@Nonnull String valueDescription, boolean internal, boolean deprecated,
@Nonnull Optional<String> replacement )
{
this.name = name;
this.description = description;
this.documentedDefaultValue = documentedDefaultValue;
this.value = value;
this.valueDescription = valueDescription;
this.internal = internal;
this.deprecated = deprecated;
this.replacement = replacement;
Expand Down Expand Up @@ -89,8 +92,14 @@ public boolean internal()
}

@Nonnull
public Optional<String> getDocumentedDefaultValue()
public Optional<String> documentedDefaultValue()
{
return documentedDefaultValue;
}

@Nonnull
public String valueDescription()
{
return valueDescription;
}
}
34 changes: 25 additions & 9 deletions community/configuration/src/main/java/org/neo4j/configuration/ExternalSettings.java
View file
Expand Up @@ -24,6 +24,7 @@
import java.util.Map;
import java.util.function.Function;

import org.neo4j.graphdb.config.BaseSetting;
import org.neo4j.graphdb.config.Configuration;
import org.neo4j.graphdb.config.Setting;

Expand All @@ -44,31 +45,40 @@ public class ExternalSettings implements LoadableConfig
public static final Setting<String> additionalJvm = dummySetting( "dbms.jvm.additional" );

@Description( "Initial heap size. By default it is calculated based on available system resources." )
public static final Setting<String> initialHeapSize = dummySetting( "dbms.memory.heap.initial_size" );
public static final Setting<String> initialHeapSize = dummySetting( "dbms.memory.heap.initial_size",
"", "a byte size (valid units are `k`, `K`, `m`, `M`, `g`, `G`)" );

@Description( "Maximum heap size. By default it is calculated based on available system resources." )
public static final Setting<String> maxHeapSize = dummySetting( "dbms.memory.heap.max_size" );
public static final Setting<String> maxHeapSize = dummySetting( "dbms.memory.heap.max_size", "",
"a byte size (valid units are `k`, `K`, `m`, `M`, `g`, `G`)" );

static final DummySetting dummySetting( String name )
private static DummySetting dummySetting( String name )
{
return new DummySetting( name, "" );
return new DummySetting( name, "", "a string" );
}

static final DummySetting dummySetting( String name, String defVal )
private static DummySetting dummySetting( String name, String defVal)
{
return new DummySetting( name, defVal );
return new DummySetting( name, defVal, "a string" );
}

static class DummySetting implements Setting<String>
private static DummySetting dummySetting( String name, String defVal, String valDesc )
{
return new DummySetting( name, defVal, valDesc );
}

static class DummySetting extends BaseSetting<String>
{

private final String name;
private final String defaultValue;
private final String valueDescription;

public DummySetting( String name, String defVal )
public DummySetting( String name, String defVal, String valueDescription )
{
this.name = name;
this.defaultValue = defVal;
this.valueDescription = valueDescription;
}

@Override
Expand Down Expand Up @@ -102,9 +112,15 @@ public String apply( Function<String,String> provider )
}

@Override
public List<Setting> settings( Map<String,String> params )
public List<Setting<String>> settings( Map<String,String> params )
{
return Collections.singletonList( this );
}

@Override
public String valueDescription()
{
return valueDescription;
}
}
}
43 changes: 16 additions & 27 deletions community/configuration/src/main/java/org/neo4j/configuration/LoadableConfig.java
View file
Expand Up @@ -22,11 +22,11 @@
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.ServiceLoader;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import org.neo4j.graphdb.config.BaseSetting;
import org.neo4j.graphdb.config.SettingGroup;

/**
Expand All @@ -48,51 +48,40 @@ default List<ConfigOptions> getConfigOptions()
try
{
Object publicSetting = f.get( this );
if ( publicSetting instanceof SettingGroup )
if ( publicSetting instanceof BaseSetting )
{
BaseSetting setting = (BaseSetting) publicSetting;

final Description documentation = f.getAnnotation( Description.class );
final Optional<String> description;
if ( documentation == null )
{
description = Optional.empty();
}
else
if ( documentation != null )
{
description = Optional.of( documentation.value() );
setting.setDescription( documentation.value() );
}

final DocumentedDefaultValue defValue =
f.getAnnotation( DocumentedDefaultValue.class );
final Optional<String> documentedDefaultValue;
if ( defValue == null )
{
documentedDefaultValue = Optional.empty();
}
else
if ( defValue != null )
{
documentedDefaultValue = Optional.of( defValue.value() );
setting.setDocumentedDefaultValue( defValue.value() );
}

final Deprecated deprecatedAnnotation = f.getAnnotation( Deprecated.class );
final boolean deprecated = deprecatedAnnotation != null;
setting.setDeprecated( deprecatedAnnotation != null );

final ReplacedBy replacedByAnnotation = f.getAnnotation( ReplacedBy.class );
final Optional<String> replacement;
if (replacedByAnnotation == null )
if ( replacedByAnnotation != null )
{
replacement = Optional.empty();
}
else
{
replacement = Optional.of( replacedByAnnotation.value() );
setting.setReplacement( replacedByAnnotation.value() );
}

final Internal internalAnnotation = f.getAnnotation( Internal.class );
final boolean internal = internalAnnotation != null;
setting.setInternal( internalAnnotation != null );
}

configOptions.add( new ConfigOptions( (SettingGroup) publicSetting, description,
documentedDefaultValue, internal, deprecated, replacement ) );
if ( publicSetting instanceof SettingGroup )
{
SettingGroup setting = (SettingGroup) publicSetting;
configOptions.add( new ConfigOptions( setting ) );
}
}
catch ( IllegalAccessException ignored )
Expand Down
18 changes: 10 additions & 8 deletions community/configuration/src/test/java/org/neo4j/configuration/ConfigOptionsTest.java
View file
Expand Up @@ -27,14 +27,15 @@
import java.util.Optional;
import java.util.function.Function;

import org.neo4j.graphdb.config.BaseSetting;
import org.neo4j.graphdb.config.Configuration;
import org.neo4j.graphdb.config.Setting;

import static org.junit.Assert.assertEquals;

public class ConfigOptionsTest
{
private Setting<Integer> setting = new Setting<Integer>()
private Setting<Integer> setting = new BaseSetting<Integer>()
{
@Override
public String name()
Expand Down Expand Up @@ -65,13 +66,19 @@ public Integer apply( Function<String,String> provider )
{
return Integer.parseInt( provider.apply( name() ) );
}

@Override
public String valueDescription()
{
return "a special test integer";
}
};
private ConfigOptions configOptions;

@Before
public void setup()
{
this.configOptions = new ConfigOptions( setting, Optional.empty(), Optional.empty(), false, false, Optional.empty() );
this.configOptions = new ConfigOptions( setting );
}

@Test
Expand All @@ -80,12 +87,6 @@ public void setting() throws Exception
assertEquals( setting, configOptions.settingGroup() );
}

@Test
public void description() throws Exception
{
assertEquals( Optional.empty(), configOptions.description() );
}

@Test
public void asConfigValue() throws Exception
{
Expand All @@ -96,5 +97,6 @@ public void asConfigValue() throws Exception
assertEquals( Optional.of( 123 ), values.get( 0 ).value() );
assertEquals( "myInt", values.get( 0 ).name() );
assertEquals( Optional.empty(), values.get( 0 ).description() );
assertEquals( "a special test integer", values.get( 0 ).valueDescription() );
}
}
27 changes: 23 additions & 4 deletions community/configuration/src/test/java/org/neo4j/configuration/ConfigValueTest.java
View file
Expand Up @@ -32,7 +32,8 @@ public class ConfigValueTest
@Test
public void handlesEmptyValue() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(), false, false, Optional.empty() );
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
"description", false, false, Optional.empty() );

assertEquals( Optional.empty(), value.value() );
assertEquals( "null", value.toString() );
Expand All @@ -44,7 +45,8 @@ public void handlesEmptyValue() throws Exception
@Test
public void handlesInternal() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(), true, false,
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.empty(),
"description", true, false,
Optional.empty() );

assertTrue( value.internal() );
Expand All @@ -53,7 +55,8 @@ public void handlesInternal() throws Exception
@Test
public void handlesNonEmptyValue() throws Exception
{
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.of( 1 ), false, false, Optional.empty() );
ConfigValue value = new ConfigValue( "name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"description", false, false, Optional.empty() );

assertEquals( Optional.of( 1 ), value.value() );
assertEquals( "1", value.toString() );
Expand All @@ -65,7 +68,8 @@ public void handlesNonEmptyValue() throws Exception
@Test
public void handlesDeprecationAndReplacement() throws Exception
{
ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ), false, true,
ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"description", false, true,
Optional.of( "new_name" ) );

assertEquals( Optional.of( 1 ), value.value() );
Expand All @@ -74,4 +78,19 @@ public void handlesDeprecationAndReplacement() throws Exception
assertEquals( "new_name", value.replacement().get() );
assertFalse( value.internal() );
}

@Test
public void handlesValueDescription() throws Exception
{
ConfigValue value = new ConfigValue( "old_name", Optional.empty(), Optional.empty(), Optional.of( 1 ),
"a simple integer", false, true,
Optional.of( "new_name" ) );

assertEquals( Optional.of( 1 ), value.value() );
assertEquals( "1", value.toString() );
assertTrue( value.deprecated() );
assertEquals( "new_name", value.replacement().get() );
assertFalse( value.internal() );
assertEquals( "a simple integer", value.valueDescription() );
}
}

0 comments on commit f40d0cf

Please sign in to comment.