refactor(track): re-design habit tracking entity

Logic is now encapsulated in an entity that contains all tracking dates instead of leaking into the controller layer. An entity now contains all tracked dates for a given habit instead of one entity per tracked date.
codecentric · Nov 16, 2023 · 8e1ac1c · 8e1ac1c
1 parent ab3ecde
commit 8e1ac1c
Show file tree

Hide file tree

Showing 11 changed files with 174 additions and 129 deletions.
diff --git a/.../src/intTest/java/de/codecentric/habitcentric/track/habit/HabitModuleIntegrationTest.java b/.../src/intTest/java/de/codecentric/habitcentric/track/habit/HabitModuleIntegrationTest.java
@@ -28,24 +28,30 @@ void tearDown() {
 
   @Test
   void shouldPublishDateTrackedEventWhenHabitTrackingIsSaved(Scenario scenario) {
+    habitTrackingRepository.save(
+            HabitTracking.from("userId", 1L, Set.of(LocalDate.parse("2023-09-29"))));
+
     scenario
         .stimulate(
             () ->
                 habitTrackingController.putHabitTrackingRecords(
-                    "userId", 1L, Set.of(LocalDate.parse("2023-09-29"))))
+                    "userId",
+                    1L,
+                    Set.of(LocalDate.parse("2023-09-29"), LocalDate.parse("2023-09-30"))))
         .andWaitForEventOfType(HabitTracking.DateTracked.class)
         .toArriveAndVerify(
             event -> {
               assertThat(event.habitId()).isEqualTo(1L);
               assertThat(event.userId()).isEqualTo("userId");
-              assertThat(event.trackDate()).isEqualTo(LocalDate.parse("2023-09-29"));
+              assertThat(event.trackDate()).isEqualTo(LocalDate.parse("2023-09-30"));
             });
   }
 
   @Test
   void shouldPublishDateUntrackedEventWhenExistingHabitTrackingIsRemoved(Scenario scenario) {
-    habitTrackingController.putHabitTrackingRecords(
-        "userId", 1L, Set.of(LocalDate.parse("2023-09-29"), LocalDate.parse("2023-09-30")));
+    habitTrackingRepository.save(
+            HabitTracking.from(
+            "userId", 1L, Set.of(LocalDate.parse("2023-09-29"), LocalDate.parse("2023-09-30"))));
 
     scenario
         .stimulate(

diff --git a/.../java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerRestAssuredTest.java b/.../java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerRestAssuredTest.java
@@ -30,7 +30,7 @@ public class HabitTrackingControllerRestAssuredTest extends RestAssuredTest {
 
   @AfterEach
   public void cleanUp() {
-    String[] tableNames = {"HC_TRACK.HABIT_TRACKING"};
+    String[] tableNames = {"HC_TRACK.TRACKED_DATES", "HC_TRACK.HABIT_TRACKINGS"};
     JdbcTestUtils.deleteFromTables(jdbcTemplate, tableNames);
   }
 

diff --git a/...e/codecentric/habitcentric/track/habit/jwt/HabitTrackingControllerJwtRestAssuredTest.java b/...e/codecentric/habitcentric/track/habit/jwt/HabitTrackingControllerJwtRestAssuredTest.java
@@ -29,7 +29,7 @@ public class HabitTrackingControllerJwtRestAssuredTest extends RestAssuredTest {
 
   @AfterEach
   public void cleanUp() {
-    String[] tableNames = {"HC_TRACK.HABIT_TRACKING"};
+    String[] tableNames = {"HC_TRACK.TRACKED_DATES", "HC_TRACK.HABIT_TRACKINGS"};
     JdbcTestUtils.deleteFromTables(jdbcTemplate, tableNames);
   }
 

diff --git a/services/track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTracking.java b/services/track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTracking.java
@@ -1,8 +1,13 @@
 package de.codecentric.habitcentric.track.habit;
 
+import jakarta.persistence.CollectionTable;
+import jakarta.persistence.Column;
+import jakarta.persistence.ElementCollection;
 import jakarta.persistence.Embeddable;
 import jakarta.persistence.EmbeddedId;
 import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
+import jakarta.persistence.JoinColumn;
 import jakarta.persistence.Table;
 import jakarta.validation.Valid;
 import jakarta.validation.constraints.NotBlank;
@@ -11,6 +16,11 @@
 import jakarta.validation.constraints.Size;
 import java.io.Serializable;
 import java.time.LocalDate;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
 import lombok.Getter;
@@ -32,14 +42,54 @@ public class HabitTracking extends AbstractAggregateRoot<HabitTracking> {
 
   @EmbeddedId @Valid private Id id;
 
-  public HabitTracking(String userId, Long habitId, LocalDate trackDate) {
-    this.id = new HabitTracking.Id(userId, habitId, trackDate);
+  @ElementCollection(fetch = FetchType.EAGER)
+  @CollectionTable(name = "tracked_dates", joinColumns = {
+          @JoinColumn(name = "habit_id"),
+          @JoinColumn(name = "user_id")
+  })
+  @Column(name = "tracking_date")
+  private Set<LocalDate> trackings;
+
+  public void track(Set<LocalDate> dates) {
+    var untrackedDates = calculateUntrackedDates(dates);
+    var trackedDates = calculateTrackedDates(dates);
+
+    untrackedDates.forEach(this::registerUntrackedEvent);
+    trackedDates.forEach(this::registerTrackedEvent);
+
+    trackings = new HashSet<>(dates);
+  }
+
+  private Set<LocalDate> calculateUntrackedDates(Set<LocalDate> dates) {
+    Set<LocalDate> copy = new HashSet<>(trackings);
+    copy.removeAll(dates);
+    return copy;
+  }
+
+  private Set<LocalDate> calculateTrackedDates(Set<LocalDate> dates) {
+    Set<LocalDate> copy = new HashSet<>(dates);
+    copy.removeAll(trackings);
+    return copy;
+  }
 
-    registerEvent(new DateTracked(userId, habitId, trackDate));
+  private void registerTrackedEvent(LocalDate date) {
+    registerEvent(new DateTracked(id.userId, id.habitId, date));
   }
 
-  public void untrack() {
-    registerEvent(new DateUntracked(id.userId, id.habitId, id.trackDate));
+  private void registerUntrackedEvent(LocalDate date) {
+    registerEvent(new DateUntracked(id.userId, id.habitId, date));
+  }
+
+  public List<LocalDate> getSortedTrackingDates() {
+    return trackings.stream().sorted().toList();
+  }
+
+  public static HabitTracking from(String userId, Long habitId) {
+    return new HabitTracking(new Id(userId, habitId), new HashSet<>());
+  }
+
+  public static HabitTracking from(String userId, Long habitId, Collection<LocalDate> trackings) {
+    return new HabitTracking(new Id(userId, habitId), new HashSet<>(trackings));
   }
 
   @Embeddable
@@ -50,14 +100,11 @@ public void untrack() {
   @EqualsAndHashCode
   @ToString
   public static class Id implements Serializable {
-
     @NotBlank
     @Size(max = 64)
     private String userId;
 
     @NotNull @Positive private Long habitId;
-
-    @NotNull private LocalDate trackDate;
   }
 
   @Externalized("habit-tracking-events::#{#this.getId()}")

diff --git a/.../track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTrackingController.java b/.../track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTrackingController.java
@@ -5,9 +5,8 @@
 import jakarta.transaction.Transactional;
 import java.time.LocalDate;
 import java.util.Collection;
-import java.util.List;
+import java.util.Collections;
 import java.util.Set;
-import java.util.stream.Collectors;
 import org.springframework.stereotype.Controller;
 import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.GetMapping;
@@ -43,38 +42,16 @@ public Collection<LocalDate> putHabitTrackingRecords(
       @PathVariable @UserId String userId,
       @PathVariable @HabitId Long habitId,
       @RequestBody Set<LocalDate> dates) {
-    var existingHabitTrackings = repository.findByIdUserIdAndIdHabitId(userId, habitId);
+    var existingHabitTrackings =
+        repository
+            .findByIdUserIdAndIdHabitId(userId, habitId)
+            .orElse(HabitTracking.from(userId, habitId));
 
-    untrackRemovedTrackingDates(dates, existingHabitTrackings);
-    trackNewTrackingDates(userId, habitId, dates, existingHabitTrackings);
+    existingHabitTrackings.track(dates);
 
-    return dates.stream().sorted().toList();
-  }
+    repository.save(existingHabitTrackings);
 
-  private void untrackRemovedTrackingDates(
-      Set<LocalDate> dates, List<HabitTracking> existingHabitTrackings) {
-    existingHabitTrackings.stream()
-        .filter(ht -> !dates.contains(ht.getId().getTrackDate()))
-        .forEach(
-            ht -> {
-              ht.untrack();
-              repository.delete(ht);
-            });
-  }
-
-  private void trackNewTrackingDates(String userId, Long habitId, Set<LocalDate> dates, List<HabitTracking> existingHabitTrackings) {
-    var existingTrackDates =
-            existingHabitTrackings.stream()
-                                  .map(ht -> ht.getId().getTrackDate())
-                                  .collect(Collectors.toSet());
-
-    var newHabitTrackings =
-            dates.stream()
-                 .filter(date -> !existingTrackDates.contains(date))
-                 .map(date -> new HabitTracking(userId, habitId, date))
-                 .toList();
-
-    repository.saveAll(newHabitTrackings);
+    return existingHabitTrackings.getSortedTrackingDates();
   }
 
   @GetMapping("/track/habits/{habitId}")
@@ -88,13 +65,9 @@ public Iterable<LocalDate> getHabitTrackingRecordsWithJwt(
   @ResponseBody
   public Iterable<LocalDate> getHabitTrackingRecords(
       @PathVariable @UserId String userId, @PathVariable @HabitId Long habitId) {
-    return extractDates(repository.findByIdUserIdAndIdHabitId(userId, habitId));
-  }
-
-  protected List<LocalDate> extractDates(List<HabitTracking> trackRecords) {
-    return trackRecords.stream()
-        .map(tracking -> tracking.getId().getTrackDate())
-        .sorted()
-        .collect(Collectors.toList());
+    return repository
+        .findByIdUserIdAndIdHabitId(userId, habitId)
+        .map(HabitTracking::getSortedTrackingDates)
+        .orElse(Collections.emptyList());
   }
 }
diff --git a/.../track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTrackingRepository.java b/.../track/src/main/java/de/codecentric/habitcentric/track/habit/HabitTrackingRepository.java
@@ -1,12 +1,9 @@
 package de.codecentric.habitcentric.track.habit;
 
-import de.codecentric.habitcentric.track.habit.HabitTracking.Id;
-import java.util.List;
 import org.springframework.data.jpa.repository.JpaRepository;
 
-public interface HabitTrackingRepository extends JpaRepository<HabitTracking, Id> {
+import java.util.Optional;
 
-  List<HabitTracking> findByIdUserIdAndIdHabitId(String userId, Long habitId);
-
-  void deleteByIdHabitId(Long habitId);
+public interface HabitTrackingRepository extends JpaRepository<HabitTracking, HabitTracking.Id> {
+    Optional<HabitTracking> findByIdUserIdAndIdHabitId(String userId, Long habitId);
 }
diff --git a/services/track/src/main/resources/db/migration/V1__create-schema.sql b/services/track/src/main/resources/db/migration/V1__create-schema.sql
@@ -1,2 +1,5 @@
-CREATE TABLE habit_tracking (user_id VARCHAR(64) NOT NULL, habit_id BIGINT NOT NULL, track_date DATE NOT NULL);
-ALTER TABLE habit_tracking ADD PRIMARY KEY (user_id, habit_id, track_date);
+CREATE TABLE habit_tracking (user_id VARCHAR(64) NOT NULL, habit_id BIGINT NOT NULL);
+ALTER TABLE habit_tracking ADD PRIMARY KEY (user_id, habit_id);
+
+CREATE TABLE tracked_dates(tracking_date DATE NOT NULL, user_id VARCHAR(64) NOT NULL, habit_id BIGINT NOT NULL);
+ALTER TABLE tracked_dates ADD FOREIGN KEY (user_id, habit_id) REFERENCES habit_tracking(user_id, habit_id);
diff --git a/...ck/src/test/java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerTest.java b/...ck/src/test/java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerTest.java
diff --git a/.../test/java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerWebMvcTest.java b/.../test/java/de/codecentric/habitcentric/track/habit/HabitTrackingControllerWebMvcTest.java
@@ -2,7 +2,7 @@
 
 import static java.time.Month.DECEMBER;
 import static java.time.Month.JANUARY;
-import static org.mockito.ArgumentMatchers.anyIterable;
+import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
 import static org.springframework.http.MediaType.APPLICATION_JSON;
 import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
@@ -11,9 +11,9 @@
 import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
 
 import java.time.LocalDate;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -31,11 +31,14 @@ public class HabitTrackingControllerWebMvcTest {
   private final String userId = "abc.def";
   private final Long habitId = 123L;
 
-  private final List<HabitTracking> defaultTrackRecords =
-      Arrays.asList(
-          new HabitTracking(userId, habitId, LocalDate.of(2019, JANUARY, 31)),
-          new HabitTracking(userId, habitId, LocalDate.of(2018, DECEMBER, 31)),
-          new HabitTracking(userId, habitId, LocalDate.of(2019, JANUARY, 1)));
+  private final HabitTracking defaultTrackRecords =
+      HabitTracking.from(
+          userId,
+          habitId,
+          Set.of(
+              LocalDate.of(2019, JANUARY, 31),
+              LocalDate.of(2018, DECEMBER, 31),
+              LocalDate.of(2019, JANUARY, 1)));
 
   private final String expected = "[\"2018-12-31\",\"2019-01-01\",\"2019-01-31\"]";
 
@@ -47,7 +50,7 @@ public class HabitTrackingControllerWebMvcTest {
 
   @Test
   public void shouldReturnTrackRecords() throws Exception {
-    given(repository.findByIdUserIdAndIdHabitId(userId, habitId)).willReturn(defaultTrackRecords);
+    given(repository.findByIdUserIdAndIdHabitId(userId, habitId)).willReturn(Optional.of(defaultTrackRecords));
     mockMvc
         .perform(get(urlTemplate, userId, habitId))
         .andExpect(status().isOk())
@@ -56,7 +59,7 @@ public void shouldReturnTrackRecords() throws Exception {
 
   @Test
   public void shouldReturnEmptyArrayWhenTrackRecordsAreNotFound() throws Exception {
-    given(repository.findByIdUserIdAndIdHabitId(userId, habitId)).willReturn(new ArrayList<>());
+    given(repository.findByIdUserIdAndIdHabitId(userId, habitId)).willReturn(Optional.of(HabitTracking.from(userId, habitId)));
     mockMvc
         .perform(get(urlTemplate, userId, habitId))
         .andExpect(status().isOk())
@@ -65,7 +68,8 @@ public void shouldReturnEmptyArrayWhenTrackRecordsAreNotFound() throws Exception
 
   @Test
   public void shouldFilterOutDuplicateTrackRecords() throws Exception {
-    given(repository.saveAll(anyIterable())).willReturn(defaultTrackRecords);
+    given(repository.findByIdUserIdAndIdHabitId(userId, habitId)).willReturn(Optional.of(defaultTrackRecords));
+    given(repository.save(any())).willReturn(defaultTrackRecords);
     mockMvc
         .perform(
             put(urlTemplate, userId, habitId)