addressing pr comment for configs

Signed-off-by: Atanas Atanasov <[email protected]>
hashgraph · Dec 5, 2024 · 34c1c4e · 34c1c4e
1 parent 17f7602
commit 34c1c4e
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 60 deletions.
diff --git 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/...r/src/main/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfig.java b/...r/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/...c/test/java/com/hedera/block/server/persistence/storage/PersistenceStorageConfigTest.java b/...c/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<Arguments> 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<Arguments> 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