Reworked the Cargo-Itinerary relationship OR-mapping to make it possible to move the full responsibility for purging orphaned itineraries to the implementation of the Cargo repository, making it a concern of the infrastructure layer.

Also added explicit names for foreign key constraints to mapping files.

dddsample/src/main/java/se/citerus/dddsample/domain/Cargo.java

@@ -96,19 +96,25 @@ public final class Cargo implements Entity<Cargo> {
   }
 
   /**
-   * Assigns a new itinerary to this cargo.
+   * Attach a new itinerary to this cargo.
    *
-   * @param itinerary an itinerary. Mat not be null.
+   * @param itinerary an itinerary. May not be null.
    */
   public void attachItinerary(final Itinerary itinerary) {
     Validate.notNull(itinerary);
+
+    // Decouple the old itinerary from this cargo 
+    itinerary().setCargo(null);
+    // Couple this cargo and the new itinerary
     this.itinerary = itinerary;
+    itinerary().setCargo(this);
   }
 
   /**
    * Detaches the current itinerary from the cargo.
    */
   public void detachItinerary() {
+    itinerary().setCargo(null);
     this.itinerary = null;
   }
 

dddsample/src/main/java/se/citerus/dddsample/domain/Itinerary.java

@@ -11,6 +11,7 @@ import java.util.List;
  */
 public final class Itinerary implements ValueObject<Itinerary> {
 
+  private Cargo cargo;
   private List<Leg> legs = Collections.emptyList();
 
   static final Itinerary EMPTY_ITINERARY = new Itinerary();
@@ -28,7 +29,20 @@ public final class Itinerary implements ValueObject<Itinerary> {
   }
 
   /**
-   * @return the legs of this itinerary
+   * For maintaing referential integrity inside the Cargo aggregate,
+   * with package level visibility. 
+   *
+   * @see Cargo#attachItinerary(Itinerary)
+   * @see Cargo#detachItinerary()
+   *  
+   * @param cargo the cargo that this itinerary is for
+   */
+  void setCargo(Cargo cargo) {
+    this.cargo = cargo;
+  }
+  
+  /**
+   * @return the legs of this itinerary, as an <b>immutable</b> list.
    */
   public List<Leg> legs() {
     return legs;

dddsample/src/main/java/se/citerus/dddsample/repository/CargoRepositoryHibernate.java

@@ -43,7 +43,15 @@ public class CargoRepositoryHibernate extends HibernateRepository implements Car
   }
 
   public void save(Cargo cargo) {
-    getSession().saveOrUpdate(cargo);
+    getSession().persist(cargo);
+
+    // Delete orphaned itineraries
+    final List<Itinerary> orphans = getSession().
+      createQuery("from Itinerary where cargo = null").
+      list();
+    for (Itinerary orphan : orphans) {
+      getSession().delete(orphan);
+    }
   }
 
   public TrackingId nextTrackingId() {

dddsample/src/main/java/se/citerus/dddsample/service/BookingServiceImpl.java

@@ -71,13 +71,6 @@ public final class BookingServiceImpl implements BookingService {
       throw new IllegalArgumentException("Can't assign itinerary to non-existing cargo " + trackingId);
     }
 
-    // TODO try to leave as much of orphan deletion as possible to the OR mapper - rethink mapping, or implement transparent callback
-    
-    // Delete orphaned itinerary - it's just a value object
-    final Itinerary oldItinerary = cargo.itinerary();
-    cargoRepository.deleteItinerary(oldItinerary);
-    cargo.detachItinerary();
-
     // Assign the new itinerary to the cargo
     cargo.attachItinerary(newItinerary);
     cargoRepository.save(cargo);

dddsample/src/main/java/se/citerus/dddsample/util/SampleDataGenerator.java

@@ -117,16 +117,16 @@ public class SampleDataGenerator implements ServletContextListener {
 
   private static void loadCargoData(JdbcTemplate jdbcTemplate) {
     String cargoSql =
-      "insert into Cargo (id, tracking_id, origin_id, destination_id, itinerary_id) " +
-      "values (?, ?, ?, ?, ?)";
+      "insert into Cargo (id, tracking_id, origin_id, destination_id) " +
+      "values (?, ?, ?, ?)";
 
     Object[][] cargoArgs = {
-      {1, "XYZ", 1, 2, null},
-      {2, "ABC", 1, 5, null},
-      {3, "ZYX", 2, 1, null},
-      {4, "CBA", 5, 1, null},
-      {5, "FGH", 3, 5, 1},
-      {6, "JKL", 6, 4, 2}
+      {1, "XYZ", 1, 2},
+      {2, "ABC", 1, 5},
+      {3, "ZYX", 2, 1},
+      {4, "CBA", 5, 1},
+      {5, "FGH", 3, 5},
+      {6, "JKL", 6, 4}
     };
     executeUpdate(jdbcTemplate, cargoSql, cargoArgs);
   }
@@ -149,11 +149,11 @@ public class SampleDataGenerator implements ServletContextListener {
   }
 
   private static void loadItineraryData(JdbcTemplate jdbcTemplate) {
-    String itinerarySql = "insert into Itinerary (id) values (?)";
+    String itinerarySql = "insert into Itinerary (id, cargo_id) values (?,?)";
 
     Object[][] itineraryArgs = {
-      {1},
-      {2}
+      {1, 5},
+      {2, 6}
     };
     executeUpdate(jdbcTemplate, itinerarySql, itineraryArgs);
 
@@ -188,8 +188,8 @@ public class SampleDataGenerator implements ServletContextListener {
       protected void doInTransactionWithoutResult(TransactionStatus status) {
         loadLocationData(jdbcTemplate);
         loadCarrierMovementData(jdbcTemplate);
-        loadItineraryData(jdbcTemplate);
         loadCargoData(jdbcTemplate);
+        loadItineraryData(jdbcTemplate);
         loadHandlingEventData(jdbcTemplate);
       }
     });

dddsample/src/main/resources/se/citerus/dddsample/domain/Cargo.hbm.xml

@@ -12,8 +12,8 @@
     <component name="trackingId" unique="true" update="false">
       <property name="id" column="tracking_id"/>
     </component>
-    <many-to-one name="origin" column="origin_id" cascade="none" fetch="join" update="false"/>
-    <many-to-one name="destination" column="destination_id" cascade="none" fetch="join"/>
-    <many-to-one name="itinerary" column="itinerary_id" cascade="all" fetch="join"/>
+    <many-to-one name="origin" column="origin_id" cascade="none" fetch="join" update="false" foreign-key="origin_fk"/>
+    <many-to-one name="destination" column="destination_id" cascade="none" fetch="join" foreign-key="destination_fk"/>
+    <one-to-one name="itinerary" property-ref="cargo" cascade="all" fetch="join"/>
   </class>
 </hibernate-mapping>

dddsample/src/main/resources/se/citerus/dddsample/domain/CarrierMovement.hbm.xml

@@ -12,7 +12,7 @@
     <component name="carrierMovementId" update="false">
       <property name="id" column="carrier_movement_id" not-null="true"/>
     </component>
-    <many-to-one name="from" column="from_id" not-null="true" update="false"/>
-    <many-to-one name="to" column="to_id" not-null="true" update="false"/>
+    <many-to-one name="from" column="from_id" not-null="true" update="false" foreign-key="from_location_fk" fetch="join"/>
+    <many-to-one name="to" column="to_id" not-null="true" update="false" foreign-key="to_location_fk" fetch="join"/>
   </class>
 </hibernate-mapping>

dddsample/src/main/resources/se/citerus/dddsample/domain/HandlingEvent.hbm.xml

@@ -9,9 +9,9 @@
     <id name="id" column="id">
       <generator class="org.hibernate.id.IdentityGenerator"/>
     </id>
-    <many-to-one name="carrierMovement" column="carrierMovement_id" not-null="false" cascade="none"/>
-    <many-to-one name="location" column="location_id" not-null="true" cascade="none"/>
-    <many-to-one name="cargo" column="cargo_id" not-null="true" cascade="none"/>
+    <many-to-one name="carrierMovement" column="carrierMovement_id" not-null="false" cascade="none" foreign-key="carrier_movement_fk"/>
+    <many-to-one name="location" column="location_id" not-null="true" cascade="none" foreign-key="location_fk"/>
+    <many-to-one name="cargo" column="cargo_id" not-null="true" cascade="none" foreign-key="cargo_fk"/>
     <property name="completionTime" column="completionTime" not-null="true"/>
     <property name="registrationTime" column="registrationTime" not-null="true"/>
     <property name="type" column="type" not-null="true">

dddsample/src/main/resources/se/citerus/dddsample/domain/Itinerary.hbm.xml

@@ -10,9 +10,10 @@
       <generator class="org.hibernate.id.IdentityGenerator"/>
     </id>
     <list name="legs" cascade="all">
-      <key column="itinerary_id"/>
+      <key column="itinerary_id" foreign-key="itinerary_fk"/>
       <index column="leg_index"/>
       <one-to-many class="se.citerus.dddsample.domain.Leg"/>
     </list>
+    <many-to-one name="cargo" column="cargo_id" cascade="none" foreign-key="cargo_fk" not-null="false"/>
   </class>
 </hibernate-mapping>

dddsample/src/main/resources/se/citerus/dddsample/domain/Leg.hbm.xml

@@ -9,8 +9,8 @@
     <id name="id" column="id">
       <generator class="org.hibernate.id.IdentityGenerator"/>
     </id>
-    <many-to-one name="carrierMovement" column="carrierMovement_id" fetch="join" cascade="none"/>
-    <many-to-one name="from" column="from_id" fetch="join" cascade="none"/>
-    <many-to-one name="to" column="to_id" fetch="join" cascade="none"/>
+    <many-to-one name="carrierMovement" column="carrierMovement_id" fetch="join" cascade="none" foreign-key="carrier_movement_fk"/>
+    <many-to-one name="from" column="from_id" fetch="join" cascade="none" foreign-key="from_location_fk"/>
+    <many-to-one name="to" column="to_id" fetch="join" cascade="none" foreign-key="to_location_fk"/>
   </class>
 </hibernate-mapping>

dddsample/src/test/java/se/citerus/dddsample/repository/AbstractRepositoryTest.java

@@ -48,6 +48,7 @@ public abstract class AbstractRepositoryTest extends AbstractTransactionalDataSo
     return sessionFactory.getCurrentSession();
   }
 
+  // Instead of exposing a getId() on persistent classes
   protected Long getLongId(Object o) {
     if (getSession().contains(o)) {
       return (Long) getSession().getIdentifier(o);

dddsample/src/test/java/se/citerus/dddsample/repository/CargoRepositoryTest.java

@@ -15,6 +15,7 @@ public class CargoRepositoryTest extends AbstractRepositoryTest {
 
   CargoRepository cargoRepository;
   LocationRepository locationRepository;
+  CarrierMovementRepository carrierMovementRepository;
 
   public void setCargoRepository(CargoRepository cargoRepository) {
     this.cargoRepository = cargoRepository;
@@ -24,6 +25,10 @@ public class CargoRepositoryTest extends AbstractRepositoryTest {
     this.locationRepository = locationRepository;
   }
 
+  public void setCarrierMovementRepository(CarrierMovementRepository carrierMovementRepository) {
+    this.carrierMovementRepository = carrierMovementRepository;
+  }
+
   public void testFindByCargoId() {
     Cargo cargo = cargoRepository.find(new TrackingId("FGH"));
     assertEquals(HONGKONG, cargo.origin());
@@ -103,13 +108,45 @@ public class CargoRepositoryTest extends AbstractRepositoryTest {
     assertNull(map.get("ITINERARY_ID"));
   }
 
-  public void testSaveShouldNotCascadeToHandlingEvents() {
-    /* TODO:
-       this test indicates that the addEvent/addEvents methods on DeliveryHistory
-       are somewhat unintuitive, since added events are not cascade-savded with the cargo.
-       Also, it's not really needed except when loading a cargo, so perhaps something like
-       Cargo.attachDeliveryHistory() would be better? */
+  public void testDeleteOrphanedItinerary() {
+    Cargo cargo = cargoRepository.find(new TrackingId("FGH"));
+    Long itineraryId = getLongId(cargo.itinerary());
+
+    assertEquals(1, sjt.queryForInt("select count(*) from Itinerary where id = ?", itineraryId));
+
+    cargo.detachItinerary();
+    cargoRepository.save(cargo);
+    flush();
+
+    // Repository is responsible for deleting orphaned, detached itineraries
+    assertEquals(0, sjt.queryForInt("select count(*) from Itinerary where id = ?", itineraryId));
+  }
 
+  public void testReplaceItinerary() {
+    Cargo cargo = cargoRepository.find(new TrackingId("FGH"));
+    Long oldItineraryId = getLongId(cargo.itinerary());
+    assertEquals(1, sjt.queryForInt("select count(*) from Itinerary where id = ?", oldItineraryId));
+
+    CarrierMovement cm = carrierMovementRepository.find(new CarrierMovementId("CAR_006"));
+    Location legFrom = locationRepository.find(new UnLocode("FIHEL"));
+    Location legTo = locationRepository.find(new UnLocode("DEHAM"));
+    Itinerary newItinerary = new Itinerary(Arrays.asList(new Leg(cm, legFrom, legTo)));
+
+    cargo.attachItinerary(newItinerary);
+
+    cargoRepository.save(cargo);
+    flush();
+
+    // Old itinerary should be deleted
+    assertEquals(0, sjt.queryForInt("select count(*) from Itinerary where id = ?", oldItineraryId));
+
+    // New itinerary should be cascade-saved
+    Long newItineraryId = getLongId(cargo.itinerary());
+    assertEquals(1, sjt.queryForInt("select count(*) from Itinerary where id = ?", newItineraryId));
+  }
+
+
+  public void testSaveShouldNotCascadeToHandlingEvents() {
     Cargo cargo = cargoRepository.find(new TrackingId("FGH"));
     int eventCount = cargo.deliveryHistory().eventsOrderedByCompletionTime().size();