diff --git a/NEWS.md b/NEWS.md index 8d636822..4c49452c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,7 @@ ### Bug fixes * Fix context mix-up on data propagation ([MODELINKS-273](https://folio-org.atlassian.net/browse/MODELINKS-273)) +* Close input stream on s3 file read ([MODELINKS-278](https://folio-org.atlassian.net/browse/MODELINKS-278)) ### Tech Dept * Add missing interface `source-storage-batch` dependency in module descriptor ([MODELINKS-275](https://folio-org.atlassian.net/browse/MODELINKS-275)) diff --git a/src/main/java/org/folio/entlinks/service/authority/BulkAuthorityS3Client.java b/src/main/java/org/folio/entlinks/service/authority/BulkAuthorityS3Client.java index 770556e4..65de3c6c 100644 --- a/src/main/java/org/folio/entlinks/service/authority/BulkAuthorityS3Client.java +++ b/src/main/java/org/folio/entlinks/service/authority/BulkAuthorityS3Client.java @@ -1,6 +1,7 @@ package org.folio.entlinks.service.authority; import java.io.BufferedReader; +import java.io.IOException; import java.io.InputStreamReader; import java.util.List; import lombok.RequiredArgsConstructor; @@ -10,6 +11,9 @@ import org.springframework.retry.annotation.Retryable; import org.springframework.stereotype.Component; +/** + * Code partially generated using GitHub Copilot. + * */ @Log4j2 @Component @RequiredArgsConstructor @@ -23,10 +27,13 @@ public class BulkAuthorityS3Client { backoff = @Backoff(delayExpression = "${folio.remote-storage.retryDelayMs}")) public List readFile(String remoteFileName) { log.info("readFile::Reading lines from the file [filename: {}]", remoteFileName); - var inputStream = s3Client.read(remoteFileName); - return new BufferedReader(new InputStreamReader(inputStream)) - .lines() - .toList(); + try (var inputStream = s3Client.read(remoteFileName); + var reader = new BufferedReader(new InputStreamReader(inputStream))) { + return reader.lines().toList(); + } catch (IOException e) { + log.error("readFile::Error reading file [filename: {}]", remoteFileName, e); + throw new IllegalStateException("Error reading file: " + remoteFileName, e); + } } @Retryable( diff --git a/src/test/java/org/folio/entlinks/service/authority/BulkAuthorityS3ClientTest.java b/src/test/java/org/folio/entlinks/service/authority/BulkAuthorityS3ClientTest.java index 6ff9631d..2bce4945 100644 --- a/src/test/java/org/folio/entlinks/service/authority/BulkAuthorityS3ClientTest.java +++ b/src/test/java/org/folio/entlinks/service/authority/BulkAuthorityS3ClientTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -15,6 +16,9 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +/** + * Code partially generated using GitHub Copilot. + * */ @UnitTest @ExtendWith(MockitoExtension.class) class BulkAuthorityS3ClientTest { @@ -42,6 +46,33 @@ void readFile_ReturnsListOfStringAuthority() { assertThat(stringAuthority).contains(AUTHORITY_UUID, "Test Authority"); } + @Test + void readFile_ReturnsEmptyListWhenFileIsEmpty() { + // Arrange + var remoteFileName = "empty-file"; + var inputStream = new ByteArrayInputStream(new byte[0]); + when(s3Client.read(remoteFileName)).thenReturn(inputStream); + + // Act + var resultList = client.readFile(remoteFileName); + + // Assert + assertThat(resultList).isEmpty(); + } + + @Test + void readFile_ThrowsIllegalStateExceptionWhenIoExceptionOccurs() { + // Arrange + var remoteFileName = "error-file"; + when(s3Client.read(remoteFileName)).thenAnswer(invocation -> { + throw new IOException("Test IOException"); + }); + + // Act & Assert + var exception = assertThrows(IllegalStateException.class, () -> client.readFile(remoteFileName)); + assertThat(exception).hasMessageContaining("Error reading file: " + remoteFileName); + } + @Test void uploadErrorFiles_uploads() throws IOException { // Arrange