From a48252db26e2ccd031ddb2046d87737d47c248a6 Mon Sep 17 00:00:00 2001 From: Sirisha_Manchikanti Date: Tue, 2 Nov 2021 05:35:12 +0000 Subject: Remove code smells in controlloop runtime Remove code-smells reported by sonar cloud https://sonarcloud.io/project/issues?id=onap_policy-clamp&resolved=false Issue-ID: POLICY-3540 Signed-off-by: Sirisha_Manchikanti Change-Id: I8ed1c4599e7eeebbc3a31e74cea4148eb0435847 --- .../models/controlloop/concepts/ControlLoop.java | 2 +- .../rest/instantiation/ControlLoopPrimedResponse.java | 2 -- .../models/controlloop/concepts/ControlLoopTest.java | 2 +- .../provider/ServiceTemplateProviderTest.java | 9 ++++----- .../kubernetes/handler/ControlLoopElementHandler.java | 14 ++++++++++---- .../participant/kubernetes/helm/HelmClient.java | 3 ++- .../policy/main/handler/ControlLoopElementHandler.java | 4 ++-- .../api/impl/ParticipantIntermediaryApiImpl.java | 2 +- .../intermediary/handler/ParticipantHandler.java | 13 +++++++++++++ .../intermediary/handler/ControlLoopHandlerTest.java | 4 ++-- .../instantiation/ControlLoopInstantiationProvider.java | 7 ++++--- .../runtime/monitoring/MonitoringProvider.java | 8 -------- .../runtime/supervision/SupervisionHandlerTest.java | 17 ++++++++--------- 13 files changed, 48 insertions(+), 39 deletions(-) diff --git a/models/src/main/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoop.java b/models/src/main/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoop.java index 4d3d37236..cf22b7228 100644 --- a/models/src/main/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoop.java +++ b/models/src/main/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoop.java @@ -107,7 +107,7 @@ public class ControlLoop extends ToscaEntity implements Comparable */ public List getControlLoopElementStatisticsList(final ControlLoop controlLoop) { if (MapUtils.isEmpty(controlLoop.elements)) { - return null; + return List.of(); } return controlLoop.elements.values().stream().map(ControlLoopElement::getClElementStatistics) diff --git a/models/src/main/java/org/onap/policy/clamp/controlloop/models/messages/rest/instantiation/ControlLoopPrimedResponse.java b/models/src/main/java/org/onap/policy/clamp/controlloop/models/messages/rest/instantiation/ControlLoopPrimedResponse.java index 993c6e592..4d2dfaf01 100644 --- a/models/src/main/java/org/onap/policy/clamp/controlloop/models/messages/rest/instantiation/ControlLoopPrimedResponse.java +++ b/models/src/main/java/org/onap/policy/clamp/controlloop/models/messages/rest/instantiation/ControlLoopPrimedResponse.java @@ -24,8 +24,6 @@ import java.util.List; import lombok.Getter; import lombok.Setter; import lombok.ToString; -import org.onap.policy.clamp.controlloop.models.controlloop.concepts.ControlLoopOrderedState; -import org.onap.policy.clamp.controlloop.models.messages.rest.GenericNameVersion; import org.onap.policy.clamp.controlloop.models.messages.rest.SimpleResponse; diff --git a/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoopTest.java b/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoopTest.java index 51a4fc7ae..d9c518b53 100644 --- a/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoopTest.java +++ b/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/concepts/ControlLoopTest.java @@ -107,7 +107,7 @@ class ControlLoopTest { void testControlLoopElementStatisticsList() { var cl = new ControlLoop(); List emptylist = cl.getControlLoopElementStatisticsList(cl); - assertNull(emptylist); + assertEquals(List.of(), emptylist); var cl1 = getControlLoopTest(); List list = cl1.getControlLoopElementStatisticsList(cl1); diff --git a/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/persistence/provider/ServiceTemplateProviderTest.java b/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/persistence/provider/ServiceTemplateProviderTest.java index a4cbbfb0b..7f2731abf 100644 --- a/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/persistence/provider/ServiceTemplateProviderTest.java +++ b/models/src/test/java/org/onap/policy/clamp/controlloop/models/controlloop/persistence/provider/ServiceTemplateProviderTest.java @@ -25,7 +25,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -78,7 +77,7 @@ class ServiceTemplateProviderTest { var serviceTemplateProvider = new ServiceTemplateProvider(modelsProvider); var serviceTemplate = new ToscaServiceTemplate(); - when(modelsProvider.createServiceTemplate(eq(serviceTemplate))).thenReturn(serviceTemplate); + when(modelsProvider.createServiceTemplate(serviceTemplate)).thenReturn(serviceTemplate); var result = serviceTemplateProvider.createServiceTemplate(serviceTemplate); @@ -91,7 +90,7 @@ class ServiceTemplateProviderTest { serviceTemplate.setName("Name"); serviceTemplate.setVersion("1.0.0"); var modelsProvider = mock(PolicyModelsProvider.class); - when(modelsProvider.deleteServiceTemplate(eq(serviceTemplate.getName()), eq(serviceTemplate.getVersion()))) + when(modelsProvider.deleteServiceTemplate(serviceTemplate.getName(), serviceTemplate.getVersion())) .thenReturn(serviceTemplate); var serviceTemplateProvider = new ServiceTemplateProvider(modelsProvider); @@ -117,7 +116,7 @@ class ServiceTemplateProviderTest { serviceTemplate.setName("Name"); serviceTemplate.setVersion("1.0.0"); var modelsProvider = mock(PolicyModelsProvider.class); - when(modelsProvider.getServiceTemplateList(eq(serviceTemplate.getName()), eq(serviceTemplate.getVersion()))) + when(modelsProvider.getServiceTemplateList(serviceTemplate.getName(), serviceTemplate.getVersion())) .thenReturn(List.of(serviceTemplate)); var serviceTemplateProvider = new ServiceTemplateProvider(modelsProvider); @@ -133,7 +132,7 @@ class ServiceTemplateProviderTest { serviceTemplate.setName("Name"); serviceTemplate.setVersion("1.0.0"); var modelsProvider = mock(PolicyModelsProvider.class); - when(modelsProvider.getServiceTemplateList(eq(serviceTemplate.getName()), eq(serviceTemplate.getVersion()))) + when(modelsProvider.getServiceTemplateList(serviceTemplate.getName(), serviceTemplate.getVersion())) .thenReturn(List.of(serviceTemplate)); var serviceTemplateProvider = new ServiceTemplateProvider(modelsProvider); diff --git a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/handler/ControlLoopElementHandler.java b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/handler/ControlLoopElementHandler.java index e1e9195eb..3f2113d98 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/handler/ControlLoopElementHandler.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/handler/ControlLoopElementHandler.java @@ -20,9 +20,9 @@ package org.onap.policy.clamp.controlloop.participant.kubernetes.handler; - import java.io.IOException; import java.lang.invoke.MethodHandles; +import java.time.Instant; import java.util.HashMap; import java.util.Map; import java.util.UUID; @@ -30,6 +30,7 @@ import java.util.concurrent.ConcurrentHashMap; import lombok.AccessLevel; import lombok.Getter; import lombok.Setter; +import org.onap.policy.clamp.controlloop.models.controlloop.concepts.ClElementStatistics; import org.onap.policy.clamp.controlloop.models.controlloop.concepts.ControlLoopElement; import org.onap.policy.clamp.controlloop.models.controlloop.concepts.ControlLoopOrderedState; import org.onap.policy.clamp.controlloop.models.controlloop.concepts.ControlLoopState; @@ -51,7 +52,6 @@ import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; - /** * This class handles implementation of controlLoopElement updates. */ @@ -170,10 +170,16 @@ public class ControlLoopElementHandler implements ControlLoopElementListener { * Overridden method. * * @param controlLoopElementId controlLoopElement id - * @throws PfModelException incase of error + * @throws PfModelException in case of error */ @Override public synchronized void handleStatistics(UUID controlLoopElementId) throws PfModelException { - // TODO Implement statistics functionality + var clElement = intermediaryApi.getControlLoopElement(controlLoopElementId); + if (clElement != null) { + var clElementStatistics = new ClElementStatistics(); + clElementStatistics.setControlLoopState(clElement.getState()); + clElementStatistics.setTimeStamp(Instant.now()); + intermediaryApi.updateControlLoopElementStatistics(controlLoopElementId, clElementStatistics); + } } } diff --git a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/helm/HelmClient.java b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/helm/HelmClient.java index 1c405539b..31fa62f1f 100644 --- a/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/helm/HelmClient.java +++ b/participant/participant-impl/participant-impl-kubernetes/src/main/java/org/onap/policy/clamp/controlloop/participant/kubernetes/helm/HelmClient.java @@ -47,6 +47,7 @@ public class HelmClient { private ChartStore chartStore; private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + private static final String PATH_DELIMITER = "/"; /** * Install a chart. @@ -100,7 +101,7 @@ public class HelmClient { } var localHelmChartDir = chartStore.getAppPath(chart.getChartId()).toString(); logger.info("Chart not found in helm repositories, verifying local repo {} ", localHelmChartDir); - if (verifyLocalHelmRepo(new File(localHelmChartDir + "/" + chart.getChartId().getName()))) { + if (verifyLocalHelmRepo(new File(localHelmChartDir + PATH_DELIMITER + chart.getChartId().getName()))) { repository = localHelmChartDir; } return repository; diff --git a/participant/participant-impl/participant-impl-policy/src/main/java/org/onap/policy/clamp/controlloop/participant/policy/main/handler/ControlLoopElementHandler.java b/participant/participant-impl/participant-impl-policy/src/main/java/org/onap/policy/clamp/controlloop/participant/policy/main/handler/ControlLoopElementHandler.java index f6d616dac..901fb682b 100644 --- a/participant/participant-impl/participant-impl-policy/src/main/java/org/onap/policy/clamp/controlloop/participant/policy/main/handler/ControlLoopElementHandler.java +++ b/participant/participant-impl/participant-impl-policy/src/main/java/org/onap/policy/clamp/controlloop/participant/policy/main/handler/ControlLoopElementHandler.java @@ -102,7 +102,7 @@ public class ControlLoopElementHandler implements ControlLoopElementListener { break; case PASSIVE: try { - undeployPolicies(controlLoopElementId, orderedState); + undeployPolicies(controlLoopElementId); } catch (PfModelRuntimeException e) { LOGGER.debug("Undeploying policies failed - no policies to undeploy {}", controlLoopElementId); } @@ -157,7 +157,7 @@ public class ControlLoopElementHandler implements ControlLoopElementListener { ParticipantMessageType.CONTROL_LOOP_STATE_CHANGE); } - private void undeployPolicies(UUID controlLoopElementId, ControlLoopOrderedState newState) { + private void undeployPolicies(UUID controlLoopElementId) { // Undeploy all policies of this controlloop from Policy Framework if (policyMap.entrySet() != null) { for (Entry policy : policyMap.entrySet()) { diff --git a/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/api/impl/ParticipantIntermediaryApiImpl.java b/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/api/impl/ParticipantIntermediaryApiImpl.java index 43ac3464c..bbafc4678 100644 --- a/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/api/impl/ParticipantIntermediaryApiImpl.java +++ b/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/api/impl/ParticipantIntermediaryApiImpl.java @@ -89,7 +89,7 @@ public class ParticipantIntermediaryApiImpl implements ParticipantIntermediaryAp @Override public void updateParticipantStatistics(ParticipantStatistics participantStatistics) { - // TODO Auto-generated method stub + participantHandler.updateParticipantStatistics(participantStatistics); } @Override diff --git a/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ParticipantHandler.java b/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ParticipantHandler.java index d7537e3d8..89a13a84b 100644 --- a/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ParticipantHandler.java +++ b/participant/participant-intermediary/src/main/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ParticipantHandler.java @@ -189,6 +189,19 @@ public class ParticipantHandler { return getParticipant(definition.getName(), definition.getVersion()); } + /** + * Method to update participant statistics. + * + * @param statistics participant statistics + */ + public void updateParticipantStatistics(ParticipantStatistics statistics) { + participantStatistics.setState(statistics.getState()); + participantStatistics.setHealthStatus(statistics.getHealthStatus()); + participantStatistics.setTimeStamp(statistics.getTimeStamp()); + participantStatistics.setAverageExecutionTime(statistics.getAverageExecutionTime()); + participantStatistics.setEventCount(statistics.getEventCount()); + } + /** * Get participants as a {@link Participant} class. * diff --git a/participant/participant-intermediary/src/test/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ControlLoopHandlerTest.java b/participant/participant-intermediary/src/test/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ControlLoopHandlerTest.java index e77dd69ee..676747d04 100644 --- a/participant/participant-intermediary/src/test/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ControlLoopHandlerTest.java +++ b/participant/participant-intermediary/src/test/java/org/onap/policy/clamp/controlloop/participant/intermediary/handler/ControlLoopHandlerTest.java @@ -132,11 +132,11 @@ class ControlLoopHandlerTest { stateChange.setTimestamp(Instant.ofEpochMilli(3000)); var clh = setTestControlLoopHandler(id, uuid); - clh.handleControlLoopStateChange(stateChange); + clh.handleControlLoopStateChange(stateChange, List.of()); var newid = new ToscaConceptIdentifier("id", "1.2.3"); stateChange.setControlLoopId(newid); stateChange.setParticipantId(newid); - assertDoesNotThrow(() -> clh.handleControlLoopStateChange(stateChange)); + assertDoesNotThrow(() -> clh.handleControlLoopStateChange(stateChange, List.of())); List clElementDefinitions = new ArrayList<>(); var cld = new ControlLoopElementDefinition(); diff --git a/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/instantiation/ControlLoopInstantiationProvider.java b/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/instantiation/ControlLoopInstantiationProvider.java index 5ec6158bd..98ceacc3a 100644 --- a/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/instantiation/ControlLoopInstantiationProvider.java +++ b/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/instantiation/ControlLoopInstantiationProvider.java @@ -90,6 +90,7 @@ public class ControlLoopInstantiationProvider { private final ParticipantProvider participantProvider; private static final Object lockit = new Object(); + private static final String ENTRY = "entry "; /** * Creates Instance Properties and Control Loop. @@ -239,7 +240,7 @@ public class ControlLoopInstantiationProvider { var result = new BeanValidationResult("ControlLoops", controlLoops); for (ControlLoop controlLoop : controlLoops.getControlLoopList()) { - var subResult = new BeanValidationResult("entry " + controlLoop.getDefinition().getName(), controlLoop); + var subResult = new BeanValidationResult(ENTRY + controlLoop.getDefinition().getName(), controlLoop); List toscaNodeTemplates = commissioningProvider.getControlLoopDefinitions( controlLoop.getDefinition().getName(), controlLoop.getDefinition().getVersion()); @@ -280,7 +281,7 @@ public class ControlLoopInstantiationProvider { */ private ValidationResult validateDefinition(Map definitions, ToscaConceptIdentifier definition) { - var result = new BeanValidationResult("entry " + definition.getName(), definition); + var result = new BeanValidationResult(ENTRY + definition.getName(), definition); ToscaConceptIdentifier identifier = definitions.get(definition.getName()); if (identifier == null) { result.setResult(ValidationStatus.INVALID, "Not FOUND"); @@ -383,7 +384,7 @@ public class ControlLoopInstantiationProvider { for (ControlLoop controlLoop : controlLoops) { for (var element : controlLoop.getElements().values()) { - var subResult = new BeanValidationResult("entry " + element.getDefinition().getName(), element); + var subResult = new BeanValidationResult(ENTRY + element.getDefinition().getName(), element); Participant p = participantMap.get(element.getParticipantId()); if (p == null) { diff --git a/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/monitoring/MonitoringProvider.java b/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/monitoring/MonitoringProvider.java index 1f6246bd6..802413a6a 100644 --- a/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/monitoring/MonitoringProvider.java +++ b/runtime-controlloop/src/main/java/org/onap/policy/clamp/controlloop/runtime/monitoring/MonitoringProvider.java @@ -232,12 +232,4 @@ public class MonitoringProvider { } return clElementId; } - - public void updateClElementStatistics(List clElementStatistics) { - // TODO Auto-generated method stub - } - - public void updateParticipantStatistics(List statisticsList) { - // TODO Auto-generated method stub - } } diff --git a/runtime-controlloop/src/test/java/org/onap/policy/clamp/controlloop/runtime/supervision/SupervisionHandlerTest.java b/runtime-controlloop/src/test/java/org/onap/policy/clamp/controlloop/runtime/supervision/SupervisionHandlerTest.java index 0fcf52ab9..bbc4deec1 100644 --- a/runtime-controlloop/src/test/java/org/onap/policy/clamp/controlloop/runtime/supervision/SupervisionHandlerTest.java +++ b/runtime-controlloop/src/test/java/org/onap/policy/clamp/controlloop/runtime/supervision/SupervisionHandlerTest.java @@ -23,7 +23,6 @@ package org.onap.policy.clamp.controlloop.runtime.supervision; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -137,7 +136,7 @@ class SupervisionHandlerTest { participant.setParticipantType(participantType); var participantProvider = mock(ParticipantProvider.class); - when(participantProvider.getParticipants(eq(participantId.getName()), eq(participantId.getVersion()))) + when(participantProvider.getParticipants(participantId.getName(), participantId.getVersion())) .thenReturn(List.of(participant)); var participantDeregisterMessage = new ParticipantDeregister(); @@ -153,7 +152,7 @@ class SupervisionHandlerTest { handler.handleParticipantMessage(participantDeregisterMessage); verify(participantProvider).updateParticipants(anyList()); - verify(participantDeregisterAckPublisher).send(eq(participantDeregisterMessage.getMessageId())); + verify(participantDeregisterAckPublisher).send(participantDeregisterMessage.getMessageId()); } @Test @@ -177,8 +176,8 @@ class SupervisionHandlerTest { handler.handleParticipantMessage(participantRegisterMessage); verify(participantProvider).createParticipants(anyList()); - verify(participantRegisterAckPublisher).send(eq(participantRegisterMessage.getMessageId()), eq(participantId), - eq(participantType)); + verify(participantRegisterAckPublisher).send(participantRegisterMessage.getMessageId(), participantId, + participantType); } @Test @@ -189,7 +188,7 @@ class SupervisionHandlerTest { participant.setParticipantType(participantType); var participantProvider = mock(ParticipantProvider.class); - when(participantProvider.getParticipants(eq(participantId.getName()), eq(participantId.getVersion()))) + when(participantProvider.getParticipants(participantId.getName(), participantId.getVersion())) .thenReturn(List.of(participant)); var participantUpdateAckMessage = new ParticipantUpdateAck(); @@ -235,8 +234,8 @@ class SupervisionHandlerTest { participantUpdatePublisher); handler.handleSendCommissionMessage(participantId.getName(), participantId.getVersion()); - verify(participantUpdatePublisher).sendComissioningBroadcast(eq(participantId.getName()), - eq(participantId.getVersion())); + verify(participantUpdatePublisher).sendComissioningBroadcast(participantId.getName(), + participantId.getVersion()); } @Test @@ -264,7 +263,7 @@ class SupervisionHandlerTest { var controlLoopStateChangePublisher = mock(ControlLoopStateChangePublisher.class); - when(controlLoopProvider.getControlLoop(eq(identifier))).thenReturn(controlLoop); + when(controlLoopProvider.getControlLoop(identifier)).thenReturn(controlLoop); var modelsProvider = Mockito.mock(PolicyModelsProvider.class); when(modelsProvider.getServiceTemplateList(any(), any())) -- cgit 1.2.3-korg