From 9ff7fb3a46f2c4165c6cccdda9137ec284153b50 Mon Sep 17 00:00:00 2001
From: Felix Reichenbach <felix.reichenbach@mgm-tp.com>
Date: Thu, 20 Mar 2025 11:57:44 +0100
Subject: [PATCH] OZG-7573 remove waitForFinishedFileUpload
---
.../redirect/ForwardingRemoteService.java | 24 +--
.../redirect/ForwardingRemoteServiceTest.java | 171 ++----------------
2 files changed, 16 insertions(+), 179 deletions(-)
diff --git a/vorgang-manager-server/src/main/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteService.java b/vorgang-manager-server/src/main/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteService.java
index f9d26cb6d..18a53bf2f 100644
--- a/vorgang-manager-server/src/main/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteService.java
+++ b/vorgang-manager-server/src/main/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteService.java
@@ -32,7 +32,6 @@ import java.util.concurrent.TimeoutException;
import java.util.function.BiFunction;
import java.util.function.Function;
-import org.apache.commons.io.IOUtils;
import org.springframework.stereotype.Service;
import com.google.protobuf.ByteString;
@@ -109,8 +108,7 @@ class ForwardingRemoteService {
private void sendAttachmentFile(StreamObserver<GrpcRouteForwardingRequest> requestStreamObserver, String groupName, IncomingFile file) {
var fileContentStream = fileService.getUploadedFileStream(file.getId());
- var fileSender = createAttachmentFileSender(requestStreamObserver, groupName, file, fileContentStream).send();
- waitForFinishedFileUpload(fileSender, fileContentStream);
+ createAttachmentFileSender(requestStreamObserver, groupName, file, fileContentStream).send();
}
FileSender<GrpcRouteForwardingRequest, GrpcRouteForwardingResponse> createAttachmentFileSender(
@@ -138,8 +136,7 @@ class ForwardingRemoteService {
void sendRepresentations(List<IncomingFile> representations, StreamObserver<GrpcRouteForwardingRequest> requestObserver) {
representations.forEach(representation -> {
var fileContentStream = fileService.getUploadedFileStream(representation.getId());
- var fileSender = createRepresentationFileSender(requestObserver, representation, fileContentStream).send();
- waitForFinishedFileUpload(fileSender, fileContentStream);
+ createRepresentationFileSender(requestObserver, representation, fileContentStream).send();
});
}
@@ -187,29 +184,16 @@ class ForwardingRemoteService {
.build();
}
- void waitForFinishedFileUpload(FileSender<GrpcRouteForwardingRequest, GrpcRouteForwardingResponse> fileSender, InputStream inputStream) {
+ void waitForCompletion(CompletableFuture<Void> responseFuture) {
try {
- fileSender.getResultFuture().get(TIMEOUT_MINUTES, TimeUnit.MINUTES);
+ responseFuture.get(TIMEOUT_MINUTES, TimeUnit.MINUTES);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
- fileSender.cancelOnError(e);
throw new TechnicalException("Waiting for finishing file upload was interrupted.", e);
} catch (ExecutionException e) {
- fileSender.cancelOnError(e);
throw new TechnicalException("Error on uploading file content.", e);
} catch (TimeoutException e) {
- fileSender.cancelOnTimeout();
throw new TechnicalException("Timeout on uploading file content.", e);
- } finally {
- IOUtils.closeQuietly(inputStream);
- }
- }
-
- void waitForCompletion(CompletableFuture<Void> responseFuture) {
- try {
- responseFuture.get();
- } catch (Throwable t) {
- throw new TechnicalException("Forwarding failed", t);
}
}
diff --git a/vorgang-manager-server/src/test/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteServiceTest.java b/vorgang-manager-server/src/test/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteServiceTest.java
index 8a1de85f2..dab4e8bae 100644
--- a/vorgang-manager-server/src/test/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteServiceTest.java
+++ b/vorgang-manager-server/src/test/java/de/ozgcloud/vorgang/vorgang/redirect/ForwardingRemoteServiceTest.java
@@ -320,7 +320,6 @@ class ForwardingRemoteServiceTest {
when(fileService.getUploadedFileStream(any())).thenReturn(inputStream);
doReturn(fileSender).when(service).createAttachmentFileSender(any(), any(), any(), any());
when(fileSender.send()).thenReturn(fileSender);
- doNothing().when(service).waitForFinishedFileUpload(any(), any());
}
@Test
@@ -345,16 +344,6 @@ class ForwardingRemoteServiceTest {
verify(fileSender).send();
}
- @Test
- void shouldCallWaitForFinishedFileUploadAfterSend() {
- var inOrder = inOrder(fileSender, service);
-
- sendAttachments();
-
- inOrder.verify(fileSender).send();
- inOrder.verify(service).waitForFinishedFileUpload(fileSender, inputStream);
- }
-
private void sendAttachments() {
service.sendAttachments(List.of(attachment), requestObserver);
}
@@ -485,7 +474,6 @@ class ForwardingRemoteServiceTest {
when(fileService.getUploadedFileStream(any())).thenReturn(inputStream);
doReturn(fileSender).when(service).createRepresentationFileSender(any(), any(), any());
when(fileSender.send()).thenReturn(fileSender);
- doNothing().when(service).waitForFinishedFileUpload(any(), any());
}
@Test
@@ -509,16 +497,6 @@ class ForwardingRemoteServiceTest {
verify(fileSender).send();
}
- @Test
- void shouldCallWaitForFinishedFileUploadAfterSend() {
- var inOrder = inOrder(fileSender, service);
-
- sendRepresentations();
-
- inOrder.verify(fileSender).send();
- inOrder.verify(service).waitForFinishedFileUpload(fileSender, inputStream);
- }
-
private void sendRepresentations() {
service.sendRepresentations(List.of(representation), requestObserver);
}
@@ -710,45 +688,17 @@ class ForwardingRemoteServiceTest {
}
@Nested
- class TestWaitForFinishedFileUpload {
+ class TestWaitForCompletion {
@Mock
- private FileSender<GrpcRouteForwardingRequest, GrpcRouteForwardingResponse> fileSender;
- @Mock
- private InputStream inputStream;
- @Mock
- private CompletableFuture<GrpcRouteForwardingResponse> resultFuture;
-
- @BeforeEach
- void mock() {
- when(fileSender.getResultFuture()).thenReturn(resultFuture);
- }
-
- @Test
- void shouldGetResultFuture() {
- waitForFinishedFileUpload();
-
- verify(fileSender).getResultFuture();
- }
+ private CompletableFuture<Void> future;
- @Test
@SneakyThrows
- void shouldGetResultFromFuture() {
- waitForFinishedFileUpload();
-
- verify(resultFuture).get(2, TimeUnit.MINUTES);
- }
-
@Test
- @SneakyThrows
- void shouldCloseInputStream() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
+ void shouldGetFromFuture() {
+ waitForCompletion();
- verify(inputStream).close();
+ verify(future).get(2, TimeUnit.MINUTES);
}
@Nested
@@ -759,47 +709,24 @@ class ForwardingRemoteServiceTest {
@BeforeEach
@SneakyThrows
void mock() {
- when(resultFuture.get(anyLong(), any())).thenThrow(exception);
+ when(future.get(anyLong(), any())).thenThrow(exception);
}
@Test
void shouldThrowTechnicalException() {
- assertThrows(TechnicalException.class, () -> waitForFinishedFileUpload());
+ assertThrows(TechnicalException.class, () -> waitForCompletion());
}
@Test
void shouldInterruptThread() {
try {
- waitForFinishedFileUpload();
+ waitForCompletion();
} catch (TechnicalException e) {
// expected
}
assertThat(Thread.currentThread().isInterrupted()).isTrue();
}
-
- @Test
- void shouldCancelOnError() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(fileSender).cancelOnError(exception);
- }
-
- @Test
- @SneakyThrows
- void shouldCloseInputStream() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(inputStream).close();
- }
}
@Nested
@@ -810,35 +737,12 @@ class ForwardingRemoteServiceTest {
@BeforeEach
@SneakyThrows
void mock() {
- when(resultFuture.get(anyLong(), any())).thenThrow(exception);
+ when(future.get(anyLong(), any())).thenThrow(exception);
}
@Test
void shouldThrowTechnicalException() {
- assertThrows(TechnicalException.class, () -> waitForFinishedFileUpload());
- }
-
- @Test
- void shouldCancelOnError() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(fileSender).cancelOnError(exception);
- }
-
- @Test
- @SneakyThrows
- void shouldCloseInputStream() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(inputStream).close();
+ assertThrows(TechnicalException.class, () -> waitForCompletion());
}
}
@@ -850,64 +754,13 @@ class ForwardingRemoteServiceTest {
@BeforeEach
@SneakyThrows
void mock() {
- when(resultFuture.get(anyLong(), any())).thenThrow(exception);
+ when(future.get(anyLong(), any())).thenThrow(exception);
}
@Test
void shouldThrowTechnicalException() {
- assertThrows(TechnicalException.class, () -> waitForFinishedFileUpload());
+ assertThrows(TechnicalException.class, () -> waitForCompletion());
}
-
- @Test
- void shouldCancelOnTimeout() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(fileSender).cancelOnTimeout();
- }
-
- @Test
- @SneakyThrows
- void shouldCloseInputStream() {
- try {
- waitForFinishedFileUpload();
- } catch (TechnicalException e) {
- // expected
- }
-
- verify(inputStream).close();
- }
- }
-
- private void waitForFinishedFileUpload() {
- service.waitForFinishedFileUpload(fileSender, inputStream);
- }
- }
-
- @Nested
- class TestWaitForCompletion {
-
- @Mock
- private CompletableFuture<Void> future;
-
- @SneakyThrows
- @Test
- void shouldGetFromFuture() {
- waitForCompletion();
-
- verify(future).get();
- }
-
- @SneakyThrows
- @Test
- void shouldThrowTechnicalException() {
- var exception = new RuntimeException();
- when(future.get()).thenThrow(exception);
-
- assertThatThrownBy(this::waitForCompletion).isInstanceOf(TechnicalException.class).hasCause(exception);
}
private void waitForCompletion() {
--
GitLab