Delay DB updates until all policies processed

If an exception is thrown in the middle of processing policies, it's
possible for the DB to reflect a partial deployment.  This change
queues the DB changes so that they're all made at once, after all
policies have been processed.
Extracted sort() into a common place.
Corrected some comments.
Simplified the list retrieval in the junits since all DB creates and
updates are now batched together into one operation each.

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

5 files changed, 302 insertions, 66 deletions

diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java
index 3a3673d5..a1e69565 100644
--- a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/ProviderSuper.java
@@ -29,6 +29,7 @@ import static org.mockito.Mockito.when;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import org.junit.Before;
 import org.mockito.ArgumentCaptor;
@@ -125,27 +126,25 @@ public class ProviderSuper {
     /**
      * Gets the input to the method.
      *
-     * @param count the number of times the method is expected to have been called.
      * @return the input that was passed to the dao.createPdpGroups() method
      * @throws Exception if an error occurred
      */
-    protected List<List<PdpGroup>> getGroupCreates(int count) throws Exception {
-        verify(dao, times(count)).createPdpGroups(createCaptor.capture());
+    protected List<PdpGroup> getGroupCreates() throws Exception {
+        verify(dao).createPdpGroups(createCaptor.capture());
 
-        return copyLists(createCaptor.getAllValues());
+        return copyList(createCaptor.getValue());
     }
 
     /**
      * Gets the input to the method.
      *
-     * @param count the number of times the method is expected to have been called
      * @return the input that was passed to the dao.updatePdpGroups() method
      * @throws Exception if an error occurred
      */
-    protected List<List<PdpGroup>> getGroupUpdates(int count) throws Exception {
-        verify(dao, times(count)).updatePdpGroups(updateCaptor.capture());
+    protected List<PdpGroup> getGroupUpdates() throws Exception {
+        verify(dao).updatePdpGroups(updateCaptor.capture());
 
-        return copyLists(updateCaptor.getAllValues());
+        return copyList(updateCaptor.getValue());
     }
 
     /**
@@ -163,19 +162,15 @@ public class ProviderSuper {
     }
 
     /**
-     * Makes a partly deep copy of the list.
+     * Copies a list and sorts it by group name.
      *
      * @param source source list to copy
      * @return a copy of the source list
      */
-    private List<List<PdpGroup>> copyLists(List<List<PdpGroup>> source) {
-        List<List<PdpGroup>> target = new ArrayList<>(source.size());
-
-        for (List<PdpGroup> lst : source) {
-            target.add(new ArrayList<>(lst));
-        }
-
-        return target;
+    private List<PdpGroup> copyList(List<PdpGroup> source) {
+        List<PdpGroup> newlst = new ArrayList<>(source);
+        Collections.sort(newlst, (left, right) -> left.getName().compareTo(right.getName()));
+        return newlst;
     }
 
     /**
diff --git a/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestGroupData.java b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestGroupData.java
new file mode 100644
index 00000000..a0d4989f
--- /dev/null
+++ b/main/src/test/java/org/onap/policy/pap/main/rest/depundep/TestGroupData.java
@@ -0,0 +1,97 @@
+/*
+ * ============LICENSE_START=======================================================
+ * ONAP PAP
+ * ================================================================================
+ * Copyright (C) 2019 AT&T Intellectual Property. All rights reserved.
+ * ================================================================================
+ * 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.
+ * ============LICENSE_END=========================================================
+ */
+
+package org.onap.policy.pap.main.rest.depundep;
+
+import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.onap.policy.common.utils.validation.Version;
+import org.onap.policy.models.pdp.concepts.PdpGroup;
+
+public class TestGroupData {
+    private static final String NEW_VERSION = "2.0.0";
+    private static final String NAME = "my-name";
+
+    private PdpGroup oldGroup;
+    private PdpGroup newGroup;
+    private GroupData data;
+    private Version version;
+
+    /**
+     * Sets up.
+     */
+    @Before
+    public void setUp() {
+        oldGroup = new PdpGroup();
+        oldGroup.setName(NAME);
+
+        newGroup = new PdpGroup(oldGroup);
+
+        version = new Version(1, 2, 3);
+
+        data = new GroupData(oldGroup);
+    }
+
+    @Test
+    public void test() {
+        assertFalse(data.isNew());
+        assertSame(oldGroup, data.getOldGroup());
+        assertSame(oldGroup, data.getCurrentGroup());
+
+        data.setLatestVersion(version);
+        data.setNewGroup(newGroup);
+
+        assertTrue(data.isNew());
+        assertSame(oldGroup, data.getOldGroup());
+        assertSame(newGroup, data.getCurrentGroup());
+        assertEquals(NEW_VERSION, data.getLatestVersion().toString());
+        assertEquals(NEW_VERSION, newGroup.getVersion());
+
+        // repeat
+        newGroup = new PdpGroup(oldGroup);
+        data.setNewGroup(newGroup);
+        assertSame(oldGroup, data.getOldGroup());
+        assertSame(newGroup, data.getCurrentGroup());
+        assertEquals(NEW_VERSION, data.getLatestVersion().toString());
+        assertEquals(NEW_VERSION, newGroup.getVersion());
+    }
+
+    @Test
+    public void testSetNewGroup_DifferentName() {
+        newGroup.setName("different-name");
+
+        data.setLatestVersion(version);
+        assertThatIllegalArgumentException().isThrownBy(() -> data.setNewGroup(newGroup))
+                        .withMessage("attempt to change group name from my-name to different-name");
+    }
+
+    @Test
+    public void testSetNewGroup_VersionNotSet() {
+        assertThatIllegalStateException().isThrownBy(() -> data.setNewGroup(newGroup))
+                        .withMessage("latestVersion not set for group: my-name");
+    }
+}
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 d3ffda84..018f117c 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
@@ -149,8 +149,8 @@ public class TestPdpGroupDeployProvider extends ProviderSuper {
         assertEquals(Status.OK, pair.getLeft());
         assertNull(pair.getRight().getErrorDetails());
 
-        assertGroup(getGroupUpdates(1).get(0), GROUP1_NAME, GROUP1_VERSION);
-        assertGroup(getGroupCreates(1).get(0), GROUP1_NAME, GROUP1_NEW_VERSION);
+        assertGroup(getGroupUpdates(), GROUP1_NAME, GROUP1_VERSION);
+        assertGroup(getGroupCreates(), GROUP1_NAME, GROUP1_NEW_VERSION);
 
         List<PdpUpdate> requests = getUpdateRequests(2);
         assertUpdate(requests, GROUP1_NAME, PDP2_TYPE, PDP2);
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 f64f77da..883b3d55 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
@@ -109,8 +109,8 @@ public class TestProviderBase extends ProviderSuper {
         assertEquals(Status.OK, pair.getLeft());
         assertNull(pair.getRight().getErrorDetails());
 
-        assertGroup(getGroupUpdates(1).get(0), GROUP1_NAME, GROUP1_VERSION);
-        assertGroup(getGroupCreates(1).get(0), GROUP1_NAME, GROUP1_NEW_VERSION);
+        assertGroup(getGroupUpdates(), GROUP1_NAME, GROUP1_VERSION);
+        assertGroup(getGroupCreates(), GROUP1_NAME, GROUP1_NEW_VERSION);
 
         assertUpdate(getUpdateRequests(1), GROUP1_NAME, PDP1_TYPE, PDP1);
     }
@@ -197,8 +197,8 @@ public class TestProviderBase extends ProviderSuper {
         assertEquals(Status.OK, pair.getLeft());
         assertNull(pair.getRight().getErrorDetails());
 
-        assertGroup(getGroupUpdates(1).get(0), GROUP1_NAME, GROUP1_VERSION);
-        assertGroup(getGroupCreates(1).get(0), GROUP1_NAME, GROUP1_NEW_VERSION);
+        assertGroup(getGroupUpdates(), GROUP1_NAME, GROUP1_VERSION);
+        assertGroup(getGroupCreates(), GROUP1_NAME, GROUP1_NEW_VERSION);
     }
 
     @Test
@@ -224,8 +224,8 @@ public class TestProviderBase extends ProviderSuper {
         assertEquals(Status.OK, pair.getLeft());
         assertNull(pair.getRight().getErrorDetails());
 
-        assertGroup(getGroupUpdates(1).get(0), GROUP1_NAME, GROUP1_VERSION);
-        assertGroup(getGroupCreates(1).get(0), GROUP1_NAME, GROUP1_NEW_VERSION);
+        assertGroup(getGroupUpdates(), GROUP1_NAME, GROUP1_VERSION);
+        assertGroup(getGroupCreates(), GROUP1_NAME, GROUP1_NEW_VERSION);
 
         List<PdpUpdate> requests = getUpdateRequests(2);
         assertUpdate(requests, GROUP1_NAME, PDP2_TYPE, PDP2);
@@ -282,15 +282,15 @@ public class TestProviderBase extends ProviderSuper {
         assertEquals(Status.OK, pair.getLeft());
         assertNull(pair.getRight().getErrorDetails());
 
-        List<List<PdpGroup>> creates = getGroupCreates(2);
-        assertGroup(creates.get(0), GROUP1_NAME, GROUP1_NEW_VERSION);
-        assertGroup(creates.get(1), GROUP2_NAME, "301.0.0");
+        // verify creates
+        List<PdpGroup> changes = getGroupCreates();
+        assertGroup(changes, GROUP1_NAME, GROUP1_NEW_VERSION);
+        assertGroup(changes, GROUP2_NAME, "301.0.0");
 
-        List<List<PdpGroup>> updates = getGroupUpdates(4);
-        assertGroup(updates.get(0), GROUP1_NAME, GROUP1_VERSION);
-        assertGroup(updates.get(1), GROUP2_NAME, "300.2.3");
-        assertGroup(updates.get(2), GROUP1_NAME, GROUP1_NEW_VERSION);
-        assertGroup(updates.get(3), GROUP1_NAME, GROUP1_NEW_VERSION);
+        // verify updates
+        changes = getGroupUpdates();
+        assertGroup(changes, GROUP1_NAME, GROUP1_VERSION);
+        assertGroup(changes, GROUP2_NAME, "300.2.3");
 
         List<PdpUpdate> requests = getUpdateRequests(3);
         assertUpdateIgnorePolicy(requests, GROUP1_NAME, PDP1_TYPE, PDP1);
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 6be5c8ac..1c2b8233 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
@@ -20,12 +20,14 @@
 
 package org.onap.policy.pap.main.rest.depundep;
 
+import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
 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.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -49,9 +51,10 @@ import org.onap.policy.pap.main.PolicyPapRuntimeException;
 
 public class TestSessionData extends ProviderSuper {
     private static final String GROUP_VERSION_PREFIX = "9.8.";
-    private static final String GROUP_NAME = "group";
+    private static final String GROUP_NAME = "groupA";
     private static final String GROUP_VERSION = GROUP_VERSION_PREFIX + "7";
-    private static final String GROUP_NAME2 = "group2";
+    private static final String GROUP_NEW_VERSION = "10.0.0";
+    private static final String GROUP_NAME2 = "groupB";
     private static final String GROUP_VERSION2 = GROUP_VERSION_PREFIX + "6";
     private static final String PDP1 = "pdp_1";
     private static final String PDP2 = "pdp_2";
@@ -60,9 +63,15 @@ public class TestSessionData extends ProviderSuper {
     private static final String POLICY_NAME = "myPolicy";
     private static final String POLICY_VERSION = POLICY_VERSION_PREFIX + "3";
     private static final String POLICY_VERSION2 = POLICY_VERSION_PREFIX + "4";
+    private static final String POLICY_TYPE = "myType";
+    private static final String POLICY_TYPE_VERSION = "10.20.30";
 
     private SessionData session;
     private ToscaPolicyIdentifier ident;
+    private ToscaPolicyTypeIdentifier type;
+    private ToscaPolicyTypeIdentifier type2;
+    private PdpGroup group1;
+    private PdpGroup group2;
 
     /**
      * Initializes mocks and a session.
@@ -74,6 +83,10 @@ public class TestSessionData extends ProviderSuper {
         super.setUp();
 
         ident = new ToscaPolicyIdentifier(POLICY_NAME, POLICY_VERSION);
+        type = new ToscaPolicyTypeIdentifier(POLICY_TYPE, POLICY_TYPE_VERSION);
+        type2 = new ToscaPolicyTypeIdentifier(POLICY_TYPE, POLICY_TYPE_VERSION + "0");
+        group1 = makeGroup(GROUP_NAME, GROUP_VERSION);
+        group2 = makeGroup(GROUP_NAME2, GROUP_VERSION2);
 
         session = new SessionData(dao);
     }
@@ -126,7 +139,7 @@ public class TestSessionData extends ProviderSuper {
     }
 
     @Test
-    public void testAddUpdate() {
+    public void testAddUpdate_testGetPdpUpdates() {
         // several different updates, but one duplicate
         PdpUpdate update1 = makeUpdate(PDP1);
         session.addUpdate(update1);
@@ -137,14 +150,14 @@ public class TestSessionData extends ProviderSuper {
         PdpUpdate update3 = makeUpdate(PDP3);
         session.addUpdate(update3);
 
-        List<PdpUpdate> lst = sort(session.getUpdates(), this::compare);
+        List<PdpUpdate> lst = sort(session.getPdpUpdates(), this::compare);
         assertEquals(Arrays.asList(update1, update2, update3).toString(), lst.toString());
 
         // overwrite one
         update2 = makeUpdate(PDP2);
         session.addUpdate(update2);
 
-        lst = sort(session.getUpdates(), this::compare);
+        lst = sort(session.getPdpUpdates(), this::compare);
         assertEquals(Arrays.asList(update1, update2, update3).toString(), lst.toString());
     }
 
@@ -164,8 +177,15 @@ public class TestSessionData extends ProviderSuper {
         when(dao.getFilteredPolicyList(any())).thenReturn(Arrays.asList(policy1));
 
         assertSame(policy1, session.getPolicyMaxVersion(POLICY_NAME));
+        assertSame(policy1, session.getPolicyMaxVersion(POLICY_NAME));
+        assertSame(policy1, session.getPolicyMaxVersion(POLICY_NAME));
+
+        // should have only invoked DAO once; used cache for other requests
+        verify(dao, times(1)).getFilteredPolicyList(any());
+    }
 
-        // no matching policies - should throw an exception
+    @Test
+    public void testGetPolicyMaxVersion_NotFound() throws Exception {
         when(dao.getFilteredPolicyList(any())).thenReturn(Collections.emptyList());
         assertThatThrownBy(() -> session.getPolicyMaxVersion(POLICY_NAME)).hasMessage("cannot find policy: myPolicy");
     }
@@ -174,24 +194,34 @@ public class TestSessionData extends ProviderSuper {
     public void testIsNewlyCreated_testCreatePdpGroup() throws Exception {
         assertFalse(session.isNewlyCreated(GROUP_NAME));
 
-        PdpGroup group1 = makeGroup(GROUP_NAME, GROUP_VERSION);
-        when(dao.createPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        // cause the group to be loaded into the cache
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        session.getActivePdpGroupsByPolicyType(type);
 
-        session.createPdpGroup(group1);
+        // not new yet
+        assertFalse(session.isNewlyCreated(GROUP_NAME));
 
+        // update it
+        session.setNewGroup(new PdpGroup(group1));
         assertTrue(session.isNewlyCreated(GROUP_NAME));
-        assertFalse(session.isNewlyCreated(GROUP_NAME2));
 
-        PdpGroup group2 = makeGroup(GROUP_NAME2, GROUP_VERSION2);
-        when(dao.createPdpGroups(any())).thenReturn(Arrays.asList(group2));
-        session.createPdpGroup(group2);
+        /*
+         * now try group2
+         */
+        assertFalse(session.isNewlyCreated(GROUP_NAME2));
 
-        List<List<PdpGroup>> creates = getGroupCreates(2);
-        assertEquals(group1, creates.get(0).get(0));
-        assertEquals(group2, creates.get(1).get(0));
+        // cause the group to be loaded into the cache
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group2));
+        session.getActivePdpGroupsByPolicyType(type2);
 
+        // not new yet
+        assertFalse(session.isNewlyCreated(GROUP_NAME2));
         assertTrue(session.isNewlyCreated(GROUP_NAME));
+
+        // update it
+        session.setNewGroup(new PdpGroup(group2));
         assertTrue(session.isNewlyCreated(GROUP_NAME2));
+        assertTrue(session.isNewlyCreated(GROUP_NAME));
     }
 
     private PdpGroup makeGroup(String name, String version) {
@@ -204,37 +234,151 @@ public class TestSessionData extends ProviderSuper {
     }
 
     @Test
-    public void testGetPdpGroupMaxVersion() throws Exception {
-        PdpGroup group = makeGroup(GROUP_NAME, GROUP_VERSION);
-        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group));
+    public void testSetNewGroup() throws Exception {
+        // force the groups into the cache
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1, group2));
+        session.getActivePdpGroupsByPolicyType(type);
+
+        /*
+         * try group 1
+         */
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        PdpGroup newgrp = new PdpGroup(group1);
+        session.setNewGroup(newgrp);
+        assertEquals(GROUP_NEW_VERSION, newgrp.getVersion().toString());
+
+        // repeat - version should be unchanged
+        newgrp = new PdpGroup(group1);
+        session.setNewGroup(newgrp);
+        assertEquals(GROUP_NEW_VERSION, newgrp.getVersion().toString());
+
+        /*
+         * try group 2
+         */
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group2));
+        newgrp = new PdpGroup(group2);
+        session.setNewGroup(newgrp);
+        assertEquals(GROUP_NEW_VERSION, newgrp.getVersion().toString());
+
+        // repeat - version should be unchanged
+        newgrp = new PdpGroup(group2);
+        session.setNewGroup(newgrp);
+        assertEquals(GROUP_NEW_VERSION, newgrp.getVersion().toString());
+
+        // should have queried the DB once by type and twice by for latest version
+        verify(dao, times(3)).getFilteredPdpGroups(any());
+    }
+
+    @Test
+    public void testSetNewGroup_NotInCache() throws Exception {
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
 
-        assertEquals(group, session.getPdpGroupMaxVersion(GROUP_NAME));
+        assertThatIllegalStateException().isThrownBy(() -> session.setNewGroup(new PdpGroup(group1)))
+                        .withMessage("group not cached: groupA");
+    }
 
-        // try empty list
+    @Test
+    public void testSetNewGroup_NotFound() throws Exception {
+        // force the group into the cache
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        session.getActivePdpGroupsByPolicyType(type);
+
+        // query for latest version will return an empty list
         when(dao.getFilteredPdpGroups(any())).thenReturn(Collections.emptyList());
-        assertThatThrownBy(() -> session.getPdpGroupMaxVersion(GROUP_NAME))
-                        .isInstanceOf(PolicyPapRuntimeException.class).hasMessage("cannot find group: group");
+
+        assertThatThrownBy(() -> session.setNewGroup(new PdpGroup(group1)))
+                        .isInstanceOf(PolicyPapRuntimeException.class).hasMessage("cannot find group: groupA");
+    }
+
+    @Test
+    public void testSetNewGroup_InvalidVersion() throws Exception {
+        // force the groups into the cache
+        group1.setVersion("invalid version");
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        session.getActivePdpGroupsByPolicyType(type);
+
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        PdpGroup newgrp = new PdpGroup(group1);
+        session.setNewGroup(newgrp);
+        assertEquals("1.0.0", newgrp.getVersion().toString());
     }
 
     @Test
     public void testGetActivePdpGroupsByPolicyType() throws Exception {
-        List<PdpGroup> groups =
-                        Arrays.asList(makeGroup(GROUP_NAME, GROUP_VERSION), makeGroup(GROUP_NAME2, GROUP_VERSION2));
+        List<PdpGroup> groups = Arrays.asList(group1, group2);
         when(dao.getFilteredPdpGroups(any())).thenReturn(groups);
 
-        assertEquals(groups, session
-                        .getActivePdpGroupsByPolicyType(new ToscaPolicyTypeIdentifier(POLICY_NAME, POLICY_VERSION)));
+        // repeat
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type));
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type));
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type));
+
+        // only invoked once - should have used the cache for the rest
+        verify(dao, times(1)).getFilteredPdpGroups(any());
     }
 
     @Test
-    public void testUpdatePdpGroup() throws Exception {
-        PdpGroup group = makeGroup(GROUP_NAME, GROUP_VERSION);
-        when(dao.updatePdpGroups(any())).thenReturn(Arrays.asList(group));
+    public void testAddGroup() throws Exception {
+        List<PdpGroup> groups = Arrays.asList(group1, group2);
+        when(dao.getFilteredPdpGroups(any())).thenReturn(groups);
 
-        session.updatePdpGroup(group);
+        // query by each type
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type));
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type2));
 
-        List<PdpGroup> updates = getGroupUpdates(1).get(0);
-        assertEquals(group, updates.get(0));
+        // invoked once for each type
+        verify(dao, times(2)).getFilteredPdpGroups(any());
+
+        // repeat - should be no more invocations
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type));
+        assertEquals(groups, session.getActivePdpGroupsByPolicyType(type2));
+        verify(dao, times(2)).getFilteredPdpGroups(any());
+    }
+
+    @Test
+    public void testUpdateDb() throws Exception {
+        // force the groups into the cache
+        PdpGroup group3 = makeGroup("groupC", GROUP_VERSION2);
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1, group2, group3));
+        session.getActivePdpGroupsByPolicyType(type);
+
+        // update group 1
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1));
+        PdpGroup newgrp1 = new PdpGroup(group1);
+        session.setNewGroup(newgrp1);
+
+        // another update
+        newgrp1 = new PdpGroup(newgrp1);
+        session.setNewGroup(newgrp1);
+
+        // update group 3
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group3));
+        PdpGroup newgrp3 = new PdpGroup(group3);
+        session.setNewGroup(newgrp3);
+
+        // push the changes to the DB
+        session.updateDb();
+
+        // expect one create for groups 1 & 3
+        List<PdpGroup> changes = getGroupCreates();
+        assertSame(newgrp1, changes.get(0));
+        assertSame(newgrp3, changes.get(1));
+
+        // expect one update for groups 1 & 3
+        changes = getGroupUpdates();
+        assertSame(group1, changes.get(0));
+        assertSame(group3, changes.get(1));
+    }
+
+    @Test
+    public void testUpdateDb_Empty() throws Exception {
+        // force data into the cache
+        when(dao.getFilteredPdpGroups(any())).thenReturn(Arrays.asList(group1, group2));
+        session.getActivePdpGroupsByPolicyType(type);
+
+        session.updateDb();
+        verify(dao, never()).createPdpGroups(any());
+        verify(dao, never()).updatePdpGroups(any());
     }
 
     private PdpUpdate makeUpdate(String pdpName) {