Merge "Policy Executor: handle errors, part 2 (fighting between IntelliJ and Checkstyle best practices)"
diff --git a/checkstyle/src/main/resources/cps-java-style.xml b/checkstyle/src/main/resources/cps-java-style.xml
index 67b2863..d10484c 100644
--- a/checkstyle/src/main/resources/cps-java-style.xml
+++ b/checkstyle/src/main/resources/cps-java-style.xml
@@ -32,4 +32,4 @@
</module>
<module name="UnusedImports"/>
</module>
-</module>
\ No newline at end of file
+</module>
diff --git a/cps-application/src/main/resources/application.yml b/cps-application/src/main/resources/application.yml
index b97eaba..05b0d09 100644
--- a/cps-application/src/main/resources/application.yml
+++ b/cps-application/src/main/resources/application.yml
@@ -190,7 +190,7 @@
ncmp:
policy-executor:
enabled: ${POLICY_SERVICE_ENABLED:false}
- defaultDecision: "allow"
+ defaultDecision: ${POLICY_SERVICE_DEFAULT_DECISION:"allow"}
server:
address: ${POLICY_SERVICE_URL:http://policy-executor-stub}
port: ${POLICY_SERVICE_PORT:8093}
diff --git a/cps-dependencies/pom.xml b/cps-dependencies/pom.xml
index 2defa70..58e87a8 100644
--- a/cps-dependencies/pom.xml
+++ b/cps-dependencies/pom.xml
@@ -126,7 +126,12 @@
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs</artifactId>
- <version>4.2.3</version>
+ <version>4.8.6</version>
+ </dependency>
+ <dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ <version>3.1.3</version>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyRestExceptionHandlerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyRestExceptionHandlerSpec.groovy
index 9d36d10..e87acac 100644
--- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyRestExceptionHandlerSpec.groovy
+++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/NetworkCmProxyRestExceptionHandlerSpec.groovy
@@ -129,19 +129,19 @@
then: 'an HTTP response is returned with correct message and details'
assertTestResponse(response, expectedErrorCode, expectedErrorMessage, expectedErrorDetails)
where:
- scenario | exception || expectedErrorCode | expectedErrorMessage | expectedErrorDetails
- 'CPS' | new CpsException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | sampleErrorDetails
- 'NCMP-server' | new ServerNcmpException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null
- 'DMI Request' | new DmiRequestException(sampleErrorMessage, sampleErrorDetails) || BAD_REQUEST | sampleErrorMessage | null
- 'Invalid Operation' | new InvalidOperationException('some reason') || BAD_REQUEST | 'some reason' | null
- 'Unsupported Operation' | new OperationNotSupportedException('not yet') || BAD_REQUEST | 'not yet' | null
- 'DataNode Validation' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | null
- 'other' | new IllegalStateException(sampleErrorMessage) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null
- 'Data Node Not Found' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | 'DataNode not found'
- 'Existing entry' | new AlreadyDefinedException('name',null) || CONFLICT | 'Already defined exception' | 'name already exists'
- 'Existing entries' | AlreadyDefinedException.forDataNodes(['A', 'B'], 'myAnchorName') || CONFLICT | 'Already defined exception' | '2 data node(s) already exist'
- 'Operation too large' | new PayloadTooLargeException(sampleErrorMessage) || PAYLOAD_TOO_LARGE | sampleErrorMessage | 'Check logs'
- 'Policy Executor' | new PolicyExecutorException(sampleErrorMessage, sampleErrorDetails) || CONFLICT | sampleErrorMessage | sampleErrorDetails
+ scenario | exception || expectedErrorCode | expectedErrorMessage | expectedErrorDetails
+ 'CPS' | new CpsException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | sampleErrorDetails
+ 'NCMP-server' | new ServerNcmpException(sampleErrorMessage, sampleErrorDetails) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null
+ 'DMI Request' | new DmiRequestException(sampleErrorMessage, sampleErrorDetails) || BAD_REQUEST | sampleErrorMessage | null
+ 'Invalid Operation' | new InvalidOperationException('some reason') || BAD_REQUEST | 'some reason' | null
+ 'Unsupported Operation' | new OperationNotSupportedException('not yet') || BAD_REQUEST | 'not yet' | null
+ 'DataNode Validation' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | null
+ 'other' | new IllegalStateException(sampleErrorMessage) || INTERNAL_SERVER_ERROR | sampleErrorMessage | null
+ 'Data Node Not Found' | new DataNodeNotFoundException('myDataspaceName', 'myAnchorName') || NOT_FOUND | 'DataNode not found' | 'DataNode not found'
+ 'Existing entry' | new AlreadyDefinedException('name',null) || CONFLICT | 'Already defined exception' | 'name already exists'
+ 'Existing entries' | AlreadyDefinedException.forDataNodes(['A', 'B'], 'myAnchorName') || CONFLICT | 'Already defined exception' | '2 data node(s) already exist'
+ 'Operation too large' | new PayloadTooLargeException(sampleErrorMessage) || PAYLOAD_TOO_LARGE | sampleErrorMessage | 'Check logs'
+ 'Policy Executor' | new PolicyExecutorException(sampleErrorMessage, sampleErrorDetails, null) || CONFLICT | sampleErrorMessage | sampleErrorDetails
}
def 'Post request with exception returns correct HTTP Status.'() {
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
index 6754965..3c81d0f 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/NcmpException.java
@@ -39,7 +39,18 @@
* @param details the error details
*/
public NcmpException(final String message, final String details) {
- super(message);
+ this(message, details, null);
+ }
+
+ /**
+ * Constructor with cause.
+ *
+ * @param message the error message
+ * @param details the error details
+ * @param cause the cause of the exception
+ */
+ public NcmpException(final String message, final String details, final Throwable cause) {
+ super(message, cause);
this.details = details;
}
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
index 333c122..bb753b8 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/exceptions/PolicyExecutorException.java
@@ -35,8 +35,9 @@
*
* @param message response message
* @param details response details
+ * @param cause the cause of the exception
*/
- public PolicyExecutorException(final String message, final String details) {
- super(message, details);
+ public PolicyExecutorException(final String message, final String details, final Throwable cause) {
+ super(message, details, cause);
}
}
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
index caed28a..af43318 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutor.java
@@ -23,6 +23,7 @@
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
+import java.net.UnknownHostException;
import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.Collections;
@@ -45,6 +46,7 @@
import org.springframework.stereotype.Service;
import org.springframework.web.reactive.function.BodyInserters;
import org.springframework.web.reactive.function.client.WebClient;
+import org.springframework.web.reactive.function.client.WebClientResponseException;
@Slf4j
@Service
@@ -71,6 +73,8 @@
private final ObjectMapper objectMapper;
+ private static final Throwable NO_ERROR = null;
+
/**
* Use the Policy Executor to check permission for a cm write operation.
* Wil throw an exception when the operation is not permitted (work in progress)
@@ -88,26 +92,20 @@
final String changeRequestAsJson) {
log.trace("Policy Executor Enabled: {}", enabled);
if (enabled) {
- ResponseEntity<JsonNode> responseEntity = null;
try {
- responseEntity =
- getPolicyExecutorResponse(yangModelCmHandle, operationType, authorization, resourceIdentifier,
- changeRequestAsJson);
- } catch (final RuntimeException runtimeException) {
- processException(runtimeException);
- }
- if (responseEntity == null) {
- log.warn("No valid response from Policy Executor, ignored");
- return;
- }
- if (responseEntity.getStatusCode().is2xxSuccessful()) {
- if (responseEntity.getBody() == null) {
+ final ResponseEntity<JsonNode> responseEntity = getPolicyExecutorResponse(yangModelCmHandle,
+ operationType,
+ authorization,
+ resourceIdentifier,
+ changeRequestAsJson);
+ final JsonNode responseBody = responseEntity.getBody();
+ if (responseBody == null) {
log.warn("No valid response body from Policy Executor, ignored");
return;
}
- processSuccessResponse(responseEntity.getBody());
- } else {
- processNon2xxResponse(responseEntity.getStatusCode().value());
+ processSuccessResponse(responseBody);
+ } catch (final RuntimeException runtimeException) {
+ processException(runtimeException);
}
}
}
@@ -173,35 +171,17 @@
.block();
}
- private void processNon2xxResponse(final int httpStatusCode) {
- processFallbackResponse("Policy Executor returned HTTP Status code " + httpStatusCode + ".");
- }
-
- private void processException(final RuntimeException runtimeException) {
- if (runtimeException.getCause() instanceof TimeoutException) {
- processFallbackResponse("Policy Executor request timed out.");
- } else {
- log.warn("Request to Policy Executor failed with unexpected exception", runtimeException);
- throw runtimeException;
- }
- }
-
- private void processFallbackResponse(final String message) {
- final String decisionId = "N/A";
- final String decision = defaultDecision;
- final String warning = message + " Falling back to configured default decision: " + defaultDecision;
- log.warn(warning);
- processDecision(decisionId, decision, warning);
- }
-
private static void processSuccessResponse(final JsonNode responseBody) {
final String decisionId = responseBody.path("decisionId").asText("unknown id");
final String decision = responseBody.path("decision").asText("unknown");
final String messageFromPolicyExecutor = responseBody.path("message").asText();
- processDecision(decisionId, decision, messageFromPolicyExecutor);
+ processDecision(decisionId, decision, messageFromPolicyExecutor, NO_ERROR);
}
- private static void processDecision(final String decisionId, final String decision, final String details) {
+ private static void processDecision(final String decisionId,
+ final String decision,
+ final String details,
+ final Throwable optionalCauseOfError) {
log.trace("Policy Executor decision id: {} ", decisionId);
if ("allow".equals(decision)) {
log.trace("Operation allowed.");
@@ -209,8 +189,37 @@
log.warn("Policy Executor decision: {}", decision);
log.warn("Policy Executor message: {}", details);
final String message = "Operation not allowed. Decision id " + decisionId + " : " + decision;
- throw new PolicyExecutorException(message, details);
+ throw new PolicyExecutorException(message, details, optionalCauseOfError);
}
}
+ private void processException(final RuntimeException runtimeException) {
+ if (runtimeException instanceof WebClientResponseException) {
+ final WebClientResponseException webClientResponseException = (WebClientResponseException) runtimeException;
+ final int httpStatusCode = webClientResponseException.getStatusCode().value();
+ processFallbackResponse("Policy Executor returned HTTP Status code " + httpStatusCode + ".",
+ webClientResponseException);
+ } else {
+ final Throwable cause = runtimeException.getCause();
+ if (cause instanceof TimeoutException) {
+ processFallbackResponse("Policy Executor request timed out.", cause);
+ } else if (cause instanceof UnknownHostException) {
+ final String message
+ = String.format("Cannot connect to Policy Executor (%s:%s).", serverAddress, serverPort);
+ processFallbackResponse(message, cause);
+ } else {
+ log.warn("Request to Policy Executor failed with unexpected exception", runtimeException);
+ throw runtimeException;
+ }
+ }
+ }
+
+ private void processFallbackResponse(final String message, final Throwable cause) {
+ final String decisionId = "N/A";
+ final String decision = defaultDecision;
+ final String warning = message + " Falling back to configured default decision: " + defaultDecision;
+ log.warn(warning);
+ processDecision(decisionId, decision, warning, cause);
+ }
+
}
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
index 4bc2f10..1d4e3c8 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/sync/lcm/LcmEventType.java
@@ -26,10 +26,10 @@
private final String eventName;
- private final String eventTypeTemplate = "org.onap.ncmp.cmhandle-lcm-event.%s";
+ private static final String EVENT_TYPE_TEMPLATE = "org.onap.ncmp.cmhandle-lcm-event.%s";
LcmEventType(final String eventName) {
- this.eventName = String.format(eventTypeTemplate, eventName);
+ this.eventName = String.format(EVENT_TYPE_TEMPLATE, eventName);
}
public String getEventType() {
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
index 46c0dde..33dcf5d 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/policyexecutor/PolicyExecutorSpec.groovy
@@ -33,6 +33,7 @@
import org.springframework.http.HttpStatus
import org.springframework.http.ResponseEntity
import org.springframework.web.reactive.function.client.WebClient
+import org.springframework.web.reactive.function.client.WebClientResponseException
import reactor.core.publisher.Mono
import spock.lang.Specification
@@ -59,6 +60,8 @@
def setup() {
setupLogger()
objectUnderTest.enabled = true
+ objectUnderTest.serverAddress = 'some host'
+ objectUnderTest.serverPort = 'some port'
mockWebClient.post() >> mockRequestBodyUriSpec
mockRequestBodyUriSpec.uri(*_) >> mockRequestBodyUriSpec
mockRequestBodyUriSpec.header(*_) >> mockRequestBodyUriSpec
@@ -95,8 +98,8 @@
}
def 'Permission check with non-2xx response and "allow" default decision.'() {
- given: 'other http response'
- mockResponse([], HttpStatus.I_AM_A_TEAPOT)
+ given: 'non-2xx http response'
+ mockErrorResponse()
and: 'the configured default decision is "allow"'
objectUnderTest.defaultDecision = 'allow'
when: 'permission is checked for an operation'
@@ -106,8 +109,8 @@
}
def 'Permission check with non-2xx response and "other" default decision.'() {
- given: 'other http response'
- mockResponse([], HttpStatus.I_AM_A_TEAPOT)
+ given: 'non-2xx http response'
+ def webClientException = mockErrorResponse()
and: 'the configured default decision is NOT "allow"'
objectUnderTest.defaultDecision = 'deny by default'
when: 'permission is checked for an operation'
@@ -116,25 +119,23 @@
def thrownException = thrown(PolicyExecutorException)
assert thrownException.message == 'Operation not allowed. Decision id N/A : deny by default'
assert thrownException.details == 'Policy Executor returned HTTP Status code 418. Falling back to configured default decision: deny by default'
+ and: 'the cause is the original web client exception'
+ assert thrownException.cause == webClientException
}
def 'Permission check with invalid response from Policy Executor.'() {
given: 'invalid response from Policy executor'
- mockResponseSpec.toEntity(*_) >> invalidResponse
+ mockResponseSpec.toEntity(*_) >> Mono.just(new ResponseEntity<>(null, HttpStatus.OK))
when: 'permission is checked for an operation'
objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', someValidJson)
then: 'system logs the expected message'
- assert getLogEntry(1) == expectedMessage
- where: 'following invalid responses are received'
- invalidResponse || expectedMessage
- Mono.empty() || 'No valid response from Policy Executor, ignored'
- Mono.just(new ResponseEntity<>(null, HttpStatus.OK)) || 'No valid response body from Policy Executor, ignored'
+ assert getLogEntry(1) == 'No valid response body from Policy Executor, ignored'
}
def 'Permission check with timeout exception.'() {
given: 'a timeout during the request'
- def cause = new TimeoutException()
- mockResponseSpec.toEntity(*_) >> { throw new RuntimeException(cause) }
+ def timeoutException = new TimeoutException()
+ mockResponseSpec.toEntity(*_) >> { throw new RuntimeException(timeoutException) }
and: 'the configured default decision is NOT "allow"'
objectUnderTest.defaultDecision = 'deny by default'
when: 'permission is checked for an operation'
@@ -143,6 +144,39 @@
def thrownException = thrown(PolicyExecutorException)
assert thrownException.message == 'Operation not allowed. Decision id N/A : deny by default'
assert thrownException.details == 'Policy Executor request timed out. Falling back to configured default decision: deny by default'
+ and: 'the cause is the original time out exception'
+ assert thrownException.cause == timeoutException
+ }
+
+ def 'Permission check with unknown host.'() {
+ given: 'a unknown host exception during the request'
+ def unknownHostException = new UnknownHostException()
+ mockResponseSpec.toEntity(*_) >> { throw new RuntimeException(unknownHostException) }
+ and: 'the configured default decision is NOT "allow"'
+ objectUnderTest.defaultDecision = 'deny by default'
+ when: 'permission is checked for an operation'
+ objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', someValidJson)
+ then: 'Policy Executor exception is thrown'
+ def thrownException = thrown(PolicyExecutorException)
+ assert thrownException.message == 'Operation not allowed. Decision id N/A : deny by default'
+ assert thrownException.details == 'Cannot connect to Policy Executor (some host:some port). Falling back to configured default decision: deny by default'
+ and: 'the cause is the original unknown host exception'
+ assert thrownException.cause == unknownHostException
+ }
+
+ def 'Permission check with #scenario exception and default decision "allow".'() {
+ given: 'a #scenario exception during the request'
+ mockResponseSpec.toEntity(*_) >> { throw new RuntimeException(cause)}
+ and: 'the configured default decision is "allow"'
+ objectUnderTest.defaultDecision = 'allow'
+ when: 'permission is checked for an operation'
+ objectUnderTest.checkPermission(new YangModelCmHandle(), CREATE, 'my credentials', 'my resource', someValidJson)
+ then: 'no exception is thrown'
+ noExceptionThrown()
+ where: 'the following exceptions are thrown during the request'
+ scenario | cause
+ 'timeout' | new TimeoutException()
+ 'unknown host' | new UnknownHostException()
}
def 'Permission check with other runtime exception.'() {
@@ -180,6 +214,13 @@
mockResponseSpec.toEntity(*_) >> mono
}
+ def mockErrorResponse() {
+ def webClientResponseException = Mock(WebClientResponseException)
+ webClientResponseException.getStatusCode() >> HttpStatus.I_AM_A_TEAPOT
+ mockResponseSpec.toEntity(*_) >> { throw webClientResponseException }
+ return webClientResponseException
+ }
+
def setupLogger() {
def logger = LoggerFactory.getLogger(PolicyExecutor)
logger.setLevel(Level.TRACE)
diff --git a/cps-parent/pom.xml b/cps-parent/pom.xml
index d583f1d..256e7a7 100644
--- a/cps-parent/pom.xml
+++ b/cps-parent/pom.xml
@@ -154,12 +154,12 @@
<plugin>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
- <version>4.4.2</version>
+ <version>4.8.6.4</version>
<dependencies>
<dependency>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs</artifactId>
- <version>4.2.3</version>
+ <version>4.8.6</version>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
diff --git a/cps-ri/pom.xml b/cps-ri/pom.xml
index c3ad6a2..bcf6c80 100644
--- a/cps-ri/pom.xml
+++ b/cps-ri/pom.xml
@@ -68,6 +68,11 @@
<artifactId>postgresql</artifactId>
<version>${postgres.version}</version>
</dependency>
+ <!-- Disable SpotBug Rules -->
+ <dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ </dependency>
<!-- Add Hibernate support for Postgres datatype JSONB and Postgres arrays -->
<dependency>
<groupId>io.hypersistence</groupId>
diff --git a/cps-ri/src/main/java/org/onap/cps/ri/repository/ModuleReferenceRepositoryImpl.java b/cps-ri/src/main/java/org/onap/cps/ri/repository/ModuleReferenceRepositoryImpl.java
index 702b589..4ca02a9 100644
--- a/cps-ri/src/main/java/org/onap/cps/ri/repository/ModuleReferenceRepositoryImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/ri/repository/ModuleReferenceRepositoryImpl.java
@@ -20,6 +20,7 @@
package org.onap.cps.ri.repository;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import jakarta.persistence.EntityManager;
import jakarta.persistence.PersistenceContext;
import jakarta.persistence.Query;
@@ -118,6 +119,7 @@
query.setParameter(4, dataspaceName);
}
+ @SuppressFBWarnings(value = "VA_FORMAT_STRING_USES_NEWLINE", justification = "no \n in string just in file format")
private String buildModuleReferencesSqlQuery(final String parentFragmentClause, final String childFragmentClause) {
return """
WITH Fragment AS (
diff --git a/cps-service/pom.xml b/cps-service/pom.xml
index 6e53c2d..68cd206 100644
--- a/cps-service/pom.xml
+++ b/cps-service/pom.xml
@@ -64,6 +64,11 @@
<groupId>com.github.ben-manes.caffeine</groupId>
<artifactId>caffeine</artifactId>
</dependency>
+ <!-- Disable SpotBug Rules -->
+ <dependency>
+ <groupId>com.github.spotbugs</groupId>
+ <artifactId>spotbugs-annotations</artifactId>
+ </dependency>
<dependency>
<!-- For parsing JSON object -->
<groupId>com.google.code.gson</groupId>
@@ -139,6 +144,7 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
+
<!-- T E S T D E P E N D E N C I E S -->
<dependency>
<groupId>org.codehaus.groovy</groupId>
diff --git a/cps-service/src/main/java/org/onap/cps/utils/YangParserHelper.java b/cps-service/src/main/java/org/onap/cps/utils/YangParserHelper.java
index a5865be..d95acea 100644
--- a/cps-service/src/main/java/org/onap/cps/utils/YangParserHelper.java
+++ b/cps-service/src/main/java/org/onap/cps/utils/YangParserHelper.java
@@ -22,6 +22,7 @@
import com.google.gson.JsonSyntaxException;
import com.google.gson.stream.JsonReader;
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.io.IOException;
import java.io.StringReader;
import java.net.URISyntaxException;
@@ -128,6 +129,7 @@
return dataContainerNodeBuilder.build();
}
+ @SuppressFBWarnings(value = "DCN_NULLPOINTER_EXCEPTION", justification = "Problem originates in 3PP code")
private ContainerNode parseXmlData(final String xmlData,
final SchemaContext schemaContext,
final String parentNodeXpath) {
diff --git a/docker-compose/config/nginx/nginx.conf b/docker-compose/config/nginx/nginx.conf
index 61fed51..7d6b997 100644
--- a/docker-compose/config/nginx/nginx.conf
+++ b/docker-compose/config/nginx/nginx.conf
@@ -22,6 +22,7 @@
upstream cps-and-ncmp {
least_conn;
server docker-compose-cps-and-ncmp-1:8080;
+ ### DEBUG: Disable next line for easier debugging on 1 instance (see also docker-compose.yml)
server docker-compose-cps-and-ncmp-2:8080;
}
diff --git a/docker-compose/docker-compose.yml b/docker-compose/docker-compose.yml
index 1e47d47..fd1df38 100644
--- a/docker-compose/docker-compose.yml
+++ b/docker-compose/docker-compose.yml
@@ -23,6 +23,7 @@
### docker-compose --profile dmi-stub --profile tracing up -d -> run CPS with stubbed dmi-plugin (for open telemetry tracing testing make ONAP_TRACING_ENABLED "true" later "http://localhost:16686" can be accessed from browser)
### docker-compose --profile dmi-stub --profile policy-executor-stub up -d -> run CPS with stubbed dmi-plugin and policy executor stub (for policy executor service testing make POLICY_SERVICE_ENABLED "true")
### to disable notifications make notification.enabled to false & comment out kafka/zookeeper services ###
+ ### DEBUG: Look for '### DEBUG' comments to enable CPS-NCMP debugging
dbpostgresql:
container_name: dbpostgresql
@@ -60,10 +61,14 @@
ONAP_OTEL_SAMPLER_JAEGER_REMOTE_ENDPOINT: http://jaeger-service:14250
ONAP_OTEL_EXPORTER_ENDPOINT: http://jaeger-service:4317
POLICY_SERVICE_ENABLED: 'false'
+ POLICY_SERVICE_DEFAULT_DECISION: 'deny from env'
+ ### DEBUG: Uncomment next line to enable java debugging
+ ### DEBUG: JAVA_TOOL_OPTIONS: -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=*:5005
restart: unless-stopped
depends_on:
- dbpostgresql
deploy:
+ ### DEBUG: For easier debugging use just 1 instance (also update docker-compose/config/nginx/nginx.conf !)
replicas: 2
resources:
reservations:
@@ -72,6 +77,9 @@
limits:
cpus: '3'
memory: 3G
+ ### DEBUG: Uncomment next 2 lines to enable java debugging (ensure 'ports' aligns with 'deploy')
+ ### DEBUG ports:
+ ### DEBUG - ${CPS_CORE_DEBUG_PORT:-5005}:5005
nginx:
container_name: nginx-loadbalancer
diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/base/PolicyDispatcher.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/base/PolicyDispatcher.groovy
index c93a527..b08d1c1 100644
--- a/integration-test/src/test/groovy/org/onap/cps/integration/base/PolicyDispatcher.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/base/PolicyDispatcher.groovy
@@ -20,15 +20,14 @@
package org.onap.cps.integration.base
-
import okhttp3.mockwebserver.Dispatcher
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.RecordedRequest
-import org.springframework.beans.factory.annotation.Value
import org.springframework.http.HttpHeaders
import org.springframework.http.HttpStatus
import org.springframework.http.MediaType
import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper
+
import java.util.concurrent.TimeUnit
/**
diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/PolicyExecutorIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/PolicyExecutorIntegrationSpec.groovy
index 1d4d19b..56d4bfa 100644
--- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/PolicyExecutorIntegrationSpec.groovy
+++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/PolicyExecutorIntegrationSpec.groovy
@@ -68,7 +68,7 @@
'accepted cm handle' | 'ch-1' | 'mock expects "ABC"' || 201 || 'allow'
'un-accepted cm handle' | 'ch-2' | 'mock expects "ABC"' || 409 || 'deny from mock server (dispatcher)'
'timeout' | 'ch-3' | 'mock expects "ABC"' || 409 || 'test default decision'
- 'invalid authorization' | 'ch-1' | 'something else' || 500 || '401 Unauthorized from POST http://localhost:8790/policy-executor/api/v1/execute'
+ 'invalid authorization' | 'ch-1' | 'something else' || 409 || 'test default decision'
}
}
diff --git a/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubController.java b/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubController.java
index cdd26c9..88073c0 100644
--- a/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubController.java
+++ b/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubController.java
@@ -41,9 +41,11 @@
@Slf4j
public class PolicyExecutorStubController implements PolicyExecutorApi {
+ private final Sleeper sleeper;
private final ObjectMapper objectMapper;
private static final Pattern ERROR_CODE_PATTERN = Pattern.compile("(\\d{3})");
private int decisionCounter = 0;
+ private static int slowResponseTimeInSeconds = 40;
@Override
public ResponseEntity<PolicyExecutionResponse> executePolicyAction(
@@ -85,7 +87,14 @@
final String decisionId = String.valueOf(++decisionCounter);
final String decision;
final String message;
-
+ if (targetIdentifier.toLowerCase(Locale.getDefault()).contains("slow")) {
+ try {
+ sleeper.haveALittleRest(slowResponseTimeInSeconds);
+ } catch (final InterruptedException e) {
+ log.trace("Sleep interrupted, re-interrupting the thread");
+ Thread.currentThread().interrupt(); // Re-interrupt the thread
+ }
+ }
if (targetIdentifier.toLowerCase(Locale.getDefault()).contains("cps-is-great")) {
decision = "allow";
message = "All good";
diff --git a/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/Sleeper.java b/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/Sleeper.java
new file mode 100644
index 0000000..8f904cc
--- /dev/null
+++ b/policy-executor-stub/src/main/java/org/onap/cps/policyexecutor/stub/controller/Sleeper.java
@@ -0,0 +1,35 @@
+/*
+ * ============LICENSE_START=======================================================
+ * Copyright (C) 2024 Nordix Foundation
+ * ================================================================================
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.cps.policyexecutor.stub.controller;
+
+import java.util.concurrent.TimeUnit;
+import org.springframework.stereotype.Service;
+
+/**
+ * This class is a successfull experiment to extract out sleep functionality so the interrupted exception handling can
+ * be covered with a test (e.g. using spy ion Sleeper) and help to get too 100% code coverage.
+ */
+@Service
+public class Sleeper {
+ public void haveALittleRest(final int timeInSeconds) throws InterruptedException {
+ TimeUnit.SECONDS.sleep(timeInSeconds);
+ }
+}
diff --git a/policy-executor-stub/src/test/groovy/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubControllerSpec.groovy b/policy-executor-stub/src/test/groovy/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubControllerSpec.groovy
index 064e023..44460da 100644
--- a/policy-executor-stub/src/test/groovy/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubControllerSpec.groovy
+++ b/policy-executor-stub/src/test/groovy/org/onap/cps/policyexecutor/stub/controller/PolicyExecutorStubControllerSpec.groovy
@@ -25,6 +25,7 @@
import org.onap.cps.policyexecutor.stub.model.PolicyExecutionRequest
import org.onap.cps.policyexecutor.stub.model.PolicyExecutionResponse
import org.onap.cps.policyexecutor.stub.model.Request
+import org.spockframework.spring.SpringBean
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest
import org.springframework.http.HttpStatus
@@ -43,8 +44,15 @@
@Autowired
ObjectMapper objectMapper
+ @SpringBean
+ Sleeper sleeper = Spy()
+
def url = '/policy-executor/api/v1/some-action'
+ def setup() {
+ PolicyExecutorStubController.slowResponseTimeInSeconds = 1
+ }
+
def 'Execute policy action.'() {
given: 'a policy execution request with target: #targetIdentifier'
def requestBody = createRequestBody(targetIdentifier)
@@ -66,6 +74,7 @@
targetIdentifier || expectedDecsisonId | expectedDecision | expectedMessage
'some fdn' || '1' | 'deny' | "Only FDNs containing 'cps-is-great' are allowed"
'fdn with cps-is-great' || '2' | 'allow' | 'All good'
+ 'slow' || '3' | 'deny' | "Only FDNs containing 'cps-is-great' are allowed"
}
def 'Execute policy action with a HTTP error code.'() {
@@ -118,6 +127,19 @@
assert response.status == HttpStatus.BAD_REQUEST.value()
}
+ def 'Execute policy action with interrupted exception during slow response.'() {
+ given: 'a policy execution request with target: "slow"'
+ def requestBody = createRequestBody('slow')
+ sleeper.haveALittleRest(_) >> { throw new InterruptedException() }
+ when: 'request is posted'
+ mockMvc.perform(post(url)
+ .header('Authorization','some string')
+ .contentType(MediaType.APPLICATION_JSON)
+ .content(requestBody))
+ then: 'response status is Bad Request'
+ noExceptionThrown()
+ }
+
def 'Execute policy action with missing or invalid attributes.'() {
given: 'a policy execution request with decisionType=#decisionType, schema=#schema, targetIdentifier=#targetIdentifier'
def requestBody = createRequestBody(decisionType, schema, targetIdentifier)