Fix participantHtml issue with multi instances in ACM

In AutomationCompositionElementHandler of participantHtml in ACM,
refactor the result of the httpClient, that should not be a
parameter class, and handle InterruptedException

Issue-ID: POLICY-4560
Change-Id: Id5af639734f29f3600b12205bb83d315a162df99
Signed-off-by: FrancescoFioraEst <francesco.fiora@est.tech>

4 files changed, 124 insertions, 121 deletions

diff --git a/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/handler/AutomationCompositionElementHandler.java b/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/handler/AutomationCompositionElementHandler.java
index c62216dfc..0568d3b98 100644
--- a/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/handler/AutomationCompositionElementHandler.java
+++ b/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/handler/AutomationCompositionElementHandler.java
@@ -31,7 +31,8 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.stream.Collectors;
 import javax.validation.Validation;
-import javax.validation.ValidationException;
+import javax.ws.rs.core.Response.Status;
+import lombok.RequiredArgsConstructor;
 import lombok.Setter;
 import org.apache.commons.lang3.tuple.Pair;
 import org.onap.policy.clamp.acm.participant.http.main.models.ConfigRequest;
@@ -55,6 +56,7 @@ import org.springframework.stereotype.Component;
  * This class handles implementation of automationCompositionElement updates.
  */
 @Component
+@RequiredArgsConstructor
 public class AutomationCompositionElementHandler implements AutomationCompositionElementListener, Closeable {
 
     private static final Coder CODER = new StandardCoder();
@@ -63,22 +65,20 @@ public class AutomationCompositionElementHandler implements AutomationCompositio
 
     private final ExecutorService executor = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors());
 
-    private final Map<ToscaConceptIdentifier, Pair<Integer, String>> restResponseMap = new ConcurrentHashMap<>();
-
     @Setter
     private ParticipantIntermediaryApi intermediaryApi;
 
+    private final AcHttpClient acHttpClient;
+
     /**
      * Handle a automation composition element state change.
      *
      * @param automationCompositionElementId the ID of the automation composition element
-     * @throws PfModelException in case of a model exception
      */
     @Override
-    public void undeploy(UUID automationCompositionId,
-        UUID automationCompositionElementId) {
-        intermediaryApi.updateAutomationCompositionElementState(automationCompositionId,
-                automationCompositionElementId, DeployState.UNDEPLOYED, LockState.NONE);
+    public void undeploy(UUID automationCompositionId, UUID automationCompositionElementId) {
+        intermediaryApi.updateAutomationCompositionElementState(automationCompositionId, automationCompositionElementId,
+                DeployState.UNDEPLOYED, LockState.NONE);
     }
 
     /**
@@ -87,31 +87,36 @@ public class AutomationCompositionElementHandler implements AutomationCompositio
      * @param automationCompositionId the automationComposition Id
      * @param element the information on the automation composition element
      * @param properties properties Map
+     * @throws PfModelException in case of a exception
      */
     @Override
-    public void deploy(UUID automationCompositionId,
-            AcElementDeploy element, Map<String, Object> properties) {
+    public void deploy(UUID automationCompositionId, AcElementDeploy element, Map<String, Object> properties)
+            throws PfModelException {
+        var configRequest = getConfigRequest(properties);
+        var restResponseMap = invokeHttpClient(configRequest);
+        var failedResponseStatus = restResponseMap.values().stream()
+                .filter(response -> !HttpStatus.valueOf(response.getKey()).is2xxSuccessful())
+                .collect(Collectors.toList());
+        if (failedResponseStatus.isEmpty()) {
+            intermediaryApi.updateAutomationCompositionElementState(automationCompositionId, element.getId(),
+                    DeployState.DEPLOYED, LockState.LOCKED);
+        } else {
+            throw new PfModelException(Status.BAD_REQUEST, "Error on Invoking the http request: {}",
+                    failedResponseStatus);
+        }
+    }
+
+    private ConfigRequest getConfigRequest(Map<String, Object> properties) throws PfModelException {
         try {
             var configRequest = CODER.convert(properties, ConfigRequest.class);
-            var violations =
-                Validation.buildDefaultValidatorFactory().getValidator().validate(configRequest);
-            if (violations.isEmpty()) {
-                invokeHttpClient(configRequest);
-                var failedResponseStatus = restResponseMap.values().stream()
-                        .filter(response -> !HttpStatus.valueOf(response.getKey())
-                        .is2xxSuccessful()).collect(Collectors.toList());
-                if (failedResponseStatus.isEmpty()) {
-                    intermediaryApi.updateAutomationCompositionElementState(automationCompositionId, element.getId(),
-                            DeployState.DEPLOYED, LockState.LOCKED);
-                } else {
-                    LOGGER.error("Error on Invoking the http request: {}", failedResponseStatus);
-                }
-            } else {
+            var violations = Validation.buildDefaultValidatorFactory().getValidator().validate(configRequest);
+            if (!violations.isEmpty()) {
                 LOGGER.error("Violations found in the config request parameters: {}", violations);
-                throw new ValidationException("Constraint violations in the config request");
+                throw new PfModelException(Status.BAD_REQUEST, "Constraint violations in the config request");
             }
-        } catch (CoderException | ValidationException | InterruptedException | ExecutionException e) {
-            LOGGER.error("Error invoking the http request for the config ", e);
+            return configRequest;
+        } catch (CoderException e) {
+            throw new PfModelException(Status.BAD_REQUEST, "Error extracting ConfigRequest ", e);
         }
     }
 
@@ -120,11 +125,21 @@ public class AutomationCompositionElementHandler implements AutomationCompositio
      *
      * @param configRequest ConfigRequest
      */
-    public void invokeHttpClient(ConfigRequest configRequest) throws ExecutionException, InterruptedException {
-        // Invoke runnable thread to execute https requests of all config entities
-        var result = executor.submit(new AcHttpClient(configRequest, restResponseMap), restResponseMap);
-        if (!result.get().isEmpty()) {
-            LOGGER.debug("Http Request Completed: {}", result.isDone());
+    private Map<ToscaConceptIdentifier, Pair<Integer, String>> invokeHttpClient(ConfigRequest configRequest)
+            throws PfModelException {
+        try {
+            Map<ToscaConceptIdentifier, Pair<Integer, String>> restResponseMap = new ConcurrentHashMap<>();
+            // Invoke runnable thread to execute https requests of all config entities
+            var result = executor.submit(() -> acHttpClient.run(configRequest, restResponseMap), restResponseMap);
+            if (!result.get().isEmpty()) {
+                LOGGER.debug("Http Request Completed: {}", result.isDone());
+            }
+            return restResponseMap;
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new PfModelException(Status.BAD_REQUEST, "Error invoking ExecutorService ", e);
+        } catch (ExecutionException e) {
+            throw new PfModelException(Status.BAD_REQUEST, "Error invoking the http request for the config ", e);
         }
     }
 
diff --git a/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/webclient/AcHttpClient.java b/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/webclient/AcHttpClient.java
index 563daecd9..c71d73f22 100644
--- a/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/webclient/AcHttpClient.java
+++ b/participant/participant-impl/participant-impl-http/src/main/java/org/onap/policy/clamp/acm/participant/http/main/webclient/AcHttpClient.java
@@ -1,6 +1,6 @@
 /*-
  * ============LICENSE_START=======================================================
- *  Copyright (C) 2021 Nordix Foundation.
+ *  Copyright (C) 2021,2023 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,83 +35,69 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
+import org.springframework.stereotype.Component;
 import org.springframework.web.reactive.function.BodyInserters;
 import org.springframework.web.reactive.function.client.WebClient;
 import org.springframework.web.util.UriComponentsBuilder;
 import reactor.core.publisher.Mono;
 
-public class AcHttpClient implements Runnable {
+@Component
+public class AcHttpClient {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-    private final ConfigRequest configRequest;
-
-    private Map<ToscaConceptIdentifier, Pair<Integer, String>> responseMap;
-
-    /**
-     * Constructor.
-     */
-    public AcHttpClient(ConfigRequest configRequest, Map<ToscaConceptIdentifier, Pair<Integer, String>> responseMap) {
-        this.configRequest = configRequest;
-        this.responseMap = responseMap;
-    }
-
     /**
      * Runnable to execute http requests.
      */
-    @Override
-    public void run() {
+    public void run(ConfigRequest configRequest, Map<ToscaConceptIdentifier, Pair<Integer, String>> responseMap) {
 
-        var webClient = WebClient.builder()
-            .baseUrl(configRequest.getBaseUrl())
-            .defaultHeaders(httpHeaders -> httpHeaders.addAll(createHeaders(configRequest)))
-            .build();
+        var webClient = WebClient.builder().baseUrl(configRequest.getBaseUrl())
+                .defaultHeaders(httpHeaders -> httpHeaders.addAll(createHeaders(configRequest))).build();
 
-        for (ConfigurationEntity configurationEntity : configRequest.getConfigurationEntities()) {
+        for (var configurationEntity : configRequest.getConfigurationEntities()) {
             LOGGER.info("Executing http requests for the config entity {}",
-                configurationEntity.getConfigurationEntityId());
+                    configurationEntity.getConfigurationEntityId());
 
-            executeRequest(webClient, configurationEntity);
+            executeRequest(webClient, configRequest, configurationEntity, responseMap);
         }
     }
 
-    private void executeRequest(WebClient client, ConfigurationEntity configurationEntity)  {
+    private void executeRequest(WebClient client, ConfigRequest configRequest, ConfigurationEntity configurationEntity,
+            Map<ToscaConceptIdentifier, Pair<Integer, String>> responseMap) {
 
         // Iterate the sequence of http requests
-        for (RestParams request: configurationEntity.getRestSequence()) {
-            String response = null;
+        for (var request : configurationEntity.getRestSequence()) {
             try {
                 var httpMethod = Objects.requireNonNull(HttpMethod.resolve(request.getHttpMethod()));
                 var uri = createUriString(request);
                 LOGGER.info("Executing HTTP request: {} for the Rest request id: {}", httpMethod,
                         request.getRestRequestId());
 
-                response = client.method(httpMethod)
-                    .uri(uri)
-                    .body(request.getBody() == null ? BodyInserters.empty()
-                        : BodyInserters.fromValue(request.getBody()))
-                    .exchangeToMono(clientResponse ->
-                        clientResponse.statusCode().value() == request.getExpectedResponse()
-                            ? clientResponse.bodyToMono(String.class)
-                            : Mono.error(new HttpWebClientException(clientResponse.statusCode().value(),
-                                clientResponse.bodyToMono(String.class).toString())))
-                    .block(Duration.ofMillis(configRequest.getUninitializedToPassiveTimeout() * 1000L));
+                var response = client.method(httpMethod).uri(uri)
+                        .body(request.getBody() == null ? BodyInserters.empty()
+                                : BodyInserters.fromValue(request.getBody()))
+                        .exchangeToMono(
+                                clientResponse -> clientResponse.statusCode().value() == request.getExpectedResponse()
+                                        ? clientResponse.bodyToMono(String.class)
+                                        : Mono.error(new HttpWebClientException(clientResponse.statusCode().value(),
+                                                clientResponse.bodyToMono(String.class).toString())))
+                        .block(Duration.ofMillis(configRequest.getUninitializedToPassiveTimeout() * 1000L));
 
                 LOGGER.info("HTTP response for the {} request : {}", httpMethod, response);
-                responseMap.put(request.getRestRequestId(), new ImmutablePair<>(request.getExpectedResponse(),
-                    response));
+                responseMap.put(request.getRestRequestId(),
+                        new ImmutablePair<>(request.getExpectedResponse(), response));
 
             } catch (HttpWebClientException ex) {
                 LOGGER.error("Error occurred on the HTTP request ", ex);
-                responseMap.put(request.getRestRequestId(), new ImmutablePair<>(ex.getStatusCode().value(),
-                     ex.getResponseBodyAsString()));
+                responseMap.put(request.getRestRequestId(),
+                        new ImmutablePair<>(ex.getStatusCode().value(), ex.getResponseBodyAsString()));
             }
         }
     }
 
     private HttpHeaders createHeaders(ConfigRequest request) {
         var headers = new HttpHeaders();
-        for (Map.Entry<String, String> entry: request.getHttpHeaders().entrySet()) {
+        for (var entry : request.getHttpHeaders().entrySet()) {
             headers.add(entry.getKey(), entry.getValue());
         }
         return headers;
@@ -125,7 +111,7 @@ public class AcHttpClient implements Runnable {
         }
         // Add query params if present
         if (restParams.getQueryParams() != null) {
-            for (Map.Entry<String, String> entry : restParams.getQueryParams().entrySet()) {
+            for (var entry : restParams.getQueryParams().entrySet()) {
                 uriComponentsBuilder.queryParam(entry.getKey(), entry.getValue());
             }
         }
diff --git a/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/handler/AcElementHandlerTest.java b/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/handler/AcElementHandlerTest.java
index 857490ef5..e48fdf91c 100644
--- a/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/handler/AcElementHandlerTest.java
+++ b/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/handler/AcElementHandlerTest.java
@@ -20,76 +20,78 @@
 
 package org.onap.policy.clamp.acm.participant.http.handler;
 
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.doNothing;
+import static org.mockito.ArgumentMatchers.anyMap;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 import java.io.IOException;
 import java.util.HashMap;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.BeforeEach;
+import java.util.Map;
 import org.junit.jupiter.api.Test;
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.mockito.InjectMocks;
-import org.mockito.Mockito;
-import org.mockito.Spy;
 import org.onap.policy.clamp.acm.participant.http.main.handler.AutomationCompositionElementHandler;
 import org.onap.policy.clamp.acm.participant.http.main.models.ConfigRequest;
+import org.onap.policy.clamp.acm.participant.http.main.webclient.AcHttpClient;
 import org.onap.policy.clamp.acm.participant.http.utils.CommonTestData;
 import org.onap.policy.clamp.acm.participant.http.utils.ToscaUtils;
 import org.onap.policy.clamp.acm.participant.intermediary.api.ParticipantIntermediaryApi;
-import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate;
-import org.springframework.test.context.junit.jupiter.SpringExtension;
+import org.onap.policy.clamp.models.acm.concepts.DeployState;
+import org.onap.policy.clamp.models.acm.concepts.LockState;
 
-@ExtendWith(SpringExtension.class)
 class AcElementHandlerTest {
 
-    @InjectMocks
-    @Spy
-    private AutomationCompositionElementHandler automationCompositionElementHandler =
-            new AutomationCompositionElementHandler();
-
     private final CommonTestData commonTestData = new CommonTestData();
-
-    private static ToscaServiceTemplate serviceTemplate;
     private static final String HTTP_AUTOMATION_COMPOSITION_ELEMENT =
             "org.onap.domain.database.Http_PMSHMicroserviceAutomationCompositionElement";
 
-    @BeforeAll
-    static void init() {
-        serviceTemplate = ToscaUtils.readAutomationCompositionFromTosca();
-    }
-
-    @BeforeEach
-    void startMocks() {
-        automationCompositionElementHandler.setIntermediaryApi(Mockito.mock(ParticipantIntermediaryApi.class));
+    @Test
+    void testUndeploy() throws IOException {
+        var instanceId = commonTestData.getAutomationCompositionId();
+        var element = commonTestData.getAutomationCompositionElement();
+        var acElementId = element.getId();
+
+        try (var automationCompositionElementHandler =
+                new AutomationCompositionElementHandler(mock(AcHttpClient.class))) {
+            var participantIntermediaryApi = mock(ParticipantIntermediaryApi.class);
+            automationCompositionElementHandler.setIntermediaryApi(participantIntermediaryApi);
+            automationCompositionElementHandler.undeploy(instanceId, acElementId);
+            verify(participantIntermediaryApi).updateAutomationCompositionElementState(instanceId, acElementId,
+                    DeployState.UNDEPLOYED, LockState.NONE);
+        }
     }
 
     @Test
-    void test_automationCompositionElementStateChange() throws IOException {
-        var automationCompositionId = commonTestData.getAutomationCompositionId();
+    void testDeployError() throws IOException {
+        var instanceId = commonTestData.getAutomationCompositionId();
         var element = commonTestData.getAutomationCompositionElement();
-        var automationCompositionElementId = element.getId();
 
-        var config = Mockito.mock(ConfigRequest.class);
-        assertDoesNotThrow(() -> automationCompositionElementHandler.invokeHttpClient(config));
-
-        assertDoesNotThrow(() -> automationCompositionElementHandler.undeploy(
-                automationCompositionId, automationCompositionElementId));
-
-        automationCompositionElementHandler.close();
+        try (var automationCompositionElementHandler =
+                new AutomationCompositionElementHandler(mock(AcHttpClient.class))) {
+            automationCompositionElementHandler.setIntermediaryApi(mock(ParticipantIntermediaryApi.class));
+            Map<String, Object> map = new HashMap<>();
+            assertThatThrownBy(() -> automationCompositionElementHandler.deploy(instanceId, element, map))
+                    .hasMessage("Constraint violations in the config request");
+        }
     }
 
     @Test
-    void test_AutomationCompositionElementUpdate() throws Exception {
-        doNothing().when(automationCompositionElementHandler).invokeHttpClient(any());
-        var element = commonTestData.getAutomationCompositionElement();
-
+    void testDeploy() throws Exception {
+        var serviceTemplate = ToscaUtils.readAutomationCompositionFromTosca();
         var nodeTemplatesMap = serviceTemplate.getToscaTopologyTemplate().getNodeTemplates();
         var map = new HashMap<>(nodeTemplatesMap.get(HTTP_AUTOMATION_COMPOSITION_ELEMENT).getProperties());
+        var element = commonTestData.getAutomationCompositionElement();
         map.putAll(element.getProperties());
+        var instanceId = commonTestData.getAutomationCompositionId();
+        var acHttpClient = mock(AcHttpClient.class);
+        try (var automationCompositionElementHandler = new AutomationCompositionElementHandler(acHttpClient)) {
+            var participantIntermediaryApi = mock(ParticipantIntermediaryApi.class);
+            automationCompositionElementHandler.setIntermediaryApi(participantIntermediaryApi);
+            automationCompositionElementHandler.deploy(instanceId, element, map);
+            verify(acHttpClient).run(any(ConfigRequest.class), anyMap());
+            verify(participantIntermediaryApi).updateAutomationCompositionElementState(instanceId, element.getId(),
+                    DeployState.DEPLOYED, LockState.LOCKED);
+        }
 
-        assertDoesNotThrow(() -> automationCompositionElementHandler
-                .deploy(commonTestData.getAutomationCompositionId(), element, map));
     }
 }
diff --git a/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/webclient/AcHttpClientTest.java b/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/webclient/AcHttpClientTest.java
index 3ddd7b17c..d8e0c9b58 100644
--- a/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/webclient/AcHttpClientTest.java
+++ b/participant/participant-impl/participant-impl-http/src/test/java/org/onap/policy/clamp/acm/participant/http/webclient/AcHttpClientTest.java
@@ -1,6 +1,6 @@
 /*-
  * ============LICENSE_START=======================================================
- *  Copyright (C) 2021-2022 Nordix Foundation.
+ *  Copyright (C) 2021-2023 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -78,8 +78,8 @@ class AcHttpClientTest {
         var configRequest =
                 new ConfigRequest(testMockUrl + ":" + mockServerPort, headers, List.of(configurationEntity), 10);
 
-        var client = new AcHttpClient(configRequest, responseMap);
-        assertDoesNotThrow(client::run);
+        var client = new AcHttpClient();
+        assertDoesNotThrow(() -> client.run(configRequest, responseMap));
         assertThat(responseMap).hasSize(2).containsKey(commonTestData.restParamsWithGet().getRestRequestId());
 
         var restResponseMap = responseMap.get(commonTestData.restParamsWithGet().getRestRequestId());
@@ -96,8 +96,8 @@ class AcHttpClientTest {
         var configRequest =
                 new ConfigRequest(testMockUrl + ":" + mockServerPort, headers, List.of(configurationEntity), 10);
 
-        var client = new AcHttpClient(configRequest, responseMap);
-        assertDoesNotThrow(client::run);
+        var client = new AcHttpClient();
+        assertDoesNotThrow(() -> client.run(configRequest, responseMap));
         assertThat(responseMap).hasSize(2).containsKey(commonTestData.restParamsWithGet().getRestRequestId());
         var response = responseMap.get(commonTestData.restParamsWithInvalidPost().getRestRequestId());
         assertThat(response.getKey()).isEqualTo(404);