Merge pull request 'OZG-5839 Nachricht lässt sich nicht an BayernID schicken'...

Merge pull request 'OZG-5839 Nachricht lässt sich nicht an BayernID schicken' (#7) from OZG-5839-Nachricht-laesst-sich-nicht-an-BayernID-schicken into master

Reviewed-on: https://git.ozg-sh.de/ozgcloud-app/nachrichten-manager/pulls/7


Reviewed-by: OZGCloud <ozgcloud@mgm-tp.com>

nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/postfach/bayernid/FileSender.java

@@ -101,6 +101,10 @@ class FileSender {
 		requestObserver.onNext(chunk);
 	}
 
+	public boolean isDone() {
+		return done.get();
+	}
+
 	@RequiredArgsConstructor
 	class StreamReader {
 

nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/postfach/bayernid/MessageWithFilesSender.java

@@ -27,6 +27,7 @@ import static java.util.Objects.*;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiFunction;
@@ -62,7 +63,8 @@ public class MessageWithFilesSender {
 	private final BiFunction<byte[], Integer, GrpcSendBayernIdMessageRequest> chunkBuilder;
 
 	private CallStreamObserver<GrpcSendBayernIdMessageRequest> requestObserver;
-	private List<FileSender> fileSenders;
+	private int currentAttachmentIndex = 0;
+	private FileSender currentFileSender;
 
 	public MessageWithFilesSender send() {
 		var responseStreamObserver = BinaryFileUploadStreamObserver.create(resultFuture, this::sendNext);
@@ -99,18 +101,40 @@ public class MessageWithFilesSender {
 	}
 
 	void sendAttachments() {
-		if (isNull(fileSenders)) {
-			fileSenders = createFileSenders();
+		if (attachmentIds.isEmpty()) {
+			completeRequest();
+		} else {
+			sendData();
 		}
-		fileSenders.forEach(FileSender::send);
-		completeRequest();
 	}
 
-	List<FileSender> createFileSenders() {
-		return attachmentIds.stream().map(toAttachment).map(this::buildFileSender).toList();
+	void sendData() {
+		Optional<FileSender> fileSender;
+		while ((fileSender = getFileSender()).isPresent() && requestObserver.isReady()) {
+			fileSender.get().send();
+		}
+		if (fileSender.isEmpty()) {
+			completeRequest();
+		}
+	}
+
+	Optional<FileSender> getFileSender() {
+		if (isNull(currentFileSender)) {
+			currentFileSender = buildNextFileSender();
+			return Optional.of(currentFileSender);
+		}
+		if (!currentFileSender.isDone()) {
+			return Optional.of(currentFileSender);
+		}
+		if (currentAttachmentIndex < attachmentIds.size()) {
+			currentFileSender = buildNextFileSender();
+			return Optional.of(currentFileSender);
+		}
+		return Optional.empty();
 	}
 
-	FileSender buildFileSender(BayernIdAttachment attachment) {
+	FileSender buildNextFileSender() {
+		var attachment = toAttachment.apply(attachmentIds.get(currentAttachmentIndex++));
 		return new FileSender(chunkBuilder, requestObserver, attachmentMetadataMapper.apply(attachment), attachment.getContent());
 	}
 

nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/postfach/bayernid/MessageWithFilesSenderTest.java

@@ -28,11 +28,13 @@ import static org.mockito.Mockito.*;
 
 import java.io.InputStream;
 import java.util.List;
+import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
 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.Mock;
@@ -178,149 +180,239 @@ class MessageWithFilesSenderTest {
 	@Nested
 	class TestSendAttachments {
 
-		private String attachmentId = "id1";
+		@Test
+		void shouldCallCompleteRequest() {
+			doNothing().when(messageWithFilesSender).completeRequest();
+
+			messageWithFilesSender.sendAttachments();
+
+			verify(messageWithFilesSender).completeRequest();
+		}
+
+		@Test
+		void shouldCallSendData() {
+			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentIds", List.of("attachmentId"));
+			doNothing().when(messageWithFilesSender).sendData();
+
+			messageWithFilesSender.sendAttachments();
+
+			verify(messageWithFilesSender).sendData();
+		}
+
+	}
+
+	@Nested
+	class TestSendData {
 
 		@Mock
 		private FileSender fileSender;
+		@Mock
+		private CallStreamObserver<GrpcSendBayernIdMessageRequest> requestObserver;
 
 		@BeforeEach
 		void setup() {
-			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentIds", List.of(attachmentId));
-			doNothing().when(messageWithFilesSender).completeRequest();
+			ReflectionTestUtils.setField(messageWithFilesSender, "requestObserver", requestObserver);
 		}
 
 		@Test
-		void shouldCallCreateFileSenders() {
-			doReturn(fileSender).when(messageWithFilesSender).buildFileSender(any());
+		void shouldCallGetFileSender() {
+			doReturn(Optional.of(fileSender)).when(messageWithFilesSender).getFileSender();
 
-			messageWithFilesSender.sendAttachments();
+			messageWithFilesSender.sendData();
 
-			verify(messageWithFilesSender).createFileSenders();
+			verify(messageWithFilesSender).getFileSender();
 		}
 
 		@Test
-		void shouldNotRecreateFileSenders() {
-			ReflectionTestUtils.setField(messageWithFilesSender, "fileSenders", List.of(fileSender));
+		void shouldCallSendOnFileSender() {
+			doReturn(Optional.of(fileSender)).when(messageWithFilesSender).getFileSender();
+			when(requestObserver.isReady()).thenReturn(true, false);
 
-			messageWithFilesSender.sendAttachments();
+			messageWithFilesSender.sendData();
 
-			verify(messageWithFilesSender, never()).createFileSenders();
+			verify(fileSender).send();
 		}
 
 		@Test
-		void shouldCallSendOnFileSender() {
-			doReturn(fileSender).when(messageWithFilesSender).buildFileSender(any());
+		@DisplayName("should not complete request if attachments not sent")
+		void shouldNotCompleteIfNotDone() {
+			doReturn(Optional.of(fileSender)).when(messageWithFilesSender).getFileSender();
+			when(requestObserver.isReady()).thenReturn(true, false);
 
-			messageWithFilesSender.sendAttachments();
+			messageWithFilesSender.sendData();
 
-			verify(fileSender).send();
+			verify(messageWithFilesSender, never()).completeRequest();
 		}
 
 		@Test
-		void shouldCallCompleteRequest() {
-			doReturn(fileSender).when(messageWithFilesSender).buildFileSender(any());
+		void shouldCompleteRequest() {
+			doReturn(Optional.empty()).when(messageWithFilesSender).getFileSender();
 
-			messageWithFilesSender.sendAttachments();
+			messageWithFilesSender.sendData();
 
 			verify(messageWithFilesSender).completeRequest();
 		}
 	}
 
 	@Nested
-	class TestCreateFileSenders {
-
-		private String attachmentId = "id1";
+	class TestGetFileSender {
 
 		@Mock
 		private FileSender fileSender;
 
-		@BeforeEach
-		void setup() {
-			doReturn(fileSender).when(messageWithFilesSender).buildFileSender(any());
-			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentIds", List.of(attachmentId));
-		}
+		@Nested
+		class TestCreateFirstFileSender {
 
-		@Test
-		void shouldCalToAttachment() {
-			messageWithFilesSender.createFileSenders();
+			@BeforeEach
+			void setup() {
+				doReturn(fileSender).when(messageWithFilesSender).buildNextFileSender();
+			}
+
+			@Test
+			void shouldCallBuildNextSender() {
+				messageWithFilesSender.getFileSender();
+
+				verify(messageWithFilesSender).buildNextFileSender();
+			}
+
+			@Test
+			void shouldSetCurrentFileSender() {
+				messageWithFilesSender.getFileSender();
+
+				assertThat(messageWithFilesSender).extracting("currentFileSender").isSameAs(fileSender);
+			}
+
+			@Test
+			void shouldReturnCurrentFileSender() {
+				var result = messageWithFilesSender.getFileSender();
+
+				assertThat(result).containsSame(fileSender);
+			}
 
-			verify(toAttachment).apply(attachmentId);
 		}
 
-		@Test
-		void shouldCallBuildFileSender() {
-			messageWithFilesSender.createFileSenders();
+		@Nested
+		class TestReturnCurrentFileSender {
+
+			@BeforeEach
+			void setup() {
+				ReflectionTestUtils.setField(messageWithFilesSender, "currentFileSender", fileSender);
+			}
+
+			@Test
+			void shouldReturnCurrentFileSender() {
+				var result = messageWithFilesSender.getFileSender();
 
-			verify(messageWithFilesSender).buildFileSender(any());
+				assertThat(result).containsSame(fileSender);
+			}
+
+			@Test
+			void shouldNotCallBuildNextSender() {
+				messageWithFilesSender.getFileSender();
+
+				verify(messageWithFilesSender, never()).buildNextFileSender();
+			}
 		}
 
-		@Test
-		void shouldReturnFileSenders() {
-			var result = messageWithFilesSender.createFileSenders();
+		@Nested
+		class TestCreateNextFileSender {
+
+			@Mock
+			private FileSender nextFileSender;
+
+			@BeforeEach
+			void setup() {
+				when(fileSender.isDone()).thenReturn(true);
+				ReflectionTestUtils.setField(messageWithFilesSender, "currentFileSender", fileSender);
+				ReflectionTestUtils.setField(messageWithFilesSender, "attachmentIds", List.of("attachmentId"));
+				doReturn(nextFileSender).when(messageWithFilesSender).buildNextFileSender();
+			}
+
+			@Test
+			void shouldCallBuildNextSender() {
+				messageWithFilesSender.getFileSender();
+
+				verify(messageWithFilesSender).buildNextFileSender();
+			}
+
+			@Test
+			void shouldSetCurrentFileSender() {
+				messageWithFilesSender.getFileSender();
+
+				assertThat(messageWithFilesSender).extracting("currentFileSender").isSameAs(nextFileSender);
+			}
+
+			@Test
+			void shouldReturnCurrentFileSender() {
+				var result = messageWithFilesSender.getFileSender();
 
-			assertThat(result).containsExactly(fileSender);
+				assertThat(result).containsSame(nextFileSender);
+			}
 		}
+
 	}
 
 	@Nested
-	class TestBuildFileSender {
+	class TestBuildNextFileSender {
 
-		private String attachmentId = "id1";
+		private final String attachmentId = "id1";
 
 		@Mock
 		private CallStreamObserver<GrpcSendBayernIdMessageRequest> requestObserver;
 		@Mock
 		private GrpcSendBayernIdMessageRequest attachmentMetadata;
 
-		@Mock
-		private BayernIdAttachment attachment;
 		@Mock
 		private InputStream inputStream;
 
+		private final BayernIdAttachment attachment = BayernIdAttachmentTestFactory.create();
+
 		@BeforeEach
 		void setup() {
-			ReflectionTestUtils.setField(messageWithFilesSender, "requestObserver", requestObserver);
+			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentIds", List.of(attachmentId));
+			when(toAttachment.apply(any())).thenReturn(attachment);
+		}
+
+		@Test
+		void shouldCallToAttachment() {
+			messageWithFilesSender.buildNextFileSender();
+
+			verify(toAttachment).apply(attachmentId);
 		}
 
 		@Test
 		void shouldSetChunkBuilder() {
-			var result = messageWithFilesSender.buildFileSender(attachment);
+			var result = messageWithFilesSender.buildNextFileSender();
 
 			assertThat(result).extracting("chunkBuilder").isSameAs(chunkBuilder);
 		}
 
 		@Test
 		void shouldSetRequestObserver() {
-			var result = messageWithFilesSender.buildFileSender(attachment);
+			ReflectionTestUtils.setField(messageWithFilesSender, "requestObserver", requestObserver);
+
+			var result = messageWithFilesSender.buildNextFileSender();
 
 			assertThat(result).extracting("requestObserver").isSameAs(requestObserver);
 		}
 
 		@Test
 		void shouldCallAttachmentMetadataMapper() {
-			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentMetadataMapper", attachmentMetadataMapper);
 
-			messageWithFilesSender.buildFileSender(attachment);
+			messageWithFilesSender.buildNextFileSender();
 
 			verify(attachmentMetadataMapper).apply(attachment);
 		}
 
 		@Test
 		void shouldSetAttachmentMetadata() {
-			ReflectionTestUtils.setField(messageWithFilesSender, "attachmentMetadataMapper", attachmentMetadataMapper);
 			when(attachmentMetadataMapper.apply(any())).thenReturn(attachmentMetadata);
 
-			var result = messageWithFilesSender.buildFileSender(attachment);
+			var result = messageWithFilesSender.buildNextFileSender();
 
 			assertThat(result).extracting("metadata").isSameAs(attachmentMetadata);
 		}
 
-		@Test
-		void shouldGetContent() {
-			messageWithFilesSender.buildFileSender(attachment);
-
-			verify(attachment).getContent();
-		}
 	}
 
 	@Nested