From 1f50cd907a05c6df1156e821d7e2dba542c32458 Mon Sep 17 00:00:00 2001 From: OZGCloud <ozgcloud@mgm-tp.com> Date: Mon, 21 Oct 2024 16:40:07 +0200 Subject: [PATCH] OZG-6944 Code review comments --- .../src/main/protobuf/antragraum.proto | 2 +- nachrichten-manager-server/pom.xml | 16 +++--- .../antragraum/AntragraumGrpcService.java | 8 +-- .../antragraum/AntragraumService.java | 12 ++--- .../antragraum/AttachmentFileRequest.java | 37 +++++++++++++- .../AttachmentFileRequestMapper.java | 4 +- .../antragraum/AntragraumGrpcServiceTest.java | 51 +++++-------------- .../AttachmentFileRequestMapperTest.java | 31 +++++++++-- .../AttachmentFileRequestTestFactory.java | 11 +++- 9 files changed, 104 insertions(+), 68 deletions(-) diff --git a/nachrichten-manager-interface/src/main/protobuf/antragraum.proto b/nachrichten-manager-interface/src/main/protobuf/antragraum.proto index f5f2074..b82fb27 100644 --- a/nachrichten-manager-interface/src/main/protobuf/antragraum.proto +++ b/nachrichten-manager-interface/src/main/protobuf/antragraum.proto @@ -44,7 +44,7 @@ service AntragraumService { rpc GetAttachmentContent (GrpcGetAttachmentContentRequest) returns (stream GrpcGetAttachmentContentResponse){ } - rpc GetAttachmentMetadata (GrpcGetAttachmentMetadataRequest) returns (stream GrpcGetAttachmentMetadataResponse){ + rpc GetAttachmentMetadata (GrpcGetAttachmentMetadataRequest) returns (GrpcGetAttachmentMetadataResponse){ } } \ No newline at end of file diff --git a/nachrichten-manager-server/pom.xml b/nachrichten-manager-server/pom.xml index fffa802..0292cdc 100644 --- a/nachrichten-manager-server/pom.xml +++ b/nachrichten-manager-server/pom.xml @@ -48,7 +48,7 @@ <bayernid-proxy-interface.version>0.1.0</bayernid-proxy-interface.version> <vorgang-manager.version>2.15.0</vorgang-manager.version> <muk-postfach.version>0.1.0-SNAPSHOT</muk-postfach.version> - <ozgcloud-starter.version>0.13.0-SNAPSHOT</ozgcloud-starter.version> + <api-lib.version>0.13.0-SNAPSHOT</api-lib.version> </properties> <dependencies> @@ -88,18 +88,18 @@ <version>${muk-postfach.version}</version> </dependency> - <dependency> - <groupId>de.ozgcloud.api-lib</groupId> - <artifactId>ozg-cloud-spring-boot-starter</artifactId> - <version>${ozgcloud-starter.version}</version> - </dependency> - <dependency> <groupId>de.ozgcloud.info</groupId> <artifactId>info-manager-interface</artifactId> <version>${ozg-info-manager-interface.version}</version> </dependency> + <dependency> + <groupId>de.ozgcloud.api-lib</groupId> + <artifactId>api-lib-core</artifactId> + <version>${api-lib.version}</version> + </dependency> + <dependency> <groupId>org.springframework.boot</groupId> <artifactId>spring-boot-starter-mail</artifactId> @@ -225,7 +225,7 @@ <dependency> <groupId>de.ozgcloud.api-lib</groupId> <artifactId>api-lib-core</artifactId> - <version>${ozgcloud-starter.version}</version> + <version>${api-lib.version}</version> <type>test-jar</type> <scope>test</scope> </dependency> diff --git a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcService.java b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcService.java index a1b095a..0f96740 100644 --- a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcService.java +++ b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcService.java @@ -43,7 +43,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import com.google.protobuf.ByteString; import de.ozgcloud.apilib.common.errorhandling.NotFoundException; -import de.ozgcloud.apilib.file.OzgCloudFile; import de.ozgcloud.common.errorhandling.TechnicalException; import de.ozgcloud.nachrichten.NachrichtenManagerConfiguration; import de.ozgcloud.nachrichten.common.vorgang.VorgangService; @@ -152,7 +151,7 @@ class AntragraumGrpcService extends AntragraumServiceGrpc.AntragraumServiceImplB } void getAttachmentFileContent(GrpcGetAttachmentContentRequest request, PipedOutputStream pipedOutputStream) { - service.getAttachmentContent(attachmentFileRequestMapper.fromGrpcContentRequest(request), pipedOutputStream); + service.getAttachmentContent(attachmentFileRequestMapper.fromContentRequest(request), pipedOutputStream); } void sendFileContent(InputStream fileContent, StreamObserver<GrpcGetAttachmentContentResponse> responseObserver) { @@ -201,11 +200,8 @@ class AntragraumGrpcService extends AntragraumServiceGrpc.AntragraumServiceImplB @Override public void getAttachmentMetadata(GrpcGetAttachmentMetadataRequest request, StreamObserver<GrpcGetAttachmentMetadataResponse> responseObserver) { - var attachment = service.getAttachmentMetadata(attachmentFileRequestMapper.fromGrpcMetadataRequest(request)); - sendFileMetadata(attachment, responseObserver); - } + var attachment = service.getAttachmentMetadata(attachmentFileRequestMapper.fromMetadataRequest(request)); - void sendFileMetadata(OzgCloudFile attachment, StreamObserver<GrpcGetAttachmentMetadataResponse> responseObserver) { responseObserver.onNext(GrpcGetAttachmentMetadataResponse.newBuilder() .setFileMetadata(ozgCloudFileMapper.toMetadata(attachment)).build()); diff --git a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumService.java b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumService.java index 61e1c01..5da9037 100644 --- a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumService.java +++ b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AntragraumService.java @@ -208,7 +208,7 @@ public class AntragraumService { } try (var dataOutput = new DataOutputStream(pipedOutputStream)) { - ozgCloudFileService.writeFileDataToStream(OzgCloudFileId.from(request.fileId()), dataOutput); + ozgCloudFileService.writeFileDataToStream(OzgCloudFileId.from(request.getFileId()), dataOutput); } catch (IOException e) { throw new TechnicalException("Error on getting attachment file content.", e); } @@ -216,14 +216,14 @@ public class AntragraumService { public OzgCloudFile getAttachmentMetadata(AttachmentFileRequest request) { verifyAccessToFile(request); - return ozgCloudFileService.getFile(OzgCloudFileId.from(request.fileId())); + return ozgCloudFileService.getFile(OzgCloudFileId.from(request.getFileId())); } void verifyAccessToFile(AttachmentFileRequest request) { - verifyToken(request.samlToken()); - var nachricht = attachedItemService.getPostfachNachricht(request.nachrichtId()); - verifyAccessToVorgang(request.samlToken(), nachricht); - verifyFileId(request.fileId(), nachricht); + verifyToken(request.getSamlToken()); + var nachricht = attachedItemService.getPostfachNachricht(request.getNachrichtId()); + verifyAccessToVorgang(request.getSamlToken(), nachricht); + verifyFileId(request.getFileId(), nachricht); } void verifyToken(String token) { diff --git a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequest.java b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequest.java index 0ec4c7b..51d0c97 100644 --- a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequest.java +++ b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequest.java @@ -1,4 +1,39 @@ +/* + * 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.nachrichten.antragraum; -public record AttachmentFileRequest(String samlToken, String nachrichtId, String fileId) { +import lombok.Builder; +import lombok.EqualsAndHashCode; +import lombok.Getter; + +@Builder +@Getter +@EqualsAndHashCode +public class AttachmentFileRequest { + + private String samlToken; + private String nachrichtId; + private String fileId; + } diff --git a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapper.java b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapper.java index 82319b6..fc6bfb5 100644 --- a/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapper.java +++ b/nachrichten-manager-server/src/main/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapper.java @@ -7,7 +7,7 @@ import org.mapstruct.ReportingPolicy; @Mapper(nullValueCheckStrategy = NullValueCheckStrategy.ALWAYS, unmappedTargetPolicy = ReportingPolicy.WARN) interface AttachmentFileRequestMapper { - AttachmentFileRequest fromGrpcContentRequest(GrpcGetAttachmentContentRequest request); + AttachmentFileRequest fromContentRequest(GrpcGetAttachmentContentRequest request); - AttachmentFileRequest fromGrpcMetadataRequest(GrpcGetAttachmentMetadataRequest request); + AttachmentFileRequest fromMetadataRequest(GrpcGetAttachmentMetadataRequest request); } diff --git a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcServiceTest.java b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcServiceTest.java index 6e99cae..aeb61e1 100644 --- a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcServiceTest.java +++ b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AntragraumGrpcServiceTest.java @@ -640,14 +640,14 @@ class AntragraumGrpcServiceTest { @BeforeEach void mock() { - when(attachmentFileRequestMapper.fromGrpcContentRequest(grpcRequest)).thenReturn(request); + when(attachmentFileRequestMapper.fromContentRequest(grpcRequest)).thenReturn(request); } @Test void shouldMapRequest() { callGetRueckfrageAttachmentFile(); - verify(attachmentFileRequestMapper).fromGrpcContentRequest(grpcRequest); + verify(attachmentFileRequestMapper).fromContentRequest(grpcRequest); } @Test @@ -908,21 +908,21 @@ class AntragraumGrpcServiceTest { private final GrpcGetAttachmentMetadataRequest grpcRequest = GrpcGetAttachmentMetadataRequestTestFactory.create(); private final AttachmentFileRequest request = AttachmentFileRequestTestFactory.create(); - - private OzgCloudFile attachment = OzgCloudFileTestFactory.create(); + private final OzgCloudFile attachment = OzgCloudFileTestFactory.create(); + private final GrpcFileMetadata metadata = GrpcFileMetadataTestFactory.create(); @BeforeEach void mock() { - when(attachmentFileRequestMapper.fromGrpcMetadataRequest(grpcRequest)).thenReturn(request); + when(attachmentFileRequestMapper.fromMetadataRequest(grpcRequest)).thenReturn(request); when(service.getAttachmentMetadata(request)).thenReturn(attachment); - doNothing().when(grpcService).sendFileMetadata(attachment, responseObserver); + when(ozgCloudFileMapper.toMetadata(attachment)).thenReturn(metadata); } @Test void shouldMapRequest() { callGetAttachmentMetadata(); - verify(attachmentFileRequestMapper).fromGrpcMetadataRequest(grpcRequest); + verify(attachmentFileRequestMapper).fromMetadataRequest(grpcRequest); } @Test @@ -932,56 +932,29 @@ class AntragraumGrpcServiceTest { verify(service).getAttachmentMetadata(request); } - @Test - void shouldCallSendFileMetadata() { - callGetAttachmentMetadata(); - - verify(grpcService).sendFileMetadata(attachment, responseObserver); - } - - private void callGetAttachmentMetadata() { - grpcService.getAttachmentMetadata(grpcRequest, responseObserver); - } - } - - @Nested - class TestSendFileMetadata { - - @Mock - private StreamObserver<GrpcGetAttachmentMetadataResponse> responseObserver; - - private final OzgCloudFile attachment = OzgCloudFileTestFactory.create(); - - private final GrpcFileMetadata metadata = GrpcFileMetadataTestFactory.create(); - - @BeforeEach - void mock() { - when(ozgCloudFileMapper.toMetadata(attachment)).thenReturn(metadata); - } - @Test void shouldMapMetadata() { - callSendFileMetadata(); + callGetAttachmentMetadata(); verify(ozgCloudFileMapper).toMetadata(attachment); } @Test void shouldSendMetadata() { - callSendFileMetadata(); + callGetAttachmentMetadata(); verify(responseObserver).onNext(argThat((response) -> response.getFileMetadata().equals(metadata))); } @Test void shouldCallOnCompleted() { - callSendFileMetadata(); + callGetAttachmentMetadata(); verify(responseObserver).onCompleted(); } - private void callSendFileMetadata() { - grpcService.sendFileMetadata(attachment, responseObserver); + private void callGetAttachmentMetadata() { + grpcService.getAttachmentMetadata(grpcRequest, responseObserver); } } diff --git a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapperTest.java b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapperTest.java index 2b39fbd..f76b44c 100644 --- a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapperTest.java +++ b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestMapperTest.java @@ -1,3 +1,26 @@ +/* + * 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.nachrichten.antragraum; import static org.assertj.core.api.Assertions.*; @@ -11,22 +34,22 @@ class AttachmentFileRequestMapperTest { private final AttachmentFileRequestMapper mapper = Mappers.getMapper(AttachmentFileRequestMapper.class); @Nested - class TestFromGrpcContentRequest { + class TestFromContentRequest { @Test void shouldMapToRueckfrageAttachmentFileRequest() { - var request = mapper.fromGrpcContentRequest(GrpcGetAttachmentContentRequestTestFactory.create()); + var request = mapper.fromContentRequest(GrpcGetAttachmentContentRequestTestFactory.create()); assertThat(request).isEqualTo(AttachmentFileRequestTestFactory.create()); } } @Nested - class TestFromGrpcMetadataRequest { + class TestFromMetadataRequest { @Test void shouldMapToRueckfrageAttachmentFileRequest() { - var request = mapper.fromGrpcMetadataRequest(GrpcGetAttachmentMetadataRequestTestFactory.create()); + var request = mapper.fromMetadataRequest(GrpcGetAttachmentMetadataRequestTestFactory.create()); assertThat(request).isEqualTo(AttachmentFileRequestTestFactory.create()); } diff --git a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestTestFactory.java b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestTestFactory.java index bf4269a..5697bb2 100644 --- a/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestTestFactory.java +++ b/nachrichten-manager-server/src/test/java/de/ozgcloud/nachrichten/antragraum/AttachmentFileRequestTestFactory.java @@ -4,6 +4,7 @@ import java.util.UUID; import com.thedeanda.lorem.LoremIpsum; +import de.ozgcloud.nachrichten.antragraum.AttachmentFileRequest.AttachmentFileRequestBuilder; import de.ozgcloud.nachrichten.postfach.PostfachNachrichtTestFactory; public class AttachmentFileRequestTestFactory { @@ -13,6 +14,14 @@ public class AttachmentFileRequestTestFactory { public static final String NACHRICHT_ID = PostfachNachrichtTestFactory.ID; public static AttachmentFileRequest create() { - return new AttachmentFileRequest(TOKEN, NACHRICHT_ID, FILE_ID); + return createBuilder() + .build(); + } + + public static AttachmentFileRequestBuilder createBuilder() { + return AttachmentFileRequest.builder() + .samlToken(TOKEN) + .fileId(FILE_ID) + .nachrichtId(NACHRICHT_ID); } } -- GitLab