From 69bf73afbb8469dc05fd1918d805b9d59d6e0529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Necmettin=20DEN=C4=B0Z?= Date: Tue, 5 Oct 2021 15:31:35 +0300 Subject: [PATCH] NCI-Agency/anet#3864:Check history conflict when merging position --- .../java/mil/dds/anet/database/PersonDao.java | 13 +++---- .../dds/anet/resources/PersonResource.java | 2 +- .../dds/anet/resources/PositionResource.java | 11 ++++-- .../dds/anet/test/database/PersonDaoTest.java | 35 ++++++++++++------- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/main/java/mil/dds/anet/database/PersonDao.java b/src/main/java/mil/dds/anet/database/PersonDao.java index c448ae6fde..7be2c77c16 100644 --- 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 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 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 0) { diff --git a/src/main/java/mil/dds/anet/resources/PersonResource.java b/src/main/java/mil/dds/anet/resources/PersonResource.java index b963b0dfa2..716fc89eb9 100644 --- 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 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 index 5358d15c8e..fd459d2742 100644 --- 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 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 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 index ffb45cae35..5876732eff 100644 --- a/src/test/java/mil/dds/anet/test/database/PersonDaoTest.java +++ b/src/test/java/mil/dds/anet/test/database/PersonDaoTest.java @@ -1,21 +1,20 @@ package mil.dds.anet.test.database; import static org.assertj.core.api.Assertions.assertThat; - import java.lang.invoke.MethodHandles; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import mil.dds.anet.AnetObjectEngine; -import mil.dds.anet.beans.PersonPositionHistory; -import mil.dds.anet.database.PersonDao; -import mil.dds.anet.test.integration.utils.TestApp; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import mil.dds.anet.AnetObjectEngine; +import mil.dds.anet.beans.PersonPositionHistory; +import mil.dds.anet.database.PersonDao; +import mil.dds.anet.test.integration.utils.TestApp; @ExtendWith(TestApp.class) public class PersonDaoTest { @@ -27,6 +26,8 @@ public class PersonDaoTest { private static final String DVISOR_UUID = "39d02d26-49eb-43b5-9cec-344777213a67"; // SOLENOID, Selena private static final String SELENA_UUID = "00b19ebf-0d4d-4b0f-93c8-9023ccb59c49"; + // STEVESON, Steve for looser position + private static final String STEVESON_STEVE = "90fa5784-9e63-4353-8119-357bcd88e287"; // EF 2.2 Advisor Sewing Facilities private static final String EF22_ASF_UUID = "2b7d86a9-3ed4-4843-ab4e-136c3ab109bf"; // EF 1.2 Advisor @@ -126,21 +127,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); + } + + @Test + public void hasHistoryConflictForMergingPositionTest() { + // History tests for merging positions if EF22_ASF_UUID is winner + checkHistoryConflict(DVISOR_UUID, STEVESON_STEVE, EF22_ASF_UUID, false, testData1); + // History tests for merging positions if EF12_A_UUID is winner + checkHistoryConflict(DVISOR_UUID, STEVESON_STEVE, EF12_A_UUID, false, testData2); } - private void checkHistoryConflict(final String personUuid, final String positionUuid, - final boolean checkPerson, final Object[][] testData) { + 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 +163,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); } }