From: saul.gill Date: Wed, 11 Jun 2025 18:07:01 +0000 (+0100) Subject: Fix code smells X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=94d5058adadf0faebd650eaecde3f3a1aa03d713;p=ccsdk%2Foran.git Fix code smells Issue-ID: CCSDK-4120 Change-Id: I26ce2f82d84972e9a310f980cd3039633e8e6cf0 Signed-off-by: saul.gill --- diff --git a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/PolicyService.java b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/PolicyService.java index bd3dbc3d..5c5d0215 100644 --- a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/PolicyService.java +++ b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/PolicyService.java @@ -174,7 +174,7 @@ public class PolicyService { } public Mono> getPolicyTypeDefinitionService(String policyTypeId) - throws EntityNotFoundException{ + throws EntityNotFoundException { PolicyType singlePolicyType = policyTypes.get(policyTypeId); if (singlePolicyType == null) throw new EntityNotFoundException("PolicyType not found with ID: " + policyTypeId); @@ -183,7 +183,7 @@ public class PolicyService { try { policyTypeObject.setPolicySchema(gson.fromJson(singlePolicyType.getSchema(), Object.class)); } catch (JsonSyntaxException e) { - throw new RuntimeException("Failed to deserialize policy schema", e); + logger.error("Failed to deserialize policy schema for policy type ID: {}", policyTypeId, e); } return Mono.just(new ResponseEntity(policyTypeObject, HttpStatus.OK)); @@ -224,9 +224,13 @@ public class PolicyService { .doOnError(errorHandlingService::handleError); } - public Mono> getPolicyStatus(String policyId, ServerWebExchange exchange) throws Exception { + public Mono> getPolicyStatus(String policyId, ServerWebExchange exchange) throws EntityNotFoundException { Policy policy = policies.getPolicy(policyId); + if (policy == null) { + throw new EntityNotFoundException("Policy not found with ID: " + policyId); + } + return authorizationService.authCheck(exchange, policy, AccessType.READ) .doOnError(errorHandlingService::handleError) .flatMap(policyLock -> policy.getRic().getLock().lock(Lock.LockType.SHARED, "getStatus")) diff --git a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/TokenService.java b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/TokenService.java index bee29c83..c060930e 100644 --- a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/TokenService.java +++ b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/service/v3/TokenService.java @@ -18,85 +18,81 @@ * ========================LICENSE_END=================================== */ - package org.onap.ccsdk.oran.a1policymanagementservice.service.v3; +package org.onap.ccsdk.oran.a1policymanagementservice.service.v3; - import java.lang.invoke.MethodHandles; - import java.util.Base64; +import java.lang.invoke.MethodHandles; +import java.util.Base64; - import org.onap.ccsdk.oran.a1policymanagementservice.models.v2.PolicyInfo; - import org.onap.ccsdk.oran.a1policymanagementservice.models.v3.PolicyObjectInformation; - import org.slf4j.Logger; - import org.slf4j.LoggerFactory; - import org.springframework.http.HttpHeaders; - import org.springframework.stereotype.Service; - import org.springframework.web.server.ServerWebExchange; +import org.onap.ccsdk.oran.a1policymanagementservice.models.v2.PolicyInfo; +import org.onap.ccsdk.oran.a1policymanagementservice.models.v3.PolicyObjectInformation; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.http.HttpHeaders; +import org.springframework.stereotype.Service; +import org.springframework.web.server.ServerWebExchange; - import com.google.gson.JsonObject; - import com.google.gson.JsonParser; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; - @Service - public class TokenService { - private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); +@Service +public class TokenService { + private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); - // Prefix used to identify Bearer tokens in the Authorization header - private static final String BEARER_PREFIX = "Bearer "; + // Prefix used to identify Bearer tokens in the Authorization header + private static final String BEARER_PREFIX = "Bearer "; - /** - * Retrieves the service ID for version 3 (v3) of the API, which uses PolicyObjectInformation. - * - * @param policyInfoValue The PolicyObjectInformation object containing the policy details. - * @param exchange The ServerWebExchange object that contains request and response information. - * @return The service ID, either from the policy information or derived from the client ID in the token. - */ - public String getServiceId(PolicyObjectInformation policyInfoValue, ServerWebExchange exchange) { - String serviceId = policyInfoValue.getServiceId(); - String clientId = extractClientIdFromToken(exchange); + /** + * Retrieves the service ID for version 3 (v3) of the API, which uses PolicyObjectInformation. + * + * @param policyInfoValue The PolicyObjectInformation object containing the policy details. + * @param exchange The ServerWebExchange object that contains request and response information. + * @return The service ID, either from the policy information or derived from the client ID in the token. + */ + public String getServiceId(PolicyObjectInformation policyInfoValue, ServerWebExchange exchange) { + String serviceId = policyInfoValue.getServiceId(); + String clientId = extractClientIdFromToken(exchange); - // If the service ID from the policy is blank, use the client ID from the token instead - if (serviceId.isBlank()) { - if (clientId != null && !clientId.isBlank()) { - serviceId = clientId; - } - } - // Return the determined service ID - logger.debug("ServiceID extracted from token: " + serviceId); - return serviceId; - } + // If the service ID from the policy is blank, use the client ID from the token instead + if (serviceId.isBlank() && clientId != null && !clientId.isBlank()) { + serviceId = clientId; + } + // Return the determined service ID + logger.debug("ServiceID extracted from token: {}", serviceId); + return serviceId; + } - /** - * Retrieves the service ID for version 2 (v2) of the API, which uses PolicyInfo. - * - * @param policyInfoValue The PolicyInfo object containing the policy details. - * @param exchange The ServerWebExchange object that contains request and response information. - * @return The service ID, either from the policy information or derived from the client ID in the token. - */ - public String getServiceId(PolicyInfo policyInfoValue, ServerWebExchange exchange) { - String serviceId = policyInfoValue.getServiceId(); - String clientId = extractClientIdFromToken(exchange); + /** + * Retrieves the service ID for version 2 (v2) of the API, which uses PolicyInfo. + * + * @param policyInfoValue The PolicyInfo object containing the policy details. + * @param exchange The ServerWebExchange object that contains request and response information. + * @return The service ID, either from the policy information or derived from the client ID in the token. + */ + public String getServiceId(PolicyInfo policyInfoValue, ServerWebExchange exchange) { + String serviceId = policyInfoValue.getServiceId(); + String clientId = extractClientIdFromToken(exchange); - // If the service ID from the policy is blank, use the client ID from the token instead - if (serviceId.isBlank()) { - if (clientId != null && !clientId.isBlank()) { - serviceId = clientId; - } + // If the service ID from the policy is blank, use the client ID from the token instead + if (serviceId.isBlank() && clientId != null && !clientId.isBlank()) { + serviceId = clientId; } // Return the determined service ID - logger.debug("ServiceID extracted from token: " + serviceId); + logger.debug("ServiceID extracted from token: {}", serviceId); return serviceId; - } + } - /** - * Extracts the client ID from the Bearer token present in the Authorization header. - * - * @param exchange The ServerWebExchange object that contains request and response information. - * @return The client ID extracted from the token, or null if the token is invalid or missing. - */ - private String extractClientIdFromToken(ServerWebExchange exchange) { - HttpHeaders headers = exchange.getRequest().getHeaders(); - String authHeader = headers.getFirst(HttpHeaders.AUTHORIZATION); + /** + * Extracts the client ID from the Bearer token present in the Authorization header. + * + * @param exchange The ServerWebExchange object that contains request and response information. + * @return The client ID extracted from the token, or null if the token is invalid or missing. + */ + private String extractClientIdFromToken(ServerWebExchange exchange) { + HttpHeaders headers = exchange.getRequest().getHeaders(); + String authHeader = headers.getFirst(HttpHeaders.AUTHORIZATION); - // Check if the Authorization header exists and contains a Bearer token + // Check if the Authorization header exists and contains a Bearer token if (authHeader != null && authHeader.startsWith(BEARER_PREFIX)) { String token = authHeader.substring(BEARER_PREFIX.length()); return decodeClientId(token); @@ -105,28 +101,28 @@ logger.debug("Authorization header is missing or does not contain a Bearer token"); } return null; - } + } - /** - * Decodes the client ID from the JWT token. - * - * @param token The JWT token string. - * @return The client ID extracted from the token, or null if decoding fails. - */ - private String decodeClientId(String token) { - try { - // Split the JWT token to get the payload part - String[] chunks = token.split("\\."); - Base64.Decoder decoder = Base64.getUrlDecoder(); - String payload = new String(decoder.decode(chunks[1])); - JsonObject jsonObject = JsonParser.parseString(payload).getAsJsonObject(); + /** + * Decodes the client ID from the JWT token. + * + * @param token The JWT token string. + * @return The client ID extracted from the token, or null if decoding fails. + */ + private String decodeClientId(String token) { + try { + // Split the JWT token to get the payload part + String[] chunks = token.split("\\."); + Base64.Decoder decoder = Base64.getUrlDecoder(); + String payload = new String(decoder.decode(chunks[1])); + JsonObject jsonObject = JsonParser.parseString(payload).getAsJsonObject(); - // Return the client ID from the payload - return jsonObject.get("client_id").getAsString(); - } catch (Exception e) { - // Log an error if decoding fails - logger.error("Error decoding client ID from token", e); - return null; - } - } - } + // Return the client ID from the payload + return jsonObject.get("client_id").getAsString(); + } catch (Exception e) { + // Log an error if decoding fails + logger.error("Error decoding client ID from token", e); + return null; + } + } +} diff --git a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/util/v3/ReactiveEntryExitFilter.java b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/util/v3/ReactiveEntryExitFilter.java index 866688bf..ff44242d 100644 --- a/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/util/v3/ReactiveEntryExitFilter.java +++ b/a1-policy-management/src/main/java/org/onap/ccsdk/oran/a1policymanagementservice/util/v3/ReactiveEntryExitFilter.java @@ -25,7 +25,6 @@ import com.google.gson.JsonParser; import java.lang.invoke.MethodHandles; import java.nio.charset.StandardCharsets; import java.util.ArrayList; -import java.util.Arrays; import java.util.Base64; import java.util.List; diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterDisableTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterDisableTest.java index 94ab47db..e0764281 100644 --- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterDisableTest.java +++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterDisableTest.java @@ -42,7 +42,9 @@ import reactor.core.publisher.Mono; import java.lang.invoke.MethodHandles; import java.nio.file.Path; +import java.time.Duration; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertFalse; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -94,8 +96,13 @@ class ReactiveEntryExitFilterDisableTest { testHelperTest.testSuccessResponse(responseMono, HttpStatus.CREATED, responseBody -> responseBody.contains("{\"scope\":{\"ueId\":\"ue5100\",\"qosId\":\"qos5100\"},\"qosObjectives\":{\"priorityLevel\":5100.0}}")); testHelperTest.testSuccessHeader(responseMono, "location", headerValue -> headerValue.contains(testHelperTest.baseUrl() + "/a1-policy-management/v1/policies/")); - assertFalse(capturedOutput.getOut().contains("Request received with path: /a1-policy-management/v1/policies")); - assertFalse(capturedOutput.getOut().contains("the Status code of the response: 201 CREATED")); - assertFalse(capturedOutput.getOut().contains("the response is:")); + + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertFalse(capturedOutput.getOut().contains("Request received with path: /a1-policy-management/v1/policies")); + assertFalse(capturedOutput.getOut().contains("the Status code of the response: 201 CREATED")); + assertFalse(capturedOutput.getOut().contains("the response is:")); + }); } } diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludeMultiPathTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludeMultiPathTest.java index 7b2f28d6..15cfa140 100644 --- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludeMultiPathTest.java +++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludeMultiPathTest.java @@ -36,8 +36,9 @@ import org.springframework.http.ResponseEntity; import org.springframework.test.context.TestPropertySource; import reactor.core.publisher.Mono; -import java.io.IOException; +import java.time.Duration; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertFalse; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -70,18 +71,23 @@ class ReactiveEntryExitFilterExcludeMultiPathTest { @Test @DisplayName("test verify entry exit log for health actuator is absent") - void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) throws Exception { + void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) { String url = "/actuator/health"; Mono> responseGetHealthMono = testHelperTest.restClient(testHelperTest.baseUrl(), false).getForEntity(url); testHelperTest.testSuccessResponse(responseGetHealthMono, HttpStatus.OK, responseBody -> responseBody.contains("UP")); - assertFalse(capturedOutput.getOut().contains("Request received with path: /actuator/health")); - assertFalse(capturedOutput.getOut().contains("the response is:")); + + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertFalse(capturedOutput.getOut().contains("Request received with path: /actuator/health")); + assertFalse(capturedOutput.getOut().contains("the response is:")); + }); } @Test @DisplayName("test verify entry exit log for the rics endpoint is absent") - void testGetRicsFilterOmitted(CapturedOutput capturedOutput) throws Exception { + void testGetRicsFilterOmitted(CapturedOutput capturedOutput) { String url = "/rics"; Mono> responseEntityMono = testHelperTest.restClientV3().getForEntity(url); testHelperTest.testSuccessResponse(responseEntityMono, HttpStatus.OK, responseBody -> responseBody diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludePathTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludePathTest.java index 9588e0c5..9d738e27 100644 --- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludePathTest.java +++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterExcludePathTest.java @@ -37,6 +37,9 @@ import org.springframework.test.context.TestPropertySource; import reactor.core.publisher.Mono; +import java.time.Duration; + +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertFalse; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -69,12 +72,17 @@ class ReactiveEntryExitFilterExcludePathTest { @Test @DisplayName("test verify entry exit log for health actuator is absent") - void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) throws Exception { + void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) { String url = "/actuator/health"; Mono> responseGetHealthMono = testHelperTest.restClient(testHelperTest.baseUrl(), false).getForEntity(url); testHelperTest.testSuccessResponse(responseGetHealthMono, HttpStatus.OK, responseBody -> responseBody.contains("UP")); - assertFalse(capturedOutput.getOut().contains("Request received with path: /actuator/health")); - assertFalse(capturedOutput.getOut().contains("the response is:")); + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertFalse(capturedOutput.getOut().contains("Request received with path: /actuator/health")); + assertFalse(capturedOutput.getOut().contains("the response is:")); + }); + } } \ No newline at end of file diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterNullPathTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterNullPathTest.java index ac4779e7..d0aa0ccf 100644 --- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterNullPathTest.java +++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterNullPathTest.java @@ -36,6 +36,8 @@ import org.springframework.http.ResponseEntity; import org.springframework.test.context.TestPropertySource; import reactor.core.publisher.Mono; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.awaitility.Awaitility.await; +import java.time.Duration; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @ExtendWith({OutputCaptureExtension.class}) @@ -66,12 +68,16 @@ class ReactiveEntryExitFilterNullPathTest { @Test @DisplayName("test verify entry exit log for health actuator is present") - void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) throws Exception { + void testHealthActuatorFilterOmitted(CapturedOutput capturedOutput) { String url = "/actuator/health"; Mono> responseGetHealthMono = testHelperTest.restClient(testHelperTest.baseUrl(), false).getForEntity(url); testHelperTest.testSuccessResponse(responseGetHealthMono, HttpStatus.OK, responseBody -> responseBody.contains("UP")); - assertTrue(capturedOutput.getOut().contains("Request received with path: /actuator/health")); - assertTrue(capturedOutput.getOut().contains("the response is:")); + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertTrue(capturedOutput.getOut().contains("Request received with path: /actuator/health")); + assertTrue(capturedOutput.getOut().contains("the response is:")); + }); } } diff --git a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterTest.java b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterTest.java index c51d27b4..da6a2798 100644 --- a/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterTest.java +++ b/a1-policy-management/src/test/java/org/onap/ccsdk/oran/a1policymanagementservice/utils/v3/ReactiveEntryExitFilterTest.java @@ -42,7 +42,9 @@ import reactor.core.publisher.Mono; import java.lang.invoke.MethodHandles; import java.nio.file.Path; +import java.time.Duration; +import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertTrue; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -95,19 +97,29 @@ class ReactiveEntryExitFilterTest { testHelperTest.testSuccessResponse(responseMono, HttpStatus.CREATED, responseBody -> responseBody.contains("{\"scope\":{\"ueId\":\"ue5100\",\"qosId\":\"qos5100\"},\"qosObjectives\":{\"priorityLevel\":5100.0}}")); testHelperTest.testSuccessHeader(responseMono, "location", headerValue -> headerValue.contains(testHelperTest.baseUrl() + "/a1-policy-management/v1/policies/")); - assertTrue(capturedOutput.getOut().contains("Request received with path: /a1-policy-management/v1/policies")); - assertTrue(capturedOutput.getOut().contains("the Status code of the response: 201 CREATED")); - assertTrue(capturedOutput.getOut().contains("the response is:")); + + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertTrue(capturedOutput.getOut().contains("Request received with path: /a1-policy-management/v1/policies")); + assertTrue(capturedOutput.getOut().contains("the Status code of the response: 201 CREATED")); + assertTrue(capturedOutput.getOut().contains("the response is:")); + }); } @Test @DisplayName("test verify entry exit log for health actuator is present") - void testHealthActuatorFilterIncluded(CapturedOutput capturedOutput) throws Exception { + void testHealthActuatorFilterIncluded(CapturedOutput capturedOutput) { String url = "/actuator/health"; Mono> responseGetHealthMono = testHelperTest.restClient(testHelperTest.baseUrl(), false).getForEntity(url); testHelperTest.testSuccessResponse(responseGetHealthMono, HttpStatus.OK, responseBody -> responseBody.contains("UP")); - assertTrue(capturedOutput.getOut().contains("Request received with path: /actuator/health")); - assertTrue(capturedOutput.getOut().contains("the response is:")); + + await().atMost(Duration.ofSeconds(5)) + .pollInterval(Duration.ofMillis(50)) + .untilAsserted(() -> { + assertTrue(capturedOutput.getOut().contains("Request received with path: /actuator/health")); + assertTrue(capturedOutput.getOut().contains("the response is:")); + }); } }