ItEr09S09AdministracionGrupos: Criterion now has a surrogate key. It's required to enable criterion edition.

navalplanner-business/src/main/java/org/navalplanner/business/resources/bootstrap/CriterionsBootstrap.java

@@ -9,7 +9,6 @@ import java.util.Map.Entry;
 import org.navalplanner.business.resources.entities.Criterion;
 import org.navalplanner.business.resources.entities.ICriterionType;
 import org.navalplanner.business.resources.services.CriterionService;
-import org.navalplanner.business.resources.services.ResourceService;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
@@ -30,9 +29,6 @@ public class CriterionsBootstrap implements ICriterionsBootstrap {
     @Autowired
     private List<ICriterionTypeProvider> providers;
 
-    @Autowired
-    private ResourceService resourceService;
-
     public CriterionsBootstrap() {
     }
 

navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/ICriterionDAO.java

@@ -1,12 +1,20 @@
 package org.navalplanner.business.resources.daos;
 
 import org.navalplanner.business.common.daos.IGenericDao;
+import org.navalplanner.business.common.exceptions.InstanceNotFoundException;
+import org.navalplanner.business.resources.daos.impl.CriterionDAO;
 import org.navalplanner.business.resources.entities.Criterion;
 
 /**
- * Description goes here. <br />
+ * Contract for {@link CriterionDAO} <br />
  * @author Óscar González Fernández <ogonzalez@igalia.com>
  */
-public interface ICriterionDAO extends IGenericDao<Criterion, Criterion> {
+public interface ICriterionDAO extends IGenericDao<Criterion, Long> {
+
+    Criterion findByNameAndType(Criterion criterion);
+
+    boolean existsByNameAndType(Criterion entity);
+
+    Criterion find(Criterion criterion) throws InstanceNotFoundException;
 
 }

navalplanner-business/src/main/java/org/navalplanner/business/resources/daos/impl/CriterionDAO.java

@@ -2,6 +2,7 @@ package org.navalplanner.business.resources.daos.impl;
 
 import org.hibernate.criterion.Restrictions;
 import org.navalplanner.business.common.daos.impl.GenericDaoHibernate;
+import org.navalplanner.business.common.exceptions.InstanceNotFoundException;
 import org.navalplanner.business.resources.daos.ICriterionDAO;
 import org.navalplanner.business.resources.entities.Criterion;
 
@@ -9,15 +10,28 @@ import org.navalplanner.business.resources.entities.Criterion;
  * DAO implementation for Criterion. <br />
  * @author Óscar González Fernández <ogonzalez@igalia.com>
  */
-public class CriterionDAO extends GenericDaoHibernate<Criterion, Criterion>
+public class CriterionDAO extends GenericDaoHibernate<Criterion, Long>
         implements ICriterionDAO {
 
-    @Override
+    public Criterion findByNameAndType(Criterion criterion) {
-    public boolean exists(Criterion criterion) {
+        return (Criterion) getSession().createCriteria(Criterion.class).add(
-        return getSession().createCriteria(Criterion.class).add(
                 Restrictions.eq("name", criterion.getName())).add(
-                Restrictions.eq("type", criterion.getType()))
+                Restrictions.eq("type", criterion.getType())).uniqueResult();
-                .uniqueResult() != null;
+    }
+
+    public boolean existsByNameAndType(Criterion criterion) {
+        return findByNameAndType(criterion) != null;
+    }
+
+    @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());
+        return result;
     }
 
 }

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

@@ -1,18 +1,18 @@
 package org.navalplanner.business.resources.entities;
 
-import java.io.Serializable;
 import java.util.Date;
 
 import org.apache.commons.lang.Validate;
 import org.apache.commons.lang.builder.EqualsBuilder;
-import org.apache.commons.lang.builder.HashCodeBuilder;
 import org.hibernate.validator.NotEmpty;
 
 /**
  * A criterion stored in the database <br />
  * @author Óscar González Fernández <ogonzalez@igalia.com>
  */
-public class Criterion implements ICriterion, Serializable {
+public class Criterion implements ICriterion {
+
+    private Long id;
 
     @SuppressWarnings("unused")
     private long version;
@@ -25,19 +25,34 @@ public class Criterion implements ICriterion, Serializable {
 
     private boolean active = true;
 
+    public static Criterion ofType(String type) {
+        Validate.notEmpty(type);
+        Criterion result = new Criterion();
+        result.type = type;
+        return result;
+    }
+
+    public static Criterion withNameAndType(String name, String type) {
+        return new Criterion(name, type);
+    }
+
     /**
      * Constructor for hibernate. Do not use!
      */
     public Criterion() {
     }
 
-    public Criterion(String name, String type) {
+    private Criterion(String name, String type) {
-        Validate.notNull(name);
+        Validate.notEmpty(name);
-        Validate.notNull(type);
+        Validate.notEmpty(type);
         this.type = type;
         this.name = name;
     }
 
+    public Long getId() {
+        return id;
+    }
+
     @Override
     public boolean isSatisfiedBy(Resource resource) {
         return !resource.getActiveSatisfactionsFor(this).isEmpty();
@@ -52,25 +67,14 @@ public class Criterion implements ICriterion, Serializable {
         return name;
     }
 
+    public void setName(String name) {
+        this.name = name;
+    }
+
     public String getType() {
         return type;
     }
 
-    @Override
-    public int hashCode() {
-        return new HashCodeBuilder().append(name).append(type).toHashCode();
-    }
-
-    @Override
-    public boolean equals(Object obj) {
-        if (obj instanceof Criterion) {
-            Criterion other = (Criterion) obj;
-            return new EqualsBuilder().append(name, other.name).append(type,
-                    other.type).isEquals();
-        }
-        return false;
-    }
-
     public boolean isActive() {
         return active;
     }
@@ -79,4 +83,13 @@ public class Criterion implements ICriterion, Serializable {
         this.active = active;
     }
 
+    public boolean isEquivalent(ICriterion criterion) {
+        if (criterion instanceof Criterion) {
+            Criterion other = (Criterion) criterion;
+            return new EqualsBuilder().append(name, other.name).append(type,
+                    other.type).isEquals();
+        }
+        return false;
+    }
+
 }

navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/ICriterionType.java

@@ -18,8 +18,11 @@ public interface ICriterionType<C extends ICriterion> {
 
     public C createCriterion(String name);
 
+    public C createCriterionWithoutNameYet();
+
     public boolean contains(ICriterion criterion);
 
     public boolean criterionCanBeRelatedTo(Class<? extends Resource> klass);
 
+
 }

navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/PredefinedCriterionTypes.java

@@ -79,7 +79,12 @@ public enum PredefinedCriterionTypes implements ICriterionType<Criterion> {
 
     @Override
     public Criterion createCriterion(String name) {
-        return new Criterion(name, getType());
+        return Criterion.withNameAndType(name, getType());
+    }
+
+    @Override
+    public Criterion createCriterionWithoutNameYet() {
+        return Criterion.ofType(getType());
     }
 
     public abstract List<Criterion> getPredefined();

navalplanner-business/src/main/java/org/navalplanner/business/resources/entities/Resource.java

@@ -77,7 +77,7 @@ public abstract class Resource {
     public List<CriterionSatisfaction> getSatisfactionsFor(ICriterion criterion) {
         ArrayList<CriterionSatisfaction> result = new ArrayList<CriterionSatisfaction>();
         for (CriterionSatisfaction criterionSatisfaction : getAllSatisfactions()) {
-            if (criterionSatisfaction.getCriterion().equals(criterion)) {
+            if (criterionSatisfaction.getCriterion().isEquivalent(criterion)) {
                 result.add(criterionSatisfaction);
             }
         }
@@ -132,7 +132,7 @@ public abstract class Resource {
             ICriterion criterion) {
         Set<CriterionSatisfaction> result = new HashSet<CriterionSatisfaction>();
         for (CriterionSatisfaction satisfaction : criterionSatisfactions) {
-            if (satisfaction.getCriterion().equals(criterion)) {
+            if (satisfaction.getCriterion().isEquivalent(criterion)) {
                 result.add(satisfaction);
             }
         }

navalplanner-business/src/main/java/org/navalplanner/business/resources/services/impl/CriterionServiceImpl.java

@@ -7,6 +7,7 @@ import java.util.List;
 
 import org.apache.commons.lang.Validate;
 import org.navalplanner.business.common.exceptions.InstanceNotFoundException;
+import org.navalplanner.business.resources.daos.ICriterionDAO;
 import org.navalplanner.business.resources.daos.impl.CriterionDAO;
 import org.navalplanner.business.resources.entities.Criterion;
 import org.navalplanner.business.resources.entities.CriterionSatisfaction;
@@ -28,13 +29,14 @@ import org.springframework.transaction.annotation.Transactional;
 public class CriterionServiceImpl implements CriterionService {
 
     @Autowired
-    private CriterionDAO criterionDAO;
+    private ICriterionDAO criterionDAO;
 
     @Autowired
     private ResourceService resourceService;
 
     public boolean exists(Criterion criterion) {
-        return criterionDAO.exists(criterion);
+        return criterionDAO.exists(criterion.getId())
+                || criterionDAO.existsByNameAndType(criterion);
     }
 
     public Criterion find(Criterion criterion) throws InstanceNotFoundException {
@@ -46,10 +48,20 @@ public class CriterionServiceImpl implements CriterionService {
     }
 
     public void remove(Criterion criterion) throws InstanceNotFoundException {
-        criterionDAO.remove(criterion);
+        criterionDAO.remove(criterion.getId());
     }
 
     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());
+        }
         criterionDAO.save(entity);
     }
 
@@ -174,7 +186,7 @@ public class CriterionServiceImpl implements CriterionService {
     @Override
     public Criterion load(Criterion criterion) {
         try {
-            return criterionDAO.find(criterion);
+            return criterionDAO.find(criterion.getId());
         } catch (InstanceNotFoundException e) {
             throw new RuntimeException(e);
         }

navalplanner-business/src/main/resources/org/navalplanner/business/resources/entities/Resources.hbm.xml

@@ -24,11 +24,12 @@
   </class>
 
   <class name="Criterion">
-    <composite-id>
+    <id access="field" name="id">
-        <key-property access="field" name="name"/>
+      <generator class="native"/>
-        <key-property access="field" name="type"/>
+    </id>
-    </composite-id>
     <version access="field" name="version" type="long"/>
+    <property name="name" access="field"/>
+    <property name="type" access="field"/>
     <property access="field" name="active"/>
   </class>
 
@@ -39,10 +40,7 @@
     <version access="field" name="version" type="long"/>
     <property access="field" name="startDate" not-null="true"/>
     <property access="field" name="finishDate"/>
-    <many-to-one name="criterion" access="field" not-null="true" lazy="false">
+    <many-to-one name="criterion" access="field" not-null="true" />
-        <column name="name" not-null="true"></column>
-        <column name="type" not-null="true"></column>
-    </many-to-one>
     <many-to-one access="field" column="resource" name="resource" not-null="true"/>
   </class>
 </hibernate-mapping>

navalplanner-business/src/test/java/org/navalplanner/business/test/resources/bootstrap/CriterionsBootstrapTest.java

@@ -1,5 +1,9 @@
 package org.navalplanner.business.test.resources.bootstrap;
 
+import static org.junit.Assert.assertTrue;
+import static org.navalplanner.business.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_FILE;
+import static org.navalplanner.business.test.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_TEST_FILE;
+
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.navalplanner.business.resources.bootstrap.ICriterionsBootstrap;
@@ -10,10 +14,6 @@ import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.transaction.annotation.Transactional;
 
-import static junit.framework.Assert.assertTrue;
-import static org.navalplanner.business.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_FILE;
-import static org.navalplanner.business.test.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_TEST_FILE;
-
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(locations = { BUSINESS_SPRING_CONFIG_FILE,
         BUSINESS_SPRING_CONFIG_TEST_FILE })

navalplanner-business/src/test/java/org/navalplanner/business/test/resources/daos/CriterionDAOTest.java

@@ -1,5 +1,12 @@
 package org.navalplanner.business.test.resources.daos;
 
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.assertNotNull;
+import static junit.framework.Assert.assertTrue;
+import static org.navalplanner.business.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_FILE;
+import static org.navalplanner.business.test.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_TEST_FILE;
+
 import java.util.List;
 import java.util.UUID;
 
@@ -13,14 +20,6 @@ import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.transaction.annotation.Transactional;
 
-import static junit.framework.Assert.assertEquals;
-import static junit.framework.Assert.assertFalse;
-import static junit.framework.Assert.assertNotNull;
-import static junit.framework.Assert.assertTrue;
-
-import static org.navalplanner.business.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_FILE;
-import static org.navalplanner.business.test.BusinessGlobalNames.BUSINESS_SPRING_CONFIG_TEST_FILE;
-
 /**
  * Test cases for CriterionDAO <br />
  * Created at May 13, 2009
@@ -44,20 +43,19 @@ public class CriterionDAOTest {
     public void testSaveCriterions() throws Exception {
         Criterion criterion = createValidCriterion();
         criterionDAO.save(criterion);
-        assertTrue(criterionDAO.exists(criterion));
+        assertTrue(criterionDAO.exists(criterion.getId()));
     }
 
     public static Criterion createValidCriterion() {
-        return new Criterion(UUID.randomUUID().toString(), "pruebaType");
+        return Criterion.withNameAndType(UUID.randomUUID().toString(), "pruebaType");
     }
 
     @Test
     public void testRemove() throws InstanceNotFoundException {
         Criterion criterion = createValidCriterion();
         criterionDAO.save(criterion);
-        assertTrue(criterionDAO.exists(criterion));
+        criterionDAO.remove(criterion.getId());
-        criterionDAO.remove(criterion);
+        assertFalse(criterionDAO.exists(criterion.getId()));
-        assertFalse(criterionDAO.exists(criterion));
     }
 
     @Test

navalplanner-business/src/test/java/org/navalplanner/business/test/resources/entities/CriterionTest.java

@@ -1,5 +1,11 @@
 package org.navalplanner.business.test.resources.entities;
 
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.navalplanner.business.resources.entities.CriterionCompounder.atom;
+import static org.navalplanner.business.resources.entities.CriterionCompounder.build;
+import static org.navalplanner.business.resources.entities.CriterionCompounder.not;
+
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashSet;
@@ -12,13 +18,6 @@ import org.navalplanner.business.resources.entities.PredefinedCriterionTypes;
 import org.navalplanner.business.resources.entities.Resource;
 import org.navalplanner.business.resources.entities.Worker;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.navalplanner.business.resources.entities.CriterionCompounder.atom;
-import static org.navalplanner.business.resources.entities.CriterionCompounder.build;
-import static org.navalplanner.business.resources.entities.CriterionCompounder.not;
-
 /**
  * Tests for criterion. <br />
  * Created at May 12, 2009
@@ -34,14 +33,6 @@ public class CriterionTest {
                 .contains(firedCriterion));
     }
 
-    @Test
-    public void testCriterionNameAndTypeIsInmutableBusinessKey()
-            throws Exception {
-        Criterion criterion = new Criterion("name", "type");
-        Criterion other = new Criterion("name", "type");
-        assertEquals(criterion.hashCode(), other.hashCode());
-    }
-
     @Test
     public void testCompounding() throws Exception {
         Worker worker1 = new Worker();

navalplanner-business/src/test/java/org/navalplanner/business/test/resources/entities/ResourceTest.java

@@ -1,5 +1,12 @@
 package org.navalplanner.business.test.resources.entities;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
 import java.util.Arrays;
 import java.util.HashSet;
 
@@ -15,13 +22,6 @@ import org.navalplanner.business.resources.entities.Worker;
 import org.navalplanner.business.test.resources.daos.CriterionDAOTest;
 import org.navalplanner.business.test.resources.daos.CriterionSatisfactionDAOTest;
 
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
 /**
  * Tests for {@link Resource}. <br />
  * @author Óscar González Fernández <ogonzalez@igalia.com>
@@ -109,6 +109,11 @@ public class ResourceTest {
                     Class<? extends Resource> klass) {
                 return true;
             }
+
+            @Override
+            public Criterion createCriterionWithoutNameYet() {
+                return null;
+            }
         };
     }
 
@@ -280,6 +285,11 @@ public class ResourceTest {
                     Class<? extends Resource> klass) {
                 return false;
             }
+
+            @Override
+            public Criterion createCriterionWithoutNameYet() {
+                return null;
+            }
         };
         CriterionWithItsType criterionWithItsType = new CriterionWithItsType(
                 type, criterion);

navalplanner-business/src/test/java/org/navalplanner/business/test/resources/services/CriterionServiceTest.java

@@ -15,7 +15,7 @@ import org.hibernate.SessionFactory;
 import org.hibernate.validator.InvalidStateException;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.navalplanner.business.resources.bootstrap.ICriterionsBootstrap;
+import org.navalplanner.business.resources.daos.impl.CriterionDAO;
 import org.navalplanner.business.resources.entities.Criterion;
 import org.navalplanner.business.resources.entities.CriterionWithItsType;
 import org.navalplanner.business.resources.entities.ICriterion;
@@ -56,11 +56,13 @@ public class CriterionServiceTest {
     private SessionFactory sessionFactory;
 
     @Autowired
-    private ICriterionsBootstrap criterionsBootstrap;
+    private CriterionDAO criterionDAO;
+
 
     @Test(expected = InvalidStateException.class)
     public void testCantSaveCriterionWithoutNameAndType() throws Exception {
-        Criterion criterion = new Criterion("", "");
+        Criterion criterion = Criterion.withNameAndType("valido", "valido");
+        criterion.setName("");
         criterionService.save(criterion);
         sessionFactory.getCurrentSession().flush();
     }
@@ -73,6 +75,31 @@ public class CriterionServiceTest {
         criterionService.save(criterion);
     }
 
+    @Test
+    @NotTransactional
+    public void testEditingCriterion() throws Exception {
+        String unique = UUID.randomUUID().toString();
+        Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP
+                .createCriterion(unique);
+        int initial = criterionService.getCriterionsFor(
+                PredefinedCriterionTypes.WORK_RELATIONSHIP).size();
+        criterionService.save(criterion);
+        assertThat("after saving one more", criterionService
+                .getCriterionsFor(
+                PredefinedCriterionTypes.WORK_RELATIONSHIP).size(),
+                equalTo(initial + 1));
+        criterion.setActive(false);
+        String newName = "prueba";
+        criterion.setName(newName);
+        criterionService.save(criterion);
+        assertThat("after editing there are the same", criterionService
+                .getCriterionsFor(PredefinedCriterionTypes.WORK_RELATIONSHIP)
+                .size(), equalTo(initial + 1));
+        Criterion retrieved = criterionService.load(criterion);
+        assertThat(retrieved.getName(), equalTo(newName));
+        criterionService.remove(criterion);
+    }
+
     @Test(expected = Exception.class)
     public void testUniqueNameForCriterion() {
         String unique = UUID.randomUUID().toString();
@@ -95,18 +122,6 @@ public class CriterionServiceTest {
                         .createCriterion(unique));
     }
 
-    @Test
-    public void testPersistingDoesNotChangeEquality() throws Exception {
-        String unique = UUID.randomUUID().toString();
-        Criterion criterion = PredefinedCriterionTypes.WORK_RELATIONSHIP
-                .createCriterion(unique);
-        Criterion other = PredefinedCriterionTypes.WORK_RELATIONSHIP
-                .createCriterion(unique);
-        assertEquals(criterion, other);
-        criterionService.save(criterion);
-        assertEquals(criterion, other);
-    }
-
     @Test(expected = DataIntegrityViolationException.class)
     public void testCreateCriterionSatisfactionOnTransientCriterion()
             throws Exception {
@@ -330,7 +345,7 @@ public class CriterionServiceTest {
 
             @Override
             public boolean contains(ICriterion c) {
-                return criterion == c;
+                return criterion.equals(c);
             }
 
             @Override
@@ -340,19 +355,16 @@ public class CriterionServiceTest {
 
             @Override
             public String getName() {
-                // TODO Auto-generated method stub
                 return null;
             }
 
             @Override
             public boolean allowAdding() {
-                // TODO Auto-generated method stub
                 return false;
             }
 
             @Override
             public boolean allowEditing() {
-                // TODO Auto-generated method stub
                 return false;
             }
 
@@ -361,6 +373,11 @@ public class CriterionServiceTest {
                     Class<? extends Resource> klass) {
                 return true;
             }
+
+            @Override
+            public Criterion createCriterionWithoutNameYet() {
+                return null;
+            }
         };
     }