correct unverified path resolution && adding test to assert writer wi…

…ll overwrite existing unverified block file

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

server/src/main/java/com/hedera/block/server/persistence/storage/write/AsyncBlockAsLocalFileWriter.java

@@ -20,6 +20,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.OpenOption;
 import java.nio.file.Path;
 import java.util.LinkedList;
 import java.util.List;
@@ -115,6 +116,8 @@ private BlockPersistenceResult doPersistBlock() {
                 }
             }
             // proceed to persist the items
+            // providing no {@link OpenOption} to the newOutputStream method
+            // will create the file if it does not exist or truncate it if it does
             try (final WritableStreamingData wsd = new WritableStreamingData(
                     compression.wrap(Files.newOutputStream(getResolvedUnverifiedBlockPath())))) {
                 final BlockUnparsed blockToWrite =
@@ -128,21 +131,19 @@ private BlockPersistenceResult doPersistBlock() {
         }
     }
 
-    private Path getResolvedUnverifiedBlockPath() throws IOException {
+    /**
+     * This method will resolve the path to where the unverified block must be
+     * written. We only need to resolve the path to the block. Unverified blocks
+     * can be overwritten. This path will be used to open a new output stream
+     * using {@link Files#newOutputStream(Path, OpenOption...)}. By default, if
+     * no options are provided, the file will be created if it does not exist or
+     * truncated if it does exist (effectively overwritten).
+     *
+     * @return the resolved path to the unverified block
+     */
+    private Path getResolvedUnverifiedBlockPath() {
         final Path rawUnverifiedBlockPath = blockPathResolver.resolveLiveRawUnverifiedPathToBlock(blockNumber);
-        final Path resolvedRawUnverifiedBlockPath =
-                FileUtilities.appendExtension(rawUnverifiedBlockPath, compression.getCompressionFileExtension());
-        if (Files.deleteIfExists(resolvedRawUnverifiedBlockPath)) {
-            // We should not throw, unverified blocks are allowed to be overwritten
-            // in the beginning of the task we check if the block is already persisted
-            // and verified. Those must never be overwritten! If we have reached
-            // here, we must know that the block has never been persisted before.
-            // Else we need not create the file, it will be overwritten since is
-            // unverified. If the createFile method throws an exception here,
-            // it is either a bug, or a potential race condition!
-            FileUtilities.createFile(resolvedRawUnverifiedBlockPath);
-        }
-        return resolvedRawUnverifiedBlockPath;
+        return FileUtilities.appendExtension(rawUnverifiedBlockPath, compression.getCompressionFileExtension());
     }
 
     /**

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

@@ -24,6 +24,7 @@
 import com.swirlds.metrics.api.Counter;
 import java.io.IOException;
 import java.io.OutputStream;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -114,6 +115,67 @@ void testSuccessfulWrite(final long validBlockNumber) throws Exception {
         verifySuccessfulPersistencePublish(expectedResult);
     }
 
+    /**
+     * This test aims to verify that the {@link AsyncBlockAsLocalFileWriter#call()}
+     * correctly overwrites an already existing block if the call is executed.
+     * It is important to assert this since the writer writes blocks as
+     * unverified. These blocks are ephemeral and can be overwritten if they
+     * fail verification. This test ensures that the writer can overwrite an
+     * existing  unverified block. No duplicate is found and no problems arose
+     * during write.
+     *
+     * @param validBlockNumber parameterized, valid block number
+     */
+    @Timeout(value = TEST_TIMEOUT_MILLIS, unit = TimeUnit.MILLISECONDS)
+    @ParameterizedTest
+    @MethodSource("validBlockNumbers")
+    void testSuccessfulWriteOverwriteExisting(final long validBlockNumber) throws Exception {
+        // setup
+        final List<BlockItemUnparsed> validBlock =
+                PersistTestUtils.generateBlockItemsUnparsedForWithBlockNumber(validBlockNumber);
+        final AsyncBlockWriter toTest = new AsyncBlockAsLocalFileWriter(
+                validBlockNumber,
+                blockPathResolverMock,
+                blockRemoverMock,
+                compressionMock,
+                ackHandlerMock,
+                metricsServiceMock);
+        final TransferQueue<BlockItemUnparsed> q = toTest.getQueue();
+        validBlock.forEach(q::offer);
+
+        // when
+        final Path expectedWrittenBlockFile = testTempDir.resolve(validBlockNumber + Constants.BLOCK_FILE_EXTENSION);
+        when(blockPathResolverMock.resolveLiveRawUnverifiedPathToBlock(validBlockNumber))
+                .thenReturn(expectedWrittenBlockFile);
+        when(blockPathResolverMock.existsVerifiedBlock(validBlockNumber)).thenReturn(false);
+        when(compressionMock.getCompressionFileExtension()).thenReturn("");
+        when(compressionMock.wrap(any(OutputStream.class))).thenAnswer(invocation -> invocation.getArgument(0));
+        when(metricsServiceMock.get(BlocksPersisted)).thenReturn(successfulPersistenceCounterMock);
+
+        // before call, create the expected file and pollute it with some data
+        // we expect that the writer will overwrite this file as it is writing
+        // to an unverified block file path that is permitted, since unverified
+        // blocks could fail verification so they need to be able to be overwritten
+        final String pollution = "block file is polluted";
+        Files.writeString(expectedWrittenBlockFile, pollution);
+        assertThat(expectedWrittenBlockFile)
+                .exists()
+                .isRegularFile()
+                .isReadable()
+                .hasContent(pollution);
+
+        // then (we expect overwrite to happen)
+        toTest.call();
+        assertThat(expectedWrittenBlockFile)
+                .exists()
+                .isRegularFile()
+                .isReadable()
+                .hasBinaryContent(generateByteArrayOfTestBlock(validBlock));
+        final BlockPersistenceResult expectedResult =
+                new BlockPersistenceResult(validBlockNumber, BlockPersistenceStatus.SUCCESS);
+        verifySuccessfulPersistencePublish(expectedResult);
+    }
+
     /**
      * This test aims to verify that the {@link AsyncBlockAsLocalFileWriter#call()}
      * correctly returns a successful result if the offered block is complete,