#3864:Check history conflict when merging position

NCI-Agency · Oct 12, 2021 · c91a8f6 · c91a8f6
1 parent 54c5a92
commit c91a8f6
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 17 deletions.
diff --git a/src/main/java/mil/dds/anet/database/PersonDao.java b/src/main/java/mil/dds/anet/database/PersonDao.java
@@ -475,11 +475,11 @@ public int updatePersonHistory(Person p) {
   }
 
   @InTransaction
-  public boolean hasHistoryConflict(final String uuid, final List<PersonPositionHistory> history,
-      final boolean checkPerson) {
-    final String personPositionClause =
-        checkPerson ? "\"personUuid\" != :personUuid AND \"positionUuid\" = :positionUuid"
-            : "\"personUuid\" = :personUuid AND \"positionUuid\" != :positionUuid";
+  public boolean hasHistoryConflict(final String uuid, final String loserUuid,
+      final List<PersonPositionHistory> history, final boolean checkPerson) {
+    final String personPositionClause = checkPerson
+        ? "\"personUuid\" != :personUuid AND \"personUuid\" != :loserUuid AND \"positionUuid\" = :positionUuid"
+        : "\"personUuid\" = :personUuid AND \"positionUuid\" != :positionUuid AND \"positionUuid\" != :loserUuid";
     for (final PersonPositionHistory pph : history) {
       final Query q;
       final Instant endTime = pph.getEndTime();
@@ -498,7 +498,8 @@ public boolean hasHistoryConflict(final String uuid, final List<PersonPositionHi
       final Number count =
           (Number) q.bind("startTime", DaoUtils.asLocalDateTime(pph.getStartTime()))
               .bind("personUuid", checkPerson ? uuid : histUuid)
-              .bind("positionUuid", checkPerson ? histUuid : uuid).map(new MapMapper(false)).one()
+              .bind("positionUuid", checkPerson ? histUuid : uuid)
+              .bind("loserUuid", Utils.orIfNull(loserUuid, "")).map(new MapMapper(false)).one()
               .get("count");
 
       if (count.longValue() > 0) {

diff --git a/src/main/java/mil/dds/anet/resources/PersonResource.java b/src/main/java/mil/dds/anet/resources/PersonResource.java
@@ -201,7 +201,7 @@ public int updatePersonHistory(@GraphQLRootContext Map<String, Object> context,
     assertCanUpdatePerson(user, existing);
     ResourceUtils.validateHistoryInput(p.getUuid(), p.getPreviousPositions());
 
-    if (AnetObjectEngine.getInstance().getPersonDao().hasHistoryConflict(p.getUuid(),
+    if (AnetObjectEngine.getInstance().getPersonDao().hasHistoryConflict(p.getUuid(), null,
         p.getPreviousPositions(), true)) {
       throw new WebApplicationException(
           "At least one of the positions in the history is occupied for the specified period.",

diff --git a/src/main/java/mil/dds/anet/resources/PositionResource.java b/src/main/java/mil/dds/anet/resources/PositionResource.java
@@ -172,7 +172,7 @@ public int updatePositionHistory(@GraphQLRootContext Map<String, Object> context
     final Position existing = dao.getByUuid(pos.getUuid());
     ResourceUtils.validateHistoryInput(pos.getUuid(), pos.getPreviousPeople());
     assertCanUpdatePosition(user, existing);
-    if (AnetObjectEngine.getInstance().getPersonDao().hasHistoryConflict(pos.getUuid(),
+    if (AnetObjectEngine.getInstance().getPersonDao().hasHistoryConflict(pos.getUuid(), null,
         pos.getPreviousPeople(), false)) {
       throw new WebApplicationException(
           "At least one of the positions in the history is occupied for the specified period.",
@@ -262,10 +262,17 @@ public Position mergePositions(@GraphQLRootContext Map<String, Object> context,
       @GraphQLArgument(name = "loserUuid") String loserUuid) {
     final Person user = DaoUtils.getUserFromContext(context);
     final Position loserPosition = dao.getByUuid(loserUuid);
-
+    ResourceUtils.validateHistoryInput(winnerPosition.getUuid(),
+        winnerPosition.getPreviousPeople());
     assertCanUpdatePosition(user, winnerPosition);
     // Check that given two position can be merged
     arePositionsMergeable(winnerPosition, loserPosition);
+    if (AnetObjectEngine.getInstance().getPersonDao().hasHistoryConflict(winnerPosition.getUuid(),
+        loserUuid, winnerPosition.getPreviousPeople(), false)) {
+      throw new WebApplicationException(
+          "At least one of the positions in the history is occupied for the specified period.",
+          Status.CONFLICT);
+    }
     validatePosition(user, winnerPosition);
 
     int numRows = dao.mergePositions(winnerPosition, loserPosition);

diff --git a/src/test/java/mil/dds/anet/test/database/PersonDaoTest.java b/src/test/java/mil/dds/anet/test/database/PersonDaoTest.java
@@ -126,21 +126,29 @@ public static void setUpClass() throws Exception {
   @Test
   public void hasHistoryConflictForPersonTest() {
     // History tests for Dvisor and Dvisor's own position
-    checkHistoryConflict(DVISOR_UUID, EF22_ASF_UUID, true, testData1);
+    checkHistoryConflict(DVISOR_UUID, null, EF22_ASF_UUID, true, testData1);
     // History tests for Selena and Dvisor's position
-    checkHistoryConflict(SELENA_UUID, EF22_ASF_UUID, true, testData2);
+    checkHistoryConflict(SELENA_UUID, null, EF22_ASF_UUID, true, testData2);
   }
 
   @Test
   public void hasHistoryConflictForPositionTest() {
     // History tests for Dvisor's own position and Dvisor
-    checkHistoryConflict(DVISOR_UUID, EF22_ASF_UUID, false, testData1);
+    checkHistoryConflict(DVISOR_UUID, null, EF22_ASF_UUID, false, testData1);
     // History tests for Selena's position and Dvisor:
-    checkHistoryConflict(DVISOR_UUID, EF12_A_UUID, false, testData2);
+    checkHistoryConflict(DVISOR_UUID, null, EF12_A_UUID, false, testData2);
   }
 
-  private void checkHistoryConflict(final String personUuid, final String positionUuid,
-      final boolean checkPerson, final Object[][] testData) {
+  @Test
+  public void hasHistoryConflictForMergingPositionTest() {
+    // History tests for merging positions if EF22_ASF_UUID is winner
+    checkHistoryConflict(DVISOR_UUID, EF12_A_UUID, EF22_ASF_UUID, false, testData1);
+    // History tests for merging positions if EF12_A_UUID is winner
+    checkHistoryConflict(DVISOR_UUID, EF22_ASF_UUID, EF12_A_UUID, false, testData2);
+  }
+
+  private void checkHistoryConflict(final String personUuid, final String loserUuid,
+      final String positionUuid, final boolean checkPerson, final Object[][] testData) {
     for (final Object[] testItem : testData) {
       int i = 0;
       final boolean hasConflict = (boolean) testItem[i++];
@@ -154,8 +162,8 @@ private void checkHistoryConflict(final String personUuid, final String position
         hist.add(pph);
       }
       logger.debug("checking {}", Arrays.toString(testItem));
-      final boolean hasHistoryConflict =
-          personDao.hasHistoryConflict(checkPerson ? personUuid : positionUuid, hist, checkPerson);
+      final boolean hasHistoryConflict = personDao.hasHistoryConflict(
+          checkPerson ? personUuid : positionUuid, loserUuid, hist, checkPerson);
       assertThat(hasHistoryConflict).isEqualTo(hasConflict);
     }
   }