From de6964691148f198143baaca0ae611b137cd56cc Mon Sep 17 00:00:00 2001
From: Felix Reichenbach <felix.reichenbach@mgm-tp.com>
Date: Fri, 7 Feb 2025 13:44:43 +0100
Subject: [PATCH] OZG-6354 apply code review comments
---
.../OrganisationsEinheitRemoteService.java | 12 ++-
...ltipleOrganisationsEinheitenException.java | 2 -
.../GrpcOrganisationsEinheitTestFactory.java | 21 ++---
...OrganisationsEinheitRemoteServiceTest.java | 93 ++++++++++++-------
4 files changed, 74 insertions(+), 54 deletions(-)
diff --git a/router/src/main/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteService.java b/router/src/main/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteService.java
index e42d5d210..02f4e6bfd 100644
--- a/router/src/main/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteService.java
+++ b/router/src/main/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteService.java
@@ -29,6 +29,8 @@ import de.ozgcloud.eingang.Application;
import de.ozgcloud.eingang.router.errorhandling.MultipleOrganisationsEinheitenException;
import de.ozgcloud.eingang.router.errorhandling.UnknownOrganisationseinheitException;
import de.ozgcloud.zufi.grpc.organisationseinheit.GrpcGetByOrganisationsEinheitIdRequest;
+import de.ozgcloud.zufi.grpc.organisationseinheit.GrpcGetByOrganisationsEinheitIdResponse;
+import de.ozgcloud.zufi.grpc.organisationseinheit.GrpcOrganisationsEinheit;
import de.ozgcloud.zufi.grpc.organisationseinheit.OrganisationsEinheitServiceGrpc.OrganisationsEinheitServiceBlockingStub;
import lombok.RequiredArgsConstructor;
import net.devh.boot.grpc.client.inject.GrpcClient;
@@ -44,12 +46,20 @@ class OrganisationsEinheitRemoteService {
public OrganisationsEinheit getByOrganisationEinheitId(String id) {
var response = serviceStub
.getByOrganisationsEinheitId(GrpcGetByOrganisationsEinheitIdRequest.newBuilder().setOrganisationsEinheitId(id).build());
+ verifyResponse(id, response);
+ return mapper.fromGrpc(getOrganisationEinheit(response));
+ }
+
+ void verifyResponse(String id, GrpcGetByOrganisationsEinheitIdResponse response) {
if (response.getOrganisationsEinheitenCount() > 1) {
throw new MultipleOrganisationsEinheitenException(id);
}
if (response.getOrganisationsEinheitenCount() == 0) {
throw new UnknownOrganisationseinheitException();
}
- return mapper.fromGrpc(response.getOrganisationsEinheitenList().get(0));
+ }
+
+ private GrpcOrganisationsEinheit getOrganisationEinheit(GrpcGetByOrganisationsEinheitIdResponse response) {
+ return response.getOrganisationsEinheitenList().get(0);
}
}
diff --git a/router/src/main/java/de/ozgcloud/eingang/router/errorhandling/MultipleOrganisationsEinheitenException.java b/router/src/main/java/de/ozgcloud/eingang/router/errorhandling/MultipleOrganisationsEinheitenException.java
index e59a88e4b..2228d9209 100644
--- a/router/src/main/java/de/ozgcloud/eingang/router/errorhandling/MultipleOrganisationsEinheitenException.java
+++ b/router/src/main/java/de/ozgcloud/eingang/router/errorhandling/MultipleOrganisationsEinheitenException.java
@@ -25,8 +25,6 @@ package de.ozgcloud.eingang.router.errorhandling;
public class MultipleOrganisationsEinheitenException extends RuntimeException {
- private static final long serialVersionUID = 1L;
-
public MultipleOrganisationsEinheitenException(String organisationEinheitId) {
super("Multiple Organisationseinheiten with Id %s found!".formatted(organisationEinheitId));
}
diff --git a/router/src/test/java/de/ozgcloud/eingang/common/zufi/GrpcOrganisationsEinheitTestFactory.java b/router/src/test/java/de/ozgcloud/eingang/common/zufi/GrpcOrganisationsEinheitTestFactory.java
index d61ca027d..e25de9618 100644
--- a/router/src/test/java/de/ozgcloud/eingang/common/zufi/GrpcOrganisationsEinheitTestFactory.java
+++ b/router/src/test/java/de/ozgcloud/eingang/common/zufi/GrpcOrganisationsEinheitTestFactory.java
@@ -39,22 +39,13 @@ public class GrpcOrganisationsEinheitTestFactory {
return createBuilder().build();
}
- public static GrpcOrganisationsEinheit createWithoutSynonyme() {
- return createBuilderWithoutSynonyme()
- .build();
- }
-
public static Builder createBuilder() {
- return createBuilderWithoutSynonyme()
- .setSynonyme(SYNONYME);
- }
-
- public static Builder createBuilderWithoutSynonyme() {
return GrpcOrganisationsEinheit.newBuilder()
- .setId(OrganisationsEinheitTestFactory.ID)
- .setName(OrganisationsEinheitTestFactory.NAME)
- .setSynonyme(OrganisationsEinheitTestFactory.SYNONYME)
- .setVorgangManagerAddress(OrganisationsEinheitTestFactory.VORGANG_MANAGER_ADDRESS)
- .setXzufiId(XZUFI_ID);
+ .setId(ID)
+ .setName(NAME)
+ .setSynonyme(SYNONYME)
+ .setVorgangManagerAddress(VORGANG_MANAGER_ADDRESS)
+ .setXzufiId(XZUFI_ID)
+ .setSynonyme(SYNONYME);
}
}
\ No newline at end of file
diff --git a/router/src/test/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteServiceTest.java b/router/src/test/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteServiceTest.java
index 3561c1f85..52004608e 100644
--- a/router/src/test/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteServiceTest.java
+++ b/router/src/test/java/de/ozgcloud/eingang/common/zufi/OrganisationsEinheitRemoteServiceTest.java
@@ -24,6 +24,7 @@
package de.ozgcloud.eingang.common.zufi;
import static org.assertj.core.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
@@ -32,6 +33,7 @@ import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.mockito.InjectMocks;
import org.mockito.Mock;
+import org.mockito.Spy;
import de.ozgcloud.eingang.router.errorhandling.MultipleOrganisationsEinheitenException;
import de.ozgcloud.eingang.router.errorhandling.UnknownOrganisationseinheitException;
@@ -41,6 +43,7 @@ import de.ozgcloud.zufi.grpc.organisationseinheit.OrganisationsEinheitServiceGrp
class OrganisationsEinheitRemoteServiceTest {
@InjectMocks
+ @Spy
private OrganisationsEinheitRemoteService service;
@Mock
private OrganisationsEinheitServiceBlockingStub stub;
@@ -50,53 +53,62 @@ class OrganisationsEinheitRemoteServiceTest {
@Nested
class TestGetByOrganisationEinheitId {
- @Nested
- class TestOnSingleOrganisationEinheitFound {
+ private final GrpcGetByOrganisationsEinheitIdResponse response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.create();
+ private final OrganisationsEinheit mappedOrganisationsEinheit = OrganisationsEinheitTestFactory.create();
- private final GrpcGetByOrganisationsEinheitIdResponse response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.create();
- private final OrganisationsEinheit mappedOrganisationsEinheit = OrganisationsEinheitTestFactory.create();
+ @BeforeEach
+ void mock() {
+ when(stub.getByOrganisationsEinheitId(any())).thenReturn(response);
+ when(mapper.fromGrpc(any())).thenReturn(mappedOrganisationsEinheit);
+ doNothing().when(service).verifyResponse(any(), any());
+ }
- @BeforeEach
- void mock() {
- when(stub.getByOrganisationsEinheitId(any())).thenReturn(response);
- when(mapper.fromGrpc(any())).thenReturn(mappedOrganisationsEinheit);
- }
+ @Test
+ void shouldCallRemoteService() {
+ service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
- @Test
- void shouldCallRemoteService() {
- service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
+ verify(stub).getByOrganisationsEinheitId(GrpcGetByOrganisationsEinheitIdRequestTestFactory.create());
+ }
- verify(stub).getByOrganisationsEinheitId(GrpcGetByOrganisationsEinheitIdRequestTestFactory.create());
- }
+ @Test
+ void shouldCallVerifyResponse() {
+ service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
- @Test
- void shouldCallMapper() {
- service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
+ verify(service).verifyResponse(OrganisationsEinheitTestFactory.ID, response);
+ }
- verify(mapper).fromGrpc(GrpcGetByOrganisationsEinheitIdResponseTestFactory.ORGANISATIONS_EINHEIT);
- }
+ @Test
+ void shouldCallMapperAfterVerifyingResponse() {
+ var order = inOrder(service, mapper);
- @Test
- void shouldReturnValue() {
- var organisationsEinheit = service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
+ service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
- assertThat(organisationsEinheit).isEqualTo(mappedOrganisationsEinheit);
- }
+ order.verify(service).verifyResponse(any(), any());
+ order.verify(mapper).fromGrpc(GrpcGetByOrganisationsEinheitIdResponseTestFactory.ORGANISATIONS_EINHEIT);
+ }
+
+ @Test
+ void shouldReturnMappedOrganisationsEinheit() {
+ var organisationsEinheit = service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID);
+
+ assertThat(organisationsEinheit).isEqualTo(mappedOrganisationsEinheit);
}
+ }
+
+ @Nested
+ class TestVerifyResponse {
@Nested
class TestOnMultipleOrganisationEinheitFound {
- private final GrpcGetByOrganisationsEinheitIdResponse response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.createBuilder()
- .addOrganisationsEinheiten(GrpcOrganisationsEinheitTestFactory.create())
- .build();
-
@Test
void shouldThrowMultipleOrganisationsEinheitenException() {
- when(stub.getByOrganisationsEinheitId(any())).thenReturn(response);
+ var response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.createBuilder()
+ .addOrganisationsEinheiten(GrpcOrganisationsEinheitTestFactory.create())
+ .build();
assertThatExceptionOfType(MultipleOrganisationsEinheitenException.class)
- .isThrownBy(() -> service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID));
+ .isThrownBy(() -> service.verifyResponse(OrganisationsEinheitTestFactory.ID, response));
}
}
@@ -104,17 +116,26 @@ class OrganisationsEinheitRemoteServiceTest {
@Nested
class TestOnNoOrganisationEinheitFound {
- private final GrpcGetByOrganisationsEinheitIdResponse response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.createBuilder()
- .clearOrganisationsEinheiten()
- .build();
-
@Test
void shouldThrowUnknownOrganisationseinheitException() {
- when(stub.getByOrganisationsEinheitId(any())).thenReturn(response);
+ var response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.createBuilder()
+ .clearOrganisationsEinheiten()
+ .build();
assertThatExceptionOfType(UnknownOrganisationseinheitException.class)
- .isThrownBy(() -> service.getByOrganisationEinheitId(OrganisationsEinheitTestFactory.ID));
+ .isThrownBy(() -> service.verifyResponse(OrganisationsEinheitTestFactory.ID, response));
+
+ }
+ }
+
+ @Nested
+ class TestOnSingleOrganisationEinheitFound {
+
+ @Test
+ void shouldNotThrowException() {
+ var response = GrpcGetByOrganisationsEinheitIdResponseTestFactory.create();
+ assertDoesNotThrow(() -> service.verifyResponse(OrganisationsEinheitTestFactory.ID, response));
}
}
}
--
GitLab