From 6f79cc49b2a713c64642655b0cac5f4f4cb43d09 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:13:57 -0600 Subject: [PATCH 01/23] created a custom config data for consumer, with a single property: timeoutThresholdMillis Signed-off-by: Alfredo Gutierrez --- .../com/hedera/block/server/consumer/ConsumerConfig.java | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java diff --git a/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java new file mode 100644 index 000000000..9d3f82fc0 --- /dev/null +++ b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java @@ -0,0 +1,9 @@ +package com.hedera.block.server.consumer; + +import com.swirlds.config.api.ConfigData; +import com.swirlds.config.api.ConfigProperty; + +@ConfigData("consumer") +public record ConsumerConfig( + @ConfigProperty(defaultValue = "1500") long timeoutThresholdMillis +) {} From eb7667d1778bc1bf21338c17569cc28bab569dc9 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:14:17 -0600 Subject: [PATCH 02/23] deleted unneeded application.yaml props file. Signed-off-by: Alfredo Gutierrez --- server/src/main/resources/application.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 server/src/main/resources/application.yaml diff --git a/server/src/main/resources/application.yaml b/server/src/main/resources/application.yaml deleted file mode 100644 index e69de29bb..000000000 From 3bc6e9afaf92158bdb71001697c07d4f1e946765 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:15:00 -0600 Subject: [PATCH 03/23] adding test app.properties for tests, with prometheus endpoint disabled since is not needed and is littering the logs with errors Signed-off-by: Alfredo Gutierrez --- server/src/test/resources/app.properties | 1 + 1 file changed, 1 insertion(+) create mode 100644 server/src/test/resources/app.properties diff --git a/server/src/test/resources/app.properties b/server/src/test/resources/app.properties new file mode 100644 index 000000000..7356a8e80 --- /dev/null +++ b/server/src/test/resources/app.properties @@ -0,0 +1 @@ +prometheus.endpointEnabled=false From e32db20c99d8de5aaff3cd58394885f3a27a6e03 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:16:19 -0600 Subject: [PATCH 04/23] added scaffolding pieces for tests that need context and configuration, so providing custom properties is possible, and also, getting a test context with test config is easier. Signed-off-by: Alfredo Gutierrez --- .../server/config/TestConfigBuilder.java | 236 ++++++++++++++++++ .../block/server/util/TestConfigUtil.java | 56 +++++ 2 files changed, 292 insertions(+) create mode 100644 server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java create mode 100644 server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java diff --git a/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java b/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java new file mode 100644 index 000000000..1f3846c06 --- /dev/null +++ b/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java @@ -0,0 +1,236 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.block.server.config; + +import com.swirlds.common.config.singleton.ConfigurationHolder; +import com.swirlds.common.threading.locks.AutoClosableLock; +import com.swirlds.common.threading.locks.Locks; +import com.swirlds.common.threading.locks.locked.Locked; +import com.swirlds.config.api.ConfigData; +import com.swirlds.config.api.Configuration; +import com.swirlds.config.api.ConfigurationBuilder; +import com.swirlds.config.api.converter.ConfigConverter; +import com.swirlds.config.api.source.ConfigSource; +import com.swirlds.config.api.validation.ConfigValidator; +import com.swirlds.config.extensions.sources.SimpleConfigSource; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; +import java.util.Objects; + +/** + * Helper for use the config in test and change the config for specific tests. Instance can be used per class or per + * test. + */ +public class TestConfigBuilder { + + private final AutoClosableLock configLock = Locks.createAutoLock(); + + private Configuration configuration = null; + + private final ConfigurationBuilder builder; + + /** + * Creates a new instance and automatically registers all config data records (see {@link ConfigData}) on classpath + * / modulepath that are part of the packages {@code com.swirlds} and {@code com.hedera}. + */ + public TestConfigBuilder() { + this(true); + } + + /** + * Creates a new instance and automatically registers all given config data records. This call will not do a class + * scan for config data records on classpath / modulepath like some of the other constructors do. + * + * @param dataTypes + */ + public TestConfigBuilder(@Nullable final Class dataTypes) { + this(false); + if (dataTypes != null) { + this.builder.withConfigDataTypes(dataTypes); + } + } + + /** + * Creates a new instance and automatically registers all config data records (see {@link ConfigData}) on classpath + * / modulepath that are part of the packages {@code com.swirlds} and {@code com.hedera}. + * if the {@code registerAllTypes} param is true. + * + * @param registerAllTypes if true all config data records on classpath will automatically be registered + */ + public TestConfigBuilder(final boolean registerAllTypes) { + if (registerAllTypes) { + this.builder = ConfigurationBuilder.create().autoDiscoverExtensions(); + } else { + this.builder = ConfigurationBuilder.create(); + } + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, @Nullable final String value) { + return withSource(new SimpleConfigSource(propertyName, value)); + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, final int value) { + return withSource(new SimpleConfigSource(propertyName, value)); + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, final double value) { + return withSource(new SimpleConfigSource(propertyName, value)); + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, final long value) { + return withSource(new SimpleConfigSource(propertyName, value)); + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, final boolean value) { + return withSource(new SimpleConfigSource(propertyName, value)); + } + + /** + * Sets the value for the config. + * + * @param propertyName name of the property + * @param value the value + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValue(@NonNull final String propertyName, @NonNull final Object value) { + Objects.requireNonNull(value, "value must not be null"); + return withSource(new SimpleConfigSource(propertyName, value.toString())); + } + + /** + * This method returns the {@link Configuration} instance. If the method is called for the first time the + * {@link Configuration} instance will be created. All values that have been set (see + * {@link #withValue(String, int)}) methods will be part of the config. Next to this the config will support all + * config data record types (see {@link ConfigData}) that are on the classpath. + * + * @return the created configuration + */ + @NonNull + public Configuration getOrCreateConfig() { + try (final Locked ignore = configLock.lock()) { + if (configuration == null) { + configuration = builder.build(); + ConfigurationHolder.getInstance().setConfiguration(configuration); + } + return configuration; + } + } + + private void checkConfigState() { + try (final Locked ignore = configLock.lock()) { + if (configuration != null) { + throw new IllegalStateException("Configuration already created!"); + } + } + } + + /** + * Adds the given config source to the builder + * + * @param configSource the config source that will be added + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withSource(@NonNull final ConfigSource configSource) { + checkConfigState(); + builder.withSource(configSource); + return this; + } + + /** + * Adds the given config converter to the builder + * + * @param converterType the type of the config converter + * @param converter the config converter that will be added + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withConverter( + @NonNull final Class converterType, @NonNull final ConfigConverter converter) { + checkConfigState(); + builder.withConverter(converterType, converter); + return this; + } + + /** + * Adds the given config validator to the builder + * + * @param validator the config validator that will be added + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withValidator(@NonNull final ConfigValidator validator) { + checkConfigState(); + builder.withValidator(validator); + return this; + } + + /** + * Adds the given config data type to the builder + * + * @param type the config data type that will be added + * @param the type of the config data type + * @return the {@link TestConfigBuilder} instance (for fluent API) + */ + @NonNull + public TestConfigBuilder withConfigDataType(@NonNull final Class type) { + checkConfigState(); + builder.withConfigDataType(type); + return this; + } +} diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java new file mode 100644 index 000000000..375d37929 --- /dev/null +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -0,0 +1,56 @@ +package com.hedera.block.server.util; + +import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.config.TestConfigBuilder; +import com.hedera.block.server.consumer.ConsumerConfig; +import com.swirlds.config.api.Configuration; +import com.swirlds.config.extensions.sources.ClasspathFileConfigSource; +import edu.umd.cs.findbugs.annotations.NonNull; + +import java.io.IOException; +import java.nio.file.Path; +import java.util.Collections; +import java.util.Map; + +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +public class TestConfigUtil { + private TestConfigUtil() {} + + @NonNull + public static BlockNodeContext getSpyBlockNodeContext(Map customProperties) throws IOException { + // If customProperties is null, assign it an empty map + if (customProperties == null) { + customProperties = Collections.emptyMap(); + } + + // we still use the BlockNodeContextFactory to create the BlockNodeContext temporally, + // but we will replace the configuration with a test configuration + // sooner we will need to create a metrics mock, and never use the BlockNodeContextFactory. + BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + + // create test configuration + TestConfigBuilder testConfigBuilder = new TestConfigBuilder(true) + .withSource(new ClasspathFileConfigSource(Path.of("app.properties"))); + + for (Map.Entry entry : customProperties.entrySet()) { + String key = entry.getKey(); + String value = entry.getValue(); + testConfigBuilder = testConfigBuilder.withValue(key, value); + } + + testConfigBuilder = testConfigBuilder.withConfigDataType(ConsumerConfig.class); + + Configuration testConfiguration = testConfigBuilder.getOrCreateConfig(); + + BlockNodeContext spyBlockNodeContext = spy(blockNodeContext); + when(spyBlockNodeContext.configuration()).thenReturn(testConfiguration); + return spyBlockNodeContext; + } + + public static BlockNodeContext getSpyBlockNodeContext() throws IOException { + return getSpyBlockNodeContext(null); + } +} From becb521b33060c6c78d848de5b53066a93048534 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:17:36 -0600 Subject: [PATCH 05/23] Changed Consumer package to use ConsumerConfig instead of timeoutThresholdMillis, all needed changes from server to tests Signed-off-by: Alfredo Gutierrez --- .../block/server/BlockStreamService.java | 8 ++---- .../java/com/hedera/block/server/Server.java | 6 ----- .../ConsumerStreamResponseObserver.java | 7 +++--- .../block/server/BlockStreamServiceIT.java | 16 ++++++------ .../block/server/BlockStreamServiceTest.java | 7 ------ .../ConsumerStreamResponseObserverTest.java | 19 ++++++++------ .../mediator/LiveStreamMediatorImplTest.java | 25 +++++++++++-------- .../ProducerBlockItemObserverTest.java | 11 ++++---- 8 files changed, 45 insertions(+), 54 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/BlockStreamService.java b/server/src/main/java/com/hedera/block/server/BlockStreamService.java index 5b0f1d80d..3e4c2ea07 100644 --- a/server/src/main/java/com/hedera/block/server/BlockStreamService.java +++ b/server/src/main/java/com/hedera/block/server/BlockStreamService.java @@ -21,6 +21,7 @@ import com.google.protobuf.Descriptors; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.consumer.ConsumerConfig; import com.hedera.block.server.consumer.ConsumerStreamResponseObserver; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.StreamMediator; @@ -44,7 +45,6 @@ public class BlockStreamService implements GrpcService { private final System.Logger LOGGER = System.getLogger(getClass().getName()); - private final long timeoutThresholdMillis; private final ItemAckBuilder itemAckBuilder; private final StreamMediator> streamMediator; private final ServiceStatus serviceStatus; @@ -55,8 +55,6 @@ public class BlockStreamService implements GrpcService { * Constructor for the BlockStreamService class. It initializes the BlockStreamService with the * given parameters. * - * @param timeoutThresholdMillis the timeout threshold in milliseconds for the producer to - * publish block items * @param itemAckBuilder the item acknowledgement builder to send responses back to the producer * @param streamMediator the stream mediator to proxy block items from the producer to the * subscribers and manage the subscription lifecycle for subscribers @@ -66,7 +64,6 @@ public class BlockStreamService implements GrpcService { * stop the service and web server in the event of an unrecoverable exception */ BlockStreamService( - final long timeoutThresholdMillis, @NonNull final ItemAckBuilder itemAckBuilder, @NonNull final StreamMediator> @@ -74,7 +71,6 @@ public class BlockStreamService implements GrpcService { @NonNull final BlockReader blockReader, @NonNull final ServiceStatus serviceStatus, @NonNull final BlockNodeContext blockNodeContext) { - this.timeoutThresholdMillis = timeoutThresholdMillis; this.itemAckBuilder = itemAckBuilder; this.streamMediator = streamMediator; this.blockReader = blockReader; @@ -143,7 +139,7 @@ void subscribeBlockStream( @NonNull final var streamObserver = new ConsumerStreamResponseObserver( - timeoutThresholdMillis, + blockNodeContext.configuration().getConfigData(ConsumerConfig.class), Clock.systemDefaultZone(), streamMediator, subscribeStreamResponseObserver); diff --git a/server/src/main/java/com/hedera/block/server/Server.java b/server/src/main/java/com/hedera/block/server/Server.java index 980c8efd3..d3973b3c9 100644 --- a/server/src/main/java/com/hedera/block/server/Server.java +++ b/server/src/main/java/com/hedera/block/server/Server.java @@ -17,7 +17,6 @@ package com.hedera.block.server; import static com.hedera.block.protos.BlockStreamService.*; -import static com.hedera.block.server.Constants.BLOCKNODE_SERVER_CONSUMER_TIMEOUT_THRESHOLD_KEY; import static com.hedera.block.server.Constants.BLOCKNODE_STORAGE_ROOT_PATH_KEY; import com.hedera.block.server.config.BlockNodeContext; @@ -112,12 +111,7 @@ private static BlockStreamService buildBlockStreamService( @NonNull final ServiceStatus serviceStatus, @NonNull final BlockNodeContext blockNodeContext) { - // Get Timeout threshold from configuration - final long consumerTimeoutThreshold = - config.get(BLOCKNODE_SERVER_CONSUMER_TIMEOUT_THRESHOLD_KEY).asLong().orElse(1500L); - return new BlockStreamService( - consumerTimeoutThreshold, new ItemAckBuilder(), streamMediator, blockReader, diff --git a/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java b/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java index 6beec8878..d257c0e2d 100644 --- a/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java +++ b/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java @@ -66,15 +66,14 @@ public class ConsumerStreamResponseObserver * SubscribeStreamResponse events from the Disruptor and passing them to the downstream consumer * via the subscribeStreamResponseObserver. * - * @param timeoutThresholdMillis the timeout threshold in milliseconds for the producer to - * publish block items + * @param consumerConfig contains the Consumer configuration settings * @param producerLivenessClock the clock to use to determine the producer liveness * @param subscriptionHandler the subscription handler to use to manage the subscription * lifecycle * @param subscribeStreamResponseObserver the observer to use to send responses to the consumer */ public ConsumerStreamResponseObserver( - final long timeoutThresholdMillis, + @NonNull final ConsumerConfig consumerConfig, @NonNull final InstantSource producerLivenessClock, @NonNull final SubscriptionHandler> @@ -82,7 +81,7 @@ public ConsumerStreamResponseObserver( @NonNull final StreamObserver subscribeStreamResponseObserver) { - this.timeoutThresholdMillis = timeoutThresholdMillis; + this.timeoutThresholdMillis = consumerConfig.timeoutThresholdMillis(); this.subscriptionHandler = subscriptionHandler; // The ServerCallStreamObserver can be configured with Runnable handlers to diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index e867c5f92..53de89013 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -35,6 +35,7 @@ import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.producer.ItemAckBuilder; +import com.hedera.block.server.util.TestConfigUtil; import com.hedera.block.server.util.TestUtils; import com.lmax.disruptor.BatchEventProcessor; import com.lmax.disruptor.EventHandler; @@ -111,11 +112,12 @@ public void tearDown() { public void testPublishBlockStreamRegistrationAndExecution() throws IOException, NoSuchAlgorithmException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + final BlockNodeContext blockNodeContext = mock(BlockNodeContext.class); + + final BlockStreamService blockStreamService = new BlockStreamService( - 1500L, - new ItemAckBuilder(), + new ItemAckBuilder(), streamMediator, blockReader, serviceStatus, @@ -159,7 +161,8 @@ public void testSubscribeBlockStream() throws IOException { final ServiceStatus serviceStatus = new ServiceStatusImpl(); serviceStatus.setWebServer(webServer); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + final BlockNodeContext blockNodeContext = TestConfigUtil.getSpyBlockNodeContext(); + final var streamMediator = LiveStreamMediatorBuilder.newBuilder(blockWriter, blockNodeContext, serviceStatus) .build(); @@ -167,7 +170,6 @@ public void testSubscribeBlockStream() throws IOException { // Build the BlockStreamService final BlockStreamService blockStreamService = new BlockStreamService( - 2000L, new ItemAckBuilder(), streamMediator, blockReader, @@ -607,9 +609,9 @@ private BlockStreamService buildBlockStreamService( final ServiceStatus serviceStatus) throws IOException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + final BlockNodeContext blockNodeContext = TestConfigUtil.getSpyBlockNodeContext(); + return new BlockStreamService( - 2000, new ItemAckBuilder(), streamMediator, blockReader, diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index 277185764..25fd4c628 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -101,7 +101,6 @@ public void testServiceName() throws IOException, NoSuchAlgorithmException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -121,7 +120,6 @@ public void testProto() throws IOException, NoSuchAlgorithmException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -145,7 +143,6 @@ void testSingleBlockHappyPath() throws IOException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -201,7 +198,6 @@ void testSingleBlockNotFoundPath() throws IOException { // Call the service final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -221,7 +217,6 @@ void testSingleBlockServiceNotAvailable() throws IOException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -245,7 +240,6 @@ public void testSingleBlockIOExceptionPath() throws IOException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, @@ -271,7 +265,6 @@ public void testUpdateInvokesRoutingWithLambdas() throws IOException { final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( - TIMEOUT_THRESHOLD_MILLIS, itemAckBuilder, streamMediator, blockReader, diff --git a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java index bce43f3ea..7eba5f12c 100644 --- a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java @@ -44,14 +44,17 @@ public class ConsumerStreamResponseObserverTest { @Mock private ServerCallStreamObserver serverCallStreamObserver; @Mock private InstantSource testClock; + final ConsumerConfig consumerConfig = new ConsumerConfig(TIMEOUT_THRESHOLD_MILLIS); + @Test public void testProducerTimeoutWithinWindow() { + when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + TIMEOUT_THRESHOLD_MILLIS); final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, responseStreamObserver); @@ -81,7 +84,7 @@ public void testProducerTimeoutOutsideWindow() throws InterruptedException { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, responseStreamObserver); @@ -98,7 +101,7 @@ public void testHandlersSetOnObserver() throws InterruptedException { when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + TIMEOUT_THRESHOLD_MILLIS); new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); verify(serverCallStreamObserver, timeout(50).times(1)).setOnCloseHandler(any()); verify(serverCallStreamObserver, timeout(50).times(1)).setOnCancelHandler(any()); @@ -109,7 +112,7 @@ public void testResponseNotPermittedAfterCancel() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, serverCallStreamObserver); @@ -137,7 +140,7 @@ public void testResponseNotPermittedAfterClose() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, serverCallStreamObserver); @@ -169,7 +172,7 @@ public void testConsumerNotToSendBeforeBlockHeader() { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, responseStreamObserver); @@ -209,12 +212,12 @@ public void testConsumerNotToSendBeforeBlockHeader() { private static class TestConsumerStreamResponseObserver extends ConsumerStreamResponseObserver { public TestConsumerStreamResponseObserver( - long timeoutThresholdMillis, + ConsumerConfig consumerConfig, InstantSource producerLivenessClock, StreamMediator> subscriptionHandler, StreamObserver subscribeStreamResponseObserver) { super( - timeoutThresholdMillis, + consumerConfig, producerLivenessClock, subscriptionHandler, subscribeStreamResponseObserver); diff --git a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java index 0227d6b4e..ad60530e3 100644 --- a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java +++ b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java @@ -24,6 +24,7 @@ import com.hedera.block.server.ServiceStatusImpl; import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.consumer.ConsumerConfig; import com.hedera.block.server.consumer.ConsumerStreamResponseObserver; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.persistence.storage.write.BlockWriter; @@ -60,6 +61,8 @@ public class LiveStreamMediatorImplTest { private static final int testTimeout = 200; + private final ConsumerConfig consumerConfig = new ConsumerConfig(TIMEOUT_THRESHOLD_MILLIS); + @Test public void testUnsubscribeEach() throws InterruptedException, IOException { @@ -139,15 +142,15 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var concreteObserver1 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver1); + consumerConfig, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver2); + consumerConfig, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver3); + consumerConfig, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); @@ -196,15 +199,15 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var concreteObserver1 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver1); + consumerConfig, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver2); + consumerConfig, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver3); + consumerConfig, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); @@ -232,7 +235,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, serverCallStreamObserver); @@ -271,7 +274,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, serverCallStreamObserver); @@ -340,7 +343,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) .build(); final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, + consumerConfig, testClock, streamMediator, serverCallStreamObserver); @@ -357,13 +360,13 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) private static class TestConsumerStreamResponseObserver extends ConsumerStreamResponseObserver { public TestConsumerStreamResponseObserver( - long timeoutThresholdMillis, + ConsumerConfig consumerConfig, final InstantSource producerLivenessClock, final StreamMediator> streamMediator, final StreamObserver responseStreamObserver) { super( - timeoutThresholdMillis, + consumerConfig, producerLivenessClock, streamMediator, responseStreamObserver); diff --git a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java index d21f9393c..efbb2724c 100644 --- a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java @@ -30,6 +30,7 @@ import com.hedera.block.server.ServiceStatusImpl; import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.consumer.ConsumerConfig; import com.hedera.block.server.consumer.ConsumerStreamResponseObserver; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.LiveStreamMediatorBuilder; @@ -108,21 +109,21 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) // Mock a clock with 2 different return values in response to anticipated // millis() calls. Here the second call will always be inside the timeout window. - long TIMEOUT_THRESHOLD_MILLIS = 100L; + ConsumerConfig consumerConfig = new ConsumerConfig(100); long TEST_TIME = 1_719_427_664_950L; - when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + TIMEOUT_THRESHOLD_MILLIS); + when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + consumerConfig.timeoutThresholdMillis()); final var concreteObserver1 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver1); + consumerConfig, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver2); + consumerConfig, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - TIMEOUT_THRESHOLD_MILLIS, testClock, streamMediator, streamObserver3); + consumerConfig, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); From 67e41562466ba1527fc3b79bcb044a49adef29d1 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 19:18:19 -0600 Subject: [PATCH 06/23] style fixes Signed-off-by: Alfredo Gutierrez --- .../java/com/hedera/block/server/Server.java | 6 +-- .../block/server/consumer/ConsumerConfig.java | 20 ++++++-- .../block/server/BlockStreamServiceIT.java | 9 +--- .../server/config/TestConfigBuilder.java | 51 ++++++++++--------- .../ConsumerStreamResponseObserverTest.java | 26 ++-------- .../mediator/LiveStreamMediatorImplTest.java | 21 ++------ .../ProducerBlockItemObserverTest.java | 3 +- .../block/server/util/TestConfigUtil.java | 31 ++++++++--- 8 files changed, 83 insertions(+), 84 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/Server.java b/server/src/main/java/com/hedera/block/server/Server.java index d3973b3c9..37d979934 100644 --- a/server/src/main/java/com/hedera/block/server/Server.java +++ b/server/src/main/java/com/hedera/block/server/Server.java @@ -112,10 +112,6 @@ private static BlockStreamService buildBlockStreamService( @NonNull final BlockNodeContext blockNodeContext) { return new BlockStreamService( - new ItemAckBuilder(), - streamMediator, - blockReader, - serviceStatus, - blockNodeContext); + new ItemAckBuilder(), streamMediator, blockReader, serviceStatus, blockNodeContext); } } diff --git a/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java index 9d3f82fc0..e07d43990 100644 --- a/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java +++ b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java @@ -1,9 +1,23 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.block.server.consumer; import com.swirlds.config.api.ConfigData; import com.swirlds.config.api.ConfigProperty; @ConfigData("consumer") -public record ConsumerConfig( - @ConfigProperty(defaultValue = "1500") long timeoutThresholdMillis -) {} +public record ConsumerConfig(@ConfigProperty(defaultValue = "1500") long timeoutThresholdMillis) {} diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index 53de89013..dd22d79ce 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -114,10 +114,9 @@ public void testPublishBlockStreamRegistrationAndExecution() final BlockNodeContext blockNodeContext = mock(BlockNodeContext.class); - final BlockStreamService blockStreamService = new BlockStreamService( - new ItemAckBuilder(), + new ItemAckBuilder(), streamMediator, blockReader, serviceStatus, @@ -612,10 +611,6 @@ private BlockStreamService buildBlockStreamService( final BlockNodeContext blockNodeContext = TestConfigUtil.getSpyBlockNodeContext(); return new BlockStreamService( - new ItemAckBuilder(), - streamMediator, - blockReader, - serviceStatus, - blockNodeContext); + new ItemAckBuilder(), streamMediator, blockReader, serviceStatus, blockNodeContext); } } diff --git a/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java b/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java index 1f3846c06..fc627f50e 100644 --- a/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java +++ b/server/src/test/java/com/hedera/block/server/config/TestConfigBuilder.java @@ -32,8 +32,8 @@ import java.util.Objects; /** - * Helper for use the config in test and change the config for specific tests. Instance can be used per class or per - * test. + * Helper for use the config in test and change the config for specific tests. Instance can be used + * per class or per test. */ public class TestConfigBuilder { @@ -44,16 +44,18 @@ public class TestConfigBuilder { private final ConfigurationBuilder builder; /** - * Creates a new instance and automatically registers all config data records (see {@link ConfigData}) on classpath - * / modulepath that are part of the packages {@code com.swirlds} and {@code com.hedera}. + * Creates a new instance and automatically registers all config data records (see {@link + * ConfigData}) on classpath / modulepath that are part of the packages {@code com.swirlds} and + * {@code com.hedera}. */ public TestConfigBuilder() { this(true); } /** - * Creates a new instance and automatically registers all given config data records. This call will not do a class - * scan for config data records on classpath / modulepath like some of the other constructors do. + * Creates a new instance and automatically registers all given config data records. This call + * will not do a class scan for config data records on classpath / modulepath like some of the + * other constructors do. * * @param dataTypes */ @@ -65,11 +67,12 @@ public TestConfigBuilder(@Nullable final Class dataTypes) { } /** - * Creates a new instance and automatically registers all config data records (see {@link ConfigData}) on classpath - * / modulepath that are part of the packages {@code com.swirlds} and {@code com.hedera}. - * if the {@code registerAllTypes} param is true. + * Creates a new instance and automatically registers all config data records (see {@link + * ConfigData}) on classpath / modulepath that are part of the packages {@code com.swirlds} and + * {@code com.hedera}. if the {@code registerAllTypes} param is true. * - * @param registerAllTypes if true all config data records on classpath will automatically be registered + * @param registerAllTypes if true all config data records on classpath will automatically be + * registered */ public TestConfigBuilder(final boolean registerAllTypes) { if (registerAllTypes) { @@ -83,11 +86,12 @@ public TestConfigBuilder(final boolean registerAllTypes) { * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull - public TestConfigBuilder withValue(@NonNull final String propertyName, @Nullable final String value) { + public TestConfigBuilder withValue( + @NonNull final String propertyName, @Nullable final String value) { return withSource(new SimpleConfigSource(propertyName, value)); } @@ -95,7 +99,7 @@ public TestConfigBuilder withValue(@NonNull final String propertyName, @Nullable * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull @@ -107,7 +111,7 @@ public TestConfigBuilder withValue(@NonNull final String propertyName, final int * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull @@ -119,7 +123,7 @@ public TestConfigBuilder withValue(@NonNull final String propertyName, final dou * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull @@ -131,7 +135,7 @@ public TestConfigBuilder withValue(@NonNull final String propertyName, final lon * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull @@ -143,20 +147,21 @@ public TestConfigBuilder withValue(@NonNull final String propertyName, final boo * Sets the value for the config. * * @param propertyName name of the property - * @param value the value + * @param value the value * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull - public TestConfigBuilder withValue(@NonNull final String propertyName, @NonNull final Object value) { + public TestConfigBuilder withValue( + @NonNull final String propertyName, @NonNull final Object value) { Objects.requireNonNull(value, "value must not be null"); return withSource(new SimpleConfigSource(propertyName, value.toString())); } /** - * This method returns the {@link Configuration} instance. If the method is called for the first time the - * {@link Configuration} instance will be created. All values that have been set (see - * {@link #withValue(String, int)}) methods will be part of the config. Next to this the config will support all - * config data record types (see {@link ConfigData}) that are on the classpath. + * This method returns the {@link Configuration} instance. If the method is called for the first + * time the {@link Configuration} instance will be created. All values that have been set (see + * {@link #withValue(String, int)}) methods will be part of the config. Next to this the config + * will support all config data record types (see {@link ConfigData}) that are on the classpath. * * @return the created configuration */ @@ -224,7 +229,7 @@ public TestConfigBuilder withValidator(@NonNull final ConfigValidator validator) * Adds the given config data type to the builder * * @param type the config data type that will be added - * @param the type of the config data type + * @param the type of the config data type * @return the {@link TestConfigBuilder} instance (for fluent API) */ @NonNull diff --git a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java index 7eba5f12c..d7ec7561e 100644 --- a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java @@ -49,15 +49,11 @@ public class ConsumerStreamResponseObserverTest { @Test public void testProducerTimeoutWithinWindow() { - when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + TIMEOUT_THRESHOLD_MILLIS); final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - responseStreamObserver); + consumerConfig, testClock, streamMediator, responseStreamObserver); final BlockHeader blockHeader = BlockHeader.newBuilder().setBlockNumber(1).build(); final BlockItem blockItem = BlockItem.newBuilder().setHeader(blockHeader).build(); @@ -84,10 +80,7 @@ public void testProducerTimeoutOutsideWindow() throws InterruptedException { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - responseStreamObserver); + consumerConfig, testClock, streamMediator, responseStreamObserver); consumerBlockItemObserver.onEvent(objectEvent, 0, true); verify(streamMediator).unsubscribe(consumerBlockItemObserver); @@ -112,10 +105,7 @@ public void testResponseNotPermittedAfterCancel() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); final List blockItems = generateBlockItems(1); final SubscribeStreamResponse subscribeStreamResponse = @@ -140,10 +130,7 @@ public void testResponseNotPermittedAfterClose() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); final List blockItems = generateBlockItems(1); final SubscribeStreamResponse subscribeStreamResponse = @@ -172,10 +159,7 @@ public void testConsumerNotToSendBeforeBlockHeader() { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - responseStreamObserver); + consumerConfig, testClock, streamMediator, responseStreamObserver); // Send non-header BlockItems to validate that the observer does not send them for (int i = 1; i <= 10; i++) { diff --git a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java index ad60530e3..cf1fbfb52 100644 --- a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java +++ b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java @@ -235,10 +235,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); streamMediator.subscribe(testConsumerBlockItemObserver); assertTrue(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -274,10 +271,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); streamMediator.subscribe(testConsumerBlockItemObserver); assertTrue(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -343,10 +337,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) .build(); final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, - testClock, - streamMediator, - serverCallStreamObserver); + consumerConfig, testClock, streamMediator, serverCallStreamObserver); // Confirm the observer is not subscribed assertFalse(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -365,11 +356,7 @@ public TestConsumerStreamResponseObserver( final StreamMediator> streamMediator, final StreamObserver responseStreamObserver) { - super( - consumerConfig, - producerLivenessClock, - streamMediator, - responseStreamObserver); + super(consumerConfig, producerLivenessClock, streamMediator, responseStreamObserver); } @NonNull diff --git a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java index efbb2724c..ad0d58d5e 100644 --- a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java @@ -111,7 +111,8 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) // millis() calls. Here the second call will always be inside the timeout window. ConsumerConfig consumerConfig = new ConsumerConfig(100); long TEST_TIME = 1_719_427_664_950L; - when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + consumerConfig.timeoutThresholdMillis()); + when(testClock.millis()) + .thenReturn(TEST_TIME, TEST_TIME + consumerConfig.timeoutThresholdMillis()); final var concreteObserver1 = new ConsumerStreamResponseObserver( diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java index 375d37929..52739b1c5 100644 --- a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -1,5 +1,24 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package com.hedera.block.server.util; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.config.TestConfigBuilder; @@ -7,20 +26,17 @@ import com.swirlds.config.api.Configuration; import com.swirlds.config.extensions.sources.ClasspathFileConfigSource; import edu.umd.cs.findbugs.annotations.NonNull; - import java.io.IOException; import java.nio.file.Path; import java.util.Collections; import java.util.Map; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; - public class TestConfigUtil { private TestConfigUtil() {} @NonNull - public static BlockNodeContext getSpyBlockNodeContext(Map customProperties) throws IOException { + public static BlockNodeContext getSpyBlockNodeContext(Map customProperties) + throws IOException { // If customProperties is null, assign it an empty map if (customProperties == null) { customProperties = Collections.emptyMap(); @@ -32,8 +48,9 @@ public static BlockNodeContext getSpyBlockNodeContext(Map custom BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); // create test configuration - TestConfigBuilder testConfigBuilder = new TestConfigBuilder(true) - .withSource(new ClasspathFileConfigSource(Path.of("app.properties"))); + TestConfigBuilder testConfigBuilder = + new TestConfigBuilder(true) + .withSource(new ClasspathFileConfigSource(Path.of("app.properties"))); for (Map.Entry entry : customProperties.entrySet()) { String key = entry.getKey(); From 736f5cb6a8fa593ef71d0c990fbc01d242ee6f6f Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 23:30:07 -0600 Subject: [PATCH 07/23] Refactor Persistence Storage to use PersistenceStorageConfig Signed-off-by: Alfredo Gutierrez --- .gitignore | 1 + .../com/hedera/block/server/Constants.java | 9 -- .../java/com/hedera/block/server/Server.java | 19 ++--- .../config/BlockNodeConfigExtension.java | 14 ++- .../block/server/consumer/ConsumerConfig.java | 6 ++ .../mediator/LiveStreamMediatorImpl.java | 1 + .../storage/PersistenceStorageConfig.java | 50 +++++++++++ .../storage/read/BlockAsDirReader.java | 8 +- .../storage/read/BlockAsDirReaderBuilder.java | 16 ++-- .../storage/write/BlockAsDirWriter.java | 17 ++-- .../write/BlockAsDirWriterBuilder.java | 26 ++---- server/src/main/java/module-info.java | 6 +- .../block/server/BlockStreamServiceIT.java | 30 +++---- .../block/server/BlockStreamServiceTest.java | 44 +++++----- .../storage/PersistenceStorageConfigTest.java | 65 ++++++++++++++ .../storage/read/BlockAsDirReaderTest.java | 81 +++++++++--------- .../storage/remove/BlockAsDirRemoverTest.java | 26 +++--- .../storage/write/BlockAsDirWriterTest.java | 85 +++++++------------ .../block/server/util/TestConfigUtil.java | 4 +- 19 files changed, 299 insertions(+), 209 deletions(-) create mode 100644 server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java create mode 100644 server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java diff --git a/.gitignore b/.gitignore index 594c0f46a..54f5cbbf4 100644 --- a/.gitignore +++ b/.gitignore @@ -51,3 +51,4 @@ gradle-app.setting .DS_Store # .env files server/docker/.env +/server/data/ diff --git a/server/src/main/java/com/hedera/block/server/Constants.java b/server/src/main/java/com/hedera/block/server/Constants.java index 65eaf30ce..d21f62852 100644 --- a/server/src/main/java/com/hedera/block/server/Constants.java +++ b/server/src/main/java/com/hedera/block/server/Constants.java @@ -22,15 +22,6 @@ public final class Constants { private Constants() {} - /** Constant mapped to the root path config key where the block files are stored */ - @NonNull - public static final String BLOCKNODE_STORAGE_ROOT_PATH_KEY = "blocknode.storage.root.path"; - - /** Constant mapped to the timeout for stream consumers in milliseconds */ - @NonNull - public static final String BLOCKNODE_SERVER_CONSUMER_TIMEOUT_THRESHOLD_KEY = - "blocknode.server.consumer.timeout.threshold"; - /** Constant mapped to the name of the service in the .proto file */ @NonNull public static final String SERVICE_NAME = "BlockStreamGrpcService"; diff --git a/server/src/main/java/com/hedera/block/server/Server.java b/server/src/main/java/com/hedera/block/server/Server.java index 37d979934..c19b6d3f4 100644 --- a/server/src/main/java/com/hedera/block/server/Server.java +++ b/server/src/main/java/com/hedera/block/server/Server.java @@ -17,20 +17,19 @@ package com.hedera.block.server; import static com.hedera.block.protos.BlockStreamService.*; -import static com.hedera.block.server.Constants.BLOCKNODE_STORAGE_ROOT_PATH_KEY; import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.LiveStreamMediatorBuilder; import com.hedera.block.server.mediator.StreamMediator; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.producer.ItemAckBuilder; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; import io.helidon.webserver.WebServer; import io.helidon.webserver.grpc.GrpcRouting; import java.io.IOException; @@ -56,16 +55,14 @@ public static void main(final String[] args) { @NonNull final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); // Set the global configuration - @NonNull final Config config = Config.create(); - Config.global(config); + // @NonNull final Config config = Config.create(); + // Config.global(config); @NonNull final ServiceStatus serviceStatus = new ServiceStatusImpl(); @NonNull final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder( - BLOCKNODE_STORAGE_ROOT_PATH_KEY, config, blockNodeContext) - .build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); @NonNull final StreamMediator> streamMediator = LiveStreamMediatorBuilder.newBuilder( @@ -74,13 +71,16 @@ public static void main(final String[] args) { @NonNull final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(BLOCKNODE_STORAGE_ROOT_PATH_KEY, config) + BlockAsDirReaderBuilder.newBuilder( + blockNodeContext + .configuration() + .getConfigData(PersistenceStorageConfig.class)) .build(); @NonNull final BlockStreamService blockStreamService = buildBlockStreamService( - config, streamMediator, blockReader, serviceStatus, blockNodeContext); + streamMediator, blockReader, serviceStatus, blockNodeContext); @NonNull final GrpcRouting.Builder grpcRouting = @@ -103,7 +103,6 @@ public static void main(final String[] args) { @NonNull private static BlockStreamService buildBlockStreamService( - @NonNull final Config config, @NonNull final StreamMediator> streamMediator, diff --git a/server/src/main/java/com/hedera/block/server/config/BlockNodeConfigExtension.java b/server/src/main/java/com/hedera/block/server/config/BlockNodeConfigExtension.java index 731030d4b..d99c08b9f 100644 --- a/server/src/main/java/com/hedera/block/server/config/BlockNodeConfigExtension.java +++ b/server/src/main/java/com/hedera/block/server/config/BlockNodeConfigExtension.java @@ -17,6 +17,8 @@ package com.hedera.block.server.config; import com.google.auto.service.AutoService; +import com.hedera.block.server.consumer.ConsumerConfig; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.swirlds.common.config.BasicCommonConfig; import com.swirlds.common.metrics.config.MetricsConfig; import com.swirlds.common.metrics.platform.prometheus.PrometheusConfig; @@ -28,6 +30,11 @@ @AutoService(ConfigurationExtension.class) public class BlockNodeConfigExtension implements ConfigurationExtension { + /** Explicitly defined constructor. */ + public BlockNodeConfigExtension() { + super(); + } + /** * {@inheritDoc} * @@ -36,6 +43,11 @@ public class BlockNodeConfigExtension implements ConfigurationExtension { @NonNull @Override public Set> getConfigDataTypes() { - return Set.of(BasicCommonConfig.class, MetricsConfig.class, PrometheusConfig.class); + return Set.of( + BasicCommonConfig.class, + MetricsConfig.class, + PrometheusConfig.class, + ConsumerConfig.class, + PersistenceStorageConfig.class); } } diff --git a/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java index e07d43990..fc4d0124f 100644 --- a/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java +++ b/server/src/main/java/com/hedera/block/server/consumer/ConsumerConfig.java @@ -19,5 +19,11 @@ import com.swirlds.config.api.ConfigData; import com.swirlds.config.api.ConfigProperty; +/** + * Use this configuration across the consumer package. + * + * @param timeoutThresholdMillis after this time of inactivity, the consumer will be considered + * timed out and will be disconnected + */ @ConfigData("consumer") public record ConsumerConfig(@ConfigProperty(defaultValue = "1500") long timeoutThresholdMillis) {} diff --git a/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java b/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java index 09645e440..5c5fdc0a1 100644 --- a/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java +++ b/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java @@ -88,6 +88,7 @@ class LiveStreamMediatorImpl // Initialize and start the disruptor @NonNull final Disruptor> disruptor = + // TODO: replace ring buffer size with a configurable value new Disruptor<>(ObjectEvent::new, 1024, DaemonThreadFactory.INSTANCE); this.ringBuffer = disruptor.start(); this.executor = Executors.newCachedThreadPool(DaemonThreadFactory.INSTANCE); diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java new file mode 100644 index 000000000..f8541ad0b --- /dev/null +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.block.server.persistence.storage; + +import com.swirlds.config.api.ConfigData; +import com.swirlds.config.api.ConfigProperty; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; + +/** + * Use this configuration across the persistent storage package + * + * @param rootPath provides the root path for saving block data + */ +@ConfigData("persistence.storage") +public record PersistenceStorageConfig(@ConfigProperty(defaultValue = "") String rootPath) { + /** + * Constructor to set the default root path if not provided, it will be set to the data + * directory in the current working directory + */ + public PersistenceStorageConfig { + if (rootPath.isEmpty()) { + Path defaultPath = Paths.get(rootPath).toAbsolutePath().resolve("data"); + if (!Files.exists(defaultPath)) { + try { + Files.createDirectories(defaultPath); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + rootPath = defaultPath.toString(); + } + } +} diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java index 7c5c8f156..de1fc4b0c 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReader.java @@ -21,8 +21,8 @@ import static com.hedera.block.protos.BlockStreamService.BlockItem; import static com.hedera.block.server.Constants.BLOCK_FILE_EXTENSION; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -49,18 +49,16 @@ class BlockAsDirReader implements BlockReader { * Constructor for the BlockAsDirReader class. It initializes the BlockAsDirReader with the * given parameters. * - * @param key the key to retrieve the block node root path from the configuration * @param config the configuration to retrieve the block node root path * @param filePerms the file permissions to set on the block node root path */ BlockAsDirReader( - @NonNull final String key, - @NonNull final Config config, + @NonNull final PersistenceStorageConfig config, @NonNull final FileAttribute> filePerms) { LOGGER.log(System.Logger.Level.INFO, "Initializing FileSystemBlockReader"); - @NonNull final Path blockNodeRootPath = Path.of(config.get(key).asString().get()); + @NonNull final Path blockNodeRootPath = Path.of(config.rootPath()); LOGGER.log(System.Logger.Level.INFO, config.toString()); LOGGER.log(System.Logger.Level.INFO, "Block Node Root Path: " + blockNodeRootPath); diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java index c0a3f9d54..f5051949f 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java @@ -18,9 +18,9 @@ import static com.hedera.block.protos.BlockStreamService.Block; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; import java.util.Set; @@ -32,27 +32,23 @@ */ public class BlockAsDirReaderBuilder { - private final String key; - private final Config config; private FileAttribute> filePerms = Util.defaultPerms; + private final PersistenceStorageConfig config; - private BlockAsDirReaderBuilder(@NonNull final String key, @NonNull final Config config) { - this.key = key; + private BlockAsDirReaderBuilder(@NonNull PersistenceStorageConfig config) { this.config = config; } /** * Creates a new block reader builder using the minimum required parameters. * - * @param key is required to read pertinent configuration info. * @param config is required to supply pertinent configuration info for the block reader to * access storage. * @return a block reader builder configured with required parameters. */ @NonNull - public static BlockAsDirReaderBuilder newBuilder( - @NonNull final String key, @NonNull final Config config) { - return new BlockAsDirReaderBuilder(key, config); + public static BlockAsDirReaderBuilder newBuilder(@NonNull PersistenceStorageConfig config) { + return new BlockAsDirReaderBuilder(config); } /** @@ -80,6 +76,6 @@ public BlockAsDirReaderBuilder filePerms( */ @NonNull public BlockReader build() { - return new BlockAsDirReader(key, config, filePerms); + return new BlockAsDirReader(config, filePerms); } } diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java index c04965454..749d311a3 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java @@ -17,14 +17,13 @@ package com.hedera.block.server.persistence.storage.write; import static com.hedera.block.protos.BlockStreamService.BlockItem; -import static com.hedera.block.server.Constants.BLOCKNODE_STORAGE_ROOT_PATH_KEY; import static com.hedera.block.server.Constants.BLOCK_FILE_EXTENSION; import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.metrics.MetricsService; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.remove.BlockRemover; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; import java.io.FileOutputStream; import java.io.IOException; import java.nio.file.Files; @@ -57,16 +56,12 @@ class BlockAsDirWriter implements BlockWriter { * Constructor for the BlockAsDirWriter class. It initializes the BlockAsDirWriter with the * given key, config, block remover, and file permissions. * - * @param key the key to use to retrieve the block node root path from the config - * @param config the config to use to retrieve the block node root path * @param blockRemover the block remover to use to remove blocks if there is an exception while * writing a partial block * @param filePerms the file permissions to set on the block node root path * @throws IOException if an error occurs while initializing the BlockAsDirWriter */ BlockAsDirWriter( - @NonNull final String key, - @NonNull final Config config, @NonNull final BlockRemover blockRemover, @NonNull final FileAttribute> filePerms, @NonNull final BlockNodeContext blockNodeContext) @@ -74,9 +69,12 @@ class BlockAsDirWriter implements BlockWriter { LOGGER.log(System.Logger.Level.INFO, "Initializing FileSystemBlockStorage"); - final Path blockNodeRootPath = Path.of(config.get(key).asString().get()); + PersistenceStorageConfig config = + blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); + final Path blockNodeRootPath = Path.of(config.rootPath()); - LOGGER.log(System.Logger.Level.INFO, config.toString()); + // TODO: Remove comment but, Ask matt why we are logging the config? + // LOGGER.log(System.Logger.Level.INFO, config.toString()); LOGGER.log(System.Logger.Level.INFO, "Block Node Root Path: " + blockNodeRootPath); this.blockNodeRootPath = blockNodeRootPath; @@ -84,8 +82,7 @@ class BlockAsDirWriter implements BlockWriter { this.filePerms = filePerms; if (!blockNodeRootPath.isAbsolute()) { - throw new IllegalArgumentException( - BLOCKNODE_STORAGE_ROOT_PATH_KEY + " must be an absolute path"); + throw new IllegalArgumentException(config.rootPath() + " must be an absolute path"); } // Initialize the block node root directory if it does not exist diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java index 579e8d5e5..036ee3a9d 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java @@ -19,11 +19,11 @@ import static com.hedera.block.protos.BlockStreamService.BlockItem; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.remove.BlockAsDirRemover; import com.hedera.block.server.persistence.storage.remove.BlockRemover; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; import java.io.IOException; import java.nio.file.Path; import java.nio.file.attribute.FileAttribute; @@ -37,39 +37,29 @@ */ public class BlockAsDirWriterBuilder { - private final String key; - private final Config config; private final BlockNodeContext blockNodeContext; private FileAttribute> filePerms = Util.defaultPerms; private BlockRemover blockRemover; - private BlockAsDirWriterBuilder( - @NonNull final String key, - @NonNull final Config config, - @NonNull final BlockNodeContext blockNodeContext) { - this.key = key; - this.config = config; + private BlockAsDirWriterBuilder(@NonNull final BlockNodeContext blockNodeContext) { this.blockNodeContext = blockNodeContext; - this.blockRemover = - new BlockAsDirRemover(Path.of(config.get(key).asString().get()), Util.defaultPerms); + PersistenceStorageConfig config = + blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); + + this.blockRemover = new BlockAsDirRemover(Path.of(config.rootPath()), Util.defaultPerms); } /** * Creates a new block writer builder using the minimum required parameters. * - * @param key is required to read pertinent configuration info. - * @param config is required to supply pertinent configuration info for the block writer to - * access storage. * @param blockNodeContext is required to provide metrics reporting mechanisms . * @return a block writer builder configured with required parameters. */ @NonNull public static BlockAsDirWriterBuilder newBuilder( - @NonNull final String key, - @NonNull final Config config, @NonNull final BlockNodeContext blockNodeContext) { - return new BlockAsDirWriterBuilder(key, config, blockNodeContext); + return new BlockAsDirWriterBuilder(blockNodeContext); } /** @@ -114,6 +104,6 @@ public BlockAsDirWriterBuilder blockRemover(@NonNull BlockRemover blockRemover) */ @NonNull public BlockWriter build() throws IOException { - return new BlockAsDirWriter(key, config, blockRemover, filePerms, blockNodeContext); + return new BlockAsDirWriter(blockRemover, filePerms, blockNodeContext); } } diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index c78b82458..6f4c805d1 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -2,6 +2,11 @@ /** Runtime module of the server. */ module com.hedera.block.server { + exports com.hedera.block.server.consumer to + com.swirlds.config.impl; + exports com.hedera.block.server.persistence.storage to + com.swirlds.config.impl; + requires com.hedera.block.protos; requires com.google.protobuf; requires com.lmax.disruptor; @@ -11,7 +16,6 @@ requires com.swirlds.metrics.api; requires io.grpc.stub; requires io.helidon.common; - requires io.helidon.config; requires io.helidon.webserver.grpc; requires io.helidon.webserver; requires static com.github.spotbugs.annotations; diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index dd22d79ce..9a010cebb 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -23,10 +23,10 @@ import static org.mockito.Mockito.*; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.LiveStreamMediatorBuilder; import com.hedera.block.server.mediator.StreamMediator; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; @@ -40,9 +40,6 @@ import com.lmax.disruptor.BatchEventProcessor; import com.lmax.disruptor.EventHandler; import io.grpc.stub.StreamObserver; -import io.helidon.config.Config; -import io.helidon.config.MapConfigSource; -import io.helidon.config.spi.ConfigSource; import io.helidon.webserver.WebServer; import java.io.IOException; import java.nio.file.Files; @@ -89,7 +86,8 @@ public class BlockStreamServiceIT { private static final String JUNIT = "my-junit-test"; private Path testPath; - private Config testConfig; + private BlockNodeContext blockNodeContext; + private PersistenceStorageConfig testConfig; private static final int testTimeout = 200; @@ -98,9 +96,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - Map testProperties = Map.of(JUNIT, testPath.toString()); - ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - testConfig = Config.builder(testConfigSource).build(); + testConfig = new PersistenceStorageConfig(testPath.toString()); + blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext( + Map.of("persistence.storage.rootPath", testPath.toString())); } @AfterEach @@ -112,7 +111,7 @@ public void tearDown() { public void testPublishBlockStreamRegistrationAndExecution() throws IOException, NoSuchAlgorithmException { - final BlockNodeContext blockNodeContext = mock(BlockNodeContext.class); + // final BlockNodeContext blockNodeContext = mock(BlockNodeContext.class); final BlockStreamService blockStreamService = new BlockStreamService( @@ -499,7 +498,7 @@ public void testMediatorExceptionHandlingWhenPersistenceFailure() throws IOExcep // Now verify the block was removed from the file system. final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockAsDirReaderBuilder.newBuilder(testConfig).build(); final Optional blockOpt = blockReader.read(1); assertTrue(blockOpt.isEmpty()); @@ -525,8 +524,9 @@ public void testMediatorExceptionHandlingWhenPersistenceFailure() throws IOExcep .onNext(expectedSubscriberStreamNotAvailable); } - private void removeRootPathWritePerms(final Config config) throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + private void removeRootPathWritePerms(final PersistenceStorageConfig config) + throws IOException { + final Path blockNodeRootPath = Path.of(config.rootPath()); Files.setPosixFilePermissions(blockNodeRootPath, TestUtils.getNoWrite().value()); } @@ -584,12 +584,10 @@ private StreamMediator> buildStr // Initialize with concrete a concrete BlockReader, BlockWriter and Mediator final BlockRemover blockRemover = - new BlockAsDirRemover( - Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext) + BlockAsDirWriterBuilder.newBuilder(blockNodeContext) .blockRemover(blockRemover) .filePerms(filePerms) .build(); diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index 25fd4c628..21260a8b2 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -28,20 +28,19 @@ import com.google.protobuf.Descriptors; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.config.BlockNodeContextFactory; +// import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.StreamMediator; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.producer.ItemAckBuilder; +import com.hedera.block.server.util.TestConfigUtil; import com.hedera.block.server.util.TestUtils; import io.grpc.stub.ServerCalls; import io.grpc.stub.StreamObserver; -import io.helidon.config.Config; -import io.helidon.config.MapConfigSource; -import io.helidon.config.spi.ConfigSource; import io.helidon.webserver.grpc.GrpcService; import java.io.IOException; import java.nio.file.Files; @@ -60,8 +59,6 @@ @ExtendWith(MockitoExtension.class) public class BlockStreamServiceTest { - private final long TIMEOUT_THRESHOLD_MILLIS = 50L; - @Mock private StreamObserver responseObserver; @Mock private ItemAckBuilder itemAckBuilder; @@ -78,16 +75,24 @@ public class BlockStreamServiceTest { private static final String JUNIT = "my-junit-test"; private Path testPath; - private Config testConfig; + // private Config testConfig; + private BlockNodeContext blockNodeContext; + private PersistenceStorageConfig config; @BeforeEach public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - Map testProperties = Map.of(JUNIT, testPath.toString()); - ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - testConfig = Config.builder(testConfigSource).build(); + // Map testProperties = Map.of(JUNIT, testPath.toString()); + // ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); + // testConfig = Config.builder(testConfigSource).build(); + + config = new PersistenceStorageConfig(testPath.toString()); + + blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext( + Map.of("persistence.storage.rootPath", testPath.toString())); } @AfterEach @@ -98,7 +103,7 @@ public void tearDown() { @Test public void testServiceName() throws IOException, NoSuchAlgorithmException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -117,7 +122,7 @@ public void testServiceName() throws IOException, NoSuchAlgorithmException { @Test public void testProto() throws IOException, NoSuchAlgorithmException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -138,9 +143,8 @@ public void testProto() throws IOException, NoSuchAlgorithmException { @Test void testSingleBlockHappyPath() throws IOException { - final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -154,7 +158,7 @@ void testSingleBlockHappyPath() throws IOException { // Generate and persist a block final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); final List blockItems = generateBlockItems(1); for (BlockItem blockItem : blockItems) { blockWriter.write(blockItem); @@ -183,7 +187,7 @@ void testSingleBlockHappyPath() throws IOException { @Test void testSingleBlockNotFoundPath() throws IOException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); // Get the block so we can verify the response payload when(blockReader.read(1)).thenReturn(Optional.empty()); @@ -214,7 +218,7 @@ void testSingleBlockNotFoundPath() throws IOException { @Test void testSingleBlockServiceNotAvailable() throws IOException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -237,7 +241,7 @@ void testSingleBlockServiceNotAvailable() throws IOException { @Test public void testSingleBlockIOExceptionPath() throws IOException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -262,7 +266,7 @@ public void testSingleBlockIOExceptionPath() throws IOException { @Test public void testUpdateInvokesRoutingWithLambdas() throws IOException { - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java new file mode 100644 index 000000000..c3ba8ddbe --- /dev/null +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java @@ -0,0 +1,65 @@ +/* + * Copyright (C) 2024 Hedera Hashgraph, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.hedera.block.server.persistence.storage; + +import static org.junit.jupiter.api.Assertions.*; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Comparator; +import java.util.stream.Stream; +import org.junit.jupiter.api.Test; + +class PersistenceStorageConfigTest { + + @Test + void testPersistenceStorageConfig() { + PersistenceStorageConfig persistenceStorageConfig = + new PersistenceStorageConfig("rootPath"); + assertEquals("rootPath", persistenceStorageConfig.rootPath()); + } + + @Test + void testPersistenceStorageConfig_emptyRootPath() throws IOException { + final String expectedDefaultRootPath = + Paths.get("").toAbsolutePath().resolve("data").toString(); + // delete if exists + deleteDirectory(Paths.get(expectedDefaultRootPath)); + + PersistenceStorageConfig persistenceStorageConfig = new PersistenceStorageConfig(""); + assertEquals(expectedDefaultRootPath, persistenceStorageConfig.rootPath()); + } + + public static void deleteDirectory(Path path) throws IOException { + if(!Files.exists(path)) { + return; + } + try (Stream walk = Files.walk(path)) { + walk.sorted(Comparator.reverseOrder()) + .forEach( + p -> { + try { + Files.delete(p); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); + } + } +} diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java index aec8c6ba0..19ee05d42 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java @@ -25,16 +25,14 @@ import static org.mockito.Mockito.spy; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.util.PersistTestUtils; +import com.hedera.block.server.util.TestConfigUtil; import com.hedera.block.server.util.TestUtils; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; -import io.helidon.config.MapConfigSource; -import io.helidon.config.spi.ConfigSource; import java.io.FileOutputStream; import java.io.IOException; import java.nio.file.Files; @@ -53,19 +51,27 @@ public class BlockAsDirReaderTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); private static final String TEMP_DIR = "block-node-unit-test-dir"; - private static final String JUNIT = "my-junit-test"; + // private static final String JUNIT = "my-junit-test"; private Path testPath; - private Config testConfig; + // private Config testConfig; + private BlockNodeContext blockNodeContext; + private PersistenceStorageConfig config; @BeforeEach public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - final Map testProperties = Map.of(JUNIT, testPath.toString()); - final ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - testConfig = Config.builder(testConfigSource).build(); + // final Map testProperties = Map.of(JUNIT, testPath.toString()); + // final ConfigSource testConfigSource = + // MapConfigSource.builder().map(testProperties).build(); + // testConfig = Config.builder(testConfigSource).build(); + + config = new PersistenceStorageConfig(testPath.toString()); + blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext( + Map.of("persistence.storage.rootPath", testPath.toString())); } @AfterEach @@ -79,8 +85,7 @@ public void tearDown() { @Test public void testReadBlockDoesNotExist() throws IOException { - final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); final Optional blockOpt = blockReader.read(10000); assertTrue(blockOpt.isEmpty()); } @@ -89,19 +94,18 @@ public void testReadBlockDoesNotExist() throws IOException { public void testReadPermsRepairSucceeded() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); + // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } // Make the block unreadable - removeBlockReadPerms(1, testConfig); + removeBlockReadPerms(1, config); // The default BlockReader will attempt to repair the permissions and should succeed - final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); final Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); assertEquals(10, blockOpt.get().getBlockItemsList().size()); @@ -111,20 +115,19 @@ public void testReadPermsRepairSucceeded() throws IOException { public void testRemoveBlockReadPermsRepairFailed() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } // Make the block unreadable - removeBlockReadPerms(1, testConfig); + removeBlockReadPerms(1, config); // For this test, build the Reader with ineffective repair permissions to // simulate a failed repair (root changed the perms, etc.) final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig) + BlockAsDirReaderBuilder.newBuilder(config) .filePerms(TestUtils.getNoPerms()) .build(); final Optional blockOpt = blockReader.read(1); @@ -135,31 +138,28 @@ public void testRemoveBlockReadPermsRepairFailed() throws IOException { public void testRemoveBlockItemReadPerms() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } - removeBlockItemReadPerms(1, 1, testConfig); + removeBlockItemReadPerms(1, 1, config); - final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); assertThrows(IOException.class, () -> blockReader.read(1)); } @Test public void testPathIsNotDirectory() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final Path blockNodeRootPath = Path.of(testConfig.get(JUNIT).asString().get()); + final Path blockNodeRootPath = Path.of(config.rootPath()); // Write a file named "1" where a directory should be writeFileToPath(blockNodeRootPath.resolve(Path.of("1")), blockItems.getFirst()); // Should return empty because the path is not a directory - final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); final Optional blockOpt = blockReader.read(1); assertTrue(blockOpt.isEmpty()); } @@ -169,19 +169,18 @@ public void testRepairReadPermsFails() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (final BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } - removeBlockReadPerms(1, testConfig); + removeBlockReadPerms(1, config); // Use a spy on a subclass of the BlockAsDirReader to proxy calls // to the actual methods but to also throw an IOException when // the setPerm method is called. - final TestBlockAsDirReader blockReader = spy(new TestBlockAsDirReader(JUNIT, testConfig)); + final TestBlockAsDirReader blockReader = spy(new TestBlockAsDirReader(config)); doThrow(IOException.class).when(blockReader).setPerm(any(), any()); final Optional blockOpt = blockReader.read(1); @@ -192,12 +191,12 @@ public void testRepairReadPermsFails() throws IOException { public void testBlockNodePathReadFails() throws IOException { // Remove read perm on the root path - removePathReadPerms(Path.of(testConfig.get(JUNIT).asString().get())); + removePathReadPerms(Path.of(config.rootPath())); // Use a spy on a subclass of the BlockAsDirReader to proxy calls // to the actual methods but to also throw an IOException when // the setPerm method is called. - final TestBlockAsDirReader blockReader = spy(new TestBlockAsDirReader(JUNIT, testConfig)); + final TestBlockAsDirReader blockReader = spy(new TestBlockAsDirReader(config)); doThrow(IOException.class).when(blockReader).setPerm(any(), any()); final Optional blockOpt = blockReader.read(1); @@ -212,9 +211,9 @@ private void writeFileToPath(final Path path, final BlockItem blockItem) throws } } - public static void removeBlockReadPerms(int blockNumber, final Config config) + public static void removeBlockReadPerms(int blockNumber, final PersistenceStorageConfig config) throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + final Path blockNodeRootPath = Path.of(config.rootPath()); final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); removePathReadPerms(blockPath); } @@ -223,9 +222,9 @@ static void removePathReadPerms(final Path path) throws IOException { Files.setPosixFilePermissions(path, TestUtils.getNoRead().value()); } - private void removeBlockItemReadPerms(int blockNumber, int blockItem, Config config) - throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + private void removeBlockItemReadPerms( + int blockNumber, int blockItem, PersistenceStorageConfig config) throws IOException { + final Path blockNodeRootPath = Path.of(config.rootPath()); final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); final Path blockItemPath = blockPath.resolve(blockItem + BLOCK_FILE_EXTENSION); Files.setPosixFilePermissions(blockItemPath, TestUtils.getNoRead().value()); @@ -234,8 +233,8 @@ private void removeBlockItemReadPerms(int blockNumber, int blockItem, Config con // TestBlockAsDirReader overrides the setPerm() method to allow a test spy to simulate an // IOException while allowing the real setPerm() method to remain protected. private static final class TestBlockAsDirReader extends BlockAsDirReader { - public TestBlockAsDirReader(String key, Config config) { - super(key, config, Util.defaultPerms); + public TestBlockAsDirReader(PersistenceStorageConfig config) { + super(config, Util.defaultPerms); } @Override diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java index 32767b63a..385ee4f85 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java @@ -21,17 +21,15 @@ import com.hedera.block.protos.BlockStreamService.Block; import com.hedera.block.protos.BlockStreamService.BlockItem; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.util.PersistTestUtils; +import com.hedera.block.server.util.TestConfigUtil; import com.hedera.block.server.util.TestUtils; -import io.helidon.config.Config; -import io.helidon.config.MapConfigSource; -import io.helidon.config.spi.ConfigSource; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -49,16 +47,18 @@ public class BlockAsDirRemoverTest { private static final String JUNIT = "my-junit-test"; private Path testPath; - private Config testConfig; + private BlockNodeContext blockNodeContext; + private PersistenceStorageConfig testConfig; @BeforeEach public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - Map testProperties = Map.of(JUNIT, testPath.toString()); - ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - testConfig = Config.builder(testConfigSource).build(); + testConfig = new PersistenceStorageConfig(testPath.toString()); + blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext( + Map.of("persistence.storage.rootPath", testPath.toString())); } @Test @@ -67,9 +67,8 @@ public void testRemoveNonExistentBlock() throws IOException { // Write a block final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (final BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } @@ -80,7 +79,7 @@ public void testRemoveNonExistentBlock() throws IOException { // Verify the block was not removed final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(1); assert (blockOpt.isPresent()); assertEquals( @@ -100,9 +99,8 @@ public void testRemoveBlockWithPermException() throws IOException { // Write a block final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (final BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } @@ -113,7 +111,7 @@ public void testRemoveBlockWithPermException() throws IOException { // Verify the block was not removed final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(1); assert (blockOpt.isPresent()); assertEquals( diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java index 31c4aea4d..150b2c653 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java @@ -23,18 +23,16 @@ import static org.mockito.Mockito.*; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.config.BlockNodeContextFactory; +import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.remove.BlockAsDirRemover; import com.hedera.block.server.persistence.storage.remove.BlockRemover; import com.hedera.block.server.util.PersistTestUtils; +import com.hedera.block.server.util.TestConfigUtil; import com.hedera.block.server.util.TestUtils; import edu.umd.cs.findbugs.annotations.NonNull; -import io.helidon.config.Config; -import io.helidon.config.MapConfigSource; -import io.helidon.config.spi.ConfigSource; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -53,19 +51,21 @@ public class BlockAsDirWriterTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); private static final String TEMP_DIR = "block-node-unit-test-dir"; - private static final String JUNIT = "my-junit-test"; + private static final String PERSISTENCE_STORAGE_ROOT_PATH_KEY = "persistence.storage.rootPath"; private Path testPath; - private Config testConfig; + private BlockNodeContext blockNodeContext; + private PersistenceStorageConfig testConfig; @BeforeEach public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - Map testProperties = Map.of(JUNIT, testPath.toString()); - ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - testConfig = Config.builder(testConfigSource).build(); + testConfig = new PersistenceStorageConfig(testPath.toString()); + blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext( + Map.of(PERSISTENCE_STORAGE_ROOT_PATH_KEY, testPath.toString())); } @AfterEach @@ -79,16 +79,15 @@ public void tearDown() { @Test public void testConstructorWithInvalidPath() throws IOException { - final Map testProperties = Map.of(JUNIT, "invalid-path"); - final ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - final Config testConfig = Config.builder(testConfigSource).build(); + final Map testProperties = + Map.of(PERSISTENCE_STORAGE_ROOT_PATH_KEY, "invalid-path"); + + final BlockNodeContext blockNodeContext = + TestConfigUtil.getSpyBlockNodeContext(testProperties); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); assertThrows( IllegalArgumentException.class, - () -> - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext) - .build()); + () -> BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build()); } @Test @@ -97,16 +96,14 @@ public void testWriterAndReaderHappyPath() throws IOException { // Write a block final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } // Confirm the block - BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); @@ -135,9 +132,8 @@ public void testRemoveBlockWritePerms() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); // Change the permissions on the block node root directory removeRootWritePerms(testConfig); @@ -147,8 +143,7 @@ public void testRemoveBlockWritePerms() throws IOException { blockWriter.write(blockItems.getFirst()); // Confirm we're able to read 1 block item - BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); assertEquals(1, blockOpt.get().getBlockItemsList().size()); @@ -181,14 +176,12 @@ public void testUnrecoverableIOExceptionOnWrite() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); final BlockRemover blockRemover = - new BlockAsDirRemover( - Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); // Use a spy to simulate an IOException when the first block item is written - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = spy( - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext) + BlockAsDirWriterBuilder.newBuilder(blockNodeContext) .blockRemover(blockRemover) .build()); doThrow(IOException.class).when(blockWriter).write(blockItems.getFirst()); @@ -199,9 +192,8 @@ public void testUnrecoverableIOExceptionOnWrite() throws IOException { public void testRemoveRootDirReadPerm() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = - BlockAsDirWriterBuilder.newBuilder(JUNIT, testConfig, blockNodeContext).build(); + BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); // Write the first block item to create the block // directory @@ -218,8 +210,7 @@ public void testRemoveRootDirReadPerm() throws IOException { blockWriter.write(blockItems.get(i)); } - BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); assertEquals(10, blockOpt.get().getBlockItemsList().size()); @@ -229,22 +220,14 @@ public void testRemoveRootDirReadPerm() throws IOException { public void testPartialBlockRemoval() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(3); final BlockRemover blockRemover = - new BlockAsDirRemover( - Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); // Use a spy of TestBlockAsDirWriter to proxy block items to the real writer // for the first 22 block items. Then simulate an IOException on the 23rd block item // thrown from a protected write method in the real class. This should trigger the // blockRemover instance to remove the partially written block. - final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final TestBlockAsDirWriter blockWriter = - spy( - new TestBlockAsDirWriter( - JUNIT, - testConfig, - blockRemover, - Util.defaultPerms, - blockNodeContext)); + spy(new TestBlockAsDirWriter(blockRemover, Util.defaultPerms, blockNodeContext)); for (int i = 0; i < 23; i++) { // Prepare the block writer to call the actual write method @@ -266,7 +249,7 @@ public void testPartialBlockRemoval() throws IOException { // Verify the partially written block was removed final BlockReader blockReader = - BlockAsDirReaderBuilder.newBuilder(JUNIT, testConfig).build(); + BlockAsDirReaderBuilder.newBuilder(testConfig).build(); Optional blockOpt = blockReader.read(3); assertTrue(blockOpt.isEmpty()); @@ -282,19 +265,19 @@ public void testPartialBlockRemoval() throws IOException { assertEquals(2, blockOpt.get().getBlockItems(0).getHeader().getBlockNumber()); } - private void removeRootWritePerms(final Config config) throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + private void removeRootWritePerms(final PersistenceStorageConfig config) throws IOException { + final Path blockNodeRootPath = Path.of(config.rootPath()); Files.setPosixFilePermissions(blockNodeRootPath, TestUtils.getNoWrite().value()); } - private void removeRootReadPerms(final Config config) throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + private void removeRootReadPerms(final PersistenceStorageConfig config) throws IOException { + final Path blockNodeRootPath = Path.of(config.rootPath()); Files.setPosixFilePermissions(blockNodeRootPath, TestUtils.getNoRead().value()); } - private void removeBlockAllPerms(final int blockNumber, final Config config) + private void removeBlockAllPerms(final int blockNumber, final PersistenceStorageConfig config) throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + final Path blockNodeRootPath = Path.of(config.rootPath()); final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); Files.setPosixFilePermissions(blockPath, TestUtils.getNoPerms().value()); } @@ -303,13 +286,11 @@ private void removeBlockAllPerms(final int blockNumber, final Config config) // IOException while allowing the real write() method to remain protected. private static final class TestBlockAsDirWriter extends BlockAsDirWriter { public TestBlockAsDirWriter( - final String key, - final Config config, final BlockRemover blockRemover, final FileAttribute> filePerms, final BlockNodeContext blockNodeContext) throws IOException { - super(key, config, blockRemover, filePerms, blockNodeContext); + super(blockRemover, filePerms, blockNodeContext); } @Override diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java index 52739b1c5..17c7863e1 100644 --- a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -17,7 +17,6 @@ package com.hedera.block.server.util; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; @@ -30,6 +29,7 @@ import java.nio.file.Path; import java.util.Collections; import java.util.Map; +import org.mockito.Mockito; public class TestConfigUtil { private TestConfigUtil() {} @@ -63,7 +63,7 @@ public static BlockNodeContext getSpyBlockNodeContext(Map custom Configuration testConfiguration = testConfigBuilder.getOrCreateConfig(); BlockNodeContext spyBlockNodeContext = spy(blockNodeContext); - when(spyBlockNodeContext.configuration()).thenReturn(testConfiguration); + Mockito.lenient().when(spyBlockNodeContext.configuration()).thenReturn(testConfiguration); return spyBlockNodeContext; } From 9fd4a324bf5010a33b2264f07dee696e1de88873 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 23:33:02 -0600 Subject: [PATCH 08/23] improvements Signed-off-by: Alfredo Gutierrez --- .../block/server/BlockStreamServiceIT.java | 3 --- .../block/server/BlockStreamServiceTest.java | 18 ++---------------- .../storage/remove/BlockAsDirRemoverTest.java | 1 - 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index 9a010cebb..9b3fb1085 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -83,7 +83,6 @@ public class BlockStreamServiceIT { @Mock private BlockWriter blockWriter; private static final String TEMP_DIR = "block-node-unit-test-dir"; - private static final String JUNIT = "my-junit-test"; private Path testPath; private BlockNodeContext blockNodeContext; @@ -111,8 +110,6 @@ public void tearDown() { public void testPublishBlockStreamRegistrationAndExecution() throws IOException, NoSuchAlgorithmException { - // final BlockNodeContext blockNodeContext = mock(BlockNodeContext.class); - final BlockStreamService blockStreamService = new BlockStreamService( new ItemAckBuilder(), diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index 21260a8b2..700715590 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -72,7 +72,6 @@ public class BlockStreamServiceTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); private static final String TEMP_DIR = "block-node-unit-test-dir"; - private static final String JUNIT = "my-junit-test"; private Path testPath; // private Config testConfig; @@ -84,12 +83,7 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - // Map testProperties = Map.of(JUNIT, testPath.toString()); - // ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - // testConfig = Config.builder(testConfigSource).build(); - config = new PersistenceStorageConfig(testPath.toString()); - blockNodeContext = TestConfigUtil.getSpyBlockNodeContext( Map.of("persistence.storage.rootPath", testPath.toString())); @@ -103,7 +97,6 @@ public void tearDown() { @Test public void testServiceName() throws IOException, NoSuchAlgorithmException { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -122,7 +115,6 @@ public void testServiceName() throws IOException, NoSuchAlgorithmException { @Test public void testProto() throws IOException, NoSuchAlgorithmException { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -144,7 +136,6 @@ public void testProto() throws IOException, NoSuchAlgorithmException { void testSingleBlockHappyPath() throws IOException { final BlockReader blockReader = BlockAsDirReaderBuilder.newBuilder(config).build(); - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -187,8 +178,6 @@ void testSingleBlockHappyPath() throws IOException { @Test void testSingleBlockNotFoundPath() throws IOException { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); - // Get the block so we can verify the response payload when(blockReader.read(1)).thenReturn(Optional.empty()); @@ -216,9 +205,8 @@ void testSingleBlockNotFoundPath() throws IOException { } @Test - void testSingleBlockServiceNotAvailable() throws IOException { + void testSingleBlockServiceNotAvailable() { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -241,7 +229,6 @@ void testSingleBlockServiceNotAvailable() throws IOException { @Test public void testSingleBlockIOExceptionPath() throws IOException { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, @@ -264,9 +251,8 @@ public void testSingleBlockIOExceptionPath() throws IOException { } @Test - public void testUpdateInvokesRoutingWithLambdas() throws IOException { + public void testUpdateInvokesRoutingWithLambdas() { - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockStreamService blockStreamService = new BlockStreamService( itemAckBuilder, diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java index 385ee4f85..596d404f6 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java @@ -44,7 +44,6 @@ public class BlockAsDirRemoverTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); private static final String TEMP_DIR = "block-node-unit-test-dir"; - private static final String JUNIT = "my-junit-test"; private Path testPath; private BlockNodeContext blockNodeContext; From bd13246f81190b2e2a5a1255dde8ab0f97703a3c Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Tue, 13 Aug 2024 23:36:29 -0600 Subject: [PATCH 09/23] style fixes and removal of comment and log Signed-off-by: Alfredo Gutierrez --- .../hedera/block/server/mediator/LiveStreamMediatorImpl.java | 2 +- .../server/persistence/storage/write/BlockAsDirWriter.java | 2 -- .../persistence/storage/PersistenceStorageConfigTest.java | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java b/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java index 5c5fdc0a1..015eb09e7 100644 --- a/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java +++ b/server/src/main/java/com/hedera/block/server/mediator/LiveStreamMediatorImpl.java @@ -88,7 +88,7 @@ class LiveStreamMediatorImpl // Initialize and start the disruptor @NonNull final Disruptor> disruptor = - // TODO: replace ring buffer size with a configurable value + // TODO: replace ring buffer size with a configurable value, create a MediatorConfig new Disruptor<>(ObjectEvent::new, 1024, DaemonThreadFactory.INSTANCE); this.ringBuffer = disruptor.start(); this.executor = Executors.newCachedThreadPool(DaemonThreadFactory.INSTANCE); diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java index 749d311a3..1dedf6500 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java @@ -73,8 +73,6 @@ class BlockAsDirWriter implements BlockWriter { blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); final Path blockNodeRootPath = Path.of(config.rootPath()); - // TODO: Remove comment but, Ask matt why we are logging the config? - // LOGGER.log(System.Logger.Level.INFO, config.toString()); LOGGER.log(System.Logger.Level.INFO, "Block Node Root Path: " + blockNodeRootPath); this.blockNodeRootPath = blockNodeRootPath; diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java index c3ba8ddbe..a869b7091 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java @@ -47,7 +47,7 @@ void testPersistenceStorageConfig_emptyRootPath() throws IOException { } public static void deleteDirectory(Path path) throws IOException { - if(!Files.exists(path)) { + if (!Files.exists(path)) { return; } try (Stream walk = Files.walk(path)) { From 915379eb4614190ed8c6e56c0dbfdd9edaf61a22 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 10:10:50 -0600 Subject: [PATCH 10/23] readme changes and improvements Signed-off-by: Alfredo Gutierrez --- README.md | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index e118ae70c..13e6c4444 100644 --- a/README.md +++ b/README.md @@ -30,19 +30,17 @@ Please do not file a public ticket mentioning the vulnerability. Refer to the se --- -# Running Locally +# Configuration -1) Create a local temp directory. For example, use `mktemp -d -t block-stream-temp-dir` to create a directory -2) Configuration variables -``` -export BLOCKNODE_STORAGE_ROOT_PATH= # You can add this to your .zshrc, etc -``` -3) Optional Configuration variables -``` -export BLOCKNODE_SERVER_CONSUMER_TIMEOUT_THRESHOLD="" #Default is 1500 -``` +| Environment Variable | Description | Default Value | +|---------------------------------|---------------------------------------------------------------------------------------------------------------|---------------| +| persistence.storage.rootPath | The root path for the storage, if not provided will attempt to create a `data` on the working dir of the app. | ./data | +| consumer.timeoutThresholdMillis | Time to wait for subscribers before disconnecting in milliseconds | 1500 | -3) ./gradlew run # ./gradlew run --debug-jvm to run in debug mode +3) To run in debug mode: +```bash +./gradlew run # ./gradlew run --debug-jvm to run in debug mode +``` # Running Tests 1) ./gradlew build From 059b234ccecb26a78af5c43c40c5027050536835 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 11:12:12 -0600 Subject: [PATCH 11/23] some variables refactor and improvements generally Signed-off-by: Alfredo Gutierrez --- .../block/server/BlockStreamService.java | 1 + .../java/com/hedera/block/server/Server.java | 4 --- .../storage/PersistenceStorageConfig.java | 3 ++- .../block/server/BlockStreamServiceIT.java | 8 +++--- .../block/server/BlockStreamServiceTest.java | 4 +-- .../storage/read/BlockAsDirReaderTest.java | 10 ++----- .../storage/remove/BlockAsDirRemoverTest.java | 4 +-- .../storage/write/BlockAsDirWriterTest.java | 6 ++--- .../block/server/util/TestConfigUtil.java | 26 ++++++++----------- server/src/test/resources/test.app.properties | 1 + 10 files changed, 28 insertions(+), 39 deletions(-) create mode 100644 server/src/test/resources/test.app.properties diff --git a/server/src/main/java/com/hedera/block/server/BlockStreamService.java b/server/src/main/java/com/hedera/block/server/BlockStreamService.java index 3e4c2ea07..b9fcbb088 100644 --- a/server/src/main/java/com/hedera/block/server/BlockStreamService.java +++ b/server/src/main/java/com/hedera/block/server/BlockStreamService.java @@ -139,6 +139,7 @@ void subscribeBlockStream( @NonNull final var streamObserver = new ConsumerStreamResponseObserver( + // TODO, send the context anyway. blockNodeContext.configuration().getConfigData(ConsumerConfig.class), Clock.systemDefaultZone(), streamMediator, diff --git a/server/src/main/java/com/hedera/block/server/Server.java b/server/src/main/java/com/hedera/block/server/Server.java index c19b6d3f4..53dd1e0fa 100644 --- a/server/src/main/java/com/hedera/block/server/Server.java +++ b/server/src/main/java/com/hedera/block/server/Server.java @@ -54,10 +54,6 @@ public static void main(final String[] args) { // init context, metrics, and configuration. @NonNull final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); - // Set the global configuration - // @NonNull final Config config = Config.create(); - // Config.global(config); - @NonNull final ServiceStatus serviceStatus = new ServiceStatusImpl(); @NonNull diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java index f8541ad0b..835d92c00 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java @@ -26,7 +26,8 @@ /** * Use this configuration across the persistent storage package * - * @param rootPath provides the root path for saving block data + * @param rootPath provides the root path for saving block data, if you want to override it need to set + * it as persistence.storage.rootPath */ @ConfigData("persistence.storage") public record PersistenceStorageConfig(@ConfigProperty(defaultValue = "") String rootPath) { diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index 9b3fb1085..d11d799f8 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -95,10 +95,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - testConfig = new PersistenceStorageConfig(testPath.toString()); blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext( + TestConfigUtil.getTestBlockNodeContext( Map.of("persistence.storage.rootPath", testPath.toString())); + testConfig = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); } @AfterEach @@ -156,7 +156,7 @@ public void testSubscribeBlockStream() throws IOException { final ServiceStatus serviceStatus = new ServiceStatusImpl(); serviceStatus.setWebServer(webServer); - final BlockNodeContext blockNodeContext = TestConfigUtil.getSpyBlockNodeContext(); + final BlockNodeContext blockNodeContext = TestConfigUtil.getTestBlockNodeContext(); final var streamMediator = LiveStreamMediatorBuilder.newBuilder(blockWriter, blockNodeContext, serviceStatus) @@ -603,7 +603,7 @@ private BlockStreamService buildBlockStreamService( final ServiceStatus serviceStatus) throws IOException { - final BlockNodeContext blockNodeContext = TestConfigUtil.getSpyBlockNodeContext(); + final BlockNodeContext blockNodeContext = TestConfigUtil.getTestBlockNodeContext(); return new BlockStreamService( new ItemAckBuilder(), streamMediator, blockReader, serviceStatus, blockNodeContext); diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index 700715590..1df1b4266 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -83,10 +83,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - config = new PersistenceStorageConfig(testPath.toString()); blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext( + TestConfigUtil.getTestBlockNodeContext( Map.of("persistence.storage.rootPath", testPath.toString())); + config = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); } @AfterEach diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java index 19ee05d42..550796e34 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java @@ -63,15 +63,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - // final Map testProperties = Map.of(JUNIT, testPath.toString()); - // final ConfigSource testConfigSource = - // MapConfigSource.builder().map(testProperties).build(); - // testConfig = Config.builder(testConfigSource).build(); - - config = new PersistenceStorageConfig(testPath.toString()); blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext( + TestConfigUtil.getTestBlockNodeContext( Map.of("persistence.storage.rootPath", testPath.toString())); + config = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); } @AfterEach @@ -94,7 +89,6 @@ public void testReadBlockDoesNotExist() throws IOException { public void testReadPermsRepairSucceeded() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); - // final BlockNodeContext blockNodeContext = BlockNodeContextFactory.create(); final BlockWriter blockWriter = BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build(); for (BlockItem blockItem : blockItems) { diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java index 596d404f6..b324a0b4e 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java @@ -54,10 +54,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - testConfig = new PersistenceStorageConfig(testPath.toString()); blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext( + TestConfigUtil.getTestBlockNodeContext( Map.of("persistence.storage.rootPath", testPath.toString())); + testConfig = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); } @Test diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java index 150b2c653..18d123503 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java @@ -62,10 +62,10 @@ public void setUp() throws IOException { testPath = Files.createTempDirectory(TEMP_DIR); LOGGER.log(System.Logger.Level.INFO, "Created temp directory: " + testPath.toString()); - testConfig = new PersistenceStorageConfig(testPath.toString()); blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext( + TestConfigUtil.getTestBlockNodeContext( Map.of(PERSISTENCE_STORAGE_ROOT_PATH_KEY, testPath.toString())); + testConfig = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); } @AfterEach @@ -83,7 +83,7 @@ public void testConstructorWithInvalidPath() throws IOException { Map.of(PERSISTENCE_STORAGE_ROOT_PATH_KEY, "invalid-path"); final BlockNodeContext blockNodeContext = - TestConfigUtil.getSpyBlockNodeContext(testProperties); + TestConfigUtil.getTestBlockNodeContext(testProperties); assertThrows( IllegalArgumentException.class, diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java index 17c7863e1..32bf6138b 100644 --- a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -16,8 +16,6 @@ package com.hedera.block.server.util; -import static org.mockito.Mockito.spy; - import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.config.TestConfigBuilder; @@ -29,18 +27,16 @@ import java.nio.file.Path; import java.util.Collections; import java.util.Map; -import org.mockito.Mockito; public class TestConfigUtil { + + public static final String TEST_APP_PROPERTIES_FILE = "test.app.properties"; + private TestConfigUtil() {} @NonNull - public static BlockNodeContext getSpyBlockNodeContext(Map customProperties) - throws IOException { - // If customProperties is null, assign it an empty map - if (customProperties == null) { - customProperties = Collections.emptyMap(); - } + public static BlockNodeContext getTestBlockNodeContext( + @NonNull Map customProperties) throws IOException { // we still use the BlockNodeContextFactory to create the BlockNodeContext temporally, // but we will replace the configuration with a test configuration @@ -50,7 +46,8 @@ public static BlockNodeContext getSpyBlockNodeContext(Map custom // create test configuration TestConfigBuilder testConfigBuilder = new TestConfigBuilder(true) - .withSource(new ClasspathFileConfigSource(Path.of("app.properties"))); + .withSource( + new ClasspathFileConfigSource(Path.of(TEST_APP_PROPERTIES_FILE))); for (Map.Entry entry : customProperties.entrySet()) { String key = entry.getKey(); @@ -62,12 +59,11 @@ public static BlockNodeContext getSpyBlockNodeContext(Map custom Configuration testConfiguration = testConfigBuilder.getOrCreateConfig(); - BlockNodeContext spyBlockNodeContext = spy(blockNodeContext); - Mockito.lenient().when(spyBlockNodeContext.configuration()).thenReturn(testConfiguration); - return spyBlockNodeContext; + return new BlockNodeContext( + blockNodeContext.metrics(), blockNodeContext.metricsService(), testConfiguration); } - public static BlockNodeContext getSpyBlockNodeContext() throws IOException { - return getSpyBlockNodeContext(null); + public static BlockNodeContext getTestBlockNodeContext() throws IOException { + return getTestBlockNodeContext(Collections.emptyMap()); } } diff --git a/server/src/test/resources/test.app.properties b/server/src/test/resources/test.app.properties new file mode 100644 index 000000000..7356a8e80 --- /dev/null +++ b/server/src/test/resources/test.app.properties @@ -0,0 +1 @@ +prometheus.endpointEnabled=false From 9d454db39adefdc96c577ffe55062dc3e5fbc149 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 11:31:31 -0600 Subject: [PATCH 12/23] refactored BlockStreamService to send the whole context instead of just the ConsumerConfig For now, only the config is useful but in the future we will include metrics as well Signed-off-by: Alfredo Gutierrez --- .../block/server/BlockStreamService.java | 4 +-- .../ConsumerStreamResponseObserver.java | 10 ++++-- .../ConsumerStreamResponseObserverTest.java | 30 ++++++++++++----- .../mediator/LiveStreamMediatorImplTest.java | 33 ++++++++++++------- .../ProducerBlockItemObserverTest.java | 16 ++++++--- .../block/server/util/TestConfigUtil.java | 4 ++- 6 files changed, 66 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/BlockStreamService.java b/server/src/main/java/com/hedera/block/server/BlockStreamService.java index b9fcbb088..8b2a0a43b 100644 --- a/server/src/main/java/com/hedera/block/server/BlockStreamService.java +++ b/server/src/main/java/com/hedera/block/server/BlockStreamService.java @@ -21,7 +21,6 @@ import com.google.protobuf.Descriptors; import com.hedera.block.server.config.BlockNodeContext; -import com.hedera.block.server.consumer.ConsumerConfig; import com.hedera.block.server.consumer.ConsumerStreamResponseObserver; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.StreamMediator; @@ -139,8 +138,7 @@ void subscribeBlockStream( @NonNull final var streamObserver = new ConsumerStreamResponseObserver( - // TODO, send the context anyway. - blockNodeContext.configuration().getConfigData(ConsumerConfig.class), + blockNodeContext, Clock.systemDefaultZone(), streamMediator, subscribeStreamResponseObserver); diff --git a/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java b/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java index d257c0e2d..91a9f6cc5 100644 --- a/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java +++ b/server/src/main/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserver.java @@ -19,6 +19,7 @@ import static com.hedera.block.protos.BlockStreamService.BlockItem; import static com.hedera.block.protos.BlockStreamService.SubscribeStreamResponse; +import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.SubscriptionHandler; import com.lmax.disruptor.EventHandler; @@ -66,14 +67,14 @@ public class ConsumerStreamResponseObserver * SubscribeStreamResponse events from the Disruptor and passing them to the downstream consumer * via the subscribeStreamResponseObserver. * - * @param consumerConfig contains the Consumer configuration settings + * @param context contains the context with metrics and configuration for the application * @param producerLivenessClock the clock to use to determine the producer liveness * @param subscriptionHandler the subscription handler to use to manage the subscription * lifecycle * @param subscribeStreamResponseObserver the observer to use to send responses to the consumer */ public ConsumerStreamResponseObserver( - @NonNull final ConsumerConfig consumerConfig, + @NonNull final BlockNodeContext context, @NonNull final InstantSource producerLivenessClock, @NonNull final SubscriptionHandler> @@ -81,7 +82,10 @@ public ConsumerStreamResponseObserver( @NonNull final StreamObserver subscribeStreamResponseObserver) { - this.timeoutThresholdMillis = consumerConfig.timeoutThresholdMillis(); + this.timeoutThresholdMillis = + context.configuration() + .getConfigData(ConsumerConfig.class) + .timeoutThresholdMillis(); this.subscriptionHandler = subscriptionHandler; // The ServerCallStreamObserver can be configured with Runnable handlers to diff --git a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java index d7ec7561e..5fd2c44bc 100644 --- a/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/consumer/ConsumerStreamResponseObserverTest.java @@ -20,12 +20,16 @@ import static com.hedera.block.server.util.PersistTestUtils.generateBlockItems; import static org.mockito.Mockito.*; +import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.StreamMediator; +import com.hedera.block.server.util.TestConfigUtil; import io.grpc.stub.ServerCallStreamObserver; import io.grpc.stub.StreamObserver; +import java.io.IOException; import java.time.InstantSource; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -44,7 +48,15 @@ public class ConsumerStreamResponseObserverTest { @Mock private ServerCallStreamObserver serverCallStreamObserver; @Mock private InstantSource testClock; - final ConsumerConfig consumerConfig = new ConsumerConfig(TIMEOUT_THRESHOLD_MILLIS); + final BlockNodeContext testContext; + + public ConsumerStreamResponseObserverTest() throws IOException { + this.testContext = + TestConfigUtil.getTestBlockNodeContext( + Map.of( + TestConfigUtil.CONSUMER_TIMEOUT_THRESHOLD_KEY, + String.valueOf(TIMEOUT_THRESHOLD_MILLIS))); + } @Test public void testProducerTimeoutWithinWindow() { @@ -53,7 +65,7 @@ public void testProducerTimeoutWithinWindow() { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, responseStreamObserver); + testContext, testClock, streamMediator, responseStreamObserver); final BlockHeader blockHeader = BlockHeader.newBuilder().setBlockNumber(1).build(); final BlockItem blockItem = BlockItem.newBuilder().setHeader(blockHeader).build(); @@ -80,7 +92,7 @@ public void testProducerTimeoutOutsideWindow() throws InterruptedException { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, responseStreamObserver); + testContext, testClock, streamMediator, responseStreamObserver); consumerBlockItemObserver.onEvent(objectEvent, 0, true); verify(streamMediator).unsubscribe(consumerBlockItemObserver); @@ -94,7 +106,7 @@ public void testHandlersSetOnObserver() throws InterruptedException { when(testClock.millis()).thenReturn(TEST_TIME, TEST_TIME + TIMEOUT_THRESHOLD_MILLIS); new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); verify(serverCallStreamObserver, timeout(50).times(1)).setOnCloseHandler(any()); verify(serverCallStreamObserver, timeout(50).times(1)).setOnCancelHandler(any()); @@ -105,7 +117,7 @@ public void testResponseNotPermittedAfterCancel() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); final List blockItems = generateBlockItems(1); final SubscribeStreamResponse subscribeStreamResponse = @@ -130,7 +142,7 @@ public void testResponseNotPermittedAfterClose() { final TestConsumerStreamResponseObserver consumerStreamResponseObserver = new TestConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); final List blockItems = generateBlockItems(1); final SubscribeStreamResponse subscribeStreamResponse = @@ -159,7 +171,7 @@ public void testConsumerNotToSendBeforeBlockHeader() { final var consumerBlockItemObserver = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, responseStreamObserver); + testContext, testClock, streamMediator, responseStreamObserver); // Send non-header BlockItems to validate that the observer does not send them for (int i = 1; i <= 10; i++) { @@ -196,12 +208,12 @@ public void testConsumerNotToSendBeforeBlockHeader() { private static class TestConsumerStreamResponseObserver extends ConsumerStreamResponseObserver { public TestConsumerStreamResponseObserver( - ConsumerConfig consumerConfig, + BlockNodeContext context, InstantSource producerLivenessClock, StreamMediator> subscriptionHandler, StreamObserver subscribeStreamResponseObserver) { super( - consumerConfig, + context, producerLivenessClock, subscriptionHandler, subscribeStreamResponseObserver); diff --git a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java index cf1fbfb52..1d8eb1774 100644 --- a/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java +++ b/server/src/test/java/com/hedera/block/server/mediator/LiveStreamMediatorImplTest.java @@ -28,6 +28,7 @@ import com.hedera.block.server.consumer.ConsumerStreamResponseObserver; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.persistence.storage.write.BlockWriter; +import com.hedera.block.server.util.TestConfigUtil; import com.lmax.disruptor.EventHandler; import edu.umd.cs.findbugs.annotations.NonNull; import io.grpc.stub.ServerCallStreamObserver; @@ -35,6 +36,7 @@ import java.io.IOException; import java.time.InstantSource; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -62,6 +64,15 @@ public class LiveStreamMediatorImplTest { private static final int testTimeout = 200; private final ConsumerConfig consumerConfig = new ConsumerConfig(TIMEOUT_THRESHOLD_MILLIS); + private final BlockNodeContext testContext; + + public LiveStreamMediatorImplTest() throws IOException { + this.testContext = + TestConfigUtil.getTestBlockNodeContext( + Map.of( + TestConfigUtil.CONSUMER_TIMEOUT_THRESHOLD_KEY, + String.valueOf(TIMEOUT_THRESHOLD_MILLIS))); + } @Test public void testUnsubscribeEach() throws InterruptedException, IOException { @@ -142,15 +153,15 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var concreteObserver1 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver1); + testContext, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver2); + testContext, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver3); + testContext, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); @@ -199,15 +210,15 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var concreteObserver1 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver1); + testContext, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver2); + testContext, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver3); + testContext, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); @@ -235,7 +246,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); streamMediator.subscribe(testConsumerBlockItemObserver); assertTrue(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -271,7 +282,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); streamMediator.subscribe(testConsumerBlockItemObserver); assertTrue(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -337,7 +348,7 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) .build(); final var testConsumerBlockItemObserver = new TestConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, serverCallStreamObserver); + testContext, testClock, streamMediator, serverCallStreamObserver); // Confirm the observer is not subscribed assertFalse(streamMediator.isSubscribed(testConsumerBlockItemObserver)); @@ -351,12 +362,12 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) private static class TestConsumerStreamResponseObserver extends ConsumerStreamResponseObserver { public TestConsumerStreamResponseObserver( - ConsumerConfig consumerConfig, + BlockNodeContext context, final InstantSource producerLivenessClock, final StreamMediator> streamMediator, final StreamObserver responseStreamObserver) { - super(consumerConfig, producerLivenessClock, streamMediator, responseStreamObserver); + super(context, producerLivenessClock, streamMediator, responseStreamObserver); } @NonNull diff --git a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java index ad0d58d5e..b8fb861f7 100644 --- a/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java +++ b/server/src/test/java/com/hedera/block/server/producer/ProducerBlockItemObserverTest.java @@ -20,6 +20,7 @@ import static com.hedera.block.protos.BlockStreamService.PublishStreamResponse.ItemAcknowledgement; import static com.hedera.block.server.producer.Util.getFakeHash; import static com.hedera.block.server.util.PersistTestUtils.generateBlockItems; +import static com.hedera.block.server.util.TestConfigUtil.CONSUMER_TIMEOUT_THRESHOLD_KEY; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.*; @@ -36,11 +37,13 @@ import com.hedera.block.server.mediator.LiveStreamMediatorBuilder; import com.hedera.block.server.mediator.StreamMediator; import com.hedera.block.server.persistence.storage.write.BlockWriter; +import com.hedera.block.server.util.TestConfigUtil; import io.grpc.stub.StreamObserver; import java.io.IOException; import java.security.NoSuchAlgorithmException; import java.time.InstantSource; import java.util.List; +import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; @@ -109,22 +112,27 @@ blockWriter, blockNodeContext, new ServiceStatusImpl()) // Mock a clock with 2 different return values in response to anticipated // millis() calls. Here the second call will always be inside the timeout window. - ConsumerConfig consumerConfig = new ConsumerConfig(100); + final BlockNodeContext testContext = + TestConfigUtil.getTestBlockNodeContext( + Map.of(CONSUMER_TIMEOUT_THRESHOLD_KEY, "100")); + final ConsumerConfig consumerConfig = + testContext.configuration().getConfigData(ConsumerConfig.class); + long TEST_TIME = 1_719_427_664_950L; when(testClock.millis()) .thenReturn(TEST_TIME, TEST_TIME + consumerConfig.timeoutThresholdMillis()); final var concreteObserver1 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver1); + testContext, testClock, streamMediator, streamObserver1); final var concreteObserver2 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver2); + testContext, testClock, streamMediator, streamObserver2); final var concreteObserver3 = new ConsumerStreamResponseObserver( - consumerConfig, testClock, streamMediator, streamObserver3); + testContext, testClock, streamMediator, streamObserver3); // Set up the subscribers streamMediator.subscribe(concreteObserver1); diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java index 32bf6138b..d65845348 100644 --- a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -30,7 +30,9 @@ public class TestConfigUtil { - public static final String TEST_APP_PROPERTIES_FILE = "test.app.properties"; + public static final String CONSUMER_TIMEOUT_THRESHOLD_KEY = "consumer.timeoutThresholdMillis"; + + private static final String TEST_APP_PROPERTIES_FILE = "test.app.properties"; private TestConfigUtil() {} From e4a8ebfce251cbdf7dd4d50b163356b4a254f433 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 11:31:42 -0600 Subject: [PATCH 13/23] style fix Signed-off-by: Alfredo Gutierrez --- .../server/persistence/storage/PersistenceStorageConfig.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java index 835d92c00..73d5b738a 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java @@ -26,8 +26,8 @@ /** * Use this configuration across the persistent storage package * - * @param rootPath provides the root path for saving block data, if you want to override it need to set - * it as persistence.storage.rootPath + * @param rootPath provides the root path for saving block data, if you want to override it need to + * set it as persistence.storage.rootPath */ @ConfigData("persistence.storage") public record PersistenceStorageConfig(@ConfigProperty(defaultValue = "") String rootPath) { From 3ba979ab6d133109ae505d5304478df721a40dd9 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 11:49:29 -0600 Subject: [PATCH 14/23] more improvements to README.md Signed-off-by: Alfredo Gutierrez --- README.md | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 13e6c4444..f64a40e7a 100644 --- a/README.md +++ b/README.md @@ -30,16 +30,30 @@ Please do not file a public ticket mentioning the vulnerability. Refer to the se --- -# Configuration +# Usage + +## Configuration | Environment Variable | Description | Default Value | |---------------------------------|---------------------------------------------------------------------------------------------------------------|---------------| | persistence.storage.rootPath | The root path for the storage, if not provided will attempt to create a `data` on the working dir of the app. | ./data | | consumer.timeoutThresholdMillis | Time to wait for subscribers before disconnecting in milliseconds | 1500 | -3) To run in debug mode: + + +# Staring locally: +```bash +./gradlew run +``` + +In debug mode, you can attach a debugger to the port 5005. +```bash +./gradlew run --debug-jvm +``` + +Also you can run on docker locally: ```bash -./gradlew run # ./gradlew run --debug-jvm to run in debug mode +./gradlew startDockerContainer ``` # Running Tests From faf74d5ef52c40cf4b8b71b8aed2fc9d0658d9a3 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 11:50:21 -0600 Subject: [PATCH 15/23] fixed Typo Signed-off-by: Alfredo Gutierrez --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f64a40e7a..9828504f5 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,7 @@ Please do not file a public ticket mentioning the vulnerability. Refer to the se -# Staring locally: +# Starting locally: ```bash ./gradlew run ``` From 79014d29c674cb1d679cac7b6b771d2f7b1af520 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 12:05:56 -0600 Subject: [PATCH 16/23] removed leftover comments Signed-off-by: Alfredo Gutierrez --- .../java/com/hedera/block/server/BlockStreamServiceTest.java | 1 - .../server/persistence/storage/read/BlockAsDirReaderTest.java | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index 1df1b4266..fac2b10a0 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -28,7 +28,6 @@ import com.google.protobuf.Descriptors; import com.hedera.block.server.config.BlockNodeContext; -// import com.hedera.block.server.config.BlockNodeContextFactory; import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.StreamMediator; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java index 550796e34..8b624d452 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java @@ -51,10 +51,9 @@ public class BlockAsDirReaderTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); private static final String TEMP_DIR = "block-node-unit-test-dir"; - // private static final String JUNIT = "my-junit-test"; private Path testPath; - // private Config testConfig; + private BlockNodeContext blockNodeContext; private PersistenceStorageConfig config; From 759f3c8c46f56ccee3a777f7f7ff69706f4566af Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 12:06:47 -0600 Subject: [PATCH 17/23] delete app.properties in test resources in favor of test.app.properties that is more descriptive. Signed-off-by: Alfredo Gutierrez --- server/src/test/resources/app.properties | 1 - 1 file changed, 1 deletion(-) delete mode 100644 server/src/test/resources/app.properties diff --git a/server/src/test/resources/app.properties b/server/src/test/resources/app.properties deleted file mode 100644 index 7356a8e80..000000000 --- a/server/src/test/resources/app.properties +++ /dev/null @@ -1 +0,0 @@ -prometheus.endpointEnabled=false From 4a720e1b32b0cc45d010c869e3072380d21c40f4 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 13:31:15 -0600 Subject: [PATCH 18/23] return to app.properties Signed-off-by: Alfredo Gutierrez --- .../test/java/com/hedera/block/server/util/TestConfigUtil.java | 2 +- .../src/test/resources/{test.app.properties => app.properties} | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) rename server/src/test/resources/{test.app.properties => app.properties} (58%) diff --git a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java index d65845348..30f3e1219 100644 --- a/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java +++ b/server/src/test/java/com/hedera/block/server/util/TestConfigUtil.java @@ -32,7 +32,7 @@ public class TestConfigUtil { public static final String CONSUMER_TIMEOUT_THRESHOLD_KEY = "consumer.timeoutThresholdMillis"; - private static final String TEST_APP_PROPERTIES_FILE = "test.app.properties"; + private static final String TEST_APP_PROPERTIES_FILE = "app.properties"; private TestConfigUtil() {} diff --git a/server/src/test/resources/test.app.properties b/server/src/test/resources/app.properties similarity index 58% rename from server/src/test/resources/test.app.properties rename to server/src/test/resources/app.properties index 7356a8e80..ff6694edb 100644 --- a/server/src/test/resources/test.app.properties +++ b/server/src/test/resources/app.properties @@ -1 +1,2 @@ +# Test Properties File prometheus.endpointEnabled=false From ed109adc039c28b00327079d47c07c0bebc04990 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 13:53:18 -0600 Subject: [PATCH 19/23] removed unneeded exports on module-info.java Signed-off-by: Alfredo Gutierrez --- server/src/main/java/module-info.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index 6f4c805d1..895ff1c28 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -2,11 +2,6 @@ /** Runtime module of the server. */ module com.hedera.block.server { - exports com.hedera.block.server.consumer to - com.swirlds.config.impl; - exports com.hedera.block.server.persistence.storage to - com.swirlds.config.impl; - requires com.hedera.block.protos; requires com.google.protobuf; requires com.lmax.disruptor; From 1f5713fbf0c56b54fe892e0ea4b8b7c624c77a16 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 15:09:47 -0600 Subject: [PATCH 20/23] Refactored Util to FileUtils on persistence package. Added the `createPathIfNotExists` that is used in both the PersistenceStorageConfig and BlockAsDirWriter. Refactored needed tests. Signed-off-by: Alfredo Gutierrez --- .../storage/{Util.java => FileUtils.java} | 26 ++++++++++++++-- .../storage/PersistenceStorageConfig.java | 26 +++++++++++----- .../storage/read/BlockAsDirReaderBuilder.java | 8 ++--- .../storage/write/BlockAsDirWriter.java | 23 +++----------- .../write/BlockAsDirWriterBuilder.java | 11 ++++--- .../block/server/BlockStreamServiceIT.java | 8 ++--- .../storage/PersistenceStorageConfigTest.java | 31 +++++++++++++++++-- .../storage/read/BlockAsDirReaderTest.java | 4 +-- .../storage/remove/BlockAsDirRemoverTest.java | 6 ++-- .../storage/write/BlockAsDirWriterTest.java | 23 ++++---------- 10 files changed, 98 insertions(+), 68 deletions(-) rename server/src/main/java/com/hedera/block/server/persistence/storage/{Util.java => FileUtils.java} (63%) diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/Util.java b/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java similarity index 63% rename from server/src/main/java/com/hedera/block/server/persistence/storage/Util.java rename to server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java index 5dec622c0..78dd02b8f 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/Util.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java @@ -17,14 +17,20 @@ package com.hedera.block.server.persistence.storage; import edu.umd.cs.findbugs.annotations.NonNull; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; import java.nio.file.attribute.PosixFilePermissions; import java.util.Set; -/** Util methods provide common functionality for the storage package. */ -public final class Util { - private Util() {} +/** FileUtils methods provide common functionality for the storage package. */ +public final class FileUtils { + + private static final System.Logger LOGGER = System.getLogger(FileUtils.class.getName()); + + private FileUtils() {} /** * Default file permissions defines the file and directory for the storage package. @@ -42,4 +48,18 @@ private Util() {} PosixFilePermission.GROUP_EXECUTE, PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_EXECUTE)); + + public static void createPathIfNotExists( + @NonNull final Path blockNodePath, + @NonNull final System.Logger.Level logLevel, + @NonNull FileAttribute> perms) + throws IOException { + // Initialize the Block directory if it does not exist + if (Files.notExists(blockNodePath)) { + Files.createDirectory(blockNodePath, perms); + LOGGER.log(logLevel, "Created block node root directory: " + blockNodePath); + } else { + LOGGER.log(logLevel, "Using existing block node root directory: " + blockNodePath); + } + } } diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java index 73d5b738a..eb91065f2 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java @@ -31,21 +31,31 @@ */ @ConfigData("persistence.storage") public record PersistenceStorageConfig(@ConfigProperty(defaultValue = "") String rootPath) { + /** * Constructor to set the default root path if not provided, it will be set to the data * directory in the current working directory */ public PersistenceStorageConfig { + // verify rootPath prop + Path path = Path.of(rootPath); if (rootPath.isEmpty()) { - Path defaultPath = Paths.get(rootPath).toAbsolutePath().resolve("data"); - if (!Files.exists(defaultPath)) { - try { - Files.createDirectories(defaultPath); - } catch (IOException e) { - throw new RuntimeException(e); - } + path = Paths.get(rootPath).toAbsolutePath().resolve("data"); + } + // Check if absolute + if (!path.isAbsolute()) { + throw new IllegalArgumentException(rootPath + " Root path must be absolute"); + } + // Create Directory if it does not exist + if (Files.notExists(path)) { + try { + FileUtils.createPathIfNotExists( + path, System.Logger.Level.ERROR, FileUtils.defaultPerms); + } catch (IOException e) { + throw new RuntimeException(e); } - rootPath = defaultPath.toString(); } + + rootPath = path.toString(); } } diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java index f5051949f..b61d9fe93 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderBuilder.java @@ -18,8 +18,8 @@ import static com.hedera.block.protos.BlockStreamService.Block; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import edu.umd.cs.findbugs.annotations.NonNull; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.PosixFilePermission; @@ -32,7 +32,7 @@ */ public class BlockAsDirReaderBuilder { - private FileAttribute> filePerms = Util.defaultPerms; + private FileAttribute> filePerms = FileUtils.defaultPerms; private final PersistenceStorageConfig config; private BlockAsDirReaderBuilder(@NonNull PersistenceStorageConfig config) { @@ -56,8 +56,8 @@ public static BlockAsDirReaderBuilder newBuilder(@NonNull PersistenceStorageConf * and directories. * *

By default, the block reader will use the permissions defined in {@link - * Util#defaultPerms}. This method is primarily used for testing purposes. Default values should - * be sufficient for production use. + * FileUtils#defaultPerms}. This method is primarily used for testing purposes. Default values + * should be sufficient for production use. * * @param filePerms the file permissions to use when managing block files and directories. * @return a block reader builder configured with required parameters. diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java index 1dedf6500..09dd83dc1 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java @@ -21,6 +21,7 @@ import com.hedera.block.server.config.BlockNodeContext; import com.hedera.block.server.metrics.MetricsService; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; import com.hedera.block.server.persistence.storage.remove.BlockRemover; import edu.umd.cs.findbugs.annotations.NonNull; @@ -79,12 +80,8 @@ class BlockAsDirWriter implements BlockWriter { this.blockRemover = blockRemover; this.filePerms = filePerms; - if (!blockNodeRootPath.isAbsolute()) { - throw new IllegalArgumentException(config.rootPath() + " must be an absolute path"); - } - // Initialize the block node root directory if it does not exist - createPath(blockNodeRootPath, System.Logger.Level.INFO); + FileUtils.createPathIfNotExists(blockNodeRootPath, System.Logger.Level.INFO, filePerms); this.blockNodeContext = blockNodeContext; } @@ -145,7 +142,7 @@ protected void write(@NonNull final Path blockItemFilePath, @NonNull final Block final FileOutputStream fos = new FileOutputStream(blockItemFilePath.toString())) { blockItem.writeTo(fos); LOGGER.log( - System.Logger.Level.INFO, + System.Logger.Level.DEBUG, "Successfully wrote the block item file: {0}", blockItemFilePath); } catch (IOException e) { @@ -167,7 +164,7 @@ private void resetState(@NonNull final BlockItem blockItem) throws IOException { repairPermissions(blockNodeRootPath); // Construct the path to the block directory - createPath(calculateBlockPath(), System.Logger.Level.DEBUG); + FileUtils.createPathIfNotExists(calculateBlockPath(), System.Logger.Level.DEBUG, filePerms); // Reset blockNodeFileNameIndex = 0; @@ -210,16 +207,4 @@ private Path calculateBlockItemPath() { private Path calculateBlockPath() { return blockNodeRootPath.resolve(currentBlockDir); } - - private void createPath( - @NonNull final Path blockNodePath, @NonNull final System.Logger.Level logLevel) - throws IOException { - // Initialize the Block directory if it does not exist - if (Files.notExists(blockNodePath)) { - Files.createDirectory(blockNodePath, filePerms); - LOGGER.log(logLevel, "Created block node root directory: " + blockNodePath); - } else { - LOGGER.log(logLevel, "Using existing block node root directory: " + blockNodePath); - } - } } diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java index 036ee3a9d..f0f1e8fe1 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterBuilder.java @@ -19,8 +19,8 @@ import static com.hedera.block.protos.BlockStreamService.BlockItem; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.remove.BlockAsDirRemover; import com.hedera.block.server.persistence.storage.remove.BlockRemover; import edu.umd.cs.findbugs.annotations.NonNull; @@ -38,7 +38,7 @@ public class BlockAsDirWriterBuilder { private final BlockNodeContext blockNodeContext; - private FileAttribute> filePerms = Util.defaultPerms; + private FileAttribute> filePerms = FileUtils.defaultPerms; private BlockRemover blockRemover; private BlockAsDirWriterBuilder(@NonNull final BlockNodeContext blockNodeContext) { @@ -46,7 +46,8 @@ private BlockAsDirWriterBuilder(@NonNull final BlockNodeContext blockNodeContext PersistenceStorageConfig config = blockNodeContext.configuration().getConfigData(PersistenceStorageConfig.class); - this.blockRemover = new BlockAsDirRemover(Path.of(config.rootPath()), Util.defaultPerms); + this.blockRemover = + new BlockAsDirRemover(Path.of(config.rootPath()), FileUtils.defaultPerms); } /** @@ -67,8 +68,8 @@ public static BlockAsDirWriterBuilder newBuilder( * and directories. * *

By default, the block writer will use the permissions defined in {@link - * Util#defaultPerms}. This method is primarily used for testing purposes. Default values should - * be sufficient for production use. + * FileUtils#defaultPerms}. This method is primarily used for testing purposes. Default values + * should be sufficient for production use. * * @param filePerms the file permissions to use when managing block files and directories. * @return a block writer builder configured with required parameters. diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java index d11d799f8..0fddbe65b 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceIT.java @@ -26,8 +26,8 @@ import com.hedera.block.server.data.ObjectEvent; import com.hedera.block.server.mediator.LiveStreamMediatorBuilder; import com.hedera.block.server.mediator.StreamMediator; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.remove.BlockAsDirRemover; @@ -318,7 +318,7 @@ public void testSubAndUnsubWhileStreaming() throws IOException { EventHandler>, BatchEventProcessor>> subscribers = new LinkedHashMap<>(); - final var streamMediator = buildStreamMediator(subscribers, Util.defaultPerms); + final var streamMediator = buildStreamMediator(subscribers, FileUtils.defaultPerms); final var blockStreamService = buildBlockStreamService(streamMediator, blockReader, serviceStatus); @@ -566,7 +566,7 @@ private static SubscribeStreamResponse buildSubscribeStreamResponse(BlockItem bl private BlockStreamService buildBlockStreamService() throws IOException { final var streamMediator = - buildStreamMediator(new ConcurrentHashMap<>(32), Util.defaultPerms); + buildStreamMediator(new ConcurrentHashMap<>(32), FileUtils.defaultPerms); return buildBlockStreamService(streamMediator, blockReader, serviceStatus); } @@ -581,7 +581,7 @@ private StreamMediator> buildStr // Initialize with concrete a concrete BlockReader, BlockWriter and Mediator final BlockRemover blockRemover = - new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), FileUtils.defaultPerms); final BlockWriter blockWriter = BlockAsDirWriterBuilder.newBuilder(blockNodeContext) diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java index a869b7091..bcde8e3ea 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java @@ -28,11 +28,16 @@ class PersistenceStorageConfigTest { + final String TEMP_DIR = "block-node-unit-test-dir"; + @Test - void testPersistenceStorageConfig() { + void testPersistenceStorageConfig_happyPath() throws IOException { + + Path testPath = Files.createTempDirectory(TEMP_DIR); + PersistenceStorageConfig persistenceStorageConfig = - new PersistenceStorageConfig("rootPath"); - assertEquals("rootPath", persistenceStorageConfig.rootPath()); + new PersistenceStorageConfig(testPath.toString()); + assertEquals(testPath.toString(), persistenceStorageConfig.rootPath()); } @Test @@ -46,6 +51,26 @@ void testPersistenceStorageConfig_emptyRootPath() throws IOException { assertEquals(expectedDefaultRootPath, persistenceStorageConfig.rootPath()); } + @Test + void persistenceStorageConfig_throwsExceptionForRelativePath() { + IllegalArgumentException exception = + assertThrows( + IllegalArgumentException.class, + () -> new PersistenceStorageConfig("relative/path")); + assertEquals("relative/path Root path must be absolute", exception.getMessage()); + } + + @Test + void persistenceStorageConfig_throwsRuntimeExceptionOnIOException() { + Path invalidPath = Paths.get("/invalid/path"); + + RuntimeException exception = + assertThrows( + RuntimeException.class, + () -> new PersistenceStorageConfig(invalidPath.toString())); + assertInstanceOf(IOException.class, exception.getCause()); + } + public static void deleteDirectory(Path path) throws IOException { if (!Files.exists(path)) { return; diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java index 8b624d452..a7d8deaa1 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsDirReaderTest.java @@ -25,8 +25,8 @@ import static org.mockito.Mockito.spy; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; import com.hedera.block.server.persistence.storage.write.BlockWriter; import com.hedera.block.server.util.PersistTestUtils; @@ -227,7 +227,7 @@ private void removeBlockItemReadPerms( // IOException while allowing the real setPerm() method to remain protected. private static final class TestBlockAsDirReader extends BlockAsDirReader { public TestBlockAsDirReader(PersistenceStorageConfig config) { - super(config, Util.defaultPerms); + super(config, FileUtils.defaultPerms); } @Override diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java index b324a0b4e..ec9ab0b8e 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsDirRemoverTest.java @@ -21,8 +21,8 @@ import com.hedera.block.protos.BlockStreamService.Block; import com.hedera.block.protos.BlockStreamService.BlockItem; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.write.BlockAsDirWriterBuilder; @@ -73,7 +73,7 @@ public void testRemoveNonExistentBlock() throws IOException { } // Remove a block that does not exist - final BlockRemover blockRemover = new BlockAsDirRemover(testPath, Util.defaultPerms); + final BlockRemover blockRemover = new BlockAsDirRemover(testPath, FileUtils.defaultPerms); blockRemover.remove(2); // Verify the block was not removed @@ -117,7 +117,7 @@ public void testRemoveBlockWithPermException() throws IOException { blockItems.getFirst().getHeader(), blockOpt.get().getBlockItems(0).getHeader()); // Now remove the block - blockRemover = new BlockAsDirRemover(testPath, Util.defaultPerms); + blockRemover = new BlockAsDirRemover(testPath, FileUtils.defaultPerms); blockRemover.remove(1); // Verify the block is removed diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java index 18d123503..4d01af248 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java @@ -23,8 +23,8 @@ import static org.mockito.Mockito.*; import com.hedera.block.server.config.BlockNodeContext; +import com.hedera.block.server.persistence.storage.FileUtils; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig; -import com.hedera.block.server.persistence.storage.Util; import com.hedera.block.server.persistence.storage.read.BlockAsDirReaderBuilder; import com.hedera.block.server.persistence.storage.read.BlockReader; import com.hedera.block.server.persistence.storage.remove.BlockAsDirRemover; @@ -77,19 +77,6 @@ public void tearDown() { } } - @Test - public void testConstructorWithInvalidPath() throws IOException { - final Map testProperties = - Map.of(PERSISTENCE_STORAGE_ROOT_PATH_KEY, "invalid-path"); - - final BlockNodeContext blockNodeContext = - TestConfigUtil.getTestBlockNodeContext(testProperties); - - assertThrows( - IllegalArgumentException.class, - () -> BlockAsDirWriterBuilder.newBuilder(blockNodeContext).build()); - } - @Test public void testWriterAndReaderHappyPath() throws IOException { @@ -176,7 +163,7 @@ public void testUnrecoverableIOExceptionOnWrite() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(1); final BlockRemover blockRemover = - new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), FileUtils.defaultPerms); // Use a spy to simulate an IOException when the first block item is written final BlockWriter blockWriter = @@ -220,14 +207,16 @@ public void testRemoveRootDirReadPerm() throws IOException { public void testPartialBlockRemoval() throws IOException { final List blockItems = PersistTestUtils.generateBlockItems(3); final BlockRemover blockRemover = - new BlockAsDirRemover(Path.of(testConfig.rootPath()), Util.defaultPerms); + new BlockAsDirRemover(Path.of(testConfig.rootPath()), FileUtils.defaultPerms); // Use a spy of TestBlockAsDirWriter to proxy block items to the real writer // for the first 22 block items. Then simulate an IOException on the 23rd block item // thrown from a protected write method in the real class. This should trigger the // blockRemover instance to remove the partially written block. final TestBlockAsDirWriter blockWriter = - spy(new TestBlockAsDirWriter(blockRemover, Util.defaultPerms, blockNodeContext)); + spy( + new TestBlockAsDirWriter( + blockRemover, FileUtils.defaultPerms, blockNodeContext)); for (int i = 0; i < 23; i++) { // Prepare the block writer to call the actual write method From ec5cca997f67806e363dce0e8d77d3077d185831 Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 15:24:23 -0600 Subject: [PATCH 21/23] JavaDocs improvements Signed-off-by: Alfredo Gutierrez --- .../block/server/persistence/storage/FileUtils.java | 8 ++++++++ .../persistence/storage/write/BlockAsDirWriter.java | 10 +++++----- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java b/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java index 78dd02b8f..680d15fb4 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/FileUtils.java @@ -49,6 +49,14 @@ private FileUtils() {} PosixFilePermission.OTHERS_READ, PosixFilePermission.OTHERS_EXECUTE)); + /** + * Use this to create a Dir if it does not exist with the given permissions and log the result. + * + * @param blockNodePath the path to create + * @param logLevel the log level to use + * @param perms the permissions to use when creating the directory + * @throws IOException if the directory cannot be created + */ public static void createPathIfNotExists( @NonNull final Path blockNodePath, @NonNull final System.Logger.Level logLevel, diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java index 09dd83dc1..22b10b91b 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java @@ -54,12 +54,12 @@ class BlockAsDirWriter implements BlockWriter { private final BlockNodeContext blockNodeContext; /** - * Constructor for the BlockAsDirWriter class. It initializes the BlockAsDirWriter with the - * given key, config, block remover, and file permissions. + * Use the corresponding builder to construct a new BlockAsDirWriter with the + * given parameters. * - * @param blockRemover the block remover to use to remove blocks if there is an exception while - * writing a partial block - * @param filePerms the file permissions to set on the block node root path + * @param blockRemover the block remover to use for removing blocks + * @param filePerms the file permissions to use for writing blocks + * @param blockNodeContext the block node context to use for writing blocks * @throws IOException if an error occurs while initializing the BlockAsDirWriter */ BlockAsDirWriter( From 26aa6f5e270a06c6c9e9eb2195554daa9223e29c Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 15:27:33 -0600 Subject: [PATCH 22/23] style fix Signed-off-by: Alfredo Gutierrez --- .../server/persistence/storage/write/BlockAsDirWriter.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java index 22b10b91b..df59417ed 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriter.java @@ -54,8 +54,7 @@ class BlockAsDirWriter implements BlockWriter { private final BlockNodeContext blockNodeContext; /** - * Use the corresponding builder to construct a new BlockAsDirWriter with the - * given parameters. + * Use the corresponding builder to construct a new BlockAsDirWriter with the given parameters. * * @param blockRemover the block remover to use for removing blocks * @param filePerms the file permissions to use for writing blocks From 60678e952bb19998c2c629ce447379919f1a9d8a Mon Sep 17 00:00:00 2001 From: Alfredo Gutierrez Date: Wed, 14 Aug 2024 15:34:29 -0600 Subject: [PATCH 23/23] removed left comment Signed-off-by: Alfredo Gutierrez --- .../java/com/hedera/block/server/BlockStreamServiceTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java index fac2b10a0..ad4bf1ac1 100644 --- a/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java +++ b/server/src/test/java/com/hedera/block/server/BlockStreamServiceTest.java @@ -73,7 +73,6 @@ public class BlockStreamServiceTest { private static final String TEMP_DIR = "block-node-unit-test-dir"; private Path testPath; - // private Config testConfig; private BlockNodeContext blockNodeContext; private PersistenceStorageConfig config;