From 34c1c4e9df8f5a4dee6b25884c2f7e12b5075b54 Mon Sep 17 00:00:00 2001 From: Atanas Atanasov Date: Thu, 5 Dec 2024 13:28:40 +0200 Subject: [PATCH] addressing pr comment for configs Signed-off-by: Atanas Atanasov --- server/docs/configuration.md | 4 +-- .../storage/PersistenceStorageConfig.java | 30 ++++------------ .../storage/PersistenceStorageConfigTest.java | 34 ------------------- 3 files changed, 8 insertions(+), 60 deletions(-) diff --git a/server/docs/configuration.md b/server/docs/configuration.md index 5dc5f08e6..c68f65f4b 100644 --- a/server/docs/configuration.md +++ b/server/docs/configuration.md @@ -11,8 +11,8 @@ defaults and can be left unchanged. It is recommended to browse the properties b | Environment Variable | Description | Default Value | |:---|:---|---:| -| PERSISTENCE_STORAGE_LIVE_ROOT_PATH | The root path for the live storage. The provided path must be absolute! | | -| PERSISTENCE_STORAGE_ARCHIVE_ROOT_PATH | The root path for the archive storage. The provided path must be absolute! | | +| PERSISTENCE_STORAGE_LIVE_ROOT_PATH | The root path for the live storage. | | +| PERSISTENCE_STORAGE_ARCHIVE_ROOT_PATH | The root path for the archive storage. | | | PERSISTENCE_STORAGE_TYPE | Type of the persistence storage | BLOCK_AS_LOCAL_FILE | | CONSUMER_TIMEOUT_THRESHOLD_MILLIS | Time to wait for subscribers before disconnecting in milliseconds | 1500 | | SERVICE_DELAY_MILLIS | Service shutdown delay in milliseconds | 500 | 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 f2eacfd1e..c385378b1 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 @@ -52,8 +52,7 @@ public record PersistenceStorageConfig( Path.of("hashgraph/blocknode/data/archive/").toAbsolutePath().toString(); /** - * Constructor to set the default root path if not provided, it will be set to the data - * directory in the current working directory + * Constructor. */ public PersistenceStorageConfig { Objects.requireNonNull(type); @@ -79,40 +78,23 @@ public record PersistenceStorageConfig( @NonNull private String resolvePath( final String pathToResolve, @NonNull final String defaultIfBlank, @NonNull final String semanticPathName) { - final Path normalized = getNormalizedPath(pathToResolve, defaultIfBlank, semanticPathName); + final Path normalized = getNormalizedPath(pathToResolve, defaultIfBlank); createDirectoryPath(normalized, semanticPathName); return normalized.toString(); } /** * This method normalizes a given path. If the path to normalize is blank, - * a default path is used. The normalized path must be absolute! If the - * normalized path is not absolute, an {@link IllegalArgumentException} is - * thrown. + * a default path is used. The normalized path must be absolute! * * @param pathToNormalize the path to normalize * @param defaultIfBlank the default path if the path to normalize is blank - * @param semanticPathName the semantic name of the path used for logging - * @return the normalized path * @throws IllegalArgumentException if the path to normalize is not absolute */ @NonNull - private Path getNormalizedPath( - final String pathToNormalize, - @NonNull final String defaultIfBlank, - @NonNull final String semanticPathName) { - final Path result; - if (StringUtilities.isBlank(pathToNormalize)) { - result = Path.of(defaultIfBlank).normalize().toAbsolutePath(); - } else { - result = Path.of(pathToNormalize).normalize().toAbsolutePath(); - } - - if (!result.isAbsolute()) { - throw new IllegalArgumentException("Path provided for [%s] must be absolute!".formatted(semanticPathName)); - } else { - return result; - } + private Path getNormalizedPath(final String pathToNormalize, @NonNull final String defaultIfBlank) { + final String actualToNormalize = StringUtilities.isBlank(pathToNormalize) ? defaultIfBlank : pathToNormalize; + return Path.of(actualToNormalize).normalize().toAbsolutePath(); } /** 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 3db2ded21..771bd4652 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 @@ -18,7 +18,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.from; import com.hedera.block.server.persistence.storage.PersistenceStorageConfig.StorageType; @@ -118,25 +117,6 @@ void testPersistenceStorageConfigInvalidRootPaths( invalidLiveRootPathToTest, invalidArchiveRootPathToTest, StorageType.BLOCK_AS_LOCAL_FILE)); } - /** - * This test aims to verify that the {@link PersistenceStorageConfig} class - * correctly throws an {@link IllegalArgumentException} when either the live - * or archive root paths are relative. - * - * @param invalidLiveRootPathToTest parameterized, the invalid live root - * path to test - * @param invalidArchiveRootPathToTest parameterized, the invalid archive - * root path to test - */ - @ParameterizedTest - @MethodSource({"validNonAbsolutePaths"}) - void testPersistenceStorageConfigRelativeRootPaths( - final String invalidLiveRootPathToTest, final String invalidArchiveRootPathToTest) { - assertThatIllegalArgumentException() - .isThrownBy(() -> new PersistenceStorageConfig( - invalidLiveRootPathToTest, invalidArchiveRootPathToTest, StorageType.BLOCK_AS_LOCAL_FILE)); - } - /** * All storage types dynamically provided. */ @@ -233,20 +213,6 @@ private static Stream validAbsoluteNonDefaultRootPaths() { Arguments.of(liveToTest2, liveToTest2, archiveToTest2, archiveToTest2)); } - /** - * Supplying blank is valid, both must be valid paths in order to be able - * to create the config instance. If either liveRootPath or archiveRootPath - * is invalid, we expect to fail. If we provide external paths, they must be - * absolute. - */ - private static Stream validNonAbsolutePaths() { - // these must fail, if we provide external paths, they must be absolute - final String liveToTest = "hashgraph/blocknode/data/relative/live/"; - final String archiveToTest = "hashgraph/blocknode/data/relative/archive/"; - return Stream.of( - Arguments.of("", archiveToTest), Arguments.of(liveToTest, ""), Arguments.of(liveToTest, archiveToTest)); - } - /** * Supplying blank is valid, both must be valid paths in order to be able * to create the config instance. If either liveRootPath or archiveRootPath