From 2d8c38903af63bf65cb2c9363f8851e0179ce731 Mon Sep 17 00:00:00 2001
From: OZGCloud <ozgcloud@mgm-tp.com>
Date: Mon, 29 Apr 2024 21:33:15 +0200
Subject: [PATCH] OZG-5630 implement comments from code review
---
bescheid-manager/pom.xml | 2 +-
.../java/de/ozgcloud/bescheid/Bescheid.java | 2 +-
.../de/ozgcloud/bescheid/BescheidService.java | 14 +--
.../attacheditem/AttachedItemService.java | 34 +++---
.../bescheid/BescheidServiceTest.java | 67 +-----------
.../attacheditem/AttachedItemServiceTest.java | 100 +++++-------------
6 files changed, 52 insertions(+), 167 deletions(-)
diff --git a/bescheid-manager/pom.xml b/bescheid-manager/pom.xml
index 428c8f808..e208cd9f4 100644
--- a/bescheid-manager/pom.xml
+++ b/bescheid-manager/pom.xml
@@ -17,7 +17,7 @@
<properties>
<vorgang-manager.version>2.7.0-SNAPSHOT</vorgang-manager.version>
<nachrichten-manager.version>2.7.0-SNAPSHOT</nachrichten-manager.version>
- <api-lib.version>0.7.0</api-lib.version>
+ <api-lib.version>0.8.0-SNAPSHOT</api-lib.version>
</properties>
<dependencies>
diff --git a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/Bescheid.java b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/Bescheid.java
index c08d2a90c..b39352700 100644
--- a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/Bescheid.java
+++ b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/Bescheid.java
@@ -46,7 +46,7 @@ public class Bescheid {
private Vorgang.ServiceKonto serviceKonto;
public enum Status {
- DRAFT, BESCHEID, SEND;
+ DRAFT, SEND;
public boolean not(String value) {
return !hasValue(value);
diff --git a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/BescheidService.java b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/BescheidService.java
index 3e317170a..c1a19bad9 100644
--- a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/BescheidService.java
+++ b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/BescheidService.java
@@ -1,6 +1,5 @@
package de.ozgcloud.bescheid;
-import java.util.Map;
import java.util.Optional;
import jakarta.annotation.PostConstruct;
@@ -137,12 +136,11 @@ class BescheidService {
}
void sendBescheid(AttachedItem bescheidItem) {
- attachedItemService.patch(setBescheidSendStatus(bescheidItem));
+ attachedItemService.setBescheidStatus(bescheidItem.getId(), bescheidItem.getVersion(), Bescheid.Status.SEND);
try {
vorgangService.bescheiden(bescheidItem.getVorgangId());
} catch (Exception e) {
- var item = attachedItemService.getItem(bescheidItem.getId());
- attachedItemService.patch(setBescheidDraftStatus(item));
+ attachedItemService.setBescheidStatus(bescheidItem.getId(), bescheidItem.getVersion(), Bescheid.Status.DRAFT);
throw e;
}
bescheidClientAttributeService.setAntragResult(bescheidItem.getVorgangId(), getBewilligt(bescheidItem));
@@ -152,14 +150,6 @@ class BescheidService {
return MapUtils.getBooleanValue(bescheidItem.getItem(), Bescheid.FIELD_BEWILLIGT, false);
}
- AttachedItem setBescheidSendStatus(AttachedItem bescheidItem) {
- return bescheidItem.toBuilder().item(Map.of(Bescheid.FIELD_STATUS, Bescheid.Status.SEND.name())).build();
- }
-
- AttachedItem setBescheidDraftStatus(AttachedItem bescheidItem) {
- return bescheidItem.toBuilder().item(Map.of(Bescheid.FIELD_STATUS, Bescheid.Status.DRAFT.name())).build();
- }
-
public BescheidManagerConfig getConfig() {
return BescheidManagerConfig.builder()
.version(buildProperties.getVersion())
diff --git a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/attacheditem/AttachedItemService.java b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/attacheditem/AttachedItemService.java
index 0b2b63407..2865120c7 100644
--- a/bescheid-manager/src/main/java/de/ozgcloud/bescheid/attacheditem/AttachedItemService.java
+++ b/bescheid-manager/src/main/java/de/ozgcloud/bescheid/attacheditem/AttachedItemService.java
@@ -36,6 +36,7 @@ import org.springframework.stereotype.Service;
import de.ozgcloud.apilib.common.command.OzgCloudCommand;
import de.ozgcloud.apilib.common.command.OzgCloudCommandService;
import de.ozgcloud.apilib.common.command.grpc.CommandMapper;
+import de.ozgcloud.bescheid.Bescheid;
import de.ozgcloud.bescheid.BescheidCallContextAttachingInterceptor;
import de.ozgcloud.command.Command;
import de.ozgcloud.common.errorhandling.TechnicalException;
@@ -152,14 +153,14 @@ public class AttachedItemService {
void validateBescheidData(Map<String, Object> bodyObject) {
if (isNull(bodyObject.get(BescheidItem.FIELD_BESCHIEDEN_AM))) {
- throw new TechnicalException("Fields '%s' is required for bescheid creation" .formatted(BescheidItem.FIELD_BESCHIEDEN_AM));
+ throw new TechnicalException("Fields '%s' is required for bescheid creation".formatted(BescheidItem.FIELD_BESCHIEDEN_AM));
}
if (isNull(bodyObject.get(BescheidItem.FIELD_BEWILLIGT))) {
- throw new TechnicalException("Fields '%s' is required for bescheid creation" .formatted(BescheidItem.FIELD_BEWILLIGT));
+ throw new TechnicalException("Fields '%s' is required for bescheid creation".formatted(BescheidItem.FIELD_BEWILLIGT));
}
Optional.ofNullable(MapUtils.getString(bodyObject, BescheidItem.FIELD_SEND_BY)).filter(notExpectedSendByValue)
.ifPresent(sendBy ->
- LOG.warn("Unexpected value for field '%s': %s. Allowed are: %s" .formatted(BescheidItem.FIELD_SEND_BY, sendBy,
+ LOG.warn("Unexpected value for field '%s': %s. Allowed are: %s".formatted(BescheidItem.FIELD_SEND_BY, sendBy,
String.join(", ", BescheidItem.ACCEPTED_SEND_BY_VALUES)))
);
}
@@ -173,7 +174,7 @@ public class AttachedItemService {
void validateBescheidStatus(BescheidItem bescheid) {
var bescheidStatus = MapUtils.getString(bescheid.getBescheidData(), BescheidItem.FIELD_STATUS);
if (BescheidItem.Status.DRAFT.not(bescheidStatus)) {
- throw new TechnicalException("Bescheid draft with ID '%s' has an unexpected status: '%s'" .formatted(bescheid.getId(), bescheidStatus));
+ throw new TechnicalException("Bescheid draft with ID '%s' has an unexpected status: '%s'".formatted(bescheid.getId(), bescheidStatus));
}
}
@@ -190,28 +191,21 @@ public class AttachedItemService {
return remoteService.getItem(id);
}
- public void patch(AttachedItem item) {
- commandService.createAndWaitUntilDone(buildPatchBescheidCommand(item));
+ public void setBescheidStatus(String id, long version, Bescheid.Status status) {
+ commandService.createAndWaitUntilDone(buildPatchBescheidCommand(id, version, status));
}
- OzgCloudCommand buildPatchBescheidCommand(AttachedItem bescheidItem) {
+ OzgCloudCommand buildPatchBescheidCommand(String bescheidId, long bescheidVersion, Bescheid.Status bescheidStatus) {
return OzgCloudCommand.builder()
- .vorgangId(commandMapper.toOzgCloudVorgangId(bescheidItem.getVorgangId()))
- .relationId(commandMapper.mapRelationId(bescheidItem.getId()))
- .relationVersion(bescheidItem.getVersion())
+ .relationId(commandMapper.mapRelationId(bescheidId))
+ .relationVersion(bescheidVersion)
.order(PATCH_ATTACHED_ITEM)
- .bodyObject(buildObjectMap(bescheidItem))
+ .bodyObject(buildObjectMap(bescheidId, bescheidStatus))
.build();
}
- Map<String, Object> buildObjectMap(AttachedItem bescheidItem) {
- var bodyObject = new HashMap<String, Object>();
- bodyObject.put(AttachedItem.PROPERTY_ID, bescheidItem.getId());
- bodyObject.put(AttachedItem.PROPERTY_CLIENT, bescheidItem.getClient());
- bodyObject.put(AttachedItem.PROPERTY_VORGANG_ID, bescheidItem.getVorgangId());
- bodyObject.put(AttachedItem.PROPERTY_ITEM_NAME, bescheidItem.getItemName());
- bodyObject.put(AttachedItem.PROPERTY_VERSION, bescheidItem.getVersion());
- bodyObject.put(AttachedItem.PROPERTY_ITEM, bescheidItem.getItem());
- return bodyObject;
+ Map<String, Object> buildObjectMap(String bescheidId, Bescheid.Status bescheidStatus) {
+ return Map.of(AttachedItem.PROPERTY_ID, bescheidId,
+ AttachedItem.PROPERTY_ITEM, Map.of(Bescheid.FIELD_STATUS, bescheidStatus.name()));
}
}
diff --git a/bescheid-manager/src/test/java/de/ozgcloud/bescheid/BescheidServiceTest.java b/bescheid-manager/src/test/java/de/ozgcloud/bescheid/BescheidServiceTest.java
index 873d36123..a7320efc4 100644
--- a/bescheid-manager/src/test/java/de/ozgcloud/bescheid/BescheidServiceTest.java
+++ b/bescheid-manager/src/test/java/de/ozgcloud/bescheid/BescheidServiceTest.java
@@ -5,7 +5,6 @@ import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.*;
import static org.mockito.Mockito.*;
-import java.util.Collections;
import java.util.Map;
import java.util.Optional;
@@ -26,6 +25,7 @@ import org.springframework.test.util.ReflectionTestUtils;
import de.ozgcloud.bescheid.attacheditem.AttachedItem;
import de.ozgcloud.bescheid.attacheditem.AttachedItemService;
import de.ozgcloud.bescheid.attacheditem.AttachedItemTestFactory;
+import de.ozgcloud.bescheid.attacheditem.BescheidItemTestFactory;
import de.ozgcloud.bescheid.attributes.ClientAttributeService;
import de.ozgcloud.bescheid.common.callcontext.CurrentUserService;
import de.ozgcloud.bescheid.common.callcontext.UserProfile;
@@ -462,19 +462,10 @@ class BescheidServiceTest {
private AttachedItem bescheidSendItem;
@Test
- void shouldCallBuildBescheidSend() {
+ void shouldCallSetBescheidStatusSent() {
sendBescheid();
- verify(service).setBescheidSendStatus(bescheidItem);
- }
-
- @Test
- void shouldCallPatch() {
- doReturn(bescheidSendItem).when(service).setBescheidSendStatus(any());
-
- sendBescheid();
-
- verify(attachedItemService).patch(bescheidSendItem);
+ verify(attachedItemService).setBescheidStatus(BescheidItemTestFactory.ID, BescheidItemTestFactory.VERSION, Bescheid.Status.SEND);
}
@Test
@@ -485,34 +476,12 @@ class BescheidServiceTest {
}
@Test
- void shouldCallGetItem() {
- when(attachedItemService.getItem(any())).thenReturn(AttachedItemTestFactory.createBescheid());
+ void shouldCallSetBescheidStatusDraft() {
doThrow(new TechnicalException("error")).when(vorgangService).bescheiden(anyString());
assertThrows(TechnicalException.class, this::sendBescheid);
- verify(attachedItemService).getItem(AttachedItemTestFactory.ID);
- }
-
- @Test
- void shouldCallSetBescheidDraftStatus() {
- doThrow(new TechnicalException("error")).when(vorgangService).bescheiden(anyString());
- var updatedBescheidUitem = AttachedItemTestFactory.createBescheid();
- when(attachedItemService.getItem(any())).thenReturn(updatedBescheidUitem);
-
- assertThrows(TechnicalException.class, this::sendBescheid);
-
- verify(service).setBescheidDraftStatus(updatedBescheidUitem);
- }
-
- @Test
- void shouldCallPatchWhenBescheidenFails() {
- doReturn(bescheidSendItem).when(service).setBescheidDraftStatus(any());
- doThrow(new TechnicalException("error")).when(vorgangService).bescheiden(anyString());
-
- assertThrows(TechnicalException.class, this::sendBescheid);
-
- verify(attachedItemService).patch(bescheidSendItem);
+ verify(attachedItemService).setBescheidStatus(BescheidItemTestFactory.ID, BescheidItemTestFactory.VERSION, Bescheid.Status.SEND);
}
@Test
@@ -558,32 +527,6 @@ class BescheidServiceTest {
}
}
- @Nested
- class TestSetBescheidSendStatus {
-
- @Test
- void shouldSetSendStatus() {
- var bescheidItem = AttachedItemTestFactory.createBescheid();
-
- var result = service.setBescheidSendStatus(bescheidItem);
-
- assertThat(result.getItem()).containsEntry(Bescheid.FIELD_STATUS, Bescheid.Status.SEND.name());
- }
- }
-
- @Nested
- class TestSetBescheidDraftStatus {
-
- @Test
- void shouldSetDraftStatus() {
- var bescheidItem = AttachedItemTestFactory.createBescheidBuilder().item(Collections.emptyMap()).build();
-
- var result = service.setBescheidDraftStatus(bescheidItem);
-
- assertThat(result.getItem()).containsEntry(Bescheid.FIELD_STATUS, Bescheid.Status.DRAFT.name());
- }
- }
-
@Nested
class TestGetConfig {
diff --git a/bescheid-manager/src/test/java/de/ozgcloud/bescheid/attacheditem/AttachedItemServiceTest.java b/bescheid-manager/src/test/java/de/ozgcloud/bescheid/attacheditem/AttachedItemServiceTest.java
index abb9739ab..4b4b55c3d 100644
--- a/bescheid-manager/src/test/java/de/ozgcloud/bescheid/attacheditem/AttachedItemServiceTest.java
+++ b/bescheid-manager/src/test/java/de/ozgcloud/bescheid/attacheditem/AttachedItemServiceTest.java
@@ -48,6 +48,7 @@ import de.ozgcloud.apilib.common.command.OzgCloudCommandService;
import de.ozgcloud.apilib.common.command.grpc.CommandMapper;
import de.ozgcloud.apilib.common.datatypes.GenericId;
import de.ozgcloud.apilib.vorgang.OzgCloudVorgangId;
+import de.ozgcloud.bescheid.Bescheid;
import de.ozgcloud.bescheid.BescheidCallContextAttachingInterceptor;
import de.ozgcloud.command.Command;
import de.ozgcloud.command.CommandTestFactory;
@@ -759,44 +760,31 @@ class AttachedItemServiceTest {
void shouldCallBuildPatchBescheidCommand() {
var item = AttachedItemTestFactory.createDocument();
- service.patch(item);
+ setBescheidStatus();
- verify(service).buildPatchBescheidCommand(item);
+ verify(service).buildPatchBescheidCommand(AttachedItemTestFactory.ID, AttachedItemTestFactory.VERSION, Bescheid.Status.DRAFT);
}
@Test
void shouldCallCommandService() {
- doReturn(command).when(service).buildPatchBescheidCommand(any());
+ doReturn(command).when(service).buildPatchBescheidCommand(any(), anyLong(), any());
- service.patch(AttachedItemTestFactory.createDocument());
+ setBescheidStatus();
verify(commandService).createAndWaitUntilDone(command);
}
+
+ private void setBescheidStatus() {
+ service.setBescheidStatus(AttachedItemTestFactory.ID, AttachedItemTestFactory.VERSION, Bescheid.Status.DRAFT);
+ }
}
@Nested
class TestBuildPatchBescheidCommand {
- @Test
- void shouldCallVorgangIdMapper() {
- service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
-
- verify(commandMapper).toOzgCloudVorgangId(CommandTestFactory.VORGANG_ID);
- }
-
- @Test
- void shouldSetVorgangId() {
- var expectedVorgangId = OzgCloudVorgangId.from(CommandTestFactory.VORGANG_ID);
- when(commandMapper.toOzgCloudVorgangId(any())).thenReturn(expectedVorgangId);
-
- var result = service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
-
- assertThat(result.getVorgangId()).isEqualTo(expectedVorgangId);
- }
-
@Test
void shouldCallRelationIdMapper() {
- service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
+ buildPatchBescheidCommand();
verify(commandMapper).mapRelationId(AttachedItemTestFactory.ID);
}
@@ -806,92 +794,62 @@ class AttachedItemServiceTest {
var expectedId = GenericId.from(AttachedItemTestFactory.ID);
when(commandMapper.mapRelationId(any())).thenReturn(expectedId);
- var result = service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
+ var result = buildPatchBescheidCommand();
assertThat(result.getRelationId()).isEqualTo(expectedId);
}
@Test
void shouldSetRelationVersion() {
- var result = service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
+ var result = buildPatchBescheidCommand();
assertThat(result.getRelationVersion()).isEqualTo(AttachedItemTestFactory.VERSION);
}
@Test
void shouldSetOrder() {
- var result = service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
+ var result = buildPatchBescheidCommand();
assertThat(result.getOrder()).isEqualTo(AttachedItemService.PATCH_ATTACHED_ITEM);
}
@Test
void shouldCallBuildObjectMap() {
- var bescheidItem = AttachedItemTestFactory.createDocument();
+ var result = buildPatchBescheidCommand();
- service.buildPatchBescheidCommand(bescheidItem);
-
- verify(service).buildObjectMap(bescheidItem);
+ verify(service).buildObjectMap(AttachedItemTestFactory.ID, Bescheid.Status.DRAFT);
}
@Test
void shouldSetBodyObject() {
- var expectedMap = Map.of("key", (Object) "value");
- doReturn(expectedMap).when(service).buildObjectMap(any());
+ var bodyObject = Map.of("key", (Object) "value");
+ doReturn(bodyObject).when(service).buildObjectMap(any(), any());
- var result = service.buildPatchBescheidCommand(AttachedItemTestFactory.createDocument());
+ var result = buildPatchBescheidCommand();
+
+ assertThat(result.getBodyObject()).isSameAs(bodyObject);
+ }
- assertThat(result.getBodyObject()).containsAllEntriesOf(expectedMap);
+ private OzgCloudCommand buildPatchBescheidCommand() {
+ return service.buildPatchBescheidCommand(BescheidItemTestFactory.ID, BescheidItemTestFactory.VERSION, Bescheid.Status.DRAFT);
}
}
@Nested
- class TestBuildBodyObject {
+ class TestBuildObjectMap {
@Test
- void shouldSetId() {
- var result = buildObjectMap();
+ void shouldSetBescheidId() {
+ var result = service.buildObjectMap(AttachedItemTestFactory.ID, Bescheid.Status.DRAFT);
assertThat(result).containsEntry(AttachedItem.PROPERTY_ID, AttachedItemTestFactory.ID);
}
@Test
- void shouldSetClient() {
- var result = buildObjectMap();
-
- assertThat(result).containsEntry(AttachedItem.PROPERTY_CLIENT, AttachedItemTestFactory.CLIENT);
- }
-
- @Test
- void shouldSetVorgangId() {
- var result = buildObjectMap();
-
- assertThat(result).containsEntry(AttachedItem.PROPERTY_VORGANG_ID, CommandTestFactory.VORGANG_ID);
- }
-
- @Test
- void shouldSetItemName() {
- var result = buildObjectMap();
-
- assertThat(result).containsEntry(AttachedItem.PROPERTY_ITEM_NAME, AttachedItemService.BESCHEID_ITEM_NAME);
- }
-
- @Test
- void shouldSetVersion() {
- var result = buildObjectMap();
-
- assertThat(result).containsEntry(AttachedItem.PROPERTY_VERSION, AttachedItemTestFactory.VERSION);
- }
-
- @Test
- void shouldSetItem() {
- var result = buildObjectMap();
-
- assertThat(result).containsEntry(AttachedItem.PROPERTY_ITEM, AttachedItemTestFactory.createBescheidItem());
- }
+ void shouldSetBescheidStatus() {
+ var result = service.buildObjectMap(AttachedItemTestFactory.ID, Bescheid.Status.DRAFT);
- private Map<String, Object> buildObjectMap() {
- return service.buildObjectMap(AttachedItemTestFactory.createBescheid());
+ assertThat(result).extracting(AttachedItem.PROPERTY_ITEM, MAP).containsEntry(Bescheid.FIELD_STATUS, Bescheid.Status.DRAFT.name());
}
}
}
\ No newline at end of file
--
GitLab