From f04de2d7683595e4fb8ace9842bdae5c68eb6282 Mon Sep 17 00:00:00 2001 From: Fernando Bellas Permuy Date: Thu, 14 Jan 2010 13:44:01 +0100 Subject: [PATCH] ItEr43S08CUImportacionRecursosProductivosItEr42S12: Added support for detecting overlapping in criterion satisfactions when importing resources. Please note that criterion satisfaction overlapping means: (1) if criterion satisfactions refer to the same Criterion, they must not overlap (regardless of its CriterionType allows simultaneous criterion satisfactions per resource), and (2) if CriterionType does not allow simultaneous criterion satisfactions per resource, criterion satisfactions (of the same type) must not overlap (regardless of they refer to different Criterion objects). --- .../entities/CriterionSatisfaction.java | 11 -- .../business/resources/entities/Resource.java | 97 +++++++++++++---- .../resources/criterion/CriterionsModel.java | 2 - .../web/resources/worker/WorkerModel.java | 1 - .../ws/resources/api/ResourceServiceTest.java | 100 +++++++++++++++++- scripts/rest-clients/resources-sample.xml | 54 +++++++++- 6 files changed, 229 insertions(+), 36 deletions(-) diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/CriterionSatisfaction.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/CriterionSatisfaction.java index d540fc3e5..f3e402ca2 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/CriterionSatisfaction.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/CriterionSatisfaction.java @@ -275,17 +275,6 @@ public class CriterionSatisfaction extends BaseEntity { } } - /* - * IMPORTANT: This method currently redefines BaseEntity::validate avoiding - * Hibernate Validator to run. This should be fixed in a future. - */ - public void validate(){ - Validate.notNull(resource); - Validate.notNull(startDate); - Validate.notNull(criterion); - Validate.isTrue(checkConstraintPositiveTimeInterval()); - } - public ResourceEnum getResourceType() { return criterion.getType().getResource(); } diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Resource.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Resource.java index 4d5b3cd23..f6cdda303 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Resource.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Resource.java @@ -570,6 +570,9 @@ public abstract class Resource extends BaseEntity{ return criterionSatisfactions.contains(satisfaction); } + /** + * @throws IllegalArgumentException in case of overlapping + */ public void checkNotOverlaps() { checkNotOverlaps(getRelatedTypes()); } @@ -582,36 +585,62 @@ public abstract class Resource extends BaseEntity{ return types; } + /** + * @throws IllegalArgumentException in case of overlapping + */ private void checkNotOverlaps(List types) { for (CriterionType criterionType : types) { - if (!criterionType.isAllowSimultaneousCriterionsPerResource()) { - List satisfactions = query().from( - criterionType).result(); - ListIterator listIterator = satisfactions - .listIterator(); - while (listIterator.hasNext()) { - CriterionSatisfaction current = listIterator.next(); - CriterionSatisfaction previous = getPrevious(listIterator); - CriterionSatisfaction next = getNext(listIterator); - if (previous != null) { - checkNotOverlaps(previous, current); - } - if (next != null) { - checkNotOverlaps(current, next); - } + List satisfactions = query().from( + criterionType).result(); + ListIterator listIterator = satisfactions + .listIterator(); + while (listIterator.hasNext()) { + CriterionSatisfaction current = listIterator.next(); + CriterionSatisfaction previous = getPrevious(listIterator); + CriterionSatisfaction next = getNext(listIterator); + if (previous != null) { + checkNotOverlaps(previous, current); + } + if (next != null) { + checkNotOverlaps(current, next); } } - } - } + /** + * IMPORTANT: before and after must refer to the + * same CriterionType + * + * @throws IllegalArgumentException in case of overlapping + */ private void checkNotOverlaps(CriterionSatisfaction before, CriterionSatisfaction after) { - if (!before.goesBeforeWithoutOverlapping(after)) { + + CriterionType criterionType = before.getCriterion().getType(); + + /* + * If criterion satisfactions refer to the same Criterion, they must not + * overlap (regardless of its CriterionType allows simultaneous + * criterion satisfactions per resource). + */ + if (before.getCriterion().equals(after.getCriterion()) && + !before.goesBeforeWithoutOverlapping(after)) { + throw new IllegalArgumentException(createOverlapsMessage(before, + after)); + } + + /* + * If CriterionType does not allow simultaneous criterion satisfactions + * per resource, criterion satisfactions must not overlap (regardless + * of they refer to different Criterion objects). + */ + if (!criterionType.isAllowSimultaneousCriterionsPerResource() && + !before.goesBeforeWithoutOverlapping(after)) { throw new IllegalArgumentException(createOverlapsMessage(before, after)); } + } private String createOverlapsMessage(CriterionSatisfaction before, @@ -814,8 +843,38 @@ public abstract class Resource extends BaseEntity{ assignment.setResource(null); } + @AssertTrue(message="Some criterion satisfactions overlap in time") + public boolean checkConstraintCriterionSatisfactionsOverlapping() { + + /* + * Check if time intervals in criterion satisfactions are correct in + * isolation. If not, it does not make sense to check criterion + * satisfaction overlapping. + */ + for (CriterionSatisfaction i : getCriterionSatisfactions()) { + + if (!(i.isStartDateSpecified() && + i.checkConstraintPositiveTimeInterval())) { + return true; + } + + } + + /* + * Check assignment overlapping. + */ + try { + checkNotOverlaps(); + } catch (IllegalArgumentException e) { + return false; + } + + return true; + + } + @AssertFalse(message="Some cost category assignments overlap in time") - public boolean checkConstraintAssignmentsOverlap() { + public boolean checkConstraintAssignmentsOverlapping() { /* * Check if time intervals in cost assignments are correct in isolation. diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionsModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionsModel.java index c16425142..54b678c8e 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionsModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionsModel.java @@ -195,7 +195,6 @@ public class CriterionsModel implements ICriterionsModel { reloaded .addSatisfaction(new CriterionWithItsType(criterionType, criterion)); resourceDAO.save(reloaded); - reloaded.checkNotOverlaps(); } } @@ -207,7 +206,6 @@ public class CriterionsModel implements ICriterionsModel { reloaded.finish(new CriterionWithItsType(criterionType, criterion)); resourceDAO.save(reloaded); - reloaded.checkNotOverlaps(); } } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/worker/WorkerModel.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/worker/WorkerModel.java index 04032a8c1..b468123bf 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/worker/WorkerModel.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/worker/WorkerModel.java @@ -115,7 +115,6 @@ public class WorkerModel implements IWorkerModel { assignedCriterionsModel.confirm(); } resourceDAO.save(worker); - worker.checkNotOverlaps(); worker = null; localizationsAssigner = null; } diff --git a/navalplanner-webapp/src/test/java/org/navalplanner/web/test/ws/resources/api/ResourceServiceTest.java b/navalplanner-webapp/src/test/java/org/navalplanner/web/test/ws/resources/api/ResourceServiceTest.java index 633100b08..23a051c38 100644 --- a/navalplanner-webapp/src/test/java/org/navalplanner/web/test/ws/resources/api/ResourceServiceTest.java +++ b/navalplanner-webapp/src/test/java/org/navalplanner/web/test/ws/resources/api/ResourceServiceTest.java @@ -263,7 +263,7 @@ public class ResourceServiceTest { ' ' + ct.getName().toUpperCase() + // Upper case and blank ' ', " C1 ", // spaces intentionally // added (OK). - getDate(2001, 1, 1), null)); + getDate(2001, 1, 1), getDate(2001, 2, 1))); machineDTO.criterionSatisfactions.add( new CriterionSatisfactionDTO(ct.getName(), "c2", getDate(2001, 1, 1), null)); @@ -331,6 +331,73 @@ public class ResourceServiceTest { } + @Test + @NotTransactional + public void testAddResourceWithOverlappingCriterionSatisfactionsAllowed() { + + /* Create a criterion type. */ + CriterionType ct = createCriterionType(); + + /* + * Create a machine DTO. OK, because + * ct.isAllowSimultaneousCriterionsPerResource() is true. + */ + MachineDTO machineDTO = createMachineDTOWithTwoCriterionSatisfactions( + "machine", ct.getName(), + "c1", getDate(2000, 1, 1), getDate(2000, 2, 1), + "c2", getDate(2000, 1, 15), getDate(2000, 2, 1)); + + /* Test. */ + assertNoConstraintViolations( + resourceService.addResources(createResourceListDTO(machineDTO))); + assertTrue(machineDAO.existsMachineWithCodeInAnotherTransaction( + machineDTO.code)); + + } + + @Test + @NotTransactional + public void testAddResourceWithOverlappingCriterionSatisfactions() { + + /* Create criterion types. */ + CriterionType ct1 = createCriterionType(); + CriterionType ct2 = createCriterionType(ResourceEnum.RESOURCE, false); + + /* + * Create resource DTOs. Each resource contains one criterion + * satisfaction overlapping. + * + */ + MachineDTO m1 = createMachineDTOWithTwoCriterionSatisfactions( + "m1", ct1.getName(), // Interval overlapping in "c1". + "c1", getDate(2000, 1, 1), getDate(2000, 2, 1), + "c1", getDate(2000, 1, 15), getDate(2000, 2, 1)); + + MachineDTO m2 = createMachineDTOWithTwoCriterionSatisfactions( + "m2", ct2.getName(), // Overlapping because "ct2" does not allow + // simultaneous criterion satisfactions in + // intervals that overlap. + "c1", getDate(2000, 1, 1), getDate(2000, 2, 1), + "c2", getDate(2000, 1, 15), getDate(2000, 2, 1)); + + /* Test. */ + ResourceListDTO resourceDTOs = createResourceListDTO( + m1, m2); + + assertOneConstraintViolationPerInstance( + resourceService.addResources(resourceDTOs), + resourceDTOs.resources.size()); + + for (ResourceDTO r : resourceDTOs.resources) { + MachineDTO m = (MachineDTO) r; + assertFalse( + "Machine " + m.name + " not expected", + machineDAO.existsMachineWithCodeInAnotherTransaction( + ((MachineDTO) r).code)); + } + + } + @Test @NotTransactional public void testAddResourcesWithCriterionSatisfactionsWithIncorrectType() { @@ -640,10 +707,15 @@ public class ResourceServiceTest { } private CriterionType createCriterionType() { - return createCriterionType(ResourceEnum.RESOURCE); + return createCriterionType(ResourceEnum.RESOURCE, true); } private CriterionType createCriterionType(final ResourceEnum resourceType) { + return createCriterionType(resourceType, true); + } + + private CriterionType createCriterionType(final ResourceEnum resourceType, + final boolean allowSimultaneousCriterionsPerResource) { IOnTransaction createCriterionType = new IOnTransaction() { @@ -653,6 +725,8 @@ public class ResourceServiceTest { CriterionType ct = CriterionType.create(getUniqueName(), "desc"); + ct.setAllowSimultaneousCriterionsPerResource( + allowSimultaneousCriterionsPerResource); ct.setResource(resourceType); Criterion c1 = Criterion.create("c1", ct); Criterion c2 = Criterion.create("c2", ct); @@ -785,6 +859,28 @@ public class ResourceServiceTest { } + + private MachineDTO createMachineDTOWithTwoCriterionSatisfactions( + String machineName, String criterionTypeName, + String criterionName1, XMLGregorianCalendar startDate1, + XMLGregorianCalendar endDate1, + String criterionName2, XMLGregorianCalendar startDate2, + XMLGregorianCalendar endDate2) { + + MachineDTO machineDTO = new MachineDTO(getUniqueName(), machineName, + "desc"); + + machineDTO.criterionSatisfactions.add( + new CriterionSatisfactionDTO(criterionTypeName, criterionName1, + startDate1, endDate1)); + machineDTO.criterionSatisfactions.add( + new CriterionSatisfactionDTO(criterionTypeName, criterionName2, + startDate2, endDate2)); + + return machineDTO; + + } + private MachineDTO createMachineDTOWithTwoCostsAssignments( String machineName, String costCategoryName, XMLGregorianCalendar startDate1, XMLGregorianCalendar endDate1, diff --git a/scripts/rest-clients/resources-sample.xml b/scripts/rest-clients/resources-sample.xml index 9d17305f8..b308bb2dd 100644 --- a/scripts/rest-clients/resources-sample.xml +++ b/scripts/rest-clients/resources-sample.xml @@ -186,7 +186,7 @@ - + + + + + + + + + + + + + + + + + + + + + + + + +