From 2b05f15336c57960af51e3a6cbf5705b6b88c4fd Mon Sep 17 00:00:00 2001 From: elinuxhenrik Date: Mon, 11 May 2020 14:12:43 +0200 Subject: [PATCH] Fix Sonar and CheckStyle warnings Change-Id: Ifbbb999aec5ecbf382c799a3bb733ca272f93da7 Issue-ID: NONRTRIC-209 Signed-off-by: elinuxhenrik --- .../org/oransc/policyagent/aspect/LogAspect.java | 6 +- .../policyagent/clients/SdncOscA1Client.java | 19 +++-- .../configuration/ApplicationConfigParser.java | 4 +- .../policyagent/dmaap/DmaapMessageConsumer.java | 10 ++- .../policyagent/dmaap/DmaapMessageHandler.java | 1 + .../org/oransc/policyagent/MockPolicyAgent.java | 30 ++++++-- .../oransc/policyagent/repository/LockTest.java | 5 +- .../northbound/provider/NonrtRicApiProvider.java | 89 +++++++++++----------- 8 files changed, 100 insertions(+), 64 deletions(-) diff --git a/policy-agent/src/main/java/org/oransc/policyagent/aspect/LogAspect.java b/policy-agent/src/main/java/org/oransc/policyagent/aspect/LogAspect.java index f78d6e7f..93b2ec0d 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/aspect/LogAspect.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/aspect/LogAspect.java @@ -40,13 +40,13 @@ public class LogAspect { @Around("execution(* org.oransc.policyagent..*(..)))") public void executimeTime(ProceedingJoinPoint proceedingJoinPoint) throws Throwable { - MethodSignature methodSignature = (MethodSignature) proceedingJoinPoint.getSignature(); - String className = methodSignature.getDeclaringType().getSimpleName(); - String methodName = methodSignature.getName(); final StopWatch stopWatch = new StopWatch(); stopWatch.start(); proceedingJoinPoint.proceed(); stopWatch.stop(); + MethodSignature methodSignature = (MethodSignature) proceedingJoinPoint.getSignature(); + String className = methodSignature.getDeclaringType().getSimpleName(); + String methodName = methodSignature.getName(); logger.trace("Execution time of {}.{}: {} ms", className, methodName, stopWatch.getTotalTimeMillis()); } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/clients/SdncOscA1Client.java b/policy-agent/src/main/java/org/oransc/policyagent/clients/SdncOscA1Client.java index 2daa4d68..a5735f7d 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/clients/SdncOscA1Client.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/clients/SdncOscA1Client.java @@ -78,14 +78,12 @@ public class SdncOscA1Client implements A1Client { private final A1ProtocolType protocolType; /** - * Constructor - * + * Constructor that creates the REST client to use. + * * @param protocolType the southbound protocol of the controller. Supported * protocols are SDNC_OSC_STD_V1_1 and SDNC_OSC_OSC_V1 - * @param ricConfig - * @param controllerBaseUrl the base URL of the SDNC controller - * @param username username to accesss the SDNC controller - * @param password password to accesss the SDNC controller + * @param ricConfig the configuration of the Ric to communicate with + * @param controllerConfig the configuration of the SDNC controller to use */ public SdncOscA1Client(A1ProtocolType protocolType, RicConfig ricConfig, ControllerConfig controllerConfig) { this(protocolType, ricConfig, controllerConfig, @@ -93,6 +91,15 @@ public class SdncOscA1Client implements A1Client { logger.debug("SdncOscA1Client for ric: {}, a1Controller: {}", ricConfig.name(), controllerConfig); } + /** + * Constructor where the REST client to use is provided. + * + * @param protocolType the southbound protocol of the controller. Supported + * protocols are SDNC_OSC_STD_V1_1 and SDNC_OSC_OSC_V1 + * @param ricConfig the configuration of the Ric to communicate with + * @param controllerConfig the configuration of the SDNC controller to use + * @param restClient the REST client to use + */ public SdncOscA1Client(A1ProtocolType protocolType, RicConfig ricConfig, ControllerConfig controllerConfig, AsyncRestClient restClient) { this.restClient = restClient; diff --git a/policy-agent/src/main/java/org/oransc/policyagent/configuration/ApplicationConfigParser.java b/policy-agent/src/main/java/org/oransc/policyagent/configuration/ApplicationConfigParser.java index 8418749e..54957107 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/configuration/ApplicationConfigParser.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/configuration/ApplicationConfigParser.java @@ -70,8 +70,6 @@ public class ApplicationConfigParser { Properties dmaapConsumerConfig = new Properties(); JsonObject agentConfigJson = root.getAsJsonObject(CONFIG); - List ricConfigs = parseRics(agentConfigJson); - Map controllerConfigs = parseControllerConfigs(agentConfigJson); JsonObject json = agentConfigJson.getAsJsonObject("streams_publishes"); if (json != null) { @@ -83,6 +81,8 @@ public class ApplicationConfigParser { dmaapConsumerConfig = parseDmaapConfig(json); } + List ricConfigs = parseRics(agentConfigJson); + Map controllerConfigs = parseControllerConfigs(agentConfigJson); checkConfigurationConsistency(ricConfigs, controllerConfigs); return ImmutableConfigParserResult.builder() // diff --git a/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageConsumer.java b/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageConsumer.java index dd60d394..6312e375 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageConsumer.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageConsumer.java @@ -44,10 +44,12 @@ import org.springframework.stereotype.Component; * The class fetches incoming requests from DMAAP. It uses the timeout parameter that lets the MessageRouter keep the * connection with the Kafka open until requests are sent in. * + *

* If there is no DMaaP configuration in the application configuration, then this service will regularly check the * configuration and start polling DMaaP if the configuration is added. If the DMaaP configuration is removed, then the * service will stop polling and resume checking for configuration. * + *

* Each received request is processed by {@link DmaapMessageHandler}. */ @Component @@ -67,8 +69,14 @@ public class DmaapMessageConsumer { this.applicationConfig = applicationConfig; } + /** + * Starts the consumer. If there is a DMaaP configuration, it will start polling for messages. Otherwise it will + * check regularly for the configuration. + * + * @return the running thread, for test purposes. + */ public Thread start() { - Thread thread = new Thread(() -> this.checkConfigLoop()); + Thread thread = new Thread(this::checkConfigLoop); thread.start(); return thread; } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageHandler.java b/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageHandler.java index 3c44f085..7125135e 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageHandler.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/dmaap/DmaapMessageHandler.java @@ -17,6 +17,7 @@ * limitations under the License. * ========================LICENSE_END=================================== */ + package org.oransc.policyagent.dmaap; import com.google.gson.Gson; diff --git a/policy-agent/src/test/java/org/oransc/policyagent/MockPolicyAgent.java b/policy-agent/src/test/java/org/oransc/policyagent/MockPolicyAgent.java index 7bdd7968..d8a97184 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/MockPolicyAgent.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/MockPolicyAgent.java @@ -20,6 +20,8 @@ package org.oransc.policyagent; +import static org.awaitility.Awaitility.await; + import com.google.gson.JsonObject; import com.google.gson.JsonParser; @@ -49,6 +51,7 @@ import org.springframework.boot.test.context.TestConfiguration; import org.springframework.boot.web.server.LocalServerPort; import org.springframework.context.annotation.Bean; import org.springframework.test.context.junit.jupiter.SpringExtension; +import org.springframework.util.StringUtils; @ExtendWith(SpringExtension.class) @SpringBootTest(webEnvironment = WebEnvironment.DEFINED_PORT) @@ -143,14 +146,25 @@ public class MockPolicyAgent { private int port; private void keepServerAlive() throws InterruptedException, IOException { - logger.info("Keeping server alive!"); - Thread.sleep(1000); + waitForConfigurationToBeLoaded(); loadInstances(); + logger.info("Keeping server alive!"); synchronized (this) { this.wait(); } } + private void waitForConfigurationToBeLoaded() throws IOException { + String json = getConfigJsonFromFile(); + try { + int noOfRicsInConfigFile = StringUtils.countOccurrencesOf(json, "baseUrl"); + await().until(() -> rics.size() == noOfRicsInConfigFile); + } catch (Exception e) { + logger.info("Loaded rics: {}, and no of rics in config file: {} never matched!", rics.size(), + StringUtils.countOccurrencesOf(json, "baseUrl")); + } + } + private static String title(String jsonSchema) { JsonObject parsedSchema = (JsonObject) JsonParser.parseString(jsonSchema); String title = parsedSchema.get("title").getAsString(); @@ -160,8 +174,7 @@ public class MockPolicyAgent { private void loadInstances() throws IOException { PolicyType unnamedPolicyType = policyTypes.get(""); Ric ric = rics.get("ric1"); - File jsonFile = getFile("test_application_configuration.json"); - String json = new String(Files.readAllBytes(jsonFile.toPath())); + String json = getConfigJsonFromFile(); Policy policy = ImmutablePolicy.builder() // .id("typelessPolicy") // @@ -174,9 +187,14 @@ public class MockPolicyAgent { this.policies.put(policy); } + private String getConfigJsonFromFile() throws IOException { + File jsonFile = getFile("test_application_configuration.json"); + String json = new String(Files.readAllBytes(jsonFile.toPath())); + return json; + } + @Test - @SuppressWarnings("squid:S2699") // Tests should include assertions. This test is only for keeping the server - // alive, + @SuppressWarnings("squid:S2699") // Tests should include assertions. This test is only for keeping the server alive, // so it will only be confusing to add an assertion. public void runMock() throws Exception { keepServerAlive(); diff --git a/policy-agent/src/test/java/org/oransc/policyagent/repository/LockTest.java b/policy-agent/src/test/java/org/oransc/policyagent/repository/LockTest.java index ee7e04fd..5c1cc140 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/repository/LockTest.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/repository/LockTest.java @@ -41,15 +41,16 @@ public class LockTest { try { Thread.sleep(100); } catch (InterruptedException e) { + // Do nothing. } } private void asynchUnlock(Lock lock) { - Thread t = new Thread(() -> { + Thread thread = new Thread(() -> { sleep(); lock.unlockBlocking(); }); - t.start(); + thread.start(); } @Test diff --git a/sdnc-a1-controller/northbound/nonrt-ric-api/provider/src/main/java/org/o_ran_sc/nonrtric/sdnc_a1/northbound/provider/NonrtRicApiProvider.java b/sdnc-a1-controller/northbound/nonrt-ric-api/provider/src/main/java/org/o_ran_sc/nonrtric/sdnc_a1/northbound/provider/NonrtRicApiProvider.java index 9a625b1b..a0a6fadd 100644 --- a/sdnc-a1-controller/northbound/nonrt-ric-api/provider/src/main/java/org/o_ran_sc/nonrtric/sdnc_a1/northbound/provider/NonrtRicApiProvider.java +++ b/sdnc-a1-controller/northbound/nonrt-ric-api/provider/src/main/java/org/o_ran_sc/nonrtric/sdnc_a1/northbound/provider/NonrtRicApiProvider.java @@ -78,6 +78,7 @@ public class NonrtRicApiProvider implements AutoCloseable, A1ADAPTERAPIService { protected static final String NO_SERVICE_LOGIC_ACTIVE = "No service logic active for "; private static final String NON_NULL_PARAM = "non-null"; private static final String NULL_PARAM = "null"; + private static final String REST_CLIENT_RESPONSE_EXCEPTION_MSG = "Caught RestClientResponseException: {}"; private final Logger log = LoggerFactory.getLogger(NonrtRicApiProvider.class); private final ExecutorService executor; @@ -157,80 +158,80 @@ public class NonrtRicApiProvider implements AutoCloseable, A1ADAPTERAPIService { @Override public ListenableFuture> putA1Policy(PutA1PolicyInput input) { log.info("Start of putPolicy"); - PutA1PolicyOutputBuilder responseBuilder = new PutA1PolicyOutputBuilder(); + PutA1PolicyOutputBuilder putPolicyResponseBuilder = new PutA1PolicyOutputBuilder(); try { final Uri uri = input.getNearRtRicUrl(); log.info("PUT Request input.GetA1Policy() : {} ", uri); - ResponseEntity response = restAdapter.put(uri.getValue(), input.getBody(), String.class); - if (response.hasBody()) { - log.info("Response PutA1Policy : {} ", response.getBody()); - responseBuilder.setBody(response.getBody()); + ResponseEntity putPolicyResponse = restAdapter.put(uri.getValue(), input.getBody(), String.class); + if (putPolicyResponse.hasBody()) { + log.info("Response PutA1Policy : {} ", putPolicyResponse.getBody()); + putPolicyResponseBuilder.setBody(putPolicyResponse.getBody()); } - responseBuilder.setHttpStatus(response.getStatusCodeValue()); + putPolicyResponseBuilder.setHttpStatus(putPolicyResponse.getStatusCodeValue()); } catch (RestClientResponseException ex) { - log.error("Caught RestClientResponseException: {}", ex.getMessage()); + log.error(REST_CLIENT_RESPONSE_EXCEPTION_MSG, ex.getMessage()); if (ex.getResponseBodyAsByteArray() != null) { - responseBuilder.setBody(ex.getResponseBodyAsString()); + putPolicyResponseBuilder.setBody(ex.getResponseBodyAsString()); } - responseBuilder.setHttpStatus(ex.getRawStatusCode()); + putPolicyResponseBuilder.setHttpStatus(ex.getRawStatusCode()); } log.info("End of PutA1Policy"); RpcResult rpcResult = RpcResultBuilder.status(true) - .withResult(responseBuilder.build()).build(); + .withResult(putPolicyResponseBuilder.build()).build(); return Futures.immediateFuture(rpcResult); } @Override public ListenableFuture> deleteA1Policy(DeleteA1PolicyInput input) { log.info("Start of DeleteA1Policy"); - DeleteA1PolicyOutputBuilder responseBuilder = new DeleteA1PolicyOutputBuilder(); + DeleteA1PolicyOutputBuilder deletePolicyResponseBuilder = new DeleteA1PolicyOutputBuilder(); try { final Uri uri = input.getNearRtRicUrl(); - ResponseEntity response = restAdapter.delete(uri.getValue()); - if (response.hasBody()) { - log.info("Response DeleteA1Policy : {} ", response.getBody()); - responseBuilder.setBody(response.getBody().toString()); + ResponseEntity deletePolicyResponse = restAdapter.delete(uri.getValue()); + if (deletePolicyResponse.hasBody()) { + log.info("Response DeleteA1Policy : {} ", deletePolicyResponse.getBody()); + deletePolicyResponseBuilder.setBody(deletePolicyResponse.getBody().toString()); } - responseBuilder.setHttpStatus(response.getStatusCodeValue()); + deletePolicyResponseBuilder.setHttpStatus(deletePolicyResponse.getStatusCodeValue()); } catch (RestClientResponseException ex) { - log.error("Caught RestClientResponseException: {}", ex.getMessage()); + log.error(REST_CLIENT_RESPONSE_EXCEPTION_MSG, ex.getMessage()); if (ex.getResponseBodyAsByteArray() != null) { - responseBuilder.setBody(ex.getResponseBodyAsString()); + deletePolicyResponseBuilder.setBody(ex.getResponseBodyAsString()); } - responseBuilder.setHttpStatus(ex.getRawStatusCode()); + deletePolicyResponseBuilder.setHttpStatus(ex.getRawStatusCode()); } log.info("End of DeleteA1Policy"); RpcResult rpcResult = RpcResultBuilder.status(true) - .withResult(responseBuilder.build()).build(); + .withResult(deletePolicyResponseBuilder.build()).build(); return Futures.immediateFuture(rpcResult); } private GetA1PolicyOutput getA1(GetA1PolicyInput input) { log.info("Start of getA1"); - GetA1PolicyOutputBuilder responseBuilder = new GetA1PolicyOutputBuilder(); + GetA1PolicyOutputBuilder getPolicyResponseBuilder = new GetA1PolicyOutputBuilder(); try { final Uri uri = input.getNearRtRicUrl(); - ResponseEntity response = restAdapter.get(uri.getValue(), String.class); - if (response.hasBody()) { - log.info("Response getA1 : {} ", response.getBody()); - responseBuilder.setBody(response.getBody()); + ResponseEntity getPolicyResponse = restAdapter.get(uri.getValue(), String.class); + if (getPolicyResponse.hasBody()) { + log.info("Response getA1 : {} ", getPolicyResponse.getBody()); + getPolicyResponseBuilder.setBody(getPolicyResponse.getBody()); } - responseBuilder.setHttpStatus(response.getStatusCodeValue()); + getPolicyResponseBuilder.setHttpStatus(getPolicyResponse.getStatusCodeValue()); } catch (RestClientResponseException ex) { - log.error("Caught RestClientResponseException: {}", ex.getMessage()); + log.error(REST_CLIENT_RESPONSE_EXCEPTION_MSG, ex.getMessage()); if (ex.getResponseBodyAsByteArray() != null) { - responseBuilder.setBody(ex.getResponseBodyAsString()); + getPolicyResponseBuilder.setBody(ex.getResponseBodyAsString()); } - responseBuilder.setHttpStatus(ex.getRawStatusCode()); + getPolicyResponseBuilder.setHttpStatus(ex.getRawStatusCode()); } log.info("End of getA1"); - return responseBuilder.build(); + return getPolicyResponseBuilder.build(); } @Override @@ -243,31 +244,31 @@ public class NonrtRicApiProvider implements AutoCloseable, A1ADAPTERAPIService { @Override public ListenableFuture> getA1PolicyStatus(GetA1PolicyStatusInput input) { - GetA1PolicyInputBuilder getInputBuilder = new GetA1PolicyInputBuilder(); - getInputBuilder.setNearRtRicUrl(input.getNearRtRicUrl()); - GetA1PolicyOutput getOutput = getA1(getInputBuilder.build()); + GetA1PolicyInputBuilder getPolicyStatusInputBuilder = new GetA1PolicyInputBuilder(); + getPolicyStatusInputBuilder.setNearRtRicUrl(input.getNearRtRicUrl()); + GetA1PolicyOutput getOutput = getA1(getPolicyStatusInputBuilder.build()); - GetA1PolicyStatusOutputBuilder outputBuilder = new GetA1PolicyStatusOutputBuilder(); - outputBuilder.setBody(getOutput.getBody()); - outputBuilder.setHttpStatus(getOutput.getHttpStatus()); + GetA1PolicyStatusOutputBuilder getPolicyStatusoutputBuilder = new GetA1PolicyStatusOutputBuilder(); + getPolicyStatusoutputBuilder.setBody(getOutput.getBody()); + getPolicyStatusoutputBuilder.setHttpStatus(getOutput.getHttpStatus()); return Futures.immediateFuture(RpcResultBuilder.status(true) // - .withResult(outputBuilder.build()) // + .withResult(getPolicyStatusoutputBuilder.build()) // .build()); } @Override public ListenableFuture> getA1PolicyType(GetA1PolicyTypeInput input) { - GetA1PolicyInputBuilder getInputBuilder = new GetA1PolicyInputBuilder(); - getInputBuilder.setNearRtRicUrl(input.getNearRtRicUrl()); - GetA1PolicyOutput getOutput = getA1(getInputBuilder.build()); + GetA1PolicyInputBuilder getPolicyTypeInputBuilder = new GetA1PolicyInputBuilder(); + getPolicyTypeInputBuilder.setNearRtRicUrl(input.getNearRtRicUrl()); + GetA1PolicyOutput getOutput = getA1(getPolicyTypeInputBuilder.build()); - GetA1PolicyTypeOutputBuilder outputBuilder = new GetA1PolicyTypeOutputBuilder(); - outputBuilder.setBody(getOutput.getBody()); - outputBuilder.setHttpStatus(getOutput.getHttpStatus()); + GetA1PolicyTypeOutputBuilder getPolicyTypeOutputBuilder = new GetA1PolicyTypeOutputBuilder(); + getPolicyTypeOutputBuilder.setBody(getOutput.getBody()); + getPolicyTypeOutputBuilder.setHttpStatus(getOutput.getHttpStatus()); return Futures.immediateFuture(RpcResultBuilder.status(true) // - .withResult(outputBuilder.build()) // + .withResult(getPolicyTypeOutputBuilder.build()) // .build()); } -- 2.16.6