RenJuan/TASKPM
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

ItEr43S05ValidacionEProbasFuncionaisItEr42S05: Minor improvements to criterion REST service.

Browse source 
The following improvements have been implemented: (1) criterion type and criterion names are trimmed with StringUtils.trim, (2) case is ignored for criterion type and criterion names, (3) just one constraint violation is reported for a criterion type with a name already used by another criterion type being imported, (4) CriterionType::checkConstraintNonRepeatedCriterionNames now discards criterions with names fulfilling StringUtils.isBlank(criterionName), (5) Criterion::createUnvalidated and CriterionType::createUnvalidated now use BaseEntity::create (to avoid to call setNewObject and other possible future actions), and (6) assertTrue/assertFalse JUnit methods in CriterionServiceTest now make use of the toString method provided by constraint violations related DTOs (to make debugging easier).
This commit is contained in:
Fernando Bellas Permuy 2010-01-13 17:59:06 +01:00 • committed by Javier Moran Rua
parent 3d4c488ac6
commit b9fae80d22
6 changed files with 103 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Criterion.java
View file

@ -45,13 +45,12 @@ public class Criterion extends BaseEntity implements ICriterion {
public static Criterion createUnvalidated(String name, CriterionType type,
Criterion parent, boolean active) {
Criterion criterion = new Criterion();
Criterion criterion = create(new Criterion());
criterion.name = name;
criterion.type = type;
criterion.parent = parent;
criterion.active = active;
criterion.setNewObject(true);
return criterion;

10
navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/CriterionType.java
View file

@ -56,7 +56,7 @@ public class CriterionType extends BaseEntity implements
Boolean allowSimultaneousCriterionsPerResource, Boolean enabled,
ResourceEnum resource) {
CriterionType criterionType = new CriterionType();
CriterionType criterionType = create(new CriterionType());
criterionType.name = name;
criterionType.description = description;
@ -65,7 +65,6 @@ public class CriterionType extends BaseEntity implements
allowSimultaneousCriterionsPerResource;
criterionType.enabled = enabled;
criterionType.resource = resource;
criterionType.setNewObject(true);
return criterionType;
@ -306,10 +305,13 @@ public class CriterionType extends BaseEntity implements
Set<String> criterionNames = new HashSet<String>();
for (Criterion c : criterions) {
if (criterionNames.contains(c.getName())) {
if (!StringUtils.isBlank(c.getName())) {
if (criterionNames.contains(c.getName().toLowerCase())) {
return false;
} else {
criterionNames.add(c.getName().toLowerCase());
}
}
criterionNames.add(c.getName());
}
return true;

11
navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/criterion/impl/CriterionConverter.java
View file

@ -24,6 +24,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import org.apache.commons.lang.StringUtils;
import org.navalplanner.business.resources.entities.Criterion;
import org.navalplanner.business.resources.entities.CriterionType;
import org.navalplanner.ws.common.impl.ResourceEnumConverter;
@ -101,7 +102,8 @@ public final class CriterionConverter {
CriterionTypeDTO criterionTypeDTO) {
CriterionType criterionType = CriterionType.createUnvalidated(
trim(criterionTypeDTO.name), trim(criterionTypeDTO.description),
StringUtils.trim(criterionTypeDTO.name),
StringUtils.trim(criterionTypeDTO.description),
criterionTypeDTO.allowHierarchy,
criterionTypeDTO.allowSimultaneousCriterionsPerResource,
criterionTypeDTO.enabled,
@ -136,15 +138,12 @@ public final class CriterionConverter {
CriterionDTO childDTO, CriterionType criterionType,
Criterion criterionParent) {
Criterion criterion = Criterion.createUnvalidated(trim(childDTO.name),
Criterion criterion = Criterion.createUnvalidated(
StringUtils.trim(childDTO.name),
criterionType, criterionParent, childDTO.active);
return criterion;
}
private static String trim(String s) {
return s == null ? null : s.trim();
}
}

74
navalplanner-webapp/src/main/java/org/navalplanner/ws/resources/criterion/impl/CriterionServiceREST.java
View file

@ -36,7 +36,6 @@ import javax.ws.rs.Produces;
import org.navalplanner.business.common.exceptions.ValidationException;
import org.navalplanner.business.resources.daos.ICriterionTypeDAO;
import org.navalplanner.business.resources.entities.CriterionType;
import org.navalplanner.ws.common.api.ConstraintViolationDTO;
import org.navalplanner.ws.common.api.InstanceConstraintViolationsDTO;
import org.navalplanner.ws.common.api.InstanceConstraintViolationsListDTO;
import org.navalplanner.ws.common.impl.ConstraintViolationConverter;
@ -80,17 +79,35 @@ public class CriterionServiceREST implements ICriterionService {
int instanceNumber = 1;
Set<String> criterionTypeNames = new HashSet<String>();
/* Process criterion types. */
for (CriterionTypeDTO criterionTypeDTO :
criterionTypes.criterionTypes) {
CriterionType criterionType =
CriterionConverter.toEntity(criterionTypeDTO);
/* Convert DTO to entity. */
InstanceConstraintViolationsDTO instanceConstraintViolationsDTO =
null;
boolean criterionTypeNameRepeated =
criterionTypeNames.contains(criterionType.getName());
CriterionType criterionType =
CriterionConverter.toEntity(criterionTypeDTO);
/*
* Check if the criterion type name is used by another criterion
* type being imported.
*/
if (criterionType.getName() != null && criterionTypeNames.contains(
criterionType.getName().toLowerCase())) {
instanceConstraintViolationsDTO =
InstanceConstraintViolationsDTO.create(
Util.generateInstanceId(instanceNumber,
criterionTypeDTO.name),
_("criterion type name is used by another criterion " +
"type being imported"));
} else {
/* Validate criterion type. */
try {
/*
* "validate" is executed before "save", since "save" first
* adds the object to the underlying ORM session and then
@ -101,18 +118,23 @@ public class CriterionServiceREST implements ICriterionService {
* when the ORM commits the transaction. As a side effect,
* validations are executed twice. Note also, that
* "CriterionType::checkConstraintUniqueCriterionTypeName"
* only checks if a criterion type with the same name already
* exists in the *database*, and that the criterion types
* being imported are inserted in the database when the
* transaction is committed. In consequence, we can only call
* "save" if the criterion type is valid according to "validate"
* method and its name is not used by another previously
* *imported* (not in the database yet) criterion type.
* only checks if a criterion type with the same name
* already exists in the *database*, and that the criterion
* types being imported are inserted in the database when
* the transaction is committed. In consequence, we can only
* call "save" if the criterion type is valid according to
* "validate" method and its name is not used by another
* previously *imported* (not in the database yet) criterion
* type.
*/
criterionType.validate();
if (!criterionTypeNameRepeated) {
criterionTypeDAO.save(criterionType);
if (criterionType.getName() != null) {
criterionTypeNames.add(criterionType.getName().
toLowerCase());
}
} catch (ValidationException e) {
instanceConstraintViolationsDTO =
ConstraintViolationConverter.toDTO(
@ -121,33 +143,9 @@ public class CriterionServiceREST implements ICriterionService {
e.getInvalidValues());
}
/*
* If criterion type name is repeated, add it to the list of
* constraint violations.
*/
if (criterionTypeNameRepeated) {
if (instanceConstraintViolationsDTO == null) {
instanceConstraintViolationsDTO =
new InstanceConstraintViolationsDTO(
Util.generateInstanceId(instanceNumber,
criterionTypeDTO.name),
new ArrayList<ConstraintViolationDTO>());
}
instanceConstraintViolationsDTO.constraintViolations.add(
new ConstraintViolationDTO(
CriterionType.class.getSimpleName() + ":name",
_("criterion type name is used by another criterion " +
"type being imported")));
} else {
if (criterionType.getName() != null) {
criterionTypeNames.add(criterionType.getName());
}
}
/*
* Add constraint violations of this criterion type to the returned
* list of constraint violations.
*/
/* Add constraint violations (if any). */
if (instanceConstraintViolationsDTO != null) {
instanceConstraintViolationsList.add(
instanceConstraintViolationsDTO);

49
navalplanner-webapp/src/test/java/org/navalplanner/web/test/ws/resources/criterion/api/CriterionServiceTest.java
View file

@ -84,7 +84,7 @@ public class CriterionServiceTest {
ct1c2Criterions);
CriterionDTO ct1c3 = new CriterionDTO("c3", true,
new ArrayList<CriterionDTO>());
CriterionDTO ct1c4 = new CriterionDTO("c3", true,
CriterionDTO ct1c4 = new CriterionDTO(" C3 ", true,
new ArrayList<CriterionDTO>()); // Repeated criterion name.
List<CriterionDTO> ct1Criterions = new ArrayList<CriterionDTO>();
ct1Criterions.add(ct1c1);
@ -129,18 +129,12 @@ public class CriterionServiceTest {
CriterionTypeDTO ct3 = new CriterionTypeDTO(ct3Name, "desc",
true, true, true, ResourceEnumDTO.RESOURCE, ct3Criterions);
/* Build criterion type "ct4" (2 constraint violations). */
CriterionDTO ct4c1 = new CriterionDTO(null, true, // Missing criterion
new ArrayList<CriterionDTO>()); // name.
CriterionDTO ct4c2 = new CriterionDTO("c2", true,
new ArrayList<CriterionDTO>());
List<CriterionDTO> ct4Criterions = new ArrayList<CriterionDTO>();
ct4Criterions.add(ct4c1);
ct4Criterions.add(ct4c2);
CriterionTypeDTO ct4 = // Repeated criterion
new CriterionTypeDTO(ct3Name, // type name (see previous
"desc", true, true, true, // criterion type).
ResourceEnumDTO.RESOURCE, ct4Criterions);
/* Build criterion type "ct4" (1 constraint violation). */
CriterionTypeDTO ct4 =
new CriterionTypeDTO( // Repeated criterion
' ' + ct3Name.toUpperCase() + ' ', // type name.
"desc", true, true, true,
ResourceEnumDTO.RESOURCE, new ArrayList<CriterionDTO>());
/* Criterion type list. */
List<CriterionTypeDTO> criterionTypes =
@ -155,13 +149,24 @@ public class CriterionServiceTest {
new CriterionTypeListDTO(criterionTypes)).
instanceConstraintViolationsList;
assertTrue(instanceConstraintViolationsList.size() == 3);
assertTrue(instanceConstraintViolationsList.get(0).
assertTrue(
instanceConstraintViolationsList.toString(),
instanceConstraintViolationsList.size() == 3);
assertTrue(
instanceConstraintViolationsList.get(0).
constraintViolations.toString(),
instanceConstraintViolationsList.get(0).
constraintViolations.size() == 4);
assertTrue(instanceConstraintViolationsList.get(1).
constraintViolations.size() == 2);
assertTrue(instanceConstraintViolationsList.get(2).
assertTrue(
instanceConstraintViolationsList.get(1).
constraintViolations.toString(),
instanceConstraintViolationsList.get(1).
constraintViolations.size() == 2);
assertTrue(
instanceConstraintViolationsList.get(2).
constraintViolations.toString(),
instanceConstraintViolationsList.get(2).
constraintViolations.size() == 1);
/* Find criterion types. */
List<CriterionTypeDTO> returnedCriterionTypes =
@ -205,13 +210,17 @@ public class CriterionServiceTest {
transactionService.runOnTransaction(
createCriterionType).
instanceConstraintViolationsList;
assertTrue(instanceConstraintViolationsList.size() == 0);
assertTrue(
instanceConstraintViolationsList.toString(),
instanceConstraintViolationsList.size() == 0);
instanceConstraintViolationsList =
transactionService.runOnTransaction(
createCriterionType).
instanceConstraintViolationsList;
assertTrue(instanceConstraintViolationsList.size() == 1);
assertTrue(
instanceConstraintViolationsList.toString(),
instanceConstraintViolationsList.size() == 1);
}

15
scripts/rest-clients/criterion-types-sample.xml
View file

@ -21,7 +21,7 @@
</children>
</criterion>
<!-- Repeated criterion name -->
<criterion name="c2-1" active="true"/>
<criterion name=" C2-1 " active="true"/>
</children>
</criterion>
</criterion-list>
@ -59,14 +59,17 @@
</criterion-type>
<!-- Repeated criterion type name (see above) -->
<criterion-type name="ct-4" description="ct-4 desc" allow-hierarchy="false"
<criterion-type name=" CT-4 " description="ct-4 desc"
allow-hierarchy="false"
allow-simultaneous-criterions-per-resource="false" enabled="false"
resource="WORKER"/>
<!-- Repeated criterion type name (probably a criterion type with this name
already exists in the database) -->
<criterion-type name="TRAINING" description="ct-4 desc"
allow-hierarchy="false" allow-simultaneous-criterions-per-resource="false" enabled="false" resource="WORKER"/>
<criterion-type name="training" description="ct-4 desc"
allow-hierarchy="false"
allow-simultaneous-criterions-per-resource="false" enabled="false"
resource="WORKER"/>
<!-- A non-active criterion has an active subcriterion -->
<criterion-type name="ct-5" description="ct-5 desc" allow-hierarchy="true"
@ -90,7 +93,9 @@
<!-- OK -->
<criterion-type name="ct-6" description="ct-6 desc"
allow-hierarchy="false" allow-simultaneous-criterions-per-resource="false" enabled="false" resource="WORKER"/>
allow-hierarchy="false"
allow-simultaneous-criterions-per-resource="false" enabled="false"
resource="WORKER"/>
<!-- Resource type does not allow enabled criteria -->
<criterion-type name="ct-7" description="ct-5 desc" allow-hierarchy="true"