diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/bootstrap/CriterionsBootstrap.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/bootstrap/CriterionsBootstrap.java index d481f9e50..7d302863b 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/bootstrap/CriterionsBootstrap.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/bootstrap/CriterionsBootstrap.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.ICriterionType; import org.navalplanner.business.resources.services.CriterionService; @@ -47,7 +48,11 @@ public class CriterionsBootstrap implements ICriterionsBootstrap { for (Entry, List> entry : typesWithCriterions .entrySet()) { for (Criterion criterion : entry.getValue()) { - criterionService.createIfNotExists(criterion); + try { + criterionService.createIfNotExists(criterion); + } catch (ValidationException e) { + e.printStackTrace(); + } } } } diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/ICriterionDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/ICriterionDAO.java index 2548c41b4..074382fe2 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/ICriterionDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/ICriterionDAO.java @@ -1,5 +1,6 @@ package org.navalplanner.business.resources.daos; +import java.util.List; import org.navalplanner.business.common.daos.IGenericDao; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; import org.navalplanner.business.resources.daos.impl.CriterionDAO; @@ -13,7 +14,9 @@ public interface ICriterionDAO extends IGenericDao { public void removeByNameAndType(Criterion criterion); - Criterion findByNameAndType(Criterion criterion); + List findByNameAndType(Criterion criterion); + + Criterion findUniqueByNameAndType(Criterion criterion) throws InstanceNotFoundException; boolean existsByNameAndType(Criterion entity); diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/impl/CriterionDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/impl/CriterionDAO.java index 0d40ba343..507233c51 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/impl/CriterionDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/impl/CriterionDAO.java @@ -1,7 +1,9 @@ package org.navalplanner.business.resources.daos.impl; -import java.util.logging.Level; -import java.util.logging.Logger; +import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.hibernate.Criteria; import org.hibernate.criterion.Restrictions; import org.navalplanner.business.common.daos.impl.GenericDaoHibernate; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; @@ -10,36 +12,58 @@ import org.navalplanner.business.resources.entities.Criterion; /** * DAO implementation for Criterion.
+ * * @author Óscar González Fernández + * @author Diego Pino García */ public class CriterionDAO extends GenericDaoHibernate implements ICriterionDAO { - public Criterion findByNameAndType(Criterion criterion) { - return (Criterion) getSession().createCriteria(Criterion.class).add( - Restrictions.eq("name", criterion.getName())).add( - Restrictions.eq("type", criterion.getType())).uniqueResult(); + private static final Log log = LogFactory.getLog(CriterionDAO.class); + + @Override + public List findByNameAndType(Criterion criterion) { + Criteria c = getSession().createCriteria(Criterion.class); + + c.add(Restrictions.eq("name", criterion.getName()).ignoreCase()); + c.add(Restrictions.eq("type", criterion.getType()).ignoreCase()); + + List result = (List) c.list(); + + return result; + } + + public Criterion findUniqueByNameAndType(Criterion criterion) throws InstanceNotFoundException { + List list = findByNameAndType(criterion); + + if (list.size() != 1) + throw new InstanceNotFoundException(criterion, Criterion.class + .getName()); + + return list.get(0); } public boolean existsByNameAndType(Criterion criterion) { - return findByNameAndType(criterion) != null; + try { + return findUniqueByNameAndType(criterion) != null; + } catch (InstanceNotFoundException e) { + return false; + } } @Override public Criterion find(Criterion criterion) throws InstanceNotFoundException { if (criterion.getId() != null) return super.find(criterion.getId()); - Criterion result = findByNameAndType(criterion); - if (result == null) - throw new InstanceNotFoundException(criterion, Criterion.class - .getName()); + Criterion result = findUniqueByNameAndType(criterion); + return result; } @Override public void removeByNameAndType(Criterion criterion) { - Criterion reloaded = findByNameAndType(criterion); try { + Criterion reloaded = findUniqueByNameAndType(criterion); remove(reloaded.getId()); } catch (InstanceNotFoundException ex) { throw new RuntimeException(ex); diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Criterion.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Criterion.java index 3fca9d4ca..cfdb7515d 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Criterion.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Criterion.java @@ -4,6 +4,7 @@ import java.util.Date; import org.apache.commons.lang.Validate; import org.apache.commons.lang.builder.EqualsBuilder; +import org.apache.commons.lang.builder.ToStringBuilder; import org.hibernate.validator.NotEmpty; /** @@ -93,4 +94,8 @@ public class Criterion implements ICriterion { return false; } + @Override + public String toString() { + return new ToStringBuilder(this).getStringBuffer().toString(); + } } diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/CriterionService.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/CriterionService.java index 47f7fd15e..2bdb31668 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/CriterionService.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/CriterionService.java @@ -6,6 +6,7 @@ import java.util.List; import org.navalplanner.business.common.OnTransaction; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; +import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.CriterionSatisfaction; import org.navalplanner.business.resources.entities.ICriterion; @@ -23,7 +24,14 @@ public interface CriterionService { void remove(Criterion criterion) throws InstanceNotFoundException; - void save(Criterion entity); + /** + * Save or update criterion + * + * "name" and "type" fields must be both unique + * + * @param entity + */ + void save(Criterion entity) throws ValidationException; Collection getResourcesSatisfying(ICriterion criterion); @@ -36,7 +44,7 @@ public interface CriterionService { Collection getSatisfactionsFor( ICriterionType criterionType, Date begin, Date end); - void createIfNotExists(Criterion criterion); + void createIfNotExists(Criterion criterion) throws ValidationException; boolean exists(Criterion criterion); diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/impl/CriterionServiceImpl.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/impl/CriterionServiceImpl.java index 96bbde5fd..3d272cca1 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/impl/CriterionServiceImpl.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/services/impl/CriterionServiceImpl.java @@ -5,9 +5,12 @@ import java.util.Collection; import java.util.Date; import java.util.List; +import javax.transaction.RollbackException; import org.apache.commons.lang.Validate; +import org.hibernate.validator.InvalidValue; import org.navalplanner.business.common.OnTransaction; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; +import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.daos.ICriterionDAO; import org.navalplanner.business.resources.daos.impl.CriterionDAO; import org.navalplanner.business.resources.entities.Criterion; @@ -57,18 +60,19 @@ public class CriterionServiceImpl implements CriterionService { } } - public void save(Criterion entity) { - if (criterionDAO.existsByNameAndType(entity)) { - /* - * TODO It's an unchecked exception by now. I consider this error an - * expected error condition, so we should send a checked exception. - */ - throw new RuntimeException( - "there must be only one criterion with name " - + entity.getName() + " and type " - + entity.getType()); - } + @Transactional(rollbackFor=ValidationException.class) + public void save(Criterion entity) throws ValidationException { criterionDAO.save(entity); + if (criterionDAO.findByNameAndType(entity).size() > 1) { + + InvalidValue[] invalidValues = { + new InvalidValue(entity.getName() + " already exists", + Criterion.class, "name", entity.getName(), entity) + }; + + throw new ValidationException(invalidValues, + "Couldn't save new criterion"); + } } @Override @@ -119,7 +123,7 @@ public class CriterionServiceImpl implements CriterionService { } @Override - public void createIfNotExists(Criterion criterion) { + public void createIfNotExists(Criterion criterion) throws ValidationException { if (!exists(criterion)) save(criterion); } diff --git a/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/services/CriterionServiceTest.java b/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/services/CriterionServiceTest.java index b3653d38d..014ea414b 100644 --- a/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/services/CriterionServiceTest.java +++ b/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/services/CriterionServiceTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.navalplanner.business.common.OnTransaction; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; +import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.CriterionSatisfaction; import org.navalplanner.business.resources.entities.CriterionWithItsType; @@ -88,7 +89,7 @@ public class CriterionServiceTest { PredefinedCriterionTypes.WORK_RELATIONSHIP).size(), equalTo(initial + 1)); criterion.setActive(false); - String newName = "prueba"; + String newName = UUID.randomUUID().toString() + "random"; criterion.setName(newName); criterionService.save(criterion); assertThat("after editing there are the same", criterionService @@ -99,18 +100,29 @@ public class CriterionServiceTest { criterionService.remove(criterion); } - @Test(expected = Exception.class) - public void testUniqueNameForCriterion() { + @Test + public void testSaveCriterionTwice() throws ValidationException { String unique = UUID.randomUUID().toString(); Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP .createCriterion(unique); criterionService.save(criterion); - criterionService.save(PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique)); + criterionService.save(criterion); + } + + @Test(expected=ValidationException.class) + @NotTransactional + public void testCannotExistTwoDifferentCriterionsWithSameNameAndType() throws ValidationException { + String unique = UUID.randomUUID().toString(); + Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP + .createCriterion(unique); + criterionService.save(criterion); + Criterion criterion2 = PredefinedCriterionTypes.WORK_RELATIONSHIP + .createCriterion(unique); + criterionService.save(criterion2); } @Test - public void testCreateIfNotExists() { + public void testCreateIfNotExists() throws ValidationException { String unique = UUID.randomUUID().toString(); Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP .createCriterion(unique); @@ -186,7 +198,7 @@ public class CriterionServiceTest { } @Test - public void testGetSetOfResourcesSubclassSatisfyingCriterion() { + public void testGetSetOfResourcesSubclassSatisfyingCriterion() throws ValidationException { Criterion criterion = CriterionDAOTest.createValidCriterion(); criterionService.save(criterion); ICriterionType type = createTypeThatMatches(criterion); @@ -203,7 +215,7 @@ public class CriterionServiceTest { } @Test - public void shouldLetCreateCriterionOnData() { + public void shouldLetCreateCriterionOnData() throws ValidationException { Criterion criterion = CriterionDAOTest.createValidCriterion(); ICriterionType type = createTypeThatMatches(criterion); criterionService.save(criterion); @@ -260,7 +272,7 @@ public class CriterionServiceTest { @Test @NotTransactional - public void shouldntThrowExceptionDueToTransparentProxyGotcha() { + public void shouldntThrowExceptionDueToTransparentProxyGotcha() throws ValidationException { Criterion criterion = CriterionDAOTest.createValidCriterion(); ICriterionType type = createTypeThatMatches(criterion); criterionService.save(criterion); @@ -274,7 +286,7 @@ public class CriterionServiceTest { } @Test(expected = IllegalArgumentException.class) - public void mustBeCorrectInterval() { + public void mustBeCorrectInterval() throws ValidationException { Criterion criterion = CriterionDAOTest.createValidCriterion(); criterionService.save(criterion); criterionService.getResourcesSatisfying(criterion, diff --git a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionAdminController.java b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionAdminController.java index 913d76aca..6ad195dd8 100644 --- a/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionAdminController.java +++ b/navalplanner-webapp/src/main/java/org/navalplanner/web/resources/criterion/CriterionAdminController.java @@ -2,6 +2,8 @@ package org.navalplanner.web.resources.criterion; import java.util.List; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.ICriterionType; @@ -31,6 +33,8 @@ import org.zkoss.zul.api.Group; */ public class CriterionAdminController extends GenericForwardComposer { + private static final Log log = LogFactory.getLog(CriterionAdminController.class); + private ICriterionsModel criterionsModel; private Component messagesContainer; 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 43fab7e9b..9b30bbc0a 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 @@ -5,6 +5,8 @@ import java.util.Collection; import java.util.List; import org.apache.commons.lang.Validate; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.hibernate.validator.ClassValidator; import org.hibernate.validator.InvalidValue; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; @@ -31,6 +33,8 @@ import org.springframework.transaction.annotation.Transactional; @Scope(BeanDefinition.SCOPE_PROTOTYPE) public class CriterionsModel implements ICriterionsModel { + private static final Log log = LogFactory.getLog(CriterionsModel.class); + private ClassValidator criterionValidator = new ClassValidator( Criterion.class); @@ -88,15 +92,20 @@ public class CriterionsModel implements ICriterionsModel { } @Override - @Transactional public void saveCriterion() throws ValidationException { InvalidValue[] invalidValues = criterionValidator .getInvalidValues(criterion); if (invalidValues.length > 0) throw new ValidationException(invalidValues); - criterionService.save(criterion); - criterion = null; - criterionType = null; + + try { + criterionService.save(criterion); + } catch (ValidationException ve) { + throw ve; + } finally { + criterion = null; + criterionType = null; + } } @Override