Handle null policy lists

UpdateReq throws NPE if the policy list in the request or the
response is null.

Change-Id: I0d71d6ad4cc9b96315c3b3ba89af8e4d8f3f94e6
Issue-ID: POLICY-1669
Signed-off-by: Jim Hahn <jrh3@att.com>

diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
index 8dcb979..0d012c0 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/PdpModifyRequestMap.java
@@ -21,6 +21,7 @@
 package org.onap.policy.pap.main.comm;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -377,6 +378,7 @@ public class PdpModifyRequestMap {
                 // send an update, too
                 PdpUpdate update = new PdpUpdate();
                 update.setName(requests.getPdpName());
+                update.setPolicies(Collections.emptyList());
 
                 addRequest(update, change);
 

diff --git a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
index 9067131..8efdb7c 100644 (file)
--- a/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
+++ b/main/src/main/java/org/onap/policy/pap/main/comm/msgdata/UpdateReq.java
@@ -20,7 +20,9 @@
 
 package org.onap.policy.pap.main.comm.msgdata;
 
+import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.apache.commons.lang3.StringUtils;
@@ -72,9 +74,9 @@ public class UpdateReq extends RequestImpl {
         }
 
         // see if the policies match
-        Set<ToscaPolicyIdentifier> set1 = new HashSet<>(response.getPolicies());
-        Set<ToscaPolicyIdentifier> set2 = new HashSet<>(
-                        message.getPolicies().stream().map(ToscaPolicy::getIdentifier).collect(Collectors.toSet()));
+        Set<ToscaPolicyIdentifier> set1 = new HashSet<>(alwaysList(response.getPolicies()));
+        Set<ToscaPolicyIdentifier> set2 = new HashSet<>(alwaysList(message.getPolicies()).stream()
+                        .map(ToscaPolicy::getIdentifier).collect(Collectors.toSet()));
 
         if (!set1.equals(set2)) {
             return "policies do not match";
@@ -101,12 +103,22 @@ public class UpdateReq extends RequestImpl {
         }
 
         // see if the policies are the same
-        Set<ToscaPolicy> set1 = new HashSet<>(first.getPolicies());
-        Set<ToscaPolicy> set2 = new HashSet<>(second.getPolicies());
+        Set<ToscaPolicy> set1 = new HashSet<>(alwaysList(first.getPolicies()));
+        Set<ToscaPolicy> set2 = new HashSet<>(alwaysList(second.getPolicies()));
 
         return set1.equals(set2);
     }
 
+    /**
+     * Always get a list, even if the original is {@code null}.
+     *
+     * @param list the original list, or {@code null}
+     * @return the list, or an empty list if the original was {@code null}
+     */
+    private <T> List<T> alwaysList(List<T> list) {
+        return (list != null ? list : Collections.emptyList());
+    }
+
     @Override
     public int getPriority() {
         return 1;

diff --git a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
index 156e9c8..a798770 100644 (file)
--- a/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
+++ b/main/src/test/java/org/onap/policy/pap/main/comm/msgdata/UpdateReqTest.java
@@ -73,6 +73,11 @@ public class UpdateReqTest extends CommonRequestBase {
     @Test
     public void testCheckResponse() {
         assertNull(data.checkResponse(response));
+
+        // both policy lists null
+        update.setPolicies(null);
+        response.setPolicies(null);
+        assertNull(data.checkResponse(response));
     }
 
     @Test
@@ -113,6 +118,20 @@ public class UpdateReqTest extends CommonRequestBase {
         assertEquals("policies do not match", data.checkResponse(response));
     }
 
+    @Test
+    public void testUpdateReqCheckResponse_MismatchedPolicies_Null_NotNull() {
+        update.setPolicies(null);
+
+        assertEquals("policies do not match", data.checkResponse(response));
+    }
+
+    @Test
+    public void testUpdateReqCheckResponse_MismatchedPolicies_NotNull_Null() {
+        response.setPolicies(null);
+
+        assertEquals("policies do not match", data.checkResponse(response));
+    }
+
     @Test
     public void isSameContent() {
         PdpUpdate msg2 = new PdpUpdate(update);
@@ -121,6 +140,11 @@ public class UpdateReqTest extends CommonRequestBase {
 
         // different request type
         assertFalse(data.isSameContent(new StateChangeReq(reqParams, MY_REQ_NAME, new PdpStateChange())));
+
+        // both policy lists null
+        update.setPolicies(null);
+        msg2.setPolicies(null);
+        assertTrue(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
     }
 
     @Test
@@ -176,6 +200,23 @@ public class UpdateReqTest extends CommonRequestBase {
         assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
     }
 
+    @Test
+    public void isSameContent_DiffPolicies_NotNull_Null() {
+        PdpUpdate msg2 = new PdpUpdate(update);
+        msg2.setPolicies(null);
+
+        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+    }
+
+    @Test
+    public void isSameContent_DiffPolicies_Null_NotNull() {
+        PdpUpdate msg2 = new PdpUpdate(update);
+
+        update.setPolicies(null);
+
+        assertFalse(data.isSameContent(new UpdateReq(reqParams, MY_REQ_NAME, msg2)));
+    }
+
     @Test
     public void testGetPriority() {
         assertTrue(data.getPriority() > new StateChangeReq(reqParams, MY_REQ_NAME, new PdpStateChange()).getPriority());