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 @@ - + + + + + + + + + + + + + + + + + + + + + + + + +