Fixed up tag immutability, values, made timestamp comparable and consistency issues with owner.

Author: wolpert

common/src/main/java/com/codeheadsystems/motif/model/Event.java

@@ -20,7 +20,10 @@ public record Event(Owner owner,
     }
     identifier = Objects.requireNonNullElseGet(identifier, Identifier::new);
     timestamp = Objects.requireNonNullElse(timestamp, new Timestamp());
-    tags = Objects.requireNonNullElseGet(tags, List::of);
+    tags = tags == null ? List.of() : List.copyOf(tags);
+    if (!owner.equals(subject.owner())) {
+      throw new IllegalArgumentException("owner must match subject's owner");
+    }
   }
 
   public static Builder from(Event event) {

common/src/main/java/com/codeheadsystems/motif/model/Note.java

@@ -6,7 +6,7 @@
 
 public record Note(Owner owner,
                    Subject subject,
-                   @Nullable String value,
+                   String value,
                    @Nullable List<Tag> tags,
                    @Nullable Identifier identifier,
                    @Nullable Event event,
@@ -15,11 +15,17 @@ public record Note(Owner owner,
   public Note {
     Objects.requireNonNull(owner, "owner cannot be null");
     Objects.requireNonNull(subject, "subject cannot be null");
-    value = Objects.requireNonNullElse(value, "").strip();
+    value = Objects.requireNonNull(value, "value cannot be null").strip();
+    if (value.isEmpty()) {
+      throw new IllegalArgumentException("value cannot be empty");
+    }
     if (value.length() > 4096) {
       throw new IllegalArgumentException("value cannot be longer than 4096 characters");
     }
-    tags = Objects.requireNonNullElseGet(tags, List::of);
+    tags = tags == null ? List.of() : List.copyOf(tags);
+    if (!owner.equals(subject.owner())) {
+      throw new IllegalArgumentException("owner must match subject's owner");
+    }
     identifier = Objects.requireNonNullElseGet(identifier, Identifier::new);
     timestamp = Objects.requireNonNullElse(timestamp, new Timestamp());
   }
@@ -28,40 +34,35 @@ public static Builder from(Note note) {
     return Builder.from(note);
   }
 
-  public static Builder builder(Owner owner, Subject subject) {
-    return new Builder(owner, subject);
+  public static Builder builder(Owner owner, Subject subject, String value) {
+    return new Builder(owner, subject, value);
   }
 
   public static class Builder {
     private final Owner owner;
     private final Subject subject;
-    private String value;
+    private final String value;
     private List<Tag> tags;
     private Identifier identifier;
     private Event event;
     private Timestamp timestamp;
 
-    private Builder(Owner owner, Subject subject) {
+    private Builder(Owner owner, Subject subject, String value) {
       this.owner = Objects.requireNonNull(owner, "owner cannot be null");
       this.subject = Objects.requireNonNull(subject, "subject cannot be null");
+      this.value = Objects.requireNonNull(value, "value cannot be null");
     }
 
     private static Builder from(Note note) {
       Objects.requireNonNull(note, "note cannot be null");
-      Builder builder = new Builder(note.owner(), note.subject());
-      builder.value = note.value();
+      Builder builder = new Builder(note.owner(), note.subject(), note.value());
       builder.tags = note.tags();
       builder.identifier = note.identifier();
       builder.event = note.event();
       builder.timestamp = note.timestamp();
       return builder;
     }
 
-    public Builder value(String value) {
-      this.value = value;
-      return this;
-    }
-
     public Builder tags(List<Tag> tags) {
       this.tags = tags;
       return this;

common/src/main/java/com/codeheadsystems/motif/model/Timestamp.java

@@ -12,7 +12,7 @@
  *
  * @param timestamp The instant.
  */
-public record Timestamp(Instant timestamp) {
+public record Timestamp(Instant timestamp) implements Comparable<Timestamp> {
 
   public Timestamp {
     if (timestamp == null) {
@@ -53,4 +53,9 @@ public OffsetDateTime toOffsetDateTime() {
     return timestamp.atOffset(ZoneOffset.UTC);
   }
 
+  @Override
+  public int compareTo(Timestamp other) {
+    return this.timestamp.compareTo(other.timestamp);
+  }
+
 }

common/src/test/java/com/codeheadsystems/motif/model/EventTest.java

@@ -4,6 +4,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 
@@ -112,6 +113,24 @@ void constructorPreservesProvidedTags() {
     assertThat(event.tags()).containsExactly(new Tag("A"), new Tag("B"));
   }
 
+  @Test
+  void constructorMakesDefensiveCopyOfTags() {
+    ArrayList<Tag> mutableTags = new ArrayList<>(List.of(new Tag("A")));
+
+    Event event = new Event(OWNER, SUBJECT, "value", null, null, mutableTags);
+    mutableTags.add(new Tag("B"));
+
+    assertThat(event.tags()).containsExactly(new Tag("A"));
+  }
+
+  @Test
+  void constructorRejectsOwnerMismatchWithSubject() {
+    Owner differentOwner = new Owner("DIFFERENT");
+    assertThatThrownBy(() -> new Event(differentOwner, SUBJECT, "value", null, null, null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("owner must match subject's owner");
+  }
+
   // --- Builder.from tests ---
 
   @Test

common/src/test/java/com/codeheadsystems/motif/model/NoteTest.java

@@ -4,6 +4,7 @@
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.List;
 import org.junit.jupiter.api.Test;
 
@@ -30,10 +31,24 @@ void constructorRejectsNullSubject() {
   }
 
   @Test
-  void constructorDefaultsNullValueToEmpty() {
-    Note note = new Note(OWNER, SUBJECT, null, null, null, null, null);
+  void constructorRejectsNullValue() {
+    assertThatThrownBy(() -> new Note(OWNER, SUBJECT, null, null, null, null, null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("value cannot be null");
+  }
+
+  @Test
+  void constructorRejectsEmptyValue() {
+    assertThatThrownBy(() -> new Note(OWNER, SUBJECT, "", null, null, null, null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("value cannot be empty");
+  }
 
-    assertThat(note.value()).isEmpty();
+  @Test
+  void constructorRejectsBlankValue() {
+    assertThatThrownBy(() -> new Note(OWNER, SUBJECT, "   ", null, null, null, null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("value cannot be empty");
   }
 
   @Test
@@ -77,6 +92,16 @@ void constructorPreservesProvidedTags() {
     assertThat(note.tags()).containsExactly(new Tag("A"), new Tag("B"));
   }
 
+  @Test
+  void constructorMakesDefensiveCopyOfTags() {
+    ArrayList<Tag> mutableTags = new ArrayList<>(List.of(new Tag("A")));
+
+    Note note = new Note(OWNER, SUBJECT, "value", mutableTags, null, null, null);
+    mutableTags.add(new Tag("B"));
+
+    assertThat(note.tags()).containsExactly(new Tag("A"));
+  }
+
   @Test
   void constructorDefaultsIdentifierWhenNull() {
     Note note = new Note(OWNER, SUBJECT, "value", null, null, null, null);
@@ -129,6 +154,14 @@ void constructorPreservesProvidedEvent() {
     assertThat(note.event()).isSameAs(event);
   }
 
+  @Test
+  void constructorRejectsOwnerMismatchWithSubject() {
+    Owner differentOwner = new Owner("DIFFERENT");
+    assertThatThrownBy(() -> new Note(differentOwner, SUBJECT, "value", null, null, null, null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("owner must match subject's owner");
+  }
+
   // --- Builder.from tests ---
 
   @Test
@@ -153,48 +186,47 @@ void fromCopiesAllFields() {
 
   @Test
   void fromAllowsOverridingFields() {
-    Note original = Note.builder(OWNER, SUBJECT).value("original").build();
+    Note original = Note.builder(OWNER, SUBJECT, "original").build();
 
     Event event = Event.builder(OWNER, SUBJECT, "event-value").build();
     Note modified = Note.from(original)
         .event(event)
-        .value("modified")
         .build();
 
     assertThat(modified.owner()).isEqualTo(original.owner());
     assertThat(modified.subject()).isEqualTo(original.subject());
     assertThat(modified.identifier()).isEqualTo(original.identifier());
     assertThat(modified.timestamp()).isEqualTo(original.timestamp());
-    assertThat(modified.value()).isEqualTo("modified");
+    assertThat(modified.value()).isEqualTo("original");
     assertThat(modified.event()).isSameAs(event);
   }
 
   // --- Builder tests ---
 
   @Test
   void builderRejectsNullOwner() {
-    assertThatThrownBy(() -> Note.builder(null, SUBJECT))
+    assertThatThrownBy(() -> Note.builder(null, SUBJECT, "value"))
         .isInstanceOf(NullPointerException.class)
         .hasMessage("owner cannot be null");
   }
 
   @Test
   void builderRejectsNullSubject() {
-    assertThatThrownBy(() -> Note.builder(OWNER, null))
+    assertThatThrownBy(() -> Note.builder(OWNER, null, "value"))
         .isInstanceOf(NullPointerException.class)
         .hasMessage("subject cannot be null");
   }
 
   @Test
-  void builderDefaultsNullValueToEmpty() {
-    Note note = Note.builder(OWNER, SUBJECT).build();
-
-    assertThat(note.value()).isEmpty();
+  void builderRejectsNullValue() {
+    assertThatThrownBy(() -> Note.builder(OWNER, SUBJECT, null))
+        .isInstanceOf(NullPointerException.class)
+        .hasMessage("value cannot be null");
   }
 
   @Test
   void builderWithDefaults() {
-    Note note = Note.builder(OWNER, SUBJECT).value("note-value").build();
+    Note note = Note.builder(OWNER, SUBJECT, "note-value").build();
 
     assertThat(note.owner()).isEqualTo(OWNER);
     assertThat(note.subject()).isEqualTo(SUBJECT);
@@ -212,8 +244,7 @@ void builderWithAllFields() {
     List<Tag> tags = List.of(new Tag("X"));
     Event event = Event.builder(OWNER, SUBJECT, "event-value").build();
 
-    Note note = Note.builder(OWNER, SUBJECT)
-        .value("note-value")
+    Note note = Note.builder(OWNER, SUBJECT, "note-value")
         .tags(tags)
         .identifier(id)
         .event(event)

common/src/test/java/com/codeheadsystems/motif/model/TimestampTest.java

@@ -62,4 +62,14 @@ void fromIsoHandlesOffsetFormat() {
 
     assertThat(timestamp.timestamp()).isEqualTo(Instant.parse("2026-03-28T12:30:00Z"));
   }
+
+  @Test
+  void compareToOrdersByInstant() {
+    Timestamp earlier = new Timestamp(Instant.parse("2026-01-01T00:00:00Z"));
+    Timestamp later = new Timestamp(Instant.parse("2026-06-01T00:00:00Z"));
+
+    assertThat(earlier).isLessThan(later);
+    assertThat(later).isGreaterThan(earlier);
+    assertThat(earlier.compareTo(new Timestamp(earlier.timestamp()))).isZero();
+  }
 }