Skip to content

Commit

Permalink
Remove loadConfigStrict
Browse files Browse the repository at this point in the history
  • Loading branch information
StefanBratanov committed Dec 12, 2024
1 parent 7de9aa4 commit e225be1
Show file tree
Hide file tree
Showing 10 changed files with 20 additions and 110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void shouldLoadMinGenesisTime() {

@Test
public void read_missingConfig() {
processFileAsInputStream(getInvalidConfigPath("missingChurnLimit"), this::readConfig);
processFileAsInputStream(getInvalidConfigPath("missingChurnLimit"), reader::readAndApply);

assertThatThrownBy(reader::build)
.isInstanceOf(IllegalArgumentException.class)
Expand Down Expand Up @@ -251,10 +251,6 @@ private interface InputStreamHandler {
void accept(InputStream inputStream) throws IOException;
}

private void readConfig(final InputStream preset) throws IOException {
reader.readAndApply(preset, false);
}

private Spec getSpec(final Consumer<SpecConfigBuilder> consumer) {
final SpecConfigAndParent<? extends SpecConfig> config =
SpecConfigLoader.loadConfig("ephemery", consumer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,16 +327,6 @@ public int getMaxChunkSize() {
return specConfig.getMaxChunkSize();
}

@Override
public int getTtfbTimeout() {
return specConfig.getTtfbTimeout();
}

@Override
public int getRespTimeout() {
return specConfig.getRespTimeout();
}

@Override
public int getAttestationPropagationSlotRange() {
return specConfig.getAttestationPropagationSlotRange();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ public interface NetworkingSpecConfig {

int getMinEpochsForBlockRequests();

// in seconds
int getTtfbTimeout();

// in seconds
int getRespTimeout();

int getAttestationPropagationSlotRange();

// in millis
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,14 @@ public class SpecConfigLoader {
private static final String CONFIG_PATH = "configs/";
private static final String PRESET_PATH = "presets/";

public static SpecConfigAndParent<? extends SpecConfig> loadConfigStrict(
final String configName) {
return loadConfig(configName, false, __ -> {});
}

public static SpecConfigAndParent<? extends SpecConfig> loadConfig(final String configName) {
return loadConfig(configName, __ -> {});
}

public static SpecConfigAndParent<? extends SpecConfig> loadConfig(
final String configName, final Consumer<SpecConfigBuilder> modifier) {
return loadConfig(configName, true, modifier);
}

public static SpecConfigAndParent<? extends SpecConfig> loadConfig(
final String configName,
final boolean ignoreUnknownConfigItems,
final Consumer<SpecConfigBuilder> modifier) {
final SpecConfigReader reader = new SpecConfigReader();
processConfig(configName, reader, ignoreUnknownConfigItems);
processConfig(configName, reader);
return reader.build(modifier);
}

Expand All @@ -64,28 +52,27 @@ public static SpecConfigAndParent<? extends SpecConfig> loadRemoteConfig(
final SpecConfigReader reader = new SpecConfigReader();
if (config.containsKey(SpecConfigReader.PRESET_KEY)) {
try {
applyPreset("remote", reader, true, config.get(SpecConfigReader.PRESET_KEY));
applyPreset("remote", reader, config.get(SpecConfigReader.PRESET_KEY));
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
if (config.containsKey(SpecConfigReader.CONFIG_NAME_KEY)) {
final String configNameKey = config.get(SpecConfigReader.CONFIG_NAME_KEY);
try {
processConfig(configNameKey, reader, true);
processConfig(configNameKey, reader);
} catch (IllegalArgumentException exception) {
LOG.debug(
"Failed to load base configuration from {}, {}",
() -> configNameKey,
exception::getMessage);
}
}
reader.loadFromMap(config, true);
reader.loadFromMap(config);
return reader.build();
}

static void processConfig(
final String source, final SpecConfigReader reader, final boolean ignoreUnknownConfigItems) {
static void processConfig(final String source, final SpecConfigReader reader) {
try (final InputStream configFile = loadConfigurationFile(source)) {
final Map<String, String> configValues = reader.readValues(configFile);
final Optional<String> maybePreset =
Expand All @@ -94,26 +81,22 @@ static void processConfig(
// Legacy config files won't have a preset field
if (maybePreset.isPresent()) {
final String preset = maybePreset.get();
applyPreset(source, reader, ignoreUnknownConfigItems, preset);
applyPreset(source, reader, preset);
}

reader.loadFromMap(configValues, ignoreUnknownConfigItems);
reader.loadFromMap(configValues);
} catch (IOException | IllegalArgumentException e) {
throw new IllegalArgumentException(
"Unable to load configuration for network \"" + source + "\": " + e.getMessage(), e);
}
}

private static void applyPreset(
final String source,
final SpecConfigReader reader,
final boolean ignoreUnknownConfigItems,
final String preset)
throws IOException {
final String source, final SpecConfigReader reader, final String preset) throws IOException {
for (String resource : AVAILABLE_PRESETS) {
try (final InputStream inputStream = loadPreset(preset, resource).orElse(null)) {
if (inputStream != null) {
reader.readAndApply(inputStream, ignoreUnknownConfigItems);
reader.readAndApply(inputStream);
} else if (resource.equals("phase0")) {
throw new FileNotFoundException(
String.format(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,6 @@ public class SpecConfigPhase0 implements SpecConfig {
private final int maxRequestBlocks;
private final int epochsPerSubnetSubscription;
private final int minEpochsForBlockRequests;
private final int ttfbTimeout;
private final int respTimeout;
private final int attestationPropagationSlotRange;
private final int maximumGossipClockDisparity;
private final Bytes4 messageDomainInvalidSnappy;
Expand Down Expand Up @@ -179,8 +177,6 @@ public SpecConfigPhase0(
final int maxRequestBlocks,
final int epochsPerSubnetSubscription,
final int minEpochsForBlockRequests,
final int ttfbTimeout,
final int respTimeout,
final int attestationPropagationSlotRange,
final int maximumGossipClockDisparity,
final Bytes4 messageDomainInvalidSnappy,
Expand Down Expand Up @@ -249,8 +245,6 @@ public SpecConfigPhase0(
this.maxRequestBlocks = maxRequestBlocks;
this.epochsPerSubnetSubscription = epochsPerSubnetSubscription;
this.minEpochsForBlockRequests = minEpochsForBlockRequests;
this.ttfbTimeout = ttfbTimeout;
this.respTimeout = respTimeout;
this.attestationPropagationSlotRange = attestationPropagationSlotRange;
this.maximumGossipClockDisparity = maximumGossipClockDisparity;
this.messageDomainInvalidSnappy = messageDomainInvalidSnappy;
Expand Down Expand Up @@ -585,16 +579,6 @@ public int getMinEpochsForBlockRequests() {
return minEpochsForBlockRequests;
}

@Override
public int getTtfbTimeout() {
return ttfbTimeout;
}

@Override
public int getRespTimeout() {
return respTimeout;
}

@Override
public int getAttestationPropagationSlotRange() {
return attestationPropagationSlotRange;
Expand Down Expand Up @@ -691,8 +675,6 @@ public boolean equals(final Object o) {
&& attestationSubnetCount == that.attestationSubnetCount
&& attestationSubnetExtraBits == that.attestationSubnetExtraBits
&& attestationSubnetPrefixBits == that.attestationSubnetPrefixBits
&& ttfbTimeout == that.ttfbTimeout
&& respTimeout == that.respTimeout
&& attestationPropagationSlotRange == that.attestationPropagationSlotRange
&& maximumGossipClockDisparity == that.maximumGossipClockDisparity
&& Objects.equals(eth1FollowDistance, that.eth1FollowDistance)
Expand Down Expand Up @@ -772,8 +754,6 @@ public int hashCode() {
maxChunkSize,
maxRequestBlocks,
epochsPerSubnetSubscription,
ttfbTimeout,
respTimeout,
attestationPropagationSlotRange,
maximumGossipClockDisparity,
messageDomainInvalidSnappy,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,12 @@ public SpecConfigAndParent<? extends SpecConfig> build(
* @param source The source to read
* @throws IOException Thrown if an error occurs reading the source
*/
public void readAndApply(final InputStream source, final boolean ignoreUnknownConfigItems)
throws IOException {
public void readAndApply(final InputStream source) throws IOException {
final Map<String, String> rawValues = readValues(source);
loadFromMap(rawValues, ignoreUnknownConfigItems);
loadFromMap(rawValues);
}

public void loadFromMap(
final Map<String, String> rawValues, final boolean ignoreUnknownConfigItems) {
public void loadFromMap(final Map<String, String> rawValues) {
final Map<String, String> unprocessedConfig = new HashMap<>(rawValues);
final Map<String, String> apiSpecConfig = new HashMap<>(rawValues);
// Remove any keys that we're ignoring
Expand Down Expand Up @@ -220,11 +218,7 @@ public void loadFromMap(

if (unprocessedConfig.size() > 0) {
final String unknownKeys = String.join(",", unprocessedConfig.keySet());
if (!ignoreUnknownConfigItems) {
throw new IllegalArgumentException("Detected unknown spec config entries: " + unknownKeys);
} else {
LOG.warn("Ignoring unknown items in network configuration: {}", unknownKeys);
}
LOG.warn("Ignoring unknown items in network configuration: {}", unknownKeys);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ public class SpecConfigBuilder {
private Integer maxChunkSize;
private Integer maxRequestBlocks;
private Integer epochsPerSubnetSubscription;
private Integer ttfbTimeout;
private Integer respTimeout;
private Integer attestationPropagationSlotRange;
private Integer maximumGossipClockDisparity;
private Bytes4 messageDomainInvalidSnappy;
Expand Down Expand Up @@ -205,8 +203,6 @@ public SpecConfigAndParent<SpecConfigElectra> build() {
maxRequestBlocks,
epochsPerSubnetSubscription,
minEpochsForBlockRequests,
ttfbTimeout,
respTimeout,
attestationPropagationSlotRange,
maximumGossipClockDisparity,
messageDomainInvalidSnappy,
Expand Down Expand Up @@ -279,8 +275,6 @@ private Map<String, Object> getValidationMap() {
constants.put("maxRequestBlocks", maxRequestBlocks);
constants.put("epochsPerSubnetSubscription", epochsPerSubnetSubscription);
constants.put("minEpochsForBlockRequests", minEpochsForBlockRequests);
constants.put("ttfbTimeout", ttfbTimeout);
constants.put("respTimeout", respTimeout);
constants.put("attestationPropagationSlotRange", attestationPropagationSlotRange);
constants.put("maximumGossipClockDisparity", maximumGossipClockDisparity);
constants.put("messageDomainInvalidSnappy", messageDomainInvalidSnappy);
Expand Down Expand Up @@ -651,16 +645,6 @@ public SpecConfigBuilder minEpochsForBlockRequests(final Integer minEpochsForBlo
return this;
}

public SpecConfigBuilder ttfbTimeout(final Integer ttfbTimeout) {
this.ttfbTimeout = ttfbTimeout;
return this;
}

public SpecConfigBuilder respTimeout(final Integer respTimeout) {
this.respTimeout = respTimeout;
return this;
}

public SpecConfigBuilder attestationPropagationSlotRange(
final Integer attestationPropagationSlotRange) {
this.attestationPropagationSlotRange = attestationPropagationSlotRange;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class SpecConfigLoaderTest {
@ParameterizedTest(name = "{0}")
@EnumSource(Eth2Network.class)
public void shouldLoadAllKnownNetworks(final Eth2Network network) throws Exception {
final SpecConfig config = SpecConfigLoader.loadConfigStrict(network.configName()).specConfig();
final SpecConfig config = SpecConfigLoader.loadConfig(network.configName()).specConfig();
// testing latest SpecConfig ensures all fields will be asserted on
assertAllFieldsSet(config, SpecConfigElectra.class);
}
Expand Down Expand Up @@ -136,8 +136,7 @@ public void shouldHandleInvalidPresetValue_unknownPreset(@TempDir final Path tem
@Test
public void shouldBeAbleToOverridePresetValues() {
final URL configUrl = SpecConfigLoaderTest.class.getResource("standard/with-overrides.yaml");
final SpecConfig config =
SpecConfigLoader.loadConfig(configUrl.toString(), false, __ -> {}).specConfig();
final SpecConfig config = SpecConfigLoader.loadConfig(configUrl.toString()).specConfig();
assertThat(config).isNotNull();
assertThat(config.getMaxCommitteesPerSlot()).isEqualTo(12); // Mainnet preset is 64.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,20 +54,10 @@ public void read_missingAltairConstant() {
}

@Test
void read_unknownConstant() {
assertThatThrownBy(
() -> processFileAsInputStream(getInvalidConfigPath("unknownField"), this::readConfig))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Detected unknown spec config entries: UNKNOWN_CONSTANT");
}

@Test
void read_ignoringUnknownConstant() {
void read_ignoresUnknownConstant() {
Assertions.assertThatCode(
() -> {
processFileAsInputStream(
getInvalidConfigPath("unknownField"),
source -> reader.readAndApply(source, true));
processFileAsInputStream(getInvalidConfigPath("unknownField"), this::readConfig);
assertAllAltairFieldsSet(reader.build().specConfig());
})
.doesNotThrowAnyException();
Expand Down Expand Up @@ -210,7 +200,7 @@ public void read_invalidBytes4_tooSmall() {
}

private void readConfig(final InputStream preset) throws IOException {
reader.readAndApply(preset, false);
reader.readAndApply(preset);
}

private static String getInvalidConfigPath(final String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class SchemaDefinitionCacheTest {
void shouldGetSchemasForAllMilestonesOnAllNetworks(
final Eth2Network network, final SpecMilestone specMilestone) {
final SpecConfigAndParent<? extends SpecConfig> specConfig =
SpecConfigLoader.loadConfigStrict(network.configName());
SpecConfigLoader.loadConfig(network.configName());
final Spec spec = SpecFactory.create(specConfig);
final SchemaDefinitionCache cache = new SchemaDefinitionCache(spec);
assertThat(cache.getSchemaDefinition(specMilestone)).isNotNull();
Expand Down

0 comments on commit e225be1

Please sign in to comment.