From be6d61fe41c7a8aa8272ca75fe99c400fa55bcce Mon Sep 17 00:00:00 2001 From: Mathieu DEHARBE Date: Tue, 18 Jun 2024 14:35:22 +0200 Subject: [PATCH] post review Signed-off-by: Mathieu DEHARBE --- pom.xml | 49 +++---- .../computation/ComputationConfig.java | 17 +++ .../computation/config/JacksonConfig.java | 14 -- .../AbstractComputationRunContext.java | 4 +- .../service/AbstractComputationService.java | 6 +- .../service/AbstractResultContext.java | 8 +- .../commons/computation/ComputationTest.java | 64 ++++----- .../service/ReportServiceTest.java | 132 +++++++----------- .../SpringBootApplicationForTest.java | 3 +- 9 files changed, 129 insertions(+), 168 deletions(-) create mode 100644 src/main/java/com/powsybl/ws/commons/computation/ComputationConfig.java delete mode 100644 src/main/java/com/powsybl/ws/commons/computation/config/JacksonConfig.java diff --git a/pom.xml b/pom.xml index e12a657..702927e 100644 --- a/pom.xml +++ b/pom.xml @@ -99,6 +99,22 @@ ${spring-cloud-stream.version} true + + org.aspectj + aspectjweaver + true + + + io.micrometer + micrometer-core + true + + + com.powsybl + powsybl-network-store-client + ${powsybl-network-store-client.version} + true + @@ -132,38 +148,17 @@ org.springframework.boot spring-boot-starter-web test + + + ch.qos.logback + logback-classic + + org.springframework.boot spring-boot-starter-test test - - com.squareup.okhttp3 - okhttp - test - - - com.squareup.okhttp3 - mockwebserver - test - - - - org.aspectj - aspectjweaver - true - - - io.micrometer - micrometer-core - true - - - com.powsybl - powsybl-network-store-client - ${powsybl-network-store-client.version} - true - diff --git a/src/main/java/com/powsybl/ws/commons/computation/ComputationConfig.java b/src/main/java/com/powsybl/ws/commons/computation/ComputationConfig.java new file mode 100644 index 0000000..b2685f0 --- /dev/null +++ b/src/main/java/com/powsybl/ws/commons/computation/ComputationConfig.java @@ -0,0 +1,17 @@ +package com.powsybl.ws.commons.computation; + +import com.fasterxml.jackson.databind.InjectableValues; +import com.powsybl.commons.report.ReportNodeDeserializer; +import com.powsybl.commons.report.ReportNodeJsonModule; +import org.springframework.boot.autoconfigure.jackson.Jackson2ObjectMapperBuilderCustomizer; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +@Configuration +public class ComputationConfig { + @Bean + public Jackson2ObjectMapperBuilderCustomizer jsonCustomizer() { + return builder -> builder.modulesToInstall(new ReportNodeJsonModule()) + .postConfigurer(objMapper -> objMapper.setInjectableValues(new InjectableValues.Std().addValue(ReportNodeDeserializer.DICTIONARY_VALUE_ID, null))); + } +} diff --git a/src/main/java/com/powsybl/ws/commons/computation/config/JacksonConfig.java b/src/main/java/com/powsybl/ws/commons/computation/config/JacksonConfig.java deleted file mode 100644 index 6009c8d..0000000 --- a/src/main/java/com/powsybl/ws/commons/computation/config/JacksonConfig.java +++ /dev/null @@ -1,14 +0,0 @@ -package com.powsybl.ws.commons.computation.config; - -import org.springframework.boot.autoconfigure.jackson.Jackson2ObjectMapperBuilderCustomizer; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import com.powsybl.commons.report.ReportNodeJsonModule; - -@Configuration -public class JacksonConfig { - @Bean - public Jackson2ObjectMapperBuilderCustomizer jsonCustomizer() { - return builder -> builder.build().registerModule(new ReportNodeJsonModule()); - } -} diff --git a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationRunContext.java b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationRunContext.java index 818255c..6962d4f 100644 --- a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationRunContext.java +++ b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationRunContext.java @@ -26,8 +26,8 @@ public abstract class AbstractComputationRunContext

{ private final String receiver; private final ReportInfos reportInfos; private final String userId; - private final String provider; - private final P parameters; + private String provider; + private P parameters; private ReportNode reportNode; private Network network; diff --git a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationService.java b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationService.java index 95575c0..e5a2b90 100644 --- a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationService.java +++ b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractComputationService.java @@ -16,11 +16,11 @@ /** * @author Mathieu Deharbe - * @param run context specific to a computation, including parameters + * @param run context specific to a computation, including parameters * @param run service specific to a computation * @param enum status specific to a computation */ -public abstract class AbstractComputationService, T extends AbstractComputationResultService, S> { +public abstract class AbstractComputationService, T extends AbstractComputationResultService, S> { protected ObjectMapper objectMapper; protected NotificationService notificationService; @@ -47,7 +47,7 @@ public void stop(UUID resultUuid, String receiver) { public abstract List getProviders(); - public abstract UUID runAndSaveResult(R runContext); + public abstract UUID runAndSaveResult(C runContext); public void deleteResult(UUID resultUuid) { resultService.delete(resultUuid); diff --git a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractResultContext.java b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractResultContext.java index 80280dc..1fb8207 100644 --- a/src/main/java/com/powsybl/ws/commons/computation/service/AbstractResultContext.java +++ b/src/main/java/com/powsybl/ws/commons/computation/service/AbstractResultContext.java @@ -23,10 +23,10 @@ /** * @author Mathieu Deharbe - * @param run context specific to a computation, including parameters + * @param run context specific to a computation, including parameters */ @Data -public abstract class AbstractResultContext> { +public abstract class AbstractResultContext> { protected static final String RESULT_UUID_HEADER = "resultUuid"; @@ -43,9 +43,9 @@ public abstract class AbstractResultContext { + private static class MockComputationResultService extends AbstractComputationResultService { Map mockDBStatus = new HashMap<>(); @Override @@ -107,7 +102,7 @@ public MockComputationStatus findStatus(UUID resultUuid) { } } - public static class MockComputationObserver extends AbstractComputationObserver { + private static class MockComputationObserver extends AbstractComputationObserver { protected MockComputationObserver(@NonNull ObservationRegistry observationRegistry, @NonNull MeterRegistry meterRegistry) { super(observationRegistry, meterRegistry); } @@ -118,29 +113,29 @@ protected String getComputationType() { } @Override - protected String getResultStatus(MockComputationResult res) { + protected String getResultStatus(Object res) { return res != null ? "OK" : "NOK"; } } - public static class MockComputationRunContext extends AbstractComputationRunContext { + private static class MockComputationRunContext extends AbstractComputationRunContext { // makes the mock computation to behave in a specific way @Getter @Setter ComputationResultWanted computationResWanted = ComputationResultWanted.SUCCESS; protected MockComputationRunContext(UUID networkUuid, String variantId, String receiver, ReportInfos reportInfos, - String userId, String provider, MockComputationParameters parameters) { + String userId, String provider, Object parameters) { super(networkUuid, variantId, receiver, reportInfos, userId, provider, parameters); } } - public static class MockComputationResultContext extends AbstractResultContext { + private static class MockComputationResultContext extends AbstractResultContext { protected MockComputationResultContext(UUID resultUuid, MockComputationRunContext runContext) { super(resultUuid, runContext); } } - public class MockComputationService extends AbstractComputationService { + private static class MockComputationService extends AbstractComputationService { protected MockComputationService(NotificationService notificationService, MockComputationResultService resultService, ObjectMapper objectMapper, UuidGeneratorService uuidGeneratorService, String defaultProvider) { super(notificationService, resultService, objectMapper, uuidGeneratorService, defaultProvider); } @@ -152,17 +147,17 @@ public List getProviders() { @Override public UUID runAndSaveResult(MockComputationRunContext runContext) { - return resultUuid; + return RESULT_UUID; } } - enum ComputationResultWanted { + private enum ComputationResultWanted { SUCCESS, FAIL } - public class MockComputationWorkerService extends AbstractWorkerService { - protected MockComputationWorkerService(NetworkStoreService networkStoreService, NotificationService notificationService, ReportService reportService, MockComputationResultService resultService, ExecutionService executionService, AbstractComputationObserver observer, ObjectMapper objectMapper) { + private static class MockComputationWorkerService extends AbstractWorkerService { + protected MockComputationWorkerService(NetworkStoreService networkStoreService, NotificationService notificationService, ReportService reportService, MockComputationResultService resultService, ExecutionService executionService, AbstractComputationObserver observer, ObjectMapper objectMapper) { super(networkStoreService, notificationService, reportService, resultService, executionService, observer, objectMapper); } @@ -172,7 +167,7 @@ protected AbstractResultContext fromMessage(Message resultContext, MockComputationResult result) { } + protected void saveResult(Network network, AbstractResultContext resultContext, Object result) { } @Override protected String getComputationType() { @@ -180,14 +175,14 @@ protected String getComputationType() { } @Override - protected CompletableFuture getCompletableFuture(MockComputationRunContext runContext, String provider, UUID resultUuid) { - final CompletableFuture completableFuture = new CompletableFuture<>(); + protected CompletableFuture getCompletableFuture(MockComputationRunContext runContext, String provider, UUID resultUuid) { + final CompletableFuture completableFuture = new CompletableFuture<>(); switch (runContext.getComputationResWanted()) { case FAIL: - completableFuture.completeExceptionally(new RuntimeException("Computation failed but with an artificially longer messaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaage")); + completableFuture.completeExceptionally(new RuntimeException("Computation failed")); break; case SUCCESS: - return CompletableFuture.supplyAsync(MockComputationResult::new); + return CompletableFuture.supplyAsync(Object::new); } return completableFuture; } @@ -195,10 +190,10 @@ protected CompletableFuture getCompletableFuture(MockComp private MockComputationWorkerService workerService; private MockComputationService computationService; - private MockComputationResultContext resultContext; + private static MockComputationResultContext resultContext; final UUID networkUuid = UUID.fromString("11111111-1111-1111-1111-111111111111"); final UUID reportUuid = UUID.fromString("22222222-2222-2222-2222-222222222222"); - final UUID resultUuid = UUID.fromString("33333333-3333-3333-3333-333333333333"); + static final UUID RESULT_UUID = UUID.fromString("33333333-3333-3333-3333-333333333333"); final String reporterId = "44444444-4444-4444-4444-444444444444"; final String userId = "MockComputation_UserId"; final String receiver = "MockComputation_Receiver"; @@ -223,18 +218,17 @@ void init() { MessageBuilder builder = MessageBuilder .withPayload("") - .setHeader(HEADER_RESULT_UUID, resultUuid.toString()) + .setHeader(HEADER_RESULT_UUID, RESULT_UUID.toString()) .setHeader(HEADER_RECEIVER, receiver) .setHeader(HEADER_USER_ID, userId); message = builder.build(); runContext = new MockComputationRunContext(networkUuid, null, receiver, - new ReportInfos(reportUuid, reporterId, COMPUTATION_TYPE), userId, provider, new MockComputationParameters()); - runContext.setNetwork(network); - resultContext = new MockComputationResultContext(resultUuid, runContext); + new ReportInfos(reportUuid, reporterId, COMPUTATION_TYPE), userId, provider, new Object()); + resultContext = new MockComputationResultContext(RESULT_UUID, runContext); } - void initComputationExecution() { + private void initComputationExecution() { when(networkStoreService.getNetwork(eq(networkUuid), any(PreloadingStrategy.class))) .thenReturn(network); when(network.getVariantManager()).thenReturn(variantManager); @@ -268,21 +262,21 @@ void testComputationFailed() { @Test void testComputationCancelled() { - MockComputationStatus baseStatus = NOT_DONE; - computationService.setStatus(List.of(resultUuid), baseStatus); - assertEquals(baseStatus, computationService.getStatus(resultUuid)); + MockComputationStatus baseStatus = MockComputationStatus.NOT_DONE; + computationService.setStatus(List.of(RESULT_UUID), baseStatus); + assertEquals(baseStatus, computationService.getStatus(RESULT_UUID)); - computationService.stop(resultUuid, receiver); + computationService.stop(RESULT_UUID, receiver); // test the course verify(notificationService.getPublisher(), times(1)).send(eq("publishCancel-out-0"), isA(Message.class)); Message cancelMessage = MessageBuilder.withPayload("") - .setHeader(HEADER_RESULT_UUID, resultUuid.toString()) + .setHeader(HEADER_RESULT_UUID, RESULT_UUID.toString()) .setHeader(HEADER_RECEIVER, receiver) .build(); CancelContext cancelContext = CancelContext.fromMessage(cancelMessage); - assertEquals(resultUuid, cancelContext.resultUuid()); + assertEquals(RESULT_UUID, cancelContext.resultUuid()); assertEquals(receiver, cancelContext.receiver()); } } diff --git a/src/test/java/com/powsybl/ws/commons/computation/service/ReportServiceTest.java b/src/test/java/com/powsybl/ws/commons/computation/service/ReportServiceTest.java index c13f370..57c6556 100644 --- a/src/test/java/com/powsybl/ws/commons/computation/service/ReportServiceTest.java +++ b/src/test/java/com/powsybl/ws/commons/computation/service/ReportServiceTest.java @@ -7,113 +7,83 @@ package com.powsybl.ws.commons.computation.service; -import com.fasterxml.jackson.databind.InjectableValues; -import com.fasterxml.jackson.databind.SerializationFeature; import com.powsybl.commons.report.ReportNode; -import com.powsybl.commons.report.ReportNodeDeserializer; -import com.powsybl.commons.report.ReportNodeJsonModule; -import lombok.extern.slf4j.Slf4j; -import okhttp3.HttpUrl; -import okhttp3.mockwebserver.Dispatcher; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; -import org.jetbrains.annotations.NotNull; -import org.junit.After; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.http.HttpStatus; -import org.springframework.http.converter.json.Jackson2ObjectMapperBuilder; -import org.springframework.test.context.junit4.SpringRunner; +import com.powsybl.ws.commons.computation.ComputationConfig; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.client.AutoConfigureWebClient; +import org.springframework.boot.test.autoconfigure.web.client.RestClientTest; +import org.springframework.http.HttpMethod; +import org.springframework.http.MediaType; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.web.client.MockRestServiceServer; +import org.springframework.test.web.client.match.MockRestRequestMatchers; +import org.springframework.test.web.client.response.MockRestResponseCreators; import org.springframework.web.client.RestClientException; -import org.springframework.web.client.RestTemplate; -import java.io.IOException; -import java.util.Objects; import java.util.UUID; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertThrows; +import static org.assertj.core.api.Assertions.*; /** * @author Mathieu Deharbe reportService.sendReport(REPORT_UUID, reportNode)); } - private String initMockWebServer() throws IOException { - server = new MockWebServer(); - server.start(); - - final Dispatcher dispatcher = new Dispatcher() { - @NotNull - @Override - public MockResponse dispatch(RecordedRequest request) { - String requestPath = Objects.requireNonNull(request.getPath()); - if (requestPath.equals(String.format("/v1/reports/%s", REPORT_UUID))) { - assertEquals(REPORT_JSON, request.getBody().readUtf8()); - return new MockResponse().setResponseCode(HttpStatus.OK.value()); - } else if (requestPath.equals(String.format("/v1/reports/%s?reportTypeFilter=MockReportType&errorOnReportNotFound=false", REPORT_UUID))) { - assertEquals("", request.getBody().readUtf8()); - return new MockResponse().setResponseCode(HttpStatus.OK.value()); - } else if (requestPath.equals(String.format("/v1/reports/%s", REPORT_ERROR_UUID))) { - return new MockResponse().setResponseCode(HttpStatus.INTERNAL_SERVER_ERROR.value()); - } else { - return new MockResponse().setResponseCode(HttpStatus.NOT_FOUND.value()).setBody("Path not supported: " + request.getPath()); - } - } - }; - - server.setDispatcher(dispatcher); - - // Ask the server for its URL. You'll need this to make HTTP requests. - HttpUrl baseHttpUrl = server.url(""); - return baseHttpUrl.toString().substring(0, baseHttpUrl.toString().length() - 1); + @Test + void testSendReportFailed() { + final ReportNode reportNode = ReportNode.newRootReportNode().withMessageTemplate("test", "a test").build(); + server.expect(MockRestRequestMatchers.method(HttpMethod.PUT)) + .andExpect(MockRestRequestMatchers.requestTo("http://report-server/v1/reports/" + REPORT_ERROR_UUID)) + .andRespond(MockRestResponseCreators.withServerError()); + assertThatThrownBy(() -> reportService.sendReport(REPORT_ERROR_UUID, reportNode)).isInstanceOf(RestClientException.class); } @Test - public void testSendReport() { - ReportNode reportNode = ReportNode.newRootReportNode().withMessageTemplate("test", "a test").build(); - reportService.sendReport(REPORT_UUID, reportNode); - assertThrows(RestClientException.class, () -> reportService.sendReport(REPORT_ERROR_UUID, reportNode)); + void testDeleteReport() { + server.expect(MockRestRequestMatchers.method(HttpMethod.DELETE)) + .andExpect(MockRestRequestMatchers.requestTo("http://report-server/v1/reports/" + REPORT_UUID + "?reportTypeFilter=MockReportType&errorOnReportNotFound=false")) + .andExpect(MockRestRequestMatchers.content().bytes(new byte[0])) + .andRespond(MockRestResponseCreators.withSuccess()); + assertThatNoException().isThrownBy(() -> reportService.deleteReport(REPORT_UUID, "MockReportType")); } @Test - public void testDeleteReport() { - reportService.deleteReport(REPORT_UUID, "MockReportType"); - assertThrows(RestClientException.class, () -> reportService.deleteReport(REPORT_ERROR_UUID, "MockReportType")); + void testDeleteReportFailed() { + server.expect(MockRestRequestMatchers.method(HttpMethod.DELETE)) + .andExpect(MockRestRequestMatchers.requestTo("http://report-server/v1/reports/" + REPORT_ERROR_UUID + "?reportTypeFilter=MockReportType&errorOnReportNotFound=false")) + .andExpect(MockRestRequestMatchers.content().bytes(new byte[0])) + .andRespond(MockRestResponseCreators.withServerError()); + assertThatThrownBy(() -> reportService.deleteReport(REPORT_ERROR_UUID, "MockReportType")).isInstanceOf(RestClientException.class); } } diff --git a/src/test/java/com/powsybl/ws_common_spring_test/SpringBootApplicationForTest.java b/src/test/java/com/powsybl/ws_common_spring_test/SpringBootApplicationForTest.java index 7fbc5c6..6c880ca 100644 --- a/src/test/java/com/powsybl/ws_common_spring_test/SpringBootApplicationForTest.java +++ b/src/test/java/com/powsybl/ws_common_spring_test/SpringBootApplicationForTest.java @@ -1,6 +1,5 @@ package com.powsybl.ws_common_spring_test; -import com.powsybl.ws.commons.computation.config.JacksonConfig; import org.springframework.boot.autoconfigure.SpringBootApplication; /** @@ -10,6 +9,6 @@ @SuppressWarnings({ "java:S2187" //this isn't a class containing tests }) -@SpringBootApplication(scanBasePackageClasses = {JacksonConfig.class}) +@SpringBootApplication public class SpringBootApplicationForTest { }