From cbdcd2a62dfaf1df5fe6da81fe235b69f213d841 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93scar=20Gonz=C3=A1lez=20Fern=C3=A1ndez?= Date: Wed, 12 Aug 2009 11:56:02 +0200 Subject: [PATCH] ItEr21S04ArquitecturaServidorItEr20S04: When a criterion is saved the type must exist. Previously the criterion type existence associated to the criterion was checked, and if not existed it was saved. Now it requires the criterion type to exist at database. Broken tests by this change fixed. Some refactorings were introduced at the tests to increase readability. --- .../resources/criterion/CriterionsModel.java | 15 +- .../web/orders/OrderModelTest.java | 41 ++- .../web/resources/CriterionModelTest.java | 264 ++++++------------ 3 files changed, 115 insertions(+), 205 deletions(-) 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; - } - }; - } - }