From 4960695d1fc0d231a5ccbfcfe722e2b6929a10fb Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Thu, 23 Jan 2025 10:56:25 +0100
Subject: [PATCH 1/7] OZG-7262 OZG-7566 Notify callObserver about error
---
.../GrpcBinaryFileServerDownloader.java | 2 +-
.../GrpcBinaryFileServerDownloaderTest.java | 31 ++++++-------------
2 files changed, 10 insertions(+), 23 deletions(-)
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 b992f42..cb4a208 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
@@ -143,7 +143,7 @@ public class GrpcBinaryFileServerDownloader<T> {
} catch (Exception e) {
closeOutputStream();
closeInputStream();
- throw new TechnicalException("Error occurred during downloading file content download.", e);
+ callObserver.onError(new TechnicalException("Error occurred during downloading file content download.", e));
}
}
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 6050e6b..483e38d 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
@@ -26,7 +26,6 @@ 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;
import org.junit.jupiter.api.BeforeEach;
@@ -407,11 +406,6 @@ class GrpcBinaryFileServerDownloaderTest {
verify(callObserver).onCompleted();
}
-
- @SneakyThrows
- private boolean getRequestFinished() {
- return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
- }
}
@DisplayName("Handle safety")
@@ -436,20 +430,10 @@ class GrpcBinaryFileServerDownloaderTest {
setOutputStreamField(outputStream);
}
- @SneakyThrows
- @Test
- void shouldThrowTechnicalException() {
- assertThatThrownBy(this::handleSafety).isInstanceOf(TechnicalException.class).extracting(Throwable::getCause).isEqualTo(exception);
- }
-
@SneakyThrows
@Test
void shouldCloseOutputStream() {
- try {
- handleSafety();
- } catch (Exception e) {
- // do nothing
- }
+ handleSafety();
verify(outputStream).close();
}
@@ -457,15 +441,18 @@ class GrpcBinaryFileServerDownloaderTest {
@SneakyThrows
@Test
void shouldCloseInputStream() {
- try {
- handleSafety();
- } catch (Exception e) {
- // do nothing
- }
+ handleSafety();
verify(inputStream).close();
}
+ @Test
+ void shouldNotifyCallObserver() {
+ handleSafety();
+
+ verify(callObserver).onError(argThat(TechnicalException.class::isInstance));
+ }
+
private void handleSafety() {
downloader.handleSafety(this::dummyMethodThrowingException);
}
--
GitLab
From d1187a9ec9c5a4c3a9f0e6c5a6dd0f2e29fba6e3 Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Mon, 27 Jan 2025 11:11:27 +0100
Subject: [PATCH 2/7] OZG-7262 OZG-7566 Fix error handling
---
.../BinaryFileDownloadException.java | 33 +++
.../binaryfile/CallStreamObserverWrapper.java | 78 ++++++
.../GrpcBinaryFileServerDownloader.java | 49 ++--
.../CallStreamObserverWrapperTest.java | 254 ++++++++++++++++++
.../GrpcBinaryFileServerDownloaderTest.java | 223 +++++++++------
5 files changed, 535 insertions(+), 102 deletions(-)
create mode 100644 ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
create mode 100644 ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
create mode 100644 ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
diff --git a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
new file mode 100644
index 0000000..9a5d462
--- /dev/null
+++ b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2025 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.binaryfile;
+
+import de.ozgcloud.common.errorhandling.TechnicalException;
+
+class BinaryFileDownloadException extends TechnicalException {
+
+ public BinaryFileDownloadException(Throwable cause) {
+ super("Error occurred during downloading file content", cause);
+ }
+}
diff --git a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
new file mode 100644
index 0000000..a0462c6
--- /dev/null
+++ b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2025 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.binaryfile;
+
+import java.util.function.Supplier;
+
+import de.ozgcloud.common.errorhandling.TechnicalException;
+import io.grpc.stub.CallStreamObserver;
+import lombok.RequiredArgsConstructor;
+
+@RequiredArgsConstructor
+ class CallStreamObserverWrapper<V> {
+
+ private final CallStreamObserver<V> callStreamObserver;
+ private boolean failed;
+
+ public void setOnReadyHandler(Runnable onReadyHandler) {
+ callStreamObserver.setOnReadyHandler(onReadyHandler);
+ }
+
+ public synchronized boolean isReady() {
+ return ifNotFailed(callStreamObserver::isReady);
+ }
+
+ public synchronized void onNext(V value) {
+ ifNotFailed(() -> callStreamObserver.onNext(value));
+ }
+
+ public synchronized void onError(Throwable t) {
+ if (!failed) {
+ callStreamObserver.onError(new BinaryFileDownloadException(t));
+ failed = true;
+ } else {
+ handleIllegalCallAfterError();
+ }
+ }
+
+ public synchronized void onCompleted() {
+ ifNotFailed(callStreamObserver::onCompleted);
+ }
+
+ private void ifNotFailed(Runnable runnable) {
+ if (!failed) {
+ runnable.run();
+ } else {
+ handleIllegalCallAfterError();
+ }
+ }
+
+ private <T> T ifNotFailed(Supplier<T> supplier) {
+ return !failed ? supplier.get() : handleIllegalCallAfterError();
+ }
+
+ private <T> T handleIllegalCallAfterError() {
+ throw new TechnicalException("CallStreamObserver called after error");
+ }
+}
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 cb4a208..44f3fae 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
@@ -23,14 +23,6 @@
*/
package de.ozgcloud.common.binaryfile;
-import com.google.protobuf.ByteString;
-import de.ozgcloud.common.errorhandling.TechnicalException;
-import io.grpc.stub.CallStreamObserver;
-import lombok.Builder;
-import lombok.extern.log4j.Log4j2;
-import org.apache.commons.io.IOUtils;
-import org.springframework.core.task.TaskExecutor;
-
import java.io.IOException;
import java.io.OutputStream;
import java.io.PipedInputStream;
@@ -39,12 +31,21 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.Function;
+import org.apache.commons.io.IOUtils;
+import org.springframework.core.task.TaskExecutor;
+
+import com.google.protobuf.ByteString;
+
+import io.grpc.stub.CallStreamObserver;
+import lombok.Builder;
+import lombok.extern.log4j.Log4j2;
+
@Log4j2
public class GrpcBinaryFileServerDownloader<T> {
private static final int CHUNK_SIZE = 255 * 1024;
- private final CallStreamObserver<T> callObserver;
+ private final CallStreamObserverWrapper<T> callObserver;
private final Function<ByteString, T> chunkBuilder;
private final Consumer<OutputStream> downloadConsumer;
private final TaskExecutor taskExecutor;
@@ -60,7 +61,7 @@ public class GrpcBinaryFileServerDownloader<T> {
@Builder
public GrpcBinaryFileServerDownloader(CallStreamObserver<T> callObserver, Function<ByteString, T> chunkBuilder,
Consumer<OutputStream> downloadConsumer, TaskExecutor taskExecutor) {
- this.callObserver = callObserver;
+ this.callObserver = new CallStreamObserverWrapper<>(callObserver);
this.chunkBuilder = chunkBuilder;
this.downloadConsumer = downloadConsumer;
this.taskExecutor = taskExecutor;
@@ -78,11 +79,20 @@ public class GrpcBinaryFileServerDownloader<T> {
void doStart() {
LOG.debug("Starting download.");
- handleSafety(this::setupStreams);
+ safelySetupStreams();
taskExecutor.execute(this::startDownload);
callObserver.setOnReadyHandler(this::sendChunks);
}
+ void safelySetupStreams() {
+ try {
+ setupStreams();
+ } catch (Exception e) {
+ closeStreams();
+ throw new BinaryFileDownloadException(e);
+ }
+ }
+
void setupStreams() throws IOException {
outputStream = new PipedOutputStream();
inputStream = new PipedInputStream(GrpcBinaryFileServerDownloader.CHUNK_SIZE);
@@ -90,7 +100,7 @@ public class GrpcBinaryFileServerDownloader<T> {
}
void startDownload() {
- handleSafety(this::doDownload);
+ withDownloadErrorHandling(this::doDownload);
}
void doDownload() {
@@ -103,7 +113,7 @@ public class GrpcBinaryFileServerDownloader<T> {
}
synchronized void sendChunks() {
- handleSafety(this::doSendChunks);
+ withDownloadErrorHandling(this::doSendChunks);
}
void doSendChunks() throws IOException {
@@ -137,16 +147,20 @@ public class GrpcBinaryFileServerDownloader<T> {
callObserver.onCompleted();
}
- void handleSafety(ExceptionalRunnable runnable) {
+ void withDownloadErrorHandling(ExceptionalRunnable runnable) {
try {
runnable.run();
} catch (Exception e) {
- closeOutputStream();
- closeInputStream();
- callObserver.onError(new TechnicalException("Error occurred during downloading file content download.", e));
+ closeStreams();
+ callObserver.onError(e);
}
}
+ private void closeStreams() {
+ closeOutputStream();
+ closeInputStream();
+ }
+
void closeOutputStream() {
IOUtils.closeQuietly(outputStream, e -> LOG.error("OutputStream cannot be closed.", e));
}
@@ -154,5 +168,4 @@ public class GrpcBinaryFileServerDownloader<T> {
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/CallStreamObserverWrapperTest.java b/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
new file mode 100644
index 0000000..c1d5999
--- /dev/null
+++ b/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
@@ -0,0 +1,254 @@
+/*
+ * Copyright (C) 2025 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.binaryfile;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+
+import de.ozgcloud.common.errorhandling.TechnicalException;
+import de.ozgcloud.common.test.ReflectionTestUtils;
+import io.grpc.stub.CallStreamObserver;
+
+class CallStreamObserverWrapperTest {
+
+ @Mock
+ private CallStreamObserver<GrpcResponseDummy> callObserver;
+ @InjectMocks
+ private CallStreamObserverWrapper<GrpcResponseDummy> wrapper;
+
+ @Nested
+ class TestSetOnReadyHandler {
+
+ @Test
+ void shouldForwardToObserver() {
+ var onReadyHandler = mock(Runnable.class);
+
+ wrapper.setOnReadyHandler(onReadyHandler);
+
+ verify(callObserver).setOnReadyHandler(onReadyHandler);
+ }
+ }
+
+ @Nested
+ class TestIsReady {
+
+ @Nested
+ class OnFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", true);
+ }
+
+ @Test
+ void shouldNotCallObserver() {
+ catchThrowable(wrapper::isReady);
+
+ verifyNoInteractions(callObserver);
+ }
+
+ @Test
+ void shouldThrowException() {
+ assertThatThrownBy(wrapper::isReady).isInstanceOf(TechnicalException.class);
+ }
+ }
+
+ @Nested
+ class OnNotFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", false);
+ }
+
+ @Test
+ void shouldForwardToObserver() {
+ wrapper.isReady();
+
+ verify(callObserver).isReady();
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = { true, false })
+ void shouldReturnObserverResult(boolean observerResult) {
+ when(callObserver.isReady()).thenReturn(observerResult);
+
+ var ready = wrapper.isReady();
+
+ assertThat(ready).isEqualTo(observerResult);
+ }
+ }
+ }
+
+ @Nested
+ class TestOnNext {
+
+ private final GrpcResponseDummy grpccResponse = new GrpcResponseDummy();
+
+ @Nested
+ class OnFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", true);
+ }
+
+ @Test
+ void shouldNotCallObserver() {
+ catchThrowable(() -> wrapper.onNext(grpccResponse));
+
+ verifyNoInteractions(callObserver);
+ }
+
+ @Test
+ void shouldThrowException() {
+ assertThatThrownBy(() -> wrapper.onNext(grpccResponse)).isInstanceOf(TechnicalException.class);
+ }
+ }
+
+ @Nested
+ class OnNotFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", false);
+ }
+
+ @Test
+ void shouldForwardToObserver() {
+ wrapper.onNext(grpccResponse);
+
+ verify(callObserver).onNext(grpccResponse);
+ }
+ }
+ }
+
+ @Nested
+ class TestOnError {
+
+ private final TechnicalException exception = new TechnicalException("dummy");
+
+ @Nested
+ class OnFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", true);
+ }
+
+ @Test
+ void shouldNotCallObserver() {
+ catchThrowable(() -> wrapper.onError(exception));
+
+ verifyNoInteractions(callObserver);
+ }
+
+ @Test
+ void shouldThrowException() {
+ assertThatThrownBy(() -> wrapper.onError(exception)).isInstanceOf(TechnicalException.class);
+ }
+ }
+
+ @Nested
+ class OnNotFailed {
+
+ private ArgumentCaptor<BinaryFileDownloadException> captor = ArgumentCaptor.forClass(BinaryFileDownloadException.class);
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", false);
+ }
+
+ @Test
+ void shouldForwardBinaryFileDownloadExceptionToObserver() {
+ wrapper.onError(exception);
+
+ verify(callObserver).onError(captor.capture());
+ assertThat(captor.getValue()).isInstanceOf(BinaryFileDownloadException.class);
+ assertThat(captor.getValue().getCause()).isEqualTo(exception);
+ }
+
+ @Test
+ void shouldSetFailedToTrue() {
+ wrapper.onError(exception);
+
+ assertThat(wrapper).extracting("failed").isEqualTo(true);
+ }
+ }
+ }
+
+ @Nested
+ class TestOnCompleted {
+
+ @Nested
+ class OnFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", true);
+ }
+
+ @Test
+ void shouldNotCallObserver() {
+ catchThrowable(wrapper::onCompleted);
+
+ verifyNoInteractions(callObserver);
+ }
+
+ @Test
+ void shouldThrowException() {
+ assertThatThrownBy(wrapper::onCompleted).isInstanceOf(TechnicalException.class);
+ }
+ }
+
+ @Nested
+ class OnNotFailed {
+
+ @BeforeEach
+ void init() {
+ ReflectionTestUtils.setField(wrapper, "failed", false);
+ }
+
+ @Test
+ void shouldForwardToObserver() {
+ wrapper.onCompleted();
+
+ verify(callObserver).onCompleted();
+ }
+ }
+ }
+
+ private static class GrpcResponseDummy {
+
+ }
+}
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 483e38d..e4f265d 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
@@ -23,19 +23,9 @@
*/
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.stub.CallStreamObserver;
-import lombok.SneakyThrows;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.DisplayName;
-import org.junit.jupiter.api.Nested;
-import org.junit.jupiter.api.Test;
-import org.mockito.ArgumentCaptor;
-import org.mockito.Captor;
-import org.mockito.Mock;
-import org.springframework.core.task.TaskExecutor;
+import static org.assertj.core.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.*;
+import static org.mockito.Mockito.*;
import java.io.IOException;
import java.io.InputStream;
@@ -47,14 +37,25 @@ import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.Function;
-import static org.assertj.core.api.Assertions.*;
-import static org.mockito.ArgumentMatchers.*;
-import static org.mockito.Mockito.*;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Captor;
+import org.mockito.Mock;
+import org.springframework.core.task.TaskExecutor;
+
+import com.google.protobuf.ByteString;
+
+import de.ozgcloud.common.errorhandling.TechnicalException;
+import de.ozgcloud.common.test.ReflectionTestUtils;
+import lombok.SneakyThrows;
class GrpcBinaryFileServerDownloaderTest {
@Mock
- private CallStreamObserver<GrpcResponseDummy> callObserver;
+ private CallStreamObserverWrapper<GrpcResponseDummy> callObserverWrapper;
@Mock
private Function<ByteString, GrpcResponseDummy> chunkBuilder;
@Mock
@@ -66,11 +67,11 @@ class GrpcBinaryFileServerDownloaderTest {
@BeforeEach
void init() {
- downloader = spy(GrpcBinaryFileServerDownloader.<GrpcResponseDummy>builder().callObserver(callObserver).downloadConsumer(downloadConsumer)
+ downloader = spy(GrpcBinaryFileServerDownloader.<GrpcResponseDummy>builder().downloadConsumer(downloadConsumer)
.chunkBuilder(chunkBuilder).taskExecutor(taskExecutor).build());
+ ReflectionTestUtils.setField(downloader, "callObserver", callObserverWrapper);
}
- @DisplayName("Start")
@Nested
class TestStart {
@@ -116,25 +117,23 @@ class GrpcBinaryFileServerDownloaderTest {
}
}
- @DisplayName("do start")
@Nested
class TestDoStart {
@Captor
private ArgumentCaptor<Runnable> runnableCaptor;
- @Captor
- private ArgumentCaptor<ExceptionalRunnable> setupStreamCaptor;
+
+ @BeforeEach
+ void init() {
+ doNothing().when(downloader).safelySetupStreams();
+ }
@SneakyThrows
@Test
- void shouldCallSetupStreams() {
- doNothing().when(downloader).handleSafety(any());
-
+ void shouldSafelySetupStreams() {
downloader.doStart();
- verify(downloader).handleSafety(setupStreamCaptor.capture());
- setupStreamCaptor.getValue().run();
- verify(downloader).setupStreams();
+ verify(downloader).safelySetupStreams();
}
@Test
@@ -153,12 +152,69 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetOnReadyHandler() {
downloader.doStart();
- verify(callObserver).setOnReadyHandler(runnableCaptor.capture());
+ verify(callObserverWrapper).setOnReadyHandler(runnableCaptor.capture());
assertThat(runnableCaptor.getValue()).isNotNull();
}
}
- @DisplayName("Start download")
+ @Nested
+ class TestSafelySetupStreams {
+
+ @SneakyThrows
+ @Test
+ void shouldSetupStreams() {
+ doNothing().when(downloader).setupStreams();
+
+ safelySetupStreams();
+
+ verify(downloader).setupStreams();
+ }
+
+ @Nested
+ class OnException {
+
+ @Mock
+ private PipedOutputStream outputStream;
+ @Mock
+ private PipedInputStream inputStream;
+
+ private final IOException exception = new IOException();
+
+ @SneakyThrows
+ @BeforeEach
+ void init() {
+ setInputStreamField(inputStream);
+ setOutputStreamField(outputStream);
+ doThrow(exception).when(downloader).setupStreams();
+ }
+
+ @Test
+ void shouldThrowBinaryFileDownloadException() {
+ assertThatThrownBy(() -> downloader.safelySetupStreams()).isInstanceOf(BinaryFileDownloadException.class).hasCause(exception);
+ }
+
+ @SneakyThrows
+ @Test
+ void shouldCloseOutputStream() {
+ catchThrowable(TestSafelySetupStreams.this::safelySetupStreams);
+
+ verify(outputStream).close();
+ }
+
+ @SneakyThrows
+ @Test
+ void shouldCloseInputStream() {
+ catchThrowable(TestSafelySetupStreams.this::safelySetupStreams);
+
+ verify(inputStream).close();
+ }
+ }
+
+ private void safelySetupStreams() {
+ downloader.safelySetupStreams();
+ }
+ }
+
@Nested
class TestStartDownload {
@@ -168,71 +224,64 @@ class GrpcBinaryFileServerDownloaderTest {
@SneakyThrows
@Test
void shouldCallDoDownload() {
- doNothing().when(downloader).handleSafety(any());
+ doNothing().when(downloader).withDownloadErrorHandling(any());
doNothing().when(downloader).doDownload();
downloader.startDownload();
- verify(downloader).handleSafety(runnableCaptor.capture());
+ verify(downloader).withDownloadErrorHandling(runnableCaptor.capture());
runnableCaptor.getValue().run();
verify(downloader).doDownload();
}
- }
- @DisplayName("do")
- @Nested
- class TestDoDownload {
+ @Nested
+ class TestDoDownload {
- @Mock
- private PipedOutputStream outputStream;
+ @Mock
+ private PipedOutputStream outputStream;
- @BeforeEach
- void mock() {
- setOutputStreamField(outputStream);
- }
+ @BeforeEach
+ void mock() {
+ setOutputStreamField(outputStream);
+ }
- @SneakyThrows
- @Test
- void shouldCallDownloadConsumer() {
- downloader.doDownload();
+ @SneakyThrows
+ @Test
+ void shouldCallDownloadConsumer() {
+ downloader.doDownload();
- verify(downloadConsumer).accept(outputStream);
- }
+ verify(downloadConsumer).accept(outputStream);
+ }
- @SneakyThrows
- @Test
- void shouldCloseOutputStream() {
- downloader.doDownload();
+ @SneakyThrows
+ @Test
+ void shouldCloseOutputStream() {
+ downloader.doDownload();
- verify(outputStream).close();
+ verify(outputStream).close();
+ }
}
}
- @DisplayName("Send chunks")
@Nested
class TestSendChunks {
- @SneakyThrows
- @Test
- void shouldCallHandleSafety() {
- doNothing().when(downloader).doSendChunks();
-
- downloader.sendChunks();
-
- verify(downloader).handleSafety(any(ExceptionalRunnable.class));
- }
+ @Captor
+ private ArgumentCaptor<ExceptionalRunnable> runnableCaptor;
@SneakyThrows
@Test
- void shouldCallDoDownload() {
+ void shouldCallWithDownloadErrorHandling() {
+ doNothing().when(downloader).withDownloadErrorHandling(any());
doNothing().when(downloader).doSendChunks();
downloader.sendChunks();
+ verify(downloader).withDownloadErrorHandling(runnableCaptor.capture());
+ runnableCaptor.getValue().run();
verify(downloader).doSendChunks();
}
- @DisplayName("do")
@Nested
class TestDoSendChunks {
@@ -249,7 +298,7 @@ class GrpcBinaryFileServerDownloaderTest {
@BeforeEach
void mock() {
doNothing().when(downloader).tryCompleteRequest();
- when(callObserver.isReady()).thenReturn(true);
+ when(callObserverWrapper.isReady()).thenReturn(true);
when(inputStream.read(any())).thenReturn(readBytes, -1);
setInputStreamField(inputStream);
new Random().nextBytes(buffer);
@@ -271,7 +320,7 @@ class GrpcBinaryFileServerDownloaderTest {
doSendChunks();
- verify(callObserver).onNext(grpcResponseDummy);
+ verify(callObserverWrapper).onNext(grpcResponseDummy);
}
@DisplayName("should call complete grpc stream if download has finished and stream has no data left")
@@ -404,28 +453,35 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldCallOnCompleted() {
downloader.completeRequest();
- verify(callObserver).onCompleted();
+ verify(callObserverWrapper).onCompleted();
}
}
- @DisplayName("Handle safety")
@Nested
- class TestHandleSafety {
+ class TestWithDownloadErrorHandling {
+
+ @SneakyThrows
+ @Test
+ void shouldRunRunnable() {
+ var runnable = mock(ExceptionalRunnable.class);
+
+ downloader.withDownloadErrorHandling(runnable);
+
+ verify(runnable).run();
+ }
- @DisplayName("on exception")
@Nested
- class TestOnException {
+ class OnException {
@Mock
private PipedOutputStream outputStream;
@Mock
private PipedInputStream inputStream;
- private final IOException exception = new IOException();
+ private final TechnicalException exception = new TechnicalException("dummy");
- @SneakyThrows
@BeforeEach
- void mock() {
+ void init() {
setInputStreamField(inputStream);
setOutputStreamField(outputStream);
}
@@ -433,7 +489,7 @@ class GrpcBinaryFileServerDownloaderTest {
@SneakyThrows
@Test
void shouldCloseOutputStream() {
- handleSafety();
+ withDownloadErrorHandling();
verify(outputStream).close();
}
@@ -441,27 +497,26 @@ class GrpcBinaryFileServerDownloaderTest {
@SneakyThrows
@Test
void shouldCloseInputStream() {
- handleSafety();
+ withDownloadErrorHandling();
verify(inputStream).close();
}
@Test
void shouldNotifyCallObserver() {
- handleSafety();
+ withDownloadErrorHandling();
- verify(callObserver).onError(argThat(TechnicalException.class::isInstance));
+ verify(callObserverWrapper).onError(argThat(TechnicalException.class::isInstance));
}
- private void handleSafety() {
- downloader.handleSafety(this::dummyMethodThrowingException);
+ private void withDownloadErrorHandling() {
+ downloader.withDownloadErrorHandling(this::dummyMethodThrowingException);
}
- private void dummyMethodThrowingException() throws IOException {
+ private void dummyMethodThrowingException() throws TechnicalException {
throw exception;
}
}
-
}
private void setOutputStreamField(OutputStream outputStream) {
@@ -484,6 +539,6 @@ class GrpcBinaryFileServerDownloaderTest {
return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
- static class GrpcResponseDummy {
+ private static class GrpcResponseDummy {
}
}
\ No newline at end of file
--
GitLab
From da68d4a4899bd1822aaf24c641b2c35a6603c732 Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Wed, 29 Jan 2025 15:55:51 +0100
Subject: [PATCH 3/7] OZG-7262 OZG-7627 Remove CallStreamObserverWrapper
---
.../binaryfile/CallStreamObserverWrapper.java | 78 ----
.../GrpcBinaryFileServerDownloader.java | 55 ++-
.../CallStreamObserverWrapperTest.java | 254 ------------
.../GrpcBinaryFileServerDownloaderTest.java | 390 ++++++++++++------
4 files changed, 303 insertions(+), 474 deletions(-)
delete mode 100644 ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
delete mode 100644 ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
diff --git a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
deleted file mode 100644
index a0462c6..0000000
--- a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapper.java
+++ /dev/null
@@ -1,78 +0,0 @@
-/*
- * Copyright (C) 2025 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.binaryfile;
-
-import java.util.function.Supplier;
-
-import de.ozgcloud.common.errorhandling.TechnicalException;
-import io.grpc.stub.CallStreamObserver;
-import lombok.RequiredArgsConstructor;
-
-@RequiredArgsConstructor
- class CallStreamObserverWrapper<V> {
-
- private final CallStreamObserver<V> callStreamObserver;
- private boolean failed;
-
- public void setOnReadyHandler(Runnable onReadyHandler) {
- callStreamObserver.setOnReadyHandler(onReadyHandler);
- }
-
- public synchronized boolean isReady() {
- return ifNotFailed(callStreamObserver::isReady);
- }
-
- public synchronized void onNext(V value) {
- ifNotFailed(() -> callStreamObserver.onNext(value));
- }
-
- public synchronized void onError(Throwable t) {
- if (!failed) {
- callStreamObserver.onError(new BinaryFileDownloadException(t));
- failed = true;
- } else {
- handleIllegalCallAfterError();
- }
- }
-
- public synchronized void onCompleted() {
- ifNotFailed(callStreamObserver::onCompleted);
- }
-
- private void ifNotFailed(Runnable runnable) {
- if (!failed) {
- runnable.run();
- } else {
- handleIllegalCallAfterError();
- }
- }
-
- private <T> T ifNotFailed(Supplier<T> supplier) {
- return !failed ? supplier.get() : handleIllegalCallAfterError();
- }
-
- private <T> T handleIllegalCallAfterError() {
- throw new TechnicalException("CallStreamObserver called after error");
- }
-}
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 44f3fae..090a53d 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
@@ -28,6 +28,7 @@ import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -45,15 +46,16 @@ public class GrpcBinaryFileServerDownloader<T> {
private static final int CHUNK_SIZE = 255 * 1024;
- private final CallStreamObserverWrapper<T> callObserver;
+ private final CallStreamObserver<T> callObserver;
private final Function<ByteString, T> chunkBuilder;
private final Consumer<OutputStream> downloadConsumer;
private final TaskExecutor taskExecutor;
- private final byte[] buffer = new byte[GrpcBinaryFileServerDownloader.CHUNK_SIZE];
+ private final byte[] buffer = new byte[CHUNK_SIZE];
private final AtomicBoolean started = new AtomicBoolean(false);
private final AtomicBoolean downloadFinished = new AtomicBoolean(false);
private final AtomicBoolean requestFinished = new AtomicBoolean(false);
+ private final AtomicReference<BinaryFileDownloadException> error = new AtomicReference<>();
private PipedInputStream inputStream;
private PipedOutputStream outputStream;
@@ -61,7 +63,7 @@ public class GrpcBinaryFileServerDownloader<T> {
@Builder
public GrpcBinaryFileServerDownloader(CallStreamObserver<T> callObserver, Function<ByteString, T> chunkBuilder,
Consumer<OutputStream> downloadConsumer, TaskExecutor taskExecutor) {
- this.callObserver = new CallStreamObserverWrapper<>(callObserver);
+ this.callObserver = callObserver;
this.chunkBuilder = chunkBuilder;
this.downloadConsumer = downloadConsumer;
this.taskExecutor = taskExecutor;
@@ -88,19 +90,27 @@ public class GrpcBinaryFileServerDownloader<T> {
try {
setupStreams();
} catch (Exception e) {
- closeStreams();
+ closeOutputStream();
+ closeInputStream();
throw new BinaryFileDownloadException(e);
}
}
void setupStreams() throws IOException {
outputStream = new PipedOutputStream();
- inputStream = new PipedInputStream(GrpcBinaryFileServerDownloader.CHUNK_SIZE);
+ inputStream = new PipedInputStream(CHUNK_SIZE);
outputStream.connect(inputStream);
}
void startDownload() {
- withDownloadErrorHandling(this::doDownload);
+ try {
+ doDownload();
+ } catch (Exception e) {
+ error.set(new BinaryFileDownloadException(e));
+ } finally {
+ closeOutputStream();
+ sendChunks();
+ }
}
void doDownload() {
@@ -108,12 +118,14 @@ public class GrpcBinaryFileServerDownloader<T> {
downloadConsumer.accept(outputStream);
LOG.debug("Download completed.");
downloadFinished.set(true);
- closeOutputStream();
- sendChunks();
}
synchronized void sendChunks() {
- withDownloadErrorHandling(this::doSendChunks);
+ try {
+ doSendChunks();
+ } catch (Exception e) {
+ completeRequestWithError(e);
+ }
}
void doSendChunks() throws IOException {
@@ -121,7 +133,11 @@ public class GrpcBinaryFileServerDownloader<T> {
return;
}
int bytesRead;
- while (callObserver.isReady()) {
+ while (isReady()) {
+ if (error.get() != null) {
+ completeRequestWithError(error.get());
+ break;
+ }
if ((bytesRead = inputStream.read(buffer)) == -1) {
tryCompleteRequest();
break;
@@ -131,6 +147,10 @@ public class GrpcBinaryFileServerDownloader<T> {
}
}
+ private boolean isReady() {
+ return callObserver.isReady();
+ }
+
void tryCompleteRequest() {
if (shouldCompleteRequest()) {
completeRequest();
@@ -147,18 +167,11 @@ public class GrpcBinaryFileServerDownloader<T> {
callObserver.onCompleted();
}
- void withDownloadErrorHandling(ExceptionalRunnable runnable) {
- try {
- runnable.run();
- } catch (Exception e) {
- closeStreams();
- callObserver.onError(e);
- }
- }
-
- private void closeStreams() {
- closeOutputStream();
+ void completeRequestWithError(Throwable t) {
+ LOG.debug("Complete download request with error", t);
+ requestFinished.set(true);
closeInputStream();
+ callObserver.onError(t);
}
void closeOutputStream() {
diff --git a/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java b/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
deleted file mode 100644
index c1d5999..0000000
--- a/ozgcloud-common-lib/src/test/java/de/ozgcloud/common/binaryfile/CallStreamObserverWrapperTest.java
+++ /dev/null
@@ -1,254 +0,0 @@
-/*
- * Copyright (C) 2025 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.binaryfile;
-
-import static org.assertj.core.api.Assertions.*;
-import static org.mockito.Mockito.*;
-
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Nested;
-import org.junit.jupiter.api.Test;
-import org.junit.jupiter.params.ParameterizedTest;
-import org.junit.jupiter.params.provider.ValueSource;
-import org.mockito.ArgumentCaptor;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-
-import de.ozgcloud.common.errorhandling.TechnicalException;
-import de.ozgcloud.common.test.ReflectionTestUtils;
-import io.grpc.stub.CallStreamObserver;
-
-class CallStreamObserverWrapperTest {
-
- @Mock
- private CallStreamObserver<GrpcResponseDummy> callObserver;
- @InjectMocks
- private CallStreamObserverWrapper<GrpcResponseDummy> wrapper;
-
- @Nested
- class TestSetOnReadyHandler {
-
- @Test
- void shouldForwardToObserver() {
- var onReadyHandler = mock(Runnable.class);
-
- wrapper.setOnReadyHandler(onReadyHandler);
-
- verify(callObserver).setOnReadyHandler(onReadyHandler);
- }
- }
-
- @Nested
- class TestIsReady {
-
- @Nested
- class OnFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", true);
- }
-
- @Test
- void shouldNotCallObserver() {
- catchThrowable(wrapper::isReady);
-
- verifyNoInteractions(callObserver);
- }
-
- @Test
- void shouldThrowException() {
- assertThatThrownBy(wrapper::isReady).isInstanceOf(TechnicalException.class);
- }
- }
-
- @Nested
- class OnNotFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", false);
- }
-
- @Test
- void shouldForwardToObserver() {
- wrapper.isReady();
-
- verify(callObserver).isReady();
- }
-
- @ParameterizedTest
- @ValueSource(booleans = { true, false })
- void shouldReturnObserverResult(boolean observerResult) {
- when(callObserver.isReady()).thenReturn(observerResult);
-
- var ready = wrapper.isReady();
-
- assertThat(ready).isEqualTo(observerResult);
- }
- }
- }
-
- @Nested
- class TestOnNext {
-
- private final GrpcResponseDummy grpccResponse = new GrpcResponseDummy();
-
- @Nested
- class OnFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", true);
- }
-
- @Test
- void shouldNotCallObserver() {
- catchThrowable(() -> wrapper.onNext(grpccResponse));
-
- verifyNoInteractions(callObserver);
- }
-
- @Test
- void shouldThrowException() {
- assertThatThrownBy(() -> wrapper.onNext(grpccResponse)).isInstanceOf(TechnicalException.class);
- }
- }
-
- @Nested
- class OnNotFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", false);
- }
-
- @Test
- void shouldForwardToObserver() {
- wrapper.onNext(grpccResponse);
-
- verify(callObserver).onNext(grpccResponse);
- }
- }
- }
-
- @Nested
- class TestOnError {
-
- private final TechnicalException exception = new TechnicalException("dummy");
-
- @Nested
- class OnFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", true);
- }
-
- @Test
- void shouldNotCallObserver() {
- catchThrowable(() -> wrapper.onError(exception));
-
- verifyNoInteractions(callObserver);
- }
-
- @Test
- void shouldThrowException() {
- assertThatThrownBy(() -> wrapper.onError(exception)).isInstanceOf(TechnicalException.class);
- }
- }
-
- @Nested
- class OnNotFailed {
-
- private ArgumentCaptor<BinaryFileDownloadException> captor = ArgumentCaptor.forClass(BinaryFileDownloadException.class);
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", false);
- }
-
- @Test
- void shouldForwardBinaryFileDownloadExceptionToObserver() {
- wrapper.onError(exception);
-
- verify(callObserver).onError(captor.capture());
- assertThat(captor.getValue()).isInstanceOf(BinaryFileDownloadException.class);
- assertThat(captor.getValue().getCause()).isEqualTo(exception);
- }
-
- @Test
- void shouldSetFailedToTrue() {
- wrapper.onError(exception);
-
- assertThat(wrapper).extracting("failed").isEqualTo(true);
- }
- }
- }
-
- @Nested
- class TestOnCompleted {
-
- @Nested
- class OnFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", true);
- }
-
- @Test
- void shouldNotCallObserver() {
- catchThrowable(wrapper::onCompleted);
-
- verifyNoInteractions(callObserver);
- }
-
- @Test
- void shouldThrowException() {
- assertThatThrownBy(wrapper::onCompleted).isInstanceOf(TechnicalException.class);
- }
- }
-
- @Nested
- class OnNotFailed {
-
- @BeforeEach
- void init() {
- ReflectionTestUtils.setField(wrapper, "failed", false);
- }
-
- @Test
- void shouldForwardToObserver() {
- wrapper.onCompleted();
-
- verify(callObserver).onCompleted();
- }
- }
- }
-
- private static class GrpcResponseDummy {
-
- }
-}
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 e4f265d..2e45282 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
@@ -34,6 +34,7 @@ import java.io.PipedInputStream;
import java.io.PipedOutputStream;
import java.util.Random;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import java.util.function.Function;
@@ -50,12 +51,13 @@ import com.google.protobuf.ByteString;
import de.ozgcloud.common.errorhandling.TechnicalException;
import de.ozgcloud.common.test.ReflectionTestUtils;
+import io.grpc.stub.CallStreamObserver;
import lombok.SneakyThrows;
class GrpcBinaryFileServerDownloaderTest {
@Mock
- private CallStreamObserverWrapper<GrpcResponseDummy> callObserverWrapper;
+ private CallStreamObserver<GrpcResponseDummy> callObserver;
@Mock
private Function<ByteString, GrpcResponseDummy> chunkBuilder;
@Mock
@@ -67,9 +69,8 @@ class GrpcBinaryFileServerDownloaderTest {
@BeforeEach
void init() {
- downloader = spy(GrpcBinaryFileServerDownloader.<GrpcResponseDummy>builder().downloadConsumer(downloadConsumer)
+ downloader = spy(GrpcBinaryFileServerDownloader.<GrpcResponseDummy>builder().callObserver(callObserver).downloadConsumer(downloadConsumer)
.chunkBuilder(chunkBuilder).taskExecutor(taskExecutor).build());
- ReflectionTestUtils.setField(downloader, "callObserver", callObserverWrapper);
}
@Nested
@@ -152,7 +153,7 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetOnReadyHandler() {
downloader.doStart();
- verify(callObserverWrapper).setOnReadyHandler(runnableCaptor.capture());
+ verify(callObserver).setOnReadyHandler(runnableCaptor.capture());
assertThat(runnableCaptor.getValue()).isNotNull();
}
}
@@ -218,126 +219,289 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class TestStartDownload {
- @Captor
- private ArgumentCaptor<ExceptionalRunnable> runnableCaptor;
+ @Mock
+ private PipedOutputStream outputStream;
+
+ @BeforeEach
+ void init() {
+ setOutputStreamField(outputStream);
+ }
- @SneakyThrows
@Test
- void shouldCallDoDownload() {
- doNothing().when(downloader).withDownloadErrorHandling(any());
- doNothing().when(downloader).doDownload();
+ void shouldErrorBeInitiallyNull() {
+ assertThat(getError()).isNull();
+ }
- downloader.startDownload();
+ @Nested
+ class OnNoException {
- verify(downloader).withDownloadErrorHandling(runnableCaptor.capture());
- runnableCaptor.getValue().run();
- verify(downloader).doDownload();
+ @BeforeEach
+ void init() {
+ doNothing().when(downloader).doDownload();
+ }
+
+ @Test
+ void shouldDoDownload() {
+ downloader.startDownload();
+
+ verify(downloader).doDownload();
+ }
+
+ @SneakyThrows
+ @Test
+ void shouldCloseOutputStream() {
+ downloader.startDownload();
+
+ verify(outputStream).close();
+ }
+
+ @Test
+ @DisplayName("should send chunks here to not wait for callObserver to change its ready status")
+ void shouldSendChunks() {
+ downloader.startDownload();
+
+ verify(downloader).sendChunks();
+ }
}
@Nested
- class TestDoDownload {
+ class OnException {
- @Mock
- private PipedOutputStream outputStream;
+ private final TechnicalException exception = new TechnicalException("error");
@BeforeEach
- void mock() {
- setOutputStreamField(outputStream);
+ void init() {
+ doThrow(exception).when(downloader).doDownload();
}
- @SneakyThrows
@Test
- void shouldCallDownloadConsumer() {
- downloader.doDownload();
+ void shouldSetError() {
+ downloader.startDownload();
- verify(downloadConsumer).accept(outputStream);
+ assertThat(getError()).isInstanceOf(BinaryFileDownloadException.class).hasCause(exception);
}
@SneakyThrows
@Test
void shouldCloseOutputStream() {
- downloader.doDownload();
+ downloader.startDownload();
verify(outputStream).close();
}
+
+ @Test
+ @DisplayName("should send chunks here to communicate error to callObserver")
+ void shouldSendChunks() {
+ downloader.startDownload();
+
+ verify(downloader).sendChunks();
+ }
}
}
@Nested
- class TestSendChunks {
+ class TestDoDownload {
- @Captor
- private ArgumentCaptor<ExceptionalRunnable> runnableCaptor;
+ @Mock
+ private PipedOutputStream outputStream;
+
+ @BeforeEach
+ void mock() {
+ setOutputStreamField(outputStream);
+ }
- @SneakyThrows
@Test
- void shouldCallWithDownloadErrorHandling() {
- doNothing().when(downloader).withDownloadErrorHandling(any());
- doNothing().when(downloader).doSendChunks();
+ void shouldCallDownloadConsumer() {
+ downloader.doDownload();
- downloader.sendChunks();
+ verify(downloadConsumer).accept(outputStream);
+ }
- verify(downloader).withDownloadErrorHandling(runnableCaptor.capture());
- runnableCaptor.getValue().run();
- verify(downloader).doSendChunks();
+ @Test
+ void shouldDownloadFinishedBeInitiallyFalse() {
+ assertThat(getDownloadFinished()).isFalse();
}
- @Nested
- class TestDoSendChunks {
+ @Test
+ void shouldSetDownloadFinished() {
+ downloader.doDownload();
- @Mock
- private PipedInputStream inputStream;
- @Captor
- private ArgumentCaptor<ByteString> byteStringCaptor;
+ assertThat(getDownloadFinished()).isTrue();
+ }
+ }
+
+ @Nested
+ class TestSendChunks {
- private final int readBytes = 20;
- private final byte[] buffer = new byte[readBytes];
- private final GrpcResponseDummy grpcResponseDummy = new GrpcResponseDummy();
+ @Nested
+ class OnNoException {
@SneakyThrows
@BeforeEach
- void mock() {
- doNothing().when(downloader).tryCompleteRequest();
- when(callObserverWrapper.isReady()).thenReturn(true);
- when(inputStream.read(any())).thenReturn(readBytes, -1);
- setInputStreamField(inputStream);
- new Random().nextBytes(buffer);
- ReflectionTestUtils.setField(downloader, "buffer", buffer);
+ void init() {
+ doNothing().when(downloader).doSendChunks();
}
+ @SneakyThrows
@Test
- void shouldCallChunkBuilder() {
- doSendChunks();
+ void shouldDoSendChunks() {
+ downloader.sendChunks();
- verify(chunkBuilder).apply(byteStringCaptor.capture());
- assertThat(byteStringCaptor.getValue().toByteArray()).isEqualTo(buffer);
+ verify(downloader).doSendChunks();
}
+ }
- @DisplayName("should send next chunk if callObserver is ready and stream already received data")
- @Test
- void shouldCallOnNext() {
- when(chunkBuilder.apply(any())).thenReturn(grpcResponseDummy);
+ @Nested
+ class OnException {
- doSendChunks();
+ private final TechnicalException exception = new TechnicalException("error");
- verify(callObserverWrapper).onNext(grpcResponseDummy);
+ @SneakyThrows
+ @BeforeEach
+ void init() {
+ doThrow(exception).when(downloader).doSendChunks();
}
- @DisplayName("should call complete grpc stream if download has finished and stream has no data left")
@Test
- void shouldCallCompleteDownload() {
- setDownloadFinishedField(true);
+ void shouldCompleteRequestWithError() {
+ downloader.sendChunks();
+ verify(downloader).completeRequestWithError(exception);
+ }
+ }
+ }
+
+ @Nested
+ class TestDoSendChunks {
+
+ @Nested
+ class OnRequestFinished {
+
+ @BeforeEach
+ void init() {
+ setRequestFinishedField(true);
+ }
+
+ @Test
+ void shouldNotInteractWithCallObserver() {
doSendChunks();
- verify(downloader).tryCompleteRequest();
+ verifyNoInteractions(callObserver);
}
+ }
- @SneakyThrows
- private void doSendChunks() {
- downloader.doSendChunks();
+ @Nested
+ class OnRequestNotFinished {
+
+ @Nested
+ class OnNotReady {
+
+ @BeforeEach
+ void init() {
+ when(callObserver.isReady()).thenReturn(false);
+ }
+
+ @Test
+ void shouldOnlyCallIsReadyOnObserver() {
+ doSendChunks();
+
+ verify(callObserver).isReady();
+ verifyNoMoreInteractions(callObserver);
+ }
+ }
+
+ @Nested
+ class OnReady {
+
+ @BeforeEach
+ void init() {
+ when(callObserver.isReady()).thenReturn(true);
+ }
+
+ @Nested
+ class OnHasError {
+
+ private final BinaryFileDownloadException exception = new BinaryFileDownloadException(new TechnicalException("error"));
+
+ @BeforeEach
+ void init() {
+ setErrorField(exception);
+ doNothing().when(downloader).completeRequestWithError(any());
+ }
+
+ @Test
+ void shouldOnlyCallIsReadyOnObserver() {
+ doSendChunks();
+
+ verify(callObserver).isReady();
+ verifyNoMoreInteractions(callObserver);
+ }
+
+ @Test
+ void shouldCompleteRequestWithError() {
+ doSendChunks();
+
+ verify(downloader).completeRequestWithError(exception);
+ }
+ }
+
+ @Nested
+ class OnHasNoError {
+ @Mock
+ private PipedInputStream inputStream;
+ @Captor
+ private ArgumentCaptor<ByteString> byteStringCaptor;
+
+ 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);
+ }
+
+ @DisplayName("should send next chunk if callObserver is ready and stream already received data")
+ @Test
+ void shouldCallOnNext() {
+ when(chunkBuilder.apply(any())).thenReturn(grpcResponseDummy);
+
+ doSendChunks();
+
+ verify(callObserver).onNext(grpcResponseDummy);
+ }
+
+ @DisplayName("should call complete grpc stream if download has finished and stream has no data left")
+ @Test
+ void shouldTryCompleteRequest() {
+ setDownloadFinishedField(true);
+
+ doSendChunks();
+
+ verify(downloader).tryCompleteRequest();
+ }
+ }
}
}
+
+ @SneakyThrows
+ private void doSendChunks() {
+ downloader.doSendChunks();
+ }
}
@Nested
@@ -453,69 +617,41 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldCallOnCompleted() {
downloader.completeRequest();
- verify(callObserverWrapper).onCompleted();
+ verify(callObserver).onCompleted();
}
}
@Nested
- class TestWithDownloadErrorHandling {
-
- @SneakyThrows
- @Test
- void shouldRunRunnable() {
- var runnable = mock(ExceptionalRunnable.class);
+ class TestCompleteRequestWithError {
- downloader.withDownloadErrorHandling(runnable);
+ private final Throwable error = new Throwable();
- verify(runnable).run();
+ @BeforeEach
+ void init() {
+ doNothing().when(downloader).closeInputStream();
}
- @Nested
- class OnException {
-
- @Mock
- private PipedOutputStream outputStream;
- @Mock
- private PipedInputStream inputStream;
-
- private final TechnicalException exception = new TechnicalException("dummy");
-
- @BeforeEach
- void init() {
- setInputStreamField(inputStream);
- setOutputStreamField(outputStream);
- }
-
- @SneakyThrows
- @Test
- void shouldCloseOutputStream() {
- withDownloadErrorHandling();
+ @Test
+ void shouldSetRequestFinished() {
+ assertThat(getRequestFinished()).isFalse();
- verify(outputStream).close();
- }
+ downloader.completeRequestWithError(error);
- @SneakyThrows
- @Test
- void shouldCloseInputStream() {
- withDownloadErrorHandling();
-
- verify(inputStream).close();
- }
+ assertThat(getRequestFinished()).isTrue();
+ }
- @Test
- void shouldNotifyCallObserver() {
- withDownloadErrorHandling();
+ @Test
+ void shouldCloseInputStream() {
+ downloader.completeRequestWithError(error);
- verify(callObserverWrapper).onError(argThat(TechnicalException.class::isInstance));
- }
+ verify(downloader).closeInputStream();
+ }
- private void withDownloadErrorHandling() {
- downloader.withDownloadErrorHandling(this::dummyMethodThrowingException);
- }
+ @Test
+ void shouldNotifyCallObserver() {
+ downloader.completeRequestWithError(error);
- private void dummyMethodThrowingException() throws TechnicalException {
- throw exception;
- }
+ verify(callObserver).onError(error);
}
}
@@ -527,10 +663,6 @@ class GrpcBinaryFileServerDownloaderTest {
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));
}
@@ -539,6 +671,22 @@ class GrpcBinaryFileServerDownloaderTest {
return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
+ private void setErrorField(BinaryFileDownloadException error) {
+ ReflectionTestUtils.setField(downloader, "error", new AtomicReference<>(error));
+ }
+
+ private BinaryFileDownloadException getError() {
+ return (BinaryFileDownloadException) ReflectionTestUtils.getField(downloader, "error", AtomicReference.class).get();
+ }
+
+ private void setDownloadFinishedField(boolean downloadFinished) {
+ ReflectionTestUtils.setField(downloader, "downloadFinished", new AtomicBoolean(downloadFinished));
+ }
+
+ private boolean getDownloadFinished() {
+ return ReflectionTestUtils.getField(downloader, "downloadFinished", AtomicBoolean.class).get();
+ }
+
private static class GrpcResponseDummy {
}
}
\ No newline at end of file
--
GitLab
From 844a1a873540338915ee92a5a7c5c41807bf7a95 Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Thu, 30 Jan 2025 10:42:48 +0100
Subject: [PATCH 4/7] OZG-7262 OZG-7627 Remove BinaryFileDownloadException
---
.../BinaryFileDownloadException.java | 33 -------------------
.../GrpcBinaryFileServerDownloader.java | 18 +++++-----
.../GrpcBinaryFileServerDownloaderTest.java | 21 +++++++-----
3 files changed, 22 insertions(+), 50 deletions(-)
delete mode 100644 ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
diff --git a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java b/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
deleted file mode 100644
index 9a5d462..0000000
--- a/ozgcloud-common-lib/src/main/java/de/ozgcloud/common/binaryfile/BinaryFileDownloadException.java
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (C) 2025 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.binaryfile;
-
-import de.ozgcloud.common.errorhandling.TechnicalException;
-
-class BinaryFileDownloadException extends TechnicalException {
-
- public BinaryFileDownloadException(Throwable cause) {
- super("Error occurred during downloading file content", cause);
- }
-}
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 090a53d..b2d2df3 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
@@ -27,6 +27,7 @@ import java.io.IOException;
import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
+import java.util.Objects;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
@@ -37,6 +38,7 @@ import org.springframework.core.task.TaskExecutor;
import com.google.protobuf.ByteString;
+import de.ozgcloud.common.errorhandling.TechnicalException;
import io.grpc.stub.CallStreamObserver;
import lombok.Builder;
import lombok.extern.log4j.Log4j2;
@@ -55,7 +57,7 @@ public class GrpcBinaryFileServerDownloader<T> {
private final AtomicBoolean started = new AtomicBoolean(false);
private final AtomicBoolean downloadFinished = new AtomicBoolean(false);
private final AtomicBoolean requestFinished = new AtomicBoolean(false);
- private final AtomicReference<BinaryFileDownloadException> error = new AtomicReference<>();
+ private final AtomicReference<TechnicalException> error = new AtomicReference<>();
private PipedInputStream inputStream;
private PipedOutputStream outputStream;
@@ -92,7 +94,7 @@ public class GrpcBinaryFileServerDownloader<T> {
} catch (Exception e) {
closeOutputStream();
closeInputStream();
- throw new BinaryFileDownloadException(e);
+ throw new TechnicalException("Error while setting up streams", e);
}
}
@@ -106,7 +108,7 @@ public class GrpcBinaryFileServerDownloader<T> {
try {
doDownload();
} catch (Exception e) {
- error.set(new BinaryFileDownloadException(e));
+ error.set(new TechnicalException("Error while downloading file contents", e));
} finally {
closeOutputStream();
sendChunks();
@@ -124,7 +126,7 @@ public class GrpcBinaryFileServerDownloader<T> {
try {
doSendChunks();
} catch (Exception e) {
- completeRequestWithError(e);
+ completeRequestWithError(new TechnicalException("Error while sending chunks", e));
}
}
@@ -134,7 +136,7 @@ public class GrpcBinaryFileServerDownloader<T> {
}
int bytesRead;
while (isReady()) {
- if (error.get() != null) {
+ if (Objects.nonNull(error.get())) {
completeRequestWithError(error.get());
break;
}
@@ -167,11 +169,11 @@ public class GrpcBinaryFileServerDownloader<T> {
callObserver.onCompleted();
}
- void completeRequestWithError(Throwable t) {
- LOG.debug("Complete download request with error", t);
+ void completeRequestWithError(TechnicalException e) {
+ LOG.debug("Complete download request with error", e);
requestFinished.set(true);
closeInputStream();
- callObserver.onError(t);
+ callObserver.onError(e);
}
void closeOutputStream() {
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 2e45282..81da69e 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
@@ -191,7 +191,7 @@ class GrpcBinaryFileServerDownloaderTest {
@Test
void shouldThrowBinaryFileDownloadException() {
- assertThatThrownBy(() -> downloader.safelySetupStreams()).isInstanceOf(BinaryFileDownloadException.class).hasCause(exception);
+ assertThatThrownBy(() -> downloader.safelySetupStreams()).isInstanceOf(TechnicalException.class).hasCause(exception);
}
@SneakyThrows
@@ -278,7 +278,7 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetError() {
downloader.startDownload();
- assertThat(getError()).isInstanceOf(BinaryFileDownloadException.class).hasCause(exception);
+ assertThat(getError()).isInstanceOf(TechnicalException.class).hasCause(exception);
}
@SneakyThrows
@@ -354,7 +354,9 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class OnException {
- private final TechnicalException exception = new TechnicalException("error");
+ private final IOException exception = new IOException();
+ @Captor
+ private ArgumentCaptor<TechnicalException> argumentCaptor;
@SneakyThrows
@BeforeEach
@@ -366,7 +368,8 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldCompleteRequestWithError() {
downloader.sendChunks();
- verify(downloader).completeRequestWithError(exception);
+ verify(downloader).completeRequestWithError(argumentCaptor.capture());
+ assertThat(argumentCaptor.getValue()).isInstanceOf(TechnicalException.class).hasCause(exception);
}
}
}
@@ -421,7 +424,7 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class OnHasError {
- private final BinaryFileDownloadException exception = new BinaryFileDownloadException(new TechnicalException("error"));
+ private final TechnicalException exception = new TechnicalException("error");
@BeforeEach
void init() {
@@ -624,7 +627,7 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class TestCompleteRequestWithError {
- private final Throwable error = new Throwable();
+ private final TechnicalException error = new TechnicalException("error");
@BeforeEach
void init() {
@@ -671,12 +674,12 @@ class GrpcBinaryFileServerDownloaderTest {
return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
- private void setErrorField(BinaryFileDownloadException error) {
+ private void setErrorField(TechnicalException error) {
ReflectionTestUtils.setField(downloader, "error", new AtomicReference<>(error));
}
- private BinaryFileDownloadException getError() {
- return (BinaryFileDownloadException) ReflectionTestUtils.getField(downloader, "error", AtomicReference.class).get();
+ private TechnicalException getError() {
+ return (TechnicalException) ReflectionTestUtils.getField(downloader, "error", AtomicReference.class).get();
}
private void setDownloadFinishedField(boolean downloadFinished) {
--
GitLab
From 0c9266270ee2d3014254983ca63312017c9a532d Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Thu, 30 Jan 2025 15:38:34 +0100
Subject: [PATCH 5/7] OZG-7262 OZG-7627 Don't call sendChunks() after exception
---
.../GrpcBinaryFileServerDownloader.java | 31 ++-
.../GrpcBinaryFileServerDownloaderTest.java | 239 +++++++-----------
2 files changed, 103 insertions(+), 167 deletions(-)
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 b2d2df3..8acac69 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
@@ -57,7 +57,7 @@ public class GrpcBinaryFileServerDownloader<T> {
private final AtomicBoolean started = new AtomicBoolean(false);
private final AtomicBoolean downloadFinished = new AtomicBoolean(false);
private final AtomicBoolean requestFinished = new AtomicBoolean(false);
- private final AtomicReference<TechnicalException> error = new AtomicReference<>();
+ private final AtomicReference<TechnicalException> downloadError = new AtomicReference<>();
private PipedInputStream inputStream;
private PipedOutputStream outputStream;
@@ -107,11 +107,12 @@ public class GrpcBinaryFileServerDownloader<T> {
void startDownload() {
try {
doDownload();
+ sendChunks();
} catch (Exception e) {
- error.set(new TechnicalException("Error while downloading file contents", e));
+ LOG.error("Error while downloading file contents", e);
+ downloadError.set(new TechnicalException("Error while downloading file contents", e));
} finally {
closeOutputStream();
- sendChunks();
}
}
@@ -136,10 +137,6 @@ public class GrpcBinaryFileServerDownloader<T> {
}
int bytesRead;
while (isReady()) {
- if (Objects.nonNull(error.get())) {
- completeRequestWithError(error.get());
- break;
- }
if ((bytesRead = inputStream.read(buffer)) == -1) {
tryCompleteRequest();
break;
@@ -154,26 +151,28 @@ public class GrpcBinaryFileServerDownloader<T> {
}
void tryCompleteRequest() {
- if (shouldCompleteRequest()) {
- completeRequest();
+ if (Objects.nonNull(downloadError.get())) {
+ throw downloadError.get();
+ } else if (downloadFinished.get()) {
+ completeRequestNormally();
}
}
- boolean shouldCompleteRequest() {
- return downloadFinished.get() && requestFinished.compareAndSet(false, true);
+ void completeRequestWithError(TechnicalException e) {
+ LOG.debug("Complete download request with error");
+ finishRequest();
+ throw e;
}
- void completeRequest() {
+ void completeRequestNormally() {
LOG.debug("Complete download request");
- closeInputStream();
+ finishRequest();
callObserver.onCompleted();
}
- void completeRequestWithError(TechnicalException e) {
- LOG.debug("Complete download request with error", e);
+ private void finishRequest() {
requestFinished.set(true);
closeInputStream();
- callObserver.onError(e);
}
void closeOutputStream() {
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 81da69e..8c3ad5a 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
@@ -229,7 +229,7 @@ class GrpcBinaryFileServerDownloaderTest {
@Test
void shouldErrorBeInitiallyNull() {
- assertThat(getError()).isNull();
+ assertThat(getDownloadError()).isNull();
}
@Nested
@@ -256,7 +256,6 @@ class GrpcBinaryFileServerDownloaderTest {
}
@Test
- @DisplayName("should send chunks here to not wait for callObserver to change its ready status")
void shouldSendChunks() {
downloader.startDownload();
@@ -278,7 +277,7 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetError() {
downloader.startDownload();
- assertThat(getError()).isInstanceOf(TechnicalException.class).hasCause(exception);
+ assertThat(getDownloadError()).isInstanceOf(TechnicalException.class).hasCause(exception);
}
@SneakyThrows
@@ -288,14 +287,6 @@ class GrpcBinaryFileServerDownloaderTest {
verify(outputStream).close();
}
-
- @Test
- @DisplayName("should send chunks here to communicate error to callObserver")
- void shouldSendChunks() {
- downloader.startDownload();
-
- verify(downloader).sendChunks();
- }
}
}
@@ -362,6 +353,7 @@ class GrpcBinaryFileServerDownloaderTest {
@BeforeEach
void init() {
doThrow(exception).when(downloader).doSendChunks();
+ doNothing().when(downloader).completeRequestWithError(any());
}
@Test
@@ -416,87 +408,52 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class OnReady {
+ @Mock
+ private PipedInputStream inputStream;
+ @Captor
+ private ArgumentCaptor<ByteString> byteStringCaptor;
+
+ private final int readBytes = 20;
+ private final byte[] buffer = new byte[readBytes];
+ private final GrpcResponseDummy grpcResponseDummy = new GrpcResponseDummy();
+
+ @SneakyThrows
@BeforeEach
- void init() {
+ 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);
}
- @Nested
- class OnHasError {
+ @Test
+ void shouldCallChunkBuilder() {
+ doSendChunks();
- private final TechnicalException exception = new TechnicalException("error");
+ verify(chunkBuilder).apply(byteStringCaptor.capture());
+ assertThat(byteStringCaptor.getValue().toByteArray()).isEqualTo(buffer);
+ }
- @BeforeEach
- void init() {
- setErrorField(exception);
- doNothing().when(downloader).completeRequestWithError(any());
- }
+ @DisplayName("should send next chunk if callObserver is ready and stream already received data")
+ @Test
+ void shouldCallOnNext() {
+ when(chunkBuilder.apply(any())).thenReturn(grpcResponseDummy);
- @Test
- void shouldOnlyCallIsReadyOnObserver() {
- doSendChunks();
+ doSendChunks();
- verify(callObserver).isReady();
- verifyNoMoreInteractions(callObserver);
- }
+ verify(callObserver).onNext(grpcResponseDummy);
+ }
- @Test
- void shouldCompleteRequestWithError() {
- doSendChunks();
+ @DisplayName("should call complete grpc stream if download has finished and stream has no data left")
+ @Test
+ void shouldTryCompleteRequest() {
+ setDownloadFinishedField(true);
- verify(downloader).completeRequestWithError(exception);
- }
- }
+ doSendChunks();
- @Nested
- class OnHasNoError {
- @Mock
- private PipedInputStream inputStream;
- @Captor
- private ArgumentCaptor<ByteString> byteStringCaptor;
-
- 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);
- }
-
- @DisplayName("should send next chunk if callObserver is ready and stream already received data")
- @Test
- void shouldCallOnNext() {
- when(chunkBuilder.apply(any())).thenReturn(grpcResponseDummy);
-
- doSendChunks();
-
- verify(callObserver).onNext(grpcResponseDummy);
- }
-
- @DisplayName("should call complete grpc stream if download has finished and stream has no data left")
- @Test
- void shouldTryCompleteRequest() {
- setDownloadFinishedField(true);
-
- doSendChunks();
-
- verify(downloader).tryCompleteRequest();
- }
+ verify(downloader).tryCompleteRequest();
}
}
}
@@ -510,69 +467,48 @@ class GrpcBinaryFileServerDownloaderTest {
@Nested
class TestTryCompleteRequest {
- @Test
- void shouldCallShouldCompleteRequest() {
- downloader.tryCompleteRequest();
-
- verify(downloader).shouldCompleteRequest();
- }
-
- @Test
- void shouldCallCompleteRequest() {
- doReturn(true).when(downloader).shouldCompleteRequest();
-
- downloader.tryCompleteRequest();
-
- verify(downloader).completeRequest();
- }
+ @Nested
+ class OnError {
- @Test
- void shouldNotCallCompleteRequest() {
- doReturn(false).when(downloader).shouldCompleteRequest();
+ private final TechnicalException exception = new TechnicalException("error");
- downloader.tryCompleteRequest();
+ @BeforeEach
+ void init() {
+ setDownloadErrorField(exception);
+ }
- verify(downloader, never()).completeRequest();
+ @Test
+ void shouldThrowException() {
+ assertThatThrownBy(downloader::tryCompleteRequest).isSameAs(exception);
+ }
}
- }
-
- @Nested
- class TestShouldCompleteRequest {
@Nested
- class TestWhenDownloadFinished {
+ class OnDownloadFinished {
@BeforeEach
void init() {
setDownloadFinishedField(true);
+ doNothing().when(downloader).completeRequestNormally();
}
@Test
- void shouldReturnTrue() {
- var result = downloader.shouldCompleteRequest();
+ void shouldNotCompleteRequestWithError() {
+ downloader.tryCompleteRequest();
- assertThat(result).isTrue();
+ verify(downloader, never()).completeRequestWithError(any());
}
@Test
- void shouldReturnFalseIfRequestFinished() {
- setRequestFinishedField(true);
-
- var result = downloader.shouldCompleteRequest();
+ void shouldCompleteRequestNormally() {
+ downloader.tryCompleteRequest();
- assertThat(result).isFalse();
- }
-
- @Test
- void shouldUpdateRequestFinished() {
- downloader.shouldCompleteRequest();
-
- assertThat(getRequestFinished()).isTrue();
+ verify(downloader).completeRequestNormally();
}
}
@Nested
- class TestWhenDownloadRunning {
+ class OnDownloadNotFinished {
@BeforeEach
void init() {
@@ -580,45 +516,48 @@ class GrpcBinaryFileServerDownloaderTest {
}
@Test
- void shouldReturnFalse() {
- var result = downloader.shouldCompleteRequest();
+ void shouldNotCompleteRequestNormally() {
+ downloader.tryCompleteRequest();
- assertThat(result).isFalse();
+ verify(downloader, never()).completeRequestNormally();
}
@Test
- void shouldNotUpdateRequestFinished() {
- downloader.shouldCompleteRequest();
+ void shouldNotCompleteRequestWithError() {
+ downloader.tryCompleteRequest();
- assertThat(getRequestFinished()).isFalse();
+ verify(downloader, never()).completeRequestWithError(any());
}
}
}
@Nested
- class TestCompleteRequest {
-
- @Mock
- private PipedInputStream inputStream;
+ class TestCompleteRequestNormally {
@BeforeEach
- void mock() {
- setRequestFinishedField(false);
- setDownloadFinishedField(true);
- setInputStreamField(inputStream);
+ void init() {
+ doNothing().when(downloader).closeInputStream();
}
- @SneakyThrows
@Test
- void shouldCallCloseInputStream() {
- downloader.completeRequest();
+ void shouldSetRequestFinished() {
+ assertThat(getRequestFinished()).isFalse();
+
+ downloader.completeRequestNormally();
+
+ assertThat(getRequestFinished()).isTrue();
+ }
+
+ @Test
+ void shouldCloseInputStream() {
+ downloader.completeRequestNormally();
verify(downloader).closeInputStream();
}
@Test
- void shouldCallOnCompleted() {
- downloader.completeRequest();
+ void shouldNotifyObserver() {
+ downloader.completeRequestNormally();
verify(callObserver).onCompleted();
}
@@ -638,23 +577,21 @@ class GrpcBinaryFileServerDownloaderTest {
void shouldSetRequestFinished() {
assertThat(getRequestFinished()).isFalse();
- downloader.completeRequestWithError(error);
+ catchException(() -> downloader.completeRequestWithError(error));
assertThat(getRequestFinished()).isTrue();
}
@Test
void shouldCloseInputStream() {
- downloader.completeRequestWithError(error);
+ catchException(() -> downloader.completeRequestWithError(error));
verify(downloader).closeInputStream();
}
@Test
- void shouldNotifyCallObserver() {
- downloader.completeRequestWithError(error);
-
- verify(callObserver).onError(error);
+ void shouldThrowException() {
+ assertThatThrownBy(() -> downloader.completeRequestWithError(error)).isSameAs(error);
}
}
@@ -674,12 +611,12 @@ class GrpcBinaryFileServerDownloaderTest {
return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
- private void setErrorField(TechnicalException error) {
- ReflectionTestUtils.setField(downloader, "error", new AtomicReference<>(error));
+ private void setDownloadErrorField(TechnicalException error) {
+ ReflectionTestUtils.setField(downloader, "downloadError", new AtomicReference<>(error));
}
- private TechnicalException getError() {
- return (TechnicalException) ReflectionTestUtils.getField(downloader, "error", AtomicReference.class).get();
+ private TechnicalException getDownloadError() {
+ return (TechnicalException) ReflectionTestUtils.getField(downloader, "downloadError", AtomicReference.class).get();
}
private void setDownloadFinishedField(boolean downloadFinished) {
--
GitLab
From 45401b678402dc2653153b08f1561d4ca35af051 Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Thu, 30 Jan 2025 15:48:40 +0100
Subject: [PATCH 6/7] OZG-7262 OZG-7627 Log TechnicalException
---
.../common/binaryfile/GrpcBinaryFileServerDownloader.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 8acac69..edfd2eb 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
@@ -109,8 +109,8 @@ public class GrpcBinaryFileServerDownloader<T> {
doDownload();
sendChunks();
} catch (Exception e) {
- LOG.error("Error while downloading file contents", e);
downloadError.set(new TechnicalException("Error while downloading file contents", e));
+ LOG.error(downloadError.get().getMessage(), downloadError.get());
} finally {
closeOutputStream();
}
--
GitLab
From 884dcc0b6f05d6de3086046f6f7ebcd5d16b4abd Mon Sep 17 00:00:00 2001
From: Krzysztof Witukiewicz <krzysztof.witukiewicz@mgm-tp.com>
Date: Fri, 31 Jan 2025 08:56:40 +0100
Subject: [PATCH 7/7] OZG-7262 OZG-7627 Remove log
---
.../common/binaryfile/GrpcBinaryFileServerDownloader.java | 1 -
1 file changed, 1 deletion(-)
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 edfd2eb..6083de9 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
@@ -110,7 +110,6 @@ public class GrpcBinaryFileServerDownloader<T> {
sendChunks();
} catch (Exception e) {
downloadError.set(new TechnicalException("Error while downloading file contents", e));
- LOG.error(downloadError.get().getMessage(), downloadError.get());
} finally {
closeOutputStream();
}
--
GitLab