From b13e905e3de91ac7a018f61a95064c170821329e Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Tue, 23 Apr 2019 11:43:50 -0400 Subject: Return appropriate HTTP status code Return 404 instead of 500 when items are not found. Bubble up other HTTP error codes instead of changing them all to 500. Change-Id: Ib614bb83f28cfb1ce2384679398f0a45058fc455 Issue-ID: POLICY-1686 Signed-off-by: Jim Hahn --- .../depundep/TestPdpGroupDeleteControllerV1.java | 15 +- .../rest/depundep/TestPdpGroupDeleteProvider.java | 43 ++---- .../depundep/TestPdpGroupDeployControllerV1.java | 11 +- .../rest/depundep/TestPdpGroupDeployProvider.java | 162 +++++++++++---------- .../pap/main/rest/depundep/TestProviderBase.java | 85 ++++------- .../pap/main/rest/depundep/TestSessionData.java | 6 +- .../pap/main/rest/e2e/PdpGroupDeleteTest.java | 2 +- .../pap/main/rest/e2e/PdpGroupDeployTest.java | 2 +- 8 files changed, 143 insertions(+), 183 deletions(-) (limited to 'main/src/test/java') diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteControllerV1.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteControllerV1.java index f7390553..670d1e05 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteControllerV1.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteControllerV1.java @@ -29,6 +29,9 @@ import org.junit.Test; import org.onap.policy.models.pap.concepts.PdpGroupDeleteResponse; import org.onap.policy.pap.main.rest.CommonPapRestServer; +/** + * Note: this tests failure cases; success cases are tested by tests in the "e2e" package. + */ public class TestPdpGroupDeleteControllerV1 extends CommonPapRestServer { private static final String GROUP_NOT_FOUND = "group not found"; @@ -50,12 +53,12 @@ public class TestPdpGroupDeleteControllerV1 extends CommonPapRestServer { Invocation.Builder invocationBuilder = sendRequest(uri); Response rawresp = invocationBuilder.delete(); PdpGroupDeleteResponse resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals(GROUP_NOT_FOUND, resp.getErrorDetails()); rawresp = invocationBuilder.delete(); resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals(GROUP_NOT_FOUND, resp.getErrorDetails()); // verify it fails when no authorization info is included @@ -69,12 +72,12 @@ public class TestPdpGroupDeleteControllerV1 extends CommonPapRestServer { Invocation.Builder invocationBuilder = sendRequest(uri); Response rawresp = invocationBuilder.delete(); PdpGroupDeleteResponse resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals("cannot find policy: my-name null", resp.getErrorDetails()); rawresp = invocationBuilder.delete(); resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals("cannot find policy: my-name null", resp.getErrorDetails()); // verify it fails when no authorization info is included @@ -88,12 +91,12 @@ public class TestPdpGroupDeleteControllerV1 extends CommonPapRestServer { Invocation.Builder invocationBuilder = sendRequest(uri); Response rawresp = invocationBuilder.delete(); PdpGroupDeleteResponse resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals("cannot find policy: my-name 3", resp.getErrorDetails()); rawresp = invocationBuilder.delete(); resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals("cannot find policy: my-name 3", resp.getErrorDetails()); // verify it fails when no authorization info is included diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteProvider.java index 5824b4b0..72765ce6 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteProvider.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeleteProvider.java @@ -23,7 +23,6 @@ package org.onap.policy.pap.main.rest.depundep; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; @@ -36,23 +35,19 @@ import static org.mockito.Mockito.when; import java.util.Arrays; import java.util.List; -import java.util.function.BiConsumer; import java.util.function.BiFunction; import javax.ws.rs.core.Response.Status; -import org.apache.commons.lang3.tuple.Pair; import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.onap.policy.common.utils.services.Registry; import org.onap.policy.models.base.PfModelException; -import org.onap.policy.models.pap.concepts.PdpGroupDeleteResponse; import org.onap.policy.models.pdp.concepts.PdpGroup; import org.onap.policy.models.pdp.concepts.PdpSubGroup; import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.pdp.enums.PdpState; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifier; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; -import org.onap.policy.pap.main.PolicyPapRuntimeException; public class TestPdpGroupDeleteProvider extends ProviderSuper { private static final String EXPECTED_EXCEPTION = "expected exception"; @@ -111,14 +106,17 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { when(session.getGroup(GROUP1_NAME)).thenReturn(group); - assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isInstanceOf(PolicyPapRuntimeException.class) + assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isInstanceOf(PfModelException.class) .hasMessage("group is still ACTIVE"); } @Test public void testDeleteGroup_NotFound() throws Exception { - assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isInstanceOf(PolicyPapRuntimeException.class) - .hasMessage("group not found"); + assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isInstanceOf(PfModelException.class) + .hasMessage("group not found").matches(thr -> { + PfModelException ex = (PfModelException) thr; + return (ex.getErrorResponse().getResponseCode() == Status.NOT_FOUND); + }); } @Test @@ -144,15 +142,12 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { PfModelException ex = new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); doThrow(ex).when(session).deleteGroupFromDb(group); - assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isInstanceOf(PolicyPapRuntimeException.class) - .hasMessage(ProviderBase.DB_ERROR_MSG); + assertThatThrownBy(() -> prov.deleteGroup(GROUP1_NAME)).isSameAs(ex); } @Test - public void testUndeploy_testDeletePolicy() { - Pair pair = prov.undeploy(optIdent); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + public void testUndeploy_testDeletePolicy() throws Exception { + prov.undeploy(optIdent); } /** @@ -169,9 +164,7 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group)); when(dao.getFilteredPolicyList(any())).thenReturn(Arrays.asList(policy1)); - Pair pair = new PdpGroupDeleteProvider().undeploy(optIdent); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + new PdpGroupDeleteProvider().undeploy(optIdent); // should have updated the old group List updates = getGroupUpdates(); @@ -199,8 +192,7 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { prov = spy(prov); doThrow(exc).when(prov).processPolicy(any(), any()); - assertThatThrownBy(() -> prov.undeploy(optIdent)).isInstanceOf(PolicyPapRuntimeException.class) - .hasMessage(PdpGroupDeleteProvider.DB_ERROR_MSG); + assertThatThrownBy(() -> prov.undeploy(optIdent)).isSameAs(exc); } @Test @@ -213,15 +205,6 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { assertThatThrownBy(() -> prov.undeploy(optIdent)).isSameAs(exc); } - @Test - public void testMakeResponse() { - PdpGroupDeleteResponse resp = prov.makeResponse(null); - assertNull(resp.getErrorDetails()); - - resp = prov.makeResponse(EXPECTED_EXCEPTION); - assertEquals(EXPECTED_EXCEPTION, resp.getErrorDetails()); - } - @Test public void testMakeUpdater() { /* @@ -263,10 +246,8 @@ public class TestPdpGroupDeleteProvider extends ProviderSuper { private class MyProvider extends PdpGroupDeleteProvider { @Override - protected Pair process(T request, BiConsumer processor) { + protected void process(T request, BiConsumerWithEx processor) throws PfModelException { processor.accept(session, request); - - return Pair.of(Status.OK, new PdpGroupDeleteResponse()); } @Override diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java index 6ebad5ff..8c01e029 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployControllerV1.java @@ -37,6 +37,9 @@ import org.onap.policy.models.pdp.concepts.PdpSubGroup; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; import org.onap.policy.pap.main.rest.CommonPapRestServer; +/** + * Note: this tests failure cases; success cases are tested by tests in the "e2e" package. + */ public class TestPdpGroupDeployControllerV1 extends CommonPapRestServer { private static final String DEPLOY_GROUP_ENDPOINT = "pdps"; @@ -55,12 +58,12 @@ public class TestPdpGroupDeployControllerV1 extends CommonPapRestServer { Invocation.Builder invocationBuilder = sendRequest(DEPLOY_GROUP_ENDPOINT); Response rawresp = invocationBuilder.post(entgrp); PdpGroupDeployResponse resp = rawresp.readEntity(PdpGroupDeployResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), rawresp.getStatus()); assertNotNull(resp.getErrorDetails()); rawresp = invocationBuilder.post(entgrp); resp = rawresp.readEntity(PdpGroupDeployResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), rawresp.getStatus()); assertNotNull(resp.getErrorDetails()); // verify it fails when no authorization info is included @@ -74,12 +77,12 @@ public class TestPdpGroupDeployControllerV1 extends CommonPapRestServer { Invocation.Builder invocationBuilder = sendRequest(DEPLOY_POLICIES_ENDPOINT); Response rawresp = invocationBuilder.post(entgrp); PdpGroupDeployResponse resp = rawresp.readEntity(PdpGroupDeployResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertNotNull(resp.getErrorDetails()); rawresp = invocationBuilder.post(entgrp); resp = rawresp.readEntity(PdpGroupDeployResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertNotNull(resp.getErrorDetails()); // verify it fails when no authorization info is included diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java index 12c2adad..406345c6 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestPdpGroupDeployProvider.java @@ -20,15 +20,14 @@ package org.onap.policy.pap.main.rest.depundep; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.onap.policy.pap.main.rest.depundep.ProviderBase.DB_ERROR_MSG; import java.util.ArrayList; import java.util.Arrays; @@ -37,14 +36,13 @@ import java.util.List; import java.util.TreeMap; import java.util.stream.Collectors; import javax.ws.rs.core.Response.Status; -import org.apache.commons.lang3.tuple.Pair; import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.onap.policy.common.utils.services.Registry; import org.onap.policy.models.base.PfModelException; +import org.onap.policy.models.base.PfModelRuntimeException; import org.onap.policy.models.pap.concepts.PdpDeployPolicies; -import org.onap.policy.models.pap.concepts.PdpGroupDeployResponse; import org.onap.policy.models.pdp.concepts.PdpGroup; import org.onap.policy.models.pdp.concepts.PdpGroups; import org.onap.policy.models.pdp.concepts.PdpSubGroup; @@ -56,7 +54,6 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeIdentifi public class TestPdpGroupDeployProvider extends ProviderSuper { private static final String EXPECTED_EXCEPTION = "expected exception"; - private static final Object REQUEST_FAILED_MSG = "request failed"; private static final String POLICY2_NAME = "policyB"; private static final String POLICY1_VERSION = "1.2.3"; @@ -92,9 +89,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { @Test public void testCreateOrUpdateGroups() throws Exception { - Pair pair = prov.createOrUpdateGroups(loadPdpGroups("emptyGroups.json")); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.createOrUpdateGroups(loadPdpGroups("emptyGroups.json")); // no groups, so no action should have been taken assertNoGroupAction(); @@ -102,9 +97,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { @Test public void testCreateOrUpdateGroups_InvalidRequest() throws Exception { - Pair pair = prov.createOrUpdateGroups(new PdpGroups()); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("is null")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(new PdpGroups())).isInstanceOf(PfModelException.class) + .hasMessageContaining("is null"); assertNoGroupAction(); } @@ -114,9 +108,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(PdpState.TERMINATED); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("pdpGroupState")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("pdpGroupState"); assertNoGroupAction(); } @@ -127,7 +120,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroup group = groups.getGroups().get(0); group.setPdpGroupState(PdpState.PASSIVE); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); // should not have updated the state assertEquals(PdpState.PASSIVE, group.getPdpGroupState()); @@ -140,9 +133,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(PdpState.TERMINATED); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("pdpGroupState")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("pdpGroupState"); assertNoGroupAction(); } @@ -154,32 +146,31 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { // policy won't match supported type groups.getGroups().get(0).getPdpSubgroups().get(0).getSupportedPolicyTypes().get(0).setVersion("99.99.99"); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("supported policy")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("supported policy"); assertNoGroupAction(); } @Test - public void testValidateGroupOnly_NullState() { + public void testValidateGroupOnly_NullState() throws PfModelException { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(null); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); } @Test - public void testValidateGroupOnly_Active() { + public void testValidateGroupOnly_Active() throws PfModelException { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(PdpState.ACTIVE); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); } @Test - public void testValidateGroupOnly_Passive() { + public void testValidateGroupOnly_Passive() throws PfModelException { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(PdpState.PASSIVE); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); } @Test @@ -187,9 +178,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroups groups = loadPdpGroups("createGroups.json"); groups.getGroups().get(0).setPdpGroupState(PdpState.TERMINATED); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("pdpGroupState")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("pdpGroupState"); } @Test @@ -200,7 +190,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroup group = new PdpGroup(groups.getGroups().get(0)); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertNoGroupAction(); } @@ -214,9 +204,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("properties")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("properties"); assertNoGroupAction(); } @@ -229,7 +218,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { group.setDescription("old description"); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertGroupUpdateOnly(group); @@ -243,7 +232,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); PdpGroup newgrp = groups.getGroups().get(0); assertEquals(newgrp.toString(), group.toString()); @@ -260,7 +249,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { // something different in this subgroup group.getPdpSubgroups().get(0).setDesiredInstanceCount(10); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -281,7 +270,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")) .thenReturn(loadPolicies("createGroupNewPolicy.json")); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); Collections.sort(newgrp.getPdpSubgroups().get(0).getPolicies()); Collections.sort(group.getPdpSubgroups().get(0).getPolicies()); @@ -299,7 +288,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroup group = new PdpGroup(newgrp); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertNoGroupAction(); } @@ -313,7 +302,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { group.setDescription(null); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -328,7 +317,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { newgrp.setDescription(null); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -343,7 +332,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { newgrp.setDescription(group.getDescription() + "-changed"); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -355,7 +344,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); PdpGroup newgrp = groups.getGroups().get(0); @@ -367,6 +356,30 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { assertGroupUpdateOnly(group); } + @Test + public void testAddSubGroup_ValidationPolicyNotFound() throws Exception { + PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); + when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); + + PfModelException exc = new PfModelException(Status.NOT_FOUND, EXPECTED_EXCEPTION); + when(dao.getFilteredPolicyList(any())).thenThrow(exc); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).hasMessageContaining("unknown policy"); + } + + @Test + public void testAddSubGroup_ValidationPolicyDaoEx() throws Exception { + PdpGroups groups = loadPdpGroups("createGroupsNewSub.json"); + PdpGroup group = loadPdpGroups("createGroups.json").getGroups().get(0); + when(dao.getPdpGroups(group.getName())).thenReturn(Arrays.asList(group)); + + PfModelException exc = new PfModelException(Status.CONFLICT, EXPECTED_EXCEPTION); + when(dao.getFilteredPolicyList(any())).thenThrow(exc); + + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isSameAs(exc); + } + @Test public void testUpdateSubGroup_Invalid() throws Exception { PdpGroups groups = loadPdpGroups("createGroups.json"); @@ -377,9 +390,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { // change properties newgrp.getPdpSubgroups().get(0).setProperties(new TreeMap<>()); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("properties")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("properties"); assertNoGroupAction(); } @@ -393,7 +405,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { newgrp.getPdpSubgroups().get(0).getSupportedPolicyTypes().add(new ToscaPolicyTypeIdentifier("typeX", "9.8.7")); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -408,7 +420,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { newgrp.getPdpSubgroups().get(0).setDesiredInstanceCount(20); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); assertEquals(newgrp.toString(), group.toString()); assertGroupUpdateOnly(group); @@ -427,7 +439,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { when(dao.getFilteredPolicyList(any())).thenReturn(loadPolicies("daoPolicyList.json")) .thenReturn(loadPolicies("createGroupNewPolicy.json")); - assertEquals(Status.OK, prov.createOrUpdateGroups(groups).getLeft()); + prov.createOrUpdateGroups(groups); Collections.sort(newgrp.getPdpSubgroups().get(0).getPolicies()); Collections.sort(group.getPdpSubgroups().get(0).getPolicies()); @@ -447,52 +459,52 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { newgrp.setProperties(new TreeMap<>()); - Pair pair = prov.createOrUpdateGroups(groups); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertTrue(pair.getRight().getErrorDetails().contains("properties")); + assertThatThrownBy(() -> prov.createOrUpdateGroups(groups)).isInstanceOf(PfModelException.class) + .hasMessageContaining("properties"); assertNoGroupAction(); } @Test - public void testDeployPolicies() { - Pair pair = prov.deployPolicies(loadEmptyRequest()); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + public void testDeployPolicies() throws PfModelException { + prov.deployPolicies(loadEmptyRequest()); } @Test public void testDeploySimplePolicies() throws Exception { - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.deployPolicies(loadEmptyRequest()); } @Test public void testDeploySimplePolicies_DaoEx() throws Exception { - when(dao.getFilteredPdpGroups(any())).thenThrow(new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION)); + PfModelException exc = new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); + when(dao.getFilteredPdpGroups(any())).thenThrow(exc); + + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isSameAs(exc); + } + + @Test + public void testDeploySimplePolicies_DaoPfRtEx() throws Exception { + PfModelRuntimeException exc = new PfModelRuntimeException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); + when(dao.getFilteredPdpGroups(any())).thenThrow(exc); - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals(DB_ERROR_MSG, pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isSameAs(exc); } @Test public void testDeploySimplePolicies_RuntimeEx() throws Exception { - when(dao.getFilteredPolicyList(any())).thenThrow(new RuntimeException(EXPECTED_EXCEPTION)); + RuntimeException exc = new RuntimeException(EXPECTED_EXCEPTION); + when(dao.getFilteredPolicyList(any())).thenThrow(exc); - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals(REQUEST_FAILED_MSG, pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isInstanceOf(PfModelException.class).hasCause(exc); } @Test public void testDeploySimplePolicies_NoGroups() throws Exception { when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("emptyGroups.json")); - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals("policy not supported by any PDP group: policyA 1.2.3", pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isInstanceOf(PfModelException.class) + .hasMessage("policy not supported by any PDP group: policyA 1.2.3"); } @Test @@ -511,9 +523,7 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("upgradeGroupDao.json")); - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.deployPolicies(loadRequest()); assertGroup(getGroupUpdates(), GROUP1_NAME); @@ -528,10 +538,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper { // subgroup has no PDPs when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("upgradeGroup_NoPdpsDao.json")); - Pair pair = prov.deployPolicies(loadRequest()); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals("group " + GROUP1_NAME + " subgroup " + PDP1_TYPE + " has no active PDPs", - pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.deployPolicies(loadRequest())).isInstanceOf(PfModelRuntimeException.class) + .hasMessage("group " + GROUP1_NAME + " subgroup " + PDP1_TYPE + " has no active PDPs"); verify(dao, never()).createPdpGroups(any()); verify(dao, never()).updatePdpGroups(any()); diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java index bec93e2d..e8ddd821 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestProviderBase.java @@ -22,14 +22,12 @@ package org.onap.policy.pap.main.rest.depundep; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.onap.policy.pap.main.rest.depundep.ProviderBase.DB_ERROR_MSG; import java.util.Arrays; import java.util.Collections; @@ -38,25 +36,22 @@ import java.util.List; import java.util.Queue; import java.util.function.BiFunction; import javax.ws.rs.core.Response.Status; -import org.apache.commons.lang3.tuple.Pair; import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.onap.policy.common.utils.services.Registry; import org.onap.policy.models.base.PfModelException; +import org.onap.policy.models.base.PfModelRuntimeException; import org.onap.policy.models.pap.concepts.PdpDeployPolicies; -import org.onap.policy.models.pap.concepts.SimpleResponse; import org.onap.policy.models.pdp.concepts.PdpGroup; import org.onap.policy.models.pdp.concepts.PdpSubGroup; import org.onap.policy.models.pdp.concepts.PdpUpdate; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; -import org.onap.policy.pap.main.PolicyPapRuntimeException; import org.powermock.reflect.Whitebox; public class TestProviderBase extends ProviderSuper { private static final String EXPECTED_EXCEPTION = "expected exception"; - private static final Object REQUEST_FAILED_MSG = "request failed"; private static final String POLICY1_NAME = "policyA"; private static final String POLICY1_VERSION = "1.2.3"; @@ -103,9 +98,7 @@ public class TestProviderBase extends ProviderSuper { @Test public void testProcess() throws Exception { - Pair pair = prov.process(loadRequest(), this::handle); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.process(loadRequest(), this::handle); assertGroup(getGroupUpdates(), GROUP1_NAME); @@ -114,29 +107,27 @@ public class TestProviderBase extends ProviderSuper { @Test public void testProcess_CreateEx() throws Exception { - when(daofact.create()).thenThrow(new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION)); + PfModelException ex = new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); + when(daofact.create()).thenThrow(ex); - Pair pair = prov.process(loadEmptyRequest(), this::handle); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals(DB_ERROR_MSG, pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.process(loadEmptyRequest(), this::handle)).isSameAs(ex); } @Test - public void testProcess_PapEx() throws Exception { - when(daofact.create()).thenThrow(new PolicyPapRuntimeException(EXPECTED_EXCEPTION)); + public void testProcess_PfRtEx() throws Exception { + PfModelRuntimeException ex = new PfModelRuntimeException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); + when(daofact.create()).thenThrow(ex); - Pair pair = prov.process(loadEmptyRequest(), this::handle); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals(EXPECTED_EXCEPTION, pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.process(loadEmptyRequest(), this::handle)).isSameAs(ex); } @Test public void testProcess_RuntimeEx() throws Exception { - when(daofact.create()).thenThrow(new RuntimeException(EXPECTED_EXCEPTION)); + RuntimeException ex = new RuntimeException(EXPECTED_EXCEPTION); + when(daofact.create()).thenThrow(ex); - Pair pair = prov.process(loadEmptyRequest(), this::handle); - assertEquals(Status.INTERNAL_SERVER_ERROR, pair.getLeft()); - assertEquals(REQUEST_FAILED_MSG, pair.getRight().getErrorDetails()); + assertThatThrownBy(() -> prov.process(loadEmptyRequest(), this::handle)).isInstanceOf(PfModelException.class) + .hasMessage("request failed").hasCause(ex); } @Test @@ -145,18 +136,18 @@ public class TestProviderBase extends ProviderSuper { SessionData session = new SessionData(dao); ToscaPolicyIdentifierOptVersion ident = new ToscaPolicyIdentifierOptVersion(POLICY1_NAME, POLICY1_VERSION); - assertThatThrownBy(() -> prov.processPolicy(session, ident)).isInstanceOf(PolicyPapRuntimeException.class) + assertThatThrownBy(() -> prov.processPolicy(session, ident)).isInstanceOf(PfModelException.class) .hasMessage("policy not supported by any PDP group: policyA 1.2.3"); } @Test public void testGetPolicy() throws Exception { - Pair pair = prov.process(loadRequest(), this::handle); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + PfModelException exc = new PfModelException(Status.CONFLICT, EXPECTED_EXCEPTION); + when(dao.getFilteredPolicyList(any())).thenThrow(exc); - verify(dao).getFilteredPolicyList(any()); + assertThatThrownBy(() -> prov.process(loadRequest(), this::handle)).isInstanceOf(PfModelRuntimeException.class) + .hasCause(exc); } @Test @@ -164,9 +155,7 @@ public class TestProviderBase extends ProviderSuper { when(dao.getFilteredPdpGroups(any())).thenReturn(loadGroups("getGroupDao.json")) .thenReturn(loadGroups("groups.json")); - Pair pair = prov.process(loadRequest(), this::handle); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.process(loadRequest(), this::handle); assertGroup(getGroupUpdates(), GROUP1_NAME); } @@ -190,9 +179,7 @@ public class TestProviderBase extends ProviderSuper { prov.clear(); prov.add(false, true, false, true); - Pair pair = prov.process(loadRequest(), this::handle); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.process(loadRequest(), this::handle); assertGroup(getGroupUpdates(), GROUP1_NAME); @@ -242,15 +229,12 @@ public class TestProviderBase extends ProviderSuper { // multiple policies in the request PdpDeployPolicies request = loadFile("updateGroupReqMultiple.json", PdpDeployPolicies.class); - Pair pair = prov.process(request, (data, deploy) -> { + prov.process(request, (data, deploy) -> { for (ToscaPolicyIdentifierOptVersion policy : deploy.getPolicies()) { handle(data, policy); } }); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); - // verify updates List changes = getGroupUpdates(); assertGroup(changes, GROUP1_NAME); @@ -267,9 +251,7 @@ public class TestProviderBase extends ProviderSuper { prov.clear(); prov.add(false); - Pair pair = prov.process(loadRequest(), this::handle); - assertEquals(Status.OK, pair.getLeft()); - assertNull(pair.getRight().getErrorDetails()); + prov.process(loadRequest(), this::handle); verify(dao, never()).createPdpGroups(any()); verify(dao, never()).updatePdpGroups(any()); @@ -320,18 +302,14 @@ public class TestProviderBase extends ProviderSuper { * * @param data session data * @param request request to be handled + * @throws PfModelException if an error occurred */ - private void handle(SessionData data, ToscaPolicyIdentifierOptVersion request) { - try { - prov.processPolicy(data, request); - - } catch (PfModelException e) { - throw new PolicyPapRuntimeException(e); - } + private void handle(SessionData data, ToscaPolicyIdentifierOptVersion request) throws PfModelException { + prov.processPolicy(data, request); } - private static class MyProvider extends ProviderBase { + private static class MyProvider extends ProviderBase { /** * Used to determine whether or not to make an update when * {@link #makeUpdater(ToscaPolicy)} is called. The updater function removes an @@ -356,11 +334,6 @@ public class TestProviderBase extends ProviderSuper { shouldUpdate.addAll(Arrays.asList(update)); } - @Override - public MyResponse makeResponse(String errorMsg) { - return new MyResponse(errorMsg); - } - @Override protected BiFunction makeUpdater(ToscaPolicy policy) { return (group, subgroup) -> { @@ -376,10 +349,4 @@ public class TestProviderBase extends ProviderSuper { }; } } - - private static class MyResponse extends SimpleResponse { - public MyResponse(String errorMsg) { - setErrorDetails(errorMsg); - } - } } diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java index e74f9f86..17acd5a2 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestSessionData.java @@ -52,7 +52,6 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyFilter; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyIdentifierOptVersion; import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicyTypeIdentifier; -import org.onap.policy.pap.main.PolicyPapRuntimeException; public class TestSessionData extends ProviderSuper { private static final String GROUP_NAME = "groupA"; @@ -157,7 +156,7 @@ public class TestSessionData extends ProviderSuper { PfModelException ex = new PfModelException(Status.INTERNAL_SERVER_ERROR, "expected exception"); when(dao.getFilteredPolicyList(any())).thenThrow(ex); - assertThatThrownBy(() -> session.getPolicy(ident)).hasMessage("cannot get policy: myPolicy 1.2.3").hasCause(ex); + assertThatThrownBy(() -> session.getPolicy(ident)).isSameAs(ex); } @Test @@ -370,8 +369,7 @@ public class TestSessionData extends ProviderSuper { PfModelException ex = new PfModelException(Status.BAD_REQUEST, EXPECTED_EXCEPTION); when(dao.getPdpGroups(GROUP_NAME)).thenThrow(ex); - assertThatThrownBy(() -> session.getGroup(GROUP_NAME)).isInstanceOf(PolicyPapRuntimeException.class) - .hasCause(ex); + assertThatThrownBy(() -> session.getGroup(GROUP_NAME)).isSameAs(ex); } @Test diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeleteTest.java b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeleteTest.java index 9ffe1035..463e8d6e 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeleteTest.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeleteTest.java @@ -86,7 +86,7 @@ public class PdpGroupDeleteTest extends End2EndBase { // repeat - should fail rawresp = invocationBuilder.delete(); resp = rawresp.readEntity(PdpGroupDeleteResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.NOT_FOUND.getStatusCode(), rawresp.getStatus()); assertEquals("group not found", resp.getErrorDetails()); } diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeployTest.java b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeployTest.java index 5279236a..69165d1f 100644 --- a/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeployTest.java +++ b/main/src/test/java/org/onap/policy/pap/main/rest/e2e/PdpGroupDeployTest.java @@ -124,7 +124,7 @@ public class PdpGroupDeployTest extends End2EndBase { groups.getGroups().get(0).setProperties(null); rawresp = invocationBuilder.post(entity); resp = rawresp.readEntity(PdpGroupDeployResponse.class); - assertEquals(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), rawresp.getStatus()); + assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), rawresp.getStatus()); assertTrue(resp.getErrorDetails().contains("cannot change properties")); } -- cgit 1.2.3-korg