diff --git a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/CriterionDAO.java b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/CriterionDAO.java index 23fc36906..1a8302c07 100644 --- a/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/CriterionDAO.java +++ b/navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/CriterionDAO.java @@ -14,6 +14,8 @@ import org.navalplanner.business.resources.entities.ICriterionType; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Repository; +import org.springframework.transaction.annotation.Propagation; +import org.springframework.transaction.annotation.Transactional; /** * DAO implementation for Criterion.
@@ -29,6 +31,21 @@ public class CriterionDAO extends GenericDAOHibernate private static final Log log = LogFactory.getLog(CriterionDAO.class); + @Transactional(propagation = Propagation.REQUIRES_NEW, readOnly = true) + public boolean thereIsOtherWithSameNameAndType(Criterion criterion) { + List withSameNameAndType = findByNameAndType(criterion); + if (withSameNameAndType.isEmpty()) + return false; + if (withSameNameAndType.size() > 1) + return true; + return areDifferentInDB(withSameNameAndType.get(0), criterion); + } + + private boolean areDifferentInDB(Criterion existentCriterion, + Criterion other) { + return !existentCriterion.getId().equals(other.getId()); + } + @Override public List findByNameAndType(Criterion criterion) { if (criterion.getType() == null) return new ArrayList(); 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 bdeb428a9..0191f5587 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 @@ -29,4 +29,6 @@ public interface ICriterionDAO extends IGenericDAO { List getAll(); + boolean thereIsOtherWithSameNameAndType(Criterion criterion); + } diff --git a/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/daos/CriterionDAOTest.java b/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/daos/CriterionDAOTest.java index f00e57419..c2b273fc2 100644 --- a/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/daos/CriterionDAOTest.java +++ b/navalplanner-business/src/test/java/org/navalplanner/business/test/resources/daos/CriterionDAOTest.java @@ -11,10 +11,10 @@ import java.util.Collection; import java.util.List; import java.util.UUID; -import org.junit.Assert; import org.junit.Test; -import org.junit.matchers.JUnitMatchers; import org.junit.runner.RunWith; +import org.navalplanner.business.common.IAdHocTransactionService; +import org.navalplanner.business.common.IOnTransaction; import org.navalplanner.business.common.exceptions.InstanceNotFoundException; import org.navalplanner.business.common.exceptions.ValidationException; import org.navalplanner.business.resources.daos.ICriterionDAO; @@ -26,6 +26,7 @@ import org.navalplanner.business.resources.entities.ICriterionType; import org.navalplanner.business.resources.entities.Resource; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataIntegrityViolationException; +import org.springframework.test.annotation.NotTransactional; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.transaction.annotation.Transactional; @@ -46,6 +47,9 @@ public class CriterionDAOTest { @Autowired private ICriterionTypeDAO criterionTypeDAO; + @Autowired + private IAdHocTransactionService transactionService; + private Criterion criterion; @Test @@ -112,6 +116,10 @@ public class CriterionDAOTest { return this.criterion; } + private Criterion givenUniquelyNamedCriterion() { + return givenACriterionWithAnExistentType(); + } + @Test public void listReturnsTheNewlyCreatedCriterions() { int previous = criterionDAO.list(Criterion.class).size(); @@ -141,9 +149,9 @@ public class CriterionDAOTest { } @Test - public void findByTypeOnlyReturnsTheCriterionsMatchedByType(){ + public void findByTypeOnlyReturnsTheCriterionsMatchedByType() { givenASavedCriterionWithAnExistentType(); - //saving another + // saving another givenASavedCriterionWithAnExistentType(); ICriterionType type = createTypeThatMatches(criterion); Collection criterions = criterionDAO.findByType(type); @@ -151,6 +159,55 @@ public class CriterionDAOTest { assertTrue(criterions.contains(criterion)); } + @Test + @NotTransactional + public void thereIsOtherWithSameNameAndTypeWorksIsolatedFromCurrentTransaction() { + transactionService.runOnTransaction(new IOnTransaction() { + + @Override + public Void execute() { + Criterion saved = givenASavedCriterionWithAnExistentType(); + assertFalse(criterionDAO.thereIsOtherWithSameNameAndType(saved)); + return null; + } + }); + } + + @Test + @NotTransactional + public void thereIsNoOtherIfItsTheSame() { + Criterion c = transactionService + .runOnTransaction(new IOnTransaction() { + + @Override + public Criterion execute() { + return givenASavedCriterionWithAnExistentType(); + } + }); + assertFalse(criterionDAO.thereIsOtherWithSameNameAndType(c)); + } + + @Test + @NotTransactional + public void ifItsDifferentThereIsOther() { + Criterion c = transactionService + .runOnTransaction(new IOnTransaction() { + + @Override + public Criterion execute() { + return givenASavedCriterionWithAnExistentType(); + } + }); + Criterion copy = Criterion.create(c.getName(), c.getType()); + assertTrue(criterionDAO.thereIsOtherWithSameNameAndType(copy)); + } + + @Test + public void noOtherIfTheCriterionDoesntExist() { + Criterion criterion = givenUniquelyNamedCriterion(); + assertFalse(criterionDAO.thereIsOtherWithSameNameAndType(criterion)); + } + private static ICriterionType createTypeThatMatches( final Criterion criterion) { return createTypeThatMatches(false, criterion); 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 a762722ef..a9494ca2d 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 @@ -1,5 +1,7 @@ package org.navalplanner.web.resources.criterion; +import static org.navalplanner.web.I18nHelper._; + import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -26,8 +28,6 @@ import org.springframework.context.annotation.Scope; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; -import static org.navalplanner.web.I18nHelper._; - /** * Model for criterions.
* @author Óscar González Fernández @@ -114,7 +114,7 @@ public class CriterionsModel implements ICriterionsModel { @Override @Transactional public void save(Criterion entity) throws ValidationException { - if (thereIsOtherWithSameNameAndType(entity)) { + if (criterionDAO.thereIsOtherWithSameNameAndType(entity)) { InvalidValue[] invalidValues = { new InvalidValue( _("{0} already exists", entity.getName()), @@ -127,20 +127,6 @@ public class CriterionsModel implements ICriterionsModel { criterionDAO.save(entity); } - private boolean thereIsOtherWithSameNameAndType(Criterion toSave) { - List withSameNameAndType = criterionDAO - .findByNameAndType(toSave); - if (withSameNameAndType.isEmpty()) - return false; - if (withSameNameAndType.size() > 1) - return true; - return !areSameInDB(withSameNameAndType.get(0), toSave); - } - - private boolean areSameInDB(Criterion existentCriterion, Criterion other) { - return existentCriterion.getId().equals(other.getId()); - } - @Override public boolean isEditing() { return criterion != null;