diff --git a/README.md b/README.md index 7343664a2c5255e6905b8fd4c93c0b154156ed13..c5c48801e0aebaf081d7625ef049aa4178931462 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,9 @@ ## Changelog -### 4.6.0-SNAPSHOT +### 4.7.0-SNAPSHOT + +### 4.6.0 * Update [OZGCloud License Generator](ozgcloud-common-license/readme.md) * `GrpcUtil` erweitern: mehr Schlüssel hinzufügen und Methoden, um diese Schlüssel aus gRPC-Metadaten zu extrahieren. diff --git a/ozgcloud-common-lib/pom.xml b/ozgcloud-common-lib/pom.xml index f46dc0eae17df1110398f44cf05544618009ebae..1373e1d77f30fa77cf2be275462607407e0fd3f0 100644 --- a/ozgcloud-common-lib/pom.xml +++ b/ozgcloud-common-lib/pom.xml @@ -145,6 +145,11 @@ </dependency> <!-- test --> + <dependency> + <groupId>de.ozgcloud.common</groupId> + <artifactId>ozgcloud-common-test</artifactId> + <scope>test</scope> + </dependency> <dependency> <groupId>org.assertj</groupId> <artifactId>assertj-core</artifactId> diff --git a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloader.java b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloader.java index 5eedf9f5d27de944b439ae2e5941b080a06c97fa..b992f4236a809f36e95dcf6811d6b80ad0675046 100644 --- a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloader.java +++ b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloader.java @@ -51,7 +51,8 @@ public class GrpcBinaryFileServerDownloader<T> { private final byte[] buffer = new byte[GrpcBinaryFileServerDownloader.CHUNK_SIZE]; private final AtomicBoolean started = new AtomicBoolean(false); - private final AtomicBoolean downloadInProgress = new AtomicBoolean(false); + private final AtomicBoolean downloadFinished = new AtomicBoolean(false); + private final AtomicBoolean requestFinished = new AtomicBoolean(false); private PipedInputStream inputStream; private PipedOutputStream outputStream; @@ -76,9 +77,10 @@ public class GrpcBinaryFileServerDownloader<T> { } void doStart() { + LOG.debug("Starting download."); handleSafety(this::setupStreams); taskExecutor.execute(this::startDownload); - callObserver.setOnReadyHandler(this::onReadyHandler); + callObserver.setOnReadyHandler(this::sendChunks); } void setupStreams() throws IOException { @@ -91,41 +93,66 @@ public class GrpcBinaryFileServerDownloader<T> { handleSafety(this::doDownload); } - void doDownload() throws IOException { - downloadInProgress.set(true); + void doDownload() { + LOG.debug("Downloading file content..."); downloadConsumer.accept(outputStream); - downloadInProgress.set(false); - outputStream.close(); + LOG.debug("Download completed."); + downloadFinished.set(true); + closeOutputStream(); + sendChunks(); } - synchronized void onReadyHandler() { - if (callObserver.isReady()) { - sendChunks(); - } - } - - void sendChunks() { + synchronized void sendChunks() { handleSafety(this::doSendChunks); } void doSendChunks() throws IOException { + if (requestFinished.get()) { + return; + } int bytesRead; - while (callObserver.isReady() && (bytesRead = inputStream.read(buffer)) != -1) { + while (callObserver.isReady()) { + if ((bytesRead = inputStream.read(buffer)) == -1) { + tryCompleteRequest(); + break; + } callObserver.onNext(chunkBuilder.apply(ByteString.copyFrom(buffer, 0, bytesRead))); + LOG.debug("Sent {} bytes", bytesRead); } - if (!downloadInProgress.get()) { - inputStream.close(); - callObserver.onCompleted(); + } + + void tryCompleteRequest() { + if (shouldCompleteRequest()) { + completeRequest(); } } + boolean shouldCompleteRequest() { + return downloadFinished.get() && requestFinished.compareAndSet(false, true); + } + + void completeRequest() { + LOG.debug("Complete download request"); + closeInputStream(); + callObserver.onCompleted(); + } + void handleSafety(ExceptionalRunnable runnable) { try { runnable.run(); } catch (Exception e) { - IOUtils.closeQuietly(inputStream, e1 -> LOG.error("InputStream cannot be closed.", e1)); - IOUtils.closeQuietly(outputStream, e1 -> LOG.error("OutputStream cannot be closed.", e1)); + closeOutputStream(); + closeInputStream(); throw new TechnicalException("Error occurred during downloading file content download.", e); } } + + void closeOutputStream() { + IOUtils.closeQuietly(outputStream, e -> LOG.error("OutputStream cannot be closed.", e)); + } + + void closeInputStream() { + IOUtils.closeQuietly(inputStream, e -> LOG.error("InputStream cannot be closed.", e)); + } + } \ No newline at end of file diff --git a/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloaderTest.java b/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloaderTest.java index 38bb3a5e9f607b9abfa9cd17ba7af4263e126560..6050e6ba2d06a739e87ec479956dff4fecd707b8 100644 --- a/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloaderTest.java +++ b/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/GrpcBinaryFileServerDownloaderTest.java @@ -25,6 +25,7 @@ package de.ozgcloud.common.binaryfile; import com.google.protobuf.ByteString; import de.ozgcloud.common.errorhandling.TechnicalException; +import de.ozgcloud.common.test.ReflectionTestUtils; import io.grpc.Context; import io.grpc.stub.CallStreamObserver; import lombok.SneakyThrows; @@ -42,6 +43,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.PipedInputStream; import java.io.PipedOutputStream; +import java.util.Random; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.function.Function; @@ -161,9 +163,6 @@ class GrpcBinaryFileServerDownloaderTest { @Nested class TestStartDownload { - @Mock - private Context callContext; - @Captor private ArgumentCaptor<ExceptionalRunnable> runnableCaptor; @@ -210,21 +209,6 @@ class GrpcBinaryFileServerDownloaderTest { } } - @DisplayName("On ready handler") - @Nested - class TestOnReadyHandler { - - @Test - void shouldSendChunksIfCallObserverIsReady() { - when(callObserver.isReady()).thenReturn(true); - doNothing().when(downloader).sendChunks(); - - downloader.onReadyHandler(); - - verify(downloader).sendChunks(); - } - } - @DisplayName("Send chunks") @Nested class TestSendChunks { @@ -241,7 +225,7 @@ class GrpcBinaryFileServerDownloaderTest { @SneakyThrows @Test - void shouldCallDoDownoad() { + void shouldCallDoDownload() { doNothing().when(downloader).doSendChunks(); downloader.sendChunks(); @@ -255,49 +239,181 @@ class GrpcBinaryFileServerDownloaderTest { @Mock private PipedInputStream inputStream; + @Captor + private ArgumentCaptor<ByteString> byteStringCaptor; - private final int data = 20; + private final int readBytes = 20; + private final byte[] buffer = new byte[readBytes]; + private final GrpcResponseDummy grpcResponseDummy = new GrpcResponseDummy(); + @SneakyThrows @BeforeEach void mock() { + doNothing().when(downloader).tryCompleteRequest(); + when(callObserver.isReady()).thenReturn(true); + when(inputStream.read(any())).thenReturn(readBytes, -1); setInputStreamField(inputStream); + new Random().nextBytes(buffer); + ReflectionTestUtils.setField(downloader, "buffer", buffer); + } + + @Test + void shouldCallChunkBuilder() { + doSendChunks(); + + verify(chunkBuilder).apply(byteStringCaptor.capture()); + assertThat(byteStringCaptor.getValue().toByteArray()).isEqualTo(buffer); } - @SneakyThrows @DisplayName("should send next chunk if callObserver is ready and stream already received data") @Test void shouldCallOnNext() { - when(callObserver.isReady()).thenReturn(true); - when(inputStream.read(any())).thenReturn(data, -1); + when(chunkBuilder.apply(any())).thenReturn(grpcResponseDummy); - downloader.doSendChunks(); + doSendChunks(); - verify(callObserver).onNext(any()); + verify(callObserver).onNext(grpcResponseDummy); } - @SneakyThrows @DisplayName("should call complete grpc stream if download has finished and stream has no data left") @Test - void shouldCallOnCompleted() { - setDownloadInProgressField(new AtomicBoolean(false)); + void shouldCallCompleteDownload() { + setDownloadFinishedField(true); - downloader.doSendChunks(); + doSendChunks(); - verify(callObserver).onCompleted(); + verify(downloader).tryCompleteRequest(); } @SneakyThrows + private void doSendChunks() { + downloader.doSendChunks(); + } + } + } + + @Nested + class TestTryCompleteRequest { + + @Test + void shouldCallShouldCompleteRequest() { + downloader.tryCompleteRequest(); + + verify(downloader).shouldCompleteRequest(); + } + + @Test + void shouldCallCompleteRequest() { + doReturn(true).when(downloader).shouldCompleteRequest(); + + downloader.tryCompleteRequest(); + + verify(downloader).completeRequest(); + } + + @Test + void shouldNotCallCompleteRequest() { + doReturn(false).when(downloader).shouldCompleteRequest(); + + downloader.tryCompleteRequest(); + + verify(downloader, never()).completeRequest(); + } + } + + @Nested + class TestShouldCompleteRequest { + + @Nested + class TestWhenDownloadFinished { + + @BeforeEach + void init() { + setDownloadFinishedField(true); + } + @Test - void shouldCloseInputStream() { - setDownloadInProgressField(new AtomicBoolean(false)); + void shouldReturnTrue() { + var result = downloader.shouldCompleteRequest(); - downloader.doSendChunks(); + assertThat(result).isTrue(); + } - verify(inputStream).close(); + @Test + void shouldReturnFalseIfRequestFinished() { + setRequestFinishedField(true); + + var result = downloader.shouldCompleteRequest(); + + assertThat(result).isFalse(); + } + + @Test + void shouldUpdateRequestFinished() { + downloader.shouldCompleteRequest(); + + assertThat(getRequestFinished()).isTrue(); + } + } + + @Nested + class TestWhenDownloadRunning { + + @BeforeEach + void init() { + setDownloadFinishedField(false); + } + + @Test + void shouldReturnFalse() { + var result = downloader.shouldCompleteRequest(); + + assertThat(result).isFalse(); + } + + @Test + void shouldNotUpdateRequestFinished() { + downloader.shouldCompleteRequest(); + + assertThat(getRequestFinished()).isFalse(); } } } + @Nested + class TestCompleteRequest { + + @Mock + private PipedInputStream inputStream; + + @BeforeEach + void mock() { + setRequestFinishedField(false); + setDownloadFinishedField(true); + setInputStreamField(inputStream); + } + + @SneakyThrows + @Test + void shouldCallCloseInputStream() { + downloader.completeRequest(); + + verify(downloader).closeInputStream(); + } + + @Test + void shouldCallOnCompleted() { + downloader.completeRequest(); + + verify(callObserver).onCompleted(); + } + + @SneakyThrows + private boolean getRequestFinished() { + return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get(); + } + } + @DisplayName("Handle safety") @Nested class TestHandleSafety { @@ -361,25 +477,24 @@ class GrpcBinaryFileServerDownloaderTest { } - @SneakyThrows private void setOutputStreamField(OutputStream outputStream) { - var outputStreamField = downloader.getClass().getDeclaredField("outputStream"); - outputStreamField.setAccessible(true); - outputStreamField.set(downloader, outputStream); + ReflectionTestUtils.setField(downloader, "outputStream", outputStream); } - @SneakyThrows private void setInputStreamField(InputStream inputStream) { - var inputStreamField = downloader.getClass().getDeclaredField("inputStream"); - inputStreamField.setAccessible(true); - inputStreamField.set(downloader, inputStream); + ReflectionTestUtils.setField(downloader, "inputStream", inputStream); + } + + private void setDownloadFinishedField(boolean downloadFinished) { + ReflectionTestUtils.setField(downloader, "downloadFinished", new AtomicBoolean(downloadFinished)); + } + + private void setRequestFinishedField(boolean requestFinished) { + ReflectionTestUtils.setField(downloader, "requestFinished", new AtomicBoolean(requestFinished)); } - @SneakyThrows - private void setDownloadInProgressField(AtomicBoolean downloadInProgress) { - var downloadInProgressField = downloader.getClass().getDeclaredField("downloadInProgress"); - downloadInProgressField.setAccessible(true); - downloadInProgressField.set(downloader, downloadInProgress); + private boolean getRequestFinished() { + return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get(); } static class GrpcResponseDummy { diff --git a/ozgcloud-common-test/src/main/java/de/ozgcloud/common/test/ReflectionTestUtils.java b/ozgcloud-common-test/src/main/java/de/ozgcloud/common/test/ReflectionTestUtils.java new file mode 100644 index 0000000000000000000000000000000000000000..d5cc01a399f1e095b73695e00e1193964f099636 --- /dev/null +++ b/ozgcloud-common-test/src/main/java/de/ozgcloud/common/test/ReflectionTestUtils.java @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2024 Das Land Schleswig-Holstein vertreten durch den + * Ministerpräsidenten des Landes Schleswig-Holstein + * Staatskanzlei + * Abteilung Digitalisierung und zentrales IT-Management der Landesregierung + * + * Lizenziert unter der EUPL, Version 1.2 oder - sobald + * diese von der Europäischen Kommission genehmigt wurden - + * Folgeversionen der EUPL ("Lizenz"); + * Sie dürfen dieses Werk ausschließlich gemäß + * dieser Lizenz nutzen. + * Eine Kopie der Lizenz finden Sie hier: + * + * https://joinup.ec.europa.eu/collection/eupl/eupl-text-eupl-12 + * + * Sofern nicht durch anwendbare Rechtsvorschriften + * gefordert oder in schriftlicher Form vereinbart, wird + * die unter der Lizenz verbreitete Software "so wie sie + * ist", OHNE JEGLICHE GEWÄHRLEISTUNG ODER BEDINGUNGEN - + * ausdrücklich oder stillschweigend - verbreitet. + * Die sprachspezifischen Genehmigungen und Beschränkungen + * unter der Lizenz sind dem Lizenztext zu entnehmen. + */ +package de.ozgcloud.common.test; + +import lombok.AccessLevel; +import lombok.NoArgsConstructor; +import lombok.SneakyThrows; + +import java.lang.reflect.Field; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class ReflectionTestUtils { + + @SneakyThrows + public static <T> T getField(Object target, String fieldName, Class<T> fieldType) { + return fieldType.cast(getDeclaredField(target, fieldName).get(target)); + } + + @SneakyThrows + public static void setField(Object target, String fieldName, Object value) { + getDeclaredField(target, fieldName).set(target, value); // NOSONAR + } + + @SneakyThrows + private static Field getDeclaredField(Object target, String fieldName) { + var field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); // NOSONAR + return field; + } +}