From 7828b5a50eb4db328ed3fd245ae5c7a54ade1ad3 Mon Sep 17 00:00:00 2001
From: OZGCloud <ozgcloud@mgm-tp.com>
Date: Thu, 21 Nov 2024 10:40:51 +0100
Subject: [PATCH] OZG-6990 implement comments from code review
---
ozgcloud-common-lib/pom.xml | 5 ++
.../GrpcBinaryFileServerDownloader.java | 21 ++---
.../GrpcBinaryFileServerDownloaderTest.java | 77 ++++++++++---------
.../common/test/ReflectionTestUtils.java | 51 ++++++++++++
4 files changed, 110 insertions(+), 44 deletions(-)
create mode 100644 ozgcloud-common-test/src/main/java/de/ozgcloud/common/test/ReflectionTestUtils.java
diff --git a/ozgcloud-common-lib/pom.xml b/ozgcloud-common-lib/pom.xml
index f46dc0e..1373e1d 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 e19f14e..84c34d9 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
@@ -53,7 +53,7 @@ public class GrpcBinaryFileServerDownloader<T> {
private final byte[] buffer = new byte[GrpcBinaryFileServerDownloader.CHUNK_SIZE];
private final AtomicBoolean started = new AtomicBoolean(false);
private final AtomicBoolean downloadFinished = new AtomicBoolean(false);
- private final AtomicBoolean sendingFinished = new AtomicBoolean(false);
+ private final AtomicBoolean requestFinished = new AtomicBoolean(false);
private PipedInputStream inputStream;
private PipedOutputStream outputStream;
@@ -108,15 +108,13 @@ public class GrpcBinaryFileServerDownloader<T> {
}
void doSendChunks() throws IOException {
- if (sendingFinished.get()) {
+ if (requestFinished.get()) {
return;
}
int bytesRead;
while (callObserver.isReady()) {
if ((bytesRead = inputStream.read(buffer)) == -1) {
- if (downloadFinished.get()) {
- completeDownload();
- }
+ tryCompleteRequest();
break;
}
callObserver.onNext(chunkBuilder.apply(ByteString.copyFrom(buffer, 0, bytesRead)));
@@ -124,13 +122,18 @@ public class GrpcBinaryFileServerDownloader<T> {
}
}
- void completeDownload() {
- LOG.debug("try to complete download");
- if (sendingFinished.getAndSet(true)) {
+ void tryCompleteRequest() {
+ if (downloadFinished.get()) {
+ completeRequest();
+ }
+ }
+
+ void completeRequest() {
+ if (requestFinished.getAndSet(true)) {
return;
}
+ LOG.debug("Complete download request");
closeInputStream();
- LOG.debug("Complete request.");
callObserver.onCompleted();
}
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 9e42671..3da0664 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;
@@ -271,11 +272,12 @@ class GrpcBinaryFileServerDownloaderTest {
@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);
- setBufferArray(buffer);
+ ReflectionTestUtils.setField(downloader, "buffer", buffer);
}
@Test
@@ -299,11 +301,11 @@ class GrpcBinaryFileServerDownloaderTest {
@DisplayName("should call complete grpc stream if download has finished and stream has no data left")
@Test
void shouldCallCompleteDownload() {
- setDownloadFinishedField(new AtomicBoolean(true));
+ setDownloadFinishedField(true);
doSendChunks();
- verify(downloader).completeDownload();
+ verify(downloader).tryCompleteRequest();
}
@SneakyThrows
@@ -314,45 +316,65 @@ class GrpcBinaryFileServerDownloaderTest {
}
@Nested
- class TestCompleteDownload {
+ class TestTryCompleteRequest {
+
+ @Test
+ void shouldCallCompleteRequest() {
+ setDownloadFinishedField(true);
+
+ downloader.tryCompleteRequest();
+
+ verify(downloader).completeRequest();
+ }
+
+ @Test
+ void shouldNotCallCompleteRequest() {
+ setDownloadFinishedField(false);
+
+ downloader.tryCompleteRequest();
+
+ verify(downloader, never()).completeRequest();
+ }
+ }
+
+ @Nested
+ class TestCompleteRequest {
@Mock
private PipedInputStream inputStream;
@BeforeEach
void mock() {
- setSendingFinishedField(new AtomicBoolean(false));
- setDownloadFinishedField(new AtomicBoolean(true));
+ setSendingFinishedField(false);
+ setDownloadFinishedField(true);
setInputStreamField(inputStream);
}
@Test
void shouldSetSendingFinished() {
- downloader.completeDownload();
+ downloader.completeRequest();
- assertThat(getSendingFinished()).isTrue();
+ assertThat(getRequestFinished()).isTrue();
}
@SneakyThrows
@Test
void shouldCallCloseInputStream() {
- downloader.completeDownload();
+ downloader.completeRequest();
verify(downloader).closeInputStream();
}
@Test
void shouldCallOnCompleted() {
- downloader.completeDownload();
+ downloader.completeRequest();
verify(callObserver).onCompleted();
}
@SneakyThrows
- private boolean getSendingFinished() {
- var sendingFinishedField = downloader.getClass().getDeclaredField("sendingFinished");
- sendingFinishedField.setAccessible(true);
- return ((AtomicBoolean) sendingFinishedField.get(downloader)).get();
+ private boolean getRequestFinished() {
+ return ReflectionTestUtils.getField(downloader, "requestFinished", AtomicBoolean.class).get();
}
}
@@ -421,37 +443,22 @@ 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);
- }
-
- @SneakyThrows
- private void setDownloadFinishedField(AtomicBoolean downloadInProgress) {
- var downloadInProgressField = downloader.getClass().getDeclaredField("downloadFinished");
- downloadInProgressField.setAccessible(true);
- downloadInProgressField.set(downloader, downloadInProgress);
+ ReflectionTestUtils.setField(downloader, "inputStream", inputStream);
}
@SneakyThrows
- private void setSendingFinishedField(AtomicBoolean downloadInProgress) {
- var downloadInProgressField = downloader.getClass().getDeclaredField("sendingFinished");
- downloadInProgressField.setAccessible(true);
- downloadInProgressField.set(downloader, downloadInProgress);
+ private void setDownloadFinishedField(boolean downloadFinished) {
+ ReflectionTestUtils.setField(downloader, "downloadFinished", new AtomicBoolean(downloadFinished));
}
@SneakyThrows
- private void setBufferArray(byte[] buffer) {
- var downloadInProgressField = downloader.getClass().getDeclaredField("buffer");
- downloadInProgressField.setAccessible(true);
- downloadInProgressField.set(downloader, buffer);
+ private void setSendingFinishedField(boolean requestFinished) {
+ ReflectionTestUtils.setField(downloader, "requestFinished", new AtomicBoolean(requestFinished));
}
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 0000000..d5cc01a
--- /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;
+ }
+}
--
GitLab