From 78fd1324acc44a3466e9d437a86c3d5cdf9f0f60 Mon Sep 17 00:00:00 2001 From: OZGCloud <ozgcloud@mgm-tp.com> Date: Wed, 11 Dec 2024 14:47:16 +0100 Subject: [PATCH] OZG-7037 apply more code review comments --- .../common/command/CommandService.java | 13 ++++++------ .../archive/vorgang/ArchiveEventListener.java | 9 +++++---- .../archive/vorgang/VorgangService.java | 2 -- .../common/command/CommandServiceTest.java | 20 +++++++++++++------ .../JsonToXtaIdentifierListConverterTest.java | 5 ++--- .../common/xta/XtaIdentifierTestFactory.java | 4 ++++ .../archive/common/xta/XtaServiceTest.java | 4 ++-- .../export/ExportGrpcServiceITCase.java | 4 ++-- .../archive/export/ExportServiceTest.java | 4 ++-- .../export/XdomeaPropertiesValidatorTest.java | 4 ++-- .../vorgang/ArchiveEventListenerTest.java | 18 +++++++++-------- .../archive/vorgang/KopfCreatorTest.java | 4 ++-- .../archive/vorgang/VorgangServiceTest.java | 4 ---- 13 files changed, 52 insertions(+), 43 deletions(-) diff --git a/archive-manager-server/src/main/java/de/ozgcloud/archive/common/command/CommandService.java b/archive-manager-server/src/main/java/de/ozgcloud/archive/common/command/CommandService.java index 848517e..86bf272 100644 --- a/archive-manager-server/src/main/java/de/ozgcloud/archive/common/command/CommandService.java +++ b/archive-manager-server/src/main/java/de/ozgcloud/archive/common/command/CommandService.java @@ -4,6 +4,7 @@ import java.util.Optional; import java.util.function.Predicate; import java.util.stream.Stream; +import org.apache.commons.lang3.StringUtils; import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Service; @@ -18,10 +19,6 @@ import lombok.RequiredArgsConstructor; @RequiredArgsConstructor public class CommandService { - public static final String ARCHIVE_VORGANG_ORDER = "ARCHIVE_VORGANG"; - private static final Predicate<? super OzgCloudCommand> IS_NOT_ARCHIVE_VORGANG_COMMAND = command -> !ARCHIVE_VORGANG_ORDER - .equals(command.getOrder()); - @Qualifier(ArchiveManagerConfiguration.COMMAND_REMOTE_SERVICE_NAME) // NOSONAR private final CommandRemoteService remoteService; @Qualifier(ArchiveManagerConfiguration.OZGCLOUD_COMMAND_SERVICE_NAME) @@ -31,9 +28,13 @@ public class CommandService { return remoteService.findCommands(vorgangId, Optional.of("FINISHED"), Optional.empty()); } - public boolean hasPendingCommandsExceptArchive(String vorgangId) { + public boolean hasPendingCommandsExceptWithOrder(String vorgangId, String order) { var pendingCommands = ozgCloudCommandService.getPendingCommands(OzgCloudVorgangId.from(vorgangId)); - return pendingCommands.stream().anyMatch(IS_NOT_ARCHIVE_VORGANG_COMMAND); + return pendingCommands.stream().anyMatch(hasNotOrder(order)); + } + + private Predicate<? super OzgCloudCommand> hasNotOrder(String order) { + return command -> !StringUtils.equals(command.getOrder(), order); } } diff --git a/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/ArchiveEventListener.java b/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/ArchiveEventListener.java index 1dd0868..e863cb3 100644 --- a/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/ArchiveEventListener.java +++ b/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/ArchiveEventListener.java @@ -52,13 +52,14 @@ import lombok.extern.log4j.Log4j2; @Log4j2 class ArchiveEventListener { - private static final int MAXIMUM_CHECKS_FOR_PENDING_COMMANDS = 3; + static final int MAXIMUM_CHECKS_FOR_PENDING_COMMANDS = 3; static final int WAIT_INTERVAL = 30 * 1000; private static final String LOG_MESSAGE_TEMPLATE = "{}. Command failed."; private static final String ERROR_MESSAGE_TEMPLATE = "Error on executing %s Command (id: %s)."; + static final String ARCHIVE_VORGANG_ORDER = "ARCHIVE_VORGANG"; private static final String IS_ARCHIVE_VORGANG_EVENT = "{T(de.ozgcloud.archive.vorgang.ArchiveEventListener).IS_ARCHIVE_VORGANG_COMMAND.test(event.getSource())}"; - public static final Predicate<Command> IS_ARCHIVE_VORGANG_COMMAND = command -> CommandService.ARCHIVE_VORGANG_ORDER.equals(command.getOrder()); + public static final Predicate<Command> IS_ARCHIVE_VORGANG_COMMAND = command -> ARCHIVE_VORGANG_ORDER.equals(command.getOrder()); private static final String IS_LOCKED_BY_ARCHIVE_MANAGER_EVENT = "{T(de.ozgcloud.archive.vorgang.ArchiveEventListener)." + "IS_LOCK_BY_ARCHIVE_MANAGER_COMMAND.test(event.getCommand())}"; @@ -96,7 +97,7 @@ class ArchiveEventListener { void waitForPendingCommandsToFinish(String vorgangId, long waitIntervalInMillis) { var numberOfAttempts = 0; - while (commandService.hasPendingCommandsExceptArchive(vorgangId)) { + while (commandService.hasPendingCommandsExceptWithOrder(vorgangId, ARCHIVE_VORGANG_ORDER)) { numberOfAttempts++; if (numberOfAttempts >= MAXIMUM_CHECKS_FOR_PENDING_COMMANDS) { throw new TimeoutException("Waiting for pending commands"); @@ -104,7 +105,7 @@ class ArchiveEventListener { try { Thread.sleep(waitIntervalInMillis); } catch (InterruptedException e) { - LOG.error("Error while waiting for commads to finish.", e); + LOG.error("Error while waiting for commands to finish.", e); Thread.currentThread().interrupt(); } } diff --git a/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/VorgangService.java b/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/VorgangService.java index 06a201c..497e0a0 100644 --- a/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/VorgangService.java +++ b/archive-manager-server/src/main/java/de/ozgcloud/archive/vorgang/VorgangService.java @@ -23,11 +23,9 @@ import de.xoev.xdomea.IdentifikationObjektType; import de.xoev.xdomea.NkAbgabeType; import de.xoev.xdomea.VorgangType; import lombok.RequiredArgsConstructor; -import lombok.extern.log4j.Log4j2; @RequiredArgsConstructor @Service(ArchiveManagerConfiguration.VORGANG_SERVICE_NAME) // NOSONAR -@Log4j2 public class VorgangService { static final String LOCK_VORGANG_ORDER = "LOCK_VORGANG"; diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/command/CommandServiceTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/command/CommandServiceTest.java index 3398cfa..4e23201 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/command/CommandServiceTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/command/CommandServiceTest.java @@ -14,6 +14,8 @@ import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Spy; +import com.thedeanda.lorem.LoremIpsum; + import de.ozgcloud.apilib.common.command.OzgCloudCommandService; import de.ozgcloud.apilib.common.command.grpc.OzgCloudCommandTestFactory; import de.ozgcloud.apilib.vorgang.OzgCloudVorgangId; @@ -44,9 +46,11 @@ class CommandServiceTest { @Nested class TestHasPendingCommandsExceptArchive { + private final String order = LoremIpsum.getInstance().getWords(1); + @Test void shouldCallOzgCloudCommandService() { - service.hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + hasPendingCommandsExceptWithOrder(); verify(ozgCloudCommandService).getPendingCommands(OzgCloudVorgangId.from(VorgangWithEingangTestFactory.ID)); } @@ -55,7 +59,7 @@ class CommandServiceTest { void shouldReturnFalseOnNoPendingCommands() { when(ozgCloudCommandService.getPendingCommands(any())).thenReturn(Collections.emptyList()); - var result = service.hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + var result = hasPendingCommandsExceptWithOrder(); assertThat(result).isFalse(); } @@ -64,20 +68,24 @@ class CommandServiceTest { void shouldReturnTrueOnPendingCommands() { when(ozgCloudCommandService.getPendingCommands(any())).thenReturn(List.of(OzgCloudCommandTestFactory.create())); - var result = service.hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + var result = hasPendingCommandsExceptWithOrder(); assertThat(result).isTrue(); } @Test - void shouldReturnFalseOnArchiveCommand() { + void shouldReturnFalseOnCommandWithExcpetionOrder() { when(ozgCloudCommandService.getPendingCommands(any())).thenReturn(List.of(OzgCloudCommandTestFactory.createBuilder() - .order(CommandService.ARCHIVE_VORGANG_ORDER) + .order(order) .build())); - var result = service.hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + var result = hasPendingCommandsExceptWithOrder(); assertThat(result).isFalse(); } + + private boolean hasPendingCommandsExceptWithOrder() { + return service.hasPendingCommandsExceptWithOrder(VorgangWithEingangTestFactory.ID, order); + } } } diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/JsonToXtaIdentifierListConverterTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/JsonToXtaIdentifierListConverterTest.java index d89b167..be5d9a9 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/JsonToXtaIdentifierListConverterTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/JsonToXtaIdentifierListConverterTest.java @@ -17,7 +17,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import de.ozgcloud.common.errorhandling.TechnicalException; import lombok.SneakyThrows; -public class JsonToXtaIdentifierListConverterTest { +class JsonToXtaIdentifierListConverterTest { @InjectMocks private JsonToXtaIdentifierListConverter converter; @@ -30,8 +30,7 @@ public class JsonToXtaIdentifierListConverterTest { @Test void shouldReturnListOfXtaIdentifier() { - var jsonInput = "[{\"category\":\"%s\",\"name\":\"%s\",\"value\":\"%s\"}]" - .formatted(XtaIdentifierTestFactory.CATEGORY, XtaIdentifierTestFactory.NAME, XtaIdentifierTestFactory.VALUE); + var jsonInput = XtaIdentifierTestFactory.createAsJson(); var identifiers = converter.convert(jsonInput); diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaIdentifierTestFactory.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaIdentifierTestFactory.java index 65ae101..35f2f02 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaIdentifierTestFactory.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaIdentifierTestFactory.java @@ -23,4 +23,8 @@ public class XtaIdentifierTestFactory { .value(VALUE) .category(CATEGORY); } + + public static String createAsJson() { + return "[{\"category\":\"%s\",\"name\":\"%s\",\"value\":\"%s\"}]".formatted(CATEGORY, NAME, VALUE); + } } diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaServiceTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaServiceTest.java index d2d9ca1..536adb3 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaServiceTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/common/xta/XtaServiceTest.java @@ -149,7 +149,7 @@ class XtaServiceTest { private XtaService xtaService; @Nested - class OnConfiguredXtaClient { + class TestOnConfiguredXtaClient { @BeforeEach void givenXtaClientConfigured() { @@ -166,7 +166,7 @@ class XtaServiceTest { } @Nested - class OnUnconfiguredXtaClient { + class TestOnUnconfiguredXtaClient { @BeforeEach void givenXtaClientNotConfigured() { diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportGrpcServiceITCase.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportGrpcServiceITCase.java index 713288d..f31301a 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportGrpcServiceITCase.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportGrpcServiceITCase.java @@ -98,7 +98,7 @@ class ExportGrpcServiceITCase { } @Nested - class OnMinimalVorgang { + class TestOnMinimalVorgang { @BeforeEach void setUpMock() { @@ -135,7 +135,7 @@ class ExportGrpcServiceITCase { } @Nested - class OnVorgangWithKommentarAndAttachment { + class TestOnVorgangWithKommentarAndAttachment { private final String attachmentContent = LoremIpsum.getInstance().getWords(5); @BeforeEach diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportServiceTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportServiceTest.java index 1d7f13c..128073a 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportServiceTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/ExportServiceTest.java @@ -767,7 +767,7 @@ class ExportServiceTest { private final String fileNameId = UUID.randomUUID().toString(); @Nested - class OnNoExceptionThrown { + class TestOnNoExceptionThrown { private MockedConstruction<FileDataSource> fileDataSourceConstruction; private FileDataSource fileDataSource; private File dataSourceFile; @@ -851,7 +851,7 @@ class ExportServiceTest { } @Nested - class OnIOExceptionThrown { + class TestOnIOExceptionThrown { @Test @SneakyThrows void shouldThrowTechnicalExcpetion() { diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/XdomeaPropertiesValidatorTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/XdomeaPropertiesValidatorTest.java index d8db5ae..f68a18e 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/export/XdomeaPropertiesValidatorTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/export/XdomeaPropertiesValidatorTest.java @@ -48,7 +48,7 @@ class XdomeaPropertiesValidatorTest { } @Nested - class OnUriNotSet { + class TestOnUriNotSet { private static final String PROPERTY_NAME = "behoerdenschluesselUri"; public static final String PROPERTY_PATH = "ozgcloud.xdomea." + PROPERTY_NAME; @@ -90,7 +90,7 @@ class XdomeaPropertiesValidatorTest { } @Nested - class OnVersionNotSet { + class TestOnVersionNotSet { public static final String PROPERTY_NAME = "behoerdenschluesselVersion"; public static final String PROPERTY_PATH = "ozgcloud.xdomea." + PROPERTY_NAME; diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/ArchiveEventListenerTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/ArchiveEventListenerTest.java index 7d3fbda..8f0318b 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/ArchiveEventListenerTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/ArchiveEventListenerTest.java @@ -208,7 +208,7 @@ class ArchiveEventListenerTest { } @Test - void shouldCallVorgangServiceToArchiveVorgang() { + void shouldCallRunWithSecurityContext() { onVorgangLockedEvent(); verify(eventListener).runWithSecurityContext(eq(command), commandExecutorCapture.capture()); @@ -229,30 +229,32 @@ class ArchiveEventListenerTest { @Test void shouldCallCommandServiceForPendingCommandsOnce() { - when(commandService.hasPendingCommandsExceptArchive(any())).thenReturn(false); + when(commandService.hasPendingCommandsExceptWithOrder(any(), any())).thenReturn(false); waitForPendingCommandsToFinish(); - verify(commandService).hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + verify(commandService).hasPendingCommandsExceptWithOrder(VorgangWithEingangTestFactory.ID, ArchiveEventListener.ARCHIVE_VORGANG_ORDER); } @Test void shouldCallCommandServiceForPendingCommandsTwice() { - when(commandService.hasPendingCommandsExceptArchive(any())).thenReturn(true) + when(commandService.hasPendingCommandsExceptWithOrder(any(), any())).thenReturn(true) .thenReturn(false); waitForPendingCommandsToFinish(); - verify(commandService, times(2)).hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + verify(commandService, times(2)).hasPendingCommandsExceptWithOrder(VorgangWithEingangTestFactory.ID, + ArchiveEventListener.ARCHIVE_VORGANG_ORDER); } @Test - void shouldCallAbortWaitingAfterThreeAttempts() { - when(commandService.hasPendingCommandsExceptArchive(any())).thenReturn(true); + void shouldCallAbortWaitingAfterMaxAttempts() { + when(commandService.hasPendingCommandsExceptWithOrder(any(), any())).thenReturn(true); var timeoutExceptionThrown = waitForPendingCommandsToFinishAndCatchException(); - verify(commandService, times(3)).hasPendingCommandsExceptArchive(VorgangWithEingangTestFactory.ID); + verify(commandService, times(ArchiveEventListener.MAXIMUM_CHECKS_FOR_PENDING_COMMANDS)) + .hasPendingCommandsExceptWithOrder(VorgangWithEingangTestFactory.ID, ArchiveEventListener.ARCHIVE_VORGANG_ORDER); assertThat(timeoutExceptionThrown).isTrue(); } diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/KopfCreatorTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/KopfCreatorTest.java index 1bfd738..f85bdea 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/KopfCreatorTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/KopfCreatorTest.java @@ -193,7 +193,7 @@ class KopfCreatorTest { class TestCreateKontaktType { @Nested - class OnBehoerdenschluesselNotSet { + class TestOnBehoerdenschluesselNotSet { @ParameterizedTest @NullAndEmptySource @@ -217,7 +217,7 @@ class KopfCreatorTest { } @Nested - class OnBehoerdenschluesselSet { + class TestOnBehoerdenschluesselSet { private static final BehoerdenkennungType expectedValue = new BehoerdenkennungType(); diff --git a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/VorgangServiceTest.java b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/VorgangServiceTest.java index bd9a5c9..98f5ffd 100644 --- a/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/VorgangServiceTest.java +++ b/archive-manager-server/src/test/java/de/ozgcloud/archive/vorgang/VorgangServiceTest.java @@ -51,10 +51,6 @@ public class VorgangServiceTest { private OzgCloudVorgangIdMapper ozgCloudVorgangIdMapper; @Mock private CommandMapper commandMapper; - @Mock - private VorgangTypeCreator vorgangTypeCreator; - @Mock - private KopfCreator kopfCreator; @Nested class TestGetVorgang { -- GitLab