WIP address some pr comments, local file and noop readers now created…

… via static methods

Signed-off-by: Atanas Atanasov <[email protected]>

server/src/main/java/com/hedera/block/server/persistence/PersistenceInjectionModule.java

@@ -25,8 +25,8 @@
 import com.hedera.block.server.persistence.storage.path.BlockAsLocalFilePathResolver;
 import com.hedera.block.server.persistence.storage.path.BlockPathResolver;
 import com.hedera.block.server.persistence.storage.path.NoOpBlockPathResolver;
-import com.hedera.block.server.persistence.storage.read.BlockAsFileReaderBuilder;
 import com.hedera.block.server.persistence.storage.read.BlockAsLocalDirReader;
+import com.hedera.block.server.persistence.storage.read.BlockAsLocalFileReader;
 import com.hedera.block.server.persistence.storage.read.BlockReader;
 import com.hedera.block.server.persistence.storage.read.NoOpBlockReader;
 import com.hedera.block.server.persistence.storage.remove.BlockAsLocalDirRemover;
@@ -102,9 +102,9 @@ static BlockWriter<List<BlockItemUnparsed>> providesBlockWriter(
     static BlockReader<BlockUnparsed> providesBlockReader(@NonNull final PersistenceStorageConfig config) {
         final StorageType persistenceType = config.type();
         return switch (persistenceType) {
-            case BLOCK_AS_LOCAL_FILE -> BlockAsFileReaderBuilder.newBuilder().build();
+            case BLOCK_AS_LOCAL_FILE -> BlockAsLocalFileReader.newInstance();
             case BLOCK_AS_LOCAL_DIRECTORY -> BlockAsLocalDirReader.of(config, null);
-            case NO_OP -> new NoOpBlockReader();
+            case NO_OP -> NoOpBlockReader.newInstance();
         };
     }
 

server/src/main/java/com/hedera/block/server/persistence/storage/path/NoOpBlockPathResolver.java

@@ -28,9 +28,11 @@ public final class NoOpBlockPathResolver implements BlockPathResolver {
     private NoOpBlockPathResolver() {}
 
     /**
-     * This method creates and returns a new instance of {@link NoOpBlockPathResolver}.
+     * This method creates and returns a new instance of
+     * {@link NoOpBlockPathResolver}.
      *
-     * @return a new, fully initialized instance of {@link NoOpBlockPathResolver}
+     * @return a new, fully initialized instance of
+     * {@link NoOpBlockPathResolver}
      */
     public static NoOpBlockPathResolver newInstance() {
         return new NoOpBlockPathResolver();

server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsFileReader.java → server/src/main/java/com/hedera/block/server/persistence/storage/read/BlockAsLocalFileReader.java

@@ -23,7 +23,23 @@
 /**
  * A Block reader that reads block-as-file.
  */
-class BlockAsFileReader implements BlockReader<BlockUnparsed> {
+public class BlockAsLocalFileReader implements LocalBlockReader<BlockUnparsed> {
+    /**
+     * Constructor.
+     */
+    private BlockAsLocalFileReader() {}
+
+    /**
+     * This method creates and returns a new instance of
+     * {@link BlockAsLocalFileReader}.
+     *
+     * @return a new, fully initialized instance of
+     * {@link BlockAsLocalFileReader}
+     */
+    public static BlockAsLocalFileReader newInstance() {
+        return new BlockAsLocalFileReader();
+    }
+
     /**
      * {@inheritDoc}
      */

server/src/main/java/com/hedera/block/server/persistence/storage/read/NoOpBlockReader.java

@@ -26,6 +26,22 @@
  * A no-op Block reader.
  */
 public class NoOpBlockReader implements BlockReader<BlockUnparsed> {
+    /**
+     * Constructor.
+     */
+    private NoOpBlockReader() {}
+
+    /**
+     * This method creates and returns a new instance of
+     * {@link NoOpBlockReader}.
+     *
+     * @return a new, fully initialized instance of
+     * {@link NoOpBlockReader}
+     */
+    public static NoOpBlockReader newInstance() {
+        return new NoOpBlockReader();
+    }
+
     /**
      * {@inheritDoc}
      */

server/src/test/java/com/hedera/block/server/grpc/BlockAccessServiceTest.java

@@ -111,8 +111,7 @@ public void testMethods() {
     @Test
     void testSingleBlockHappyPath() throws IOException, ParseException {
 
-        final BlockReader<BlockUnparsed> blockReader =
-                BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
 
         final PbjBlockAccessService blockAccessService =
                 new PbjBlockAccessServiceProxy(serviceStatus, blockReader, blockNodeContext);

server/src/test/java/com/hedera/block/server/persistence/storage/read/BlockAsLocalDirReaderTest.java

@@ -73,8 +73,7 @@ public void setUp() throws IOException {
 
     @Test
     public void testReadBlockDoesNotExist() throws IOException, ParseException {
-        final BlockReader<BlockUnparsed> blockReader =
-                BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
         final Optional<BlockUnparsed> blockOpt = blockReader.read(10000);
         assertTrue(blockOpt.isEmpty());
     }
@@ -92,8 +91,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
         removeBlockReadPerms(1, config);
 
         // The default BlockReader will attempt to repair the permissions and should succeed
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
         final Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertFalse(blockOpt.isEmpty());
         assertEquals(10, blockOpt.get().blockItems().size());
@@ -125,8 +123,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
 
         removeBlockItemReadPerms(1, 1, config);
 
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
         assertThrows(IOException.class, () -> blockReader.read(1));
     }
 
@@ -139,8 +136,7 @@ public void testPathIsNotDirectory() throws IOException, ParseException {
         writeBlockItemToPath(blockNodeRootPath.resolve(Path.of("1")), blockItems.getFirst());
 
         // Should return empty because the path is not a directory
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
         final Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertTrue(blockOpt.isEmpty());
     }
@@ -188,8 +184,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
         blockWriter.write(blockItems);
 
         // Read the block back and confirm it's read successfully
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(config, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(config, null);
         final Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertFalse(blockOpt.isEmpty());
 

server/src/test/java/com/hedera/block/server/persistence/storage/remove/BlockAsLocalDirRemoverTest.java

@@ -72,8 +72,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
         toTest.remove(2);
 
         // Verify the block was not removed
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(testConfig, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(testConfig, null);
         final Optional<BlockUnparsed> before = blockReader.read(1);
         assertThat(before)
                 .isNotNull()

server/src/test/java/com/hedera/block/server/persistence/storage/write/BlockAsDirWriterTest.java

@@ -111,8 +111,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
         }
 
         // Confirm the block
-        BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(testConfig, null);
+        BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(testConfig, null);
         Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertFalse(blockOpt.isEmpty());
 
@@ -154,8 +153,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
         assertFalse(result.isPresent());
 
         // Confirm we're able to read 1 block item
-        BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(testConfig, null);
+        BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(testConfig, null);
         Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertFalse(blockOpt.isEmpty());
         assertEquals(1, blockOpt.get().blockItems().size());
@@ -231,8 +229,7 @@ blockNodeContext, mock(BlockRemover.class), mock(BlockPathResolver.class))
             }
         }
 
-        BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(testConfig, null);
+        BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(testConfig, null);
         Optional<BlockUnparsed> blockOpt = blockReader.read(1);
         assertFalse(blockOpt.isEmpty());
         assertEquals(10, blockOpt.get().blockItems().size());
@@ -276,8 +273,7 @@ public void testPartialBlockRemoval() throws IOException, ParseException {
         assertThrows(IOException.class, () -> blockWriter.write(List.of(blockItems.get(23))));
 
         // Verify the partially written block was removed
-        final BlockReader<BlockUnparsed> blockReader =
-            BlockAsLocalDirReader.of(testConfig, null);
+        final BlockReader<BlockUnparsed> blockReader = BlockAsLocalDirReader.of(testConfig, null);
         Optional<BlockUnparsed> blockOpt = blockReader.read(3);
         assertTrue(blockOpt.isEmpty());