Skip to content

Commit

Permalink
addressing comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Atanas Atanasov <[email protected]>
  • Loading branch information
@ata-nas
ata-nas committed Oct 28, 2024
1 parent f517ace commit 6811957
Show file tree
Hide file tree
Showing 6 changed files with 59 additions and 47 deletions.
22 changes: 0 additions & 22 deletions common/src/main/java/com/hedera/block/common/utils/FileUtilities.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package com.hedera.block.common.utils;

import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.File;
import java.io.IOException;
import java.lang.System.Logger;
import java.nio.file.Files;
Expand Down Expand Up @@ -162,27 +161,6 @@ public static byte[] readGzipFileUnsafe(@NonNull final Path filePath) throws IOE
}
}

/**
* Read a file and return the content as a byte array.
* <p>
* This method uses default extensions for gzip and block files.
* <p>
* This method is _unsafe_ because it reads the entire file content into
* a single byte array, which can cause memory issues, and may fail if the
* file contains a large amount of data.
*
* @param file to read bytes from
* @return byte array of the content of the file or null if the file extension is not
* supported
* @throws IOException if unable to read the file.
* @throws OutOfMemoryError if a byte array large enough to contain the
* file contents cannot be allocated (either because it exceeds MAX_INT
* bytes or exceeds available heap memory).
*/
public static byte[] readFileBytesUnsafe(@NonNull final File file) throws IOException {
return readFileBytesUnsafe(file.toPath());
}

/**
* Read a file and return the content as a byte array.
* <p>
Expand Down
43 changes: 33 additions & 10 deletions common/src/test/java/com/hedera/block/common/utils/FileUtilitiesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@
import org.junit.jupiter.params.provider.MethodSource;

class FileUtilitiesTest {
private static final String FILE_WITH_UNRECOGNIZED_EXTENSION =
"src/test/resources/nonexistent.unrecognized";

@Test
void test_createPathIfNotExists_CreatesDirIfDoesNotExist(@TempDir final Path tempDir)
throws IOException {
Expand Down Expand Up @@ -144,9 +147,9 @@ void test_readGzipFileUnsafe_ThrowsIOExceptionForInvalidGzipFile(final Path file

@ParameterizedTest
@MethodSource({"validGzipFiles", "validBlkFiles"})
void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidPath(
final Path pathToTest, final String expectedContent) throws IOException {
final byte[] actualContent = FileUtilities.readFileBytesUnsafe(pathToTest);
void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidFile(
final Path filePath, final String expectedContent) throws IOException {
final byte[] actualContent = FileUtilities.readFileBytesUnsafe(filePath);
assertThat(actualContent)
.isNotNull()
.isNotEmpty()
Expand All @@ -156,17 +159,25 @@ void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidPath(
.isEqualTo(expectedContent);
}

@Test
void test_readFileBytesUnsafe_ReturnsNullByteArrayWhenExtensionIsNotRecognized()
throws IOException {
final byte[] actualContent =
FileUtilities.readFileBytesUnsafe(Path.of(FILE_WITH_UNRECOGNIZED_EXTENSION));
assertThat(actualContent).isNull();
}

@ParameterizedTest
@MethodSource("invalidFiles")
void test_readFileBytesUnsafe_ThrowsIOExceptionForInvalidPath(final Path pathToTest) {
assertThatIOException().isThrownBy(() -> FileUtilities.readFileBytesUnsafe(pathToTest));
void test_readFileBytesUnsafe_ThrowsIOExceptionForInvalidGzipFile(final Path filePath) {
assertThatIOException().isThrownBy(() -> FileUtilities.readFileBytesUnsafe(filePath));
}

@ParameterizedTest
@MethodSource({"validGzipFiles", "validBlkFiles"})
void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidFile(
final File fileToTest, final String expectedContent) throws IOException {
final byte[] actualContent = FileUtilities.readFileBytesUnsafe(fileToTest);
void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidFileWithGivenExtension(
final Path filePath, final String expectedContent) throws IOException {
final byte[] actualContent = FileUtilities.readFileBytesUnsafe(filePath, ".blk", ".gz");
assertThat(actualContent)
.isNotNull()
.isNotEmpty()
Expand All @@ -176,10 +187,22 @@ void test_readFileBytesUnsafe_ReturnsByteArrayWithValidContentForValidFile(
.isEqualTo(expectedContent);
}

@Test
void
test_readFileBytesUnsafe_ReturnsNullByteArrayWhenExtensionIsNotRecognizedWithGivenExtension()
throws IOException {
final byte[] actualContent =
FileUtilities.readFileBytesUnsafe(
Path.of(FILE_WITH_UNRECOGNIZED_EXTENSION), ".blk", ".gz");
assertThat(actualContent).isNull();
}

@ParameterizedTest
@MethodSource("invalidFiles")
void test_readFileBytesUnsafe_ThrowsIOExceptionForInvalidFile(final File fileToTest) {
assertThatIOException().isThrownBy(() -> FileUtilities.readFileBytesUnsafe(fileToTest));
void test_readFileBytesUnsafe_ThrowsIOExceptionForInvalidGzipFileWithGivenExtension(
final Path filePath) {
assertThatIOException()
.isThrownBy(() -> FileUtilities.readFileBytesUnsafe(filePath, ".blk", ".gz"));
}

private static Stream<Arguments> validGzipFiles() {
Expand Down
11 changes: 5 additions & 6 deletions simulator/src/main/java/com/hedera/block/simulator/Constants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@
package com.hedera.block.simulator;

/** The Constants class defines the constants for the block simulator. */
public class Constants {

/** Constructor to prevent instantiation. this is only a utility class */
private Constants() {}

public final class Constants {
/** The file extension for block files. */
public static final String RECORD_EXTENSION = "blk";
public static final String RECORD_EXTENSION = ".blk";

/** postfix for gzip files */
public static final String GZ_EXTENSION = ".gz";

/** Constructor to prevent instantiation. this is only a utility class */
private Constants() {}
}
5 changes: 4 additions & 1 deletion ...ator/src/main/java/com/hedera/block/simulator/generator/BlockAsDirBlockStreamManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.hedera.block.simulator.generator;

import static com.hedera.block.simulator.Constants.GZ_EXTENSION;
import static com.hedera.block.simulator.Constants.RECORD_EXTENSION;
import static java.lang.System.Logger.Level.DEBUG;
import static java.lang.System.Logger.Level.ERROR;
import static java.lang.System.Logger.Level.INFO;
Expand Down Expand Up @@ -124,7 +126,8 @@ private void loadBlocks() throws IOException, ParseException {

for (final Path pathBlockItem : sortedBlockItems) {
final byte[] blockItemBytes =
FileUtilities.readFileBytesUnsafe(pathBlockItem);
FileUtilities.readFileBytesUnsafe(
pathBlockItem, RECORD_EXTENSION, GZ_EXTENSION);
// if null means the file is not a block item and we can skip the file.
if (blockItemBytes == null) {
continue;
Expand Down
6 changes: 5 additions & 1 deletion ...tor/src/main/java/com/hedera/block/simulator/generator/BlockAsFileBlockStreamManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.hedera.block.simulator.generator;

import static com.hedera.block.simulator.Constants.GZ_EXTENSION;
import static com.hedera.block.simulator.Constants.RECORD_EXTENSION;
import static java.lang.System.Logger.Level.DEBUG;
import static java.lang.System.Logger.Level.ERROR;
import static java.lang.System.Logger.Level.INFO;
Expand Down Expand Up @@ -109,7 +111,9 @@ private void loadBlocks() throws IOException, ParseException {

for (final Path blockPath : sortedBlockFiles) {

final byte[] blockBytes = FileUtilities.readFileBytesUnsafe(blockPath);
final byte[] blockBytes =
FileUtilities.readFileBytesUnsafe(
blockPath, RECORD_EXTENSION, GZ_EXTENSION);
// skip if block is null, usually due to SO files like .DS_STORE
if (blockBytes == null) {
continue;
Expand Down
19 changes: 12 additions & 7 deletions simulator/src/main/java/com/hedera/block/simulator/generator/BlockAsFileLargeDataSets.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

package com.hedera.block.simulator.generator;

import static com.hedera.block.simulator.Constants.GZ_EXTENSION;
import static com.hedera.block.simulator.Constants.RECORD_EXTENSION;
import static java.lang.System.Logger.Level.INFO;

import com.hedera.block.common.utils.FileUtilities;
Expand All @@ -27,8 +29,9 @@
import com.hedera.pbj.runtime.ParseException;
import com.hedera.pbj.runtime.io.buffer.Bytes;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import javax.inject.Inject;

/** A block stream manager that reads blocks from files in a directory. */
Expand Down Expand Up @@ -79,17 +82,19 @@ public Block getNextBlock() throws IOException, BlockSimulatorParsingException {
currentBlockIndex++;

final String nextBlockFileName = String.format(formatString, currentBlockIndex);
final File blockFile = new File(blockstreamPath, nextBlockFileName);

if (!blockFile.exists()) {
final Path localBlockStreamPath = Path.of(blockstreamPath).resolve(nextBlockFileName);
if (!Files.exists(localBlockStreamPath)) {
return null;
}

try {
final byte[] blockBytes = FileUtilities.readFileBytesUnsafe(blockFile);
final byte[] blockBytes =
FileUtilities.readFileBytesUnsafe(
localBlockStreamPath, RECORD_EXTENSION, GZ_EXTENSION);

LOGGER.log(INFO, "Loading block: " + blockFile.getName());
LOGGER.log(INFO, "Loading block: " + localBlockStreamPath.getFileName());

// todo blockBytes could be null, should we hande in some way or we need this method to
// fail here?
final Block block = Block.PROTOBUF.parse(Bytes.wrap(blockBytes));
LOGGER.log(INFO, "block loaded with items size= " + block.items().size());
return block;
Expand Down

0 comments on commit 6811957

Please sign in to comment.