Remove TOML table headings before checking for valid config parameters (#5483)

* Remove TOML table headings before checking for valid config parameters

* Use dotted paths to find parameters under table headings

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Co-authored-by: Matthew Whitehead <matthew1001@gmail.com>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>
pull/5572/head
Matt Whitehead 1 year ago committed by GitHub
parent 287cfd1f22
commit 39577fd799
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 62
      besu/src/main/java/org/hyperledger/besu/cli/util/TomlConfigFileDefaultProvider.java
  2. 74
      besu/src/test/java/org/hyperledger/besu/cli/TomlConfigFileDefaultProviderTest.java

@ -19,7 +19,9 @@ import org.hyperledger.besu.datatypes.Wei;
import java.io.File;
import java.io.IOException;
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
@ -102,11 +104,49 @@ public class TomlConfigFileDefaultProvider implements IDefaultValueProvider {
private Optional<String> getKeyName(final OptionSpec spec) {
// If any of the names of the option are used as key in the toml results
// then returns the value of first one.
return Arrays.stream(spec.names())
Optional<String> keyName =
Arrays.stream(spec.names())
// remove leading dashes on option name as we can have "--" or "-" options
.map(name -> name.replaceFirst("^-+", ""))
.filter(result::contains)
.findFirst();
if (keyName.isEmpty()) {
// If the base key name doesn't exist in the file it may be under a TOML table heading
// e.g. TxPool.tx-pool-max-size
keyName = getDottedKeyName(spec);
}
return keyName;
}
/*
For all spec names, look to see if any of the TOML keyPathSet entries contain
the name. A key path set might look like ["TxPool", "tx-max-pool-size"] where
"TxPool" is the TOML table heading (which we ignore) and "tx-max-pool-size" is
the name of the option being requested. For a request for "tx-max-pool-size" this
function will return "TxPool.tx-max-pool-size" which can then be used directly
as a query on the TOML result structure.
*/
private Optional<String> getDottedKeyName(final OptionSpec spec) {
List<String> foundNames = new ArrayList<>();
Arrays.stream(spec.names())
.forEach(
nextSpecName -> {
String specName =
result.keyPathSet().stream()
.filter(option -> option.contains(nextSpecName.replaceFirst("^-+", "")))
.findFirst()
.orElse(new ArrayList<>())
.stream()
.collect(Collectors.joining("."));
if (specName.length() > 0) {
foundNames.add(specName);
}
});
return foundNames.stream().findFirst();
}
private String getListEntryAsString(final OptionSpec spec) {
@ -142,7 +182,8 @@ public class TomlConfigFileDefaultProvider implements IDefaultValueProvider {
// return the string representation of the numeric value corresponding to the option in toml
// file - this works for integer, double, and float
// or null if not present in the config
return getKeyName(spec).map(result::get).map(String::valueOf).orElse(null);
return getKeyName(spec).map(result::get).map(Object::toString).orElse(null);
}
private void checkConfigurationValidity() {
@ -184,8 +225,23 @@ public class TomlConfigFileDefaultProvider implements IDefaultValueProvider {
private void checkUnknownOptions(final TomlParseResult result) {
final CommandSpec commandSpec = commandLine.getCommandSpec();
// Besu ignores TOML table headings (e.g. [TxPool]) so we use keyPathSet() and take the
// last element in each one. For a TOML parameter that's not defined inside a table, the lists
// returned in keyPathSet() will contain a single entry - the config parameter itself. For a
// TOML
// entry that is in a table the list will contain N entries, the last one being the config
// parameter itself.
final Set<String> optionsWithoutTables = new HashSet<String>();
result.keyPathSet().stream()
.forEach(
strings -> {
optionsWithoutTables.add(strings.get(strings.size() - 1));
});
// Once we've stripped TOML table headings from the lists, we can check that the remaining
// options are valid
final Set<String> unknownOptionsList =
result.keySet().stream()
optionsWithoutTables.stream()
.filter(option -> !commandSpec.optionsMap().containsKey("--" + option))
.collect(Collectors.toSet());

@ -298,4 +298,78 @@ public class TomlConfigFileDefaultProviderTest {
.isInstanceOf(ParameterException.class)
.hasMessage("Unknown option in TOML configuration file: invalid_option");
}
@Test
public void tomlTableHeadingsMustBeIgnored() throws IOException {
when(mockCommandLine.getCommandSpec()).thenReturn(mockCommandSpec);
Map<String, OptionSpec> validOptionsMap = new HashMap<>();
validOptionsMap.put("--a-valid-option", null);
validOptionsMap.put("--another-valid-option", null);
validOptionsMap.put("--onemore-valid-option", null);
when(mockCommandSpec.optionsMap()).thenReturn(validOptionsMap);
final File tempConfigFile = temp.newFile("config.toml");
final BufferedWriter fileWriter = Files.newBufferedWriter(tempConfigFile.toPath(), UTF_8);
fileWriter.write("a-valid-option=123");
fileWriter.newLine();
fileWriter.write("[ignoreme]");
fileWriter.newLine();
fileWriter.write("another-valid-option=456");
fileWriter.newLine();
fileWriter.write("onemore-valid-option=789");
fileWriter.newLine();
fileWriter.flush();
final TomlConfigFileDefaultProvider providerUnderTest =
new TomlConfigFileDefaultProvider(mockCommandLine, tempConfigFile);
assertThat(
providerUnderTest.defaultValue(
OptionSpec.builder("a-valid-option").type(Integer.class).build()))
.isEqualTo("123");
assertThat(
providerUnderTest.defaultValue(
OptionSpec.builder("another-valid-option").type(Integer.class).build()))
.isEqualTo("456");
assertThat(
providerUnderTest.defaultValue(
OptionSpec.builder("onemore-valid-option").type(Integer.class).build()))
.isEqualTo("789");
}
@Test
public void tomlTableHeadingsMustNotSkipValidationOfUnknownOptions() throws IOException {
when(mockCommandLine.getCommandSpec()).thenReturn(mockCommandSpec);
Map<String, OptionSpec> validOptionsMap = new HashMap<>();
validOptionsMap.put("--a-valid-option", null);
when(mockCommandSpec.optionsMap()).thenReturn(validOptionsMap);
final File tempConfigFile = temp.newFile("config.toml");
final BufferedWriter fileWriter = Files.newBufferedWriter(tempConfigFile.toPath(), UTF_8);
fileWriter.write("[ignoreme]");
fileWriter.newLine();
fileWriter.write("a-valid-option=123");
fileWriter.newLine();
fileWriter.write("invalid-option=789");
fileWriter.newLine();
fileWriter.flush();
final TomlConfigFileDefaultProvider providerUnderTest =
new TomlConfigFileDefaultProvider(mockCommandLine, tempConfigFile);
assertThatThrownBy(
() ->
providerUnderTest.defaultValue(
OptionSpec.builder("an-option").type(String.class).build()))
.isInstanceOf(ParameterException.class)
.hasMessage("Unknown option in TOML configuration file: invalid-option");
}
}

Loading…
Cancel
Save