Added support to return status and error if pdp-x failed to load policy

Issue-ID: POLICY-2175
Change-Id: I32d2fe78846f58d6e700100dd523732817f1f04d
Signed-off-by: Ali Hockla <ah999m@att.com>

9 files changed, 98 insertions, 43 deletions

diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java
index 0b9975f7..edd33c0e 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/XacmlApplicationServiceProvider.java
@@ -89,7 +89,7 @@ public interface XacmlApplicationServiceProvider {
      *
      * @param toscaPolicy object
      */
-    boolean          loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException;
+    void          loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException;
 
     /**
      * unloadPolicy a Tosca Policy.
diff --git a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
index 12135f4a..5aea3457 100644
--- a/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
+++ b/applications/common/src/main/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProvider.java
@@ -116,7 +116,7 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
     }
 
     @Override
-    public synchronized boolean loadPolicy(ToscaPolicy toscaPolicy) {
+    public synchronized void loadPolicy(ToscaPolicy toscaPolicy) throws XacmlApplicationException {
         try {
             //
             // Convert the policies first
@@ -165,10 +165,8 @@ public abstract class StdXacmlApplicationServiceProvider implements XacmlApplica
             //
             this.mapLoadedPolicies.put(toscaPolicy, refPath);
         } catch (IOException | ToscaPolicyConversionException e) {
-            LOGGER.error("Failed to loadPolicies {}", e);
-            return false;
+            throw new XacmlApplicationException("loadPolicy failed", e);
         }
-        return true;
     }
 
     @Override
diff --git a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java
index 6139a528..e7ea049a 100644
--- a/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java
+++ b/applications/common/src/test/java/org/onap/policy/pdp/xacml/application/common/std/StdXacmlApplicationServiceProviderTest.java
@@ -190,10 +190,10 @@ public class StdXacmlApplicationServiceProviderTest {
     }
 
     @Test
-    public void testLoadPolicy_ConversionError() throws ToscaPolicyConversionException {
+    public void testLoadPolicy_ConversionError() throws XacmlApplicationException, ToscaPolicyConversionException {
         when(trans.convertPolicy(policy)).thenReturn(null);
 
-        assertFalse(prov.loadPolicy(policy));
+        assertThatThrownBy(() -> prov.loadPolicy(policy)).isInstanceOf(XacmlApplicationException.class);
     }
 
     @Test
@@ -203,7 +203,8 @@ public class StdXacmlApplicationServiceProviderTest {
 
         final Set<String> set = XACMLProperties.getRootPolicyIDs(prov.getProperties());
 
-        assertTrue(prov.loadPolicy(policy));
+        // Load policy
+        prov.loadPolicy(policy);
 
         // policy file should have been created
         File policyFile = new File(TEMP_DIR, "my-name_1.2.3.xml");
diff --git a/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java b/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java
index 3d96b4bd..03b0e9e3 100644
--- a/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java
+++ b/main/src/main/java/org/onap/policy/pdpx/main/XacmlState.java
@@ -21,6 +21,7 @@
 package org.onap.policy.pdpx.main;
 
 import java.util.Collections;
+import org.apache.commons.lang3.StringUtils;
 import org.onap.policy.common.utils.network.NetworkUtil;
 import org.onap.policy.models.pdp.concepts.PdpMessage;
 import org.onap.policy.models.pdp.concepts.PdpResponseDetails;
@@ -52,7 +53,6 @@ public class XacmlState {
      */
     private final PdpStatus status;
 
-
     /**
      * Constructs the object, initializing the state.
      */
@@ -105,7 +105,7 @@ public class XacmlState {
          * within a group/subgroup.
          */
 
-        PdpStatus status2 = makeResponse(message);
+        PdpStatus status2 = makeResponse(message, "");
 
         // start/stop rest controller based on state change
         handleXacmlRestController();
@@ -123,12 +123,12 @@ public class XacmlState {
      * @param message message from which to update the internal state
      * @return a response to the message
      */
-    public PdpStatus updateInternalState(PdpUpdate message) {
+    public PdpStatus updateInternalState(PdpUpdate message, String errMessage) {
         status.setPdpGroup(message.getPdpGroup());
         status.setPdpSubgroup(message.getPdpSubgroup());
         status.setPolicies(appManager.getToscaPolicyIdentifiers());
 
-        return makeResponse(message);
+        return makeResponse(message, errMessage);
     }
 
     /**
@@ -145,11 +145,18 @@ public class XacmlState {
      * Makes a response to the given message, based on the current state.
      *
      * @param message message for which the response should be made
+     * @param errMessage the error message to be sent to PAP
      * @return a new response
      */
-    private PdpStatus makeResponse(PdpMessage message) {
+    private PdpStatus makeResponse(PdpMessage message, String errMessage) {
         PdpResponseDetails resp = new PdpResponseDetails();
-        resp.setResponseStatus(PdpResponseStatus.SUCCESS);
+
+        if (StringUtils.isBlank(errMessage)) {
+            resp.setResponseStatus(PdpResponseStatus.SUCCESS);
+        } else {
+            resp.setResponseStatus(PdpResponseStatus.FAIL);
+            resp.setResponseMessage(errMessage);
+        }
         resp.setResponseTo(message.getRequestId());
 
         PdpStatus status2 = new PdpStatus(status);
diff --git a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java
index 686a8edc..a2f139d6 100644
--- a/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java
+++ b/main/src/main/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisher.java
@@ -20,6 +20,7 @@
 
 package org.onap.policy.pdpx.main.comm;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
@@ -27,6 +28,8 @@ import org.onap.policy.common.endpoints.event.comm.client.TopicSinkClient;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
+import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException;
+import org.onap.policy.pdp.xacml.application.common.XacmlPolicyUtils;
 import org.onap.policy.pdpx.main.XacmlState;
 import org.onap.policy.pdpx.main.rest.XacmlPdpApplicationManager;
 import org.onap.policy.pdpx.main.rest.XacmlPdpStatisticsManager;
@@ -73,13 +76,24 @@ public class XacmlPdpUpdatePublisher {
             }
         }
 
+        StringBuilder errorMessage = new StringBuilder();
         // Deploy a policy
         // if deployed policies do not contain the incoming policy load it
         for (ToscaPolicy policy : incomingPolicies) {
             if (!deployedPolicies.contains(policy)) {
-                appManager.loadDeployedPolicy(policy);
+                try {
+                    appManager.loadDeployedPolicy(policy);
+                } catch (XacmlApplicationException e) {
+                    // Failed to load policy, return error(s) to PAP
+                    LOGGER.error("Failed to load policy: {}", policy, e);
+                    errorMessage.append("Failed to load policy: " + policy + ": "
+                            + e.getMessage() + XacmlPolicyUtils.LINE_SEPARATOR);
+                }
             }
         }
+        // Return current deployed policies
+        message.setPolicies(new ArrayList<ToscaPolicy>(appManager.getToscaPolicies().keySet()));
+        LOGGER.debug("Returning current deployed policies: {} ", message.getPolicies());
 
         // update the policy count statistic
         XacmlPdpStatisticsManager stats = XacmlPdpStatisticsManager.getCurrent();
@@ -87,7 +101,7 @@ public class XacmlPdpUpdatePublisher {
             stats.setTotalPolicyCount(appManager.getPolicyCount());
         }
 
-        sendPdpUpdate(state.updateInternalState(message));
+        sendPdpUpdate(state.updateInternalState(message, errorMessage.toString()));
     }
 
     private void sendPdpUpdate(PdpStatus status) {
diff --git a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java
index 37132a8f..2f054bd1 100644
--- a/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java
+++ b/main/src/main/java/org/onap/policy/pdpx/main/rest/XacmlPdpApplicationManager.java
@@ -169,31 +169,27 @@ public class XacmlPdpApplicationManager {
     }
 
     /**
-     * Finds the appropriate application and loads the policy.
+     * Finds the appropriate application and loads the policy, throws an exception if it fails.
      *
      * @param policy Incoming policy
+     * @throws XacmlApplicationException if loadPolicy fails
      */
-    public void loadDeployedPolicy(ToscaPolicy policy) {
+    public void loadDeployedPolicy(ToscaPolicy policy) throws XacmlApplicationException {
 
         for (XacmlApplicationServiceProvider application : applicationLoader) {
-            try {
-                //
-                // There should be only one application per policytype. We can
-                // put more logic surrounding enforcement of that later. For now,
-                // just use the first one found.
-                //
-                if (application.canSupportPolicyType(policy.getTypeIdentifier())) {
-                    if (application.loadPolicy(policy)) {
-                        if (LOGGER.isInfoEnabled()) {
-                            LOGGER.info("Loaded ToscaPolicy {} into application {}", policy.getMetadata(),
-                                application.applicationName());
-                        }
-                        mapLoadedPolicies.put(policy, application);
-                    }
-                    return;
+            //
+            // There should be only one application per policytype. We can
+            // put more logic surrounding enforcement of that later. For now,
+            // just use the first one found.
+            //
+            if (application.canSupportPolicyType(policy.getTypeIdentifier())) {
+                application.loadPolicy(policy);
+                mapLoadedPolicies.put(policy, application);
+                if (LOGGER.isInfoEnabled()) {
+                    LOGGER.info("Loaded ToscaPolicy {} into application {}", policy.getMetadata(),
+                            application.applicationName());
                 }
-            } catch (XacmlApplicationException e) {
-                LOGGER.error("Failed to load the Tosca Policy", e);
+                return;
             }
         }
     }
diff --git a/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java b/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java
index eef1f1be..12d832ac 100644
--- a/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java
+++ b/main/src/test/java/org/onap/policy/pdpx/main/XacmlStateTest.java
@@ -23,6 +23,7 @@ package org.onap.policy.pdpx.main;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -154,17 +155,22 @@ public class XacmlStateTest {
         req.setPdpGroup(GROUP);
         req.setPdpSubgroup(SUBGROUP);
 
-        PdpStatus status = state.updateInternalState(req);
+        PdpStatus status = state.updateInternalState(req, "");
 
         PdpResponseDetails resp = status.getResponse();
         assertNotNull(resp);
         assertEquals(req.getRequestId(), resp.getResponseTo());
         assertEquals(PdpResponseStatus.SUCCESS, resp.getResponseStatus());
+        assertNull(resp.getResponseMessage());
 
         // ensure info was saved
         status = state.genHeartbeat();
         assertEquals(GROUP, status.getPdpGroup());
         assertEquals(SUBGROUP, status.getPdpSubgroup());
+
+        status = state.updateInternalState(req, "Failed to load policy: failLoadPolicy1: null");
+        assertEquals(status.getResponse().getResponseMessage(), "Failed to load policy: failLoadPolicy1: null");
+        assertEquals(status.getResponse().getResponseStatus(), PdpResponseStatus.FAIL);
     }
 
     @Test
diff --git a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java
index 31bec51e..c8c6a816 100644
--- a/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java
+++ b/main/src/test/java/org/onap/policy/pdpx/main/comm/XacmlPdpUpdatePublisherTest.java
@@ -22,6 +22,8 @@ package org.onap.policy.pdpx.main.comm;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.startsWith;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -38,6 +40,7 @@ import org.onap.policy.common.endpoints.event.comm.client.TopicSinkClient;
 import org.onap.policy.models.pdp.concepts.PdpStatus;
 import org.onap.policy.models.pdp.concepts.PdpUpdate;
 import org.onap.policy.models.tosca.authorative.concepts.ToscaPolicy;
+import org.onap.policy.pdp.xacml.application.common.XacmlApplicationException;
 import org.onap.policy.pdp.xacml.application.common.XacmlApplicationServiceProvider;
 import org.onap.policy.pdpx.main.XacmlState;
 import org.onap.policy.pdpx.main.rest.XacmlPdpApplicationManager;
@@ -82,8 +85,17 @@ public class XacmlPdpUpdatePublisherTest {
     private ToscaPolicy added2;
 
     @Mock
+    private ToscaPolicy failPolicy1;
+
+    @Mock
+    private ToscaPolicy failPolicy2;
+
+    @Mock
     private PdpUpdate update;
 
+    @Mock
+    private PdpUpdate failurePdpUpdate;
+
     private XacmlPdpUpdatePublisher publisher;
 
 
@@ -105,9 +117,12 @@ public class XacmlPdpUpdatePublisherTest {
         List<ToscaPolicy> updatePolicies = Arrays.asList(added1, deployed2, deployed3, added2);
         when(update.getPolicies()).thenReturn(updatePolicies);
 
+        List<ToscaPolicy> failureUpdatePolicies = Arrays.asList(added1, deployed2, deployed3, failPolicy1, failPolicy2);
+        when(failurePdpUpdate.getPolicies()).thenReturn(failureUpdatePolicies);
+
         when(appmgr.getPolicyCount()).thenReturn(NEW_COUNT);
 
-        when(state.updateInternalState(update)).thenReturn(status);
+        when(state.updateInternalState(any(), any())).thenReturn(status);
 
         when(client.send(any())).thenReturn(true);
 
@@ -115,7 +130,7 @@ public class XacmlPdpUpdatePublisherTest {
     }
 
     @Test
-    public void testHandlePdpUpdate() {
+    public void testHandlePdpUpdate() throws XacmlApplicationException {
         XacmlPdpStatisticsManager statmgr = new XacmlPdpStatisticsManager();
         XacmlPdpStatisticsManager.setCurrent(statmgr);
 
@@ -141,7 +156,25 @@ public class XacmlPdpUpdatePublisherTest {
     }
 
     @Test
-    public void testHandlePdpUpdate_NullPolicies() {
+    public void testHandlePdpUpdate_LoadPolicyFailed() throws XacmlApplicationException {
+        // Set loadPolicy to fail
+        doThrow(new XacmlApplicationException()).when(appmgr).loadDeployedPolicy(failPolicy1);
+        doThrow(new XacmlApplicationException()).when(appmgr).loadDeployedPolicy(failPolicy2);
+
+        publisher.handlePdpUpdate(failurePdpUpdate);
+
+        // two removed
+        verify(appmgr).removeUndeployedPolicy(deployed1);
+        verify(appmgr).removeUndeployedPolicy(deployed4);
+
+        verify(failurePdpUpdate).setPolicies(any());
+
+        verify(state).updateInternalState(any(), startsWith("Failed to load policy"));
+        verify(client).send(status);
+    }
+
+    @Test
+    public void testHandlePdpUpdate_NullPolicies() throws XacmlApplicationException {
         when(update.getPolicies()).thenReturn(null);
 
         publisher.handlePdpUpdate(update);
diff --git a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
index 70f9ebc0..cb12e0f6 100644
--- a/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
+++ b/xacml-test/src/main/java/org/onap/policy/pdp/xacml/xacmltest/TestUtils.java
@@ -25,7 +25,6 @@ package org.onap.policy.pdp.xacml.xacmltest;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
-
 import org.onap.policy.common.utils.coder.CoderException;
 import org.onap.policy.common.utils.coder.StandardYamlCoder;
 import org.onap.policy.common.utils.resources.ResourceUtils;
@@ -82,10 +81,11 @@ public class TestUtils {
         //
         for (Map<String, ToscaPolicy> policies : completedJtst.getToscaTopologyTemplate().getPolicies()) {
             for (ToscaPolicy policy : policies.values()) {
-                if (service.loadPolicy(policy)) {
+                try {
+                    service.loadPolicy(policy);
                     loadedPolicies.add(policy);
-                } else {
-                    LOGGER.error("Application failed to load policy");
+                } catch (XacmlApplicationException e) {
+                    LOGGER.error("Application failed to load policy", e);
                 }
             }
         }