From 19e01e9837725ca2173f120e89aa51179ffcb6a6 Mon Sep 17 00:00:00 2001 From: Enrico Del Fante Date: Mon, 11 Nov 2024 14:11:22 +0100 Subject: [PATCH] Simplify schema provider definition (#8829) --- .../tech/pegasys/teku/spec/SpecMilestone.java | 18 +- .../schemas/registry/BaseSchemaProvider.java | 99 ++++++----- .../spec/schemas/registry/SchemaProvider.java | 4 +- .../spec/schemas/registry/SchemaRegistry.java | 39 +++-- .../registry/SchemaRegistryBuilder.java | 59 ++++--- .../pegasys/teku/spec/ForkScheduleTest.java | 29 ++-- .../pegasys/teku/spec/SpecVersionTest.java | 29 ++-- .../registry/BaseSchemaProviderTest.java | 62 +++---- .../registry/SchemaRegistryBuilderTest.java | 35 +++- .../schemas/registry/SchemaRegistryTest.java | 159 ++++++++++++++---- 10 files changed, 351 insertions(+), 182 deletions(-) diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/SpecMilestone.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/SpecMilestone.java index a624747d776..301b589b512 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/SpecMilestone.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/SpecMilestone.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import java.util.Arrays; @@ -55,12 +56,21 @@ public boolean isLessThanOrEqualTo(final SpecMilestone other) { } /** Returns the milestone prior to this milestone */ + @SuppressWarnings("EnumOrdinal") public SpecMilestone getPreviousMilestone() { - if (equals(PHASE0)) { - throw new IllegalArgumentException("There is no milestone prior to Phase0"); + checkArgument(!equals(PHASE0), "There is no milestone prior to Phase0"); + final SpecMilestone[] values = SpecMilestone.values(); + return values[ordinal() - 1]; + } + + /** Returns the milestone prior to this milestone */ + @SuppressWarnings("EnumOrdinal") + public Optional getPreviousMilestoneIfExists() { + if (this.equals(PHASE0)) { + return Optional.empty(); } - final List priorMilestones = getAllPriorMilestones(this); - return priorMilestones.getLast(); + final SpecMilestone[] values = SpecMilestone.values(); + return Optional.of(values[ordinal() - 1]); } /** diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java index eb8190e234f..b8b9e778572 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProvider.java @@ -29,32 +29,38 @@ class BaseSchemaProvider implements SchemaProvider { private final TreeMap> milestoneToSchemaCreator = new TreeMap<>(); private final SchemaId schemaId; + private final boolean alwaysCreateNewSchema; private BaseSchemaProvider( final SchemaId schemaId, final List> schemaProviderCreators, final SpecMilestone untilMilestone, - final boolean isConstant) { + final boolean alwaysCreateNewSchema) { this.schemaId = schemaId; + this.alwaysCreateNewSchema = alwaysCreateNewSchema; final List> creatorsList = new ArrayList<>(schemaProviderCreators); SchemaProviderCreator lastCreator = null; for (final SpecMilestone milestone : SpecMilestone.getMilestonesUpTo(untilMilestone)) { - if (!creatorsList.isEmpty() && creatorsList.getFirst().milestone == milestone) { + if (!creatorsList.isEmpty() && creatorsList.getFirst().baseMilestone == milestone) { lastCreator = creatorsList.removeFirst(); } if (lastCreator != null) { - milestoneToSchemaCreator.put( - milestone, isConstant ? lastCreator : lastCreator.withMilestone(milestone)); + milestoneToSchemaCreator.put(milestone, lastCreator); } } } @Override - public SpecMilestone getEffectiveMilestone(final SpecMilestone milestone) { - return getSchemaCreator(milestone).milestone; + public SpecMilestone getBaseMilestone(final SpecMilestone milestone) { + return getSchemaCreator(milestone).baseMilestone; + } + + @Override + public boolean alwaysCreateNewSchema() { + return alwaysCreateNewSchema; } @Override @@ -90,71 +96,63 @@ public Set getSupportedMilestones() { } protected record SchemaProviderCreator( - SpecMilestone milestone, BiFunction creator) { - - private SchemaProviderCreator withMilestone(final SpecMilestone milestone) { - return new SchemaProviderCreator<>(milestone, creator); - } + SpecMilestone baseMilestone, BiFunction creator) { @Override public String toString() { - return MoreObjects.toStringHelper(this).add("milestone", milestone).toString(); + return MoreObjects.toStringHelper(this).add("baseMilestone", baseMilestone).toString(); } } /** - * Creates a builder for a constant schema provider.
- * This can be used when schema remains the same across multiple milestones.
+ * Creates a builder for a schema provider.
* Example usage: * * 
{@code
-   * constantProviderBuilder(EXAMPLE_SCHEMA)
-   *    .withCreator(ALTAIR, (registry, config) -> new ExampleSchemaAltair())
-   *    .withCreator(ELECTRA, (registry, config) -> new ExampleSchemaElectra())
+   * providerBuilder(EXAMPLE_SCHEMA)
+   *    .withCreator(ALTAIR, (registry, config) -> ExampleSchema1.create(registry, config))
+   *    .withCreator(ELECTRA, (registry, config) -> ExampleSchema2.create(registry, config))
    *    .build();
    *
    * }
* * this will create a schema provider that will generate:
- * - only one ExampleSchemaAltair instance which will be reused from ALTAIR to BELLATRIX
- * - only one ExampleSchemaElectra instance which will be reused from ELECTRA to last known - * milestone
- */ - static Builder constantProviderBuilder(final SchemaId schemaId) { - return new Builder<>(schemaId, true); - } - - /** - * Creates a builder for a variable schema provider.
- * This can be used when schema changes across multiple milestones (i.e. depends on changing - * schemas)
- * Example usage: + * - a new ExampleSchema1 for each milestone from ALTAIR to CAPELLA
+ * - a new ExampleSchema2 for each milestone from ELECTRA to last known milestone
+ * + *

By default, the schema provider will check for schema equality when a creator is used + * multiple times. In the previous example, if ExampleSchema1.create generates schemas that are + * equals in both ALTAIR and BELLATRIX context, the ALTAIR instance will be used for + * BELLATRIX too.
+ * Since the equality check does not consider names, semantically equivalent schemas with + * different fields or container names will be considered equal.
+ * + *

If the equality check is relevant, this behavior can be avoided in two ways:
+ * - specifying a new creator like:
* * 

{@code
    * variableProviderBuilder(EXAMPLE_SCHEMA)
-   *    .withCreator(ALTAIR, (registry, config) -> new ExampleSchema1(registry, config))
-   *    .withCreator(ELECTRA, (registry, config) -> new ExampleSchema2(registry, config))
+   *    .withCreator(ALTAIR, (registry, config) -> ExampleSchema1.create(registry, config))
+   *    .withCreator(BELLATRIX, (registry, config) -> ExampleSchema1.create(registry, config))
+   *    .withCreator(ELECTRA, (registry, config) -> ExampleSchema2.create(registry, config))
    *    .build();
    *
    * }
* - * this will create a schema provider that will generate:
- * - a new instance of ExampleSchema1 for each milestone from ALTAIR to ELECTRA
- * - a new instance of ExampleSchema2 for each milestone from ELECTRA to last known milestone
+ * - using {@link Builder#alwaysCreateNewSchema()} */ - static Builder variableProviderBuilder(final SchemaId schemaId) { - return new Builder<>(schemaId, false); + static Builder providerBuilder(final SchemaId schemaId) { + return new Builder<>(schemaId); } static class Builder { private final SchemaId schemaId; - private final boolean isConstant; final List> schemaProviderCreators = new ArrayList<>(); private SpecMilestone untilMilestone = SpecMilestone.getHighestMilestone(); + private boolean alwaysCreateNewSchema = false; - private Builder(final SchemaId schemaId, final boolean isConstant) { + private Builder(final SchemaId schemaId) { this.schemaId = schemaId; - this.isConstant = isConstant; } public Builder withCreator( @@ -162,7 +160,7 @@ public Builder withCreator( final BiFunction creationSchema) { checkArgument( schemaProviderCreators.isEmpty() - || milestone.isGreaterThan(schemaProviderCreators.getLast().milestone), + || milestone.isGreaterThan(schemaProviderCreators.getLast().baseMilestone), "Creator's milestones must added in strict ascending order for %s", schemaId); @@ -170,20 +168,35 @@ public Builder withCreator( return this; } + /** + * This can be used when a schema is deprecated and should not be used for newer milestones. + * + * @param untilMilestone the last milestone for which the schema will be created + */ public Builder until(final SpecMilestone untilMilestone) { this.untilMilestone = untilMilestone; return this; } + /** + * Forces schema provider to create a new schema on each milestone, disabling schema equality + * check with previous milestone. Refer to {@link BaseSchemaProvider} for more information. + */ + public Builder alwaysCreateNewSchema() { + this.alwaysCreateNewSchema = true; + return this; + } + public BaseSchemaProvider build() { checkArgument( !schemaProviderCreators.isEmpty(), "There should be at least 1 creator for %s", schemaId); checkArgument( - untilMilestone.isGreaterThanOrEqualTo(schemaProviderCreators.getLast().milestone), + untilMilestone.isGreaterThanOrEqualTo(schemaProviderCreators.getLast().baseMilestone), "until must be greater or equal than last creator milestone in %s", schemaId); - return new BaseSchemaProvider<>(schemaId, schemaProviderCreators, untilMilestone, isConstant); + return new BaseSchemaProvider<>( + schemaId, schemaProviderCreators, untilMilestone, alwaysCreateNewSchema); } } } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java index 75516b08fe5..d16da64c985 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaProvider.java @@ -22,7 +22,9 @@ interface SchemaProvider { Set getSupportedMilestones(); - SpecMilestone getEffectiveMilestone(SpecMilestone version); + SpecMilestone getBaseMilestone(SpecMilestone version); + + boolean alwaysCreateNewSchema(); SchemaId getSchemaId(); } diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistry.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistry.java index d595c60bfab..ca38743b31c 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistry.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistry.java @@ -75,22 +75,11 @@ public T get(final SchemaId schemaId) { + " or it does not support milestone " + milestone); } - T schema = cache.get(milestone, schemaId); + final T schema = cache.get(milestone, schemaId); if (schema != null) { return schema; } - // let's check if the schema is stored associated to the effective milestone - final SpecMilestone effectiveMilestone = provider.getEffectiveMilestone(milestone); - if (effectiveMilestone != milestone) { - schema = cache.get(effectiveMilestone, schemaId); - if (schema != null) { - // let's cache the schema for current milestone as well - cache.put(milestone, schemaId, schema); - return schema; - } - } - // The schema was not found. // we reach this point only during priming when we actually ask providers to generate schemas checkState(!primed, "Registry is primed but schema not found for %s", schemaId); @@ -101,17 +90,31 @@ public T get(final SchemaId schemaId) { } // actual schema creation (may trigger recursive registry lookups) - schema = provider.getSchema(this); + final T createdSchema = provider.getSchema(this); // release the provider INFLIGHT_PROVIDERS.remove(provider); - // cache the schema - cache.put(effectiveMilestone, schemaId, schema); - if (effectiveMilestone != milestone) { - cache.put(milestone, schemaId, schema); + // let's check if the created schema is equal to the one from the previous milestone + final SpecMilestone effectiveMilestone = provider.getBaseMilestone(milestone); + final T resolvedSchema; + if (provider.alwaysCreateNewSchema()) { + resolvedSchema = createdSchema; + } else { + resolvedSchema = + milestone + .getPreviousMilestoneIfExists() + .filter( + previousMilestone -> previousMilestone.isGreaterThanOrEqualTo(effectiveMilestone)) + .map(previousMilestone -> cache.get(previousMilestone, schemaId)) + .filter(previousSchema -> previousSchema.equals(createdSchema)) + .orElse(createdSchema); } - return schema; + + // cache the schema + cache.put(milestone, schemaId, resolvedSchema); + + return resolvedSchema; } public SpecMilestone getMilestone() { diff --git a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java index ffbf3634ab1..c3bddc64b5e 100644 --- a/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java +++ b/ethereum/spec/src/main/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilder.java @@ -13,11 +13,12 @@ package tech.pegasys.teku.spec.schemas.registry; +import static com.google.common.base.Preconditions.checkArgument; 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 static tech.pegasys.teku.spec.SpecMilestone.PHASE0; -import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.constantProviderBuilder; +import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.providerBuilder; import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.AGGREGATE_AND_PROOF_SCHEMA; import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTESTATION_SCHEMA; import static tech.pegasys.teku.spec.schemas.registry.SchemaTypes.ATTESTER_SLASHING_SCHEMA; @@ -71,6 +72,7 @@ public class SchemaRegistryBuilder { private final Set> providers = new HashSet<>(); private final Set> schemaIds = new HashSet<>(); private final SchemaCache cache; + private SpecMilestone lastBuiltSchemaRegistryMilestone; public static SchemaRegistryBuilder create() { return new SchemaRegistryBuilder() @@ -103,7 +105,7 @@ public static SchemaRegistryBuilder create() { private static SchemaProvider createBlobsBundleSchemaProvider() { // we can keep this to be constant because the blob list max length is // getMaxBlobCommitmentsPerBlock - return constantProviderBuilder(BLOBS_BUNDLE_SCHEMA) + return providerBuilder(BLOBS_BUNDLE_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> @@ -114,7 +116,7 @@ private static SchemaProvider createBlobsBundleSchemaProvider() { private static SchemaProvider createBlobKzgCommitmentsSchemaProvider() { // we can keep this to be constant because the kzg commitment list max length is // getMaxBlobCommitmentsPerBlock - return constantProviderBuilder(BLOB_KZG_COMMITMENTS_SCHEMA) + return providerBuilder(BLOB_KZG_COMMITMENTS_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> @@ -123,14 +125,14 @@ private static SchemaProvider createBlobKzgCommitmentsSchemaProvider() { } private static SchemaProvider createBlobSchemaProvider() { - return constantProviderBuilder(BLOB_SCHEMA) + return providerBuilder(BLOB_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> new BlobSchema(SpecConfigDeneb.required(specConfig))) .build(); } private static SchemaProvider createBlobsInBlockSchemaProvider() { - return constantProviderBuilder(BLOBS_IN_BLOCK_SCHEMA) + return providerBuilder(BLOBS_IN_BLOCK_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> @@ -141,7 +143,7 @@ private static SchemaProvider createBlobsInBlockSchemaProvider() { } private static SchemaProvider createBlobSidecarSchemaProvider() { - return constantProviderBuilder(BLOB_SIDECAR_SCHEMA) + return providerBuilder(BLOB_SIDECAR_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> @@ -153,7 +155,7 @@ private static SchemaProvider createBlobSidecarSchemaProvider() { } private static SchemaProvider createBlobSidecarsByRootRequestMessageSchemaProvider() { - return constantProviderBuilder(BLOB_SIDECARS_BY_ROOT_REQUEST_MESSAGE_SCHEMA) + return providerBuilder(BLOB_SIDECARS_BY_ROOT_REQUEST_MESSAGE_SCHEMA) .withCreator( DENEB, (registry, specConfig) -> @@ -162,32 +164,32 @@ private static SchemaProvider createBlobSidecarsByRootRequestMessageSchemaPro } private static SchemaProvider createHistoricalSummarySchemaProvider() { - return constantProviderBuilder(HISTORICAL_SUMMARY_SCHEMA) + return providerBuilder(HISTORICAL_SUMMARY_SCHEMA) .withCreator(CAPELLA, (registry, specConfig) -> new HistoricalSummarySchema()) .build(); } private static SchemaProvider createSignedBlsToExecutionChangeSchemaProvider() { - return constantProviderBuilder(SIGNED_BLS_TO_EXECUTION_CHANGE_SCHEMA) + return providerBuilder(SIGNED_BLS_TO_EXECUTION_CHANGE_SCHEMA) .withCreator( CAPELLA, (registry, specConfig) -> new SignedBlsToExecutionChangeSchema(registry)) .build(); } private static SchemaProvider createBlsToExecutionChangeSchemaProvider() { - return constantProviderBuilder(BLS_TO_EXECUTION_CHANGE_SCHEMA) + return providerBuilder(BLS_TO_EXECUTION_CHANGE_SCHEMA) .withCreator(CAPELLA, (registry, specConfig) -> new BlsToExecutionChangeSchema()) .build(); } private static SchemaProvider createWithdrawalSchemaProvider() { - return constantProviderBuilder(WITHDRAWAL_SCHEMA) + return providerBuilder(WITHDRAWAL_SCHEMA) .withCreator(CAPELLA, (registry, specConfig) -> new WithdrawalSchema()) .build(); } private static SchemaProvider createAttnetsENRFieldSchemaProvider() { - return constantProviderBuilder(ATTNETS_ENR_FIELD_SCHEMA) + return providerBuilder(ATTNETS_ENR_FIELD_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -196,7 +198,7 @@ private static SchemaProvider createAttnetsENRFieldSchemaProvider() { } private static SchemaProvider createSyncnetsENRFieldSchemaProvider() { - return constantProviderBuilder(SYNCNETS_ENR_FIELD_SCHEMA) + return providerBuilder(SYNCNETS_ENR_FIELD_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -205,7 +207,7 @@ private static SchemaProvider createSyncnetsENRFieldSchemaProvider() { } private static SchemaProvider createBeaconBlocksByRootRequestMessageSchemaProvider() { - return constantProviderBuilder(BEACON_BLOCKS_BY_ROOT_REQUEST_MESSAGE_SCHEMA) + return providerBuilder(BEACON_BLOCKS_BY_ROOT_REQUEST_MESSAGE_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> new BeaconBlocksByRootRequestMessageSchema(specConfig)) @@ -213,7 +215,7 @@ private static SchemaProvider createBeaconBlocksByRootRequestMessageSchemaPro } private static SchemaProvider createHistoricalBatchSchemaProvider() { - return constantProviderBuilder(HISTORICAL_BATCH_SCHEMA) + return providerBuilder(HISTORICAL_BATCH_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -222,7 +224,7 @@ private static SchemaProvider createHistoricalBatchSchemaProvider() { } private static SchemaProvider createAttesterSlashingSchemaProvider() { - return constantProviderBuilder(ATTESTER_SLASHING_SCHEMA) + return providerBuilder(ATTESTER_SLASHING_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -237,7 +239,7 @@ private static SchemaProvider createAttesterSlashingSchemaProvider() { } private static SchemaProvider createIndexedAttestationSchemaProvider() { - return constantProviderBuilder(INDEXED_ATTESTATION_SCHEMA) + return providerBuilder(INDEXED_ATTESTATION_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -254,7 +256,7 @@ private static SchemaProvider createIndexedAttestationSchemaProvider() { } private static SchemaProvider createAttestationSchemaProvider() { - return constantProviderBuilder(ATTESTATION_SCHEMA) + return providerBuilder(ATTESTATION_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -271,7 +273,7 @@ private static SchemaProvider createAttestationSchemaProvider() { } private static SchemaProvider createAggregateAndProofSchemaProvider() { - return constantProviderBuilder(AGGREGATE_AND_PROOF_SCHEMA) + return providerBuilder(AGGREGATE_AND_PROOF_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -286,7 +288,7 @@ private static SchemaProvider createAggregateAndProofSchemaProvider() { } private static SchemaProvider createSignedAggregateAndProofSchemaProvider() { - return constantProviderBuilder(SIGNED_AGGREGATE_AND_PROOF_SCHEMA) + return providerBuilder(SIGNED_AGGREGATE_AND_PROOF_SCHEMA) .withCreator( PHASE0, (registry, specConfig) -> @@ -330,8 +332,25 @@ SchemaRegistryBuilder addProvider(final SchemaProvider provider) { return this; } + @SuppressWarnings("EnumOrdinal") public synchronized SchemaRegistry build( final SpecMilestone milestone, final SpecConfig specConfig) { + + if (lastBuiltSchemaRegistryMilestone == null) { + // we recursively build all previous milestones + milestone + .getPreviousMilestoneIfExists() + .ifPresent(previousMilestone -> build(previousMilestone, specConfig)); + } else { + checkArgument( + lastBuiltSchemaRegistryMilestone.ordinal() == milestone.ordinal() - 1, + "Build must follow the milestone ordering. Last built milestone: %s, requested milestone: %s", + lastBuiltSchemaRegistryMilestone, + milestone); + } + + lastBuiltSchemaRegistryMilestone = milestone; + final SchemaRegistry registry = new SchemaRegistry(milestone, specConfig, cache); for (final SchemaProvider provider : providers) { diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/ForkScheduleTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/ForkScheduleTest.java index 299b52f1b2b..ea4b0e548fc 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/ForkScheduleTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/ForkScheduleTest.java @@ -58,12 +58,12 @@ public class ForkScheduleTest { TRANSITION_CONFIG.toVersionAltair().orElseThrow().getAltairForkVersion(); static final Bytes4 UNKNOWN_FORK_VERSION = Bytes4.fromHexStringLenient("0xFFFFFFFF"); - static final SchemaRegistryBuilder SCHEMA_REGISTRY_BUILDER = SchemaRegistryBuilder.create(); + final SchemaRegistryBuilder schemaRegistryBuilder = SchemaRegistryBuilder.create(); @Test public void build_validScheduleWithAltairTransition() { - final SpecVersion phase0 = SpecVersion.createPhase0(TRANSITION_CONFIG, SCHEMA_REGISTRY_BUILDER); - final SpecVersion altair = SpecVersion.createAltair(TRANSITION_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion phase0 = SpecVersion.createPhase0(TRANSITION_CONFIG, schemaRegistryBuilder); + final SpecVersion altair = SpecVersion.createAltair(TRANSITION_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(phase0).addNextMilestone(altair).build(); @@ -73,8 +73,8 @@ public void build_validScheduleWithAltairTransition() { @Test public void build_validScheduleWithAltairAtGenesis_phase0AndAltairSupplied() { - final SpecVersion phase0 = SpecVersion.createPhase0(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); - final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion phase0 = SpecVersion.createPhase0(ALTAIR_CONFIG, schemaRegistryBuilder); + final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(phase0).addNextMilestone(altair).build(); @@ -85,7 +85,7 @@ public void build_validScheduleWithAltairAtGenesis_phase0AndAltairSupplied() { @Test public void build_validScheduleWithAltairAtGenesis_onlyAltairSupplied() { - final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(altair).build(); @@ -95,7 +95,7 @@ public void build_validScheduleWithAltairAtGenesis_onlyAltairSupplied() { @Test public void build_validPhase0Schedule() { - final SpecVersion phase0 = SpecVersion.createPhase0(PHASE0_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion phase0 = SpecVersion.createPhase0(PHASE0_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(phase0).build(); @@ -105,7 +105,7 @@ public void build_validPhase0Schedule() { @Test public void builder_milestonesSuppliedOutOfOrder_altairProcessedAtNonZeroSlot() { - final SpecVersion altair = SpecVersion.createAltair(TRANSITION_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion altair = SpecVersion.createAltair(TRANSITION_CONFIG, schemaRegistryBuilder); final ForkSchedule.Builder builder = ForkSchedule.builder(); assertThatThrownBy(() -> builder.addNextMilestone(altair)) @@ -115,8 +115,9 @@ public void builder_milestonesSuppliedOutOfOrder_altairProcessedAtNonZeroSlot() @Test public void builder_milestonesSuppliedOutOfOrder_processAltairBeforePhase0() { - final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); - final SpecVersion phase0 = SpecVersion.createPhase0(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, schemaRegistryBuilder); + final SpecVersion phase0 = + SpecVersion.createPhase0(ALTAIR_CONFIG, SchemaRegistryBuilder.create()); final ForkSchedule.Builder builder = ForkSchedule.builder(); builder.addNextMilestone(altair); @@ -135,7 +136,7 @@ public void getSupportedMilestones_withTransition() { @Test public void getSupportedMilestones_onlyAltairConfigured() { - final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion altair = SpecVersion.createAltair(ALTAIR_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(altair).build(); @@ -145,7 +146,7 @@ public void getSupportedMilestones_onlyAltairConfigured() { @Test public void getSupportedMilestones_onlyPhase0Configured() { - final SpecVersion phase0 = SpecVersion.createPhase0(PHASE0_CONFIG, SCHEMA_REGISTRY_BUILDER); + final SpecVersion phase0 = SpecVersion.createPhase0(PHASE0_CONFIG, schemaRegistryBuilder); final ForkSchedule forkSchedule = ForkSchedule.builder().addNextMilestone(phase0).build(); @@ -401,11 +402,11 @@ public void getGenesisFork_withTransition() { private ForkSchedule buildForkSchedule(final SpecConfig specConfig) { final ForkSchedule.Builder builder = ForkSchedule.builder(); - builder.addNextMilestone(SpecVersion.createPhase0(specConfig, SCHEMA_REGISTRY_BUILDER)); + builder.addNextMilestone(SpecVersion.createPhase0(specConfig, schemaRegistryBuilder)); specConfig .toVersionAltair() .ifPresent( - a -> builder.addNextMilestone(SpecVersion.createAltair(a, SCHEMA_REGISTRY_BUILDER))); + a -> builder.addNextMilestone(SpecVersion.createAltair(a, schemaRegistryBuilder))); return builder.build(); } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecVersionTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecVersionTest.java index 0ecc34c084a..d622f58989e 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecVersionTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/SpecVersionTest.java @@ -29,7 +29,6 @@ import tech.pegasys.teku.spec.schemas.registry.SchemaRegistryBuilder; class SpecVersionTest { - private final SchemaRegistryBuilder schemaRegistryBuilder = SchemaRegistryBuilder.create(); private final SpecConfig minimalConfig = SpecConfigLoader.loadConfig(Eth2Network.MINIMAL.configName()); @@ -42,44 +41,48 @@ void shouldCreateSpec(final SpecMilestone milestone) { switch (milestone) { case PHASE0 -> { - expectedVersion = SpecVersion.createPhase0(minimalConfig, schemaRegistryBuilder); + expectedVersion = SpecVersion.createPhase0(minimalConfig, SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.PHASE0, minimalConfig, schemaRegistryBuilder); + SpecVersion.create(SpecMilestone.PHASE0, minimalConfig, SchemaRegistryBuilder.create()); } case ALTAIR -> { expectedVersion = SpecVersion.createAltair( - SpecConfigAltair.required(minimalConfig), schemaRegistryBuilder); + SpecConfigAltair.required(minimalConfig), SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.ALTAIR, minimalConfig, schemaRegistryBuilder); + SpecVersion.create(SpecMilestone.ALTAIR, minimalConfig, SchemaRegistryBuilder.create()); } case BELLATRIX -> { expectedVersion = SpecVersion.createBellatrix( - SpecConfigBellatrix.required(minimalConfig), schemaRegistryBuilder); + SpecConfigBellatrix.required(minimalConfig), SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.BELLATRIX, minimalConfig, schemaRegistryBuilder); + SpecVersion.create( + SpecMilestone.BELLATRIX, minimalConfig, SchemaRegistryBuilder.create()); } case CAPELLA -> { expectedVersion = SpecVersion.createCapella( - SpecConfigCapella.required(minimalConfig), schemaRegistryBuilder); + SpecConfigCapella.required(minimalConfig), SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.CAPELLA, minimalConfig, schemaRegistryBuilder); + SpecVersion.create( + SpecMilestone.CAPELLA, minimalConfig, SchemaRegistryBuilder.create()); } case DENEB -> { expectedVersion = - SpecVersion.createDeneb(SpecConfigDeneb.required(minimalConfig), schemaRegistryBuilder); + SpecVersion.createDeneb( + SpecConfigDeneb.required(minimalConfig), SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.DENEB, minimalConfig, schemaRegistryBuilder); + SpecVersion.create(SpecMilestone.DENEB, minimalConfig, SchemaRegistryBuilder.create()); } case ELECTRA -> { expectedVersion = SpecVersion.createElectra( - SpecConfigElectra.required(minimalConfig), schemaRegistryBuilder); + SpecConfigElectra.required(minimalConfig), SchemaRegistryBuilder.create()); actualVersion = - SpecVersion.create(SpecMilestone.ELECTRA, minimalConfig, schemaRegistryBuilder); + SpecVersion.create( + SpecMilestone.ELECTRA, minimalConfig, SchemaRegistryBuilder.create()); } } diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java index 838c118fe3f..f7a11e169b1 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/BaseSchemaProviderTest.java @@ -16,6 +16,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static tech.pegasys.teku.spec.SpecMilestone.ALTAIR; @@ -23,8 +25,7 @@ import static tech.pegasys.teku.spec.SpecMilestone.CAPELLA; import static tech.pegasys.teku.spec.SpecMilestone.DENEB; import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; -import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.constantProviderBuilder; -import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.variableProviderBuilder; +import static tech.pegasys.teku.spec.schemas.registry.BaseSchemaProvider.providerBuilder; import org.junit.jupiter.api.Test; import tech.pegasys.teku.spec.SpecMilestone; @@ -39,13 +40,13 @@ class BaseSchemaProviderTest { @Test void shouldSupportContinuousUntilHighestMilestone() { final SchemaProvider provider = - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") .build(); - assertEquals(ALTAIR, provider.getEffectiveMilestone(ALTAIR)); - assertEquals(BELLATRIX, provider.getEffectiveMilestone(BELLATRIX)); + assertEquals(ALTAIR, provider.getBaseMilestone(ALTAIR)); + assertEquals(BELLATRIX, provider.getBaseMilestone(BELLATRIX)); when(mockRegistry.getMilestone()).thenReturn(ALTAIR); assertEquals(provider.getSchema(mockRegistry), "TestSchemaAltair"); @@ -60,16 +61,16 @@ void shouldSupportContinuousUntilHighestMilestone() { @Test void shouldSupportContinuousConstantWithUntil() { final SchemaProvider provider = - constantProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(PHASE0, (r, c) -> "TestSchemaPhase0") .withCreator(BELLATRIX, (r, c) -> "TestSchemaBellatrix") .until(CAPELLA) .build(); - assertEquals(PHASE0, provider.getEffectiveMilestone(PHASE0)); - assertEquals(PHASE0, provider.getEffectiveMilestone(ALTAIR)); - assertEquals(BELLATRIX, provider.getEffectiveMilestone(BELLATRIX)); - assertEquals(BELLATRIX, provider.getEffectiveMilestone(CAPELLA)); + assertEquals(PHASE0, provider.getBaseMilestone(PHASE0)); + assertEquals(PHASE0, provider.getBaseMilestone(ALTAIR)); + assertEquals(BELLATRIX, provider.getBaseMilestone(BELLATRIX)); + assertEquals(BELLATRIX, provider.getBaseMilestone(CAPELLA)); when(mockRegistry.getMilestone()).thenReturn(PHASE0); assertEquals(provider.getSchema(mockRegistry), "TestSchemaPhase0"); @@ -88,36 +89,29 @@ void shouldSupportContinuousConstantWithUntil() { } @Test - void shouldSupportContinuousDefaultVariable() { + void shouldAlwaysCreateNewSchemaDisabledByDefault() { final SchemaProvider provider = - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(PHASE0, (r, c) -> "TestSchema" + r.getMilestone()) - .until(CAPELLA) .build(); - // variable has effective milestone always equal to the milestone - SpecMilestone.getMilestonesUpTo(CAPELLA) - .forEach(milestone -> assertEquals(milestone, provider.getEffectiveMilestone(milestone))); - - when(mockRegistry.getMilestone()).thenReturn(PHASE0); - assertEquals(provider.getSchema(mockRegistry), "TestSchemaPHASE0"); - - when(mockRegistry.getMilestone()).thenReturn(ALTAIR); - assertEquals(provider.getSchema(mockRegistry), "TestSchemaALTAIR"); - - when(mockRegistry.getMilestone()).thenReturn(BELLATRIX); - assertEquals(provider.getSchema(mockRegistry), "TestSchemaBELLATRIX"); + assertFalse(provider.alwaysCreateNewSchema()); + } - when(mockRegistry.getMilestone()).thenReturn(CAPELLA); - assertEquals(provider.getSchema(mockRegistry), "TestSchemaCAPELLA"); + @Test + void shouldSupportAlwaysCreateNewSchema() { + final SchemaProvider provider = + providerBuilder(STRING_SCHEMA_ID) + .withCreator(PHASE0, (r, c) -> "TestSchema" + r.getMilestone()) + .alwaysCreateNewSchema() + .build(); - assertThat(provider.getSupportedMilestones()) - .containsExactly(PHASE0, ALTAIR, BELLATRIX, CAPELLA); + assertTrue(provider.alwaysCreateNewSchema()); } @Test void shouldThrowWhenNoCreators() { - assertThatThrownBy(() -> variableProviderBuilder(STRING_SCHEMA_ID).build()) + assertThatThrownBy(() -> providerBuilder(STRING_SCHEMA_ID).build()) .isInstanceOf(IllegalArgumentException.class) .hasMessageStartingWith("There should be at least 1 creator"); } @@ -125,7 +119,7 @@ void shouldThrowWhenNoCreators() { @Test void shouldThrowWhenAskingForAnUnsupportedMilestone() { final SchemaProvider provider = - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(ALTAIR, (r, c) -> "TestSchemaAltair") .until(ALTAIR) .build(); @@ -141,7 +135,7 @@ void shouldThrowWhenAskingForAnUnsupportedMilestone() { void shouldThrowWhenNotAscendingMilestones() { assertThatThrownBy( () -> - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(PHASE0, (r, c) -> "TestSchema") .withCreator(PHASE0, (r, c) -> "TestSchema")) .isInstanceOf(IllegalArgumentException.class) @@ -149,7 +143,7 @@ void shouldThrowWhenNotAscendingMilestones() { assertThatThrownBy( () -> - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(ALTAIR, (r, c) -> "TestSchema") .withCreator(PHASE0, (r, c) -> "TestSchema")) .isInstanceOf(IllegalArgumentException.class) @@ -160,7 +154,7 @@ void shouldThrowWhenNotAscendingMilestones() { void shouldThrowWhenWithUntilIsPriorToMilestone() { assertThatThrownBy( () -> - variableProviderBuilder(STRING_SCHEMA_ID) + providerBuilder(STRING_SCHEMA_ID) .withCreator(PHASE0, (r, c) -> "TestSchema") .withCreator(CAPELLA, (r, c) -> "TestSchema") .until(ALTAIR) diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilderTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilderTest.java index 71a5b04db38..028384e3938 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilderTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryBuilderTest.java @@ -22,6 +22,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static tech.pegasys.teku.spec.SpecMilestone.ALTAIR; +import static tech.pegasys.teku.spec.SpecMilestone.BELLATRIX; import static tech.pegasys.teku.spec.SpecMilestone.PHASE0; import java.util.EnumSet; @@ -48,7 +49,7 @@ void setUp() { when(mockProvider.getSchemaId()).thenReturn(stringId); when(mockProvider.getSchema(any())).thenReturn(stringSchema); when(mockProvider.getSupportedMilestones()).thenReturn(supportedMilestones); - when(mockProvider.getEffectiveMilestone(any())).thenReturn(PHASE0); + when(mockProvider.getBaseMilestone(any())).thenReturn(PHASE0); } @Test @@ -71,12 +72,42 @@ void shouldAddProviderForSupportedMilestone() { @Test void shouldPrimeRegistry() { builder.addProvider(mockProvider); - builder.build(ALTAIR, specConfig); + builder.build(PHASE0, specConfig); // we should have it in cache immediately + verify(cache).put(PHASE0, stringId, stringSchema); + } + + @Test + void shouldAutomaticallyBuildPreviousMilestones() { + builder.addProvider(mockProvider); + builder.build(ALTAIR, specConfig); + + verify(cache).put(PHASE0, stringId, stringSchema); verify(cache).put(ALTAIR, stringId, stringSchema); } + @Test + void shouldThrowWhenNotBuildingInOrder() { + builder.build(PHASE0, specConfig); + + assertThatThrownBy(() -> builder.build(BELLATRIX, specConfig)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Build must follow the milestone ordering. Last built milestone: PHASE0, requested milestone: BELLATRIX"); + } + + @Test + void shouldThrowWhenBuildingSameMilestoneTwice() { + builder.build(PHASE0, specConfig); + builder.build(ALTAIR, specConfig); + + assertThatThrownBy(() -> builder.build(ALTAIR, specConfig)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining( + "Build must follow the milestone ordering. Last built milestone: ALTAIR, requested milestone: ALTAIR"); + } + @Test void shouldThrowWhenAddingTheSameProviderTwice() { builder.addProvider(mockProvider); diff --git a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryTest.java b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryTest.java index d83a6b30f01..3cdfc78b5ff 100644 --- a/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryTest.java +++ b/ethereum/spec/src/test/java/tech/pegasys/teku/spec/schemas/registry/SchemaRegistryTest.java @@ -13,6 +13,7 @@ package tech.pegasys.teku.spec.schemas.registry; +import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -24,6 +25,11 @@ import static org.mockito.Mockito.when; import org.junit.jupiter.api.Test; +import tech.pegasys.teku.infrastructure.ssz.collections.SszByteList; +import tech.pegasys.teku.infrastructure.ssz.containers.Container1; +import tech.pegasys.teku.infrastructure.ssz.containers.ContainerSchema1; +import tech.pegasys.teku.infrastructure.ssz.schema.collections.SszByteListSchema; +import tech.pegasys.teku.infrastructure.ssz.tree.TreeNode; import tech.pegasys.teku.spec.SpecMilestone; import tech.pegasys.teku.spec.config.SpecConfig; import tech.pegasys.teku.spec.schemas.registry.SchemaTypes.SchemaId; @@ -34,22 +40,22 @@ public class SchemaRegistryTest { private final SchemaCache schemaCache = spy(SchemaCache.createDefault()); @SuppressWarnings("unchecked") - private final SchemaProvider schemaProvider = mock(SchemaProvider.class); + private final SchemaProvider schemaProvider = mock(SchemaProvider.class); @SuppressWarnings("unchecked") - private final SchemaId schemaId = mock(SchemaId.class); + private final SchemaId schemaId = mock(SchemaId.class); private final SchemaRegistry schemaRegistry = new SchemaRegistry(SpecMilestone.ALTAIR, specConfig, schemaCache); @Test void shouldGetSchemaFromCache() { - final String cachedSchema = "schema"; + final TestSchema cachedSchema = new TestSchema("test", 2); when(schemaProvider.getSchemaId()).thenReturn(schemaId); when(schemaCache.get(SpecMilestone.ALTAIR, schemaId)).thenReturn(cachedSchema); schemaRegistry.registerProvider(schemaProvider); - final String result = schemaRegistry.get(schemaId); + final TestSchema result = schemaRegistry.get(schemaId); assertEquals(cachedSchema, result); verify(schemaCache).get(SpecMilestone.ALTAIR, schemaId); @@ -57,58 +63,128 @@ void shouldGetSchemaFromCache() { } @Test - void shouldGetSchemaFromProvider() { - final String newSchema = "schema"; + void shouldGetNewInstanceWhenSchemaEqualityCheckIsDisabled() { + + // two different schema instances but equal + final TestSchema newSchema = new TestSchema("test", 2); + final TestSchema cachedPhase0Schema = new TestSchema("test1", 2); + + assertThat(newSchema).isNotSameAs(cachedPhase0Schema); + assertThat(newSchema).isEqualTo(cachedPhase0Schema); + when(schemaProvider.getSchemaId()).thenReturn(schemaId); - when(schemaProvider.getEffectiveMilestone(SpecMilestone.ALTAIR)) - .thenReturn(SpecMilestone.ALTAIR); + when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(cachedPhase0Schema); + when(schemaProvider.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.PHASE0); when(schemaProvider.getSchema(schemaRegistry)).thenReturn(newSchema); + when(schemaProvider.alwaysCreateNewSchema()).thenReturn(true); schemaRegistry.registerProvider(schemaProvider); - final String result = schemaRegistry.get(schemaId); + final TestSchema result = schemaRegistry.get(schemaId); - assertEquals(newSchema, result); + assertThat(result).isSameAs(newSchema); + + verify(schemaCache, never()).get(SpecMilestone.PHASE0, schemaId); verify(schemaCache).get(SpecMilestone.ALTAIR, schemaId); verify(schemaProvider).getSchema(schemaRegistry); - verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, newSchema); + verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, result); } @Test - void shouldCacheMilestoneAndEffectiveMilestoneFromProvider() { - final String newSchema = "schema"; + void shouldGetPreviousMilestoneInstanceWhenSchemaAreEqual() { + + // two different schema instances but equal + final TestSchema newSchema = new TestSchema("test", 2); + final TestSchema cachedPhase0Schema = new TestSchema("test1", 2); + + assertThat(newSchema).isNotSameAs(cachedPhase0Schema); + assertThat(newSchema).isEqualTo(cachedPhase0Schema); + when(schemaProvider.getSchemaId()).thenReturn(schemaId); - when(schemaProvider.getEffectiveMilestone(SpecMilestone.ALTAIR)) - .thenReturn(SpecMilestone.PHASE0); + when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(cachedPhase0Schema); + when(schemaProvider.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.PHASE0); when(schemaProvider.getSchema(schemaRegistry)).thenReturn(newSchema); + when(schemaProvider.alwaysCreateNewSchema()).thenReturn(false); schemaRegistry.registerProvider(schemaProvider); - final String result = schemaRegistry.get(schemaId); + final TestSchema result = schemaRegistry.get(schemaId); + + assertThat(result).isSameAs(cachedPhase0Schema); - assertEquals(newSchema, result); verify(schemaCache).get(SpecMilestone.PHASE0, schemaId); verify(schemaCache).get(SpecMilestone.ALTAIR, schemaId); verify(schemaProvider).getSchema(schemaRegistry); - verify(schemaCache).put(SpecMilestone.PHASE0, schemaId, newSchema); - verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, newSchema); + verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, result); } @Test - void shouldGetFromCachedOfEffectiveMilestone() { - final String newSchema = "schema"; + void shouldGetNewInstanceWhenSchemaAreNotEqual() { + + // two different schema instances but equal + final TestSchema newSchema = new TestSchema("test", 3); + final TestSchema cachedPhase0Schema = new TestSchema("test1", 2); + + assertThat(newSchema).isNotEqualTo(cachedPhase0Schema); + when(schemaProvider.getSchemaId()).thenReturn(schemaId); - when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(newSchema); - when(schemaProvider.getEffectiveMilestone(SpecMilestone.ALTAIR)) - .thenReturn(SpecMilestone.PHASE0); + when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(cachedPhase0Schema); + when(schemaProvider.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.PHASE0); when(schemaProvider.getSchema(schemaRegistry)).thenReturn(newSchema); + when(schemaProvider.alwaysCreateNewSchema()).thenReturn(false); schemaRegistry.registerProvider(schemaProvider); - final String result = schemaRegistry.get(schemaId); + final TestSchema result = schemaRegistry.get(schemaId); - assertEquals(newSchema, result); - verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, newSchema); - verify(schemaProvider).getEffectiveMilestone(SpecMilestone.ALTAIR); + assertThat(result).isSameAs(newSchema); - verify(schemaProvider, never()).getSchema(schemaRegistry); + verify(schemaCache).get(SpecMilestone.PHASE0, schemaId); + verify(schemaCache).get(SpecMilestone.ALTAIR, schemaId); + verify(schemaProvider).getSchema(schemaRegistry); + verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, result); + } + + @Test + void shouldGetNewInstanceWhenNoPreviousCachedSchemaExists() { + final TestSchema newSchema = new TestSchema("test", 3); + + when(schemaProvider.getSchemaId()).thenReturn(schemaId); + when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(null); + when(schemaProvider.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.PHASE0); + when(schemaProvider.getSchema(schemaRegistry)).thenReturn(newSchema); + when(schemaProvider.alwaysCreateNewSchema()).thenReturn(false); + + schemaRegistry.registerProvider(schemaProvider); + final TestSchema result = schemaRegistry.get(schemaId); + + assertThat(result).isSameAs(newSchema); + + verify(schemaCache).get(SpecMilestone.PHASE0, schemaId); + verify(schemaCache).get(SpecMilestone.ALTAIR, schemaId); + verify(schemaProvider).getSchema(schemaRegistry); + verify(schemaCache).put(SpecMilestone.ALTAIR, schemaId, result); + } + + @Test + void shouldGetNewInstanceWhenPhase0() { + final SchemaRegistry schemaRegistry = + new SchemaRegistry(SpecMilestone.PHASE0, specConfig, schemaCache); + + // two different schema instances but equal + final TestSchema newSchema = new TestSchema("test", 3); + + when(schemaProvider.getSchemaId()).thenReturn(schemaId); + when(schemaCache.get(SpecMilestone.PHASE0, schemaId)).thenReturn(null); + when(schemaProvider.getBaseMilestone(SpecMilestone.PHASE0)).thenReturn(SpecMilestone.PHASE0); + when(schemaProvider.getSchema(schemaRegistry)).thenReturn(newSchema); + when(schemaProvider.alwaysCreateNewSchema()).thenReturn(false); + + schemaRegistry.registerProvider(schemaProvider); + final TestSchema result = schemaRegistry.get(schemaId); + + assertThat(result).isSameAs(newSchema); + + verify(schemaCache).get(SpecMilestone.PHASE0, schemaId); + verify(schemaProvider).getSchema(schemaRegistry); + verify(schemaCache).put(SpecMilestone.PHASE0, schemaId, result); } @Test @@ -204,8 +280,8 @@ void shouldResolveNonLoopedDependencies() { final SchemaId id1 = mock(SchemaId.class); final SchemaId id2 = mock(SchemaId.class); - when(provider1.getEffectiveMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); - when(provider2.getEffectiveMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); + when(provider1.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); + when(provider2.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); when(provider1.getSchemaId()).thenReturn(id1); when(provider2.getSchemaId()).thenReturn(id2); @@ -235,8 +311,8 @@ void shouldPrimeRegistry() { final SchemaId id1 = mock(SchemaId.class); final SchemaId id2 = mock(SchemaId.class); - when(provider1.getEffectiveMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); - when(provider2.getEffectiveMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); + when(provider1.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); + when(provider2.getBaseMilestone(SpecMilestone.ALTAIR)).thenReturn(SpecMilestone.ALTAIR); when(provider1.getSchemaId()).thenReturn(id1); when(provider2.getSchemaId()).thenReturn(id2); @@ -266,4 +342,21 @@ void shouldThrowIfRegisteringTheSameSchemaIdTwice() { .isInstanceOf(IllegalStateException.class) .hasMessageContaining("has been already added via another provider"); } + + static class TestView extends Container1 { + public TestView(final TestSchema schema, final SszByteList field0) { + super(schema, field0); + } + } + + static class TestSchema extends ContainerSchema1 { + public TestSchema(final String containerName, final int size) { + super(containerName, namedSchema("field0", SszByteListSchema.create(size))); + } + + @Override + public TestView createFromBackingNode(final TreeNode node) { + return null; + } + } }