Skip to content

Commit

Permalink
improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@tbenr
tbenr committed Oct 10, 2024
1 parent 1685bc7 commit f5ad741
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 82 deletions.
85 changes: 47 additions & 38 deletions ...um/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/AbstractSchemaProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
import static com.google.common.base.Preconditions.checkArgument;

import com.google.common.base.MoreObjects;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.TreeMap;
import java.util.function.BiFunction;
import java.util.stream.Stream;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.config.SpecConfig;
import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId;
Expand All @@ -36,38 +36,57 @@ abstract class AbstractSchemaProvider<T> implements SchemaProvider<T> {
protected AbstractSchemaProvider(
final SchemaId<T> schemaId, final SchemaProviderCreator<T>... schemaProviderCreators) {
this.schemaId = schemaId;
final List<SchemaProviderCreator<T>> creatorsList = Arrays.asList(schemaProviderCreators);
checkArgument(!creatorsList.isEmpty(), "There should be at least 1 creator");

Arrays.stream(schemaProviderCreators)
.forEach(
creator ->
creator
.streamSupporterMilestones()
.forEach(milestone -> putCreatorAtMilestone(milestone, creator)));
}

private void putCreatorAtMilestone(
final SpecMilestone milestone, final SchemaProviderCreator<T> creator) {
if (!milestoneToSchemaCreator.isEmpty()) {
final SpecMilestone lastMilestone = milestoneToSchemaCreator.lastKey();
if (lastMilestone.isGreaterThanOrEqualTo(milestone)) {
throw new IllegalArgumentException(
"Milestones ascending ordering error: from " + lastMilestone + " to " + milestone);
validateCreators(schemaProviderCreators);
final List<SchemaProviderCreator<T>> creatorsList =
new ArrayList<>(Arrays.asList(schemaProviderCreators));

SchemaProviderCreator<T> lastCreator = null;
for (final SpecMilestone milestone : SpecMilestone.values()) {
if (!creatorsList.isEmpty() && creatorsList.getFirst().milestone == milestone) {
lastCreator = creatorsList.removeFirst();
}

if (lastMilestone != milestone.getPreviousMilestone()) {
throw new IllegalArgumentException(
"Milestones gap detected: from "
+ lastMilestone
+ " to "
+ milestone.getPreviousMilestone());
if (lastCreator != null) {
milestoneToSchemaCreator.put(milestone, lastCreator);

final boolean untilSpecReached =
lastCreator
.untilMilestone
.map(untilMilestone -> untilMilestone == milestone)
.orElse(false);

if (untilSpecReached) {
break;
}
}
}
}

if (milestoneToSchemaCreator.put(milestone, creator) != null) {
// this can't happen due to preceded checks
throw new IllegalArgumentException("Overlapping creators detected");
@SafeVarargs
private void validateCreators(final SchemaProviderCreator<T>... schemaProviderCreators) {
checkArgument(
schemaProviderCreators.length > 0, "There should be at least 1 creator for %s", schemaId);
for (int i = 0; i < schemaProviderCreators.length; i++) {
final SchemaProviderCreator<T> currentCreator = schemaProviderCreators[i];
if (i > 0) {
checkArgument(
currentCreator.milestone.isGreaterThan(schemaProviderCreators[i - 1].milestone),
"Creator's milestones must be in order for %s",
schemaId);
}
final boolean isLast = i == schemaProviderCreators.length - 1;
if (isLast) {
checkArgument(
currentCreator.untilMilestone.isEmpty()
|| currentCreator.untilMilestone.get().isGreaterThan(currentCreator.milestone),
"Last creator untilMilestone must be greater than milestone in %s",
schemaId);
} else {
checkArgument(
currentCreator.untilMilestone.isEmpty(),
"Only last creator is allowed to use until for %s",
schemaId);
}
}
}

Expand Down Expand Up @@ -97,8 +116,6 @@ static <T> SchemaProviderCreator<T> schemaCreator(
final SpecMilestone milestone,
final SpecMilestone untilMilestone,
final BiFunction<SchemaRegistry, SpecConfig, T> creationSchema) {
checkArgument(
untilMilestone.isGreaterThan(milestone), "untilMilestone should be greater than milestone");
return new SchemaProviderCreator<>(milestone, Optional.of(untilMilestone), creationSchema);
}

Expand Down Expand Up @@ -127,14 +144,6 @@ protected record SchemaProviderCreator<T>(
SpecMilestone milestone,
Optional<SpecMilestone> untilMilestone,
BiFunction<SchemaRegistry, SpecConfig, T> creator) {
Stream<SpecMilestone> streamSupporterMilestones() {
if (untilMilestone.isEmpty()) {
return Stream.of(milestone);
}
return SpecMilestone.getMilestonesUpTo(untilMilestone.get()).stream()
.filter(m -> m.isGreaterThanOrEqualTo(milestone));
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand Down
24 changes: 0 additions & 24 deletions ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,11 @@

package tech.pegasys.teku.spec.schemas.registry;

import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX;
import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA;
import static tech.pegasys.teku.spec.SpecMilestone.DENEB;
import static tech.pegasys.teku.spec.SpecMilestone.ELECTRA;

import java.util.EnumSet;
import java.util.Set;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId;

interface SchemaProvider<T> {
Set<SpecMilestone> ALL_MILESTONES = EnumSet.allOf(SpecMilestone.class);
Set<SpecMilestone> FROM_BELLATRIX = from(BELLATRIX);
Set<SpecMilestone> FROM_CAPELLA = from(CAPELLA);
Set<SpecMilestone> FROM_DENEB = from(DENEB);
Set<SpecMilestone> FROM_ELECTRA = from(ELECTRA);

static Set<SpecMilestone> from(final SpecMilestone milestone) {
return EnumSet.copyOf(SpecMilestone.getAllMilestonesFrom(milestone));
}

static Set<SpecMilestone> fromTo(
final SpecMilestone fromMilestone, final SpecMilestone toMilestone) {
return EnumSet.copyOf(
SpecMilestone.getAllMilestonesFrom(fromMilestone).stream()
.filter(toMilestone::isLessThanOrEqualTo)
.toList());
}

T getSchema(SchemaRegistry registry);

Set<SpecMilestone> getSupportedMilestones();
Expand Down
48 changes: 28 additions & 20 deletions ...um/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static tech.pegasys.teku.spec.schemas.registry.AbstractSchemaProvider.schemaCreator;

import org.junit.jupiter.api.Test;
import tech.pegasys.teku.spec.SpecMilestone;
import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId;

class BaseSchemaProviderTest {
Expand All @@ -49,14 +50,15 @@ void shouldSupportContinuousWithoutUntil() {
when(mockRegistry.getMilestone()).thenReturn(BELLATRIX);
assertEquals(provider.getSchema(mockRegistry), "TestSchemaBellatrix");

assertThat(provider.getSupportedMilestones()).containsExactly(ALTAIR, BELLATRIX);
assertThat(provider.getSupportedMilestones())
.containsAll(SpecMilestone.getAllMilestonesFrom(ALTAIR));
}

@Test
void shouldSupportContinuousWithUntil() {
final TestSchemaProvider provider =
new TestSchemaProvider(
schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchemaPhase0"),
schemaCreator(PHASE0, (r, c) -> "TestSchemaPhase0"),
schemaCreator(BELLATRIX, CAPELLA, (r, c) -> "TestSchemaBellatrix"));

assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0));
Expand Down Expand Up @@ -84,26 +86,18 @@ void shouldSupportContinuousWithUntil() {
void shouldThrowWhenNoCreators() {
assertThatThrownBy(TestSchemaProvider::new)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("There should be at least 1 creator");
.hasMessageStartingWith("There should be at least 1 creator");
}

@Test
void shouldThrowWhenNotAscendingMilestonesWithUntil() {
assertThatThrownBy(
() ->
new TestSchemaProvider(
schemaCreator(PHASE0, ALTAIR, (r, c) -> "TestSchema"),
schemaCreator(ALTAIR, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Milestones ascending ordering error: from ALTAIR to ALTAIR");

assertThatThrownBy(
() ->
new TestSchemaProvider(
schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"),
schemaCreator(PHASE0, (r, c) -> "TestSchema")))
schemaCreator(ALTAIR, (r, c) -> "TestSchema"),
schemaCreator(PHASE0, BELLATRIX, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Milestones ascending ordering error: from BELLATRIX to PHASE0");
.hasMessageStartingWith("Creator's milestones must be in order");
}

@Test
Expand All @@ -114,26 +108,40 @@ void shouldThrowWhenNotAscendingMilestonesWithoutUntil() {
schemaCreator(PHASE0, (r, c) -> "TestSchema"),
schemaCreator(PHASE0, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Milestones ascending ordering error: from PHASE0 to PHASE0");
.hasMessageStartingWith("Creator's milestones must be in order");

assertThatThrownBy(
() ->
new TestSchemaProvider(
schemaCreator(DENEB, (r, c) -> "TestSchema"),
schemaCreator(ALTAIR, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Milestones ascending ordering error: from DENEB to ALTAIR");
.hasMessageStartingWith("Creator's milestones must be in order");
}

@Test
void shouldThrowWhenDeclaringGaps() {
void shouldThrowWhenWithUntilNotAsLast() {
assertThatThrownBy(
() ->
new TestSchemaProvider(
schemaCreator(PHASE0, (r, c) -> "TestSchema"),
schemaCreator(BELLATRIX, (r, c) -> "TestSchema")))
schemaCreator(ALTAIR, BELLATRIX, (r, c) -> "TestSchema"),
schemaCreator(CAPELLA, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Only last creator is allowed to use until");
}

@Test
void shouldThrowWhenWithUntilIsPriorToMilestone() {
assertThatThrownBy(
() ->
new TestSchemaProvider(schemaCreator(BELLATRIX, BELLATRIX, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in");

assertThatThrownBy(
() -> new TestSchemaProvider(schemaCreator(CAPELLA, BELLATRIX, (r, c) -> "TestSchema")))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Milestones gap detected: from PHASE0 to ALTAIR");
.hasMessageStartingWith("Last creator untilMilestone must be greater than milestone in");
}

private static class TestSchemaProvider extends AbstractSchemaProvider<String> {
Expand Down

0 comments on commit f5ad741

Please sign in to comment.