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 78b783ff8..c49bf5ab7 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 @@ -101,11 +101,8 @@ public class CriterionsModel implements ICriterionsModel { .getInvalidValues(criterion); if (invalidValues.length > 0) throw new ValidationException(invalidValues); - try { save(criterion); - } catch (ValidationException ve) { - throw ve; } finally { criterion = null; criterionType = null; @@ -115,11 +112,6 @@ public class CriterionsModel implements ICriterionsModel { @Override @Transactional public void save(Criterion entity) throws ValidationException { - // Save criterion.type if it's new - CriterionType criterionType = entity.getType(); - if (criterionType.getId() == null) { - entity.setType(saveCriterionType(criterionType)); - } if (thereIsOtherWithSameNameAndType(entity)) { InvalidValue[] invalidValues = { new InvalidValue(entity.getName() + " already exists", Criterion.class, "name", entity @@ -211,8 +203,8 @@ public class CriterionsModel implements ICriterionsModel { public void activateAll(Collection resources) { for (Resource resource : resources) { Resource reloaded = find(resource.getId()); - reloaded - .addSatisfaction(new CriterionWithItsType(criterionType, criterion)); + reloaded.addSatisfaction(new CriterionWithItsType(criterionType, + criterion)); resourceService.saveResource(reloaded); } } @@ -222,8 +214,7 @@ public class CriterionsModel implements ICriterionsModel { public void deactivateAll(Collection resources) { for (Resource resource : resources) { Resource reloaded = find(resource.getId()); - reloaded.finish(new CriterionWithItsType(criterionType, - criterion)); + reloaded.finish(new CriterionWithItsType(criterionType, criterion)); resourceService.saveResource(reloaded); } } diff --git a/navalplanner-webapp/src/test/java/org/navalplanner/web/orders/OrderModelTest.java b/navalplanner-webapp/src/test/java/org/navalplanner/web/orders/OrderModelTest.java index 89ae60f66..759d71161 100644 --- a/navalplanner-webapp/src/test/java/org/navalplanner/web/orders/OrderModelTest.java +++ b/navalplanner-webapp/src/test/java/org/navalplanner/web/orders/OrderModelTest.java @@ -35,6 +35,7 @@ import org.navalplanner.business.orders.entities.OrderLineGroup; import org.navalplanner.business.planner.entities.Task; import org.navalplanner.business.planner.entities.TaskElement; import org.navalplanner.business.planner.entities.TaskGroup; +import org.navalplanner.business.resources.daos.ICriterionTypeDAO; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.CriterionType; import org.navalplanner.web.resources.criterion.ICriterionsModel; @@ -46,14 +47,12 @@ import org.springframework.transaction.annotation.Transactional; /** * Tests for {@link OrderModel}.
- * * @author Óscar González Fernández * @author Manuel Rego Casasnovas */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration(locations = { BUSINESS_SPRING_CONFIG_FILE, - WEBAPP_SPRING_CONFIG_FILE, - WEBAPP_SPRING_CONFIG_TEST_FILE }) + WEBAPP_SPRING_CONFIG_FILE, WEBAPP_SPRING_CONFIG_TEST_FILE }) @Transactional public class OrderModelTest { @@ -80,6 +79,9 @@ public class OrderModelTest { @Autowired private IOrderDAO orderDAO; + @Autowired + private ICriterionTypeDAO criterionTypeDAO; + @Autowired private SessionFactory sessionFactory; @@ -89,6 +91,8 @@ public class OrderModelTest { @Autowired private ICriterionsModel criterionModel; + private Criterion criterion; + private Session getSession() { return sessionFactory.getCurrentSession(); } @@ -286,6 +290,7 @@ public class OrderModelTest { @Test @NotTransactional public void testManyToManyHoursGroupCriterionMapping() throws Exception { + givenCriterion(); final Order order = createValidOrder(); OrderLine orderLine = OrderLine.create(); @@ -301,11 +306,6 @@ public class OrderModelTest { orderLine.addHoursGroup(hoursGroup); orderLine.addHoursGroup(hoursGroup2); - CriterionType criterionType = new CriterionType("test"); - Criterion criterion = new Criterion("Test" + UUID.randomUUID(), - criterionType); - criterionModel.save(criterion); - hoursGroup.addCriterion(criterion); hoursGroup2.addCriterion(criterion); @@ -333,7 +333,8 @@ public class OrderModelTest { Criterion criterion = criterions.iterator().next(); - assertThat(criterion.getType().getName(), equalTo("test")); + assertThat(criterion.getName(), + equalTo(OrderModelTest.this.criterion.getName())); } catch (InstanceNotFoundException e) { throw new RuntimeException(e); } @@ -343,6 +344,28 @@ public class OrderModelTest { } + private void givenCriterion() throws ValidationException { + this.criterion = adHocTransaction + .onTransaction(new IOnTransaction() { + + @Override + public Criterion execute() { + // TODO Auto-generated method stub + CriterionType criterionType = new CriterionType("test" + + UUID.randomUUID()); + criterionTypeDAO.save(criterionType); + Criterion criterion = new Criterion("Test" + + UUID.randomUUID(), criterionType); + try { + criterionModel.save(criterion); + } catch (ValidationException e) { + throw new RuntimeException(e); + } + return criterion; + } + }); + } + @Test(expected = ValidationException.class) public void testAtLeastOneHoursGroup() throws Exception { Order order = createValidOrder(); diff --git a/navalplanner-webapp/src/test/java/org/navalplanner/web/resources/CriterionModelTest.java b/navalplanner-webapp/src/test/java/org/navalplanner/web/resources/CriterionModelTest.java index e1da9a315..57c1c8538 100644 --- a/navalplanner-webapp/src/test/java/org/navalplanner/web/resources/CriterionModelTest.java +++ b/navalplanner-webapp/src/test/java/org/navalplanner/web/resources/CriterionModelTest.java @@ -8,10 +8,14 @@ import static org.navalplanner.web.WebappGlobalNames.WEBAPP_SPRING_CONFIG_FILE; import static org.navalplanner.web.test.WebappGlobalNames.WEBAPP_SPRING_CONFIG_TEST_FILE; import java.util.Collection; +import java.util.List; import java.util.UUID; +import javax.management.RuntimeErrorException; + import org.hibernate.SessionFactory; import org.hibernate.validator.InvalidStateException; +import org.junit.Assert; import org.junit.Assume; import org.junit.Test; import org.junit.runner.RunWith; @@ -20,6 +24,7 @@ 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; +import org.navalplanner.business.resources.daos.ICriterionTypeDAO; import org.navalplanner.business.resources.entities.Criterion; import org.navalplanner.business.resources.entities.CriterionSatisfaction; import org.navalplanner.business.resources.entities.CriterionType; @@ -40,7 +45,6 @@ import org.springframework.transaction.annotation.Transactional; /** * Tests for {@link CriterionsModel}.
- * * @author Óscar González Fernández * @author Manuel Rego Casasnovas */ @@ -63,16 +67,24 @@ public class CriterionModelTest { private ICriterionDAO criterionDAO; @Autowired - private IResourceService resourceService; + private ICriterionTypeDAO criterionTypeDAO; + + private Criterion criterion; @Test(expected = InvalidStateException.class) - public void testCantSaveCriterionWithoutNameAndType() throws Exception { - Criterion criterion = createValidCriterion("valido"); + public void cantSaveCriterionWithoutName() throws Exception { + givenValidCriterion(); criterion.setName(""); criterionModel.save(criterion); sessionFactory.getCurrentSession().flush(); } + private Criterion givenValidCriterion() { + criterion = createValidCriterion("valido"); + criterionTypeDAO.save(criterion.getType()); + return criterion; + } + public static Criterion createValidCriterion() { return createValidCriterion(UUID.randomUUID().toString()); } @@ -93,33 +105,51 @@ public class CriterionModelTest { } @Test - public void testAddCriterion() throws Exception { - String unique = UUID.randomUUID().toString(); - Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); + public void savingCriterionIncreasesTheNumberOfCriterions() + throws Exception { + givenValidCriterionFor(PredefinedCriterionTypes.WORK_RELATIONSHIP); + int initial = getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP); criterionModel.save(criterion); + criterionDAO.flush(); + assertThat( + getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP), + equalTo(initial + 1)); + } + + private Criterion givenValidCriterionFor(PredefinedCriterionTypes type) { + return givenValidCriterionFor(type, UUID.randomUUID().toString()); + } + + private Criterion givenValidCriterionFor(PredefinedCriterionTypes type, + String name) { + this.criterion = type.createCriterion(name); + this.criterion + .setType(ensureExists(CriterionType.asCriterionType(type))); + return this.criterion; + } + + private CriterionType ensureExists(CriterionType transientType) { + List found = criterionTypeDAO.findByName(transientType); + if (!found.isEmpty()) + return found.get(0); + criterionTypeDAO.save(transientType); + return criterionTypeDAO.findByName(transientType).get(0); } @Test @NotTransactional - public void testEditingCriterion() throws Exception { - String unique = UUID.randomUUID().toString(); - final Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); - int initial = getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP); - criterionModel.save(criterion); - assertThat( - "after saving one more", - getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP), - equalTo(initial + 1)); - criterion.setActive(false); + public void modificationsAreSaved() throws Exception { + adHocTransactionService.onTransaction(new IOnTransaction() { + + @Override + public Void execute() { + givenCreatedCriterionFor(PredefinedCriterionTypes.WORK_RELATIONSHIP); + return null; + } + }); String newName = UUID.randomUUID().toString() + "random"; criterion.setName(newName); criterionModel.save(criterion); - assertThat( - "after editing there are the same", - getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP), - equalTo(initial + 1)); Criterion retrieved = adHocTransactionService .onTransaction(new IOnTransaction() { @@ -132,192 +162,58 @@ public class CriterionModelTest { } } }); - assertThat(retrieved.getName(), equalTo(newName)); + } - adHocTransactionService.onTransaction(new IOnTransaction() { + @Test + public void modifyingDontAlterTheNumberOfCriterions() throws Exception { + givenCreatedCriterionFor(PredefinedCriterionTypes.WORK_RELATIONSHIP); + int initial = getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP); + String newName = UUID.randomUUID().toString() + "random"; + criterion.setName(newName); + criterionModel.save(criterion); + assertThat( + getCriterionsNumber(PredefinedCriterionTypes.WORK_RELATIONSHIP), + equalTo(initial)); + } - @Override - public Void execute() { - if (criterion.getId() != null) { - try { - criterionDAO.remove(criterion.getId()); - } catch (InstanceNotFoundException e) { - throw new RuntimeException(e); - } - } else { - criterionDAO.removeByNameAndType(criterion); - } - return null; - } - }); + private void givenCreatedCriterionFor(PredefinedCriterionTypes type) { + givenValidCriterionFor(type); + try { + criterionModel.save(criterion); + } catch (ValidationException e) { + throw new RuntimeException(e); + } } private int getCriterionsNumber(final ICriterionType type) { - int size = adHocTransactionService + return adHocTransactionService .onTransaction(new IOnTransaction() { @Override public Integer execute() { return criterionDAO.findByType(type).size(); } - }); - return size; + }).intValue(); } @Test - public void testSaveSameCriterionTwice() throws ValidationException { - String unique = UUID.randomUUID().toString(); - Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); + public void theSameCriterionCanBeSavedTwice() throws ValidationException { + givenValidCriterion(); criterionModel.save(criterion); criterionModel.save(criterion); } - @Test - public void testCreateIfNotExists() throws ValidationException { - String unique = UUID.randomUUID().toString(); - Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); - if (!(criterionDAO.exists(criterion.getId()) || criterionDAO - .existsByNameAndType(criterion))) - criterionModel.save(criterion); - assertTrue(criterionDAO.exists(criterion.getId()) - || criterionDAO.existsByNameAndType(criterion)); - if (!(criterionDAO.exists(criterion.getId()) || criterionDAO - .existsByNameAndType(criterion))) - criterionModel.save(PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique)); - } - @Test(expected = ValidationException.class) - @NotTransactional public void twoDifferentCriterionsWithSameNameAndTypeAreDetectedIfPossible() throws ValidationException { String unique = UUID.randomUUID().toString(); - Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); + Criterion criterion = givenValidCriterionFor( + PredefinedCriterionTypes.WORK_RELATIONSHIP, unique); criterionModel.save(criterion); - Criterion criterion2 = PredefinedCriterionTypes.WORK_RELATIONSHIP - .createCriterion(unique); + Criterion criterion2 = givenValidCriterionFor( + PredefinedCriterionTypes.WORK_RELATIONSHIP, unique); criterionModel.save(criterion2); } - public static class ResourceTest extends Resource { - - @Override - public int getDailyCapacity() { - return 0; - } - - @Override - public String getDescription() { - return ""; - } - - } - - @Test - @NotTransactional - public void testCriterionIsEquivalentOnDetachedAndProxifiedCriterion() - throws Exception { - final Worker worker1 = new Worker("worker-1", "worker-2-surname", - "11111111A", 8); - resourceService.saveResource(worker1); - Criterion criterion = createValidCriterion(); - criterionModel.save(criterion); - createTypeThatMatches(criterion); - worker1.addSatisfaction(new CriterionWithItsType(criterion.getType(), - criterion)); - resourceService.saveResource(worker1); - Resource workerReloaded = adHocTransactionService - .onTransaction(new IOnTransaction() { - - @Override - public Resource execute() { - try { - Resource result = resourceService - .findResource(worker1.getId()); - forceLoadSatisfactions(result); - return result; - } catch (InstanceNotFoundException e) { - throw new RuntimeException(e); - } - } - }); - Collection satisfactionsFor = workerReloaded - .getSatisfactionsFor(criterion.getType()); - Criterion reloadedCriterion = satisfactionsFor.iterator().next() - .getCriterion(); - Assume.assumeTrue(!reloadedCriterion.getClass().equals( - criterion.getClass())); - assertTrue(reloadedCriterion.isEquivalent(criterion)); - } - - private void forceLoadSatisfactions(Resource resource) { - for (CriterionSatisfaction criterionSatisfaction : resource - .getAllSatisfactions()) { - criterionSatisfaction.getCriterion().getName(); - criterionSatisfaction.getCriterion().getType().getName(); - } - } - - private static ICriterionType createTypeThatMatches( - final Criterion criterion) { - return createTypeThatMatches(false, criterion); - } - - private static ICriterionType createTypeThatMatches( - final boolean allowSimultaneousCriterionsPerResource, - final Criterion criterion) { - return new ICriterionType() { - - @Override - public boolean allowSimultaneousCriterionsPerResource() { - return allowSimultaneousCriterionsPerResource; - } - - @Override - public boolean allowHierarchy() { - return false; - } - - @Override - public boolean contains(ICriterion c) { - return criterion.isEquivalent(c); - } - - @Override - public Criterion createCriterion(String name) { - return null; - } - - @Override - public String getName() { - return null; - } - - @Override - public boolean allowAdding() { - return false; - } - - @Override - public boolean allowEditing() { - return false; - } - - @Override - public boolean criterionCanBeRelatedTo( - Class klass) { - return true; - } - - @Override - public Criterion createCriterionWithoutNameYet() { - return null; - } - }; - } - }