From 23106d330f32d60652760bb681c977dfe67d5281 Mon Sep 17 00:00:00 2001 From: Matt Peterson Date: Wed, 31 Jul 2024 12:37:24 -0600 Subject: [PATCH] fix: moved BlockWriter tests into their own file Signed-off-by: Matt Peterson --- .../persistence/storage/BlockAsDirWriter.java | 20 +- .../storage/BlockAsDirReaderTest.java | 142 +++++++++++++ ...oryTest.java => BlockAsDirWriterTest.java} | 187 ++++++++---------- 3 files changed, 231 insertions(+), 118 deletions(-) create mode 100644 server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirReaderTest.java rename server/src/test/java/com/hedera/block/server/persistence/storage/{BlockAsDirectoryTest.java => BlockAsDirWriterTest.java} (65%) diff --git a/server/src/main/java/com/hedera/block/server/persistence/storage/BlockAsDirWriter.java b/server/src/main/java/com/hedera/block/server/persistence/storage/BlockAsDirWriter.java index af52e080c..da6e8a2a2 100644 --- a/server/src/main/java/com/hedera/block/server/persistence/storage/BlockAsDirWriter.java +++ b/server/src/main/java/com/hedera/block/server/persistence/storage/BlockAsDirWriter.java @@ -140,22 +140,12 @@ private void resetState(final BlockItem blockItem) throws IOException { } private void repairPermissions(final Path path) throws IOException { - final boolean isReadable = Files.isReadable(path); final boolean isWritable = Files.isWritable(path); - if (!isWritable || !isReadable) { - if (!isWritable) { - LOGGER.log( - System.Logger.Level.ERROR, - "Block node root directory is not writable. Attempting to change the" - + " permissions."); - } - - if (!isReadable) { - LOGGER.log( - System.Logger.Level.ERROR, - "Block node root directory is not readable. Attempting to change the" - + " permissions."); - } + if (!isWritable) { + LOGGER.log( + System.Logger.Level.ERROR, + "Block node root directory is not writable. Attempting to change the" + + " permissions."); try { // Attempt to restore the permissions on the block node root directory diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirReaderTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirReaderTest.java new file mode 100644 index 000000000..87a13159e --- /dev/null +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirReaderTest.java @@ -0,0 +1,142 @@ +/* + * 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 com.hedera.block.protos.BlockStreamService.Block; +import static com.hedera.block.protos.BlockStreamService.BlockItem; +import static com.hedera.block.server.Constants.BLOCK_FILE_EXTENSION; +import static org.junit.jupiter.api.Assertions.*; + +import com.hedera.block.server.util.PersistTestUtils; +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; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +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; + + @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(); + } + + @AfterEach + public void tearDown() { + if (!TestUtils.deleteDirectory(testPath.toFile())) { + LOGGER.log( + System.Logger.Level.ERROR, + "Failed to delete temp directory: " + testPath.toString()); + } + } + + @Test + public void testReadBlockDoesNotExist() throws IOException { + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(10000); + assertTrue(blockOpt.isEmpty()); + } + + @Test + public void testReadPermsRepairSucceeded() throws IOException { + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); + for (BlockItem blockItem : blockItems) { + blockWriter.write(blockItem); + } + + // Make the block unreadable + removeBlockReadPerms(1, testConfig); + + // The default BlockReader will attempt to repair the permissions and should succeed + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(1); + assertFalse(blockOpt.isEmpty()); + assertEquals(10, blockOpt.get().getBlockItemsList().size()); + } + + @Test + public void testRemoveBlockReadPermsRepairFailed() throws IOException { + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); + for (BlockItem blockItem : blockItems) { + blockWriter.write(blockItem); + } + + // Make the block unreadable + removeBlockReadPerms(1, testConfig); + + // For this test, build the Reader with ineffective repair permissions to + // simulate a failed repair (root changed the perms, etc.) + BlockRemover blockRemover = + new BlockAsDirRemover( + Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); + BlockReader blockReader = + new BlockAsDirReader(JUNIT, testConfig, blockRemover, TestUtils.getNoPerms()); + Optional blockOpt = blockReader.read(1); + assertTrue(blockOpt.isEmpty()); + } + + @Test + public void testRemoveBlockItemReadPerms() throws IOException { + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); + for (BlockItem blockItem : blockItems) { + blockWriter.write(blockItem); + } + + removeBlockItemReadPerms(1, 1, testConfig); + + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + assertThrows(IOException.class, () -> blockReader.read(1)); + } + + static void removeBlockReadPerms(int blockNumber, final Config config) throws IOException { + final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); + Files.setPosixFilePermissions(blockPath, TestUtils.getNoRead().value()); + } + + private void removeBlockItemReadPerms(int blockNumber, int blockItem, Config config) + throws IOException { + final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); + final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); + final Path blockItemPath = blockPath.resolve(blockItem + BLOCK_FILE_EXTENSION); + Files.setPosixFilePermissions(blockItemPath, TestUtils.getNoRead().value()); + } +} diff --git a/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirectoryTest.java b/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirWriterTest.java similarity index 65% rename from server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirectoryTest.java rename to server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirWriterTest.java index beaf397d7..3d04dae9b 100644 --- a/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirectoryTest.java +++ b/server/src/test/java/com/hedera/block/server/persistence/storage/BlockAsDirWriterTest.java @@ -16,11 +16,10 @@ package com.hedera.block.server.persistence.storage; -import static com.hedera.block.protos.BlockStreamService.Block; -import static com.hedera.block.protos.BlockStreamService.BlockItem; -import static com.hedera.block.server.Constants.BLOCK_FILE_EXTENSION; +import static com.hedera.block.server.persistence.storage.BlockAsDirReaderTest.removeBlockReadPerms; import static org.junit.jupiter.api.Assertions.*; +import com.hedera.block.protos.BlockStreamService; import com.hedera.block.server.util.PersistTestUtils; import com.hedera.block.server.util.TestUtils; import io.helidon.config.Config; @@ -39,7 +38,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class BlockAsDirectoryTest { +public class BlockAsDirWriterTest { private final System.Logger LOGGER = System.getLogger(getClass().getName()); @@ -68,27 +67,36 @@ public void tearDown() { } } + @Test + public void testConstructorWithInvalidPath() { + final Map testProperties = Map.of(JUNIT, "invalid-path"); + final ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); + final Config testConfig = Config.builder(testConfigSource).build(); + assertThrows(IllegalArgumentException.class, () -> new BlockAsDirWriter(JUNIT, testConfig)); + } + @Test public void testWriterAndReaderHappyPath() throws IOException { // Write a block - List blockItems = PersistTestUtils.generateBlockItems(1); - BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); - for (BlockItem blockItem : blockItems) { + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = + new BlockAsDirWriter(JUNIT, testConfig); + for (BlockStreamService.BlockItem blockItem : blockItems) { blockWriter.write(blockItem); } // Confirm the block - BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - Optional blockOpt = blockReader.read(1); + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); boolean hasHeader = false; boolean hasBlockProof = false; boolean hasStartEvent = false; - Block block = blockOpt.get(); - for (BlockItem blockItem : block.getBlockItemsList()) { + BlockStreamService.Block block = blockOpt.get(); + for (BlockStreamService.BlockItem blockItem : block.getBlockItemsList()) { if (blockItem.hasHeader()) { hasHeader = true; } else if (blockItem.hasStateProof()) { @@ -103,86 +111,29 @@ public void testWriterAndReaderHappyPath() throws IOException { assertTrue(hasStartEvent, "Block should have a start event"); } - @Test - public void testBlockDoesNotExist() throws IOException { - BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - Optional blockOpt = blockReader.read(10000); - assertTrue(blockOpt.isEmpty()); - } - - @Test - public void testRemoveBlockReadPermsRepairSucceeded() throws IOException { - List blockItems = PersistTestUtils.generateBlockItems(1); - BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); - for (BlockItem blockItem : blockItems) { - blockWriter.write(blockItem); - } - - // Make the block unreadable - removeBlockReadPerms(1, testConfig); - - // The default BlockReader will attempt to repair the permissions and should succeed - BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - Optional blockOpt = blockReader.read(1); - assertFalse(blockOpt.isEmpty()); - assertEquals(10, blockOpt.get().getBlockItemsList().size()); - } - - @Test - public void testRemoveBlockReadPermsRepairFailed() throws IOException { - List blockItems = PersistTestUtils.generateBlockItems(1); - BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); - for (BlockItem blockItem : blockItems) { - blockWriter.write(blockItem); - } - - // Make the block unreadable - removeBlockReadPerms(1, testConfig); - - // For this test, build the Reader with ineffective repair permissions to - // simulate a failed repair (root changed the perms, etc.) - BlockRemover blockRemover = - new BlockAsDirRemover( - Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); - BlockReader blockReader = - new BlockAsDirReader(JUNIT, testConfig, blockRemover, TestUtils.getNoPerms()); - Optional blockOpt = blockReader.read(1); - assertTrue(blockOpt.isEmpty()); - } - - @Test - public void testRemoveBlockItemReadPerms() throws IOException { - List blockItems = PersistTestUtils.generateBlockItems(1); - BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); - for (BlockItem blockItem : blockItems) { - blockWriter.write(blockItem); - } - - removeBlockItemReadPerms(1, 1, testConfig); - - BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - assertThrows(IOException.class, () -> blockReader.read(1)); - } - @Test public void testRemoveBlockWritePerms() throws IOException { - List blockItems = PersistTestUtils.generateBlockItems(1); + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = + new BlockAsDirWriter(JUNIT, testConfig); // Change the permissions on the block node root directory removeRootWritePerms(testConfig); - BlockWriter blockWriter = new BlockAsDirWriter(JUNIT, testConfig); // The first BlockItem contains a header which will create a new block directory. // The BlockWriter will attempt to repair the permissions and should succeed. blockWriter.write(blockItems.getFirst()); - BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - Optional blockOpt = blockReader.read(1); + // Confirm we're able to read 1 block item + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(1); assertFalse(blockOpt.isEmpty()); assertEquals(1, blockOpt.get().getBlockItemsList().size()); assertTrue(blockOpt.get().getBlockItems(0).hasHeader()); + // Remove all permissions on the block directory and + // attempt to write the next block item removeBlockAllPerms(1, testConfig); blockWriter.write(blockItems.get(1)); @@ -191,16 +142,30 @@ public void testRemoveBlockWritePerms() throws IOException { assertFalse(blockOpt.isEmpty()); assertEquals(2, blockOpt.get().getBlockItemsList().size()); assertFalse(blockOpt.get().getBlockItems(1).hasHeader()); + + // Remove read permission on the block directory + removeBlockReadPerms(1, testConfig); + blockWriter.write(blockItems.get(2)); + + // There should now be 3 blockItems in the block + blockOpt = blockReader.read(1); + assertFalse(blockOpt.isEmpty()); + assertEquals(3, blockOpt.get().getBlockItemsList().size()); + assertFalse(blockOpt.get().getBlockItems(1).hasHeader()); } @Test - public void testRemoveBlockItemWritePerms() throws IOException { + public void testUnrecoverableIOExceptionOnWrite() throws IOException { - final List blockItems = PersistTestUtils.generateBlockItems(1); + final List blockItems = + PersistTestUtils.generateBlockItems(1); final BlockRemover blockRemover = new BlockAsDirRemover( Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); - final BlockWriter blockWriter = + + // Use a TestIOExceptionBlockAsDirWriter to simulate an IOException when the first + // block item is written + final BlockWriter blockWriter = new TestIOExceptionBlockAsDirWriter( JUNIT, testConfig, blockRemover, Util.defaultPerms, 0); @@ -210,20 +175,43 @@ public void testRemoveBlockItemWritePerms() throws IOException { } @Test - public void testConstructorWithInvalidPath() { - final Map testProperties = Map.of(JUNIT, "invalid-path"); - final ConfigSource testConfigSource = MapConfigSource.builder().map(testProperties).build(); - final Config testConfig = Config.builder(testConfigSource).build(); - assertThrows(IllegalArgumentException.class, () -> new BlockAsDirWriter(JUNIT, testConfig)); + public void testRemoveRootDirReadPerm() throws IOException { + List blockItems = PersistTestUtils.generateBlockItems(1); + BlockWriter blockWriter = + new BlockAsDirWriter(JUNIT, testConfig); + + // Write the first block item to create the block + // directory + blockWriter.write(blockItems.getFirst()); + + // Remove root dir read permissions and + // block dir read permissions + removeRootReadPerms(testConfig); + removeBlockReadPerms(1, testConfig); + + // Attempt to write the remaining block + // items + for (int i = 1; i < 10; i++) { + blockWriter.write(blockItems.get(i)); + } + + BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(1); + assertFalse(blockOpt.isEmpty()); + assertEquals(10, blockOpt.get().getBlockItemsList().size()); } @Test public void testPartialBlockRemoval() throws IOException { - final List blockItems = PersistTestUtils.generateBlockItems(3); + final List blockItems = + PersistTestUtils.generateBlockItems(3); final BlockRemover blockRemover = new BlockAsDirRemover( Path.of(testConfig.get(JUNIT).asString().get()), Util.defaultPerms); - final BlockWriter blockWriter = + + // Use a TestIOExceptionBlockAsDirWriter to simulate an IOException after + // block item 23 is written + final BlockWriter blockWriter = new TestIOExceptionBlockAsDirWriter( JUNIT, testConfig, blockRemover, Util.defaultPerms, 23); @@ -241,8 +229,9 @@ public void testPartialBlockRemoval() throws IOException { assertThrows(IOException.class, () -> blockWriter.write(blockItems.get(23))); // Verify the partially written block was removed - final BlockReader blockReader = new BlockAsDirReader(JUNIT, testConfig); - Optional blockOpt = blockReader.read(3); + final BlockReader blockReader = + new BlockAsDirReader(JUNIT, testConfig); + Optional blockOpt = blockReader.read(3); assertTrue(blockOpt.isEmpty()); // Confirm blocks 1 and 2 still exist @@ -257,10 +246,11 @@ public void testPartialBlockRemoval() throws IOException { assertEquals(2, blockOpt.get().getBlockItems(0).getHeader().getBlockNumber()); } - private void removeBlockReadPerms(int blockNumber, final Config config) throws IOException { + private void removeBlockWritePerms(final int blockNumber, final Config config) + throws IOException { final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); - Files.setPosixFilePermissions(blockPath, TestUtils.getNoRead().value()); + Files.setPosixFilePermissions(blockPath, TestUtils.getNoWrite().value()); } private void removeRootWritePerms(final Config config) throws IOException { @@ -268,11 +258,9 @@ private void removeRootWritePerms(final Config config) throws IOException { Files.setPosixFilePermissions(blockNodeRootPath, TestUtils.getNoWrite().value()); } - private void removeBlockWritePerms(final int blockNumber, final Config config) - throws IOException { + private void removeRootReadPerms(final Config config) throws IOException { final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); - final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); - Files.setPosixFilePermissions(blockPath, TestUtils.getNoWrite().value()); + Files.setPosixFilePermissions(blockNodeRootPath, TestUtils.getNoRead().value()); } private void removeBlockAllPerms(final int blockNumber, final Config config) @@ -282,14 +270,6 @@ private void removeBlockAllPerms(final int blockNumber, final Config config) Files.setPosixFilePermissions(blockPath, TestUtils.getNoPerms().value()); } - private void removeBlockItemReadPerms(int blockNumber, int blockItem, Config config) - throws IOException { - final Path blockNodeRootPath = Path.of(config.get(JUNIT).asString().get()); - final Path blockPath = blockNodeRootPath.resolve(String.valueOf(blockNumber)); - final Path blockItemPath = blockPath.resolve(blockItem + BLOCK_FILE_EXTENSION); - Files.setPosixFilePermissions(blockItemPath, TestUtils.getNoRead().value()); - } - static class TestIOExceptionBlockAsDirWriter extends BlockAsDirWriter { final int blockItemsToAdmitBeforeFailure; @@ -307,7 +287,8 @@ public TestIOExceptionBlockAsDirWriter( } @Override - void write(final Path blockItemFilePath, final BlockItem blockItem) throws IOException { + void write(final Path blockItemFilePath, final BlockStreamService.BlockItem blockItem) + throws IOException { if (currentCount < blockItemsToAdmitBeforeFailure) { super.write(blockItemFilePath, blockItem); currentCount++;