summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPamela Dragosh <pdragosh@research.att.com>2020-02-21 12:48:49 -0500
committerPamela Dragosh <pdragosh@research.att.com>2020-02-21 12:55:43 -0500
commit3710a41879717441884c75262015c536819a34c2 (patch)
tree67bbfc04a95f08c76036234c0d9c06ac7fc84395
parent9a03ea31d42b8a73c3aa8f96bbad85093cb169a7 (diff)
Fix policycreation test
This test was only performing code coverage, it should have revealed that IllegalArgument exceptions were incorrect. Fixed bug in savePolicy when an exception being thrown has a null message. Added warning and error messages for debugging in the future. Issue-ID: POLICY-1590 Change-Id: Ie32a73adbaf944f96e411a2c612cd8293382911c Signed-off-by: Pamela Dragosh <pdragosh@research.att.com>
-rw-r--r--ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/components/PolicyDbDaoTransactionInstance.java5
-rw-r--r--ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreation.java10
-rw-r--r--ONAP-PAP-REST/src/test/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreationTest.java67
3 files changed, 51 insertions, 31 deletions
diff --git a/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/components/PolicyDbDaoTransactionInstance.java b/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/components/PolicyDbDaoTransactionInstance.java
index 67214acf2..e694f7e0b 100644
--- a/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/components/PolicyDbDaoTransactionInstance.java
+++ b/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/components/PolicyDbDaoTransactionInstance.java
@@ -2,7 +2,7 @@
* ============LICENSE_START=======================================================
* ONAP-PAP-REST
* ================================================================================
- * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2019-2020 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2019 Nordix Foundation.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -549,6 +549,9 @@ public class PolicyDbDaoTransactionInstance implements PolicyDbDaoTransaction {
IOUtils.closeQuietly(policyXmlStream);
if (PolicyDbDao.isJunit()) {
+ if (policyDataString != null) {
+ logger.warn("isJUnit will overwrite policyDataString");
+ }
// Using parentPath object to set policy data.
policyDataString = policy.policyAdapter.getParentPath();
}
diff --git a/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreation.java b/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreation.java
index 29b244046..f2f2e8dbc 100644
--- a/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreation.java
+++ b/ONAP-PAP-REST/src/main/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreation.java
@@ -2,7 +2,7 @@
* ============LICENSE_START=======================================================
* ONAP-PAP-REST
* ================================================================================
- * Copyright (C) 2017-2019 AT&T Intellectual Property. All rights reserved.
+ * Copyright (C) 2017-2020 AT&T Intellectual Property. All rights reserved.
* Modifications Copyright (C) 2019 Nordix Foundation.
* ================================================================================
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -618,7 +618,13 @@ public class PolicyCreation extends AbstractPolicyCreation {
} catch (Exception e) {
LOGGER.error("Exception Occured : " + e.getMessage(), e);
body = ERROR;
- response.addHeader(ERROR, e.getMessage());
+ //
+ // Because we are catching any old exception instead of a dedicated exception,
+ // its possible the e.getMessage() returns a null value. You cannot add a header
+ // to the response with a null value, it will throw an exception. This is something
+ // this is undesirable.
+ //
+ response.addHeader(ERROR, (e.getMessage() == null ? "missing exception message" : e.getMessage()));
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
return new ResponseEntity<>(body, status);
diff --git a/ONAP-PAP-REST/src/test/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreationTest.java b/ONAP-PAP-REST/src/test/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreationTest.java
index c182b9bb5..953fff35c 100644
--- a/ONAP-PAP-REST/src/test/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreationTest.java
+++ b/ONAP-PAP-REST/src/test/java/org/onap/policy/pap/xacml/rest/policycontroller/PolicyCreationTest.java
@@ -20,20 +20,16 @@
package org.onap.policy.pap.xacml.rest.policycontroller;
-import static org.assertj.core.api.Assertions.assertThatCode;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
-
-import com.mockrunner.mock.web.MockHttpServletRequest;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-
import org.hibernate.Criteria;
import org.hibernate.Session;
import org.hibernate.SessionFactory;
@@ -47,8 +43,10 @@ import org.onap.policy.rest.jpa.PolicyDbDaoEntity;
import org.onap.policy.rest.jpa.PolicyVersion;
import org.powermock.reflect.Whitebox;
import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.mock.web.MockHttpServletResponse;
+import com.mockrunner.mock.web.MockHttpServletRequest;
public class PolicyCreationTest {
@Test
@@ -66,7 +64,7 @@ public class PolicyCreationTest {
Exception cause = new Exception("oops");
HttpMessageNotReadableException exception = new HttpMessageNotReadableException("oops", cause);
assertEquals(HttpStatus.BAD_REQUEST,
- creation.messageNotReadableExceptionHandler(req, exception).getStatusCode());
+ creation.messageNotReadableExceptionHandler(req, exception).getStatusCode());
HttpServletResponse response = new MockHttpServletResponse();
PolicyRestAdapter policyData = new PolicyRestAdapter();
@@ -74,31 +72,39 @@ public class PolicyCreationTest {
policyData.setConfigPolicyType("ClosedLoop_Fault");
policyData.setDomainDir("domain");
policyData.setPolicyName("policyname");
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ ResponseEntity<String> responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
version.setHigherVersion(1);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setEditPolicy(true);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setEditPolicy(false);
version.setHigherVersion(0);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setEditPolicy(true);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
version.setHigherVersion(1);
policyData.setConfigPolicyType("Firewall Config");
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setConfigPolicyType("BRMS_Raw");
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
+
policyData.setConfigPolicyType("BRMS_Param");
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
+
policyData.setConfigPolicyType("Base");
Mockito.when(policyData.getRuleData()).thenReturn(new LinkedHashMap<>());
@@ -116,26 +122,31 @@ public class PolicyCreationTest {
PolicyDbDaoTransaction mockTransaction = Mockito.mock(PolicyDbDaoTransaction.class);
Mockito.when(mockPolicyDbDao.getNewTransaction()).thenReturn(mockTransaction);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
+
policyData.setConfigPolicyType("ClosedLoop_PM");
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
+
policyData.setConfigPolicyType("Micro Service");
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
+
policyData.setConfigPolicyType("Optimization");
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setPolicyType("Action");
- List<Object> choices = new ArrayList<Object>();
+ List<Object> choices = new ArrayList<>();
policyData.setRuleAlgorithmschoices(choices);
- assertThatCode(() -> creation.savePolicy(policyData, response)).doesNotThrowAnyException();
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
policyData.setPolicyType("Decision");
- List<Object> settings = new ArrayList<Object>();
+ List<Object> settings = new ArrayList<>();
policyData.setSettings(settings);
- assertThatThrownBy(() -> creation.savePolicy(policyData, response))
- .isInstanceOf(IllegalArgumentException.class);
+ responseEntity = creation.savePolicy(policyData, response);
+ assertThat(responseEntity).isNotNull();
}
}