Suggestion for handling domain validation as close to each domain object as possible - Location now requires UN locode argument to constructor to match a certain regexp, and each HandlingEvent.Type may answer type.requiresCarrierMovement() and type.prohibitsCarrierMovement(), evaluated from HandlingEvent constructor.

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

@@ -2,8 +2,6 @@ package se.citerus.dddsample.domain;
 
 import org.apache.commons.lang.Validate;
 import org.apache.commons.lang.builder.HashCodeBuilder;
-import org.apache.commons.lang.builder.ToStringBuilder;
-import org.apache.commons.lang.builder.ToStringStyle;
 
 import javax.persistence.*;
 import java.util.Comparator;
@@ -12,16 +10,13 @@ import java.util.Date;
 /**
  * A HandlingEvent is used to register the event when, for instance, a cargo is unloaded from a carrier at a some loacation at a given time. The
  * HandlingEvent's are sent from different Incident Logging Applications some time after the event occured and contain information about the
- * {@link TrackingID}, {@link Location}, timestamp of the completion of the event, and possibly, if applicable a {@link CarrierMovement}.
+ * {@link TrackingId}, {@link Location}, timestamp of the completion of the event, and possibly, if applicable a {@link CarrierMovement}.
  * <br><br>
- * HandlingEvent's could contain information about a {@link CarrierMovement} and if so, the event type must be either {@link Type.LOAD} or 
+ * HandlingEvent's could contain information about a {@link CarrierMovement} and if so, the event type must be either {@link Type.LOAD} or
  * {@link Type.UNLOAD}. All other events must be of {@link Type.RECEIVE}, {@link Type.CLAIM} or {@link Type.CUSTOMS}.
  */
 @Entity
 public class HandlingEvent {
-  private static final Type[] VALID_TYPES_WITH_CARRIERMOVEMENT = new Type[]{Type.LOAD, Type.UNLOAD};
-
-  private static final Type[] VALID_TYPES_NO_CARRIERMOVEMENT = new Type[]{Type.CLAIM, Type.RECEIVE, Type.CUSTOMS};
 
   /**
    * Comparator used to be able to sort HandlingEvents according to their completion time
@@ -54,18 +49,42 @@ public class HandlingEvent {
   private Cargo cargo;
 
   public enum Type {
-    LOAD, UNLOAD, RECEIVE, CLAIM, CUSTOMS
+    LOAD(true),
+    UNLOAD(true),
+    RECEIVE(false),
+    CLAIM(false),
+    CUSTOMS(false);
+
+    private boolean carrierMovementRequired;
+
+    private Type(boolean carrierMovementRequired) {
+      this.carrierMovementRequired = carrierMovementRequired;
+    }
+
+    /**
+     * @return True if a carrier movement association is required for this event type.
+     */
+    public boolean requiresCarrierMovement() {
+      return carrierMovementRequired;
+    }
+
+    /**
+     * @return True if a carrier movement association is prohibited for this event type.
+     */
+    public boolean prohibitsCarrierMovement() {
+      return !requiresCarrierMovement();
+    }
   }
 
 
   /**
    * Constructor for events that do not have a carrier movement associated.
    *
-   * @param cargo cargo
-   * @param completionTime completion time
+   * @param cargo            cargo
+   * @param completionTime   completion time
    * @param registrationTime registration time
-   * @param type type of event. Legal values are CLAIM, RECIEVE and CUSTOMS
-   * @param location where the event took place
+   * @param type             type of event. Legal values are CLAIM, RECIEVE and CUSTOMS
+   * @param location         where the event took place
    */
   public HandlingEvent(Cargo cargo, Date completionTime, Date registrationTime, Type type, Location location) {
     this.registrationTime = registrationTime;
@@ -73,8 +92,8 @@ public class HandlingEvent {
     this.type = type;
     this.cargo = cargo;
     this.location = location;
-    
-    validateType(type, VALID_TYPES_NO_CARRIERMOVEMENT);
+
+    validateType();
   }
 
   /**
@@ -83,12 +102,12 @@ public class HandlingEvent {
    * the location is the starting point of the movement, if the type is UNLOAD the location
    * is the end point.
    *
-   * @param cargo cargo
-   * @param completionTime completion time
+   * @param cargo            cargo
+   * @param completionTime   completion time
    * @param registrationTime registration time
-   * @param type type of event. Legal values are LOAD and UNLOAD
-   * @param location where the event took place
-   * @param carrierMovement carrier movement.
+   * @param type             type of event. Legal values are LOAD and UNLOAD
+   * @param location         where the event took place
+   * @param carrierMovement  carrier movement.
    */
   public HandlingEvent(Cargo cargo, Date completionTime, Date registrationTime, Type type, Location location, CarrierMovement carrierMovement) {
     this.registrationTime = registrationTime;
@@ -97,9 +116,9 @@ public class HandlingEvent {
     this.cargo = cargo;
     this.location = location;
     this.carrierMovement = carrierMovement;
-    
-    validateType(type, VALID_TYPES_WITH_CARRIERMOVEMENT);
+
     Validate.notNull(carrierMovement, "CarrierMovementId must not be null for this type of event");
+    validateType();
   }
 
 
@@ -145,8 +164,8 @@ public class HandlingEvent {
     }
     HandlingEvent other = (HandlingEvent) object;
     return this.location.equals(other.location) &&
-           this.completionTime.equals(other.completionTime) &&
-           this.type.equals(other.type);
+            this.completionTime.equals(other.completionTime) &&
+            this.type.equals(other.type);
   }
 
   /**
@@ -170,21 +189,21 @@ public class HandlingEvent {
   }
 
   /**
-   * Private helper that validate a HandlingEvent type agains an array of valid types
-   * 
-   * @param type The type that should be validated
-   * @param validTypes The list of valid types
+   * Validate that the event type is compatible with the carrier movement value.
+   * <p/>
+   * Only certain types of events may be associated with a carrier movement.
    */
-  private void validateType(Type type, Type[] validTypes) {
-    for (Type validType : validTypes) {
-      if (type.equals(validType)){
-        return;
-      }
+  private void validateType() {
+    if (type.requiresCarrierMovement() && carrierMovement == null) {
+      throw new IllegalArgumentException("Carrier movement is required for event type " + type);
+    }
+    if (type.prohibitsCarrierMovement() && carrierMovement != null) {
+      throw new IllegalArgumentException("Carrier movement is not allowed with event type " + type);
     }
-    throw new IllegalArgumentException("Illegal event type " + type + ". Valid types are: " + ToStringBuilder.reflectionToString(validTypes, ToStringStyle.NO_FIELD_NAMES_STYLE));
   }
-  
+
   // Needed by Hibernate
-  HandlingEvent() {}
+  HandlingEvent() {
+  }
 
 }

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

@@ -1,15 +1,14 @@
 package se.citerus.dddsample.domain;
 
+import org.apache.commons.lang.Validate;
+
 import javax.persistence.Entity;
 import javax.persistence.GeneratedValue;
 import javax.persistence.Id;
+import java.util.regex.Pattern;
 
 @Entity
 public class Location {
-  /**
-   * Special Location object that marks an unknown location.
-   */
-  public static final Location UNKNOWN = new Location("Unknown");
 
   @Id
   @GeneratedValue
@@ -17,7 +16,29 @@ public class Location {
 
   private String unlocode;
 
+  /**
+   * Regular expression of a UN Locode (exactly five letters of the english alphabet)
+   */
+  private static final Pattern unlocodePattern = Pattern.compile("[a-zA-Z]{5}");
+
+  /**
+   * Special Location object that marks an unknown location.
+   */
+  public static final Location UNKNOWN = new Location();
+
+  // Internal constructor
+  private Location() {
+    this.unlocode = "Unknown";
+  }
+
+  /**
+   * @param unlocode UN locode
+   * @throws IllegalArgumentException if the UN locode is anything other than five letters of the US alphabet
+   */
   public Location(String unlocode) {
+    Validate.notNull(unlocode);
+    Validate.isTrue(unlocodePattern.matcher(unlocode).matches(),
+            "\"" + unlocode + "\" is not a valid UN Locode");
     this.unlocode = unlocode;
   }
 
@@ -60,7 +81,4 @@ public class Location {
     return unlocode;
   }
 
-  // Needed by Hibernate
-  Location() {}
-
 }

dddsample/src/test/java/se/citerus/dddsample/domain/CargoTest.java

@@ -60,10 +60,10 @@ public class CargoTest extends TestCase {
   }
   
   public void testEquality() throws Exception {
-    Cargo c1 = new Cargo(new TrackingId("ABC"), new Location("A"), new Location("C"));
-    Cargo c2 = new Cargo(new TrackingId("CBA"), new Location("A"), new Location("C"));
-    Cargo c3 = new Cargo(new TrackingId("ABC"), new Location("A"), new Location("X"));
-    Cargo c4 = new Cargo(new TrackingId("ABC"), new Location("A"), new Location("C"));
+    Cargo c1 = new Cargo(new TrackingId("ABC"), new Location("AAAAA"), new Location("CCCCC"));
+    Cargo c2 = new Cargo(new TrackingId("CBA"), new Location("AAAAA"), new Location("CCCCC"));
+    Cargo c3 = new Cargo(new TrackingId("ABC"), new Location("AAAAA"), new Location("XXXXX"));
+    Cargo c4 = new Cargo(new TrackingId("ABC"), new Location("AAAAA"), new Location("CCCCC"));
 
     assertTrue("Cargos should be equal when TrackingIDs are equal", c1.equals(c4));
     assertTrue("Cargos should be equal when TrackingIDs are equal", c1.equals(c3));

dddsample/src/test/java/se/citerus/dddsample/domain/DeliveryHistoryTest.java

@@ -15,11 +15,11 @@ public class DeliveryHistoryTest extends TestCase {
     assertTrue(dh.eventsOrderedByCompletionTime().isEmpty());
 
     DateFormat df = new SimpleDateFormat("yyyy-MM-dd");
-    CarrierMovement carrierMovement = new CarrierMovement(new CarrierMovementId("CAR_001"), new Location("FROM"), new Location("TO"));
-    HandlingEvent he1 = new HandlingEvent(null, df.parse("2010-01-03"), new Date(), HandlingEvent.Type.RECEIVE, new Location("TO"));
-    HandlingEvent he2 = new HandlingEvent(null, df.parse("2010-01-01"), new Date(), HandlingEvent.Type.LOAD, new Location("TO"), carrierMovement);
-    HandlingEvent he3 = new HandlingEvent(null, df.parse("2010-01-04"), new Date(), HandlingEvent.Type.CLAIM, new Location("FROM"));
-    HandlingEvent he4 = new HandlingEvent(null, df.parse("2010-01-02"), new Date(), HandlingEvent.Type.UNLOAD, new Location("FROM"), carrierMovement);
+    CarrierMovement carrierMovement = new CarrierMovement(new CarrierMovementId("CAR_001"), new Location("FROMX"), new Location("TOXXX"));
+    HandlingEvent he1 = new HandlingEvent(null, df.parse("2010-01-03"), new Date(), HandlingEvent.Type.RECEIVE, new Location("TOXXX"));
+    HandlingEvent he2 = new HandlingEvent(null, df.parse("2010-01-01"), new Date(), HandlingEvent.Type.LOAD, new Location("TOXXX"), carrierMovement);
+    HandlingEvent he3 = new HandlingEvent(null, df.parse("2010-01-04"), new Date(), HandlingEvent.Type.CLAIM, new Location("FROMX"));
+    HandlingEvent he4 = new HandlingEvent(null, df.parse("2010-01-02"), new Date(), HandlingEvent.Type.UNLOAD, new Location("FROMX"), carrierMovement);
     dh.addAllEvents(Arrays.asList(he1, he2, he3, he4));
 
     List<HandlingEvent> orderEvents = dh.eventsOrderedByCompletionTime();

dddsample/src/test/java/se/citerus/dddsample/domain/HandlingEventTest.java

@@ -10,8 +10,8 @@ import java.util.Date;
 public class HandlingEventTest extends TestCase {
 
   public void testNewWithCarrierMovement() throws Exception {
-    Location origin = new Location("FROM");
-    Location finalDestination = new Location("TO");
+    Location origin = new Location("FROMX");
+    Location finalDestination = new Location("TOYYY");
     Cargo cargo = new Cargo(new TrackingId("XYZ"), origin, finalDestination);
     CarrierMovement carrierMovement = new CarrierMovement(new CarrierMovementId("C01"), origin, finalDestination);
 
@@ -20,30 +20,37 @@ public class HandlingEventTest extends TestCase {
 
     HandlingEvent e2 = new HandlingEvent(cargo, new Date(), new Date(), UNLOAD, finalDestination, carrierMovement);
     assertEquals(finalDestination, e2.location());
-    
+
+      // These event types prohibit a carrier movement association
     for (Type type : asList(CLAIM, RECEIVE, CUSTOMS)) {
       try {
         new HandlingEvent(cargo, new Date(), new Date(), type, origin, carrierMovement);
-        fail("Handling event with carrier movement and type " + type + " is not legal");
+        fail("Handling event type " + type + " prohibits carrier movement");
       } catch (IllegalArgumentException expected) {}
     }
+
+      // These event types requires a carrier movement association
+    for (Type type : asList(LOAD, UNLOAD)) {
+        try {
+          new HandlingEvent(cargo, new Date(), new Date(), type, origin);
+            fail("Handling event type " + type + " requires carrier movement");
+        } catch (IllegalArgumentException expected) {}
+    }
   }
 
   public void testNewWithLocation() throws Exception {
-    Location origin = new Location("FROM");
-    Location finalDestination = new Location("TO");
+    Location origin = new Location("FROMX");
+    Location finalDestination = new Location("TOYYY");
     Cargo cargo = new Cargo(new TrackingId("XYZ"), origin, finalDestination);
 
-    Location location = new Location("FOO");
+    Location location = new Location("FOOOO");
     HandlingEvent e1 = new HandlingEvent(cargo, new Date(), new Date(), Type.CLAIM, location);
     assertEquals(location, e1.location());
-
-    
   }
 
   public void testCurrentLocationLoadEvent() throws Exception {
-    Location locationAAA = new Location("AAA");
-    Location locationBBB = new Location("BBB");
+    Location locationAAA = new Location("AAAAA");
+    Location locationBBB = new Location("BBBBB");
     CarrierMovementId carrierMovementId = new CarrierMovementId("CAR_001");
     CarrierMovement cm = new CarrierMovement(carrierMovementId, locationAAA, locationBBB);
     
@@ -53,8 +60,8 @@ public class HandlingEventTest extends TestCase {
   }
   
   public void testCurrentLocationUnloadEvent() throws Exception {
-    Location locationAAA = new Location("AAA");
-    Location locationBBB = new Location("BBB");
+    Location locationAAA = new Location("AAAAA");
+    Location locationBBB = new Location("BBBBB");
     CarrierMovementId carrierMovementId = new CarrierMovementId("CAR_001");
     CarrierMovement cm = new CarrierMovement(carrierMovementId, locationAAA, locationBBB);
     
@@ -64,14 +71,14 @@ public class HandlingEventTest extends TestCase {
   }
   
   public void testCurrentLocationReceivedEvent() throws Exception {
-    HandlingEvent ev = new HandlingEvent(null, new Date(), new Date(), RECEIVE, new Location("TEST"));
+    HandlingEvent ev = new HandlingEvent(null, new Date(), new Date(), RECEIVE, new Location("ATEST"));
 
-    assertEquals(new Location("TEST"), ev.location());
+    assertEquals(new Location("ATEST"), ev.location());
   }
   public void testCurrentLocationClaimedEvent() throws Exception {
-    HandlingEvent ev = new HandlingEvent(null, new Date(), new Date(), CLAIM, new Location("TEST"));
+    HandlingEvent ev = new HandlingEvent(null, new Date(), new Date(), CLAIM, new Location("ATEST"));
 
-    assertEquals(new Location("TEST"), ev.location());
+    assertEquals(new Location("ATEST"), ev.location());
   }
   
   public void testParseType() throws Exception {
@@ -93,8 +100,8 @@ public class HandlingEventTest extends TestCase {
   public void testEqualsAndSameAs() throws Exception {
     Date timeOccured = new Date();
     Date timeRegistered = new Date();
-    Location locationAAA = new Location("AAA");
-    Location locationBBB = new Location("BBB");
+    Location locationAAA = new Location("AAAAA");
+    Location locationBBB = new Location("BBBBB");
     CarrierMovementId carrierMovementId = new CarrierMovementId("CAR_001");
     CarrierMovement cm = new CarrierMovement(carrierMovementId, locationAAA, locationBBB);
 

dddsample/src/test/java/se/citerus/dddsample/domain/LocationTest.java

@@ -2,17 +2,20 @@ package se.citerus.dddsample.domain;
 
 import junit.framework.TestCase;
 
+import java.util.Arrays;
+import java.util.List;
+
 public class LocationTest extends TestCase {
 
   public void testEquals() {
     // Same location string - equal
-    assertTrue(new Location("TEST").equals(new Location("TEST")));
+    assertTrue(new Location("ATEST").equals(new Location("ATEST")));
 
     // Different location strings - not equal
-    assertFalse(new Location("TEST").equals(new Location("ANOTHER_TEST")));
+    assertFalse(new Location("ATEST").equals(new Location("TESTB")));
 
     // Always equal to itself
-    Location location = new Location("TEST");
+    Location location = new Location("ATEST");
     assertTrue(location.equals(location));
 
     // Never equal to null
@@ -21,11 +24,17 @@ public class LocationTest extends TestCase {
     // Special NULL location is equal to itself
     assertTrue(Location.UNKNOWN.equals(Location.UNKNOWN));
 
-    // No other location should be equal to the NULL location
-    assertFalse(new Location(null).equals(Location.UNKNOWN));
-    assertFalse(new Location("").equals(Location.UNKNOWN));
-    assertFalse(new Location("   ").equals(Location.UNKNOWN));
-    assertFalse(new Location("   FOO  BAR ").equals(Location.UNKNOWN));
+    // These are all invalid UN locodes
+    List<String> invalidUnlocodes = Arrays.asList(null, "", "   ", "SHRT", "LOOONG", "WhAt evR 1 12 !!#6/");
+    for (String invalid : invalidUnlocodes) {
+      failInvalidUnlocode(invalid);
+    }
   }
 
+  private void failInvalidUnlocode(String invalid) {
+    try {
+      new Location(invalid);
+      fail(invalid + " should not be allowed as UN locode constructor argument");
+    } catch (IllegalArgumentException expected) {}
+  }
 }

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

@@ -29,8 +29,8 @@ public class CargoRepositoryTest extends AbstractRepositoryTest {
 
   public void testSave() {
     // TODO: introduce Location repository
-    Location finalDestination = new Location("TO");
-    Location origin = new Location("FROM");
+    Location finalDestination = new Location("TOZZZ");
+    Location origin = new Location("FROMZ");
     sessionFactory.getCurrentSession().saveOrUpdate(origin);
     sessionFactory.getCurrentSession().saveOrUpdate(finalDestination);
 

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

@@ -23,7 +23,7 @@ public class HandlingEventRepositoryTest extends AbstractRepositoryTest {
 
   public void testSave() {
     // TODO: introduce Location repository
-    Location location = new Location("ABC");
+    Location location = new Location("ABCDE");
     sessionFactory.getCurrentSession().saveOrUpdate(location);
 
     Cargo cargo = cargoRepository.find(new TrackingId("XYZ"));

dddsample/src/test/java/se/citerus/dddsample/service/CargoServiceTest.java

@@ -1,8 +1,6 @@
 package se.citerus.dddsample.service;
 
 import static org.easymock.EasyMock.*;
-
-import org.apache.commons.lang.ArrayUtils;
 import org.easymock.IAnswer;
 import org.hibernate.FlushMode;
 import org.hibernate.SessionFactory;
@@ -78,7 +76,7 @@ public class CargoServiceTest extends AbstractDependencyInjectionSpringContextTe
    * Cargo returned.
    */
   public void testCargoServiceFindByTrackingIdScenario() {
-    final Cargo cargo = new Cargo(new TrackingId("XYZ"), new Location("ORIG"), new Location("DEST"));
+    final Cargo cargo = new Cargo(new TrackingId("XYZ"), new Location("ORIGI"), new Location("DESTI"));
     HandlingEvent claimed = new HandlingEvent(cargo, new Date(10), new Date(20), HandlingEvent.Type.CLAIM, new Location("SESTO"));
     CarrierMovement carrierMovement = new CarrierMovement(new CarrierMovementId("CAR_001"), new Location("SESTO"), new Location("MUGER"));
     HandlingEvent loaded = new HandlingEvent(cargo, new Date(12), new Date(25), HandlingEvent.Type.LOAD, new Location("SESTO"), carrierMovement);
@@ -101,8 +99,8 @@ public class CargoServiceTest extends AbstractDependencyInjectionSpringContextTe
 
 
     assertEquals("XYZ", cargoDTO.getTrackingId());
-    assertEquals("ORIG", cargoDTO.getOrigin());
-    assertEquals("DEST", cargoDTO.getFinalDestination());
+    assertEquals("ORIGI", cargoDTO.getOrigin());
+    assertEquals("DESTI", cargoDTO.getFinalDestination());
     assertEquals("MUGER", cargoDTO.getCurrentLocation());
 
     List<HandlingEventDTO> events = cargoDTO.getEvents();

dddsample/src/test/java/se/citerus/dddsample/web/CargoTrackingControllerTest.java

@@ -43,8 +43,8 @@ public class CargoTrackingControllerTest extends TestCase {
   private CargoService getCargoServiceMock() {
     return new CargoService() {
       public CargoWithHistoryDTO find(String trackingId) {
-        Cargo cargo = new Cargo(new TrackingId(trackingId), new Location("AAA"), new Location("BBB"));
-        HandlingEvent event = new HandlingEvent(cargo, new Date(10L), new Date(20L), HandlingEvent.Type.RECEIVE, new Location("BBB"));
+        Cargo cargo = new Cargo(new TrackingId(trackingId), new Location("AAAAA"), new Location("BBBBB"));
+        HandlingEvent event = new HandlingEvent(cargo, new Date(10L), new Date(20L), HandlingEvent.Type.RECEIVE, new Location("BBBBB"));
 //        cargo.handle(event);
 
         // TODO: use DTO assemblers
@@ -52,7 +52,7 @@ public class CargoTrackingControllerTest extends TestCase {
                 cargo.trackingId().idString(),
                 cargo.origin().unlocode(),
                 cargo.finalDestination().unlocode(),
-                "AAA"
+                "AAAAA"
         );
         cargoDTO.addEvent(new HandlingEventDTO(
           event.location().unlocode(),
@@ -93,7 +93,7 @@ public class CargoTrackingControllerTest extends TestCase {
     // Errors, command are two standard map attributes, the third should be the cargo object
     assertEquals(3, mav.getModel().size());
     CargoWithHistoryDTO cargo = (CargoWithHistoryDTO) mav.getModel().get("cargo");
-    assertEquals("AAA", cargo.getCurrentLocation());
+    assertEquals("AAAAA", cargo.getCurrentLocation());
   }
 
   public void testUnknownCargo() throws Exception {