From 1a12344ce5d79598dbc46e5d5d48865f8d7ab0cb Mon Sep 17 00:00:00 2001 From: Fernando Bellas Permuy Date: Tue, 5 Jan 2010 20:01:25 +0100 Subject: [PATCH] ItEr42S12CUImportacionRecursosProductivosItEr41S15: Detection of resoures with hthe same "logical" name among the resources being imported. Resources with the same "logical" name (code for machines; first name, surnamne, and nif for workers) among the list of resources *being imported* are detected. This detection must be implemented at the service-level (at the entity-level, @AssertTrue annotations in Machine and Worker detect importation of resources with the same logical name as other resources already existing in the *database*). A test case has been added (furthermore, ResourceServiceTest has been refactorized a little bit). --- .../business/resources/entities/Resource.java | 2 +- .../ws/resources/api/MachineDTO.java | 5 - .../ws/resources/api/ResourceDTO.java | 2 - .../ws/resources/api/WorkerDTO.java | 6 - .../resources/impl/ResourceServiceREST.java | 78 +++++++-- .../ws/resources/api/ResourceServiceTest.java | 164 ++++++++---------- scripts/rest-clients/resources-sample.xml | 7 + 7 files changed, 149 insertions(+), 115 deletions(-) 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 0156babf3..82bebfe93 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 @@ -824,7 +824,7 @@ public abstract class Resource extends BaseEntity{ return false; } - @AssertTrue(message="There are criterion satisfactions referring to " + + @AssertTrue(message="there are criterion satisfactions referring to " + "criterion types not applicable to this resource") public boolean checkConstraintCriterionSatisfactionsWithCorrectType() { diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/MachineDTO.java b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/MachineDTO.java index d577f5ed6..2c4c33b85 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/MachineDTO.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/MachineDTO.java @@ -46,9 +46,4 @@ public class MachineDTO extends ResourceDTO { this.description = description; } - @Override - public String getUserProvidedId() { - return "machine" + '-' + code; - } - } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/ResourceDTO.java b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/ResourceDTO.java index 599e4f499..a14a561aa 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/ResourceDTO.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/ResourceDTO.java @@ -38,6 +38,4 @@ public abstract class ResourceDTO { public List criterionSatisfactions = new ArrayList(); - public abstract String getUserProvidedId(); - } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/WorkerDTO.java b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/WorkerDTO.java index a4105f6f0..ed0e4e15a 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/WorkerDTO.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/api/WorkerDTO.java @@ -46,10 +46,4 @@ public class WorkerDTO extends ResourceDTO { this.nif = nif; } - @Override - public String getUserProvidedId() { - - return "worker" + '-' + firstName + '-' + surname + '-' + nif; - } - } diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/impl/ResourceServiceREST.java b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/impl/ResourceServiceREST.java index d66d10d04..5be3ca265 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/impl/ResourceServiceREST.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/impl/ResourceServiceREST.java @@ -20,14 +20,19 @@ package org.navalplanner.ws.resources.impl; +import static org.navalplanner.web.I18nHelper._; + import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.ws.rs.Consumes; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.Produces; +import org.apache.commons.lang.StringUtils; import org.navalplanner.business.common.exceptions.CreateUnvalidatedException; import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.daos.IResourceDAO; @@ -37,8 +42,10 @@ import org.navalplanner.ws.common.api.InstanceConstraintViolationsListDTO; import org.navalplanner.ws.common.impl.ConstraintViolationConverter; import org.navalplanner.ws.common.impl.Util; import org.navalplanner.ws.resources.api.IResourceService; +import org.navalplanner.ws.resources.api.MachineDTO; import org.navalplanner.ws.resources.api.ResourceDTO; import org.navalplanner.ws.resources.api.ResourceListDTO; +import org.navalplanner.ws.resources.api.WorkerDTO; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -66,6 +73,7 @@ public class ResourceServiceREST implements IResourceService { List instanceConstraintViolationsList = new ArrayList(); int instanceNumber = 1; + Set resourceUserProvidedIds = new HashSet(); /* Process resources. */ for (ResourceDTO resourceDTO : resources.resources) { @@ -81,7 +89,7 @@ public class ResourceServiceREST implements IResourceService { instanceConstraintViolationsDTO = InstanceConstraintViolationsDTO.create( Util.generateInstanceId(instanceNumber, - resourceDTO.getUserProvidedId()), + getUserProvidedId(resourceDTO)), e.getMessage()); } @@ -89,20 +97,36 @@ public class ResourceServiceREST implements IResourceService { if (resource != null) { try { - /* - * See CriterionServiceREST::addCriterionTypes for a - * justification of the explicit use of - * BaseEntity::validate. - * - */ - resource.validate(); - resourceDAO.save(resource); + if (resourceUserProvidedIds.contains( + getUserProvidedId(resourceDTO).toLowerCase())) { + + instanceConstraintViolationsDTO = + InstanceConstraintViolationsDTO.create( + Util.generateInstanceId(instanceNumber, + getUserProvidedId(resourceDTO)), + getDuplicatedImportedResourceErrorMessage( + resourceDTO)); + + } else { + + /* + * See CriterionServiceREST::addCriterionTypes for a + * justification of the explicit use of + * BaseEntity::validate. + * + */ + resource.validate(); + resourceDAO.save(resource); + resourceUserProvidedIds.add( + getUserProvidedId(resourceDTO).toLowerCase()); + + } } catch (ValidationException e) { instanceConstraintViolationsDTO = ConstraintViolationConverter.toDTO( Util.generateInstanceId(instanceNumber, - resourceDTO.getUserProvidedId()), + getUserProvidedId(resourceDTO)), e.getInvalidValues()); } } @@ -122,4 +146,38 @@ public class ResourceServiceREST implements IResourceService { } + private String getUserProvidedId(ResourceDTO resourceDTO) { + + if (resourceDTO instanceof MachineDTO) { + MachineDTO m = (MachineDTO) resourceDTO; + return "machine" + '-' + StringUtils.trim(m.code); + } else if (resourceDTO instanceof WorkerDTO) { + WorkerDTO w = (WorkerDTO) resourceDTO; + return "worker" + '-' + StringUtils.trim(w.firstName) + + '-' + StringUtils.trim(w.surname) + '-' + + StringUtils.trim(w.nif); + } else { + throw new RuntimeException( + _("Service does not manages resource of type: {0}", + resourceDTO.getClass().getName())); + } + + } + + private String getDuplicatedImportedResourceErrorMessage( + ResourceDTO resourceDTO) { + + if (resourceDTO instanceof MachineDTO) { + return _("code is used by another machine being imported"); + } else if (resourceDTO instanceof WorkerDTO) { + return _("first name, surname, and nif are used by another " + + "worker being imported"); + } else { + throw new RuntimeException( + _("Service does not manages resource of type: {0}", + resourceDTO.getClass().getName())); + } + + } + } 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 39a9906ea..4bc921d57 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 @@ -127,6 +127,79 @@ public class ResourceServiceTest { } + @Test + @NotTransactional + public void testAddMachineWithExistingCode() + throws InstanceNotFoundException { + + /* Create a machine. */ + Machine m1 = Machine.createUnvalidated(getUniqueName(), "name", "desc"); + saveResource(m1); + + /* Create a machine DTO with the same code. */ + MachineDTO m2 = new MachineDTO(m1.getCode(), "name", "desc"); + + /* Test. */ + assertOneConstraintViolation( + resourceService.addResources(createResourceListDTO(m2))); + machineDAO.findUniqueByCodeInAnotherTransaction(m1.getCode()); + + } + + @Test + @NotTransactional + public void testAddWorkerWithExistingFirstNameSurnameAndNif() { + + /* Create a worker. */ + Worker w1 = Worker.createUnvalidated(getUniqueName(), "surname", "nif"); + saveResource(w1); + + /* + * Create a worker DTO with the same first name, surname, and nif as + * the previous one. + */ + WorkerDTO w2 = new WorkerDTO(w1.getFirstName(), w1.getSurname(), + w1.getNif()); + + /* Test. */ + assertOneConstraintViolation( + resourceService.addResources(createResourceListDTO(w2))); + assertTrue( + workerDAO.findByFirstNameSecondNameAndNifAnotherTransaction( + w2.firstName, w2.surname, w2.nif).size() == 1); + + } + + + @Test + public void testAddResourcesWithDuplicateResourcesBeingImported() + throws InstanceNotFoundException { + + /* Create resource DTOs. */ + MachineDTO m1 = new MachineDTO(getUniqueName(), "m1-name", "m1-desc"); + MachineDTO m2 = new MachineDTO(' ' + m1.code.toUpperCase() + ' ', + "m2-name", "m2-desc"); + WorkerDTO w1 = new WorkerDTO(getUniqueName(), "w1-surname", "w1-nif"); + WorkerDTO w2 = new WorkerDTO(w1.firstName, + ' ' + w1.surname.toUpperCase() + ' ', w1.nif); + + /* Test. */ + List instanceConstraintViolationsList = + resourceService.addResources(createResourceListDTO(m1, m2, w1, w2)). + instanceConstraintViolationsList; + + assertTrue(instanceConstraintViolationsList.size() == 2); + assertTrue(instanceConstraintViolationsList.get(0). + constraintViolations.size() == 1); + assertTrue(instanceConstraintViolationsList.get(1). + constraintViolations.size() == 1); + machineDAO.findUniqueByCode(m1.code); + assertTrue( + workerDAO.findByFirstNameSecondNameAndNif( + w1.firstName, w1.surname, w1.nif.trim()).size() == 1); + + } + @Test @NotTransactional public void testAddResourceWithCriterionSatisfactions() @@ -165,54 +238,6 @@ public class ResourceServiceTest { } - @Test - @NotTransactional - public void AddResourceWithCriterionSatisfactionsWithIncorrectNames() { - - /* Create a criterion type. */ - CriterionType ct = createCriterionType(); - - /* Create a machine DTO. */ - MachineDTO machineDTO = new MachineDTO(getUniqueName(), "name", "desc"); - - /* Test. */ - machineDTO.criterionSatisfactions.add( // Non-existent criterion type. - new CriterionSatisfactionDTO(ct.getName() + 'X' , "c1", - Calendar.getInstance().getTime(), null)); - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(machineDTO))); - assertFalse(machineDAO.existsMachineWithCodeInAnotherTransaction( - machineDTO.code)); - - machineDTO.criterionSatisfactions.clear(); - machineDTO.criterionSatisfactions.add( // Non-existent criterion. - new CriterionSatisfactionDTO(ct.getName() , "c1" + 'X', - Calendar.getInstance().getTime(), null)); - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(machineDTO))); - assertFalse(machineDAO.existsMachineWithCodeInAnotherTransaction( - machineDTO.code)); - - machineDTO.criterionSatisfactions.clear(); - machineDTO.criterionSatisfactions.add( // Criterion type null. - new CriterionSatisfactionDTO(null , "c1", - Calendar.getInstance().getTime(), null)); - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(machineDTO))); - assertFalse(machineDAO.existsMachineWithCodeInAnotherTransaction( - machineDTO.code)); - - machineDTO.criterionSatisfactions.clear(); - machineDTO.criterionSatisfactions.add( // Criterion null. - new CriterionSatisfactionDTO(ct.getName() , null, - Calendar.getInstance().getTime(), null)); - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(machineDTO))); - assertFalse(machineDAO.existsMachineWithCodeInAnotherTransaction( - machineDTO.code)); - - } - @Test @NotTransactional public void testAddResourceWithCriterionSatisfactionsWithoutStartDate() { @@ -317,49 +342,6 @@ public class ResourceServiceTest { } - @Test - @NotTransactional - public void createMachineWithExistingCode() - throws InstanceNotFoundException { - - /* Create a machine. */ - Machine m1 = Machine.createUnvalidated(getUniqueName(), "name", "desc"); - saveResource(m1); - - /* Create a machine DTO with the same code. */ - MachineDTO m2 = new MachineDTO(m1.getCode(), "name", "desc"); - - /* Test. */ - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(m2))); - machineDAO.findUniqueByCodeInAnotherTransaction(m1.getCode()); - - } - - @Test - @NotTransactional - public void createWorkerWithExistingFirstNameSurnameAndNif() { - - /* Create a worker. */ - Worker w1 = Worker.createUnvalidated(getUniqueName(), "surname", "nif"); - saveResource(w1); - - /* - * Create a worker DTO with the same first name, surname, and nif as - * the previous one. - */ - WorkerDTO w2 = new WorkerDTO(w1.getFirstName(), w1.getSurname(), - w1.getNif()); - - /* Test. */ - assertOneConstraintViolation( - resourceService.addResources(createResourceListDTO(w2))); - assertTrue( - workerDAO.findByFirstNameSecondNameAndNifAnotherTransaction( - w2.firstName, w2.surname, w2.nif).size() == 1); - - } - private CriterionType createCriterionType() { return createCriterionType(ResourceEnum.RESOURCE); } diff --git a/scripts/rest-clients/resources-sample.xml b/scripts/rest-clients/resources-sample.xml index c793c1937..980be3f0a 100644 --- a/scripts/rest-clients/resources-sample.xml +++ b/scripts/rest-clients/resources-sample.xml @@ -21,6 +21,9 @@ + + + @@ -94,4 +97,8 @@ + + +